| Message ID | 20190624145651.24320-1-jmaneyrol@invensense.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: imu: mpu6050: add available scan masks | expand |
On Mon, 24 Jun 2019 14:57:24 +0000 Jean-Baptiste Maneyrol <JManeyrol@invensense.com> wrote: > Only allow 3-axis gyro and/or 3-axis accel. > For icm20602, temp data is mandatory for all conf. > > Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> Hi Jean-Baptiste, Is this policy or a hardware requirement, and hence a fix for the driver? If it's a fix, please make that clearer and add a fixes tag. Thanks, Jonathan > --- > drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > index 385f14a4d5a7..bfd6d093e54d 100644 > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > @@ -851,6 +851,13 @@ static const struct iio_chan_spec inv_mpu_channels[] = { > INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z), > }; > > +static const unsigned long inv_mpu_scan_masks[] = { > + 0x07, /* 3-axis accel */ > + 0x38, /* 3-axis gyro */ > + 0x3F, /* 6-axis accel + gyro */ > + 0, > +}; > + > static const struct iio_chan_spec inv_icm20602_channels[] = { > IIO_CHAN_SOFT_TIMESTAMP(INV_ICM20602_SCAN_TIMESTAMP), > { > @@ -877,6 +884,13 @@ static const struct iio_chan_spec inv_icm20602_channels[] = { > INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_ICM20602_SCAN_ACCL_Z), > }; > > +static const unsigned long inv_icm20602_scan_masks[] = { > + 0x0F, /* 3-axis accel + temp (mandatory) */ > + 0x78, /* 3-axis gyro + temp (mandatory) */ > + 0x7F, /* 6-axis accel + gyro + temp (mandatory) */ > + 0, > +}; > + > /* > * The user can choose any frequency between INV_MPU6050_MIN_FIFO_RATE and > * INV_MPU6050_MAX_FIFO_RATE, but only these frequencies are matched by the > @@ -1136,9 +1150,11 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name, > if (chip_type == INV_ICM20602) { > indio_dev->channels = inv_icm20602_channels; > indio_dev->num_channels = ARRAY_SIZE(inv_icm20602_channels); > + indio_dev->available_scan_masks = inv_icm20602_scan_masks; > } else { > indio_dev->channels = inv_mpu_channels; > indio_dev->num_channels = ARRAY_SIZE(inv_mpu_channels); > + indio_dev->available_scan_masks = inv_mpu_scan_masks; > } > > indio_dev->info = &mpu_info;
Hi Jonathan, this is indeed a fix. Without it you cannot have a single axis buffer configuration working, or a buffer without temp for icm20602. Since 3-axis configuration is always used, it was not spotted before. The main issue is for icm20602 where it is making gyro only configuration a mess, mixing temp and gyro data together. I am sending a new patch more readable (using the scan defines rather than a constant for the masks) and add a fixes tag referring to the icm20602 fifo support. Best regards, JB From: linux-iio-owner@vger.kernel.org <linux-iio-owner@vger.kernel.org> on behalf of Jonathan Cameron <jic23@jic23.retrosnub.co.uk> Sent: Wednesday, June 26, 2019 21:36 To: Jean-Baptiste Maneyrol Cc: linux-iio@vger.kernel.org Subject: Re: [PATCH] iio: imu: mpu6050: add available scan masks 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 Mon, 24 Jun 2019 14:57:24 +0000 Jean-Baptiste Maneyrol <JManeyrol@invensense.com> wrote: > Only allow 3-axis gyro and/or 3-axis accel. > For icm20602, temp data is mandatory for all conf. > > Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> Hi Jean-Baptiste, Is this policy or a hardware requirement, and hence a fix for the driver? If it's a fix, please make that clearer and add a fixes tag. Thanks, Jonathan > --- > drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > index 385f14a4d5a7..bfd6d093e54d 100644 > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > @@ -851,6 +851,13 @@ static const struct iio_chan_spec inv_mpu_channels[] = { > INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z), > }; > > +static const unsigned long inv_mpu_scan_masks[] = { > + 0x07, /* 3-axis accel */ > + 0x38, /* 3-axis gyro */ > + 0x3F, /* 6-axis accel + gyro */ > + 0, > +}; > + > static const struct iio_chan_spec inv_icm20602_channels[] = { > IIO_CHAN_SOFT_TIMESTAMP(INV_ICM20602_SCAN_TIMESTAMP), > { > @@ -877,6 +884,13 @@ static const struct iio_chan_spec inv_icm20602_channels[] = { > INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_ICM20602_SCAN_ACCL_Z), > }; > > +static const unsigned long inv_icm20602_scan_masks[] = { > + 0x0F, /* 3-axis accel + temp (mandatory) */ > + 0x78, /* 3-axis gyro + temp (mandatory) */ > + 0x7F, /* 6-axis accel + gyro + temp (mandatory) */ > + 0, > +}; > + > /* > * The user can choose any frequency between INV_MPU6050_MIN_FIFO_RATE and > * INV_MPU6050_MAX_FIFO_RATE, but only these frequencies are matched by the > @@ -1136,9 +1150,11 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name, > if (chip_type == INV_ICM20602) { > indio_dev->channels = inv_icm20602_channels; > indio_dev->num_channels = ARRAY_SIZE(inv_icm20602_channels); > + indio_dev->available_scan_masks = inv_icm20602_scan_masks; > } else { > indio_dev->channels = inv_mpu_channels; > indio_dev->num_channels = ARRAY_SIZE(inv_mpu_channels); > + indio_dev->available_scan_masks = inv_mpu_scan_masks; > } > > indio_dev->info = &mpu_info;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c index 385f14a4d5a7..bfd6d093e54d 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c @@ -851,6 +851,13 @@ static const struct iio_chan_spec inv_mpu_channels[] = { INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_MPU6050_SCAN_ACCL_Z), }; +static const unsigned long inv_mpu_scan_masks[] = { + 0x07, /* 3-axis accel */ + 0x38, /* 3-axis gyro */ + 0x3F, /* 6-axis accel + gyro */ + 0, +}; + static const struct iio_chan_spec inv_icm20602_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(INV_ICM20602_SCAN_TIMESTAMP), { @@ -877,6 +884,13 @@ static const struct iio_chan_spec inv_icm20602_channels[] = { INV_MPU6050_CHAN(IIO_ACCEL, IIO_MOD_Z, INV_ICM20602_SCAN_ACCL_Z), }; +static const unsigned long inv_icm20602_scan_masks[] = { + 0x0F, /* 3-axis accel + temp (mandatory) */ + 0x78, /* 3-axis gyro + temp (mandatory) */ + 0x7F, /* 6-axis accel + gyro + temp (mandatory) */ + 0, +}; + /* * The user can choose any frequency between INV_MPU6050_MIN_FIFO_RATE and * INV_MPU6050_MAX_FIFO_RATE, but only these frequencies are matched by the @@ -1136,9 +1150,11 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name, if (chip_type == INV_ICM20602) { indio_dev->channels = inv_icm20602_channels; indio_dev->num_channels = ARRAY_SIZE(inv_icm20602_channels); + indio_dev->available_scan_masks = inv_icm20602_scan_masks; } else { indio_dev->channels = inv_mpu_channels; indio_dev->num_channels = ARRAY_SIZE(inv_mpu_channels); + indio_dev->available_scan_masks = inv_mpu_scan_masks; } indio_dev->info = &mpu_info;
Only allow 3-axis gyro and/or 3-axis accel. For icm20602, temp data is mandatory for all conf. Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> --- drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)