Re: [PATCH v6 3/7] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

[Geert Uytterhoeven]

Hi Andy,

On Wed, 25 Feb 2026 at 12:42, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxx> wrote:
> On Wed, Feb 25, 2026 at 11:17:11AM +0100, Francesco Lavra wrote:
> > The DRDY_MASK feature implemented in sensor chips marks gyroscope and
> > accelerometer invalid samples (i.e. samples that have been acquired during
> > the settling time of sensor filters) with the special values 0x7FFFh,
> > 0x7FFE, and 0x7FFD.
> > The driver checks FIFO samples against these special values in order to
> > discard invalid samples; however, it does the check regardless of the type
> > of samples being processed, whereas this feature is specific to gyroscope
> > and accelerometer data. This could cause valid samples to be discarded.
> >
> > Fix the above check so that it takes into account the type of samples being
> > processed. To avoid casting to __le16 * when checking sample values, clean
> > up the type representation for data read from the FIFO.
>
> ...
>
> > -     u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8);
> > +     struct {
> > +             union {
> > +                     __le16 data[3];
> > +                     __le32 fifo_ts;
> > +             };
> > +             aligned_s64 timestamp;
> > +     } iio_buff = { };
>
> Hmm... Have you considered m68k case? IIRC there the alignment is 2 (even for
> 64-bit members), so theoretically the beginning of the structure can be aligned
> to address 2. and hence the __le16 data[3] will have no gap to the following
> timestamp. In current code it's 64-bit and not 48-bit item.

Fortunately, the layout of a structure does not change because the
first member could be stored at a less-aligned address, as that would
make it incompatible with a different instance that is stored at a
differently-aligned address ;-)

The alignment of a structure is the maximum of the alignments of its
members.  So that will be 8 bytes, as the aligned_s64 definition has
an internal "__attribute__((aligned(8)))".

It would be wise to add explicit padding ("u16 pad") just before
timestamp though, for documentation purpose.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds