Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1391909
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 6/7] iio: inv_mpu6050: Check channel configuration on preenable |
| Date | 2016-05-01 23:30 +0200 |
| Message-ID | <ru76c-86o-57@gated-at.bofh.it> (permalink) |
| References | <rtlXz-2Jp-1@gated-at.bofh.it> <rtlXA-2Jp-11@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 29/04/16 20:02, Crestez Dan Leonard wrote:
> Right now it is possible to only enable some of the x/y/z channels, for
> example you can enable accel_z without x or y. If you actually do that
> what you get is actually only the x channel.
>
> In theory the device supports selecting gyro x/y/z channels
> individually. It would also be possible to selectively enable x/y/z
> accel by unpacking the data read from the hardware into a format the iio
> core accepts.
>
> It is easier to simply refuse incorrect configuration.
Or see suggestion inline. This isn't an uncommon problem!
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 2 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 4 ++++
> drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 31 ++++++++++++++++++++++++++++++
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 064fc07..712e901 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -1130,7 +1130,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> result = iio_triggered_buffer_setup(indio_dev,
> inv_mpu6050_irq_handler,
> inv_mpu6050_read_fifo,
> - NULL);
> + &inv_mpu_buffer_ops);
> if (result) {
> dev_err(dev, "configure buffer fail %d\n", result);
> return result;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 9d15633..9d406df 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -284,6 +284,9 @@ enum inv_mpu6050_scan {
> INV_MPU6050_SCAN_TIMESTAMP,
> };
>
> +#define INV_MPU6050_SCAN_MASK_ACCEL 0x07
> +#define INV_MPU6050_SCAN_MASK_GYRO 0x38
> +
> enum inv_mpu6050_filter_e {
> INV_MPU6050_FILTER_256HZ_NOLPF2 = 0,
> INV_MPU6050_FILTER_188HZ,
> @@ -340,3 +343,4 @@ int inv_mpu_core_remove(struct device *dev);
> int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on);
> extern const struct dev_pm_ops inv_mpu_pmops;
> extern const struct regmap_config inv_mpu_regmap_config;
> +extern const struct iio_buffer_setup_ops inv_mpu_buffer_ops;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 56ee1e2..e8bda7f 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -222,3 +222,34 @@ flush_fifo:
>
> return IRQ_HANDLED;
> }
> +
> +/* Validate channels are set in a correct configuration */
> +static int inv_mpu_buffer_preenable(struct iio_dev *indio_dev)
> +{
This should not be in the preenable. It's perfectly possible to know that mode was invalid
earlier than this. Use the core demux to handle this case by providing
available_scanmasks. The the core will handle demuxing the data stream if needed to
provide the channels enabled only in the kfifo.
Not sure how we failed to pick up on this one before! Kind of an impressively major bug
to have hiding in there. Ah well - I guess most users always want everything!
Jonathan
> + struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + unsigned long mask = *indio_dev->active_scan_mask;
> +
> + if ((mask & INV_MPU6050_SCAN_MASK_GYRO) &&
> + (mask & INV_MPU6050_SCAN_MASK_GYRO) != INV_MPU6050_SCAN_MASK_GYRO)
> + {
> + dev_warn(regmap_get_device(st->map),
> + "Gyro channels can only be enabled together\n");
> + return -EINVAL;
> + }
> +
> + if ((mask & INV_MPU6050_SCAN_MASK_ACCEL) &&
> + (mask & INV_MPU6050_SCAN_MASK_ACCEL) != INV_MPU6050_SCAN_MASK_ACCEL)
> + {
> + dev_warn(regmap_get_device(st->map),
> + "Accel channels can only be enabled together\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +const struct iio_buffer_setup_ops inv_mpu_buffer_ops = {
> + .preenable = inv_mpu_buffer_preenable,
> + .postenable = iio_triggered_buffer_postenable,
> + .predisable = iio_triggered_buffer_predisable,
> +};
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[RFC 0/7] iio: inv_mpu6050: Support i2c master and external readings Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-29 21:10 +0200
[RFC 5/7] iio: inv_mpu6050: Add support for auxiliary I2C master Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-29 21:10 +0200
Re: [RFC 5/7] iio: inv_mpu6050: Add support for auxiliary I2C master Jonathan Cameron <jic23@kernel.org> - 2016-05-01 23:40 +0200
Re: [RFC 5/7] iio: inv_mpu6050: Add support for auxiliary I2C master Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-05-05 14:40 +0200
Re: [RFC 5/7] iio: inv_mpu6050: Add support for auxiliary I2C master Rob Herring <robh+dt@kernel.org> - 2016-05-05 15:20 +0200
Re: [RFC 5/7] iio: inv_mpu6050: Add support for auxiliary I2C master Peter Rosin <peda@axentia.se> - 2016-05-02 17:40 +0200
[PATCH 4/7] iio: inv_mpu6050: Cache non-volatile bits of user_ctrl Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-29 21:10 +0200
Re: [PATCH 4/7] iio: inv_mpu6050: Cache non-volatile bits of user_ctrl Jonathan Cameron <jic23@kernel.org> - 2016-05-01 23:40 +0200
[PATCH 6/7] iio: inv_mpu6050: Check channel configuration on preenable Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-29 21:10 +0200
Re: [PATCH 6/7] iio: inv_mpu6050: Check channel configuration on preenable Jonathan Cameron <jic23@kernel.org> - 2016-05-01 23:30 +0200
Re: [PATCH 6/7] iio: inv_mpu6050: Check channel configuration on preenable Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-05-03 15:10 +0200
Re: [PATCH 6/7] iio: inv_mpu6050: Check channel configuration on preenable Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:40 +0200
Re: [PATCH 6/7] iio: inv_mpu6050: Check channel configuration on preenable Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-05-04 17:40 +0200
Re: [PATCH 6/7] iio: inv_mpu6050: Check channel configuration on preenable Jonathan Cameron <jic23@jic23.retrosnub.co.uk> - 2016-05-04 20:30 +0200
[PATCH 1/7] iio: inv_mpu6050: Do burst reads using spi/i2c directly Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-29 21:10 +0200
Re: [PATCH 1/7] iio: inv_mpu6050: Do burst reads using spi/i2c directly Jonathan Cameron <jic23@kernel.org> - 2016-05-01 23:30 +0200
Re: [PATCH 1/7] iio: inv_mpu6050: Do burst reads using spi/i2c directly Mark Brown <broonie@kernel.org> - 2016-05-02 17:30 +0200
[RFC 7/7] iio: inv_mpu6050: Add support for external sensors Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-29 21:10 +0200
Re: [RFC 7/7] iio: inv_mpu6050: Add support for external sensors Jonathan Cameron <jic23@kernel.org> - 2016-05-01 23:40 +0200
[PATCH 3/7] iio: inv_mpu6050: Only toggle DATA_RDY_EN in inv_reset_fifo Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-29 21:10 +0200
Re: [PATCH 3/7] iio: inv_mpu6050: Only toggle DATA_RDY_EN in inv_reset_fifo Jonathan Cameron <jic23@kernel.org> - 2016-05-01 23:30 +0200
[PATCH 2/7] iio: inv_mpu6050: Initial regcache support Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-04-29 21:10 +0200
Re: [PATCH 2/7] iio: inv_mpu6050: Initial regcache support Jonathan Cameron <jic23@kernel.org> - 2016-05-01 23:40 +0200
Re: [RFC 0/7] iio: inv_mpu6050: Support i2c master and external readings Jonathan Cameron <jic23@kernel.org> - 2016-05-01 23:40 +0200
Re: [RFC 0/7] iio: inv_mpu6050: Support i2c master and external readings Mark Brown <broonie@kernel.org> - 2016-05-02 17:30 +0200
Re: [RFC 0/7] iio: inv_mpu6050: Support i2c master and external readings Crestez Dan Leonard <leonard.crestez@intel.com> - 2016-05-03 13:30 +0200
Re: [RFC 0/7] iio: inv_mpu6050: Support i2c master and external readings Mark Brown <broonie@kernel.org> - 2016-05-03 13:40 +0200
csiph-web