[PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Javier Carrasco]

This series aims to clarify the use cases of:

- device_for_each_child_node[_scoped]()
- fwnode_for_each_available_child_node[_scoped]()

to access firmware nodes.

There have been multiple discussions [1][2] about what the first macro
implies in the sense of availability, and a number of users have opted
for the second macro in cases where the first one should have been
preferred.

The second macro is intended to be used over child nodes of a firmware
node, not direct child nodes of the device node. Instead, those users
retrieve the fwnode member from the device struct just to have access to
a macro that explicitly indicates node availability.

That workaround is not necessary because `device_for_each_child_node()`
implies availability for the existing backends (ACPI, DT, swnode).

This series does not cover other points discussed in [2] like addressing
uses of `fwnode_for_each_child_node()` where `device_*` should have been
used, using the `_avaialble_` variant of the fwnode loop whenever
possible, or adding new `_scoped` macros. Such points will be covered by
subsequent series to keep focus on the "availability" issue.

The conversion has been validated with an LTC2992 hwmon sensor, which is
one of the affected drivers. The rest of the drivers could only be
compiled and checked with static-analysis tools.

Link: https://lore.kernel.org/all/20211205190101.26de4a57@jic23-huawei/ [1]
Link: https://lore.kernel.org/all/20240523-fwnode_for_each_available_child_node_scoped-v2-0-701f3a03f2fb@gmail.com/ [2]

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Changes in v2:
- [1/6] property.h: drop "if found" from the description of
  device_for_each_child_node()
- [3/6] bd2607mvv.c: fix child node usage.
- Link to v1: https://lore.kernel.org/r/20240706-device_for_each_child_node-available-v1-0-8a3f7615e41c@gmail.com

---
Javier Carrasco (6):
      device property: document device_for_each_child_node macro
      hwmon: (ltc2992) use device_for_each_child_node_scoped() to access child nodes
      leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
      leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
      leds: pca995x: use device_for_each_child_node() to access device child nodes
      net: mvpp2: use device_for_each_child_node() to access device child nodes

 drivers/hwmon/ltc2992.c                         | 19 ++++----------
 drivers/leds/leds-bd2606mvv.c                   | 23 ++++++++---------
 drivers/leds/leds-is31fl319x.c                  | 34 ++++++++-----------------
 drivers/leds/leds-pca995x.c                     | 15 ++++-------
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 +++-------
 include/linux/property.h                        | 10 ++++++++
 6 files changed, 45 insertions(+), 69 deletions(-)
---
base-commit: 41c196e567fb1ea97f68a2ffb7faab451cd90854
change-id: 20240701-device_for_each_child_node-available-1c1eca4b6495

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

[PATCH v2 1/6] device property: document device_for_each_child_node macro

[Javier Carrasco]

There have been some misconceptions about this macro, which iterates
over available child nodes from different backends.

As that is not obvious by its name, some users have opted for the
`fwnode_for_each_available_child_node()` macro instead.
That requires an unnecessary, explicit access to the fwnode member
of the device structure.

Passing the device to `device_for_each_child_node()` is shorter,
reflects more clearly the nature of the child nodes, and renders the
same result.

In general, `fwnode_for_each_available_child_node()` should be used
whenever the parent node of the children to iterate over is a firmware
node, and not the device itself.

Document the `device_for_each_child node(dev, child)` macro to clarify
its functionality.

Suggested-by: Jonathan Cameron <jic23@kernel.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 include/linux/property.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 61fc20e5f81f..da8f1208de38 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -171,6 +171,16 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
 struct fwnode_handle *device_get_next_child_node(const struct device *dev,
 						 struct fwnode_handle *child);
 
+/**
+ * device_for_each_child_node - iterate over available child nodes of a device
+ * @dev: Pointer to the struct device
+ * @child: Pointer to an available child node in each loop iteration
+ *
+ * Unavailable nodes are skipped i.e. this macro is implicitly _available_.
+ * The reference to the child node must be dropped on early exits.
+ * See fwnode_handle_put().
+ * For a scoped version of this macro, use device_for_each_child_node_scoped().
+ */
 #define device_for_each_child_node(dev, child)				\
 	for (child = device_get_next_child_node(dev, NULL); child;	\
 	     child = device_get_next_child_node(dev, child))

-- 
2.43.0

[PATCH v2 2/6] hwmon: (ltc2992) use device_for_each_child_node_scoped() to access child nodes

