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

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 19 Mar 2021 14:33:57 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> 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.

Ah. This corner case. I have a suspicion a lot of drivers
suffer from this case.  I confess I'm often too lazy to
look at it in a great deal of depth because it's not a particularly
common thing to do - and we long ago dropped the equivalent
triggers that just used a periodic interrupt on the basis that
they didn't add any significant improvement in precision of
capture over an hrtimer based trigger.

Mind you we should definitely close down the bug either way.

Jonathan

> 
> Cc: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 11 +++++++++++
>  1 file changed, 11 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..6946d50b874a 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -11,6 +11,17 @@ 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;
> +		mask = INV_MPU6050_SENSOR_TEMP;
> +		return mask;
> +	}
> +
>  	st->chip_config.gyro_fifo_enable =
>  		test_bit(INV_MPU6050_SCAN_GYRO_X,
>  			 indio_dev->active_scan_mask) ||