* [PATCH v2] iio: st_sensors: drop temporary kmalloc buffer and reuse buffer_data
@ 2026-03-11 18:20 Sanjay Chitroda
2026-03-11 20:21 ` Andy Shevchenko
2026-03-15 18:46 ` Jonathan Cameron
0 siblings, 2 replies; 3+ messages in thread
From: Sanjay Chitroda @ 2026-03-11 18:20 UTC (permalink / raw)
To: jic23, dlechner, nuno.sa, andy
Cc: kees, linux-iio, linux-kernel, sanjayembeddedse
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;
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,
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: st_sensors: drop temporary kmalloc buffer and reuse buffer_data
2026-03-11 18:20 [PATCH v2] iio: st_sensors: drop temporary kmalloc buffer and reuse buffer_data Sanjay Chitroda
@ 2026-03-11 20:21 ` Andy Shevchenko
2026-03-15 18:46 ` Jonathan Cameron
1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-03-11 20:21 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: jic23, dlechner, nuno.sa, andy, kees, linux-iio, linux-kernel
On Wed, Mar 11, 2026 at 8:21 PM 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.
Spell check your commit messages.
> No functional change intended.
...
> 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);
> + u8 *outdata = (u8 *)sdata->buffer_data;
No. We don't mess up with C99 definitions. The only exceptions are
loop iterators and scoped variables which are none in this case.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: st_sensors: drop temporary kmalloc buffer and reuse buffer_data
2026-03-11 18:20 [PATCH v2] iio: st_sensors: drop temporary kmalloc buffer and reuse buffer_data Sanjay Chitroda
2026-03-11 20:21 ` Andy Shevchenko
@ 2026-03-15 18:46 ` Jonathan Cameron
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2026-03-15 18:46 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dlechner, nuno.sa, andy, kees, linux-iio, linux-kernel
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,
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-15 18:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 18:20 [PATCH v2] iio: st_sensors: drop temporary kmalloc buffer and reuse buffer_data Sanjay Chitroda
2026-03-11 20:21 ` Andy Shevchenko
2026-03-15 18:46 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox