| Message ID | 20240224123215.161469-6-jic23@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IIO: Use device_for_each_child_scope() | expand |
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)
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)