[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260116180341.6c0556cd@jic23-huawei>
Date: Fri, 16 Jan 2026 18:03:41 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Francesco Lavra <flavra@...libre.com>
Cc: Lorenzo Bianconi <lorenzo@...nel.org>, David Lechner
<dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>, Andy
Shevchenko <andy@...nel.org>, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] iio: imu: st_lsm6dsx: set buffer sampling frequency
for accelerometer only
On Mon, 12 Jan 2026 18:10:32 +0100
Francesco Lavra <flavra@...libre.com> wrote:
> On Sun, 2026-01-11 at 16:18 +0000, Jonathan Cameron wrote:
> > On Fri, 9 Jan 2026 19:15:26 +0100
> > Francesco Lavra <flavra@...libre.com> wrote:
> >
> > > The st_lsm6dsx_hwfifo_odr_store() function, which is called when
> > > userspace
> > > writes the buffer sampling frequency sysfs attribute, calls
> > > st_lsm6dsx_check_odr(), which accesses the odr_table array at index
> > > `sensor->id`; since this array is only 2 entries long, an access for
> > > any
> > > sensor type other than accelerometer or gyroscope is an out-of-bounds
> > > access.
> > >
> > > To prevent userspace from triggering an out-of-bounds array access, and
> > > to
> > > support the only use case for which FIFO sampling frequency values
> > > different from the sensor sampling frequency may be needed (which is
> > > for
> > > keeping FIFO data rate low while sampling acceleration data at high
> > > rates
> > > for accurate event detection), do not create the buffer sampling
> > > frequency
> > > attribute for sensor types other than the accelerometer.
> >
> > I'm not following why we need to drop this attribute for the gyroscope.
> > Perhaps lay out what the combinations of controls are and the attributes
> > we end up with.
>
> It's not like we need to drop this attribute, it's just that I don't see a
> need for it. The only reason I added this attribute was to be able to
> control (e.g. lower) the rate of data coming from the sensor while
> maintaining a high accuracy for event detection; and accurate event
> detection requires a high sampling rate for the accelerometer.
Ok. So key here is for accelerations we are looking at impacts as a typical
use case, whereas gyroscope tends to be slow orientation change stuff.
That sounds a bit usecase specific. If someone is using these to detect shaft rotation
issues they are going to care about sampling rates on the gyro as well,
or is there something inherent in the gyroscope events (i.e. maybe there
aren't any gyro events?) that makes this not relevant?
> So the
> gyroscope is not involved here, and the attribute is only needed for the
> accelerometer.
>
> Before this change, we have:
> - accel IIO device with separate samp_freq and buffer/samp_freq
> - gyro IIO device with separate samp_freq and buffer/samp_freq
> - (optionally) external sensor IIO devices with separate samp_freq and
> buffer/samp_freq (and trying to set buffer/samp_freq for these triggers an
> out-of-bounds array access)
>
> After this change, we have the accel IIO device with separate samp_freq and
> buffer/samp_freq, while the other IIO devices have only a single samp_freq
> attribute.
>
> > As you note in the cover letter we can change this now with ABI issues as
> > it is just in my tree, so I don't mind the change, just want to
> > understand
> > it a little better than I currently do!
>
> It's not just in your tree, it has been pulled into Linus's tree for 6.19.
Bit tight to get a fix in, though I gather we are going to rc8 this time
so maybe.
Jonathan
>
Powered by blists - more mailing lists