Re: [PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Thu, 17 Apr 2025 12:07:37 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 4/17/25 11:59 AM, Andy Shevchenko wrote:
> > On Thu, Apr 17, 2025 at 11:52:38AM -0500, David Lechner wrote:  
> >> Align the buffers used with iio_push_to_buffers_with_timestamp() to
> >> ensure the s64 timestamp is aligned to 8 bytes.
> >>
> >>  drivers/iio/accel/bmc150-accel.h | 2 +-
> >>  drivers/iio/imu/adis16550.c      | 2 +-  
> > 
> > Looks like a stray squash of the two independent commits.  
> 
> Oops, sure enough.
> 
> > 
> > ...
> >   
> >>  	struct bmc150_accel_trigger triggers[BMC150_ACCEL_TRIGGERS];
> >>  	struct mutex mutex;
> >>  	u8 fifo_mode, watermark;
> >> -	s16 buffer[8];
> >> +	s16 buffer[8] __aligned(8);  
> > 
> > As for the code, would it be possible to convert to actually use a sturcture
> > rather than an array?  
> 
> I do personally prefer the struct pattern, but there are very many other drivers
> using this buffer pattern that I was not tempted to try to start converting them.

For drivers like this one where there is no room for the timestamp
to sit earlier for minimal channels I think it is worth that conversion
if we are touching them anyway. 

Jonathan


> 
> > 
> > ...
> >   
> >>  	struct iio_poll_func *pf = p;
> >> -	__be32 data[ADIS16550_MAX_SCAN_DATA];
> >> +	__be32 data[ADIS16550_MAX_SCAN_DATA] __aligned(8);
This one is more complex as you need to take the
available scan masks into account to figure out that it
always has enough channels enabled to ensure the timestamp
ends up in the 3rd 64 byte position.

We get 7 channels for each of the available scan masks.
So fine, but hard to see that, so this one I'd be less tempted
to change.


> >>  	struct iio_dev *indio_dev = pf->indio_dev;
> >>  	struct adis16550 *st = iio_priv(indio_dev);  
> > 
> > Ditto.
> >   
> 
>