* [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h>
@ 2026-04-24 8:18 Giorgi Tchankvetadze
2026-04-24 9:07 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Giorgi Tchankvetadze @ 2026-04-24 8:18 UTC (permalink / raw)
To: antoniu.miclaus, lars, Michael.Hennerich, jic23
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel,
Giorgi Tchankvetadze
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.
Signed-off-by: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com>
---
drivers/iio/adc/ti-ads7138.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/adc/ti-ads7138.c b/drivers/iio/adc/ti-ads7138.c
index ee5c1b8e3a8e..ba1e9af92c6e 100644
--- a/drivers/iio/adc/ti-ads7138.c
+++ b/drivers/iio/adc/ti-ads7138.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
#include <linux/unaligned.h>
#include <linux/iio/events.h>
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h>
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
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2026-04-24 9:07 UTC (permalink / raw)
To: Giorgi Tchankvetadze
Cc: antoniu.miclaus, lars, Michael.Hennerich, jic23, dlechner,
nuno.sa, andy, linux-iio, linux-kernel
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?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h>
2026-04-24 9:07 ` Andy Shevchenko
@ 2026-04-24 10:36 ` Jonathan Cameron
2026-04-24 11:33 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2026-04-24 10:36 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giorgi Tchankvetadze, antoniu.miclaus, lars, Michael.Hennerich,
dlechner, nuno.sa, andy, linux-iio, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h>
2026-04-24 10:36 ` Jonathan Cameron
@ 2026-04-24 11:33 ` Andy Shevchenko
2026-04-24 16:57 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2026-04-24 11:33 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Giorgi Tchankvetadze, antoniu.miclaus, lars, Michael.Hennerich,
dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Fri, Apr 24, 2026 at 11:36:00AM +0100, Jonathan Cameron wrote:
> 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:
...
> > 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.
Ah, I thought it's opposite, but looking at the usage it seems like it's host
controller driver responsibility to use DMA or not for the transfer.
Why do we need to have kmalloc() here at all? I don't understand that.
> This function is just overly flexible.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h>
2026-04-24 11:33 ` Andy Shevchenko
@ 2026-04-24 16:57 ` Jonathan Cameron
2026-04-27 11:29 ` Giorgi Tchankvetadze
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2026-04-24 16:57 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giorgi Tchankvetadze, antoniu.miclaus, lars, Michael.Hennerich,
dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Fri, 24 Apr 2026 14:33:45 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Fri, Apr 24, 2026 at 11:36:00AM +0100, Jonathan Cameron wrote:
> > 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:
>
> ...
>
> > > 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.
>
> Ah, I thought it's opposite, but looking at the usage it seems like it's host
> controller driver responsibility to use DMA or not for the transfer.
>
> Why do we need to have kmalloc() here at all? I don't understand that.
Absolutely. Locally it looks like length might be large and therefore
the array not suitable to be on the stack, but that's not the case.
length == 2 so the array only has 4 elements and can go on the stack.
>
> > This function is just overly flexible.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h>
2026-04-24 16:57 ` Jonathan Cameron
@ 2026-04-27 11:29 ` Giorgi Tchankvetadze
2026-04-28 18:14 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Giorgi Tchankvetadze @ 2026-04-27 11:29 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, antoniu.miclaus, lars, Michael.Hennerich,
dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Fri, Apr 24, 2026 at 8:58 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> > Why do we need to have kmalloc() here at all? I don't understand that.
> Absolutely. Locally it looks like length might be large and therefore
> the array not suitable to be on the stack, but that's not the case.
> length == 2 so the array only has 4 elements and can go on the stack.
>
> >
> > > This function is just overly flexible.
> >
>
Thanks Jonathan and Andy. I have sent v2.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: adc: ti-ads7138: explicitly include <linux/slab.h>
2026-04-27 11:29 ` Giorgi Tchankvetadze
@ 2026-04-28 18:14 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2026-04-28 18:14 UTC (permalink / raw)
To: Giorgi Tchankvetadze
Cc: Andy Shevchenko, antoniu.miclaus, lars, Michael.Hennerich,
dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Mon, 27 Apr 2026 15:29:57 +0400
Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com> wrote:
> On Fri, Apr 24, 2026 at 8:58 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
>
> > > Why do we need to have kmalloc() here at all? I don't understand that.
> > Absolutely. Locally it looks like length might be large and therefore
> > the array not suitable to be on the stack, but that's not the case.
> > length == 2 so the array only has 4 elements and can go on the stack.
> >
> > >
> > > > This function is just overly flexible.
> > >
> >
>
> Thanks Jonathan and Andy. I have sent v2.
Whilst you are welcome you've fallen into the process related trap
of being too polite! Better to thank us in the change log of v2
and we all have one less email to check.
Note, pretty much everyone does this when they start working on the
kernel and so get this email once!
Jonathan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-04-28 18:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox