On Sat, Feb 17, 2024 at 04:42:43PM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> 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
