Jonathan Cameron schrieb:
> By using the info_mask_shared_by_all element of the channel spec, acce
The "ss" of "access" got lost somehow :o
> to the sampling frequency becomes available to in kernel users of the
> driver. It also shortens and simplifies the code a little.
I'm not totally convinced of your solution in the way that you unconditionally wake up the device, even if an invalid mask is selected in inv_mpu6050_write_raw.
Other than that, just two minor issues inline.
>
> Signed-off-by: Jonathan Cameron <jic23@xxxxxxxxxx>
> Cc: Ge Gao <ggao@xxxxxxxxxxxxxx>
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 149 +++++++++++------------------
> 1 file changed, 58 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index af287bf..088ad87 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -316,6 +316,9 @@ error_read_raw:
> default:
> return -EINVAL;
> }
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = st->chip_config.fifo_rate;
> + return IIO_VAL_INT;
> default:
> return -EINVAL;
> }
> @@ -359,51 +362,6 @@ static int inv_mpu6050_write_accel_fs(struct inv_mpu6050_state *st, int fs)
> return 0;
> }
>
> -static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int val,
> - int val2,
> - long mask) {
> - struct inv_mpu6050_state *st = iio_priv(indio_dev);
> - int result;
> -
> - mutex_lock(&indio_dev->mlock);
> - /* we should only update scale when the chip is disabled, i.e.,
> - not running */
> - if (st->chip_config.enable) {
> - result = -EBUSY;
> - goto error_write_raw;
> - }
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> - goto error_write_raw;
> -
> - switch (mask) {
> - case IIO_CHAN_INFO_SCALE:
> - switch (chan->type) {
> - case IIO_ANGL_VEL:
> - result = inv_mpu6050_write_fsr(st, val);
> - break;
> - case IIO_ACCEL:
> - result = inv_mpu6050_write_accel_fs(st, val);
> - break;
> - default:
> - result = -EINVAL;
> - break;
> - }
> - break;
> - default:
> - result = -EINVAL;
> - break;
> - }
> -
> -error_write_raw:
> - result |= inv_mpu6050_set_power_itg(st, false);
> - mutex_unlock(&indio_dev->mlock);
> -
> - return result;
> -}
> -
> /**
> * inv_mpu6050_set_lpf() - set low pass filer based on fifo rate.
> *
> @@ -435,63 +393,74 @@ static int inv_mpu6050_set_lpf(struct inv_mpu6050_state *st, int rate)
> return 0;
> }
>
> -/**
> - * inv_mpu6050_fifo_rate_store() - Set fifo rate.
> - */
> -static ssize_t inv_mpu6050_fifo_rate_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t count)
> -{
> - s32 fifo_rate;
> - u8 d;
> +static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long mask) {
> + struct inv_mpu6050_state *st = iio_priv(indio_dev);
Double whitespace ^^
> int result;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct inv_mpu6050_state *st = iio_priv(indio_dev);
> -
> - if (kstrtoint(buf, 10, &fifo_rate))
> - return -EINVAL;
> - if (fifo_rate < INV_MPU6050_MIN_FIFO_RATE ||
> - fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
> - return -EINVAL;
> - if (fifo_rate == st->chip_config.fifo_rate)
> - return count;
> + u8 d;
>
> mutex_lock(&indio_dev->mlock);
> + /* we should only update scale when the chip is disabled, i.e.,
> + not running */
Nasty multiline comment.
> if (st->chip_config.enable) {
> result = -EBUSY;
> - goto fifo_rate_fail;
> + goto error_write_raw;
> }
> result = inv_mpu6050_set_power_itg(st, true);
> if (result)
> - goto fifo_rate_fail;
> + goto error_write_raw;
>
> - d = INV_MPU6050_ONE_K_HZ / fifo_rate - 1;
> - result = inv_mpu6050_write_reg(st, st->reg->sample_rate_div, d);
> - if (result)
> - goto fifo_rate_fail;
> - st->chip_config.fifo_rate = fifo_rate;
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ANGL_VEL:
> + result = inv_mpu6050_write_fsr(st, val);
> + break;
> + case IIO_ACCEL:
> + result = inv_mpu6050_write_accel_fs(st, val);
> + break;
> + default:
> + result = -EINVAL;
> + break;
> + }
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (val < INV_MPU6050_MIN_FIFO_RATE ||
> + val > INV_MPU6050_MAX_FIFO_RATE ||
> + val2 != 0) {
> + result = -EINVAL;
> + break;
> + }
> + if (val == st->chip_config.fifo_rate) {
> + result = 0;
> + break;
> + }
>
> - result = inv_mpu6050_set_lpf(st, fifo_rate);
> - if (result)
> - goto fifo_rate_fail;
> + result = inv_mpu6050_set_power_itg(st, true);
> + if (result)
> + break;
>
> -fifo_rate_fail:
> - result |= inv_mpu6050_set_power_itg(st, false);
> - mutex_unlock(&indio_dev->mlock);
> - if (result)
> - return result;
> + d = INV_MPU6050_ONE_K_HZ / val - 1;
> + result = inv_mpu6050_write_reg(st, st->reg->sample_rate_div, d);
> + if (result)
> + break;
> + st->chip_config.fifo_rate = val;
>
> - return count;
> -}
> + result = inv_mpu6050_set_lpf(st, val);
> + break;
> + default:
> + result = -EINVAL;
> + break;
> + }
>
> -/**
> - * inv_fifo_rate_show() - Get the current sampling rate.
> - */
> -static ssize_t inv_fifo_rate_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
> +error_write_raw:
> + result |= inv_mpu6050_set_power_itg(st, false);
Don't we totally corrupt the error code when trying to OR two codes?
> + mutex_unlock(&indio_dev->mlock);
>
> - return sprintf(buf, "%d\n", st->chip_config.fifo_rate);
> + return result;
> }
>
> /**
> @@ -546,6 +515,7 @@ static int inv_mpu6050_validate_trigger(struct iio_dev *indio_dev,
> .channel2 = _channel2, \
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> .scan_index = _index, \
> .scan_type = { \
> .sign = 's', \
> @@ -580,8 +550,6 @@ static const struct iio_chan_spec inv_mpu_channels[] = {
>
> /* constant IIO attribute */
> static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("10 20 50 100 200 500");
> -static IIO_DEV_ATTR_SAMP_FREQ(S_IRUGO | S_IWUSR, inv_fifo_rate_show,
> - inv_mpu6050_fifo_rate_store);
> static IIO_DEVICE_ATTR(in_gyro_matrix, S_IRUGO, inv_attr_show, NULL,
> ATTR_GYRO_MATRIX);
> static IIO_DEVICE_ATTR(in_accel_matrix, S_IRUGO, inv_attr_show, NULL,
> @@ -590,7 +558,6 @@ static IIO_DEVICE_ATTR(in_accel_matrix, S_IRUGO, inv_attr_show, NULL,
> static struct attribute *inv_attributes[] = {
> &iio_dev_attr_in_gyro_matrix.dev_attr.attr,
> &iio_dev_attr_in_accel_matrix.dev_attr.attr,
> - &iio_dev_attr_sampling_frequency.dev_attr.attr,
> &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> NULL,
> };
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
