Re: [PATCH v2] iio: st_sensors: drop temporary kmalloc buffer and reuse buffer_data

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 11 Mar 2026 23:50:50 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> Replace the per-call kmalloc() scratch buffer with the statically allocated
> buffer_data[] field present in struct st_sensor_data. The existing buffer
> is DMA-aligned and sufficiently sized for all channel widths, so using it
> avoids unnecessary dynamic memory allocation on each read.
> 
> This simplifies the code, removes redundent code and allocation.
> No functional change intended.
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
> Changes in v2:
> - split series to individual patch
> - address review comment from David Lechner and reuse exising buffer instead of allocation
> - Link to v1 https://lore.kernel.org/all/20260310200513.2162018-4-sanjayembedded@gmail.com/
> ---
>  drivers/iio/common/st_sensors/st_sensors_core.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index dac593be5695..488e2bf6d117 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -495,20 +495,17 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
>  				     struct iio_chan_spec const *ch, int *data)
>  {
>  	int err;
> -	u8 *outdata;
>  	struct st_sensor_data *sdata = iio_priv(indio_dev);
>  	unsigned int byte_for_channel;
>  
>  	byte_for_channel = DIV_ROUND_UP(ch->scan_type.realbits +
>  					ch->scan_type.shift, 8);
> -	outdata = kmalloc(byte_for_channel, GFP_DMA | GFP_KERNEL);
> -	if (!outdata)
> -		return -ENOMEM;
> +	u8 *outdata = (u8 *)sdata->buffer_data;
Why is buffer_data a char array?  It's already implicitly cast to a u8
in one place. I'd see if it actually wants to be a char anywhere in the driver.
If not fix that as a precursor patch to this one.

I'm not sure I'd bother having a local variable for outdata given
it is only used a few times but I don't mind it much (beyond what Andy
pointed out about declaring it here).

Jonathan

>  
>  	err = regmap_bulk_read(sdata->regmap, ch->address,
>  			       outdata, byte_for_channel);
>  	if (err < 0)
> -		goto st_sensors_free_memory;
> +		return err;
>  
>  	if (byte_for_channel == 1)
>  		*data = (s8)*outdata;
> @@ -517,10 +514,7 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
>  	else if (byte_for_channel == 3)
>  		*data = (s32)sign_extend32(get_unaligned_le24(outdata), 23);
>  
> -st_sensors_free_memory:
> -	kfree(outdata);
> -
> -	return err;
> +	return 0;
>  }
>  
>  int st_sensors_read_info_raw(struct iio_dev *indio_dev,