Re: [PATCH v4 02/16] iio: adc: at91-sama5d2_adc: use cleanup.h for NVMEM buffer

[Jonathan Cameron]

On Mon, 27 Jul 2026 17:56:19 +0530
Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx> wrote:

> Use __free(kfree) and __free(nvmem_cell_put) cleanup helpers in
> at91_adc_temp_sensor_init() to simplify error handling paths.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> Signed-off-by: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx>
A couple of questions inline

> ---
>  drivers/iio/adc/at91-sama5d2_adc.c | 33 +++++++++++++-----------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index e8a5285bb6d4..4a4a25f3c715 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -35,6 +35,8 @@
>  
>  #include <dt-bindings/iio/adc/at91-sama5d2_adc.h>
>  
> +DEFINE_FREE(nvmem_cell_put, struct nvmem_cell *, if (_T) nvmem_cell_put(_T))
I've lost track if it was previously discussed, but did you propose this for
more general useage in nvmem-consumer.h

The first randomly selected file I opened with nvmem_cell_put() could
make use of this so I assume it is generally useful?


> +
>  struct at91_adc_reg_layout {
>  /* Control Register */
>  	u16				CR;
> @@ -2249,33 +2251,28 @@ static int at91_adc_temp_sensor_init(struct at91_adc_state *st,
>  				     struct device *dev)
>  {
>  	struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb;
> -	struct nvmem_cell *temp_calib;
> -	u32 *buf;
>  	size_t len;
> -	int ret = 0;
>  
>  	if (!st->soc_info.platform->temp_sensor)
>  		return 0;
>  
>  	/* Get the calibration data from NVMEM. */
> -	temp_calib = nvmem_cell_get(dev, "temperature_calib");
> +	struct nvmem_cell *temp_calib __free(nvmem_cell_put) =
> +		nvmem_cell_get(dev, "temperature_calib");
>  	if (IS_ERR(temp_calib)) {
> -		ret = PTR_ERR(temp_calib);

Why this change?  Just to avoid the need for ret?  
I'd keep it and reduce the noise in the patch.
 
> -		if (ret != -ENOENT)
> +		if (PTR_ERR(temp_calib) != -ENOENT)
>  			dev_err(dev, "Failed to get temperature_calib cell!\n");
> -		return ret;
> +		return PTR_ERR(temp_calib);
>  	}
>  
> -	buf = nvmem_cell_read(temp_calib, &len);
> -	nvmem_cell_put(temp_calib);
> -	if (IS_ERR(buf)) {
> -		dev_err(dev, "Failed to read calibration data!\n");
> -		return PTR_ERR(buf);
> -	}
> -	if (len < AT91_ADC_TS_CLB_IDX_MAX * 4) {
> +	u32 *buf __free(kfree) = nvmem_cell_read(temp_calib, &len);
> +	if (IS_ERR(buf))
> +		return dev_err_probe(dev, PTR_ERR(buf),
> +				     "Failed to read calibration data!\n");
> +
> +	if (len < AT91_ADC_TS_CLB_IDX_MAX * sizeof(*buf)) {
>  		dev_err(dev, "Invalid calibration data!\n");
> -		ret = -EINVAL;
> -		goto free_buf;
> +		return -EINVAL;
>  	}
>  
>  	/* Store calibration data for later use. */
> @@ -2288,9 +2285,7 @@ static int at91_adc_temp_sensor_init(struct at91_adc_state *st,
>  	 */
>  	clb->p1 = clb->p1 * 1000;
>  
> -free_buf:
> -	kfree(buf);
> -	return ret;
> +	return 0;
>  }
>  
>  static int at91_adc_probe(struct platform_device *pdev)