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
| ||
|
Date: Mon, 18 Jul 2022 10:01:36 +0000 From: Dmitry Rokosov <DDRokosov@...rdevices.ru> To: Jonathan Cameron <jic23@...nel.org> CC: "robh+dt@...nel.org" <robh+dt@...nel.org>, "stano.jakubek@...il.com" <stano.jakubek@...il.com>, "shawnguo@...nel.org" <shawnguo@...nel.org>, "lars@...afoo.de" <lars@...afoo.de>, "andy.shevchenko@...il.com" <andy.shevchenko@...il.com>, "stephan@...hold.net" <stephan@...hold.net>, "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, kernel <kernel@...rdevices.ru>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v3 2/3] iio: add MEMSensing MSA311 3-axis accelerometer driver Jonathan, On Sat, Jul 16, 2022 at 04:51:21PM +0100, Jonathan Cameron wrote: > On Fri, 1 Jul 2022 13:49:10 +0000 > Dmitry Rokosov <DDRokosov@...rdevices.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@...rdevices.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@...rdevices.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 > Okay, I'll try looking for proper solution and prepare RFC patch. -- Thank you, Dmitry
Powered by blists - more mailing lists