[PATCH] leds: is31fl32xx: Use fwnode/device property APIs

[PATCH] leds: is31fl32xx: Use fwnode/device property APIs

[Rosen Penev]

Convert the DT-specific helpers in is31fl32xx_parse_dt() and
is31fl32xx_parse_child_dt() to their fwnode/device equivalents.
Behaviour is unchanged for device tree: device_property_read_bool(),
device_for_each_child_node_scoped() and device_get_child_node_count()
operate on the same nodes and keep the skip of unavailable children.

Use the %pfwP format specifier in error messages, as child is now a
struct fwnode_handle and %pOF would misread it as a device_node.

Assisted-by: opencode:deepseek-v4-flash-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/leds/leds-is31fl32xx.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
index 6c8d6b833260..285085d27453 100644
--- a/drivers/leds/leds-is31fl32xx.c
+++ b/drivers/leds/leds-is31fl32xx.c
@@ -378,23 +378,23 @@ static int is31fl32xx_init_regs(struct is31fl32xx_priv *priv)
 }
 
 static int is31fl32xx_parse_child_dt(const struct device *dev,
-				     const struct device_node *child,
+				     const struct fwnode_handle *child,
 				     struct is31fl32xx_led_data *led_data)
 {
 	struct led_classdev *cdev = &led_data->cdev;
 	int ret = 0;
 	u32 reg;
 
-	ret = of_property_read_u32(child, "reg", &reg);
+	ret = fwnode_property_read_u32(child, "reg", &reg);
 	if (ret || reg < 1 || reg > led_data->priv->cdef->channels) {
 		dev_err(dev,
-			"Child node %pOF does not have a valid reg property\n",
+			"Child node %pfwP does not have a valid reg property\n",
 			child);
 		return -EINVAL;
 	}
 	led_data->channel = reg;
 
-	of_property_read_u32(child, "led-max-microamp", &led_data->max_microamp);
+	fwnode_property_read_u32(child, "led-max-microamp", &led_data->max_microamp);
 
 	cdev->brightness_set_blocking = is31fl32xx_brightness_set;
 
@@ -422,7 +422,7 @@ static int is31fl32xx_parse_dt(struct device *dev,
 	int ret = 0;
 
 	if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) &&
-	    of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm")) {
+	    device_property_read_bool(dev, "issi,22khz-pwm")) {
 
 		ret = is31fl32xx_write(priv, cdef->output_frequency_setting_reg,
 				       IS31FL32XX_PWM_FREQUENCY_22KHZ);
@@ -433,7 +433,7 @@ static int is31fl32xx_parse_dt(struct device *dev,
 		}
 	}
 
-	for_each_available_child_of_node_scoped(dev_of_node(dev), child) {
+	device_for_each_child_node_scoped(dev, child) {
 		struct led_init_data init_data = {};
 		struct is31fl32xx_led_data *led_data =
 			&priv->leds[priv->num_leds];
@@ -451,17 +451,17 @@ static int is31fl32xx_parse_dt(struct device *dev,
 							  led_data->channel);
 		if (other_led_data) {
 			dev_err(dev,
-				"Node %pOF 'reg' conflicts with another LED\n",
+				"Node %pfwP 'reg' conflicts with another LED\n",
 				child);
 			return -EINVAL;
 		}
 
-		init_data.fwnode = of_fwnode_handle(child);
+		init_data.fwnode = child;
 
 		ret = devm_led_classdev_register_ext(dev, &led_data->cdev,
 						     &init_data);
 		if (ret) {
-			dev_err(dev, "Failed to register LED for %pOF: %d\n",
+			dev_err(dev, "Failed to register LED for %pfwP: %d\n",
 				child, ret);
 			return ret;
 		}
@@ -576,7 +576,7 @@ static int is31fl32xx_probe(struct i2c_client *client)
 
 	cdef = device_get_match_data(dev);
 
-	count = of_get_available_child_count(dev_of_node(dev));
+	count = device_get_child_node_count(dev);
 	if (!count)
 		return -EINVAL;
 
-- 
2.55.0

Re: [PATCH] leds: is31fl32xx: Use fwnode/device property APIs

[sashiko-bot]

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

Pre-existing issues:
- [High] Potential out-of-bounds write to `priv->leds` array due to TOCTOU race condition during child node iteration.
- [High] NULL pointer dereference in `is31fl32xx_parse_dt()` when device match data is absent.
--

commit ed8f6f187305b333f2b97784d1f0f1abc18b4cb5
Author: Rosen Penev <rosenp@gmail.com>

leds: is31fl32xx: Use fwnode/device property APIs

This commit converts the DT-specific helpers in is31fl32xx_parse_dt() and
is31fl32xx_parse_child_dt() to their fwnode/device equivalents. It retains
the skipping of unavailable children and updates the format specifiers.

> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c
[ ... ]
> @@ -422,7 +422,7 @@ static int is31fl32xx_parse_dt(struct device *dev,
>  	int ret = 0;
>  
>  	if ((cdef->output_frequency_setting_reg != IS31FL32XX_REG_NONE) &&

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for cdef to be
NULL here?

If the device binds via the I2C ID table (e.g., fallback matching by node name
without a valid compatible string, or via sysfs new_device with an attached
software node), device_get_match_data(dev) in is31fl32xx_probe() returns NULL
because the is31fl32xx_id table entries have no .driver_data.

If the node also contains children, device_get_child_node_count() allows the
probe to proceed, and is31fl32xx_parse_dt() will then unconditionally
dereference cdef when checking cdef->output_frequency_setting_reg.

Should there be a NULL check for cdef prior to this?

> -	    of_property_read_bool(dev_of_node(dev), "issi,22khz-pwm")) {
> +	    device_property_read_bool(dev, "issi,22khz-pwm")) {
>  
>  		ret = is31fl32xx_write(priv, cdef->output_frequency_setting_reg,
>  				       IS31FL32XX_PWM_FREQUENCY_22KHZ);
> @@ -433,7 +433,7 @@ static int is31fl32xx_parse_dt(struct device *dev,
>  		}
>  	}
>  
> -	for_each_available_child_of_node_scoped(dev_of_node(dev), child) {
> +	device_for_each_child_node_scoped(dev, child) {
>  		struct led_init_data init_data = {};
>  		struct is31fl32xx_led_data *led_data =
>  			&priv->leds[priv->num_leds];

[Severity: High]
This is a pre-existing issue, but could this iteration cause an out-of-bounds
write to the priv->leds array?

In is31fl32xx_probe(), device_get_child_node_count() calculates the number of
available child nodes to allocate the priv->leds array. Later, this loop
iterates over the children and unconditionally increments priv->num_leds while
writing to the array.

If a child node is added concurrently (e.g., via a device tree overlay)
between these two lockless operations, the loop will execute more times
than the allocated count.

Would it be safer to ensure priv->num_leds does not exceed the originally
allocated count within this loop?

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