lkml.org 
[lkml]   [2022]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 2/3] iio: add MEMSensing MSA311 3-axis accelerometer driver
On Fri, 1 Jul 2022 13:49:10 +0000
Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:

> Hello Jonathan,
>
> Sorry for the delayed response.
>
> On Sun, Jun 19, 2022 at 01:27:03PM +0100, Jonathan Cameron wrote:
> > On Thu, 16 Jun 2022 10:42:14 +0000
> > Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:
> >
> > > MSA311 is a tri-axial, low-g accelerometer with I2C digital output for
> > > sensitivity consumer applications. It has dynamical user selectable full
> > > scales range of +-2g/+-4g/+-8g/+-16g and allows acceleration measurements
> > > with output data rates from 1Hz to 1000Hz.
> > >
> > > Datasheet can be found at following URL:
> > > https://cdn-shop.adafruit.com/product-files/5309/MSA311-V1.1-ENG.pdf
> > >
> > > This driver supports following MSA311 features:
> > > - IIO interface
> > > - Different power modes: NORMAL and SUSPEND (using pm_runtime)
> > > - ODR (Output Data Rate) selection
> > > - Scale and samp_freq selection
> > > - IIO triggered buffer, IIO reg access
> > > - NEW_DATA interrupt + trigger
> > >
> > > Below features to be done:
> > > - Motion Events: ACTIVE, TAP, ORIENT, FREEFALL
> > > - Low Power mode
> > >
> > > Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> > Hi Dmitry,
> >
> > A few things I missed before + I'm still not happy with the runtime
> > pm handling. One case that isn't covered well is !CONFIG_RUNTIME_PM
> >
> > Thanks,
> >
> > Jonathan
> >
>
> ...
>
> > > +static irqreturn_t msa311_buffer_thread(int irq, void *p)
> > > +{
> > > + struct iio_poll_func *pf = p;
> > > + struct iio_dev *indio_dev = pf->indio_dev;
> > > + struct msa311_priv *msa311 = iio_priv(indio_dev);
> > > + struct device *dev = &msa311->i2c->dev;
> > > + const struct iio_chan_spec *chan;
> > > + __le16 axis;
> > > + int bit = 0, err, i = 0;
> > > +
> > > + /* Ensure correct alignment of time stamp when present */
> > > + struct {
> > > + __le16 channels[MSA311_SI_Z + 1];
> > > + s64 ts __aligned(8);
> > > + } buf;
> > > +
> > > + memset(&buf, 0, sizeof(buf));
> > > +
> > > + mutex_lock(&msa311->lock);
> > > +
> > > + for_each_set_bit(bit, indio_dev->active_scan_mask,
> > > + indio_dev->masklength) {
> > > + chan = &msa311_channels[bit];
> >
> > Nothing to do with your driver, but feels like it's worth
> > exploring a
> > for_each_chan_in_iio_scan(struct iio_chan_spec, struct iio_dev) macro.
> >
> > I'll add that to my todo list.
> >
>
> If you don't mind, I can prepare such a patch.

I had a look at this whilst travelling and it's a lot more complex than I
thought it would be because of gaps in the scan_index in some drivers (not
all channels have scan indexes and not all scan indexes are used)

If we write such a thing we need to resolve that in the core and I suspect
it will require creation of an indirection structure that lets us
do scan_index based look up of channels. Whilst that works in many drivers
because there is a nice 1 to 1 mapping, there are exceptions.
Hence I think we would be looking at:

1) Check at registration time on whether scan_index == location in
iio_dev->channels, if so set another pointer say iio_dev->channels_linear =
iio_dev->channels.
2) If not, create a lookup table and make iio_dev->channels_linear
point to that.
3) Finally introduce a macro that uses channels_linear.

What fun ;)

Jonathan



\
 
 \ /
  Last update: 2022-07-16 17:44    [W:0.063 / U:0.680 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site