iio: adc: ad7606: explicit timestamp alignment

[PATCH] iio: adc: ad7606: explicit timestamp alignment

Posted by David Lechner 9 months, 2 weeks ago

Use struct with aligned_s64 timestamp to make timestamp alignment
explicit. Technically, what we have works because for all known
architectures, IIO_DMA_MINALIGN is a multiple of __alignof__(s64).
But this way, we don't have to make people read the comments to know
why there are extra elements in each buffer.

---
 drivers/iio/adc/ad7606.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
index 89d49551eaf515bab9706c12bff056dfcb707b67..441e62c521bcbea69b4f70bb2d55f65334d22276 100644
--- a/drivers/iio/adc/ad7606.h
+++ b/drivers/iio/adc/ad7606.h
@@ -155,12 +155,15 @@ struct ad7606_state {
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
 	 * transfer buffers to live in their own cache lines.
-	 * 16 * 16-bit samples + 64-bit timestamp - for AD7616
-	 * 8 * 32-bit samples + 64-bit timestamp - for AD7616C-18 (and similar)
+	 * 16 * 16-bit samples for AD7616
+	 * 8 * 32-bit samples for AD7616C-18 (and similar)
 	 */
-	union {
-		u16 buf16[20];
-		u32 buf32[10];
+	struct {
+		union {
+			u16 buf16[16];
+			u32 buf32[8];
+		};
+		aligned_s64 timestamp;
 	} data __aligned(IIO_DMA_MINALIGN);
 	__be16				d16[2];
 };

---
base-commit: b475195fecc79a1a6e7fb0846aaaab0a1a4cb2e6
change-id: 20250428-iio-adc-ad7606-fix-buffer-alignment-9fcde71687dc

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

Re: [PATCH] iio: adc: ad7606: explicit timestamp alignment

Posted by Nuno Sá 9 months, 1 week ago

On Mon, 2025-04-28 at 21:17 -0500, David Lechner wrote:
> Use struct with aligned_s64 timestamp to make timestamp alignment
> explicit. Technically, what we have works because for all known
> architectures, IIO_DMA_MINALIGN is a multiple of __alignof__(s64).
> But this way, we don't have to make people read the comments to know
> why there are extra elements in each buffer.
> 
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/ad7606.h | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> index
> 89d49551eaf515bab9706c12bff056dfcb707b67..441e62c521bcbea69b4f70bb2d55f65334d2
> 2276 100644
> --- a/drivers/iio/adc/ad7606.h
> +++ b/drivers/iio/adc/ad7606.h
> @@ -155,12 +155,15 @@ struct ad7606_state {
>  	/*
>  	 * DMA (thus cache coherency maintenance) may require the
>  	 * transfer buffers to live in their own cache lines.
> -	 * 16 * 16-bit samples + 64-bit timestamp - for AD7616
> -	 * 8 * 32-bit samples + 64-bit timestamp - for AD7616C-18 (and
> similar)
> +	 * 16 * 16-bit samples for AD7616
> +	 * 8 * 32-bit samples for AD7616C-18 (and similar)
>  	 */
> -	union {
> -		u16 buf16[20];
> -		u32 buf32[10];
> +	struct {
> +		union {
> +			u16 buf16[16];
> +			u32 buf32[8];
> +		};
> +		aligned_s64 timestamp;
>  	} data __aligned(IIO_DMA_MINALIGN);
>  	__be16				d16[2];
>  };
> 
> ---
> base-commit: b475195fecc79a1a6e7fb0846aaaab0a1a4cb2e6
> change-id: 20250428-iio-adc-ad7606-fix-buffer-alignment-9fcde71687dc
> 
> Best regards,

Re: [PATCH] iio: adc: ad7606: explicit timestamp alignment

Posted by Jonathan Cameron 9 months, 1 week ago

On Tue, 29 Apr 2025 14:01:54 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, 2025-04-28 at 21:17 -0500, David Lechner wrote:
> > Use struct with aligned_s64 timestamp to make timestamp alignment
> > explicit. Technically, what we have works because for all known
> > architectures, IIO_DMA_MINALIGN is a multiple of __alignof__(s64).
> > But this way, we don't have to make people read the comments to know
> > why there are extra elements in each buffer.
> > 
> > ---  
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Applied to the togreg branch of iio.git and pushed out as testing
for all the normal reasons.

Thanks,

Jonathan

> 
> >  drivers/iio/adc/ad7606.h | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad7606.h b/drivers/iio/adc/ad7606.h
> > index
> > 89d49551eaf515bab9706c12bff056dfcb707b67..441e62c521bcbea69b4f70bb2d55f65334d2
> > 2276 100644
> > --- a/drivers/iio/adc/ad7606.h
> > +++ b/drivers/iio/adc/ad7606.h
> > @@ -155,12 +155,15 @@ struct ad7606_state {
> >  	/*
> >  	 * DMA (thus cache coherency maintenance) may require the
> >  	 * transfer buffers to live in their own cache lines.
> > -	 * 16 * 16-bit samples + 64-bit timestamp - for AD7616
> > -	 * 8 * 32-bit samples + 64-bit timestamp - for AD7616C-18 (and
> > similar)
> > +	 * 16 * 16-bit samples for AD7616
> > +	 * 8 * 32-bit samples for AD7616C-18 (and similar)
> >  	 */
> > -	union {
> > -		u16 buf16[20];
> > -		u32 buf32[10];
> > +	struct {
> > +		union {
> > +			u16 buf16[16];
> > +			u32 buf32[8];
> > +		};
> > +		aligned_s64 timestamp;
> >  	} data __aligned(IIO_DMA_MINALIGN);
> >  	__be16				d16[2];
> >  };
> > 
> > ---
> > base-commit: b475195fecc79a1a6e7fb0846aaaab0a1a4cb2e6
> > change-id: 20250428-iio-adc-ad7606-fix-buffer-alignment-9fcde71687dc
> > 
> > Best regards,

Re: [PATCH] iio: adc: ad7606: explicit timestamp alignment

Posted by David Lechner 9 months, 1 week ago

On 5/4/25 9:55 AM, Jonathan Cameron wrote:
> On Tue, 29 Apr 2025 14:01:54 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
>> On Mon, 2025-04-28 at 21:17 -0500, David Lechner wrote:
>>> Use struct with aligned_s64 timestamp to make timestamp alignment
>>> explicit. Technically, what we have works because for all known
>>> architectures, IIO_DMA_MINALIGN is a multiple of __alignof__(s64).
>>> But this way, we don't have to make people read the comments to know
>>> why there are extra elements in each buffer.
>>>
>>> ---  
>>
>> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> Applied to the togreg branch of iio.git and pushed out as testing
> for all the normal reasons.
> 
Not sure how we all managed to this, but this is missing:

Signed-off-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH] iio: adc: ad7606: explicit timestamp alignment

Posted by Jonathan Cameron 9 months, 1 week ago

On Mon, 5 May 2025 08:57:29 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/4/25 9:55 AM, Jonathan Cameron wrote:
> > On Tue, 29 Apr 2025 14:01:54 +0100
> > Nuno Sá <noname.nuno@gmail.com> wrote:
> >   
> >> On Mon, 2025-04-28 at 21:17 -0500, David Lechner wrote:  
> >>> Use struct with aligned_s64 timestamp to make timestamp alignment
> >>> explicit. Technically, what we have works because for all known
> >>> architectures, IIO_DMA_MINALIGN is a multiple of __alignof__(s64).
> >>> But this way, we don't have to make people read the comments to know
> >>> why there are extra elements in each buffer.
> >>>
> >>> ---    
> >>
> >> Reviewed-by: Nuno Sá <nuno.sa@analog.com>  
> > Applied to the togreg branch of iio.git and pushed out as testing
> > for all the normal reasons.
> >   
> Not sure how we all managed to this, but this is missing:
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> 

Oops. 
Added.