[Javier Carrasco]

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.

In this case, the child nodes are not required outside the loop, and
the scoped version of the macro can be used to remove the repetitive
`goto put` pattern.

Use `device_for_each_child_node_scoped_scoped()` to indicate device's
direct child nodes.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/hwmon/ltc2992.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/ltc2992.c b/drivers/hwmon/ltc2992.c
index d4a93223cd3b..541fa09dc6e7 100644
--- a/drivers/hwmon/ltc2992.c
+++ b/drivers/hwmon/ltc2992.c
@@ -854,33 +854,24 @@ static const struct regmap_config ltc2992_regmap_config = {
 
 static int ltc2992_parse_dt(struct ltc2992_state *st)
 {
-	struct fwnode_handle *fwnode;
-	struct fwnode_handle *child;
 	u32 addr;
 	u32 val;
 	int ret;
 
-	fwnode = dev_fwnode(&st->client->dev);
-
-	fwnode_for_each_available_child_node(fwnode, child) {
+	device_for_each_child_node_scoped(&st->client->dev, child) {
 		ret = fwnode_property_read_u32(child, "reg", &addr);
-		if (ret < 0) {
-			fwnode_handle_put(child);
+		if (ret < 0)
 			return ret;
-		}
 
-		if (addr > 1) {
-			fwnode_handle_put(child);
+		if (addr > 1)
 			return -EINVAL;
-		}
 
 		ret = fwnode_property_read_u32(child, "shunt-resistor-micro-ohms", &val);
 		if (!ret) {
-			if (!val) {
-				fwnode_handle_put(child);
+			if (!val)
 				return dev_err_probe(&st->client->dev, -EINVAL,
 						     "shunt resistor value cannot be zero\n");
-			}
+
 			st->r_sense_uohm[addr] = val;
 		}
 	}

-- 
2.43.0

[PATCH v2 3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()

[Javier Carrasco]

The current implementation accesses the `child` fwnode handle outside of
fwnode_for_each_available_child_node() without incrementing its
refcount. Add the missing call to `fwnode_handle_get(child)`.

The cleanup process where `child` is accessed is not right either
because a single call to `fwnode_handle_put()` is carried out in case of
an error, ignoring unasigned nodes at the point when the error happens.
Keep `child` inside of the first loop, and use the helper pointer that
receives references via `fwnode_handle_get()` to handle the child nodes
within the second loop.

Moreover, the iterated nodes are direct children of the device node,
and the `device_for_each_child_node()` macro accounts for child node
availability. By restricting `child` to live within that loop, the
scoped version of it can be used to simplify the error handling.

`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_scoped()` to indicate device's direct
child nodes.

Fixes: 8325642d2757 ("leds: bd2606mvv: Driver for the Rohm 6 Channel i2c LED driver")
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/leds/leds-bd2606mvv.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/leds-bd2606mvv.c b/drivers/leds/leds-bd2606mvv.c
index 3fda712d2f80..c1181a35d0f7 100644
--- a/drivers/leds/leds-bd2606mvv.c
+++ b/drivers/leds/leds-bd2606mvv.c
@@ -69,16 +69,14 @@ static const struct regmap_config bd2606mvv_regmap = {
 
 static int bd2606mvv_probe(struct i2c_client *client)
 {
-	struct fwnode_handle *np, *child;
 	struct device *dev = &client->dev;
 	struct bd2606mvv_priv *priv;
 	struct fwnode_handle *led_fwnodes[BD2606_MAX_LEDS] = { 0 };
 	int active_pairs[BD2606_MAX_LEDS / 2] = { 0 };
 	int err, reg;
-	int i;
+	int i, j;
 
-	np = dev_fwnode(dev);
-	if (!np)
+	if (!dev_fwnode(dev))
 		return -ENODEV;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -94,20 +92,18 @@ 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_scoped(dev, child) {
 		struct bd2606mvv_led *led;
 
 		err = fwnode_property_read_u32(child, "reg", &reg);
-		if (err) {
-			fwnode_handle_put(child);
+		if (err)
 			return err;
-		}
-		if (reg < 0 || reg >= BD2606_MAX_LEDS || led_fwnodes[reg]) {
-			fwnode_handle_put(child);
+
+		if (reg < 0 || reg >= BD2606_MAX_LEDS || led_fwnodes[reg])
 			return -EINVAL;
-		}
+
 		led = &priv->leds[reg];
-		led_fwnodes[reg] = child;
+		led_fwnodes[reg] = fwnode_handle_get(child);
 		active_pairs[reg / 2]++;
 		led->priv = priv;
 		led->led_no = reg;
@@ -130,7 +126,8 @@ static int bd2606mvv_probe(struct i2c_client *client)
 						     &priv->leds[i].ldev,
 						     &init_data);
 		if (err < 0) {
-			fwnode_handle_put(child);
+			for (j = i; j < BD2606_MAX_LEDS; j++)
+				fwnode_handle_put(led_fwnodes[j]);
 			return dev_err_probe(dev, err,
 					     "couldn't register LED %s\n",
 					     priv->leds[i].ldev.name);

-- 
2.43.0

[PATCH v2 4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes

[Javier Carrasco]

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.

In this case, the child nodes are not required outside the loop, and
the scoped version of the macro can be used to remove the repetitive
`goto put` pattern.

Use `device_for_each_child_node_scoped_scoped()` to indicate device's
direct child nodes.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/leds/leds-is31fl319x.c | 34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index 5e1a4d39a107..27bfab3da479 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -392,7 +392,7 @@ static int is31fl319x_parse_child_fw(const struct device *dev,
 
 static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 {
-	struct fwnode_handle *fwnode = dev_fwnode(dev), *child;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
 	int count;
 	int ret;
 
@@ -404,7 +404,7 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 	is31->cdef = device_get_match_data(dev);
 
 	count = 0;
-	fwnode_for_each_available_child_node(fwnode, child)
+	device_for_each_child_node_scoped(dev, child)
 		count++;
 
 	dev_dbg(dev, "probing with %d leds defined in DT\n", count);
@@ -414,33 +414,25 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 				     "Number of leds defined must be between 1 and %u\n",
 				     is31->cdef->num_leds);
 
-	fwnode_for_each_available_child_node(fwnode, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		struct is31fl319x_led *led;
 		u32 reg;
 
 		ret = fwnode_property_read_u32(child, "reg", &reg);
-		if (ret) {
-			ret = dev_err_probe(dev, ret, "Failed to read led 'reg' property\n");
-			goto put_child_node;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to read led 'reg' property\n");
 
-		if (reg < 1 || reg > is31->cdef->num_leds) {
-			ret = dev_err_probe(dev, -EINVAL, "invalid led reg %u\n", reg);
-			goto put_child_node;
-		}
+		if (reg < 1 || reg > is31->cdef->num_leds)
+			return dev_err_probe(dev, -EINVAL, "invalid led reg %u\n", reg);
 
 		led = &is31->leds[reg - 1];
 
-		if (led->configured) {
-			ret = dev_err_probe(dev, -EINVAL, "led %u is already configured\n", reg);
-			goto put_child_node;
-		}
+		if (led->configured)
+			return dev_err_probe(dev, -EINVAL, "led %u is already configured\n", reg);
 
 		ret = is31fl319x_parse_child_fw(dev, child, led, is31);
-		if (ret) {
-			ret = dev_err_probe(dev, ret, "led %u DT parsing failed\n", reg);
-			goto put_child_node;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret, "led %u DT parsing failed\n", reg);
 
 		led->configured = true;
 	}
@@ -454,10 +446,6 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 	}
 
 	return 0;
-
-put_child_node:
-	fwnode_handle_put(child);
-	return ret;
 }
 
 static inline int is31fl3190_microamp_to_cs(struct device *dev, u32 microamp)

-- 
2.43.0

[PATCH v2 5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes

[Javier Carrasco]

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.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/leds/leds-pca995x.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c
index 78215dff1499..a5b32eaba724 100644
--- a/drivers/leds/leds-pca995x.c
+++ b/drivers/leds/leds-pca995x.c
@@ -102,7 +102,7 @@ static const struct regmap_config pca995x_regmap = {
 static int pca995x_probe(struct i2c_client *client)
 {
 	struct fwnode_handle *led_fwnodes[PCA995X_MAX_OUTPUTS] = { 0 };
-	struct fwnode_handle *np, *child;
+	struct fwnode_handle *child;
 	struct device *dev = &client->dev;
 	struct pca995x_chip *chip;
 	struct pca995x_led *led;
@@ -110,8 +110,7 @@ static int pca995x_probe(struct i2c_client *client)
 
 	btype = (unsigned long)device_get_match_data(&client->dev);
 
-	np = dev_fwnode(dev);
-	if (!np)
+	if (!dev_fwnode(dev))
 		return -ENODEV;
 
 	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
@@ -125,17 +124,13 @@ static int pca995x_probe(struct i2c_client *client)
 
 	i2c_set_clientdata(client, chip);
 
-	fwnode_for_each_available_child_node(np, child) {
+	device_for_each_child_node(dev, child) {
 		ret = fwnode_property_read_u32(child, "reg", &reg);
-		if (ret) {
-			fwnode_handle_put(child);
+		if (ret)
 			return ret;
-		}
 
-		if (reg < 0 || reg >= PCA995X_MAX_OUTPUTS || led_fwnodes[reg]) {
-			fwnode_handle_put(child);
+		if (reg < 0 || reg >= PCA995X_MAX_OUTPUTS || led_fwnodes[reg])
 			return -EINVAL;
-		}
 
 		led = &chip->leds[reg];
 		led_fwnodes[reg] = child;

-- 
2.43.0

[PATCH v2 6/6] net: mvpp2: use device_for_each_child_node() to access device child nodes

[Javier Carrasco]

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.

The child nodes within mvpp2_probe are not accessed outside the lopps,
and the socped version of the macro can be used to automatically
decrement the refcount on early exits.

Use `device_for_each_child_node()` and its scoped variant to indicate
device's direct child nodes.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 8c45ad983abc..f123072b1f87 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -7417,8 +7417,6 @@ static int mvpp2_get_sram(struct platform_device *pdev,
 
 static int mvpp2_probe(struct platform_device *pdev)
 {
-	struct fwnode_handle *fwnode = pdev->dev.fwnode;
-	struct fwnode_handle *port_fwnode;
 	struct mvpp2 *priv;
 	struct resource *res;
 	void __iomem *base;
@@ -7591,7 +7589,7 @@ static int mvpp2_probe(struct platform_device *pdev)
 	}
 
 	/* Map DTS-active ports. Should be done before FIFO mvpp2_init */
-	fwnode_for_each_available_child_node(fwnode, port_fwnode) {
+	device_for_each_child_node_scoped(&pdev->dev, port_fwnode) {
 		if (!fwnode_property_read_u32(port_fwnode, "port-id", &i))
 			priv->port_map |= BIT(i);
 	}
@@ -7614,7 +7612,7 @@ static int mvpp2_probe(struct platform_device *pdev)
 		goto err_axi_clk;
 
 	/* Initialize ports */
-	fwnode_for_each_available_child_node(fwnode, port_fwnode) {
+	device_for_each_child_node_scoped(&pdev->dev, port_fwnode) {
 		err = mvpp2_port_probe(pdev, port_fwnode, priv);
 		if (err < 0)
 			goto err_port_probe;
@@ -7653,10 +7651,8 @@ static int mvpp2_probe(struct platform_device *pdev)
 	return 0;
 
 err_port_probe:
-	fwnode_handle_put(port_fwnode);
-
 	i = 0;
-	fwnode_for_each_available_child_node(fwnode, port_fwnode) {
+	device_for_each_child_node_scoped(&pdev->dev, port_fwnode) {
 		if (priv->port_list[i])
 			mvpp2_port_remove(priv->port_list[i]);
 		i++;
@@ -7677,13 +7673,12 @@ static int mvpp2_probe(struct platform_device *pdev)
 static void mvpp2_remove(struct platform_device *pdev)
 {
 	struct mvpp2 *priv = platform_get_drvdata(pdev);
-	struct fwnode_handle *fwnode = pdev->dev.fwnode;
 	int i = 0, poolnum = MVPP2_BM_POOLS_NUM;
 	struct fwnode_handle *port_fwnode;
 
 	mvpp2_dbgfs_cleanup(priv);
 
-	fwnode_for_each_available_child_node(fwnode, port_fwnode) {
+	device_for_each_child_node(&pdev->dev, port_fwnode) {
 		if (priv->port_list[i]) {
 			mutex_destroy(&priv->port_list[i]->gather_stats_lock);
 			mvpp2_port_remove(priv->port_list[i]);

-- 
2.43.0

Re: [PATCH v2 2/6] hwmon: (ltc2992) use device_for_each_child_node_scoped() to access child nodes

[Guenter Roeck]

On Sun, Jul 21, 2024 at 05:19:02PM +0200, Javier Carrasco 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.
> 
> In this case, the child nodes are not required outside the loop, and
> the scoped version of the macro can be used to remove the repetitive
> `goto put` pattern.
> 
> Use `device_for_each_child_node_scoped_scoped()` to indicate device's
> direct child nodes.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Applied.

Thanks,
Guenter

Re: [PATCH v2 1/6] device property: document device_for_each_child_node macro

[Markus Elfring]

…
> + * Unavailable nodes are skipped i.e. this macro is implicitly _available_.
…

How good does presented information fit together in this comment line?

Regards,
Markus

Re: [PATCH v2 1/6] device property: document device_for_each_child_node macro

[Greg Kroah-Hartman]

On Mon, Jul 22, 2024 at 09:15:26AM +0200, Markus Elfring wrote:
> …
> > + * Unavailable nodes are skipped i.e. this macro is implicitly _available_.
> …
> 
> How good does presented information fit together in this comment line?


Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot

Re: [PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Lee Jones]

On Sun, 21 Jul 2024, Javier Carrasco wrote:

> This series aims to clarify the use cases of:
> 
> - device_for_each_child_node[_scoped]()
> - fwnode_for_each_available_child_node[_scoped]()
> 
> to access firmware nodes.
> 
> There have been multiple discussions [1][2] about what the first macro
> implies in the sense of availability, and a number of users have opted
> for the second macro in cases where the first one should have been
> preferred.
> 
> The second macro is intended to be used over child nodes of a firmware
> node, not direct child nodes of the device node. Instead, those users
> retrieve the fwnode member from the device struct just to have access to
> a macro that explicitly indicates node availability.
> 
> That workaround is not necessary because `device_for_each_child_node()`
> implies availability for the existing backends (ACPI, DT, swnode).
> 
> This series does not cover other points discussed in [2] like addressing
> uses of `fwnode_for_each_child_node()` where `device_*` should have been
> used, using the `_avaialble_` variant of the fwnode loop whenever
> possible, or adding new `_scoped` macros. Such points will be covered by
> subsequent series to keep focus on the "availability" issue.
> 
> The conversion has been validated with an LTC2992 hwmon sensor, which is
> one of the affected drivers. The rest of the drivers could only be
> compiled and checked with static-analysis tools.
> 
> Link: https://lore.kernel.org/all/20211205190101.26de4a57@jic23-huawei/ [1]
> Link: https://lore.kernel.org/all/20240523-fwnode_for_each_available_child_node_scoped-v2-0-701f3a03f2fb@gmail.com/ [2]
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> Changes in v2:
> - [1/6] property.h: drop "if found" from the description of
>   device_for_each_child_node()
> - [3/6] bd2607mvv.c: fix child node usage.
> - Link to v1: https://lore.kernel.org/r/20240706-device_for_each_child_node-available-v1-0-8a3f7615e41c@gmail.com
> 
> ---
> Javier Carrasco (6):
>       device property: document device_for_each_child_node macro
>       hwmon: (ltc2992) use device_for_each_child_node_scoped() to access child nodes
>       leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
>       leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
>       leds: pca995x: use device_for_each_child_node() to access device child nodes
>       net: mvpp2: use device_for_each_child_node() to access device child nodes
> 
>  drivers/hwmon/ltc2992.c                         | 19 ++++----------
>  drivers/leds/leds-bd2606mvv.c                   | 23 ++++++++---------
>  drivers/leds/leds-is31fl319x.c                  | 34 ++++++++-----------------
>  drivers/leds/leds-pca995x.c                     | 15 ++++-------
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 13 +++-------
>  include/linux/property.h                        | 10 ++++++++
>  6 files changed, 45 insertions(+), 69 deletions(-)
> ---
> base-commit: 41c196e567fb1ea97f68a2ffb7faab451cd90854
> change-id: 20240701-device_for_each_child_node-available-1c1eca4b6495

fatal: bad object 41c196e567fb1ea97f68a2ffb7faab451cd90854

And the LED patches do not apply to LED.

Please rebase onto -next.

-- 
Lee Jones [李琼斯]

Re: (subset) [PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Lee Jones]

On Sun, 21 Jul 2024 17:19:00 +0200, Javier Carrasco wrote:
> This series aims to clarify the use cases of:
> 
> - device_for_each_child_node[_scoped]()
> - fwnode_for_each_available_child_node[_scoped]()
> 
> to access firmware nodes.
> 
> [...]

Applied, thanks!

[3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
      commit: 75d2a77327c4917bb66163eea0374bb749428e9c
[4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
      commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
[5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
      (no commit info)

--
Lee Jones [李琼斯]

Re: (subset) [PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Javier Carrasco]

On 25/07/2024 18:28, Lee Jones wrote:
> On Sun, 21 Jul 2024 17:19:00 +0200, Javier Carrasco wrote:
>> This series aims to clarify the use cases of:
>>
>> - device_for_each_child_node[_scoped]()
>> - fwnode_for_each_available_child_node[_scoped]()
>>
>> to access firmware nodes.
>>
>> [...]
> 
> Applied, thanks!
> 
> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
>       commit: 75d2a77327c4917bb66163eea0374bb749428e9c
> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
>       commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
> [5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
>       (no commit info)
> 
> --
> Lee Jones [李琼斯]
> 

Hi Lee,

could you please tell me where you applied them? I rebased onto
linux-next to prepare for v3, and these patches are still added on top
of it. Can I find them in some leds/ branch? Thank you.

Best regards,
Javier Carrasco

Re: (subset) [PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Lee Jones]

On Mon, 29 Jul 2024, Javier Carrasco wrote:

> On 25/07/2024 18:28, Lee Jones wrote:
> > On Sun, 21 Jul 2024 17:19:00 +0200, Javier Carrasco wrote:
> >> This series aims to clarify the use cases of:
> >>
> >> - device_for_each_child_node[_scoped]()
> >> - fwnode_for_each_available_child_node[_scoped]()
> >>
> >> to access firmware nodes.
> >>
> >> [...]
> > 
> > Applied, thanks!
> > 
> > [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
> >       commit: 75d2a77327c4917bb66163eea0374bb749428e9c
> > [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
> >       commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
> > [5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
> >       (no commit info)
> > 
> > --
> > Lee Jones [李琼斯]
> > 
> 
> Hi Lee,
> 
> could you please tell me where you applied them? I rebased onto
> linux-next to prepare for v3, and these patches are still added on top
> of it. Can I find them in some leds/ branch? Thank you.

Sorry, I was side-tracked before pushing.

Pushed now.  They should be in -next tomorrow.

-- 
Lee Jones [李琼斯]

Re: (subset) [PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Javier Carrasco]

On 01/08/2024 14:39, Lee Jones wrote:
> On Mon, 29 Jul 2024, Javier Carrasco wrote:
> 
>> On 25/07/2024 18:28, Lee Jones wrote:
>>> On Sun, 21 Jul 2024 17:19:00 +0200, Javier Carrasco wrote:
>>>> This series aims to clarify the use cases of:
>>>>
>>>> - device_for_each_child_node[_scoped]()
>>>> - fwnode_for_each_available_child_node[_scoped]()
>>>>
>>>> to access firmware nodes.
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
>>>       commit: 75d2a77327c4917bb66163eea0374bb749428e9c
>>> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
>>>       commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
>>> [5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
>>>       (no commit info)
>>>
>>> --
>>> Lee Jones [李琼斯]
>>>
>>
>> Hi Lee,
>>
>> could you please tell me where you applied them? I rebased onto
>> linux-next to prepare for v3, and these patches are still added on top
>> of it. Can I find them in some leds/ branch? Thank you.
> 
> Sorry, I was side-tracked before pushing.
> 
> Pushed now.  They should be in -next tomorrow.
> 

Thanks, I see

[3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()

[4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to
access child nodes

applied to -next, but

[5/6] leds: pca995x: use device_for_each_child_node() to access device
child nodes

has not been applied yet.

Re: (subset) [PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Lee Jones]

On Fri, 02 Aug 2024, Javier Carrasco wrote:

> On 01/08/2024 14:39, Lee Jones wrote:
> > On Mon, 29 Jul 2024, Javier Carrasco wrote:
> > 
> >> On 25/07/2024 18:28, Lee Jones wrote:
> >>> On Sun, 21 Jul 2024 17:19:00 +0200, Javier Carrasco wrote:
> >>>> This series aims to clarify the use cases of:
> >>>>
> >>>> - device_for_each_child_node[_scoped]()
> >>>> - fwnode_for_each_available_child_node[_scoped]()
> >>>>
> >>>> to access firmware nodes.
> >>>>
> >>>> [...]
> >>>
> >>> Applied, thanks!
> >>>
> >>> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
> >>>       commit: 75d2a77327c4917bb66163eea0374bb749428e9c
> >>> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
> >>>       commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
> >>> [5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
> >>>       (no commit info)
> >>>
> >>> --
> >>> Lee Jones [李琼斯]
> >>>
> >>
> >> Hi Lee,
> >>
> >> could you please tell me where you applied them? I rebased onto
> >> linux-next to prepare for v3, and these patches are still added on top
> >> of it. Can I find them in some leds/ branch? Thank you.
> > 
> > Sorry, I was side-tracked before pushing.
> > 
> > Pushed now.  They should be in -next tomorrow.
> > 
> 
> Thanks, I see
> 
> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
> 
> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to
> access child nodes
> 
> applied to -next, but
> 
> [5/6] leds: pca995x: use device_for_each_child_node() to access device
> child nodes
> 
> has not been applied yet.

Yep, looks like b4 didn't like that one:

[3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
      commit: 75d2a77327c4917bb66163eea0374bb749428e9c
[4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
      commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
[5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
      (no commit info)

I'll try again and see if it can be pulled in.

If not you'll have to resubmit it.

-- 
Lee Jones [李琼斯]

Re: (subset) [PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Lee Jones]

On Mon, 05 Aug 2024, Lee Jones wrote:

> On Fri, 02 Aug 2024, Javier Carrasco wrote:
> 
> > On 01/08/2024 14:39, Lee Jones wrote:
> > > On Mon, 29 Jul 2024, Javier Carrasco wrote:
> > > 
> > >> On 25/07/2024 18:28, Lee Jones wrote:
> > >>> On Sun, 21 Jul 2024 17:19:00 +0200, Javier Carrasco wrote:
> > >>>> This series aims to clarify the use cases of:
> > >>>>
> > >>>> - device_for_each_child_node[_scoped]()
> > >>>> - fwnode_for_each_available_child_node[_scoped]()
> > >>>>
> > >>>> to access firmware nodes.
> > >>>>
> > >>>> [...]
> > >>>
> > >>> Applied, thanks!
> > >>>
> > >>> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
> > >>>       commit: 75d2a77327c4917bb66163eea0374bb749428e9c
> > >>> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
> > >>>       commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
> > >>> [5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
> > >>>       (no commit info)
> > >>>
> > >>> --
> > >>> Lee Jones [李琼斯]
> > >>>
> > >>
> > >> Hi Lee,
> > >>
> > >> could you please tell me where you applied them? I rebased onto
> > >> linux-next to prepare for v3, and these patches are still added on top
> > >> of it. Can I find them in some leds/ branch? Thank you.
> > > 
> > > Sorry, I was side-tracked before pushing.
> > > 
> > > Pushed now.  They should be in -next tomorrow.
> > > 
> > 
> > Thanks, I see
> > 
> > [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
> > 
> > [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to
> > access child nodes
> > 
> > applied to -next, but
> > 
> > [5/6] leds: pca995x: use device_for_each_child_node() to access device
> > child nodes
> > 
> > has not been applied yet.
> 
> Yep, looks like b4 didn't like that one:
> 
> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
>       commit: 75d2a77327c4917bb66163eea0374bb749428e9c
> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
>       commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
> [5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
>       (no commit info)
> 
> I'll try again and see if it can be pulled in.
> 
> If not you'll have to resubmit it.

Now results in conflict:

    Applying patch(es)
    Applying: leds: pca995x: use device_for_each_child_node() to access device child nodes
    Using index info to reconstruct a base tree...
    M	drivers/leds/leds-pca995x.c
    Checking patch drivers/leds/leds-pca995x.c...
    Applied patch drivers/leds/leds-pca995x.c cleanly.
    Falling back to patching base and 3-way merge...
    error: Your local changes to the following files would be overwritten by merge:
    	drivers/leds/leds-pca995x.c
    Please commit your changes or stash them before you merge.
    Aborting
    error: Failed to merge in the changes.
    Patch failed at 0001 leds: pca995x: use device_for_each_child_node() to access device child nodes
    hint: Use 'git am --show-current-patch=diff' to see the failed patch
    hint: When you have resolved this problem, run "git am --continue".
    hint: If you prefer to skip this patch, run "git am --skip" instead.
    hint: To restore the original branch and stop patching, run "git am --abort".
    hint: Disable this message with "git config advice.mergeConflict false"
    
    Failed to apply patches (fix and either hit return to continue or Ctrl+c to exit)

Please rebase and resubmit.

-- 
Lee Jones [李琼斯]

Re: (subset) [PATCH v2 0/6] use device_for_each_child_node() to access device child nodes

[Javier Carrasco]

On 05/08/2024 16:33, Lee Jones wrote:
> On Mon, 05 Aug 2024, Lee Jones wrote:
> 
>> On Fri, 02 Aug 2024, Javier Carrasco wrote:
>>
>>> On 01/08/2024 14:39, Lee Jones wrote:
>>>> On Mon, 29 Jul 2024, Javier Carrasco wrote:
>>>>
>>>>> On 25/07/2024 18:28, Lee Jones wrote:
>>>>>> On Sun, 21 Jul 2024 17:19:00 +0200, Javier Carrasco wrote:
>>>>>>> This series aims to clarify the use cases of:
>>>>>>>
>>>>>>> - device_for_each_child_node[_scoped]()
>>>>>>> - fwnode_for_each_available_child_node[_scoped]()
>>>>>>>
>>>>>>> to access firmware nodes.
>>>>>>>
>>>>>>> [...]
>>>>>>
>>>>>> Applied, thanks!
>>>>>>
>>>>>> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
>>>>>>       commit: 75d2a77327c4917bb66163eea0374bb749428e9c
>>>>>> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
>>>>>>       commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
>>>>>> [5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
>>>>>>       (no commit info)
>>>>>>
>>>>>> --
>>>>>> Lee Jones [李琼斯]
>>>>>>
>>>>>
>>>>> Hi Lee,
>>>>>
>>>>> could you please tell me where you applied them? I rebased onto
>>>>> linux-next to prepare for v3, and these patches are still added on top
>>>>> of it. Can I find them in some leds/ branch? Thank you.
>>>>
>>>> Sorry, I was side-tracked before pushing.
>>>>
>>>> Pushed now.  They should be in -next tomorrow.
>>>>
>>>
>>> Thanks, I see
>>>
>>> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
>>>
>>> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to
>>> access child nodes
>>>
>>> applied to -next, but
>>>
>>> [5/6] leds: pca995x: use device_for_each_child_node() to access device
>>> child nodes
>>>
>>> has not been applied yet.
>>
>> Yep, looks like b4 didn't like that one:
>>
>> [3/6] leds: bd2606mvv: fix device child node usage in bd2606mvv_probe()
>>       commit: 75d2a77327c4917bb66163eea0374bb749428e9c
>> [4/6] leds: is31fl319x: use device_for_each_child_node_scoped() to access child nodes
>>       commit: 0f5a3feb60aba5d74f0b655cdff9c35aca03e81b
>> [5/6] leds: pca995x: use device_for_each_child_node() to access device child nodes
>>       (no commit info)
>>
>> I'll try again and see if it can be pulled in.
>>
>> If not you'll have to resubmit it.
> 
> Now results in conflict:
> 
>     Applying patch(es)
>     Applying: leds: pca995x: use device_for_each_child_node() to access device child nodes
>     Using index info to reconstruct a base tree...
>     M	drivers/leds/leds-pca995x.c
>     Checking patch drivers/leds/leds-pca995x.c...
>     Applied patch drivers/leds/leds-pca995x.c cleanly.
>     Falling back to patching base and 3-way merge...
>     error: Your local changes to the following files would be overwritten by merge:
>     	drivers/leds/leds-pca995x.c
>     Please commit your changes or stash them before you merge.
>     Aborting
>     error: Failed to merge in the changes.
>     Patch failed at 0001 leds: pca995x: use device_for_each_child_node() to access device child nodes
>     hint: Use 'git am --show-current-patch=diff' to see the failed patch
>     hint: When you have resolved this problem, run "git am --continue".
>     hint: If you prefer to skip this patch, run "git am --skip" instead.
>     hint: To restore the original branch and stop patching, run "git am --abort".
>     hint: Disable this message with "git config advice.mergeConflict false"
>     
>     Failed to apply patches (fix and either hit return to continue or Ctrl+c to exit)
> 
> Please rebase and resubmit.
> 

Thank you for making the effort anyway, I will resubmit the patch.

Best regards,
Javier Carrasco