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

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

[Flaviu Nistor]

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@gmail.com>
---
Changes in v3:
- Use casting (with explanation comment) to handle the signed values as suggested
  by Guenter Roeck. This is tested and works similar to the old implementation that
  used of_property_read_s32().
- Link to v2: https://lore.kernel.org/all/20260717063730.5617-1-flaviu.nistor@gmail.com/
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@gmail.com/

 drivers/hwmon/lm90.c | 54 +++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index ae3b0a33d676..1c603272538a 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -101,6 +101,7 @@
 #include <linux/bits.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/fwnode.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -108,7 +109,7 @@
 #include <linux/hwmon.h>
 #include <linux/kstrtox.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
@@ -295,7 +296,7 @@ static const struct i2c_device_id lm90_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, lm90_id);
 
-static const struct of_device_id __maybe_unused lm90_of_match[] = {
+static const struct of_device_id lm90_of_match[] = {
 	{
 		.compatible = "adi,adm1032",
 		.data = (void *)adm1032
@@ -2602,7 +2603,6 @@ static void lm90_stop_work(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;
 
 	if (data->flags & LM90_HAVE_CONVRATE) {
@@ -2626,7 +2626,7 @@ 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 (of_property_read_bool(np, "ti,extended-range-enable"))
+		if (device_property_read_bool(&client->dev, "ti,extended-range-enable"))
 			config |= 0x04;
 		if (!(config & 0x04))
 			data->flags &= ~LM90_HAVE_EXTENDED_TEMP;
@@ -2692,36 +2692,41 @@ static irqreturn_t lm90_irq_thread(int irq, void *dev_id)
 		return IRQ_NONE;
 }
 
-static int lm90_probe_channel_from_dt(struct i2c_client *client,
-				      struct device_node *child,
-				      struct lm90_data *data)
+static int lm90_probe_channel(struct i2c_client *client,
+			      struct fwnode_handle *child,
+			      struct lm90_data *data)
 {
 	u32 id;
 	s32 val;
 	int err;
 	struct device *dev = &client->dev;
 
-	err = of_property_read_u32(child, "reg", &id);
+	err = fwnode_property_read_u32(child, "reg", &id);
 	if (err) {
-		dev_err(dev, "missing reg property of %pOFn\n", child);
+		dev_err(dev, "missing reg property of %pfw\n", child);
 		return err;
 	}
 
 	if (id >= MAX_CHANNELS) {
-		dev_err(dev, "invalid reg property value %d in %pOFn\n", id, child);
+		dev_err(dev, "invalid reg property value %d in %pfw\n", id, child);
 		return -EINVAL;
 	}
 
-	err = of_property_read_string(child, "label", &data->channel_label[id]);
+	err = fwnode_property_read_string(child, "label", &data->channel_label[id]);
 	if (err == -ENODATA || err == -EILSEQ) {
-		dev_err(dev, "invalid label property in %pOFn\n", child);
+		dev_err(dev, "invalid label property in %pfw\n", child);
 		return err;
 	}
 
 	if (data->channel_label[id])
 		data->channel_config[id] |= HWMON_T_LABEL;
 
-	err = of_property_read_s32(child, "temperature-offset-millicelsius", &val);
+	/*
+	 * fwnode_property_read_u32() has no signed equivalent.
+	 * temperature-offset-millicelsius is signed, so read and reinterpret it as s32 to
+	 * preserve negative offsets values (same behavior as the old of_property_read_s32()).
+	 */
+	err = fwnode_property_read_u32(child, "temperature-offset-millicelsius", (u32 *)&val);
 	if (!err) {
 		if (id == 0) {
 			dev_err(dev, "temperature-offset-millicelsius can't be set for internal channel\n");
@@ -2739,18 +2744,17 @@ static int lm90_probe_channel_from_dt(struct i2c_client *client,
 	return 0;
 }
 
-static int lm90_parse_dt_channel_info(struct i2c_client *client,
-				      struct lm90_data *data)
+static int lm90_parse_channel_info(struct i2c_client *client,
+				   struct lm90_data *data)
 {
 	int err;
 	struct device *dev = &client->dev;
-	const struct device_node *np = dev->of_node;
 
-	for_each_child_of_node_scoped(np, child) {
-		if (strcmp(child->name, "channel"))
+	device_for_each_child_node_scoped(dev, child) {
+		if (!fwnode_name_eq(child, "channel"))
 			continue;
 
-		err = lm90_probe_channel_from_dt(client, child, data);
+		err = lm90_probe_channel(client, child, data);
 		if (err)
 			return err;
 	}
@@ -2887,12 +2891,10 @@ static int lm90_probe(struct i2c_client *client)
 	/* Set maximum conversion rate */
 	data->max_convrate = lm90_params[data->kind].max_convrate;
 
-	/* Parse device-tree channel information */
-	if (client->dev.of_node) {
-		err = lm90_parse_dt_channel_info(client, data);
-		if (err)
-			return err;
-	}
+	/* Parse channel information */
+	err = lm90_parse_channel_info(client, data);
+	if (err)
+		return err;
 
 	/* Initialize the LM90 chip */
 	err = lm90_init_client(client, data);
@@ -2983,7 +2985,7 @@ static struct i2c_driver lm90_driver = {
 	.class		= I2C_CLASS_HWMON,
 	.driver = {
 		.name	= "lm90",
-		.of_match_table = of_match_ptr(lm90_of_match),
+		.of_match_table = lm90_of_match,
 		.pm	= pm_sleep_ptr(&lm90_pm_ops),
 	},
 	.probe		= lm90_probe,
-- 
2.34.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Switching to `device_for_each_child_node_scoped()` exposes a critical pre-existing bug in the core `fwnode_get_next_child_node()` which causes an infinite loop and double-frees if a device has both OF and secondary firmware nodes.
--

--- Patch [1]: [PATCH v3] hwmon: (lm90) Switch channel parsing to fwnode APIs ---
commit 39e5cdd2d72333fe0fa7b34c25ad27651948158c
Author: Flaviu Nistor <flaviu.nistor@gmail.com>

hwmon: (lm90) Switch channel parsing to fwnode APIs

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.

> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index 4b9c0ccdf2609..03779c1a251a2 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c

[ ... ]

> @@ -2739,18 +2744,17 @@ static int lm90_probe_channel_from_dt(struct i2c_client *client,
>  	return 0;
>  }
>  
> -static int lm90_parse_dt_channel_info(struct i2c_client *client,
> -				      struct lm90_data *data)
> +static int lm90_parse_channel_info(struct i2c_client *client,
> +				   struct lm90_data *data)
>  {
>  	int err;
>  	struct device *dev = &client->dev;
> -	const struct device_node *np = dev->of_node;
>  
> -	for_each_child_of_node_scoped(np, child) {
> -		if (strcmp(child->name, "channel"))
> +	device_for_each_child_node_scoped(dev, child) {
> +		if (!fwnode_name_eq(child, "channel"))

[Severity: Critical]
This is a pre-existing issue, but switching to
device_for_each_child_node_scoped() exposes this driver to a regression
in the core fwnode_get_next_child_node() framework.

If a device has both Open Firmware and secondary firmware nodes (swnodes),
can this iterator trigger an infinite loop and a double-free?

When transitioning from the primary Open Firmware fwnode to the secondary
swnode, the core fwnode_get_next_child_node() passes the already-put
primary child to the secondary fwnode backend:

drivers/base/property.c:fwnode_get_next_child_node() {
	...
	/* When no more children in primary, continue with secondary */
	return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
}

Since the passed child is an Open Firmware node rather than a software node,
to_swnode() inside software_node_get_next_child() returns a null pointer.
This causes it to drop the reference on the node a second time (the first
drop was in of_get_next_available_child()), leading to a double-free:

drivers/base/swnode.c:software_node_get_next_child() {
	...
	if (!p || list_empty(&p->children) ||
	    (c && list_is_last(&c->entry, &p->children))) {
		fwnode_handle_put(child);
		return NULL;
	}
}

Furthermore, on the next iteration, the returned secondary child is passed
back to the primary backend. Since it is not an Open Firmware node,
to_of_node() returns a null pointer, causing of_get_next_available_child()
to restart the search and return the first node again, creating an
infinite loop between node types.

Could this driver probe freeze the kernel on systems where a software node
is attached alongside Open Firmware child nodes?

>  			continue;
>  
> -		err = lm90_probe_channel_from_dt(client, child, data);
> +		err = lm90_probe_channel(client, child, data);
>  		if (err)
>  			return err;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730091409.10544-1-flaviu.nistor@gmail.com?part=1

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

[Guenter Roeck]

On Thu, Jul 30, 2026 at 12:14:09PM +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@gmail.com>

Applied.

Thanks,
Guenter

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

[Guenter Roeck]

On 7/30/26 02:43, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Switching to `device_for_each_child_node_scoped()` exposes a critical pre-existing bug in the core `fwnode_get_next_child_node()` which causes an infinite loop and double-frees if a device has both OF and secondary firmware nodes.
> --
> 
> --- Patch [1]: [PATCH v3] hwmon: (lm90) Switch channel parsing to fwnode APIs ---
> commit 39e5cdd2d72333fe0fa7b34c25ad27651948158c
> Author: Flaviu Nistor <flaviu.nistor@gmail.com>
> 
> hwmon: (lm90) Switch channel parsing to fwnode APIs
> 
> 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.
> 
>> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
>> index 4b9c0ccdf2609..03779c1a251a2 100644
>> --- a/drivers/hwmon/lm90.c
>> +++ b/drivers/hwmon/lm90.c
> 
> [ ... ]
> 
>> @@ -2739,18 +2744,17 @@ static int lm90_probe_channel_from_dt(struct i2c_client *client,
>>   	return 0;
>>   }
>>   
>> -static int lm90_parse_dt_channel_info(struct i2c_client *client,
>> -				      struct lm90_data *data)
>> +static int lm90_parse_channel_info(struct i2c_client *client,
>> +				   struct lm90_data *data)
>>   {
>>   	int err;
>>   	struct device *dev = &client->dev;
>> -	const struct device_node *np = dev->of_node;
>>   
>> -	for_each_child_of_node_scoped(np, child) {
>> -		if (strcmp(child->name, "channel"))
>> +	device_for_each_child_node_scoped(dev, child) {
>> +		if (!fwnode_name_eq(child, "channel"))
> 
> [Severity: Critical]
> This is a pre-existing issue, but switching to
> device_for_each_child_node_scoped() exposes this driver to a regression
> in the core fwnode_get_next_child_node() framework.
> 

Interesting report, and quite possibly valid, but not caused
by this driver.

Guenter