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

[Justin Weiss <justin@justinweiss.com>]

Jonathan Cameron <jic23@kernel.org> writes:

> On Sat, 12 Oct 2024 19:45:18 -0700
> Justin Weiss <justin@justinweiss.com> wrote:
>
>> Jonathan Cameron <jic23@kernel.org> writes:
>> 
>> > On Fri, 11 Oct 2024 08:37:49 -0700
>> > Justin Weiss <justin@justinweiss.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

Hm, that does sound tricky. If there's an example I can follow, I can
make an attempt at it. Otherwise, if there's a change I can make now
that would help with migrating in the future, I can do that instead.

Of the devices I've looked at, only one has had the interrupts usable
and that one only had a single pin available. So if this change doesn't
make it harder to add later if it's necessary, I would still be OK going
without full support for now.

Justin