| Message ID | 20200408100354.17782-1-aishwaryarj100@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | i2c: powermac: Simplify reading the "reg" and "i2c-address" property | expand |
On Wed, Apr 08, 2020 at 03:33:53PM +0530, Aishwarya R wrote: > Use of_property_read_u32 to read the "reg" and "i2c-address" property > instead of using of_get_property to check the return values. > > Signed-off-by: Aishwarya R <aishwaryarj100@gmail.com> This is quite a fragile driver. Have you tested it on HW?
>> Use of_property_read_u32 to read the "reg" and "i2c-address" property >> instead of using of_get_property to check the return values. >> >> Signed-off-by: Aishwarya R <aishwaryarj100@gmail.com> > This is quite a fragile driver. Have you tested it on HW? This change is not tested with the Hardware. But of_property_read_u32 is better here than generic of_get_property. This make sure that value read properly independent of system endianess.
On Wed, Apr 15, 2020 at 06:49:14PM +0530, Aishwarya R wrote: > >> Use of_property_read_u32 to read the "reg" and "i2c-address" property > >> instead of using of_get_property to check the return values. > >> > >> Signed-off-by: Aishwarya R <aishwaryarj100@gmail.com> > > > This is quite a fragile driver. Have you tested it on HW? > > This change is not tested with the Hardware. > But of_property_read_u32 is better here than generic of_get_property. > This make sure that value read properly independent of system endianess. This driver is only used on PPC_BE. And it is *very* fragile. The gain is not enough for me to accept it without testing. Maybe Erhard (CCed) is interested. If not, you may find someone on the ppc lists.
On Tue, 21 Apr 2020 11:37:13 +0200 Wolfram Sang <wsa@the-dreams.de> wrote: > On Wed, Apr 15, 2020 at 06:49:14PM +0530, Aishwarya R wrote: > > >> Use of_property_read_u32 to read the "reg" and "i2c-address" property > > >> instead of using of_get_property to check the return values. > > >> > > >> Signed-off-by: Aishwarya R <aishwaryarj100@gmail.com> > > > > > This is quite a fragile driver. Have you tested it on HW? > > > > This change is not tested with the Hardware. > > But of_property_read_u32 is better here than generic of_get_property. > > This make sure that value read properly independent of system endianess. > > This driver is only used on PPC_BE. And it is *very* fragile. The gain > is not enough for me to accept it without testing. Maybe Erhard (CCed) > is interested. If not, you may find someone on the ppc lists. > I applied the patch on top of kernel 5.6.6 and tested it on a PowerMac G4 3,6 DP and a PowerMac G5 11,2. Both machines run without anything suspicious going on. dmesg | grep i2c looks the same with patch and without patch. Tested-by: Erhard Furtner <erhard_f@mailbox.org>
On Wed, Apr 08, 2020 at 03:33:53PM +0530, Aishwarya R wrote: > Use of_property_read_u32 to read the "reg" and "i2c-address" property > instead of using of_get_property to check the return values. > > Signed-off-by: Aishwarya R <aishwaryarj100@gmail.com> Applied to for-next, thanks!
diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c index d565714c1f13..81506c2dab65 100644 --- a/drivers/i2c/busses/i2c-powermac.c +++ b/drivers/i2c/busses/i2c-powermac.c @@ -207,18 +207,18 @@ static u32 i2c_powermac_get_addr(struct i2c_adapter *adap, struct pmac_i2c_bus *bus, struct device_node *node) { - const __be32 *prop; - int len; + u32 prop; + int ret; /* First check for valid "reg" */ - prop = of_get_property(node, "reg", &len); - if (prop && (len >= sizeof(int))) - return (be32_to_cpup(prop) & 0xff) >> 1; + ret = of_property_read_u32(node, "reg", &prop); + if (ret == 0) + return (prop & 0xff) >> 1; /* Then check old-style "i2c-address" */ - prop = of_get_property(node, "i2c-address", &len); - if (prop && (len >= sizeof(int))) - return (be32_to_cpup(prop) & 0xff) >> 1; + ret = of_property_read_u32(node, "i2c-address", &prop); + if (ret == 0) + return (prop & 0xff) >> 1; /* Now handle some devices with missing "reg" properties */ if (of_node_name_eq(node, "cereal"))
Use of_property_read_u32 to read the "reg" and "i2c-address" property instead of using of_get_property to check the return values. Signed-off-by: Aishwarya R <aishwaryarj100@gmail.com> --- drivers/i2c/busses/i2c-powermac.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)