On 07/07/2024 18:57, Jonathan Cameron wrote: > On Sat, 06 Jul 2024 17:23:35 +0200 > Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote: > >> The iterated nodes are direct children of the device node, and the >> `device_for_each_child_node()` macro accounts for child node >> availability. >> >> `fwnode_for_each_available_child_node()` is meant to access the child >> nodes of an fwnode, and therefore not direct child nodes of the device >> node. >> >> Use `device_for_each_child_node()` to indicate device's direct child >> nodes. >> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> > Why not the scoped variant? > There look to be two error paths in there which would be simplified. > I did not use the scoped variant because "child" is used outside the loop. On the other hand, I think an fwnode_handle_get() is missing for every "led_fwnodes[reg] = child" because a simple assignment does not increment the refcount. After adding fwnode_handle_get(), the scoped variant could be used, and the call to fwnode_handle_put() would act on led_fwnodes[reg] instead. >> --- >> drivers/leds/leds-bd2606mvv.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/leds/leds-bd2606mvv.c b/drivers/leds/leds-bd2606mvv.c >> index 3fda712d2f80..4f38b7b4d9d1 100644 >> --- a/drivers/leds/leds-bd2606mvv.c >> +++ b/drivers/leds/leds-bd2606mvv.c >> @@ -69,7 +69,7 @@ static const struct regmap_config bd2606mvv_regmap = { >> >> static int bd2606mvv_probe(struct i2c_client *client) >> { >> - struct fwnode_handle *np, *child; >> + struct fwnode_handle *child; >> struct device *dev = &client->dev; >> struct bd2606mvv_priv *priv; >> struct fwnode_handle *led_fwnodes[BD2606_MAX_LEDS] = { 0 }; >> @@ -77,8 +77,7 @@ static int bd2606mvv_probe(struct i2c_client *client) >> int err, reg; >> int i; >> >> - np = dev_fwnode(dev); >> - if (!np) >> + if (!dev_fwnode(dev)) >> return -ENODEV; >> >> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> @@ -94,7 +93,7 @@ static int bd2606mvv_probe(struct i2c_client *client) >> >> i2c_set_clientdata(client, priv); >> >> - fwnode_for_each_available_child_node(np, child) { >> + device_for_each_child_node(dev, child) { >> struct bd2606mvv_led *led; >> >> err = fwnode_property_read_u32(child, "reg", ®); >> >