linux-kernel - Re: [PATCH 3/3] iio: imu: Add scale and sampling frequency to BMI270 IMU

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20241013164000.19087833@jic23-huawei>
Date: Sun, 13 Oct 2024 16:40:00 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Justin Weiss <justin@...tinweiss.com>
Cc: Alex Lanzano <lanzano.alex@...il.com>, Lars-Peter Clausen
 <lars@...afoo.de>, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 "Derek J . Clark" <derekjohn.clark@...il.com>, Philip Müller <philm@...jaro.org>
Subject: Re: [PATCH 3/3] iio: imu: Add scale and sampling frequency to
 BMI270 IMU

On Sat, 12 Oct 2024 19:45:18 -0700
Justin Weiss <justin@...tinweiss.com> wrote:

> Jonathan Cameron <jic23@...nel.org> writes:
> 
> > On Fri, 11 Oct 2024 08:37:49 -0700
> > Justin Weiss <justin@...tinweiss.com> wrote:
> >  
> >> Add read and write functions and create _available entries. Use
> >> IIO_CHAN_INFO_SAMP_FREQ instead of IIO_CHAN_INFO_FREQUENCY to match
> >> the BMI160 / BMI323 drivers.  
> >
> > Ah.  Please break dropping _FREQUENCY change out as a separate fix
> > with fixes tag etc and drag it to start of the patch. It was never
> > wired to anything anyway
> >
> > That's a straight forward ABI bug so we want that to land ahead
> > of the rest of the series.  
> 
> Thanks, I'll pull that into its own change and make it the first patch.
> 
> > Does this device have a data ready interrupt and if so what affect
> > do the different ODRs for each type of sensor have on that?
> > If there are separate data ready signals, you probably want to 
> > go with a dual buffer setup from the start as it is hard to unwind
> > that later.  
> 
> It has data ready interrupts for both accelerometer and gyroscope and a
> FIFO interrupt. I had held off on interrupts to keep this change
> simpler, but if it's a better idea to get it in earlier, I can add it
> alongside the triggered buffer change.

Ok. So the challenge is that IIO buffers are only described by external
metadata.  We don't carry tags within them.  Hence if you are using
either effectively separate datastreams (the two data ready interrupts)
or a fifo that is tagged data (how this difference of speed is normally handled
if it's one buffer) then when we push them into IIO buffers, they have
to go into separate buffers.

In older drivers this was done via the heavy weight option of registering
two separate IIO devices. Today we have the ability to support multiple buffers
in one driver. I'm not sure we've yet used it for this case, so I think
there may still be some gaps around triggering that will matter for the
separate dataready interrupt case (fifo is fine as no trigger involved).
Looking again at that code, it looks like there may need to be quite
a bit more work to cover this case proeprly.

We may be able to have a migration path from the simple case you have
(where timing is an external trigger) to multiple buffers.
It would involve:
1) Initial solution where the frequencies must match if the fifo is in use.
   Non fifo trigger from data ready might work but we'd need to figure out
   if they run in close enough timing.
2) Solution where we add a second buffer and if the channels are enabled
   in that we can allow separate timing for the two sensor types.

This is one of those hardware features that seems like a good idea
from the hardware design point of view but assumes a very specific
sort of software model :(

Jonathan