Re: LSM9DS1 testing with st_lsm6dsx driver

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4897 bytes --]

> > > > LSM9DS1 does not support hw FIFO for the moment. Are you trying to enable
> > > > buffered reading?
> > >
> > > I am not manually interacting with the device at all when this
> > > exception occurs. This happens during the driver probe.
> > >
> > > > Could you please try if the following patch helps? (just compiled)
> > >
> > > I no longer receive the exception with this patch and it makes sense, thanks.
> >
> > Hi Bobby,
> >
> > thx a lot for testing. Could you please try to drop the previous patch and
> > apply the following one? Does it fix the issue as well?
> 
> No problem, happy to help. I just tested, and unfortunately the issue
> is still present with this patch.

re-looking at the code this patch will actually does not anything since in the
current implementation st_lsm6dsx_handler_irq will return IRQ_NONE and the
thread handler will not be run. Anyway since st_lsm6dsx_handler_irq will be
removed soon I will respin this patch ontop of Sean series.

> I gave the datasheet and the hardware reference manual of the
> connected CPU a closer look and suspected a problem with my pin
> config. However even after various combinations of pull ups/downs and
> IRQ_TYPE_LEVEL_LOW/IRQ_TYPE_LEVEL_HIGH the same exception occurs, so I
> don't think that there's an issue with pin config stopping the
> interrupt line from being deasserted.

are you able to monitor the line activity through an oscilloscope?
The issue is the irq line is never asserted and the kernel complains about
lot of interrupts not managed

Looking at the datasheet, LSM9DS1 does not support IRQ_TYPE_LEVEL_LOW.
ST_LSM6DSX_REG_HLACTIVE_ADDR and ST_LSM6DSX_REG_PP_OD_ADDR registers are
actually not defined for LSM9DS1 and I will move them in hw_settings map.

@Jonathan: do you prefer this patch to be ontop of Sean's series?

Regards,
Lorenzo

> 
> >
> > iio: imu: st_lsm6dsx: check read_fifo pointer in st_lsm6dsx_handler_thread
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > index b0f3da1976e4..f4fd4842bd79 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > @@ -666,6 +666,9 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
> >         struct st_lsm6dsx_hw *hw = private;
> >         int count;
> >
> > +       if (!hw->settings->fifo_ops.read_fifo)
> > +               return IRQ_NONE;
> > +
> >         mutex_lock(&hw->fifo_lock);
> >         count = hw->settings->fifo_ops.read_fifo(hw);
> >         mutex_unlock(&hw->fifo_lock);
> > --
> > 2.21.0
> >
> > >
> > > For context I'm working with a board that has every data ready and
> > > interrupt signal connected to the LSM9DS1. Could you clarify what the
> > > proper usage of the "st,drdy-int-pin" would be in this case and
> > > whether or not I need more than one interrupt called out in my device
> > > tree node?
> > > I'm not really understanding how they're currently being utilized for
> > > this device in the driver.
> >
> > For the moment irq line in lsm9ds1 (acc/gyro) is not used at all,
> > so you can omit the "st,drdy-int-pin" property
> >
> > >
> > > Also, I know support for this device was added recently and the combo
> > > device hardware FIFO is complex, but is support for this something
> > > that's currently being worked on?
> >
> > It is actually in my ToDo list but I have no this device at the moment, so
> > patches are welcome :)
> >
> > Regards,
> > Lorenzo
> >
> > >
> > > Thanks,
> > > Bobby Jones
> > >
> > > > Regards,
> > > > Lorenzo
> > > >
> > > > iio: imu: st_lsm6dsx: do not configure the fifo if not supported
> > > >
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > index b65a6ca775e0..90a0e5ce44e5 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > @@ -1572,7 +1572,7 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
> > > >                         return err;
> > > >         }
> > > >
> > > > -       if (hw->irq > 0) {
> > > > +       if (hw->irq > 0 && hw->settings->fifo_ops.update_fifo) {
> > > >                 err = st_lsm6dsx_fifo_setup(hw);
> > > >                 if (err < 0)
> > > >                         return err;
> > > > --
> > > > 2.21.0
> > > >
> > > >

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]