On Thu, 13 Aug 2020 08:04:06 +0000
Jean-Baptiste Maneyrol <jmaney...@invensense.com> wrote:
> Hello,
>
> this is a case that should never be happening since available scan mask only
> advertise Accel + Temp, Gyro + Temp, or Accel + Gyro + Temp.
> More than that, temperature sensor is not working when MEMS engine is off.
> (it's only purpose it to measure temperature of the MEMS to do data
> temperature compensation).
>
> So I think we can let this check as it is currently, since this is not a
> supported configuration.
>
Perhaps a good option would be to add a suitably positioned comment to make
this clear so we don't get further patches 'fixing' this in the future?
Thanks,
Jonathan
> Best regards,
> JB
>
>
> From: Andy Shevchenko <andy.shevche...@gmail.com>
> Sent: Monday, August 10, 2020 10:02
> To: t...@redhat.com <t...@redhat.com>
> Cc: Jonathan Cameron <ji...@kernel.org>; Hartmut Knaack <knaac...@gmx.de>;
> Lars-Peter Clausen <l...@metafoo.de>; Peter Meerwald <pme...@pmeerw.net>;
> Jean-Baptiste Maneyrol <jmaney...@invensense.com>; Michał Mirosław
> <mirq-li...@rere.qmqm.pl>; Lee Jones <lee.jo...@linaro.org>; linux-iio
> <linux-...@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] iio: imu: inv_mpu6050: check for temp_fifo_enable
>
> 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.
>
> On Sun, Aug 9, 2020 at 7:00 PM <t...@redhat.com> wrote:
> >
> > From: Tom Rix <t...@redhat.com>
> >
> > clang static analysis reports this problem
> >
> > inv_mpu_ring.c:181:18: warning: Division by zero
> > nb = fifo_count / bytes_per_datum;
> > ~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> >
> > This is a false positive.
> > Dividing by 0 is protected by this check
> >
> > if (!(st->chip_config.accl_fifo_enable |
> > st->chip_config.gyro_fifo_enable |
> > st->chip_config.magn_fifo_enable))
> > goto end_session;
> > bytes_per_datum = 0;
> >
> > But there is another fifo, temp_fifo
> >
> > if (st->chip_config.temp_fifo_enable)
> > bytes_per_datum += INV_MPU6050_BYTES_PER_TEMP_SENSOR;
> >
> > Which would be skipped if it was the only enabled fifo.
> > So add to the check.
> >
>
> > Fixes: 2e4c0a5e2576 ("iio: imu: inv_mpu6050: add fifo temperature data
> > support")
> >
> > Signed-off-by: Tom Rix <t...@redhat.com>
>
> There shouldn't be a blank line in between.
>
> Other than that,
> Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com>
>
>
>
> > ---
> > drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > index b533fa2dad0a..5240a400dcb4 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > @@ -141,6 +141,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
> >
> > if (!(st->chip_config.accl_fifo_enable |
> > st->chip_config.gyro_fifo_enable |
> > + st->chip_config.temp_fifo_enable |
> > st->chip_config.magn_fifo_enable))
> > goto end_session;
> > bytes_per_datum = 0;
> > --
> > 2.18.1
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko
