| Message ID | 20190815125802.16500-2-max@enpas.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3,1/3] i2c/busses: Add i2c-icy for I2C on m68k/Amiga | expand |
On Thu, Aug 15, 2019 at 02:58:01PM +0200, Max Staudt wrote: > ltc2990 will now use device_property_read_u32_array() instead of > of_property_read_u32_array() - allowing the use of software nodes > via fwnode_create_software_node(). > > This allows code using i2c_new_device() to specify a default > measurement mode for the LTC2990. > > Signed-off-by: Max Staudt <max@enpas.org> Applied to hwmon-next. Thanks, Guenter
On 08/15/2019 08:33 PM, Guenter Roeck wrote: > On Thu, Aug 15, 2019 at 02:58:01PM +0200, Max Staudt wrote: >> ltc2990 will now use device_property_read_u32_array() instead of >> of_property_read_u32_array() - allowing the use of software nodes >> via fwnode_create_software_node(). >> >> This allows code using i2c_new_device() to specify a default >> measurement mode for the LTC2990. >> >> Signed-off-by: Max Staudt <max@enpas.org> > > Applied to hwmon-next. Hi Guenter, Thank you for reviewing and taking this patch, as well as teaching me about swnode. Max
On 08/15/2019 02:58 PM, Max Staudt wrote: > - if (of_node) { > - ret = of_property_read_u32_array(of_node, "lltc,meas-mode", > - data->mode, 2); > + if (i2c->dev.of_node || i2c->dev.fwnode) { One more idea, would it be better here to do the following? if (device_property_present(i2c->dev, "lltc,meas-mode")) { ret = of_property_read_u32_array(of_node, "lltc,meas-mode", data->mode, 2); } I'm happy to prepare a patch if you wish to have this in - just let me know whether it should be on top of the last one, or instead of it. Thanks, Max
On Fri, Aug 16, 2019 at 12:19:42AM +0200, Max Staudt wrote: > On 08/15/2019 02:58 PM, Max Staudt wrote: > > - if (of_node) { > > - ret = of_property_read_u32_array(of_node, "lltc,meas-mode", > > - data->mode, 2); > > + if (i2c->dev.of_node || i2c->dev.fwnode) { > > One more idea, would it be better here to do the following? > > if (device_property_present(i2c->dev, "lltc,meas-mode")) { > ret = of_property_read_u32_array(of_node, "lltc,meas-mode", > data->mode, 2); > } > > I'm happy to prepare a patch if you wish to have this in - just let me know whether it should be on top of the last one, or instead of it. That would be semantically different. The property is currently mandatory. The above code would make it optional. This might work: if (dev_fwnode(&i2c->dev)) { ret = device_property_read_u32_array(...); ... } Feel free to send another version of your patch. Thanks, Guenter
On Fri, Aug 16, 2019 at 1:50 AM Guenter Roeck <linux@roeck-us.net> wrote: > On Fri, Aug 16, 2019 at 12:19:42AM +0200, Max Staudt wrote: > > On 08/15/2019 02:58 PM, Max Staudt wrote: > > > - if (of_node) { > > > - ret = of_property_read_u32_array(of_node, "lltc,meas-mode", > > > - data->mode, 2); > > > + if (i2c->dev.of_node || i2c->dev.fwnode) { I was just going to comment on this check... > > One more idea, would it be better here to do the following? > > > > if (device_property_present(i2c->dev, "lltc,meas-mode")) { > > ret = of_property_read_u32_array(of_node, "lltc,meas-mode", > > data->mode, 2); > > } > > > > I'm happy to prepare a patch if you wish to have this in - just let me know whether it should be on top of the last one, or instead of it. > > That would be semantically different. The property is currently mandatory. > The above code would make it optional. This might work: > > if (dev_fwnode(&i2c->dev)) { > ret = device_property_read_u32_array(...); > ... > } Much better, thanks! Gr{oetje,eeting}s, Geert
On 08/16/2019 12:43 AM, Guenter Roeck wrote: > On Fri, Aug 16, 2019 at 12:19:42AM +0200, Max Staudt wrote: >> On 08/15/2019 02:58 PM, Max Staudt wrote: >>> - if (of_node) { >>> - ret = of_property_read_u32_array(of_node, "lltc,meas-mode", >>> - data->mode, 2); >>> + if (i2c->dev.of_node || i2c->dev.fwnode) { >> >> One more idea, would it be better here to do the following? >> >> if (device_property_present(i2c->dev, "lltc,meas-mode")) { >> ret = of_property_read_u32_array(of_node, "lltc,meas-mode", >> data->mode, 2); >> } >> >> I'm happy to prepare a patch if you wish to have this in - just let me know whether it should be on top of the last one, or instead of it. > > That would be semantically different. The property is currently mandatory. > The above code would make it optional. This might work: > > if (dev_fwnode(&i2c->dev)) { > ret = device_property_read_u32_array(...); > ... > } Fair point. The semantic change was part of my question, but the more I think about it, the less I want to open this can of worms. We can still make the property optional later on, while it's not as easy to make it mandatory. I'll send a patch with your suggestion. Thanks! Max
diff --git a/drivers/hwmon/ltc2990.c b/drivers/hwmon/ltc2990.c index f9431ad43..e06619ac9 100644 --- a/drivers/hwmon/ltc2990.c +++ b/drivers/hwmon/ltc2990.c @@ -206,7 +206,6 @@ static int ltc2990_i2c_probe(struct i2c_client *i2c, int ret; struct device *hwmon_dev; struct ltc2990_data *data; - struct device_node *of_node = i2c->dev.of_node; if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA)) @@ -218,9 +217,10 @@ static int ltc2990_i2c_probe(struct i2c_client *i2c, data->i2c = i2c; - if (of_node) { - ret = of_property_read_u32_array(of_node, "lltc,meas-mode", - data->mode, 2); + if (i2c->dev.of_node || i2c->dev.fwnode) { + ret = device_property_read_u32_array(&i2c->dev, + "lltc,meas-mode", + data->mode, 2); if (ret < 0) return ret;
ltc2990 will now use device_property_read_u32_array() instead of of_property_read_u32_array() - allowing the use of software nodes via fwnode_create_software_node(). This allows code using i2c_new_device() to specify a default measurement mode for the LTC2990. Signed-off-by: Max Staudt <max@enpas.org> --- drivers/hwmon/ltc2990.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)