[PATCH] iio: proximity: srf08: use stack allocated scan buffer

[PATCH] iio: proximity: srf08: use stack allocated scan buffer

[David Lechner]

Use a stack allocated scan struct in srf08_trigger_handler(). Since the
scan buffer isn't used outside of this function and doesn't need to be
DMA-safe, it doesn't need to be in struct srf08_data. We can also
eliminate an extra local variable for the return value of
srf08_read_ranging() by using scan.chan directly.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/proximity/srf08.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
index 6e32fdfd161b93a5624f757d5b7de579415b1055..a28efcf324a844a6dca43dff69e71ca38a2ccc68 100644
--- a/drivers/iio/proximity/srf08.c
+++ b/drivers/iio/proximity/srf08.c
@@ -63,12 +63,6 @@ struct srf08_data {
 	int			range_mm;
 	struct mutex		lock;
 
-	/* Ensure timestamp is naturally aligned */
-	struct {
-		s16 chan;
-		aligned_s64 timestamp;
-	} scan;
-
 	/* Sensor-Type */
 	enum srf08_sensor_type	sensor_type;
 
@@ -182,16 +176,18 @@ static irqreturn_t srf08_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct srf08_data *data = iio_priv(indio_dev);
-	s16 sensor_data;
+	struct {
+		s16 chan;
+		aligned_s64 timestamp;
+	} scan;
 
-	sensor_data = srf08_read_ranging(data);
-	if (sensor_data < 0)
+	scan.chan = srf08_read_ranging(data);
+	if (scan.chan < 0)
 		goto err;
 
 	mutex_lock(&data->lock);
 
-	data->scan.chan = sensor_data;
-	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
 				    pf->timestamp);
 
 	mutex_unlock(&data->lock);

---
base-commit: f8f559752d573a051a984adda8d2d1464f92f954
change-id: 20250711-iio-use-more-iio_declare_buffer_with_ts-6-6ffc8e99552d

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

Re: [PATCH] iio: proximity: srf08: use stack allocated scan buffer

[Andy Shevchenko]

On Fri, Jul 11, 2025 at 11:07:20AM -0500, David Lechner wrote:
> Use a stack allocated scan struct in srf08_trigger_handler(). Since the
> scan buffer isn't used outside of this function and doesn't need to be
> DMA-safe, it doesn't need to be in struct srf08_data. We can also
> eliminate an extra local variable for the return value of
> srf08_read_ranging() by using scan.chan directly.

Why not with macro?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: proximity: srf08: use stack allocated scan buffer

[David Lechner]

On 7/11/25 11:44 AM, Andy Shevchenko wrote:
> On Fri, Jul 11, 2025 at 11:07:20AM -0500, David Lechner wrote:
>> Use a stack allocated scan struct in srf08_trigger_handler(). Since the
>> scan buffer isn't used outside of this function and doesn't need to be
>> DMA-safe, it doesn't need to be in struct srf08_data. We can also
>> eliminate an extra local variable for the return value of
>> srf08_read_ranging() by using scan.chan directly.
> 
> Why not with macro?
> 

In cases like this where there are a fixed number of data values
read, the existing pattern is to use the struct like this. Furthermore,
IIO_DECLARE_BUFFER_WITH_TS() implies an array and usually we try to avoid
arrays with only one element.

Re: [PATCH] iio: proximity: srf08: use stack allocated scan buffer

[Andreas Klinger]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

Reviewed-by: Andreas Klinger <ak@it-klinger.de>

David Lechner <dlechner@baylibre.com> schrieb am Fr, 11. Jul 11:07:
> Use a stack allocated scan struct in srf08_trigger_handler(). Since the
> scan buffer isn't used outside of this function and doesn't need to be
> DMA-safe, it doesn't need to be in struct srf08_data. We can also
> eliminate an extra local variable for the return value of
> srf08_read_ranging() by using scan.chan directly.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/proximity/srf08.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
> index 6e32fdfd161b93a5624f757d5b7de579415b1055..a28efcf324a844a6dca43dff69e71ca38a2ccc68 100644
> --- a/drivers/iio/proximity/srf08.c
> +++ b/drivers/iio/proximity/srf08.c
> @@ -63,12 +63,6 @@ struct srf08_data {
>  	int			range_mm;
>  	struct mutex		lock;
>  
> -	/* Ensure timestamp is naturally aligned */
> -	struct {
> -		s16 chan;
> -		aligned_s64 timestamp;
> -	} scan;
> -
>  	/* Sensor-Type */
>  	enum srf08_sensor_type	sensor_type;
>  
> @@ -182,16 +176,18 @@ static irqreturn_t srf08_trigger_handler(int irq, void *p)
>  	struct iio_poll_func *pf = p;
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct srf08_data *data = iio_priv(indio_dev);
> -	s16 sensor_data;
> +	struct {
> +		s16 chan;
> +		aligned_s64 timestamp;
> +	} scan;
>  
> -	sensor_data = srf08_read_ranging(data);
> -	if (sensor_data < 0)
> +	scan.chan = srf08_read_ranging(data);
> +	if (scan.chan < 0)
>  		goto err;
>  
>  	mutex_lock(&data->lock);
>  
> -	data->scan.chan = sensor_data;
> -	iio_push_to_buffers_with_ts(indio_dev, &data->scan, sizeof(data->scan),
> +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
>  				    pf->timestamp);
>  
>  	mutex_unlock(&data->lock);
> 
> ---
> base-commit: f8f559752d573a051a984adda8d2d1464f92f954
> change-id: 20250711-iio-use-more-iio_declare_buffer_with_ts-6-6ffc8e99552d
> 
> Best regards,
> -- 
> David Lechner <dlechner@baylibre.com>
> 

-- 
Andreas Klinger
Grabenreith 27
84508 Burgkirchen
+49 8623 373
ak@it-klinger.de
www.it-klinger.de
www.grabenreith.de

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] iio: proximity: srf08: use stack allocated scan buffer

[Jonathan Cameron]

On Fri, 11 Jul 2025 12:07:10 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 7/11/25 11:44 AM, Andy Shevchenko wrote:
> > On Fri, Jul 11, 2025 at 11:07:20AM -0500, David Lechner wrote:  
> >> Use a stack allocated scan struct in srf08_trigger_handler(). Since the
> >> scan buffer isn't used outside of this function and doesn't need to be
> >> DMA-safe, it doesn't need to be in struct srf08_data. We can also
> >> eliminate an extra local variable for the return value of
> >> srf08_read_ranging() by using scan.chan directly.  
> > 
> > Why not with macro?
> >   
> 
> In cases like this where there are a fixed number of data values
> read, the existing pattern is to use the struct like this. Furthermore,
> IIO_DECLARE_BUFFER_WITH_TS() implies an array and usually we try to avoid
> arrays with only one element.
> 

I'd go a little further and say that to me structures are appropriate
even with multiple channels as long as they location of channels that
we are naming differently doesn't change.  That normally (but not always)
means the timestamp channel as the other channels tend to have the same
type.  So in practice if the channels take up 8 bytes or less before
the timestamp.

Jonathan