On Sat, 17 Oct 2020 00:04:27 +0530 Vaishnav M A <vaishnav@xxxxxxxxxxxxxxx> wrote: > This patch aims to replace the of_property_read_u32 for reading > the amstaos,cover-comp-gain property with device_property_read_u32 > > Thanks and Regards, > Vaishnav M A Should keep this stuff about the --- to just the patch description as that will end up in the git log when I pick up the patch. You can add any extra stuff as notes as long as it is after the --- below. > > Signed-off-by: Vaishnav M A <vaishnav@xxxxxxxxxxxxxxx> A really minor comment below, but I'd like a couple of header tweaks to be part of this patch as well to reflect that we are removing the explicit use of of_* in this driver (which is great btw!) At the moment we are getting the following headers only via indirect includes. As we are using things directly defined in them please add #include <linux/property.h> (for the device_property_read*) #include <linux/mod_devicetable.h> (for the of_device_id table) Thanks, Jonathan > --- > drivers/iio/light/tsl2563.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/iio/light/tsl2563.c b/drivers/iio/light/tsl2563.c > index abc8d7db8dc1..1f1b8b7cefa4 100644 > --- a/drivers/iio/light/tsl2563.c > +++ b/drivers/iio/light/tsl2563.c > @@ -703,7 +703,6 @@ static int tsl2563_probe(struct i2c_client *client, > struct iio_dev *indio_dev; > struct tsl2563_chip *chip; > struct tsl2563_platform_data *pdata = client->dev.platform_data; > - struct device_node *np = client->dev.of_node; > int err = 0; > u8 id = 0; > > @@ -738,13 +737,14 @@ static int tsl2563_probe(struct i2c_client *client, > chip->calib0 = tsl2563_calib_from_sysfs(CALIB_BASE_SYSFS); > chip->calib1 = tsl2563_calib_from_sysfs(CALIB_BASE_SYSFS); > > - if (pdata) > + if (pdata) { > chip->cover_comp_gain = pdata->cover_comp_gain; > - else if (np) > - of_property_read_u32(np, "amstaos,cover-comp-gain", > - &chip->cover_comp_gain); > - else > - chip->cover_comp_gain = 1; > + } else { > + err = device_property_read_u32(&client->dev, "amstaos,cover-comp-gain", > + &chip->cover_comp_gain); > + if (err) > + chip->cover_comp_gain = 1; Hmm. This is a small change in logic. I'm not sure it really matters, but should have been called out explicitly in the patch description. > + } > > dev_info(&client->dev, "model %d, rev. %d\n", id >> 4, id & 0x0f); > indio_dev->name = client->name;