Linux-IIO Archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/9] IIO: Use device_for_each_child_scope()
@ 2024-02-24 12:32 Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped() Jonathan Cameron
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Jonathan Cameron

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

Previously:
[PATCH v4 00/15] device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling.
 
The infrastructure being used here is now in iio.git (currently exposed
as testing for initial 0-day testing). I also applied a couple of users
that had received positive reviews. I split the series up like this
so that the infrastructure would make the merge window if reviews on these
take a little longer to come in (hopefully these will also make it)

Some of the driver changes haven't received any tags or specific feedback
yet, so I want to repost those for review.  Andy and Nuno gave some
feedback on other patches. Resulting changes called out in specific
patches.

Jonathan Cameron (9):
  iio: temp: ltc2983: Use __free(fwnode_handle) and
    device_for_each_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: dac: ad3552r: Use device_for_each_child_node_scoped()
  iio: dac: ad5770r: Use device_for_each_child_node_scoped()

 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       |  61 ++++++-------
 drivers/iio/adc/ti-ads1015.c      |   5 +-
 drivers/iio/adc/ti-ads131e08.c    |  13 +--
 drivers/iio/dac/ad3552r.c         |  51 +++++------
 drivers/iio/dac/ad5770r.c         |  19 ++---
 drivers/iio/temperature/ltc2983.c | 137 +++++++++++-------------------
 9 files changed, 116 insertions(+), 204 deletions(-)

-- 
2.44.0


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

* [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  2024-02-26  8:35   ` Nuno Sá
  2024-02-24 12:32 ` [PATCH v5 2/9] iio: adc: mcp3564: Use device_for_each_child_node_scoped() Jonathan Cameron
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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.

Similarly the _scoped() version of device_for_each_child_node()
removes the need for the manual calling of fwnode_handl_put() in
paths where the code exits the loop early.

Tidy up some unusual indentation in a dev_dbg() whilst here.

Cc: Cosmin Tanislav <cosmin.tanislav@analog.com>
Cc: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v5: Add the device_for_each_child_node_scoped() change (Nuno)
---
 drivers/iio/temperature/ltc2983.c | 137 +++++++++++-------------------
 1 file changed, 50 insertions(+), 87 deletions(-)

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index fcb96c44d954..7d5473e7b500 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 *
@@ -1349,8 +1321,7 @@ static irqreturn_t ltc2983_irq_handler(int irq, void *data)
 static int ltc2983_parse_dt(struct ltc2983_data *st)
 {
 	struct device *dev = &st->spi->dev;
-	struct fwnode_handle *child;
-	int ret = 0, chan = 0, channel_avail_mask = 0;
+	int ret, chan = 0, channel_avail_mask = 0;
 
 	device_property_read_u32(dev, "adi,mux-delay-config-us", &st->mux_delay_config);
 
@@ -1368,38 +1339,35 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
 		return -ENOMEM;
 
 	st->iio_channels = st->num_channels;
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		struct ltc2983_sensor sensor;
 
 		ret = fwnode_property_read_u32(child, "reg", &sensor.chan);
-		if (ret) {
-			dev_err(dev, "reg property must given for child nodes\n");
-			goto put_child;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret,
+				"reg property must given for child nodes\n");
 
 		/* check if we have a valid channel */
 		if (sensor.chan < LTC2983_MIN_CHANNELS_NR ||
-		    sensor.chan > st->info->max_channels_nr) {
-			ret = -EINVAL;
-			dev_err(dev, "chan:%d must be from %u to %u\n", sensor.chan,
-				LTC2983_MIN_CHANNELS_NR, st->info->max_channels_nr);
-			goto put_child;
-		} else if (channel_avail_mask & BIT(sensor.chan)) {
-			ret = -EINVAL;
-			dev_err(dev, "chan:%d already in use\n", sensor.chan);
-			goto put_child;
-		}
+		    sensor.chan > st->info->max_channels_nr)
+			return dev_err_probe(dev, -EINVAL,
+					     "chan:%d must be from %u to %u\n",
+					     sensor.chan,
+					     LTC2983_MIN_CHANNELS_NR,
+					     st->info->max_channels_nr);
+
+		if (channel_avail_mask & BIT(sensor.chan))
+			return dev_err_probe(dev, -EINVAL,
+					     "chan:%d already in use\n",
+					     sensor.chan);
 
 		ret = fwnode_property_read_u32(child, "adi,sensor-type", &sensor.type);
-		if (ret) {
-			dev_err(dev,
+		if (ret)
+			return dev_err_probe(dev, ret,
 				"adi,sensor-type property must given for child nodes\n");
-			goto put_child;
-		}
 
 		dev_dbg(dev, "Create new sensor, type %u, chann %u",
-								sensor.type,
-								sensor.chan);
+			sensor.type, sensor.chan);
 
 		if (sensor.type >= LTC2983_SENSOR_THERMOCOUPLE &&
 		    sensor.type <= LTC2983_SENSOR_THERMOCOUPLE_CUSTOM) {
@@ -1426,17 +1394,15 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
 			   sensor.type == LTC2983_SENSOR_ACTIVE_TEMP) {
 			st->sensors[chan] = ltc2983_temp_new(child, st, &sensor);
 		} else {
-			dev_err(dev, "Unknown sensor type %d\n", sensor.type);
-			ret = -EINVAL;
-			goto put_child;
+			return dev_err_probe(dev, -EINVAL,
+					     "Unknown sensor type %d\n",
+					     sensor.type);
 		}
 
-		if (IS_ERR(st->sensors[chan])) {
-			dev_err(dev, "Failed to create sensor %ld",
-				PTR_ERR(st->sensors[chan]));
-			ret = PTR_ERR(st->sensors[chan]);
-			goto put_child;
-		}
+		if (IS_ERR(st->sensors[chan]))
+			return dev_err_probe(dev, PTR_ERR(st->sensors[chan]),
+					     "Failed to create sensor\n");
+
 		/* set generic sensor parameters */
 		st->sensors[chan]->chan = sensor.chan;
 		st->sensors[chan]->type = sensor.type;
@@ -1446,9 +1412,6 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
 	}
 
 	return 0;
