of: automate of_node_put() - new approach to loops.

From: Jonathan Cameron <[email protected]>

Using automated cleanup to replace of_node_put() handling allows for a simplfied flow by enabling direct returns on errors.

Non available child nodes should never have been considered; that is ones where status != okay and was defined.

Signed-off-by Jonathan Cameron <[email protected]>

---
 drivers/iio/adc/rcar-gyroadc.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

drivers/iio/adc/rcar-gyroadc.c +6 -15

index d524f2e8e927..15a21d2860e7 100644
@@ -318,7 +318,6 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 	struct rcar_gyroadc *priv = iio_priv(indio_dev);
 	struct device *dev = priv->dev;
 	struct device_node *np = dev->of_node;
-	struct device_node *child;
 	struct regulator *vref;
 	unsigned int reg;
 	unsigned int adcmode = -1, childmode;

@@ -326,7 +325,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 	unsigned int num_channels;
 	int ret, first = 1;
 
-	for_each_child_of_node(np, child) {
+	for_each_available_child_of_node_scoped(np, child) {
 		of_id = of_match_node(rcar_gyroadc_child_match, child);
 		if (!of_id) {
 			dev_err(dev, "Ignoring unsupported ADC \"%pOFn\".",

@@ -352,7 +351,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 			num_channels = ARRAY_SIZE(rcar_gyroadc_iio_channels_3);
 			break;
 		default:
-			goto err_e_inval;
+			return -EINVAL;
 		}
 
 		/*

@@ -369,7 +368,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 				dev_err(dev,
 					"Failed to get child reg property of ADC \"%pOFn\".\n",
 					child);
-				goto err_of_node_put;
+				return ret;
 			}
 
 			/* Channel number is too high. */

@@ -377,7 +376,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 				dev_err(dev,
 					"Only %i channels supported with %pOFn, but reg = <%i>.\n",
 					num_channels, child, reg);
-				goto err_e_inval;
+				return -EINVAL;
 			}
 		}
 

@@ -386,7 +385,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 			dev_err(dev,
 				"Channel %i uses different ADC mode than the rest.\n",
 				reg);
-			goto err_e_inval;
+			return -EINVAL;
 		}
 
 		/* Channel is valid, grab the regulator. */

@@ -396,8 +395,7 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 		if (IS_ERR(vref)) {
 			dev_dbg(dev, "Channel %i 'vref' supply not connected.\n",
 				reg);
-			ret = PTR_ERR(vref);
-			goto err_of_node_put;
+			return PTR_ERR(vref);
 		}
 
 		priv->vref[reg] = vref;

@@ -422,7 +420,6 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 		 * we can stop parsing here.
 		 */
 		if (childmode == RCAR_GYROADC_MODE_SELECT_1_MB88101A) {
-			of_node_put(child);
 			break;
 		}
 	}

@@ -433,12 +430,6 @@ static int rcar_gyroadc_parse_subdevs(struct iio_dev *indio_dev)
 	}
 
 	return 0;
-
-err_e_inval:
-	ret = -EINVAL;
-err_of_node_put:
-	of_node_put(child);
-	return ret;
 }
 
 static void rcar_gyroadc_deinit_supplies(struct iio_dev *indio_dev)

From: Jonathan Cameron <[email protected]>

Some discussion occured on previous posting.
https://lore.kernel.org/linux-iio/[email protected]/

Summary:
* fwnode conversions should be considered when applying this infrastructure to a driver. Perhaps better to move directly to the generic FW property handling rather than improve existing of specific code.
* There are lots of potential places to use this based on detections from Julia's coccinelle scripts linked below.

The equivalent device_for_each_child_node_scoped() series for fwnode will be queued up in IIO for the merge window shortly as it has gathered sufficient tags. Hopefully the precdent set there for the approach will reassure people that instantiating the child variable inside the macro definition is the best approach.
https://lore.kernel.org/linux-iio/[email protected]/

v2: Andy suggested most of the original converted set should move to
generic fwnode / property.h handling. Within IIO that was a reasonable observation given we've been trying to move away from firmware specific handling for some time. Patches making that change to appropriate drivers posted.
As we discussed there are cases which are not suitable for such conversion and this infrastructure still provides clear benefits for them.

Ideally it would be good if this introductory series adding the infrastructure makes the 6.9 merge window. There are no dependencies on work queued in the IIO tree, so this can go via devicetree if the maintainers would prefer. I've had some off list messages asking when this would be merged, as there is interest in building on it next cycle for other parts of the kernel (where conversion to fwnode handling may be less appropriate).

Quoted 29 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Some discussion occured on previous posting.
https://lore.kernel.org/linux-iio/[email protected]/

Summary:
* fwnode conversions should be considered when applying this infrastructure to a driver. Perhaps better to move directly to the generic FW property handling rather than improve existing of specific code.
* There are lots of potential places to use this based on detections from Julia's coccinelle scripts linked below.

