[PATCH v2] iio: imu: inv_mpu6050: Use as standalone trigger

[PATCH v2] iio: imu: inv_mpu6050: Use as standalone trigger

[Linus Walleij]

It may happen that the MPU6050 is the only hardware
trigger available on your system such as this:

> lsiio
Device 003: hscdtd008a
Device 001: mpu6050
Device 002: gp2ap002
Device 000: ab8500-gpadc
Trigger 000: mpu6050-dev1

And when you want to use it to read periodically from
your magnetometer like this:

> iio_generic_buffer -a -c 100 -n hscdtd008a -t mpu6050-dev1

Then the following happens:

[  209.951334] Internal error: Oops: 5 [#1] SMP ARM
(...)
[  209.981969] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
[  209.988925] PC is at inv_scan_query_mpu6050+0x8/0xb8
[  209.993914] LR is at inv_mpu6050_set_enable+0x40/0x194

This is because since we are not using any channels from the
same device, the indio_dev->active_scan_mask is NULL.

Just checking for that and bailing out is however not enough:
we have to enable some kind of FIFO for the readout to work.
So enable the temperature as a dummy FIFO and all works
fine.

Fixes: 09a642b78523 ("Invensense MPU6050 Device Driver.")
Cc: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Add Fixes: tag, this is the initial commit but I do not
  think backporting makes sense, this is a non-regression
  non-critical fix.
- Fix a nit found by Andy (return mask value directly).
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index f7b5a70be30f..de8ed1446d60 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -11,6 +11,16 @@ static unsigned int inv_scan_query_mpu6050(struct iio_dev *indio_dev)
 	struct inv_mpu6050_state  *st = iio_priv(indio_dev);
 	unsigned int mask;
 
+	/*
+	 * If the MPU6050 is just used as a trigger, then the scan mask
+	 * is not allocated so we simply enable the temperature channel
+	 * as a dummy and bail out.
+	 */
+	if (!indio_dev->active_scan_mask) {
+		st->chip_config.temp_fifo_enable = true;
+		return INV_MPU6050_SENSOR_TEMP;
+	}
+
 	st->chip_config.gyro_fifo_enable =
 		test_bit(INV_MPU6050_SCAN_GYRO_X,
 			 indio_dev->active_scan_mask) ||
-- 
2.29.2

Re: [PATCH v2] iio: imu: inv_mpu6050: Use as standalone trigger

[Jean-Baptiste Maneyrol]

Hello,

looks good for me, thanks for the patch. Here is my ack.

Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>

We're lucky this is working without any sensor running, it was not obvious.
On which chip have you tested that?

Thanks,
JB


From: Linus Walleij <linus.walleij@linaro.org>
Sent: Monday, March 22, 2021 14:24
To: Jonathan Cameron <jic23@kernel.org>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
Cc: Hartmut Knaack <knaack.h@gmx.de>; Lars-Peter Clausen <lars@metafoo.de>; Peter Meerwald-Stadler <pmeerw@pmeerw.net>; Linus Walleij <linus.walleij@linaro.org>; Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
Subject: [PATCH v2] iio: imu: inv_mpu6050: Use as standalone trigger 
 
 CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.

It may happen that the MPU6050 is the only hardware
trigger available on your system such as this:

> lsiio
Device 003: hscdtd008a
Device 001: mpu6050
Device 002: gp2ap002
Device 000: ab8500-gpadc
Trigger 000: mpu6050-dev1

And when you want to use it to read periodically from
your magnetometer like this:

> iio_generic_buffer -a -c 100 -n hscdtd008a -t mpu6050-dev1

Then the following happens:

[  209.951334] Internal error: Oops: 5 [#1] SMP ARM
(...)
[  209.981969] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support)
[  209.988925] PC is at inv_scan_query_mpu6050+0x8/0xb8
[  209.993914] LR is at inv_mpu6050_set_enable+0x40/0x194

This is because since we are not using any channels from the
same device, the indio_dev->active_scan_mask is NULL.

Just checking for that and bailing out is however not enough:
we have to enable some kind of FIFO for the readout to work.
So enable the temperature as a dummy FIFO and all works
fine.

Fixes: 09a642b78523 ("Invensense MPU6050 Device Driver.")
Cc: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Add Fixes: tag, this is the initial commit but I do not
  think backporting makes sense, this is a non-regression
  non-critical fix.
- Fix a nit found by Andy (return mask value directly).
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index f7b5a70be30f..de8ed1446d60 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -11,6 +11,16 @@ static unsigned int inv_scan_query_mpu6050(struct iio_dev *indio_dev)
         struct inv_mpu6050_state  *st = iio_priv(indio_dev);
         unsigned int mask;
 
+       /*
+        * If the MPU6050 is just used as a trigger, then the scan mask
+        * is not allocated so we simply enable the temperature channel
+        * as a dummy and bail out.
+        */
+       if (!indio_dev->active_scan_mask) {
+               st->chip_config.temp_fifo_enable = true;
+               return INV_MPU6050_SENSOR_TEMP;
+       }
+
         st->chip_config.gyro_fifo_enable =
                 test_bit(INV_MPU6050_SCAN_GYRO_X,
                          indio_dev->active_scan_mask) ||
-- 
2.29.2

Re: [PATCH v2] iio: imu: inv_mpu6050: Use as standalone trigger

[Linus Walleij]

On Thu, Mar 25, 2021 at 6:08 PM Jean-Baptiste Maneyrol
<JManeyrol@invensense.com> wrote:

> Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
>
> We're lucky this is working without any sensor running, it was not obvious.
> On which chip have you tested that?

This was tested on the Samsung GT-I8190 (Galaxy S III mini)
mobile phone, I don't know which version of MPU6050 it is.

Yours,
Linus Walleij

Re: [PATCH v2] iio: imu: inv_mpu6050: Use as standalone trigger

[Jonathan Cameron]

On Thu, 25 Mar 2021 20:36:20 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Thu, Mar 25, 2021 at 6:08 PM Jean-Baptiste Maneyrol
> <JManeyrol@invensense.com> wrote:
> 
> > Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> >
> > We're lucky this is working without any sensor running, it was not obvious.
> > On which chip have you tested that?  
> 
> This was tested on the Samsung GT-I8190 (Galaxy S III mini)
> mobile phone, I don't know which version of MPU6050 it is.

Applied to the togreg branch to iio.git.  I added a note that it
wasn't suitable for backporting. More than possible that will get
missed though, so lets keep our eyes open for anyone picking it up.

Unlikely to do much harm if we do miss it though.

Jonathan

> 
> Yours,
> Linus Walleij