> On Fri, 28 Oct 2022 13:23:42 +0200
> Lorenzo Bianconi <lorenzo@xxxxxxxxxx> wrote:
>
> > There are some hw configuration where irq0 and/or irq1 pins are not
> > connected to the SPI or I2C/I3C controller.
>
> The might be connected to lots of other places on the application processor
> (doesn't really matter though - I think your meaning is clear enough!)
>
> > In order to avoid polling
> > the output register introduce iio-sw trigger support when irq line is
> > not available (or hw FIFO is not supported).
> >
> > Suggested-by: Mario Tesi <mario.tesi@xxxxxx>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
>
> One comment / question inline. Otherwise looks good to me.
>
> Jonathan
>
>
> > ---
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 3 +-
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 78 ++++++++++++++++++++
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 4 +-
> > 3 files changed, 81 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > index 07ad8027de73..6399b0bb6f67 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > @@ -424,7 +424,7 @@ struct st_lsm6dsx_hw {
> > struct {
> > __le16 channels[3];
> > s64 ts __aligned(8);
> > - } scan[3];
> > + } scan[ST_LSM6DSX_ID_MAX];
> > };
> >
>
> > static inline int
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > index fe5fa08b68ac..73fd5f038375 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > @@ -53,6 +53,8 @@
> > #include <linux/iio/events.h>
> > #include <linux/iio/iio.h>
> > #include <linux/iio/sysfs.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger_consumer.h>
> > #include <linux/interrupt.h>
> > #include <linux/irq.h>
> > #include <linux/minmax.h>
> > @@ -2117,6 +2119,32 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
> > return fifo_len || event ? IRQ_HANDLED : IRQ_NONE;
> > }
> >
> > +static irqreturn_t st_lsm6dsx_sw_trigger_handler_thread(int irq,
> > + void *private)
> > +{
> > + struct iio_poll_func *pf = private;
> > + struct iio_dev *iio_dev = pf->indio_dev;
> > + struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> > + struct st_lsm6dsx_hw *hw = sensor->hw;
> > +
> > + if (sensor->id == ST_LSM6DSX_ID_EXT0 ||
> > + sensor->id == ST_LSM6DSX_ID_EXT1 ||
> > + sensor->id == ST_LSM6DSX_ID_EXT2)
> > + st_lsm6dsx_shub_read_output(hw,
> > + (u8 *)hw->scan[sensor->id].channels,
> > + sizeof(hw->scan[sensor->id].channels));
>
> Are we guaranteed this particular size of readback? I'm guessing a bit
> as it's been a long time since I looked at this driver in detail, but could
> we have sensors with either a different number of axes or different number
> of registers per axis?
>
> It might be neater to have two handlers, one for the EXTN cases and one
> for the main sensors. That would push this conditional down to the
> point of registration. I'm not sure it's worth it however so up to you...
Hi Jonathan,
so far we support just magnetometers on sensor-hub (LIS2MDL and LIS3MDL).
Both LIS2MDL and LIS3MDL have 3 axis, each of them is le16, so it is fine as it
is for the moment. Do you prefer to be more generic and take into account new
possible sensors? I am not sure when they will arrive :)
Regards,
Lorenzo
>
>
> > + else
> > + st_lsm6dsx_read_locked(hw, iio_dev->channels[0].address,
> > + hw->scan[sensor->id].channels,
> > + sizeof(hw->scan[sensor->id].channels));
> > +
> > + iio_push_to_buffers_with_timestamp(iio_dev, &hw->scan[sensor->id],
> > + iio_get_time_ns(iio_dev));
> > + iio_trigger_notify_done(iio_dev->trig);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > static int st_lsm6dsx_irq_setup(struct st_lsm6dsx_hw *hw)
> > {
> > struct st_sensors_platform_data *pdata;
> > @@ -2175,6 +2203,46 @@ static int st_lsm6dsx_irq_setup(struct st_lsm6dsx_hw *hw)
> > return 0;
> > }
>
> > +static int st_lsm6dsx_sw_buffers_setup(struct st_lsm6dsx_hw *hw)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> > + int err;
> > +
> > + if (!hw->iio_devs[i])
> > + continue;
> > +
> > + err = devm_iio_triggered_buffer_setup(hw->dev,
> > + hw->iio_devs[i], NULL,
> > + st_lsm6dsx_sw_trigger_handler_thread,
> > + &st_lsm6dsx_sw_buffer_ops);
> > + if (err)
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> ...
>
Attachment:
signature.asc
Description: PGP signature
