linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling.
@ 2024-02-17 16:42 Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h Jonathan Cameron
                   ` (15 more replies)
  0 siblings, 16 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Sorry for the rapid resend, Andy noticed I'd messed up creating the v3 patch
set with some updates committed in the wrong patch.

Since v3: The updates to alignment of device_for_each_child_node_scopd() were
    in the wrong patch. Move them to patch 4 where they should always
    have been. (thanks Andy!)

Since v2: Thanks to Sakari and Andy for reviews.
- New first patch moving fwnode_handle_put() into property.h
- Tweak alignment in the loop macro
- Pick up tags.
- scopd -> scoped typo fix in some patch descriptions.

As can be seen by the examples from IIO that follow this can save
a reasonable amount of complexity and boiler plate code, often enabling
additional cleanups in related code such as use of
return dev_err_probe().

Given we are now fairly late in the cycle, I'd expect to take this
through the IIO tree and we can make use of it elsewhere next cycle.

Note I don't have the hardware so this is compile tested only.
Hence I'd appreciate some Tested-by tags if anyone can poke one of the
effected drivers.

Julia Lawal has posted some nice coccinelle magic for the DT equivalents.
Referenced from that cover letter.  Similar may help us convert more
drivers to use this new approach, but often hand tweaking can take
additional advantage of other cleanup.h based magic, or things like
return dev_err_probe().
https://lore.kernel.org/all/20240211174237.182947-1-jic23@kernel.org/

Jonathan Cameron (15):
  device property: Move fwnode_handle_put() into property.h
  device property: Add cleanup.h based fwnode_handle_put() scope based
    cleanup.
  iio: temp: ltc2983: Use __free(fwnode_handle) to replace
    fwnode_handle_put() calls
  device property: Introduce device_for_each_child_node_scoped()
  iio: adc: max11410: Use device_for_each_child_node_scoped()
  iio: adc: mcp3564: Use device_for_each_child_node_scoped()
  iio: adc: qcom-spmi-adc5: Use device_for_each_child_node_scoped()
  iio: adc: rzg2l_adc: Use device_for_each_child_node_scoped()
  iio: adc: stm32: Use device_for_each_child_node_scoped()
  iio: adc: ti-ads1015: Use device_for_each_child_node_scoped()
  iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped()
  iio: addac: ad74413r: Use device_for_each_child_node_scoped()
  iio: dac: ad3552r: Use device_for_each_child_node_scoped()
  iio: dac: ad5770r: Use device_for_each_child_node_scoped()
  iio: dac: ltc2688: Use device_for_each_child_node_scoped()

 drivers/base/property.c           | 14 ------
 drivers/iio/adc/max11410.c        | 27 +++--------
 drivers/iio/adc/mcp3564.c         | 16 +++----
 drivers/iio/adc/qcom-spmi-adc5.c  |  7 +--
 drivers/iio/adc/rzg2l_adc.c       | 11 ++---
 drivers/iio/adc/stm32-adc.c       | 63 ++++++++++---------------
 drivers/iio/adc/ti-ads1015.c      |  5 +-
 drivers/iio/adc/ti-ads131e08.c    | 13 ++----
 drivers/iio/addac/ad74413r.c      | 10 +---
 drivers/iio/dac/ad3552r.c         | 51 ++++++++-------------
 drivers/iio/dac/ad5770r.c         | 19 +++-----
 drivers/iio/dac/ltc2688.c         | 24 +++-------
 drivers/iio/temperature/ltc2983.c | 76 ++++++++++---------------------
 include/linux/property.h          | 22 ++++++++-
 14 files changed, 126 insertions(+), 232 deletions(-)

-- 
2.43.2


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19  8:55   ` Greg Kroah-Hartman
  2024-02-20  6:55   ` Sakari Ailus
  2024-02-17 16:42 ` [PATCH v4 02/15] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup Jonathan Cameron
                   ` (14 subsequent siblings)
  15 siblings, 2 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

By having this function as static inline in the header, the compiler
is able to see if can optmize the call out if (IS_ERR_OR_NULL(fwnode))
This will allow a simpler DEFINE_FREE() call in the following patch.

Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/base/property.c  | 14 --------------
 include/linux/property.h | 14 +++++++++++++-
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index a1b01ab42052..53e42031c646 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -923,20 +923,6 @@ struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode)
 }
 EXPORT_SYMBOL_GPL(fwnode_handle_get);
 
-/**
- * fwnode_handle_put - Drop reference to a device node
- * @fwnode: Pointer to the device node to drop the reference to.
- *
- * This has to be used when terminating device_for_each_child_node() iteration
- * with break or return to prevent stale device node references from being left
- * behind.
- */
-void fwnode_handle_put(struct fwnode_handle *fwnode)
-{
-	fwnode_call_void_op(fwnode, put);
-}
-EXPORT_SYMBOL_GPL(fwnode_handle_put);
-
 /**
  * fwnode_device_is_available - check if a device is available for use
  * @fwnode: Pointer to the fwnode of the device.
diff --git a/include/linux/property.h b/include/linux/property.h
index e6516d0b7d52..151bcab4f92a 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -187,7 +187,19 @@ struct fwnode_handle *device_get_named_child_node(const struct device *dev,
 						  const char *childname);
 
 struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode);
-void fwnode_handle_put(struct fwnode_handle *fwnode);
+
+/**
+ * fwnode_handle_put - Drop reference to a device node
+ * @fwnode: Pointer to the device node to drop the reference to.
+ *
+ * This has to be used when terminating device_for_each_child_node() iteration
+ * with break or return to prevent stale device node references from being left
+ * behind.
+ */
+static inline void fwnode_handle_put(struct fwnode_handle *fwnode)
+{
+	fwnode_call_void_op(fwnode, put);
+}
 
 int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index);
 int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 02/15] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup.
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19  8:55   ` Greg Kroah-Hartman
  2024-02-20  6:55   ` Sakari Ailus
  2024-02-17 16:42 ` [PATCH v4 03/15] iio: temp: ltc2983: Use __free(fwnode_handle) to replace fwnode_handle_put() calls Jonathan Cameron
                   ` (13 subsequent siblings)
  15 siblings, 2 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Useful where the fwnode_handle was obtained from a call such as
fwnode_find_reference() as it will safely do nothing if IS_ERR() is true
and will automatically release the reference on the variable leaving
scope.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 include/linux/property.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 151bcab4f92a..9e67c3c4df6e 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -12,6 +12,7 @@
 
 #include <linux/args.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/fwnode.h>
 #include <linux/stddef.h>
 #include <linux/types.h>
@@ -201,6 +202,8 @@ static inline void fwnode_handle_put(struct fwnode_handle *fwnode)
 	fwnode_call_void_op(fwnode, put);
 }
 
+DEFINE_FREE(fwnode_handle, struct fwnode_handle *, fwnode_handle_put(_T))
+
 int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index);
 int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);
 
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 03/15] iio: temp: ltc2983: Use __free(fwnode_handle) to replace fwnode_handle_put() calls
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 02/15] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19 13:52   ` Nuno Sá
  2024-02-17 16:42 ` [PATCH v4 04/15] device property: Introduce device_for_each_child_node_scoped() Jonathan Cameron
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This use of the new cleanup.h scope based freeing infrastructure allows
us to exit directly from error conditions and in the good path with
the reference obtained from fwnode_find_reference() (which may be an error
pointer) automatically released.

Cc: Cosmin Tanislav <cosmin.tanislav@analog.com>
Cc: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v2: Move the declarations down to where they are assigned.
This both clearly associates the cleanup with the action that it
is cleaning up and avoid potential future missordering of cleanup.
---
 drivers/iio/temperature/ltc2983.c | 76 ++++++++++---------------------
 1 file changed, 24 insertions(+), 52 deletions(-)

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index fcb96c44d954..47380d5e6b92 100644
--- a/drivers/iio/temperature/ltc2983.c
+++ b/drivers/iio/temperature/ltc2983.c
@@ -656,7 +656,6 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
 			 const struct ltc2983_sensor *sensor)
 {
 	struct ltc2983_thermocouple *thermo;
-	struct fwnode_handle *ref;
 	u32 oc_current;
 	int ret;
 
@@ -703,7 +702,8 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
 		return ERR_PTR(-EINVAL);
 	}
 
-	ref = fwnode_find_reference(child, "adi,cold-junction-handle", 0);
+	struct fwnode_handle *ref __free(fwnode_handle) =
+		fwnode_find_reference(child, "adi,cold-junction-handle", 0);
 	if (IS_ERR(ref)) {
 		ref = NULL;
 	} else {
@@ -714,7 +714,7 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
 			 * the error right away.
 			 */
 			dev_err(&st->spi->dev, "Property reg must be given\n");
-			goto fail;
+			return ERR_PTR(ret);
 		}
 	}
 
@@ -725,22 +725,15 @@ ltc2983_thermocouple_new(const struct fwnode_handle *child, struct ltc2983_data
 		thermo->custom = __ltc2983_custom_sensor_new(st, child,
 							     propname, false,
 							     16384, true);
-		if (IS_ERR(thermo->custom)) {
-			ret = PTR_ERR(thermo->custom);
-			goto fail;
-		}
+		if (IS_ERR(thermo->custom))
+			return ERR_CAST(thermo->custom);
 	}
 
 	/* set common parameters */
 	thermo->sensor.fault_handler = ltc2983_thermocouple_fault_handler;
 	thermo->sensor.assign_chan = ltc2983_thermocouple_assign_chan;
 
-	fwnode_handle_put(ref);
 	return &thermo->sensor;
-
-fail:
-	fwnode_handle_put(ref);
-	return ERR_PTR(ret);
 }
 
 static struct ltc2983_sensor *
@@ -750,14 +743,14 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
 	struct ltc2983_rtd *rtd;
 	int ret = 0;
 	struct device *dev = &st->spi->dev;
-	struct fwnode_handle *ref;
 	u32 excitation_current = 0, n_wires = 0;
 
 	rtd = devm_kzalloc(dev, sizeof(*rtd), GFP_KERNEL);
 	if (!rtd)
 		return ERR_PTR(-ENOMEM);
 
-	ref = fwnode_find_reference(child, "adi,rsense-handle", 0);
+	struct fwnode_handle *ref __free(fwnode_handle) =
+		fwnode_find_reference(child, "adi,rsense-handle", 0);
 	if (IS_ERR(ref)) {
 		dev_err(dev, "Property adi,rsense-handle missing or invalid");
 		return ERR_CAST(ref);
@@ -766,7 +759,7 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
 	ret = fwnode_property_read_u32(ref, "reg", &rtd->r_sense_chan);
 	if (ret) {
 		dev_err(dev, "Property reg must be given\n");
-		goto fail;
+		return ERR_PTR(ret);
 	}
 
 	ret = fwnode_property_read_u32(child, "adi,number-of-wires", &n_wires);
@@ -787,8 +780,7 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
 			break;
 		default:
 			dev_err(dev, "Invalid number of wires:%u\n", n_wires);
-			ret = -EINVAL;
-			goto fail;
+			return ERR_PTR(-EINVAL);
 		}
 	}
 
@@ -798,8 +790,7 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
 			if (n_wires == 2 || n_wires == 3) {
 				dev_err(dev,
 					"Rotation not allowed for 2/3 Wire RTDs");
-				ret = -EINVAL;
-				goto fail;
+				return ERR_PTR(-EINVAL);
 			}
 			rtd->sensor_config |= LTC2983_RTD_C_ROTATE(1);
 		} else {
@@ -829,16 +820,14 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
 				"Invalid rsense chann:%d to use in kelvin rsense",
 				rtd->r_sense_chan);
 
-			ret = -EINVAL;
-			goto fail;
+			return ERR_PTR(-EINVAL);
 		}
 
 		if (sensor->chan < min || sensor->chan > max) {
 			dev_err(dev, "Invalid chann:%d for the rtd config",
 				sensor->chan);
 
-			ret = -EINVAL;
-			goto fail;
+			return ERR_PTR(-EINVAL);
 		}
 	} else {
 		/* same as differential case */
@@ -846,8 +835,7 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
 			dev_err(&st->spi->dev,
 				"Invalid chann:%d for RTD", sensor->chan);
 
-			ret = -EINVAL;
-			goto fail;
+			return ERR_PTR(-EINVAL);
 		}
 	}
 
@@ -856,10 +844,8 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
 		rtd->custom = __ltc2983_custom_sensor_new(st, child,
 							  "adi,custom-rtd",
 							  false, 2048, false);
-		if (IS_ERR(rtd->custom)) {
-			ret = PTR_ERR(rtd->custom);
-			goto fail;
-		}
+		if (IS_ERR(rtd->custom))
+			return ERR_CAST(rtd->custom);
 	}
 
 	/* set common parameters */
@@ -901,18 +887,13 @@ ltc2983_rtd_new(const struct fwnode_handle *child, struct ltc2983_data *st,
 			dev_err(&st->spi->dev,
 				"Invalid value for excitation current(%u)",
 				excitation_current);
-			ret = -EINVAL;
-			goto fail;
+			return ERR_PTR(-EINVAL);
 		}
 	}
 
 	fwnode_property_read_u32(child, "adi,rtd-curve", &rtd->rtd_curve);
 
-	fwnode_handle_put(ref);
 	return &rtd->sensor;
-fail:
-	fwnode_handle_put(ref);
-	return ERR_PTR(ret);
 }
 
 static struct ltc2983_sensor *
@@ -921,7 +902,6 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
 {
 	struct ltc2983_thermistor *thermistor;
 	struct device *dev = &st->spi->dev;
-	struct fwnode_handle *ref;
 	u32 excitation_current = 0;
 	int ret = 0;
 
@@ -929,7 +909,8 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
 	if (!thermistor)
 		return ERR_PTR(-ENOMEM);
 
-	ref = fwnode_find_reference(child, "adi,rsense-handle", 0);
+	struct fwnode_handle *ref __free(fwnode_handle) =
+		fwnode_find_reference(child, "adi,rsense-handle", 0);
 	if (IS_ERR(ref)) {
 		dev_err(dev, "Property adi,rsense-handle missing or invalid");
 		return ERR_CAST(ref);
@@ -938,7 +919,7 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
 	ret = fwnode_property_read_u32(ref, "reg", &thermistor->r_sense_chan);
 	if (ret) {
 		dev_err(dev, "rsense channel must be configured...\n");
-		goto fail;
+		return ERR_PTR(ret);
 	}
 
 	if (fwnode_property_read_bool(child, "adi,single-ended")) {
@@ -958,8 +939,7 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
 		dev_err(&st->spi->dev,
 			"Invalid chann:%d for differential thermistor",
 			sensor->chan);
-		ret = -EINVAL;
-		goto fail;
+		return ERR_PTR(-EINVAL);
 	}
 
 	/* check custom sensor */
@@ -978,10 +958,8 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
 								 propname,
 								 steinhart,
 								 64, false);
-		if (IS_ERR(thermistor->custom)) {
-			ret = PTR_ERR(thermistor->custom);
-			goto fail;
-		}
+		if (IS_ERR(thermistor->custom))
+			return ERR_CAST(thermistor->custom);
 	}
 	/* set common parameters */
 	thermistor->sensor.fault_handler = ltc2983_common_fault_handler;
@@ -1005,8 +983,7 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
 			    LTC2983_SENSOR_THERMISTOR_STEINHART) {
 				dev_err(&st->spi->dev,
 					"Auto Range not allowed for custom sensors\n");
-				ret = -EINVAL;
-				goto fail;
+				return ERR_PTR(-EINVAL);
 			}
 			thermistor->excitation_current = 0x0c;
 			break;
@@ -1047,16 +1024,11 @@ ltc2983_thermistor_new(const struct fwnode_handle *child, struct ltc2983_data *s
 			dev_err(&st->spi->dev,
 				"Invalid value for excitation current(%u)",
 				excitation_current);
-			ret = -EINVAL;
-			goto fail;
+			return ERR_PTR(-EINVAL);
 		}
 	}
 
-	fwnode_handle_put(ref);
 	return &thermistor->sensor;
-fail:
-	fwnode_handle_put(ref);
-	return ERR_PTR(ret);
 }
 
 static struct ltc2983_sensor *
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 04/15] device property: Introduce device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (2 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 03/15] iio: temp: ltc2983: Use __free(fwnode_handle) to replace fwnode_handle_put() calls Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19  8:56   ` Greg Kroah-Hartman
  2024-02-20  6:59   ` Sakari Ailus
  2024-02-17 16:42 ` [PATCH v4 05/15] iio: adc: max11410: Use device_for_each_child_node_scoped() Jonathan Cameron
                   ` (11 subsequent siblings)
  15 siblings, 2 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Similar to recently propose for_each_child_of_node_scoped() this
new version of the loop macro instantiates a new local
struct fwnode_handle * that uses the __free(fwnode_handle) auto
cleanup handling so that if a reference to a node is held on early
exit from the loop the reference will be released. If the loop
runs to completion, the child pointer will be NULL and no action will
be taken.

The reason this is useful is that it removes the need for
fwnode_handle_put() on early loop exits.  If there is a need
to retain the reference, then return_ptr(child) or no_free_ptr(child)
may be used to safely disable the auto cleanup.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v4: Include the alignement changes that were in patch 15 of v3 with
    a tweak as I missed the first line. Thanks Andy!

 include/linux/property.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 9e67c3c4df6e..eefd662a2f9d 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -182,6 +182,11 @@ struct fwnode_handle *device_get_next_child_node(const struct device *dev,
 	for (child = device_get_next_child_node(dev, NULL); child;	\
 	     child = device_get_next_child_node(dev, child))
 
+#define device_for_each_child_node_scoped(dev, child)			\
+	for (struct fwnode_handle *child __free(fwnode_handle) =	\
+		device_get_next_child_node(dev, NULL);			\
+	     child; child = device_get_next_child_node(dev, child))
+
 struct fwnode_handle *fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
 						  const char *childname);
 struct fwnode_handle *device_get_named_child_node(const struct device *dev,
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 05/15] iio: adc: max11410: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (3 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 04/15] device property: Introduce device_for_each_child_node_scoped() Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19 13:54   ` Nuno Sá
  2024-02-17 16:42 ` [PATCH v4 06/15] iio: adc: mcp3564: " Jonathan Cameron
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Nuno Sá <nuno.sa@analog.com>
Cc: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/max11410.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/max11410.c b/drivers/iio/adc/max11410.c
index 6af829349b4e..45368850b220 100644
--- a/drivers/iio/adc/max11410.c
+++ b/drivers/iio/adc/max11410.c
@@ -696,7 +696,6 @@ static int max11410_parse_channels(struct max11410_state *st,
 	struct device *dev = &st->spi_dev->dev;
 	struct max11410_channel_config *cfg;
 	struct iio_chan_spec *channels;
-	struct fwnode_handle *child;
 	u32 reference, sig_path;
 	const char *node_name;
 	u32 inputs[2], scale;
@@ -720,7 +719,7 @@ static int max11410_parse_channels(struct max11410_state *st,
 	if (!st->channels)
 		return -ENOMEM;
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		node_name = fwnode_get_name(child);
 		if (fwnode_property_present(child, "diff-channels")) {
 			ret = fwnode_property_read_u32_array(child,
@@ -735,47 +734,37 @@ static int max11410_parse_channels(struct max11410_state *st,
 			inputs[1] = 0;
 			chanspec.differential = 0;
 		}
-		if (ret) {
-			fwnode_handle_put(child);
+		if (ret)
 			return ret;
-		}
 
 		if (inputs[0] > MAX11410_CHANNEL_INDEX_MAX ||
-		    inputs[1] > MAX11410_CHANNEL_INDEX_MAX) {
-			fwnode_handle_put(child);
+		    inputs[1] > MAX11410_CHANNEL_INDEX_MAX)
 			return dev_err_probe(&indio_dev->dev, -EINVAL,
 					     "Invalid channel index for %s, should be less than %d\n",
 					     node_name,
 					     MAX11410_CHANNEL_INDEX_MAX + 1);
-		}
 
 		cfg = &st->channels[chan_idx];
 
 		reference = MAX11410_REFSEL_AVDD_AGND;
 		fwnode_property_read_u32(child, "adi,reference", &reference);
-		if (reference > MAX11410_REFSEL_MAX) {
-			fwnode_handle_put(child);
+		if (reference > MAX11410_REFSEL_MAX)
 			return dev_err_probe(&indio_dev->dev, -EINVAL,
 					     "Invalid adi,reference value for %s, should be less than %d.\n",
 					     node_name, MAX11410_REFSEL_MAX + 1);
-		}
 
 		if (!max11410_get_vrefp(st, reference) ||
-		    (!max11410_get_vrefn(st, reference) && reference <= 2)) {
-			fwnode_handle_put(child);
+		    (!max11410_get_vrefn(st, reference) && reference <= 2))
 			return dev_err_probe(&indio_dev->dev, -EINVAL,
 					     "Invalid VREF configuration for %s, either specify corresponding VREF regulators or change adi,reference property.\n",
 					     node_name);
-		}
 
 		sig_path = MAX11410_PGA_SIG_PATH_BUFFERED;
 		fwnode_property_read_u32(child, "adi,input-mode", &sig_path);
-		if (sig_path > MAX11410_SIG_PATH_MAX) {
-			fwnode_handle_put(child);
+		if (sig_path > MAX11410_SIG_PATH_MAX)
 			return dev_err_probe(&indio_dev->dev, -EINVAL,
 					     "Invalid adi,input-mode value for %s, should be less than %d.\n",
 					     node_name, MAX11410_SIG_PATH_MAX + 1);
-		}
 
 		fwnode_property_read_u32(child, "settling-time-us",
 					 &cfg->settling_time_us);
@@ -793,10 +782,8 @@ static int max11410_parse_channels(struct max11410_state *st,
 			cfg->scale_avail = devm_kcalloc(dev, MAX11410_SCALE_AVAIL_SIZE * 2,
 							sizeof(*cfg->scale_avail),
 							GFP_KERNEL);
-			if (!cfg->scale_avail) {
-				fwnode_handle_put(child);
+			if (!cfg->scale_avail)
 				return -ENOMEM;
-			}
 
 			scale = max11410_get_scale(st, *cfg);
 			for (i = 0; i < MAX11410_SCALE_AVAIL_SIZE; i++) {
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 06/15] iio: adc: mcp3564: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (4 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 05/15] iio: adc: max11410: Use device_for_each_child_node_scoped() Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 07/15] iio: adc: qcom-spmi-adc5: " Jonathan Cameron
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Marius Cristea <marius.cristea@microchip.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/mcp3564.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/mcp3564.c b/drivers/iio/adc/mcp3564.c
index 311b613b6057..e2ae13f1e842 100644
--- a/drivers/iio/adc/mcp3564.c
+++ b/drivers/iio/adc/mcp3564.c
@@ -998,7 +998,6 @@ static int mcp3564_parse_fw_children(struct iio_dev *indio_dev)
 	struct mcp3564_state *adc = iio_priv(indio_dev);
 	struct device *dev = &adc->spi->dev;
 	struct iio_chan_spec *channels;
-	struct fwnode_handle *child;
 	struct iio_chan_spec chanspec = mcp3564_channel_template;
 	struct iio_chan_spec temp_chanspec = mcp3564_temp_channel_template;
 	struct iio_chan_spec burnout_chanspec = mcp3564_burnout_channel_template;
@@ -1025,7 +1024,7 @@ static int mcp3564_parse_fw_children(struct iio_dev *indio_dev)
 	if (!channels)
 		return dev_err_probe(dev, -ENOMEM, "Can't allocate memory\n");
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		node_name = fwnode_get_name(child);
 
 		if (fwnode_property_present(child, "diff-channels")) {
@@ -1033,26 +1032,25 @@ static int mcp3564_parse_fw_children(struct iio_dev *indio_dev)
 							     "diff-channels",
 							     inputs,
 							     ARRAY_SIZE(inputs));
+			if (ret)
+				return ret;
+
 			chanspec.differential = 1;
 		} else {
 			ret = fwnode_property_read_u32(child, "reg", &inputs[0]);
+			if (ret)
+				return ret;
 
 			chanspec.differential = 0;
 			inputs[1] = MCP3564_AGND;
 		}
-		if (ret) {
-			fwnode_handle_put(child);
-			return ret;
-		}
 
 		if (inputs[0] > MCP3564_INTERNAL_VCM ||
-		    inputs[1] > MCP3564_INTERNAL_VCM) {
-			fwnode_handle_put(child);
+		    inputs[1] > MCP3564_INTERNAL_VCM)
 			return dev_err_probe(&indio_dev->dev, -EINVAL,
 					     "Channel index > %d, for %s\n",
 					     MCP3564_INTERNAL_VCM + 1,
 					     node_name);
-		}
 
 		chanspec.address = (inputs[0] << 4) | inputs[1];
 		chanspec.channel = inputs[0];
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 07/15] iio: adc: qcom-spmi-adc5: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (5 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 06/15] iio: adc: mcp3564: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 08/15] iio: adc: rzg2l_adc: " Jonathan Cameron
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

A slightly less convincing usecase than many as all the failure paths
are wrapped up in a call to a per fwnode_handle utility function.
The complexity in that function is sufficient that it makes sense to
factor it out even if it this new auto cleanup would enable simpler
returns if the code was inline at the call site. Hence I've left it alone.

Cc: Marijn Suijten <marijn.suijten@somainline.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/qcom-spmi-adc5.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
index b6b612d733ff..9b69f40beed8 100644
--- a/drivers/iio/adc/qcom-spmi-adc5.c
+++ b/drivers/iio/adc/qcom-spmi-adc5.c
@@ -825,7 +825,6 @@ static int adc5_get_fw_data(struct adc5_chip *adc)
 	const struct adc5_channels *adc_chan;
 	struct iio_chan_spec *iio_chan;
 	struct adc5_channel_prop prop, *chan_props;
-	struct fwnode_handle *child;
 	unsigned int index = 0;
 	int ret;
 
@@ -849,12 +848,10 @@ static int adc5_get_fw_data(struct adc5_chip *adc)
 	if (!adc->data)
 		adc->data = &adc5_data_pmic;
 
-	device_for_each_child_node(adc->dev, child) {
+	device_for_each_child_node_scoped(adc->dev, child) {
 		ret = adc5_get_fw_channel_data(adc, &prop, child, adc->data);
-		if (ret) {
-			fwnode_handle_put(child);
+		if (ret)
 			return ret;
-		}
 
 		prop.scale_fn_type =
 			adc->data->adc_chans[prop.channel].scale_fn_type;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 08/15] iio: adc: rzg2l_adc: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (6 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 07/15] iio: adc: qcom-spmi-adc5: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 09/15] iio: adc: stm32: " Jonathan Cameron
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/rzg2l_adc.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
index 0921ff2d9b3a..cd3a7e46ea53 100644
--- a/drivers/iio/adc/rzg2l_adc.c
+++ b/drivers/iio/adc/rzg2l_adc.c
@@ -302,7 +302,6 @@ static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id)
 static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l_adc *adc)
 {
 	struct iio_chan_spec *chan_array;
-	struct fwnode_handle *fwnode;
 	struct rzg2l_adc_data *data;
 	unsigned int channel;
 	int num_channels;
@@ -330,17 +329,13 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l
 		return -ENOMEM;
 
 	i = 0;
-	device_for_each_child_node(&pdev->dev, fwnode) {
+	device_for_each_child_node_scoped(&pdev->dev, fwnode) {
 		ret = fwnode_property_read_u32(fwnode, "reg", &channel);
-		if (ret) {
-			fwnode_handle_put(fwnode);
+		if (ret)
 			return ret;
-		}
 
-		if (channel >= RZG2L_ADC_MAX_CHANNELS) {
-			fwnode_handle_put(fwnode);
+		if (channel >= RZG2L_ADC_MAX_CHANNELS)
 			return -EINVAL;
-		}
 
 		chan_array[i].type = IIO_VOLTAGE;
 		chan_array[i].indexed = 1;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 09/15] iio: adc: stm32: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (7 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 08/15] iio: adc: rzg2l_adc: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19 11:45   ` Andy Shevchenko
  2024-02-17 16:42 ` [PATCH v4 10/15] iio: adc: ti-ads1015: " Jonathan Cameron
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Note this includes a probable fix as in one path an error message was
printed with ret == 0.

Took advantage of dev_err_probe() to futher simplify things given no
longer a need for the goto err.

Cc: Olivier Moysan <olivier.moysan@foss.st.com>
Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/stm32-adc.c | 63 ++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index b5d3c9cea5c4..9fd94a792f42 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -2187,58 +2187,50 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 				       struct iio_chan_spec *channels)
 {
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
-	struct fwnode_handle *child;
 	const char *name;
 	int val, scan_index = 0, ret;
 	bool differential;
 	u32 vin[2];
 
-	device_for_each_child_node(&indio_dev->dev, child) {
+	device_for_each_child_node_scoped(&indio_dev->dev, child) {
 		ret = fwnode_property_read_u32(child, "reg", &val);
-		if (ret) {
-			dev_err(&indio_dev->dev, "Missing channel index %d\n", ret);
-			goto err;
-		}
+		if (ret)
+			return dev_err_probe(&indio_dev->dev, ret,
+					     "Missing channel index\n");
 
 		ret = fwnode_property_read_string(child, "label", &name);
 		/* label is optional */
 		if (!ret) {
-			if (strlen(name) >= STM32_ADC_CH_SZ) {
-				dev_err(&indio_dev->dev, "Label %s exceeds %d characters\n",
-					name, STM32_ADC_CH_SZ);
-				ret = -EINVAL;
-				goto err;
-			}
+			if (strlen(name) >= STM32_ADC_CH_SZ)
+				return dev_err_probe(&indio_dev->dev, -EINVAL,
+						     "Label %s exceeds %d characters\n",
+						     name, STM32_ADC_CH_SZ);
+
 			strscpy(adc->chan_name[val], name, STM32_ADC_CH_SZ);
 			ret = stm32_adc_populate_int_ch(indio_dev, name, val);
 			if (ret == -ENOENT)
 				continue;
 			else if (ret)
-				goto err;
-		} else if (ret != -EINVAL) {
-			dev_err(&indio_dev->dev, "Invalid label %d\n", ret);
-			goto err;
-		}
+				return ret;
+		} else if (ret != -EINVAL)
+			return dev_err_probe(&indio_dev->dev, ret, "Invalid label\n");
 
-		if (val >= adc_info->max_channels) {
-			dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
-			ret = -EINVAL;
-			goto err;
-		}
+		if (val >= adc_info->max_channels)
+			return dev_err_probe(&indio_dev->dev, -EINVAL,
+					     "Invalid channel %d\n", val);
 
 		differential = false;
 		ret = fwnode_property_read_u32_array(child, "diff-channels", vin, 2);
 		/* diff-channels is optional */
 		if (!ret) {
 			differential = true;
-			if (vin[0] != val || vin[1] >= adc_info->max_channels) {
-				dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
-					vin[0], vin[1]);
-				goto err;
-			}
+			if (vin[0] != val || vin[1] >= adc_info->max_channels)
+				return dev_err_probe(&indio_dev->dev, -EINVAL,
+						     "Invalid channel in%d-in%d\n",
+						     vin[0], vin[1]);
 		} else if (ret != -EINVAL) {
-			dev_err(&indio_dev->dev, "Invalid diff-channels property %d\n", ret);
-			goto err;
+			return dev_err_probe(&indio_dev->dev, ret,
+					     "Invalid diff-channels property\n");
 		}
 
 		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
@@ -2247,11 +2239,9 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 		val = 0;
 		ret = fwnode_property_read_u32(child, "st,min-sample-time-ns", &val);
 		/* st,min-sample-time-ns is optional */
-		if (ret && ret != -EINVAL) {
-			dev_err(&indio_dev->dev, "Invalid st,min-sample-time-ns property %d\n",
-				ret);
-			goto err;
-		}
+		if (ret && ret != -EINVAL)
+			return dev_err_probe(&indio_dev->dev, ret,
+					     "Invalid st,min-sample-time-ns property\n");
 
 		stm32_adc_smpr_init(adc, channels[scan_index].channel, val);
 		if (differential)
@@ -2261,11 +2251,6 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 	}
 
 	return scan_index;
-
-err:
-	fwnode_handle_put(child);
-
-	return ret;
 }
 
 static int stm32_adc_chan_fw_init(struct iio_dev *indio_dev, bool timestamping)
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 10/15] iio: adc: ti-ads1015: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (8 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 09/15] iio: adc: stm32: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 11/15] iio: adc: ti-ads131e08: " Jonathan Cameron
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-ads1015.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 6ae967e4d8fa..d3363d02f292 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -902,10 +902,9 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 	struct ads1015_data *data = iio_priv(indio_dev);
 	struct device *dev = &client->dev;
-	struct fwnode_handle *node;
 	int i = -1;
 
-	device_for_each_child_node(dev, node) {
+	device_for_each_child_node_scoped(dev, node) {
 		u32 pval;
 		unsigned int channel;
 		unsigned int pga = ADS1015_DEFAULT_PGA;
@@ -927,7 +926,6 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
 			pga = pval;
 			if (pga > 5) {
 				dev_err(dev, "invalid gain on %pfw\n", node);
-				fwnode_handle_put(node);
 				return -EINVAL;
 			}
 		}
@@ -936,7 +934,6 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
 			data_rate = pval;
 			if (data_rate > 7) {
 				dev_err(dev, "invalid data_rate on %pfw\n", node);
-				fwnode_handle_put(node);
 				return -EINVAL;
 			}
 		}
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 11/15] iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (9 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 10/15] iio: adc: ti-ads1015: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 12/15] iio: addac: ad74413r: " Jonathan Cameron
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Tomislav Denis <tomislav.denis@avl.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-ads131e08.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ti-ads131e08.c b/drivers/iio/adc/ti-ads131e08.c
index fcfc46254313..f653654f7c5d 100644
--- a/drivers/iio/adc/ti-ads131e08.c
+++ b/drivers/iio/adc/ti-ads131e08.c
@@ -694,7 +694,6 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 	struct ads131e08_channel_config *channel_config;
 	struct device *dev = &st->spi->dev;
 	struct iio_chan_spec *channels;
-	struct fwnode_handle *node;
 	unsigned int channel, tmp;
 	int num_channels, i, ret;
 
@@ -736,10 +735,10 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 		return -ENOMEM;
 
 	i = 0;
-	device_for_each_child_node(dev, node) {
+	device_for_each_child_node_scoped(dev, node) {
 		ret = fwnode_property_read_u32(node, "reg", &channel);
 		if (ret)
-			goto err_child_out;
+			return ret;
 
 		ret = fwnode_property_read_u32(node, "ti,gain", &tmp);
 		if (ret) {
@@ -747,7 +746,7 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 		} else {
 			ret = ads131e08_pga_gain_to_field_value(st, tmp);
 			if (ret < 0)
-				goto err_child_out;
+				return ret;
 
 			channel_config[i].pga_gain = tmp;
 		}
@@ -758,7 +757,7 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 		} else {
 			ret = ads131e08_validate_channel_mux(st, tmp);
 			if (ret)
-				goto err_child_out;
+				return ret;
 
 			channel_config[i].mux = tmp;
 		}
@@ -784,10 +783,6 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 	st->channel_config = channel_config;
 
 	return 0;
-
-err_child_out:
-	fwnode_handle_put(node);
-	return ret;
 }
 
 static void ads131e08_regulator_disable(void *data)
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 12/15] iio: addac: ad74413r: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (10 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 11/15] iio: adc: ti-ads131e08: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19 13:55   ` Nuno Sá
  2024-02-17 16:42 ` [PATCH v4 13/15] iio: dac: ad3552r: " Jonathan Cameron
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

The use of fwnode_for_each_available_child_node() here is assumed
to have been down to a false assumption that device_for_each_child_node()
doesn't check avaialble - so this transition to the scoped
device_for_each_child_node_scoped() is equivalent.

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/addac/ad74413r.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index 7af3e4b8fe3b..cd26a16dc0ff 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -1255,21 +1255,15 @@ static int ad74413r_parse_channel_config(struct iio_dev *indio_dev,
 static int ad74413r_parse_channel_configs(struct iio_dev *indio_dev)
 {
 	struct ad74413r_state *st = iio_priv(indio_dev);
-	struct fwnode_handle *channel_node = NULL;
 	int ret;
 
-	fwnode_for_each_available_child_node(dev_fwnode(st->dev), channel_node) {
+	device_for_each_child_node_scoped(st->dev, channel_node) {
 		ret = ad74413r_parse_channel_config(indio_dev, channel_node);
 		if (ret)
-			goto put_channel_node;
+			return ret;
 	}
 
 	return 0;
-
-put_channel_node:
-	fwnode_handle_put(channel_node);
-
-	return ret;
 }
 
 static int ad74413r_setup_channels(struct iio_dev *indio_dev)
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 13/15] iio: dac: ad3552r: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (11 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 12/15] iio: addac: ad74413r: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19 11:46   ` Andy Shevchenko
  2024-02-17 16:42 ` [PATCH v4 14/15] iio: dac: ad5770r: " Jonathan Cameron
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Removing the goto err; statements also allows more extensive use of
dev_err_probe() further simplifying the code.

Cc: Mihail Chindris <mihail.chindris@analog.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/dac/ad3552r.c | 51 +++++++++++++++------------------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index a492e8f2fc0f..e14a065b29ca 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -880,7 +880,6 @@ static void ad3552r_reg_disable(void *reg)
 static int ad3552r_configure_device(struct ad3552r_desc *dac)
 {
 	struct device *dev = &dac->spi->dev;
-	struct fwnode_handle *child;
 	struct regulator *vref;
 	int err, cnt = 0, voltage, delta = 100000;
 	u32 vals[2], val, ch;
@@ -949,53 +948,45 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 		return -ENODEV;
 	}
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		err = fwnode_property_read_u32(child, "reg", &ch);
-		if (err) {
-			dev_err(dev, "mandatory reg property missing\n");
-			goto put_child;
-		}
-		if (ch >= AD3552R_NUM_CH) {
-			dev_err(dev, "reg must be less than %d\n",
-				AD3552R_NUM_CH);
-			err = -EINVAL;
-			goto put_child;
-		}
+		if (err)
+			return dev_err_probe(dev, err,
+					     "mandatory reg property missing\n");
+		if (ch >= AD3552R_NUM_CH)
+			return dev_err_probe(dev, -EINVAL,
+					     "reg must be less than %d\n",
+					     AD3552R_NUM_CH);
 
 		if (fwnode_property_present(child, "adi,output-range-microvolt")) {
 			err = fwnode_property_read_u32_array(child,
 							     "adi,output-range-microvolt",
 							     vals,
 							     2);
-			if (err) {
-				dev_err(dev,
+			if (err)
+				return dev_err_probe(dev, err,
 					"adi,output-range-microvolt property could not be parsed\n");
-				goto put_child;
-			}
 
 			err = ad3552r_find_range(dac->chip_id, vals);
-			if (err < 0) {
-				dev_err(dev,
-					"Invalid adi,output-range-microvolt value\n");
-				goto put_child;
-			}
+			if (err < 0)
+				return dev_err_probe(dev, err,
+						     "Invalid adi,output-range-microvolt value\n");
+
 			val = err;
 			err = ad3552r_set_ch_value(dac,
 						   AD3552R_CH_OUTPUT_RANGE_SEL,
 						   ch, val);
 			if (err)
-				goto put_child;
+				return err;
 
 			dac->ch_data[ch].range = val;
 		} else if (dac->chip_id == AD3542R_ID) {
-			dev_err(dev,
-				"adi,output-range-microvolt is required for ad3542r\n");
-			err = -EINVAL;
-			goto put_child;
+			return dev_err_probe(dev, -EINVAL,
+					     "adi,output-range-microvolt is required for ad3542r\n");
 		} else {
 			err = ad3552r_configure_custom_gain(dac, child, ch);
 			if (err)
-				goto put_child;
+				return err;
 		}
 
 		ad3552r_calc_gain_and_offset(dac, ch);
@@ -1003,7 +994,7 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 
 		err = ad3552r_set_ch_value(dac, AD3552R_CH_SELECT, ch, 1);
 		if (err < 0)
-			goto put_child;
+			return err;
 
 		dac->channels[cnt] = AD3552R_CH_DAC(ch);
 		++cnt;
@@ -1021,10 +1012,6 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 	dac->num_ch = cnt;
 
 	return 0;
-put_child:
-	fwnode_handle_put(child);
-
-	return err;
 }
 
 static int ad3552r_init(struct ad3552r_desc *dac)
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 14/15] iio: dac: ad5770r: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (12 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 13/15] iio: dac: ad3552r: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-17 16:42 ` [PATCH v4 15/15] iio: dac: ltc2688: " Jonathan Cameron
  2024-02-19 11:49 ` [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Andy Shevchenko
  15 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/dac/ad5770r.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/dac/ad5770r.c b/drivers/iio/dac/ad5770r.c
index f66d67402e43..c360ebf5297a 100644
--- a/drivers/iio/dac/ad5770r.c
+++ b/drivers/iio/dac/ad5770r.c
@@ -515,39 +515,32 @@ static int ad5770r_channel_config(struct ad5770r_state *st)
 {
 	int ret, tmp[2], min, max;
 	unsigned int num;
-	struct fwnode_handle *child;
 
 	num = device_get_child_node_count(&st->spi->dev);
 	if (num != AD5770R_MAX_CHANNELS)
 		return -EINVAL;
 
-	device_for_each_child_node(&st->spi->dev, child) {
+	device_for_each_child_node_scoped(&st->spi->dev, child) {
 		ret = fwnode_property_read_u32(child, "reg", &num);
 		if (ret)
-			goto err_child_out;
-		if (num >= AD5770R_MAX_CHANNELS) {
-			ret = -EINVAL;
-			goto err_child_out;
-		}
+			return ret;
+		if (num >= AD5770R_MAX_CHANNELS)
+			return -EINVAL;
 
 		ret = fwnode_property_read_u32_array(child,
 						     "adi,range-microamp",
 						     tmp, 2);
 		if (ret)
-			goto err_child_out;
+			return ret;
 
 		min = tmp[0] / 1000;
 		max = tmp[1] / 1000;
 		ret = ad5770r_store_output_range(st, min, max, num);
 		if (ret)
-			goto err_child_out;
+			return ret;
 	}
 
 	return 0;
-
-err_child_out:
-	fwnode_handle_put(child);
-	return ret;
 }
 
 static int ad5770r_init(struct ad5770r_state *st)
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v4 15/15] iio: dac: ltc2688: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (13 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 14/15] iio: dac: ad5770r: " Jonathan Cameron
@ 2024-02-17 16:42 ` Jonathan Cameron
  2024-02-19 11:48   ` Andy Shevchenko
  2024-02-19 13:49   ` Nuno Sá
  2024-02-19 11:49 ` [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Andy Shevchenko
  15 siblings, 2 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-17 16:42 UTC (permalink / raw)
  To: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Nuno Sá <nuno.sa@analog.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v4: Moved alignment changes back to patch 4.
v3: Tweaked the alignment after comments from Andy.

 drivers/iio/dac/ltc2688.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/dac/ltc2688.c b/drivers/iio/dac/ltc2688.c
index fc8eb53c65be..b71df03fc13b 100644
--- a/drivers/iio/dac/ltc2688.c
+++ b/drivers/iio/dac/ltc2688.c
@@ -746,26 +746,21 @@ static int ltc2688_span_lookup(const struct ltc2688_state *st, int min, int max)
 static int ltc2688_channel_config(struct ltc2688_state *st)
 {
 	struct device *dev = &st->spi->dev;
-	struct fwnode_handle *child;
 	u32 reg, clk_input, val, tmp[2];
 	int ret, span;
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		struct ltc2688_chan *chan;
 
 		ret = fwnode_property_read_u32(child, "reg", &reg);
-		if (ret) {
-			fwnode_handle_put(child);
+		if (ret)
 			return dev_err_probe(dev, ret,
 					     "Failed to get reg property\n");
-		}
 
-		if (reg >= LTC2688_DAC_CHANNELS) {
-			fwnode_handle_put(child);
+		if (reg >= LTC2688_DAC_CHANNELS)
 			return dev_err_probe(dev, -EINVAL,
 					     "reg bigger than: %d\n",
 					     LTC2688_DAC_CHANNELS);
-		}
 
 		val = 0;
 		chan = &st->channels[reg];
@@ -786,12 +781,10 @@ static int ltc2688_channel_config(struct ltc2688_state *st)
 		if (!ret) {
 			span = ltc2688_span_lookup(st, (int)tmp[0] / 1000,
 						   tmp[1] / 1000);
-			if (span < 0) {
-				fwnode_handle_put(child);
+			if (span < 0)
 				return dev_err_probe(dev, -EINVAL,
 						     "output range not valid:[%d %d]\n",
 						     tmp[0], tmp[1]);
-			}
 
 			val |= FIELD_PREP(LTC2688_CH_SPAN_MSK, span);
 		}
@@ -800,17 +793,14 @@ static int ltc2688_channel_config(struct ltc2688_state *st)
 					       &clk_input);
 		if (!ret) {
 			if (clk_input >= LTC2688_CH_TGP_MAX) {
-				fwnode_handle_put(child);
 				return dev_err_probe(dev, -EINVAL,
 						     "toggle-dither-input inv value(%d)\n",
 						     clk_input);
 			}
 
 			ret = ltc2688_tgp_clk_setup(st, chan, child, clk_input);
-			if (ret) {
-				fwnode_handle_put(child);
+			if (ret)
 				return ret;
-			}
 
 			/*
 			 * 0 means software toggle which is the default mode.
@@ -844,11 +834,9 @@ static int ltc2688_channel_config(struct ltc2688_state *st)
 
 		ret = regmap_write(st->regmap, LTC2688_CMD_CH_SETTING(reg),
 				   val);
-		if (ret) {
-			fwnode_handle_put(child);
+		if (ret)
 			return dev_err_probe(dev, -EINVAL,
 					     "failed to set chan settings\n");
-		}
 	}
 
 	return 0;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h
  2024-02-17 16:42 ` [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h Jonathan Cameron
@ 2024-02-19  8:55   ` Greg Kroah-Hartman
  2024-02-20  6:55   ` Sakari Ailus
  1 sibling, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2024-02-19  8:55 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:35PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> By having this function as static inline in the header, the compiler
> is able to see if can optmize the call out if (IS_ERR_OR_NULL(fwnode))
> This will allow a simpler DEFINE_FREE() call in the following patch.
> 
> Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/base/property.c  | 14 --------------
>  include/linux/property.h | 14 +++++++++++++-
>  2 files changed, 13 insertions(+), 15 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 02/15] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup.
  2024-02-17 16:42 ` [PATCH v4 02/15] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup Jonathan Cameron
@ 2024-02-19  8:55   ` Greg Kroah-Hartman
  2024-02-20  6:55   ` Sakari Ailus
  1 sibling, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2024-02-19  8:55 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:36PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Useful where the fwnode_handle was obtained from a call such as
> fwnode_find_reference() as it will safely do nothing if IS_ERR() is true
> and will automatically release the reference on the variable leaving
> scope.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  include/linux/property.h | 3 +++
>  1 file changed, 3 insertions(+)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 04/15] device property: Introduce device_for_each_child_node_scoped()
  2024-02-17 16:42 ` [PATCH v4 04/15] device property: Introduce device_for_each_child_node_scoped() Jonathan Cameron