The equivalent device_for_each_child_node_scoped() series for fwnode will be queued up in IIO for the merge window shortly as it has gathered sufficient tags. Hopefully the precdent set there for the approach will reassure people that instantiating the child variable inside the macro definition is the best approach.
https://lore.kernel.org/linux-iio/[email protected]/

v2: Andy suggested most of the original converted set should move to
generic fwnode / property.h handling. Within IIO that was a reasonable observation given we've been trying to move away from firmware specific handling for some time. Patches making that change to appropriate drivers posted.
As we discussed there are cases which are not suitable for such conversion and this infrastructure still provides clear benefits for them.

Quoted 7 lines

Ideally it would be good if this introductory series adding the infrastructure makes the 6.9 merge window. There are no dependencies on work queued in the IIO tree, so this can go via devicetree if the maintainers would prefer. I've had some off list messages asking when this would be merged, as there is interest in building on it next cycle for other parts of the kernel (where conversion to fwnode handling may be less appropriate).

I'll let you take it. For the series:

Reviewed-by Rob Herring <[email protected]>

I've got some drivers/of/ conversions too, but they are probably next cycle at this point.

Rob

Quoted 38 lines · On Fri, 1 Mar 2024 16:39:42 -0600 Rob Herring <[email protected]> wrote:

Quoted 29 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Some discussion occured on previous posting.
https://lore.kernel.org/linux-iio/[email protected]/

Summary:
* fwnode conversions should be considered when applying this infrastructure to a driver. Perhaps better to move directly to the generic FW property handling rather than improve existing of specific code.
* There are lots of potential places to use this based on detections from Julia's coccinelle scripts linked below.

The equivalent device_for_each_child_node_scoped() series for fwnode will be queued up in IIO for the merge window shortly as it has gathered sufficient tags. Hopefully the precdent set there for the approach will reassure people that instantiating the child variable inside the macro definition is the best approach.
https://lore.kernel.org/linux-iio/[email protected]/

v2: Andy suggested most of the original converted set should move to
generic fwnode / property.h handling. Within IIO that was a reasonable observation given we've been trying to move away from firmware specific handling for some time. Patches making that change to appropriate drivers posted.
As we discussed there are cases which are not suitable for such conversion and this infrastructure still provides clear benefits for them.

Quoted 7 lines

Ideally it would be good if this introductory series adding the infrastructure makes the 6.9 merge window. There are no dependencies on work queued in the IIO tree, so this can go via devicetree if the maintainers would prefer. I've had some off list messages asking when this would be merged, as there is interest in building on it next cycle for other parts of the kernel (where conversion to fwnode handling may be less appropriate).

Quoted 8 lines

I'll let you take it. For the series:

Reviewed-by Rob Herring <[email protected]>

I've got some drivers/of/ conversions too, but they are probably next cycle at this point.

Rob

Thanks Rob,

Whether this makes it for this cycle is probably dependent on whether Linus does decide to do got to rc8 as hinted at as a possibility
+ whether Greg feels comfortable taking these through his tree (char-misc is the normal path for IIO). I know various people are hoping this series makes it, but if doesn't we can do an immutable tree early next cycle (though obviously that may reduce speed of adoption).

We are discussing the equivalent pull request for the fwnode version here:

https://lore.kernel.org/linux-iio/2024030239-gift-cabdriver-266b@gregkh/T/#m87e7208820ebf6416a77a2973773b65a087b4796

I've optimistically applied this series to my togreg-cleanup branch and merged that into the togreg branch of iio.git for linux-next to pick up.

Thanks,

Jonathan

Quoted 59 lines · On Sun, 3 Mar 2024 11:56:33 +0000 Jonathan Cameron <[email protected]> wrote:

Quoted 38 lines · On Fri, 1 Mar 2024 16:39:42 -0600 Rob Herring <[email protected]> wrote:

Quoted 29 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Some discussion occured on previous posting.
https://lore.kernel.org/linux-iio/[email protected]/

Summary:
* fwnode conversions should be considered when applying this infrastructure to a driver. Perhaps better to move directly to the generic FW property handling rather than improve existing of specific code.
* There are lots of potential places to use this based on detections from Julia's coccinelle scripts linked below.

The equivalent device_for_each_child_node_scoped() series for fwnode will be queued up in IIO for the merge window shortly as it has gathered sufficient tags. Hopefully the precdent set there for the approach will reassure people that instantiating the child variable inside the macro definition is the best approach.
https://lore.kernel.org/linux-iio/[email protected]/

v2: Andy suggested most of the original converted set should move to
generic fwnode / property.h handling. Within IIO that was a reasonable observation given we've been trying to move away from firmware specific handling for some time. Patches making that change to appropriate drivers posted.
As we discussed there are cases which are not suitable for such conversion and this infrastructure still provides clear benefits for them.

Quoted 7 lines

