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

[Jonathan Cameron]

On Tue, 30 Jun 2026 15:05:52 +0530
Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx> wrote:

> Use __free(kfree) cleanup helper for the NVMEM data buffer in
> at91_adc_temp_sensor_init() to simplify error handling paths.
> 
> Since __free(kfree) requires a valid kfree-able pointer (not an

Does it require a a kfree-able pointer?
Definition is:
DEFINE_FREE(kfree, void *, if (!IS_ERR_OR_NULL(_T)) kfree(_T))

Some of these DEFINE_FREE() uses did change to be more resilient
to errors so maybe you have an old kernel?

> ERR_PTR), store nvmem_cell_read() result in a temporary void pointer
> first, check for errors, then assign to the managed buffer.
> 
> Signed-off-by: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx>
> ---
>  drivers/iio/adc/at91-sama5d2_adc.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 255970b2e747..5015c234289e 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -2251,9 +2251,10 @@ static int at91_adc_temp_sensor_init(struct at91_adc_state *st,
>  {
>  	struct at91_adc_temp_sensor_clb *clb = &st->soc_info.temp_sensor_clb;
>  	struct nvmem_cell *temp_calib;
> -	u32 *buf;
> +	u32 *buf __free(kfree) = NULL;
This breaks the 'rule' about having the destructor defined right next to the
destructor (IIRC there is guidance on this in cleanup.h comments).  Linus is
very keen on this always being done and doesn't like the = NULL pattern at all
(I agree but easier to blame the chief Penguin ;)

Given the argument for this seems to be wrong anyway, just define and assign
in one line below.


> +	void *cell_data;
>  	size_t len;
> -	int ret = 0;
> +	int ret;
>  
>  	if (!st->soc_info.platform->temp_sensor)
>  		return 0;
> @@ -2267,16 +2268,18 @@ static int at91_adc_temp_sensor_init(struct at91_adc_state *st,
>  		return ret;
>  	}
>  
> -	buf = nvmem_cell_read(temp_calib, &len);

	

> +	cell_data = nvmem_cell_read(temp_calib, &len);
>  	nvmem_cell_put(temp_calib);

This dance with nvmem_cell_put being called before the error check
seems like another place a cleanup.h trick is useful.

Can we have a DEFINE_FREE() for nvmem_cell_put()  I don't think
ti will matter if we hold that reference for the scope of the rest
of this function - but do check that!

With that in place, you can just do
	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");
	
	}
> -	if (IS_ERR(buf)) {
> +	if (IS_ERR(cell_data)) {
>  		dev_err(dev, "Failed to read calibration data!\n");
> -		return PTR_ERR(buf);
> +		return PTR_ERR(cell_data);
>  	}
> +
> +	buf = cell_data;

> +
>  	if (len < AT91_ADC_TS_CLB_IDX_MAX * 4) {
>  		dev_err(dev, "Invalid calibration data!\n");
> -		ret = -EINVAL;
> -		goto free_buf;
> +		return -EINVAL;
>  	}
>  
>  	/* Store calibration data for later use. */
> @@ -2289,9 +2292,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)