-put_child:
-	fwnode_handle_put(child);
-	return ret;
 }
 
 static int ltc2983_eeprom_cmd(struct ltc2983_data *st, unsigned int cmd,
-- 
2.44.0


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

* [PATCH v5 2/9] iio: adc: mcp3564: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped() Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 3/9] iio: adc: qcom-spmi-adc5: " Jonathan Cameron
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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.44.0


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

* [PATCH v5 3/9] iio: adc: qcom-spmi-adc5: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped() Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 2/9] iio: adc: mcp3564: Use device_for_each_child_node_scoped() Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 4/9] iio: adc: rzg2l_adc: " Jonathan Cameron
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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.44.0


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

* [PATCH v5 4/9] iio: adc: rzg2l_adc: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
                   ` (2 preceding siblings ...)
  2024-02-24 12:32 ` [PATCH v5 3/9] iio: adc: qcom-spmi-adc5: " Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  2024-02-27  9:52   ` Prabhakar Mahadev Lad
  2024-02-24 12:32 ` [PATCH v5 5/9] iio: adc: stm32: " Jonathan Cameron
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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.44.0


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

* [PATCH v5 5/9] iio: adc: stm32: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
                   ` (3 preceding siblings ...)
  2024-02-24 12:32 ` [PATCH v5 4/9] iio: adc: rzg2l_adc: " Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  2024-02-29 11:34   ` Fabrice Gasnier
  2024-02-24 12:32 ` [PATCH v5 6/9] iio: adc: ti-ads1015: " Jonathan Cameron
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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>

---
v5: Use a local struct device pointer.
    Add brackets back I shouldn't have dropped.

Andy had a number of other comments but they would be unrelated changes
so I'm leaving them for a future patch set.
---
 drivers/iio/adc/stm32-adc.c | 61 +++++++++++++++----------------------
 1 file changed, 24 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index b5d3c9cea5c4..36add95212c3 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -2187,58 +2187,52 @@ 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;
+	struct device *dev = &indio_dev->dev;
 	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(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(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(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;
+				return ret;
 		} else if (ret != -EINVAL) {
-			dev_err(&indio_dev->dev, "Invalid label %d\n", ret);
-			goto err;
+			return dev_err_probe(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(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(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(dev, ret,
+					     "Invalid diff-channels property\n");
 		}
 
 		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
@@ -2247,11 +2241,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(dev, ret,
+					     "Invalid st,min-sample-time-ns property\n");
 
 		stm32_adc_smpr_init(adc, channels[scan_index].channel, val);
 		if (differential)
@@ -2261,11 +2253,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.44.0


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

* [PATCH v5 6/9] iio: adc: ti-ads1015: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
                   ` (4 preceding siblings ...)
  2024-02-24 12:32 ` [PATCH v5 5/9] iio: adc: stm32: " Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 7/9] iio: adc: ti-ads131e08: " Jonathan Cameron
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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.44.0


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

* [PATCH v5 7/9] iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
                   ` (5 preceding siblings ...)
  2024-02-24 12:32 ` [PATCH v5 6/9] iio: adc: ti-ads1015: " Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 8/9] iio: dac: ad3552r: " Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 9/9] iio: dac: ad5770r: " Jonathan Cameron
  8 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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.44.0


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

* [PATCH v5 8/9] iio: dac: ad3552r: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
                   ` (6 preceding siblings ...)
  2024-02-24 12:32 ` [PATCH v5 7/9] iio: adc: ti-ads131e08: " Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  2024-02-24 12:32 ` [PATCH v5 9/9] iio: dac: ad5770r: " Jonathan Cameron
  8 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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.44.0


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

