* [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