Re: [PATCH v2] hwmon: (lm90) Switch channel parsing to fwnode APIs

[Guenter Roeck]

On 7/20/26 06:55, Flaviu Nistor wrote:
> On Fri, Jul 19, 2026 at 17:06:00, Guenter Roeck wrote:
> > On Fri, Jul 17, 2026 at 09:37:30AM +0300, Flaviu Nistor wrote:
> > > Replace OF property handling with fwnode in the probe function to read
> > > the channels properties, improving the driver compatibility since this
> > > method is not limited to Device Tree only.
> > > Add also the needed headers for explicit include and clean up related
> > > function naming.
> > >
> > > Signed-off-by: Flaviu Nistor <flaviu.nistor@xxxxxxxxx>
> > > ---
> > > Changes in v2:
> > > - Remove <linux/mod_devicetable.h> as suggested by Uwe Kleine-Konig.
> > > - Link to v1: https://lore.kernel.org/all/20260713190659.4511-1-flaviu.nistor@xxxxxxxxx/
> > >
> > >   drivers/hwmon/lm90.c | 47 +++++++++++++++++++++-----------------------
> > >   1 file changed, 22 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> > >   
> > ...
> >
> > > -	err = of_property_read_s32(child, "temperature-offset-millicelsius", &val);
> > > +	err = fwnode_property_read_u32(child, "temperature-offset-millicelsius", &val);
> >
> > I understand you consider it to be irrelevant, but temperature offsets can
> > be negative. Please explain how this is handled.
>
> It is no about the fact that I consider it irrelevant, but rather I was mislead
> that of_property_read_s32 maps to of_property_read_u32(), and the new fwnode API
> does not have a dedicated function for signed values.
> I see as a solution to add a similar approach to include/linux/property.h. Add a new
> fwnode_property_read_s32 that maps to fwnode_property_read_u32, and then use it in the driver.
> I will try to send a new patch series to have a working solution.

Type casting in the driver and adding a comment explaining the reason would be
perfectly fine. What is not ok is to silently ignore it. Also, it will need
to be tested if it is working as intended. For of_property_read_s32() we can
assume that it works since there is an API function. That is not the case
for the fwnode API, which either means that the fwnode API does not support
negative properties or that no one needed and/or tested it.

Guenter