[v2,06/20] iio: dummy: Use a fixed structure to build up scan to push to buffers.

Commit Message

Jonathan Cameron April 6, 2025, 5:19 p.m. UTC

From: Jonathan Cameron <[e-mail]>

It has long been discouraged for drivers to make use of iio_dev->scan_bytes
directly as that is an implementation detail of the core. As such our
example driver should definitely not be doing so.

A simple anonymous structure definition suffices here as even though
we have a mixture of signed and unsigned channels only the signed ones
use the full storage so the unsigned channels can used signed types as
well.

Signed-off-by: Jonathan Cameron <[e-mail]>
---
v2: Add a comment about stack buffers not being DMA safe.
---
 drivers/iio/dummy/iio_simple_dummy_buffer.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Andy Shevchenko April 7, 2025, 9:23 a.m. UTC | #1

On Sun, Apr 06, 2025 at 06:19:47PM +0100, Jonathan Cameron wrote:
> From: Jonathan Cameron <[e-mail]>
> 
> It has long been discouraged for drivers to make use of iio_dev->scan_bytes
> directly as that is an implementation detail of the core. As such our
> example driver should definitely not be doing so.
> 
> A simple anonymous structure definition suffices here as even though
> we have a mixture of signed and unsigned channels only the signed ones
> use the full storage so the unsigned channels can used signed types as
> well.

...

> ---
> v2: Add a comment about stack buffers not being DMA safe.

> -	data = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
> -	if (!data)
> -		goto done;
> +	/*
> +	 * Note that some buses such as SPI require DMA safe buffers which
> +	 * cannot be on the stack.
> +	 */
> +	struct {
> +		s16 data[ARRAY_SIZE(fakedata)];
> +		aligned_s64 timestamp;
> +	} scan;

But why not continue using kmemdup() / kzalloc() for this and put
ithe comment there, which would make more sense? Then the device
driver developer will choose either to use the on-stack or heap one?

Or didn't I get the idea behind all this?

Patch

diff --git a/drivers/iio/dummy/iio_simple_dummy_buffer.c b/drivers/iio/dummy/iio_simple_dummy_buffer.c
index 288880346707..eca5f0652d23 100644
--- a/drivers/iio/dummy/iio_simple_dummy_buffer.c
+++ b/drivers/iio/dummy/iio_simple_dummy_buffer.c
@@ -46,11 +46,14 @@  static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	int i = 0, j;
-	u16 *data;
-
-	data = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
-	if (!data)
-		goto done;
+	/*
+	 * Note that some buses such as SPI require DMA safe buffers which
+	 * cannot be on the stack.
+	 */
+	struct {
+		s16 data[ARRAY_SIZE(fakedata)];
+		aligned_s64 timestamp;
+	} scan;
 
 	/*
 	 * Three common options here:
@@ -69,14 +72,11 @@  static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
 	 * constant table fakedata.
 	 */
 	iio_for_each_active_channel(indio_dev, j)
-		data[i++] = fakedata[j];
+		scan.data[i++] = fakedata[j];
 
-	iio_push_to_buffers_with_timestamp(indio_dev, data,
+	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
 					   iio_get_time_ns(indio_dev));
 
-	kfree(data);
-
-done:
 	/*
 	 * Tell the core we are done with this trigger and ready for the
 	 * next one.