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", ®);
>>
>
