RE: [v2 2/2] driver/hwmon/lm90: enable extended range according to DTS node

[Holger Brunck <holger.brunck@hitachienergy.com>]

> On Mon, May 16, 2022 at 08:01:34PM +0000, Holger Brunck wrote:
> > > > diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c index
> > > > 1c9493c70813..6cdbcfab9f20 100644
> > > > --- a/drivers/hwmon/lm90.c
> > > > +++ b/drivers/hwmon/lm90.c
> > > > @@ -1707,6 +1707,7 @@ static void lm90_restore_conf(void *_data)
> > > >
> > > >   static int lm90_init_client(struct i2c_client *client, struct lm90_data
> *data)
> > > >   {
> > > > +     struct device_node *np = client->dev.of_node;
> > > >       int config, convrate;
> > > >
> > > >       convrate = lm90_read_reg(client, LM90_REG_R_CONVRATE); @@
> > > > -1727,7 +1728,8 @@ static int lm90_init_client(struct i2c_client
> > > > *client, struct lm90_data *data)
> > > >
> > > >       /* Check Temperature Range Select */
> > > >       if (data->flags & LM90_HAVE_EXTENDED_TEMP) {
> > > > -             if (config & 0x04)
> > > > +             if (config & 0x04 ||
> > > > +                 of_property_read_bool(np,
> > > > + "onsemi,extended-range-enable"))
> > > >                       data->flags |= LM90_FLAG_ADT7461_EXT;
> > >
> > > Maybe I am missing something, but I don't see the matching
> > > configuration change. Specifying the flag in devicetree only really
> > > makes sense if the chip configuration is changed accordingly.
> > >
> >
> > what is confusing here for me is that in the current code we have "if
> > (config & 0x4)" and if this is true we configure the flags
> > accordingly. But the bit 0x4 in config is nowhere set in current code.
> > Therefore also the flag is never set. Or do I miss something?
> >
> The idea is to pick up the configuration set by the BIOS/ROMMON.
> 

ok.
 
> > I am asking because if my assumption is correct I would replace the
> > current (never matching) check to the new property:
> 
> It does match, if set by the BIOS.
> 

ok thanks.

> > if (of_property_read_bool(np, "ti,extended-range-enable")) {
> >   config |= 0x4;
> >   data->flags |= LM90_FLAG_ADT7461_EXT; }
> >
> > Is this ok for you?
> >
> Looks ok at first glance, though of course I'll have to see the entire patch.
> 

sure, but I need to change it as now the use case that config bit 0x4 is set even
if the property is not set by DTS need to be covered as before. But anyhow, lets
discuss on the update then.

Best regards
Holger