@ 2024-02-19  8:56   ` Greg Kroah-Hartman
  2024-02-20  6:59   ` Sakari Ailus
  1 sibling, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2024-02-19  8:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:38PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Similar to recently propose for_each_child_of_node_scoped() this
> new version of the loop macro instantiates a new local
> struct fwnode_handle * that uses the __free(fwnode_handle) auto
> cleanup handling so that if a reference to a node is held on early
> exit from the loop the reference will be released. If the loop
> runs to completion, the child pointer will be NULL and no action will
> be taken.
> 
> The reason this is useful is that it removes the need for
> fwnode_handle_put() on early loop exits.  If there is a need
> to retain the reference, then return_ptr(child) or no_free_ptr(child)
> may be used to safely disable the auto cleanup.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v4: Include the alignement changes that were in patch 15 of v3 with
>     a tweak as I missed the first line. Thanks Andy!
> 
>  include/linux/property.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 9e67c3c4df6e..eefd662a2f9d 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -182,6 +182,11 @@ struct fwnode_handle *device_get_next_child_node(const struct device *dev,
>  	for (child = device_get_next_child_node(dev, NULL); child;	\
>  	     child = device_get_next_child_node(dev, child))
>  
> +#define device_for_each_child_node_scoped(dev, child)			\
> +	for (struct fwnode_handle *child __free(fwnode_handle) =	\
> +		device_get_next_child_node(dev, NULL);			\
> +	     child; child = device_get_next_child_node(dev, child))
> +
>  struct fwnode_handle *fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
>  						  const char *childname);
>  struct fwnode_handle *device_get_named_child_node(const struct device *dev,

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 09/15] iio: adc: stm32: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 ` [PATCH v4 09/15] iio: adc: stm32: " Jonathan Cameron
@ 2024-02-19 11:45   ` Andy Shevchenko
  2024-02-24 12:16     ` Jonathan Cameron
  0 siblings, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2024-02-19 11:45 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:43PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> Note this includes a probable fix as in one path an error message was
> printed with ret == 0.
> 
> Took advantage of dev_err_probe() to futher simplify things given no
> longer a need for the goto err.

...

>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;

I believe with

	struct device *dev = &indio_dev->dev;

you can make the below neater.
Also see some side notes.

> -	struct fwnode_handle *child;
>  	const char *name;
>  	int val, scan_index = 0, ret;
>  	bool differential;
>  	u32 vin[2];

...

>  		if (!ret) {

Not a fan of this pattern, below we have two different patterns for the cases
like this :-(

> +			if (strlen(name) >= STM32_ADC_CH_SZ)
> +				return dev_err_probe(&indio_dev->dev, -EINVAL,
> +						     "Label %s exceeds %d characters\n",
> +						     name, STM32_ADC_CH_SZ);
> +
>  			strscpy(adc->chan_name[val], name, STM32_ADC_CH_SZ);
>  			ret = stm32_adc_populate_int_ch(indio_dev, name, val);
>  			if (ret == -ENOENT)
>  				continue;
>  			else if (ret)


This 'else' is redundant.

> +				return ret;
> +		} else if (ret != -EINVAL)

This also...

> +			return dev_err_probe(&indio_dev->dev, ret, "Invalid label\n");

...if you first do like

		if (ret && ret != -EINVAL)
			return dev_err_probe(...);
		if (!ret) {

Another option

		if (ret) {
			if (ret != -EINVAL)
				return dev_err_probe(...);
		} else {

...

>  		differential = false;
>  		ret = fwnode_property_read_u32_array(child, "diff-channels", vin, 2);

ARRAY_SIZE()?

>  		/* diff-channels is optional */

...

>  		if (!ret) {
> +			if (vin[0] != val || vin[1] >= adc_info->max_channels)
> +				return dev_err_probe(&indio_dev->dev, -EINVAL,
> +						     "Invalid channel in%d-in%d\n",
> +						     vin[0], vin[1]);
>  		} else if (ret != -EINVAL) {
> -			dev_err(&indio_dev->dev, "Invalid diff-channels property %d\n", ret);
> -			goto err;
> +			return dev_err_probe(&indio_dev->dev, ret,
> +					     "Invalid diff-channels property\n");
>  		}

As per above?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 13/15] iio: dac: ad3552r: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 ` [PATCH v4 13/15] iio: dac: ad3552r: " Jonathan Cameron
@ 2024-02-19 11:46   ` Andy Shevchenko
  0 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2024-02-19 11:46 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:47PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> Removing the goto err; statements also allows more extensive use of
> dev_err_probe() further simplifying the code.

...

>  			err = fwnode_property_read_u32_array(child,
>  							     "adi,output-range-microvolt",
>  							     vals,
>  							     2);

Side note: ARRAY_SIZE()?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 15/15] iio: dac: ltc2688: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 ` [PATCH v4 15/15] iio: dac: ltc2688: " Jonathan Cameron
@ 2024-02-19 11:48   ` Andy Shevchenko
  2024-02-19 15:48     ` Jonathan Cameron
  2024-02-19 13:49   ` Nuno Sá
  1 sibling, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2024-02-19 11:48 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:49PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.

...

>  			span = ltc2688_span_lookup(st, (int)tmp[0] / 1000,
>  						   tmp[1] / 1000);
> -			if (span < 0) {
> -				fwnode_handle_put(child);
> +			if (span < 0)
>  				return dev_err_probe(dev, -EINVAL,
>  						     "output range not valid:[%d %d]\n",
>  						     tmp[0], tmp[1]);

Last minute observation, should not we return span instead of -EINVAL?
(Haven't checked the semantics of the former though.)

...

> +		if (ret)
>  			return dev_err_probe(dev, -EINVAL,
>  					     "failed to set chan settings\n");

Ditto.


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling.
  2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
                   ` (14 preceding siblings ...)
  2024-02-17 16:42 ` [PATCH v4 15/15] iio: dac: ltc2688: " Jonathan Cameron
@ 2024-02-19 11:49 ` Andy Shevchenko
  2024-02-19 15:49   ` Jonathan Cameron
  15 siblings, 1 reply; 36+ messages in thread
From: Andy Shevchenko @ 2024-02-19 11:49 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:34PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Sorry for the rapid resend, Andy noticed I'd messed up creating the v3 patch
> set with some updates committed in the wrong patch.
> 
> Since v3: The updates to alignment of device_for_each_child_node_scopd() were
>     in the wrong patch. Move them to patch 4 where they should always
>     have been. (thanks Andy!)
> 
> Since v2: Thanks to Sakari and Andy for reviews.
> - New first patch moving fwnode_handle_put() into property.h
> - Tweak alignment in the loop macro
> - Pick up tags.
> - scopd -> scoped typo fix in some patch descriptions.
> 
> As can be seen by the examples from IIO that follow this can save
> a reasonable amount of complexity and boiler plate code, often enabling
> additional cleanups in related code such as use of
> return dev_err_probe().
> 
> Given we are now fairly late in the cycle, I'd expect to take this
> through the IIO tree and we can make use of it elsewhere next cycle.
> 
> Note I don't have the hardware so this is compile tested only.
> Hence I'd appreciate some Tested-by tags if anyone can poke one of the
> effected drivers.
> 
> Julia Lawal has posted some nice coccinelle magic for the DT equivalents.
> Referenced from that cover letter.  Similar may help us convert more
> drivers to use this new approach, but often hand tweaking can take
> additional advantage of other cleanup.h based magic, or things like
> return dev_err_probe().
> https://lore.kernel.org/all/20240211174237.182947-1-jic23@kernel.org/

It seems you are got all necessary tags to go.
I commented with some side notes that may be addressed later on.
Up to you.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 15/15] iio: dac: ltc2688: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 ` [PATCH v4 15/15] iio: dac: ltc2688: " Jonathan Cameron
  2024-02-19 11:48   ` Andy Shevchenko
@ 2024-02-19 13:49   ` Nuno Sá
  1 sibling, 0 replies; 36+ messages in thread