Ideally it would be good if this introductory series adding the infrastructure makes the 6.9 merge window. There are no dependencies on work queued in the IIO tree, so this can go via devicetree if the maintainers would prefer. I've had some off list messages asking when this would be merged, as there is interest in building on it next cycle for other parts of the kernel (where conversion to fwnode handling may be less appropriate).

Quoted 8 lines

I'll let you take it. For the series:

Reviewed-by Rob Herring <[email protected]>

I've got some drivers/of/ conversions too, but they are probably next cycle at this point.

Rob

Thanks Rob,

Whether this makes it for this cycle is probably dependent on whether Linus does decide to do got to rc8 as hinted at as a possibility
+ whether Greg feels comfortable taking these through his tree (char-misc is the normal path for IIO). I know various people are hoping this series makes it, but if doesn't we can do an immutable tree early next cycle (though obviously that may reduce speed of adoption).

Quoted 7 lines

We are discussing the equivalent pull request for the fwnode version here:

https://lore.kernel.org/linux-iio/2024030239-gift-cabdriver-266b@gregkh/T/#m87e7208820ebf6416a77a2973773b65a087b4796

I've optimistically applied this series to my togreg-cleanup branch and merged that into the togreg branch of iio.git for linux-next to pick up.

Greg, would you consider a last minute pull request for these, or picking them up directly? It would be helpful for Rob's follow ups and the work Julia is doing with coccinelle and automating of locating cases to apply this approach.

If the device_for_each_child_node_scoped() series is fine this is almostly exactly the same thing for the device tree specific case. Not sure what your plans are for that pull request so I might be jumping the gun.

If not (and assuming the generic property version does make it in) I'll do an immutable branch based on rc1 so that others can build on this via that. Fiddlier solution for everyone but given how late we are, perhaps the wiser one.

Quoted 79 lines · Jonathan Cameron

Quoted 59 lines · On Sun, 3 Mar 2024 11:56:33 +0000 Jonathan Cameron <[email protected]> wrote:

Quoted 38 lines · On Fri, 1 Mar 2024 16:39:42 -0600 Rob Herring <[email protected]> wrote:

Quoted 29 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Some discussion occured on previous posting.
https://lore.kernel.org/linux-iio/[email protected]/

Summary:
* fwnode conversions should be considered when applying this infrastructure to a driver. Perhaps better to move directly to the generic FW property handling rather than improve existing of specific code.
* There are lots of potential places to use this based on detections from Julia's coccinelle scripts linked below.

The equivalent device_for_each_child_node_scoped() series for fwnode will be queued up in IIO for the merge window shortly as it has gathered sufficient tags. Hopefully the precdent set there for the approach will reassure people that instantiating the child variable inside the macro definition is the best approach.
https://lore.kernel.org/linux-iio/[email protected]/

v2: Andy suggested most of the original converted set should move to
generic fwnode / property.h handling. Within IIO that was a reasonable observation given we've been trying to move away from firmware specific handling for some time. Patches making that change to appropriate drivers posted.
As we discussed there are cases which are not suitable for such conversion and this infrastructure still provides clear benefits for them.

Quoted 7 lines

Ideally it would be good if this introductory series adding the infrastructure makes the 6.9 merge window. There are no dependencies on work queued in the IIO tree, so this can go via devicetree if the maintainers would prefer. I've had some off list messages asking when this would be merged, as there is interest in building on it next cycle for other parts of the kernel (where conversion to fwnode handling may be less appropriate).

Quoted 8 lines

I'll let you take it. For the series:

Reviewed-by Rob Herring <[email protected]>

I've got some drivers/of/ conversions too, but they are probably next cycle at this point.

Rob

Thanks Rob,

Whether this makes it for this cycle is probably dependent on whether Linus does decide to do got to rc8 as hinted at as a possibility
+ whether Greg feels comfortable taking these through his tree (char-misc is the normal path for IIO). I know various people are hoping this series makes it, but if doesn't we can do an immutable tree early next cycle (though obviously that may reduce speed of adoption).

Quoted 7 lines

We are discussing the equivalent pull request for the fwnode version here:

https://lore.kernel.org/linux-iio/2024030239-gift-cabdriver-266b@gregkh/T/#m87e7208820ebf6416a77a2973773b65a087b4796

I've optimistically applied this series to my togreg-cleanup branch and merged that into the togreg branch of iio.git for linux-next to pick up.

Greg, would you consider a last minute pull request for these, or picking them up directly? It would be helpful for Rob's follow ups and the work Julia is doing with coccinelle and automating of locating cases to apply this approach.

If the device_for_each_child_node_scoped() series is fine this is almostly exactly the same thing for the device tree specific case. Not sure what your plans are for that pull request so I might be jumping the gun.

Quoted 4 lines

If not (and assuming the generic property version does make it in) I'll do an immutable branch based on rc1 so that others can build on this via that. Fiddlier solution for everyone but given how late we are, perhaps the wiser one.

I'm happy to pick up the first 3 patches for 6.9 if you want.

Rob