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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250417184429.00002403@huawei.com>
Date: Thu, 17 Apr 2025 18:44:29 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: David Lechner <dlechner@...libre.com>
CC: Andy Shevchenko <andy@...nel.org>, Jonathan Cameron <jic23@...nel.org>,
	Nuno Sá <nuno.sa@...log.com>, Matthias Brugger
	<matthias.bgg@...il.com>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>, Lars-Peter Clausen
	<lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, "Cosmin
 Tanislav" <cosmin.tanislav@...log.com>, Tomasz Duszynski
	<tduszyns@...il.com>, Jean-Baptiste Maneyrol
	<jean-baptiste.maneyrol@....com>, Andreas Klinger <ak@...klinger.de>, "Petre
 Rodan" <petre.rodan@...dimension.ro>, <linux-iio@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH 6/8] iio: imu: adis16550: align buffers for timestamp

On Thu, 17 Apr 2025 12:07:37 -0500
David Lechner <dlechner@...libre.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.
> >   
> 
>