public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com>,
	antoniu.miclaus@analog.com, lars@metafoo.de,
	Michael.Hennerich@analog.com, dlechner@baylibre.com,
	nuno.sa@analog.com, andy@kernel.org, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h>
Date: Fri, 24 Apr 2026 11:36:00 +0100	[thread overview]
Message-ID: <20260424113600.4e76fdb7@jic23-huawei> (raw)
In-Reply-To: <aesyzOMUl2Iyjgjz@ashevche-desk.local>

On Fri, 24 Apr 2026 12:07:24 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Fri, Apr 24, 2026 at 12:18:10PM +0400, Giorgi Tchankvetadze wrote:
> > ti-ads7138.c uses kmalloc() and kfree(), which are provided by
> > linux/slab.h, but does not include that header directly.
> > The driver currently gets these declarations through linux/i2c.h.
> > Include linux/slab.h explicitly instead of relying on the transitive
> > include.  
> 
> If you want to make it real patch, please convert it to follow IWYU.
> What you are saying is basically comes from kernel.h (yeah, i2c.h happens
> earlier, but the kernel.h is PITA currently).
> 
> WRT to the kmalloc() use, wouldn't it be simply better to use
> i2c_master_send_dmasafe() and kill that kmalloc() dance in the driver?
> 

That sounds backwards. IIRC i2c_master_send_dmasafe() needs a heap allocation
to ensure a DMA safe buffer.  See i2c_get_dma_safe_msg_buf() and
the check on I2C_M_DMA_SAFE.

This function is just overly flexible. 
 
static int ads7138_i2c_write_block(const struct i2c_client *client, u8 reg,
				   u8 *values, u8 length)
{
	int ret;
	int len = length + 2; /* "+ 2" for OPCODE and reg */

	u8 *buf __free(kfree) = kmalloc(len, GFP_KERNEL);
	if (!buf)
		return -ENOMEM;

	buf[0] = ADS7138_OPCODE_BLOCK_WRITE;
	buf[1] = reg;
	memcpy(&buf[2], values, length);

	ret = i2c_master_send(client, buf, len);
	if (ret < 0)
		return ret;
	if (ret != len)
		return -EIO;

	return 0;
}
But length is always 2.  You could hammer it into i2c_smbus_write_i2c_block_data()
but that would be confusing at best as it would be putting the magic block write
in where the address normally goes and the address into the first data byte.

So I'd just go with
u8 buf[4];

if (length != 2)
	return -EINVAL;

Then use buf directly.

If you want to get fancy the caller could actually pass a u16 with val << 4
in the one caller (I think) and end up with same data but then we'd need
mess around with a memcpy to land that in the right bytes so maybe not
worth it.

To me either of these last two ways would be fine.

Jonathan


  reply	other threads:[~2026-04-24 10:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-24  8:18 [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h> Giorgi Tchankvetadze
2026-04-24  9:07 ` Andy Shevchenko
2026-04-24 10:36   ` Jonathan Cameron [this message]
2026-04-24 11:33     ` Andy Shevchenko
2026-04-24 16:57       ` Jonathan Cameron
2026-04-27 11:29         ` Giorgi Tchankvetadze
2026-04-28 18:14           ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260424113600.4e76fdb7@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Michael.Hennerich@analog.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=antoniu.miclaus@analog.com \
    --cc=dlechner@baylibre.com \
    --cc=giorgitchankvetadze1997@gmail.com \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox