Re: [PATCH v2] iio: adc: ti-ads7138: replace kmalloc() with stack allocation in i2c_write_block

[David Laight <david.laight.linux@gmail.com>]

On Mon, 27 Apr 2026 15:27:07 +0400
Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com> wrote:

> The ads7138_i2c_write_block() function currently utilizes kmalloc()
> to allocate a buffer for I2C transfers. However, the length
> parameter passed to this function is strictly 2 bytes across all
> driver invocations, making the total payload buffer size exactly 4 bytes.
> Invoking the heap allocator for a 4-byte buffer introduces
> unnecessary SLUB overhead. 

Have you confirmed that the buffer is never used for DMA?

Provided the lock that blocks concurrent access from two threads
is actually outside this code, a buffer for short transfers could
be allocated within 'struct i2c_client'.

	David

> 
> Replace the kmalloc() call with a statically sized 4-byte stack array.
> Add a boundary check returning -EINVAL to ensure future driver
> modifications do not overflow the stack buffer if a length
> greater than 2 is requested.
> Furthermore, dropping the dynamic allocation entirely removes the need to
> include <linux/slab.h> (which was the original motivation for this patch)
> and streamlines the PIO transfer path.
> 
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com>
> ---
>  drivers/iio/adc/ti-ads7138.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads7138.c b/drivers/iio/adc/ti-ads7138.c
> index ee5c1b8e3a8e..172893cbc17d 100644
> --- a/drivers/iio/adc/ti-ads7138.c
> +++ b/drivers/iio/adc/ti-ads7138.c
> @@ -102,21 +102,20 @@ static const int ads7138_oversampling_ratios[] = {
>  static int ads7138_i2c_write_block(const struct i2c_client *client, u8 reg,
>  				   u8 *values, u8 length)
>  {
> +	u8 buf[4];
>  	int ret;
> -	int len = length + 2; /* "+ 2" for OPCODE and reg */
>  
> -	u8 *buf __free(kfree) = kmalloc(len, GFP_KERNEL);
> -	if (!buf)
> -		return -ENOMEM;
> +	if (length != 2)
> +		return -EINVAL;
>  
>  	buf[0] = ADS7138_OPCODE_BLOCK_WRITE;
>  	buf[1] = reg;
>  	memcpy(&buf[2], values, length);
>  
> -	ret = i2c_master_send(client, buf, len);
> +	ret = i2c_master_send(client, buf, ARRAY_SIZE(buf));
>  	if (ret < 0)
>  		return ret;
> -	if (ret != len)
> +	if (ret != ARRAY_SIZE(buf))
>  		return -EIO;
>  
>  	return 0;