From: Nuno Sá @ 2024-02-19 13:49 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, Rafael J . Wysocki, Len Brown,
	linux-acpi, Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, 2024-02-17 at 16:42 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> Cc: Nuno Sá <nuno.sa@analog.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Tested-by: Nuno Sa <nuno.sa@analog.com>
Reviewed-by: Nuno Sa <nuno.sa@analog.com>

> v4: Moved alignment changes back to patch 4.
> v3: Tweaked the alignment after comments from Andy.
> 
>  drivers/iio/dac/ltc2688.c | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iio/dac/ltc2688.c b/drivers/iio/dac/ltc2688.c
> index fc8eb53c65be..b71df03fc13b 100644
> --- a/drivers/iio/dac/ltc2688.c
> +++ b/drivers/iio/dac/ltc2688.c
> @@ -746,26 +746,21 @@ static int ltc2688_span_lookup(const struct
> ltc2688_state *st, int min, int max)
>  static int ltc2688_channel_config(struct ltc2688_state *st)
>  {
>  	struct device *dev = &st->spi->dev;
> -	struct fwnode_handle *child;
>  	u32 reg, clk_input, val, tmp[2];
>  	int ret, span;
>  
> -	device_for_each_child_node(dev, child) {
> +	device_for_each_child_node_scoped(dev, child) {
>  		struct ltc2688_chan *chan;
>  
>  		ret = fwnode_property_read_u32(child, "reg", &reg);
> -		if (ret) {
> -			fwnode_handle_put(child);
> +		if (ret)
>  			return dev_err_probe(dev, ret,
>  					     "Failed to get reg property\n");
> -		}
>  
> -		if (reg >= LTC2688_DAC_CHANNELS) {
> -			fwnode_handle_put(child);
> +		if (reg >= LTC2688_DAC_CHANNELS)
>  			return dev_err_probe(dev, -EINVAL,
>  					     "reg bigger than: %d\n",
>  					     LTC2688_DAC_CHANNELS);
> -		}
>  
>  		val = 0;
>  		chan = &st->channels[reg];
> @@ -786,12 +781,10 @@ static int ltc2688_channel_config(struct ltc2688_state
> *st)
>  		if (!ret) {
>  			span = ltc2688_span_lookup(st, (int)tmp[0] / 1000,
>  						   tmp[1] / 1000);
> -			if (span < 0) {
> -				fwnode_handle_put(child);
> +			if (span < 0)
>  				return dev_err_probe(dev, -EINVAL,
>  						     "output range not
> valid:[%d %d]\n",
>  						     tmp[0], tmp[1]);
> -			}
>  
>  			val |= FIELD_PREP(LTC2688_CH_SPAN_MSK, span);
>  		}
> @@ -800,17 +793,14 @@ static int ltc2688_channel_config(struct ltc2688_state
> *st)
>  					       &clk_input);
>  		if (!ret) {
>  			if (clk_input >= LTC2688_CH_TGP_MAX) {
> -				fwnode_handle_put(child);
>  				return dev_err_probe(dev, -EINVAL,
>  						     "toggle-dither-input inv
> value(%d)\n",
>  						     clk_input);
>  			}
>  
>  			ret = ltc2688_tgp_clk_setup(st, chan, child,
> clk_input);
> -			if (ret) {
> -				fwnode_handle_put(child);
> +			if (ret)
>  				return ret;
> -			}
>  
>  			/*
>  			 * 0 means software toggle which is the default mode.
> @@ -844,11 +834,9 @@ static int ltc2688_channel_config(struct ltc2688_state
> *st)
>  
>  		ret = regmap_write(st->regmap, LTC2688_CMD_CH_SETTING(reg),
>  				   val);
> -		if (ret) {
> -			fwnode_handle_put(child);
> +		if (ret)
>  			return dev_err_probe(dev, -EINVAL,
>  					     "failed to set chan
> settings\n");
> -		}
>  	}
>  
>  	return 0;


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 03/15] iio: temp: ltc2983: Use __free(fwnode_handle) to replace fwnode_handle_put() calls
  2024-02-17 16:42 ` [PATCH v4 03/15] iio: temp: ltc2983: Use __free(fwnode_handle) to replace fwnode_handle_put() calls Jonathan Cameron
@ 2024-02-19 13:52   ` Nuno Sá
  2024-02-24 11:37     ` Jonathan Cameron
  0 siblings, 1 reply; 36+ messages in thread
From: Nuno Sá @ 2024-02-19 13:52 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, Rafael J . Wysocki, Len Brown,
	linux-acpi, Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, 2024-02-17 at 16:42 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This use of the new cleanup.h scope based freeing infrastructure allows
> us to exit directly from error conditions and in the good path with
> the reference obtained from fwnode_find_reference() (which may be an error
> pointer) automatically released.
> 
> Cc: Cosmin Tanislav <cosmin.tanislav@analog.com>
> Cc: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v2: Move the declarations down to where they are assigned.
> This both clearly associates the cleanup with the action that it
> is cleaning up and avoid potential future missordering of cleanup.
> ---

This LGTM. Only comment is that in ltc2983_parse_dt() we do have a loop where we
could make use of device_for_each_child_node_scoped(). It seems to me that
change could be included in this patch.

- Nuno Sá



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 05/15] iio: adc: max11410: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 ` [PATCH v4 05/15] iio: adc: max11410: Use device_for_each_child_node_scoped() Jonathan Cameron
@ 2024-02-19 13:54   ` Nuno Sá
  0 siblings, 0 replies; 36+ messages in thread
From: Nuno Sá @ 2024-02-19 13:54 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, Rafael J . Wysocki, Len Brown,
	linux-acpi, Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, 2024-02-17 at 16:42 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> Cc: Nuno Sá <nuno.sa@analog.com>
> Cc: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/adc/max11410.c | 27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/max11410.c b/drivers/iio/adc/max11410.c
> index 6af829349b4e..45368850b220 100644
> --- a/drivers/iio/adc/max11410.c
> +++ b/drivers/iio/adc/max11410.c
> @@ -696,7 +696,6 @@ static int max11410_parse_channels(struct max11410_state
> *st,
>  	struct device *dev = &st->spi_dev->dev;
>  	struct max11410_channel_config *cfg;
>  	struct iio_chan_spec *channels;
> -	struct fwnode_handle *child;
>  	u32 reference, sig_path;
>  	const char *node_name;
>  	u32 inputs[2], scale;
> @@ -720,7 +719,7 @@ static int max11410_parse_channels(struct max11410_state
> *st,
>  	if (!st->channels)
>  		return -ENOMEM;
>  
> -	device_for_each_child_node(dev, child) {
> +	device_for_each_child_node_scoped(dev, child) {
>  		node_name = fwnode_get_name(child);
>  		if (fwnode_property_present(child, "diff-channels")) {
>  			ret = fwnode_property_read_u32_array(child,
> @@ -735,47 +734,37 @@ static int max11410_parse_channels(struct max11410_state
> *st,
>  			inputs[1] = 0;
>  			chanspec.differential = 0;
>  		}
> -		if (ret) {
> -			fwnode_handle_put(child);
> +		if (ret)
>  			return ret;
> -		}
>  
>  		if (inputs[0] > MAX11410_CHANNEL_INDEX_MAX ||
> -		    inputs[1] > MAX11410_CHANNEL_INDEX_MAX) {
> -			fwnode_handle_put(child);
> +		    inputs[1] > MAX11410_CHANNEL_INDEX_MAX)
>  			return dev_err_probe(&indio_dev->dev, -EINVAL,
>  					     "Invalid channel index for %s,
> should be less than %d\n",
>  					     node_name,
>  					     MAX11410_CHANNEL_INDEX_MAX + 1);
> -		}
>  
>  		cfg = &st->channels[chan_idx];
>  
>  		reference = MAX11410_REFSEL_AVDD_AGND;
>  		fwnode_property_read_u32(child, "adi,reference", &reference);
> -		if (reference > MAX11410_REFSEL_MAX) {
> -			fwnode_handle_put(child);
> +		if (reference > MAX11410_REFSEL_MAX)
>  			return dev_err_probe(&indio_dev->dev, -EINVAL,
>  					     "Invalid adi,reference value for
> %s, should be less than %d.\n",
>  					     node_name, MAX11410_REFSEL_MAX +
> 1);
> -		}
>  
>  		if (!max11410_get_vrefp(st, reference) ||
> -		    (!max11410_get_vrefn(st, reference) && reference <= 2)) {
> -			fwnode_handle_put(child);
> +		    (!max11410_get_vrefn(st, reference) && reference <= 2))
>  			return dev_err_probe(&indio_dev->dev, -EINVAL,
>  					     "Invalid VREF configuration for
> %s, either specify corresponding VREF regulators or change adi,reference
> property.\n",
>  					     node_name);
> -		}
>  
>  		sig_path = MAX11410_PGA_SIG_PATH_BUFFERED;
>  		fwnode_property_read_u32(child, "adi,input-mode", &sig_path);
> -		if (sig_path > MAX11410_SIG_PATH_MAX) {
> -			fwnode_handle_put(child);
> +		if (sig_path > MAX11410_SIG_PATH_MAX)
>  			return dev_err_probe(&indio_dev->dev, -EINVAL,
>  					     "Invalid adi,input-mode value
> for %s, should be less than %d.\n",
>  					     node_name, MAX11410_SIG_PATH_MAX
> + 1);
> -		}
>  
>  		fwnode_property_read_u32(child, "settling-time-us",
>  					 &cfg->settling_time_us);
> @@ -793,10 +782,8 @@ static int max11410_parse_channels(struct max11410_state
> *st,
>  			cfg->scale_avail = devm_kcalloc(dev,
> MAX11410_SCALE_AVAIL_SIZE * 2,
>  							sizeof(*cfg-
> >scale_avail),
>  							GFP_KERNEL);
> -			if (!cfg->scale_avail) {
> -				fwnode_handle_put(child);
> +			if (!cfg->scale_avail)
>  				return -ENOMEM;
> -			}
>  
>  			scale = max11410_get_scale(st, *cfg);
>  			for (i = 0; i < MAX11410_SCALE_AVAIL_SIZE; i++) {


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 12/15] iio: addac: ad74413r: Use device_for_each_child_node_scoped()
  2024-02-17 16:42 ` [PATCH v4 12/15] iio: addac: ad74413r: " Jonathan Cameron
@ 2024-02-19 13:55   ` Nuno Sá
  0 siblings, 0 replies; 36+ messages in thread
From: Nuno Sá @ 2024-02-19 13:55 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, Rafael J . Wysocki, Len Brown,
	linux-acpi, Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall
  Cc: Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, 2024-02-17 at 16:42 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> The use of fwnode_for_each_available_child_node() here is assumed
> to have been down to a false assumption that device_for_each_child_node()
> doesn't check avaialble - so this transition to the scoped
> device_for_each_child_node_scoped() is equivalent.
> 
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/addac/ad74413r.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> index 7af3e4b8fe3b..cd26a16dc0ff 100644
> --- a/drivers/iio/addac/ad74413r.c
> +++ b/drivers/iio/addac/ad74413r.c
> @@ -1255,21 +1255,15 @@ static int ad74413r_parse_channel_config(struct
> iio_dev *indio_dev,
>  static int ad74413r_parse_channel_configs(struct iio_dev *indio_dev)
>  {
>  	struct ad74413r_state *st = iio_priv(indio_dev);
> -	struct fwnode_handle *channel_node = NULL;
>  	int ret;
>  
> -	fwnode_for_each_available_child_node(dev_fwnode(st->dev),
> channel_node) {
> +	device_for_each_child_node_scoped(st->dev, channel_node) {
>  		ret = ad74413r_parse_channel_config(indio_dev, channel_node);
>  		if (ret)
> -			goto put_channel_node;
> +			return ret;
>  	}
>  
>  	return 0;
> -
> -put_channel_node:
> -	fwnode_handle_put(channel_node);
> -
> -	return ret;
>  }
>  
>  static int ad74413r_setup_channels(struct iio_dev *indio_dev)


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 15/15] iio: dac: ltc2688: Use device_for_each_child_node_scoped()
  2024-02-19 11:48   ` Andy Shevchenko
@ 2024-02-19 15:48     ` Jonathan Cameron
  0 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-19 15:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, linux-iio, Rafael J . Wysocki, Len Brown,
	linux-acpi, Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra

On Mon, 19 Feb 2024 13:48:27 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sat, Feb 17, 2024 at 04:42:49PM +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Switching to the _scoped() version removes the need for manual
> > calling of fwnode_handle_put() in the paths where the code
> > exits the loop early. In this case that's all in error paths.  
> 
> ...
> 
> >  			span = ltc2688_span_lookup(st, (int)tmp[0] / 1000,
> >  						   tmp[1] / 1000);
> > -			if (span < 0) {
> > -				fwnode_handle_put(child);
> > +			if (span < 0)
> >  				return dev_err_probe(dev, -EINVAL,
> >  						     "output range not valid:[%d %d]\n",
> >  						     tmp[0], tmp[1]);  
> 
> Last minute observation, should not we return span instead of -EINVAL?
> (Haven't checked the semantics of the former though.)
It returns 0 or -EINVAL, so yes we should use span here.

> 
> ...
> 
> > +		if (ret)
> >  			return dev_err_probe(dev, -EINVAL,
> >  					     "failed to set chan settings\n");  
> 
> Ditto.

Definitely on that one. 

I'll aim to fold those two in whilst picking this up with a note in the patch description.
(or I'll incorporate them if I do a v5 for other reasons!)

Thanks,

Jonathan

> 
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling.
  2024-02-19 11:49 ` [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Andy Shevchenko
@ 2024-02-19 15:49   ` Jonathan Cameron
  2024-02-24 11:49     ` Jonathan Cameron
  0 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-19 15:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, linux-iio, Rafael J . Wysocki, Len Brown,
	linux-acpi, Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra

On Mon, 19 Feb 2024 13:49:22 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sat, Feb 17, 2024 at 04:42:34PM +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Sorry for the rapid resend, Andy noticed I'd messed up creating the v3 patch
> > set with some updates committed in the wrong patch.
> > 
> > Since v3: The updates to alignment of device_for_each_child_node_scopd() were
> >     in the wrong patch. Move them to patch 4 where they should always
> >     have been. (thanks Andy!)
> > 
> > Since v2: Thanks to Sakari and Andy for reviews.
> > - New first patch moving fwnode_handle_put() into property.h
> > - Tweak alignment in the loop macro
> > - Pick up tags.
> > - scopd -> scoped typo fix in some patch descriptions.
> > 
> > As can be seen by the examples from IIO that follow this can save
> > a reasonable amount of complexity and boiler plate code, often enabling
> > additional cleanups in related code such as use of
> > return dev_err_probe().
> > 
> > Given we are now fairly late in the cycle, I'd expect to take this
> > through the IIO tree and we can make use of it elsewhere next cycle.
> > 
> > Note I don't have the hardware so this is compile tested only.
> > Hence I'd appreciate some Tested-by tags if anyone can poke one of the
> > effected drivers.
> > 
> > Julia Lawal has posted some nice coccinelle magic for the DT equivalents.
> > Referenced from that cover letter.  Similar may help us convert more
> > drivers to use this new approach, but often hand tweaking can take
> > additional advantage of other cleanup.h based magic, or things like
> > return dev_err_probe().
> > https://lore.kernel.org/all/20240211174237.182947-1-jic23@kernel.org/  
> 
> It seems you are got all necessary tags to go.

Light on the driver changes to use it, but seems that we have
reached convergence on the infrastructure.

I'll let it sit until the end of the week though as I want to
get a pull request out anyway before taking this into my tree.


> I commented with some side notes that may be addressed later on.
> Up to you.
Thanks. I'll catch up with those shortly. 

Jonathan

> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h
  2024-02-17 16:42 ` [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h Jonathan Cameron
  2024-02-19  8:55   ` Greg Kroah-Hartman
@ 2024-02-20  6:55   ` Sakari Ailus
  1 sibling, 0 replies; 36+ messages in thread
From: Sakari Ailus @ 2024-02-20  6:55 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:35PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> By having this function as static inline in the header, the compiler
> is able to see if can optmize the call out if (IS_ERR_OR_NULL(fwnode))
> This will allow a simpler DEFINE_FREE() call in the following patch.
> 
> Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 02/15] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup.
  2024-02-17 16:42 ` [PATCH v4 02/15] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup Jonathan Cameron
  2024-02-19  8:55   ` Greg Kroah-Hartman
@ 2024-02-20  6:55   ` Sakari Ailus
  1 sibling, 0 replies; 36+ messages in thread
From: Sakari Ailus @ 2024-02-20  6:55 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:36PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Useful where the fwnode_handle was obtained from a call such as
> fwnode_find_reference() as it will safely do nothing if IS_ERR() is true
> and will automatically release the reference on the variable leaving
> scope.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 04/15] device property: Introduce device_for_each_child_node_scoped()
  2024-02-17 16:42 ` [PATCH v4 04/15] device property: Introduce device_for_each_child_node_scoped() Jonathan Cameron
  2024-02-19  8:56   ` Greg Kroah-Hartman
@ 2024-02-20  6:59   ` Sakari Ailus
  1 sibling, 0 replies; 36+ messages in thread
From: Sakari Ailus @ 2024-02-20  6:59 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Sat, Feb 17, 2024 at 04:42:38PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Similar to recently propose for_each_child_of_node_scoped() this
> new version of the loop macro instantiates a new local
> struct fwnode_handle * that uses the __free(fwnode_handle) auto
> cleanup handling so that if a reference to a node is held on early
> exit from the loop the reference will be released. If the loop
> runs to completion, the child pointer will be NULL and no action will
> be taken.
> 
> The reason this is useful is that it removes the need for
> fwnode_handle_put() on early loop exits.  If there is a need
> to retain the reference, then return_ptr(child) or no_free_ptr(child)
> may be used to safely disable the auto cleanup.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 03/15] iio: temp: ltc2983: Use __free(fwnode_handle) to replace fwnode_handle_put() calls
  2024-02-19 13:52   ` Nuno Sá
@ 2024-02-24 11:37     ` Jonathan Cameron
  0 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-24 11:37 UTC (permalink / raw)
  To: Nuno Sá
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Andy Shevchenko, Greg Kroah-Hartman, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Mon, 19 Feb 2024 14:52:51 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Sat, 2024-02-17 at 16:42 +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > This use of the new cleanup.h scope based freeing infrastructure allows
> > us to exit directly from error conditions and in the good path with
> > the reference obtained from fwnode_find_reference() (which may be an error
> > pointer) automatically released.
> > 
> > Cc: Cosmin Tanislav <cosmin.tanislav@analog.com>
> > Cc: Nuno Sá <nuno.sa@analog.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > v2: Move the declarations down to where they are assigned.
> > This both clearly associates the cleanup with the action that it
> > is cleaning up and avoid potential future missordering of cleanup.
> > ---  
> 
> This LGTM. Only comment is that in ltc2983_parse_dt() we do have a loop where we
> could make use of device_for_each_child_node_scoped(). It seems to me that
> change could be included in this patch.
> 
> - Nuno Sá
> 
> 

True. Not sure how I missed that.  I'll do that for v5 posting of remaining drivers.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling.
  2024-02-19 15:49   ` Jonathan Cameron
@ 2024-02-24 11:49     ` Jonathan Cameron
  2024-02-28 14:15       ` Jonathan Cameron
  0 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-24 11:49 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Andy Shevchenko, linux-iio, Rafael J . Wysocki, Len Brown,
	linux-acpi, Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra

On Mon, 19 Feb 2024 15:49:47 +0000
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:

> On Mon, 19 Feb 2024 13:49:22 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > On Sat, Feb 17, 2024 at 04:42:34PM +0000, Jonathan Cameron wrote:  
> > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > 
> > > Sorry for the rapid resend, Andy noticed I'd messed up creating the v3 patch
> > > set with some updates committed in the wrong patch.
> > > 
> > > Since v3: The updates to alignment of device_for_each_child_node_scopd() were
> > >     in the wrong patch. Move them to patch 4 where they should always
> > >     have been. (thanks Andy!)
> > > 
> > > Since v2: Thanks to Sakari and Andy for reviews.
> > > - New first patch moving fwnode_handle_put() into property.h
> > > - Tweak alignment in the loop macro
> > > - Pick up tags.
> > > - scopd -> scoped typo fix in some patch descriptions.
> > > 
> > > As can be seen by the examples from IIO that follow this can save
> > > a reasonable amount of complexity and boiler plate code, often enabling
> > > additional cleanups in related code such as use of
> > > return dev_err_probe().
> > > 
> > > Given we are now fairly late in the cycle, I'd expect to take this
> > > through the IIO tree and we can make use of it elsewhere next cycle.
> > > 
> > > Note I don't have the hardware so this is compile tested only.
> > > Hence I'd appreciate some Tested-by tags if anyone can poke one of the
> > > effected drivers.
> > > 
> > > Julia Lawal has posted some nice coccinelle magic for the DT equivalents.
> > > Referenced from that cover letter.  Similar may help us convert more
> > > drivers to use this new approach, but often hand tweaking can take
> > > additional advantage of other cleanup.h based magic, or things like
> > > return dev_err_probe().
> > > https://lore.kernel.org/all/20240211174237.182947-1-jic23@kernel.org/    
> > 
> > It seems you are got all necessary tags to go.  
> 
> Light on the driver changes to use it, but seems that we have
> reached convergence on the infrastructure.

What I'll do in the short term is pick up the changes that have been
reviewed and gained tags (so the infrastructure plus a few of the driver
changes) and then send a v5 with the remainder. I suspect the driver
changes have gotten lost in the deluge as IIO has been very busy this week.

Whilst I will occasionally pick up my own IIO changes with out review
tags I normally only do that for trivial stuff like build fixes.
These are simple but not simple enough!

So applied patches
1,2,4,5,12 and 15 to the togreg branch of iio.git which will be initially
pushed out as testing for 0-day to look at it.

Thanks,

Jonathan

> 
> I'll let it sit until the end of the week though as I want to
> get a pull request out anyway before taking this into my tree.
> 
> 
> > I commented with some side notes that may be addressed later on.
> > Up to you.  
> Thanks. I'll catch up with those shortly. 
> 
> Jonathan
> 
> >   
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 09/15] iio: adc: stm32: Use device_for_each_child_node_scoped()
  2024-02-19 11:45   ` Andy Shevchenko
@ 2024-02-24 12:16     ` Jonathan Cameron
  0 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:16 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-iio, Rafael J . Wysocki, Len Brown, linux-acpi,
	Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra, Jonathan Cameron

On Mon, 19 Feb 2024 13:45:03 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sat, Feb 17, 2024 at 04:42:43PM +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Switching to the _scoped() version removes the need for manual
> > calling of fwnode_handle_put() in the paths where the code
> > exits the loop early. In this case that's all in error paths.
> > 
> > Note this includes a probable fix as in one path an error message was
> > printed with ret == 0.
> > 
> > Took advantage of dev_err_probe() to futher simplify things given no
> > longer a need for the goto err.  
> 
> ...
> 
> >  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;  
> 
> I believe with
> 
> 	struct device *dev = &indio_dev->dev;
> 
> you can make the below neater.

Agreed.  Given the users of indio_dev are all the error messages I'm touching
anyway, this is in scope for this patch I think.

> Also see some side notes.
> 
> > -	struct fwnode_handle *child;
> >  	const char *name;
> >  	int val, scan_index = 0, ret;
> >  	bool differential;
> >  	u32 vin[2];  
> 
> ...
> 
> >  		if (!ret) {  
> 
> Not a fan of this pattern, below we have two different patterns for the cases
> like this :-(

Likewise not a fan.

Tidying this up is unrelated enough though that I won't do it in this
series.

> 
> > +			if (strlen(name) >= STM32_ADC_CH_SZ)
> > +				return dev_err_probe(&indio_dev->dev, -EINVAL,
> > +						     "Label %s exceeds %d characters\n",
> > +						     name, STM32_ADC_CH_SZ);
> > +
> >  			strscpy(adc->chan_name[val], name, STM32_ADC_CH_SZ);
> >  			ret = stm32_adc_populate_int_ch(indio_dev, name, val);
> >  			if (ret == -ENOENT)
> >  				continue;
> >  			else if (ret)  
> 
> 
> This 'else' is redundant.
> 
> > +				return ret;
> > +		} else if (ret != -EINVAL)  
>
I shouldn't have dropped the bracket here, so I'll put that back for now.
 
> This also...
> 
> > +			return dev_err_probe(&indio_dev->dev, ret, "Invalid label\n");  
> 
> ...if you first do like
> 
> 		if (ret && ret != -EINVAL)
> 			return dev_err_probe(...);
> 		if (!ret) {
> 
> Another option
> 
> 		if (ret) {
> 			if (ret != -EINVAL)
> 				return dev_err_probe(...);
> 		} else {
> 
> ...
> 
> >  		differential = false;
> >  		ret = fwnode_property_read_u32_array(child, "diff-channels", vin, 2);  
> 
> ARRAY_SIZE()?
> 
> >  		/* diff-channels is optional */  
> 
> ...
> 
> >  		if (!ret) {
> > +			if (vin[0] != val || vin[1] >= adc_info->max_channels)
> > +				return dev_err_probe(&indio_dev->dev, -EINVAL,
> > +						     "Invalid channel in%d-in%d\n",
> > +						     vin[0], vin[1]);
> >  		} else if (ret != -EINVAL) {
> > -			dev_err(&indio_dev->dev, "Invalid diff-channels property %d\n", ret);
> > -			goto err;
> > +			return dev_err_probe(&indio_dev->dev, ret,
> > +					     "Invalid diff-channels property\n");
> >  		} 
> 
> As per above?
> 
Agree with all comments, but as you said side notes. Stuff to cleanup in a series
doing a wider driver clean up.

Thanks,

Jonathan



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling.
  2024-02-24 11:49     ` Jonathan Cameron
@ 2024-02-28 14:15       ` Jonathan Cameron
  0 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2024-02-28 14:15 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Andy Shevchenko, linux-iio, Rafael J . Wysocki, Len Brown,
	linux-acpi, Greg Kroah-Hartman, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Julia Lawall, Nuno Sá,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Peter Zijlstra

On Sat, 24 Feb 2024 11:49:12 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 19 Feb 2024 15:49:47 +0000
> Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> 
> > On Mon, 19 Feb 2024 13:49:22 +0200
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> > > On Sat, Feb 17, 2024 at 04:42:34PM +0000, Jonathan Cameron wrote:    
> > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > 
> > > > Sorry for the rapid resend, Andy noticed I'd messed up creating the v3 patch
> > > > set with some updates committed in the wrong patch.
> > > > 
> > > > Since v3: The updates to alignment of device_for_each_child_node_scopd() were
> > > >     in the wrong patch. Move them to patch 4 where they should always
> > > >     have been. (thanks Andy!)
> > > > 
> > > > Since v2: Thanks to Sakari and Andy for reviews.
> > > > - New first patch moving fwnode_handle_put() into property.h
> > > > - Tweak alignment in the loop macro
> > > > - Pick up tags.
> > > > - scopd -> scoped typo fix in some patch descriptions.
> > > > 
> > > > As can be seen by the examples from IIO that follow this can save
> > > > a reasonable amount of complexity and boiler plate code, often enabling
> > > > additional cleanups in related code such as use of
> > > > return dev_err_probe().
> > > > 
> > > > Given we are now fairly late in the cycle, I'd expect to take this
> > > > through the IIO tree and we can make use of it elsewhere next cycle.
> > > > 
> > > > Note I don't have the hardware so this is compile tested only.
> > > > Hence I'd appreciate some Tested-by tags if anyone can poke one of the
> > > > effected drivers.
> > > > 
> > > > Julia Lawal has posted some nice coccinelle magic for the DT equivalents.
> > > > Referenced from that cover letter.  Similar may help us convert more
> > > > drivers to use this new approach, but often hand tweaking can take
> > > > additional advantage of other cleanup.h based magic, or things like
> > > > return dev_err_probe().
> > > > https://lore.kernel.org/all/20240211174237.182947-1-jic23@kernel.org/      
> > > 
> > > It seems you are got all necessary tags to go.    
> > 
> > Light on the driver changes to use it, but seems that we have
> > reached convergence on the infrastructure.  
> 
> What I'll do in the short term is pick up the changes that have been
> reviewed and gained tags (so the infrastructure plus a few of the driver
> changes) and then send a v5 with the remainder. I suspect the driver
> changes have gotten lost in the deluge as IIO has been very busy this week.
> 
> Whilst I will occasionally pick up my own IIO changes with out review
> tags I normally only do that for trivial stuff like build fixes.
> These are simple but not simple enough!
> 
> So applied patches
> 1,2,4,5,12 and 15 to the togreg branch of iio.git which will be initially
> pushed out as testing for 0-day to look at it.

Linus is pushing back on some of the uses for cleanup.h for not being
sufficiently standard c like.

https://lore.kernel.org/linux-cxl/170905252721.2268463.6714121678946763402.stgit@dwillia2-xfh.jf.intel.com/T/#m336ba4087e4f963abbc654ba56eba6d61b77a14b

That's fair enough, but I think makes pull requests with them in higher
risk than normal.

As such, for the things I have queued in the IIO tree (beyond the pull
request GregKH already took which we can cross fingers on),
I'm going to shuffle the tree so that the remainder can be handled
in two separate pull requests:

1) Everything else
2) cleanup.h related including this series.

That should give Greg maximum flexibility to do what makes sense for
char-misc-next.

The scoped_cond_guard() stuff can be easily modified to be near what Linus is
proposing so hopefully we can do that next cycle.

Hopefully I can get this done later today.

Jonathan




> 
> Thanks,
> 
> Jonathan
> 
> > 
> > I'll let it sit until the end of the week though as I want to
> > get a pull request out anyway before taking this into my tree.
> > 
> >   
> > > I commented with some side notes that may be addressed later on.
> > > Up to you.    
> > Thanks. I'll catch up with those shortly. 
> > 
> > Jonathan
> >   
> > >     
> >   
> 
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2024-02-28 14:15 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-17 16:42 [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 01/15] device property: Move fwnode_handle_put() into property.h Jonathan Cameron
2024-02-19  8:55   ` Greg Kroah-Hartman
2024-02-20  6:55   ` Sakari Ailus
2024-02-17 16:42 ` [PATCH v4 02/15] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup Jonathan Cameron
2024-02-19  8:55   ` Greg Kroah-Hartman
2024-02-20  6:55   ` Sakari Ailus
2024-02-17 16:42 ` [PATCH v4 03/15] iio: temp: ltc2983: Use __free(fwnode_handle) to replace fwnode_handle_put() calls Jonathan Cameron
2024-02-19 13:52   ` Nuno Sá
2024-02-24 11:37     ` Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 04/15] device property: Introduce device_for_each_child_node_scoped() Jonathan Cameron
2024-02-19  8:56   ` Greg Kroah-Hartman
2024-02-20  6:59   ` Sakari Ailus
2024-02-17 16:42 ` [PATCH v4 05/15] iio: adc: max11410: Use device_for_each_child_node_scoped() Jonathan Cameron
2024-02-19 13:54   ` Nuno Sá
2024-02-17 16:42 ` [PATCH v4 06/15] iio: adc: mcp3564: " Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 07/15] iio: adc: qcom-spmi-adc5: " Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 08/15] iio: adc: rzg2l_adc: " Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 09/15] iio: adc: stm32: " Jonathan Cameron
2024-02-19 11:45   ` Andy Shevchenko
2024-02-24 12:16     ` Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 10/15] iio: adc: ti-ads1015: " Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 11/15] iio: adc: ti-ads131e08: " Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 12/15] iio: addac: ad74413r: " Jonathan Cameron
2024-02-19 13:55   ` Nuno Sá
2024-02-17 16:42 ` [PATCH v4 13/15] iio: dac: ad3552r: " Jonathan Cameron
2024-02-19 11:46   ` Andy Shevchenko
2024-02-17 16:42 ` [PATCH v4 14/15] iio: dac: ad5770r: " Jonathan Cameron
2024-02-17 16:42 ` [PATCH v4 15/15] iio: dac: ltc2688: " Jonathan Cameron
2024-02-19 11:48   ` Andy Shevchenko
2024-02-19 15:48     ` Jonathan Cameron
2024-02-19 13:49   ` Nuno Sá
2024-02-19 11:49 ` [PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling Andy Shevchenko
2024-02-19 15:49   ` Jonathan Cameron
2024-02-24 11:49     ` Jonathan Cameron
2024-02-28 14:15       ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).