* [PATCH v5 9/9] iio: dac: ad5770r: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
                   ` (7 preceding siblings ...)
  2024-02-24 12:32 ` [PATCH v5 8/9] iio: dac: ad3552r: " Jonathan Cameron
@ 2024-02-24 12:32 ` Jonathan Cameron
  8 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-24 12:32 UTC (permalink / raw
  To: linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, 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.44.0


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

* Re: [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped()
  2024-02-24 12:32 ` [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped() Jonathan Cameron
@ 2024-02-26  8:35   ` Nuno Sá
  2024-02-26 19:30     ` Jonathan Cameron
  0 siblings, 1 reply; 16+ messages in thread
From: Nuno Sá @ 2024-02-26  8:35 UTC (permalink / raw
  To: Jonathan Cameron, linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Lad Prabhakar, Dmitry Baryshkov, Marijn Suijten, Marius Cristea,
	Ibrahim Tilki, Jonathan Cameron

On Sat, 2024-02-24 at 12:32 +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.
> 
> Similarly the _scoped() version of device_for_each_child_node()
> removes the need for the manual calling of fwnode_handl_put() in
> paths where the code exits the loop early.
> 
> Tidy up some unusual indentation in a dev_dbg() whilst here.
> 
> Cc: Cosmin Tanislav <cosmin.tanislav@analog.com>
> Cc: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> ---
> v5: Add the device_for_each_child_node_scoped() change (Nuno)
> ---

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

>  drivers/iio/temperature/ltc2983.c | 137 +++++++++++-------------------
>  1 file changed, 50 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index fcb96c44d954..7d5473e7b500 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 *
> @@ -1349,8 +1321,7 @@ static irqreturn_t ltc2983_irq_handler(int irq, void *data)
>  static int ltc2983_parse_dt(struct ltc2983_data *st)
>  {
>  	struct device *dev = &st->spi->dev;
> -	struct fwnode_handle *child;
> -	int ret = 0, chan = 0, channel_avail_mask = 0;
> +	int ret, chan = 0, channel_avail_mask = 0;
>  
>  	device_property_read_u32(dev, "adi,mux-delay-config-us", &st-
> >mux_delay_config);
>  
> @@ -1368,38 +1339,35 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
>  		return -ENOMEM;
>  
>  	st->iio_channels = st->num_channels;
> -	device_for_each_child_node(dev, child) {
> +	device_for_each_child_node_scoped(dev, child) {
>  		struct ltc2983_sensor sensor;
>  
>  		ret = fwnode_property_read_u32(child, "reg", &sensor.chan);
> -		if (ret) {
> -			dev_err(dev, "reg property must given for child nodes\n");
> -			goto put_child;
> -		}
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +				"reg property must given for child nodes\n");
>  
>  		/* check if we have a valid channel */
>  		if (sensor.chan < LTC2983_MIN_CHANNELS_NR ||
> -		    sensor.chan > st->info->max_channels_nr) {
> -			ret = -EINVAL;
> -			dev_err(dev, "chan:%d must be from %u to %u\n",
> sensor.chan,
> -				LTC2983_MIN_CHANNELS_NR, st->info-
> >max_channels_nr);
> -			goto put_child;
> -		} else if (channel_avail_mask & BIT(sensor.chan)) {
> -			ret = -EINVAL;
> -			dev_err(dev, "chan:%d already in use\n", sensor.chan);
> -			goto put_child;
> -		}
> +		    sensor.chan > st->info->max_channels_nr)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "chan:%d must be from %u to %u\n",
> +					     sensor.chan,
> +					     LTC2983_MIN_CHANNELS_NR,
> +					     st->info->max_channels_nr);
> +
> +		if (channel_avail_mask & BIT(sensor.chan))
> +			return dev_err_probe(dev, -EINVAL,
> +					     "chan:%d already in use\n",
> +					     sensor.chan);
>  
>  		ret = fwnode_property_read_u32(child, "adi,sensor-type",
> &sensor.type);
> -		if (ret) {
> -			dev_err(dev,
> +		if (ret)
> +			return dev_err_probe(dev, ret,
>  				"adi,sensor-type property must given for child
> nodes\n");
> -			goto put_child;
> -		}
>  
>  		dev_dbg(dev, "Create new sensor, type %u, chann %u",
> -								sensor.type,
> -								sensor.chan);
> +			sensor.type, sensor.chan);
>  
>  		if (sensor.type >= LTC2983_SENSOR_THERMOCOUPLE &&
>  		    sensor.type <= LTC2983_SENSOR_THERMOCOUPLE_CUSTOM) {
> @@ -1426,17 +1394,15 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
>  			   sensor.type == LTC2983_SENSOR_ACTIVE_TEMP) {
>  			st->sensors[chan] = ltc2983_temp_new(child, st, &sensor);
>  		} else {
> -			dev_err(dev, "Unknown sensor type %d\n", sensor.type);
> -			ret = -EINVAL;
> -			goto put_child;
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Unknown sensor type %d\n",
> +					     sensor.type);
>  		}
>  
> -		if (IS_ERR(st->sensors[chan])) {
> -			dev_err(dev, "Failed to create sensor %ld",
> -				PTR_ERR(st->sensors[chan]));
> -			ret = PTR_ERR(st->sensors[chan]);
> -			goto put_child;
> -		}
> +		if (IS_ERR(st->sensors[chan]))
> +			return dev_err_probe(dev, PTR_ERR(st->sensors[chan]),
> +					     "Failed to create sensor\n");
> +
>  		/* set generic sensor parameters */
>  		st->sensors[chan]->chan = sensor.chan;
>  		st->sensors[chan]->type = sensor.type;
> @@ -1446,9 +1412,6 @@ static int ltc2983_parse_dt(struct ltc2983_data *st)
>  	}
>  
>  	return 0;
> -put_child:
> -	fwnode_handle_put(child);
> -	return ret;
>  }
>  
>  static int ltc2983_eeprom_cmd(struct ltc2983_data *st, unsigned int cmd,


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

* Re: [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped()
  2024-02-26  8:35   ` Nuno Sá
@ 2024-02-26 19:30     ` Jonathan Cameron
  2024-02-27  8:08       ` Nuno Sá
  0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-26 19:30 UTC (permalink / raw
  To: Nuno Sá
  Cc: linux-iio, Nuno Sá, Andy Shevchenko, Cosmin Tanislav,
	Mihail Chindris, Rasmus Villemoes, Tomislav Denis, Marek Vasut,
	Olivier Moysan, Fabrice Gasnier, Lad Prabhakar, Dmitry Baryshkov,
	Marijn Suijten, Marius Cristea, Ibrahim Tilki, Jonathan Cameron

On Mon, 26 Feb 2024 09:35:49 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Sat, 2024-02-24 at 12:32 +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.
> > 
> > Similarly the _scoped() version of device_for_each_child_node()
> > removes the need for the manual calling of fwnode_handl_put() in
> > paths where the code exits the loop early.
> > 
> > Tidy up some unusual indentation in a dev_dbg() whilst here.
> > 
> > Cc: Cosmin Tanislav <cosmin.tanislav@analog.com>
> > Cc: Nuno Sá <nuno.sa@analog.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > ---
> > v5: Add the device_for_each_child_node_scoped() change (Nuno)
> > ---  
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
I'll pick up this now as whilst I hope someone will check the others, I know
you are building on this and it will make life easier if this is already
queued up.

Thanks,

Jonathan

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

* Re: [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped()
  2024-02-26 19:30     ` Jonathan Cameron
@ 2024-02-27  8:08       ` Nuno Sá
  0 siblings, 0 replies; 16+ messages in thread
From: Nuno Sá @ 2024-02-27  8:08 UTC (permalink / raw
  To: Jonathan Cameron
  Cc: linux-iio, Nuno Sá, Andy Shevchenko, Cosmin Tanislav,
	Mihail Chindris, Rasmus Villemoes, Tomislav Denis, Marek Vasut,
	Olivier Moysan, Fabrice Gasnier, Lad Prabhakar, Dmitry Baryshkov,
	Marijn Suijten, Marius Cristea, Ibrahim Tilki, Jonathan Cameron

On Mon, 2024-02-26 at 19:30 +0000, Jonathan Cameron wrote:
> On Mon, 26 Feb 2024 09:35:49 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Sat, 2024-02-24 at 12:32 +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.
> > > 
> > > Similarly the _scoped() version of device_for_each_child_node()
> > > removes the need for the manual calling of fwnode_handl_put() in
> > > paths where the code exits the loop early.
> > > 
> > > Tidy up some unusual indentation in a dev_dbg() whilst here.
> > > 
> > > Cc: Cosmin Tanislav <cosmin.tanislav@analog.com>
> > > Cc: Nuno Sá <nuno.sa@analog.com>
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > 
> > > ---
> > > v5: Add the device_for_each_child_node_scoped() change (Nuno)
> > > ---  
> > 
> > Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> I'll pick up this now as whilst I hope someone will check the others, I know
> you are building on this and it will make life easier if this is already
> queued up.
> 

Thanks,

Will work on top of that for my series.

- Nuno Sá

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

* RE: [PATCH v5 4/9] iio: adc: rzg2l_adc: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 ` [PATCH v5 4/9] iio: adc: rzg2l_adc: " Jonathan Cameron
@ 2024-02-27  9:52   ` Prabhakar Mahadev Lad
  2024-02-27 19:16     ` Jonathan Cameron
  0 siblings, 1 reply; 16+ messages in thread
From: Prabhakar Mahadev Lad @ 2024-02-27  9:52 UTC (permalink / raw
  To: Jonathan Cameron, linux-iio@vger.kernel.org, Nuno Sá,
	Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Dmitry Baryshkov, Marijn Suijten, Marius Cristea, Ibrahim Tilki,
	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(-)
> 
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> 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.44.0


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

* Re: [PATCH v5 4/9] iio: adc: rzg2l_adc: Use device_for_each_child_node_scoped()
  2024-02-27  9:52   ` Prabhakar Mahadev Lad
@ 2024-02-27 19:16     ` Jonathan Cameron
  0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Cameron @ 2024-02-27 19:16 UTC (permalink / raw
  To: Prabhakar Mahadev Lad
  Cc: linux-iio@vger.kernel.org, Nuno Sá, Andy Shevchenko,
	Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Fabrice Gasnier,
	Dmitry Baryshkov, Marijn Suijten, Marius Cristea, Ibrahim Tilki,
	Jonathan Cameron

On Tue, 27 Feb 2024 09:52:28 +0000
Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com> 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: 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(-)
> >   
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
I already picked up some of this series piecemeal so I'll keep going!

Applied to the togreg branch of iio.git and pushed out as testing for 0-day to take a look.

Thanks!

Jonathan

> Cheers,
> Prabhakar
> 
> > 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.44.0  
> 


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

* Re: [PATCH v5 5/9] iio: adc: stm32: Use device_for_each_child_node_scoped()
  2024-02-24 12:32 ` [PATCH v5 5/9] iio: adc: stm32: " Jonathan Cameron
@ 2024-02-29 11:34   ` Fabrice Gasnier
  0 siblings, 0 replies; 16+ messages in thread
From: Fabrice Gasnier @ 2024-02-29 11:34 UTC (permalink / raw
  To: Jonathan Cameron, linux-iio, Nuno Sá, Andy Shevchenko
  Cc: Cosmin Tanislav, Mihail Chindris, Rasmus Villemoes,
	Tomislav Denis, Marek Vasut, Olivier Moysan, Lad Prabhakar,
	Dmitry Baryshkov, Marijn Suijten, Marius Cristea, Ibrahim Tilki,
	Jonathan Cameron

On 2/24/24 13:32, 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.

Hi Jonathan,

Indeed, please find later question, inline.

> 
> 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>
> 
> ---
> v5: Use a local struct device pointer.
>     Add brackets back I shouldn't have dropped.
> 
> Andy had a number of other comments but they would be unrelated changes
> so I'm leaving them for a future patch set.
> ---
>  drivers/iio/adc/stm32-adc.c | 61 +++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index b5d3c9cea5c4..36add95212c3 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -2187,58 +2187,52 @@ 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;
> +	struct device *dev = &indio_dev->dev;
>  	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(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(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(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;
> +				return ret;
>  		} else if (ret != -EINVAL) {
> -			dev_err(&indio_dev->dev, "Invalid label %d\n", ret);
> -			goto err;
> +			return dev_err_probe(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(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]);

Good catch! I think you're talking about a missing "ret = -EINVAL;" here.

Do you think this should be split as a precursor fix patch, so it can be
picked into stable release ?

Please let me know. Appart from that, you can add my:

Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Acked-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Thanks,
Best Regards,
Fabrice

> -				goto err;
> -			}
> +			if (vin[0] != val || vin[1] >= adc_info->max_channels)
> +				return dev_err_probe(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(dev, ret,
> +					     "Invalid diff-channels property\n");
>  		}
>  
>  		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
> @@ -2247,11 +2241,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(dev, ret,
> +					     "Invalid st,min-sample-time-ns property\n");
>  
>  		stm32_adc_smpr_init(adc, channels[scan_index].channel, val);
>  		if (differential)
> @@ -2261,11 +2253,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)

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

end of thread, other threads:[~2024-02-29 11:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-24 12:32 [PATCH v5 0/9] IIO: Use device_for_each_child_scope() Jonathan Cameron
2024-02-24 12:32 ` [PATCH v5 1/9] iio: temp: ltc2983: Use __free(fwnode_handle) and device_for_each_node_scoped() Jonathan Cameron
2024-02-26  8:35   ` Nuno Sá
2024-02-26 19:30     ` Jonathan Cameron
2024-02-27  8:08       ` Nuno Sá
2024-02-24 12:32 ` [PATCH v5 2/9] iio: adc: mcp3564: Use device_for_each_child_node_scoped() Jonathan Cameron
2024-02-24 12:32 ` [PATCH v5 3/9] iio: adc: qcom-spmi-adc5: " Jonathan Cameron
2024-02-24 12:32 ` [PATCH v5 4/9] iio: adc: rzg2l_adc: " Jonathan Cameron
2024-02-27  9:52   ` Prabhakar Mahadev Lad
2024-02-27 19:16     ` Jonathan Cameron
2024-02-24 12:32 ` [PATCH v5 5/9] iio: adc: stm32: " Jonathan Cameron
2024-02-29 11:34   ` Fabrice Gasnier
2024-02-24 12:32 ` [PATCH v5 6/9] iio: adc: ti-ads1015: " Jonathan Cameron
2024-02-24 12:32 ` [PATCH v5 7/9] iio: adc: ti-ads131e08: " Jonathan Cameron
2024-02-24 12:32 ` [PATCH v5 8/9] iio: dac: ad3552r: " Jonathan Cameron
2024-02-24 12:32 ` [PATCH v5 9/9] iio: dac: ad5770r: " 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 read-only IMAP folder(s) and NNTP newsgroup(s).