Commit 68cd6e5b authored by Jean-Baptiste Maneyrol's avatar Jean-Baptiste Maneyrol Committed by Jonathan Cameron

iio: imu: inv_mpu6050: fix lock issues by using our own mutex

There are several locks issues when using buffer and direct polling
data at the same time. Use our own mutex for managing locking and
block simultaneous use of buffer and direct polling by using
iio_device_{claim/release}_direct_mode. This makes chip_config
enable bit obsolete, so delete it.
Signed-off-by: default avatarJean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Signed-off-by: default avatarJonathan Cameron <jic23@kernel.org>
parent e8ee2b67
...@@ -187,7 +187,6 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on) ...@@ -187,7 +187,6 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
int result = 0; int result = 0;
if (power_on) { if (power_on) {
/* Already under indio-dev->mlock mutex */
if (!st->powerup_count) if (!st->powerup_count)
result = regmap_write(st->map, st->reg->pwr_mgmt_1, 0); result = regmap_write(st->map, st->reg->pwr_mgmt_1, 0);
if (!result) if (!result)
...@@ -298,50 +297,37 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev, ...@@ -298,50 +297,37 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
int result; int result;
ret = IIO_VAL_INT; ret = IIO_VAL_INT;
result = 0; mutex_lock(&st->lock);
mutex_lock(&indio_dev->mlock); result = iio_device_claim_direct_mode(indio_dev);
if (!st->chip_config.enable) { if (result)
result = inv_mpu6050_set_power_itg(st, true); goto error_read_raw_unlock;
if (result) result = inv_mpu6050_set_power_itg(st, true);
goto error_read_raw; if (result)
} goto error_read_raw_release;
/* when enable is on, power is already on */
switch (chan->type) { switch (chan->type) {
case IIO_ANGL_VEL: case IIO_ANGL_VEL:
if (!st->chip_config.gyro_fifo_enable || result = inv_mpu6050_switch_engine(st, true,
!st->chip_config.enable) { INV_MPU6050_BIT_PWR_GYRO_STBY);
result = inv_mpu6050_switch_engine(st, true, if (result)
INV_MPU6050_BIT_PWR_GYRO_STBY); goto error_read_raw_power_off;
if (result)
goto error_read_raw;
}
ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro, ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
chan->channel2, val); chan->channel2, val);
if (!st->chip_config.gyro_fifo_enable || result = inv_mpu6050_switch_engine(st, false,
!st->chip_config.enable) { INV_MPU6050_BIT_PWR_GYRO_STBY);
result = inv_mpu6050_switch_engine(st, false, if (result)
INV_MPU6050_BIT_PWR_GYRO_STBY); goto error_read_raw_power_off;
if (result)
goto error_read_raw;
}
break; break;
case IIO_ACCEL: case IIO_ACCEL:
if (!st->chip_config.accl_fifo_enable || result = inv_mpu6050_switch_engine(st, true,
!st->chip_config.enable) { INV_MPU6050_BIT_PWR_ACCL_STBY);
result = inv_mpu6050_switch_engine(st, true, if (result)
INV_MPU6050_BIT_PWR_ACCL_STBY); goto error_read_raw_power_off;
if (result)
goto error_read_raw;
}
ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl, ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
chan->channel2, val); chan->channel2, val);
if (!st->chip_config.accl_fifo_enable || result = inv_mpu6050_switch_engine(st, false,
!st->chip_config.enable) { INV_MPU6050_BIT_PWR_ACCL_STBY);
result = inv_mpu6050_switch_engine(st, false, if (result)
INV_MPU6050_BIT_PWR_ACCL_STBY); goto error_read_raw_power_off;
if (result)
goto error_read_raw;
}
break; break;
case IIO_TEMP: case IIO_TEMP:
/* wait for stablization */ /* wait for stablization */
...@@ -353,10 +339,12 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev, ...@@ -353,10 +339,12 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
ret = -EINVAL; ret = -EINVAL;
break; break;
} }
error_read_raw: error_read_raw_power_off:
if (!st->chip_config.enable) result |= inv_mpu6050_set_power_itg(st, false);
result |= inv_mpu6050_set_power_itg(st, false); error_read_raw_release:
mutex_unlock(&indio_dev->mlock); iio_device_release_direct_mode(indio_dev);
error_read_raw_unlock:
mutex_unlock(&st->lock);
if (result) if (result)
return result; return result;
...@@ -365,13 +353,17 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev, ...@@ -365,13 +353,17 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SCALE: case IIO_CHAN_INFO_SCALE:
switch (chan->type) { switch (chan->type) {
case IIO_ANGL_VEL: case IIO_ANGL_VEL:
mutex_lock(&st->lock);
*val = 0; *val = 0;
*val2 = gyro_scale_6050[st->chip_config.fsr]; *val2 = gyro_scale_6050[st->chip_config.fsr];
mutex_unlock(&st->lock);
return IIO_VAL_INT_PLUS_NANO; return IIO_VAL_INT_PLUS_NANO;
case IIO_ACCEL: case IIO_ACCEL:
mutex_lock(&st->lock);
*val = 0; *val = 0;
*val2 = accel_scale[st->chip_config.accl_fs]; *val2 = accel_scale[st->chip_config.accl_fs];
mutex_unlock(&st->lock);
return IIO_VAL_INT_PLUS_MICRO; return IIO_VAL_INT_PLUS_MICRO;
case IIO_TEMP: case IIO_TEMP:
...@@ -394,12 +386,16 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev, ...@@ -394,12 +386,16 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_CALIBBIAS: case IIO_CHAN_INFO_CALIBBIAS:
switch (chan->type) { switch (chan->type) {
case IIO_ANGL_VEL: case IIO_ANGL_VEL:
mutex_lock(&st->lock);
ret = inv_mpu6050_sensor_show(st, st->reg->gyro_offset, ret = inv_mpu6050_sensor_show(st, st->reg->gyro_offset,
chan->channel2, val); chan->channel2, val);
mutex_unlock(&st->lock);
return IIO_VAL_INT; return IIO_VAL_INT;
case IIO_ACCEL: case IIO_ACCEL:
mutex_lock(&st->lock);
ret = inv_mpu6050_sensor_show(st, st->reg->accl_offset, ret = inv_mpu6050_sensor_show(st, st->reg->accl_offset,
chan->channel2, val); chan->channel2, val);
mutex_unlock(&st->lock);
return IIO_VAL_INT; return IIO_VAL_INT;
default: default:
...@@ -475,18 +471,17 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev, ...@@ -475,18 +471,17 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
struct inv_mpu6050_state *st = iio_priv(indio_dev); struct inv_mpu6050_state *st = iio_priv(indio_dev);
int result; int result;
mutex_lock(&indio_dev->mlock); mutex_lock(&st->lock);
/* /*
* we should only update scale when the chip is disabled, i.e. * we should only update scale when the chip is disabled, i.e.
* not running * not running
*/ */
if (st->chip_config.enable) { result = iio_device_claim_direct_mode(indio_dev);
result = -EBUSY; if (result)
goto error_write_raw; goto error_write_raw_unlock;
}
result = inv_mpu6050_set_power_itg(st, true); result = inv_mpu6050_set_power_itg(st, true);
if (result) if (result)
goto error_write_raw; goto error_write_raw_release;
switch (mask) { switch (mask) {
case IIO_CHAN_INFO_SCALE: case IIO_CHAN_INFO_SCALE:
...@@ -522,9 +517,11 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev, ...@@ -522,9 +517,11 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
break; break;
} }
error_write_raw:
result |= inv_mpu6050_set_power_itg(st, false); result |= inv_mpu6050_set_power_itg(st, false);
mutex_unlock(&indio_dev->mlock); error_write_raw_release:
iio_device_release_direct_mode(indio_dev);
error_write_raw_unlock:
mutex_unlock(&st->lock);
return result; return result;
} }
...@@ -578,31 +575,35 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr, ...@@ -578,31 +575,35 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
if (fifo_rate < INV_MPU6050_MIN_FIFO_RATE || if (fifo_rate < INV_MPU6050_MIN_FIFO_RATE ||
fifo_rate > INV_MPU6050_MAX_FIFO_RATE) fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
return -EINVAL; return -EINVAL;
if (fifo_rate == st->chip_config.fifo_rate)
return count;
mutex_lock(&indio_dev->mlock); mutex_lock(&st->lock);
if (st->chip_config.enable) { if (fifo_rate == st->chip_config.fifo_rate) {
result = -EBUSY; result = 0;
goto fifo_rate_fail; goto fifo_rate_fail_unlock;
} }
result = iio_device_claim_direct_mode(indio_dev);
if (result)
goto fifo_rate_fail_unlock;
result = inv_mpu6050_set_power_itg(st, true); result = inv_mpu6050_set_power_itg(st, true);
if (result) if (result)
goto fifo_rate_fail; goto fifo_rate_fail_release;
d = INV_MPU6050_ONE_K_HZ / fifo_rate - 1; d = INV_MPU6050_ONE_K_HZ / fifo_rate - 1;
result = regmap_write(st->map, st->reg->sample_rate_div, d); result = regmap_write(st->map, st->reg->sample_rate_div, d);
if (result) if (result)
goto fifo_rate_fail; goto fifo_rate_fail_power_off;
st->chip_config.fifo_rate = fifo_rate; st->chip_config.fifo_rate = fifo_rate;
result = inv_mpu6050_set_lpf(st, fifo_rate); result = inv_mpu6050_set_lpf(st, fifo_rate);
if (result) if (result)
goto fifo_rate_fail; goto fifo_rate_fail_power_off;
fifo_rate_fail: fifo_rate_fail_power_off:
result |= inv_mpu6050_set_power_itg(st, false); result |= inv_mpu6050_set_power_itg(st, false);
mutex_unlock(&indio_dev->mlock); fifo_rate_fail_release:
iio_device_release_direct_mode(indio_dev);
fifo_rate_fail_unlock:
mutex_unlock(&st->lock);
if (result) if (result)
return result; return result;
...@@ -617,8 +618,13 @@ inv_fifo_rate_show(struct device *dev, struct device_attribute *attr, ...@@ -617,8 +618,13 @@ inv_fifo_rate_show(struct device *dev, struct device_attribute *attr,
char *buf) char *buf)
{ {
struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev)); struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
unsigned fifo_rate;
mutex_lock(&st->lock);
fifo_rate = st->chip_config.fifo_rate;
mutex_unlock(&st->lock);
return sprintf(buf, "%d\n", st->chip_config.fifo_rate); return scnprintf(buf, PAGE_SIZE, "%u\n", fifo_rate);
} }
/** /**
...@@ -836,6 +842,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name, ...@@ -836,6 +842,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
return -ENODEV; return -ENODEV;
} }
st = iio_priv(indio_dev); st = iio_priv(indio_dev);
mutex_init(&st->lock);
st->chip_type = chip_type; st->chip_type = chip_type;
st->powerup_count = 0; st->powerup_count = 0;
st->irq = irq; st->irq = irq;
...@@ -929,12 +936,26 @@ EXPORT_SYMBOL_GPL(inv_mpu_core_remove); ...@@ -929,12 +936,26 @@ EXPORT_SYMBOL_GPL(inv_mpu_core_remove);
static int inv_mpu_resume(struct device *dev) static int inv_mpu_resume(struct device *dev)
{ {
return inv_mpu6050_set_power_itg(iio_priv(dev_get_drvdata(dev)), true); struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
int result;
mutex_lock(&st->lock);
result = inv_mpu6050_set_power_itg(st, true);
mutex_unlock(&st->lock);
return result;
} }
static int inv_mpu_suspend(struct device *dev) static int inv_mpu_suspend(struct device *dev)
{ {
return inv_mpu6050_set_power_itg(iio_priv(dev_get_drvdata(dev)), false); struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
int result;
mutex_lock(&st->lock);
result = inv_mpu6050_set_power_itg(st, false);
mutex_unlock(&st->lock);
return result;
} }
#endif /* CONFIG_PM_SLEEP */ #endif /* CONFIG_PM_SLEEP */
......
...@@ -32,7 +32,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id) ...@@ -32,7 +32,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
int ret = 0; int ret = 0;
/* Use the same mutex which was used everywhere to protect power-op */ /* Use the same mutex which was used everywhere to protect power-op */
mutex_lock(&indio_dev->mlock); mutex_lock(&st->lock);
if (!st->powerup_count) { if (!st->powerup_count) {
ret = regmap_write(st->map, st->reg->pwr_mgmt_1, 0); ret = regmap_write(st->map, st->reg->pwr_mgmt_1, 0);
if (ret) if (ret)
...@@ -48,7 +48,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id) ...@@ -48,7 +48,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
INV_MPU6050_BIT_BYPASS_EN); INV_MPU6050_BIT_BYPASS_EN);
} }
write_error: write_error:
mutex_unlock(&indio_dev->mlock); mutex_unlock(&st->lock);
return ret; return ret;
} }
...@@ -58,14 +58,14 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id) ...@@ -58,14 +58,14 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
struct iio_dev *indio_dev = i2c_mux_priv(muxc); struct iio_dev *indio_dev = i2c_mux_priv(muxc);
struct inv_mpu6050_state *st = iio_priv(indio_dev); struct inv_mpu6050_state *st = iio_priv(indio_dev);
mutex_lock(&indio_dev->mlock); mutex_lock(&st->lock);
/* It doesn't really mattter, if any of the calls fails */ /* It doesn't really mattter, if any of the calls fails */
regmap_write(st->map, st->reg->int_pin_cfg, INV_MPU6050_INT_PIN_CFG); regmap_write(st->map, st->reg->int_pin_cfg, INV_MPU6050_INT_PIN_CFG);
st->powerup_count--; st->powerup_count--;
if (!st->powerup_count) if (!st->powerup_count)
regmap_write(st->map, st->reg->pwr_mgmt_1, regmap_write(st->map, st->reg->pwr_mgmt_1,
INV_MPU6050_BIT_SLEEP); INV_MPU6050_BIT_SLEEP);
mutex_unlock(&indio_dev->mlock); mutex_unlock(&st->lock);
return 0; return 0;
} }
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include <linux/i2c-mux.h> #include <linux/i2c-mux.h>
#include <linux/kfifo.h> #include <linux/kfifo.h>
#include <linux/spinlock.h> #include <linux/spinlock.h>
#include <linux/mutex.h>
#include <linux/iio/iio.h> #include <linux/iio/iio.h>
#include <linux/iio/buffer.h> #include <linux/iio/buffer.h>
#include <linux/regmap.h> #include <linux/regmap.h>
...@@ -80,7 +81,6 @@ enum inv_devices { ...@@ -80,7 +81,6 @@ enum inv_devices {
* @fsr: Full scale range. * @fsr: Full scale range.
* @lpf: Digital low pass filter frequency. * @lpf: Digital low pass filter frequency.
* @accl_fs: accel full scale range. * @accl_fs: accel full scale range.
* @enable: master enable state.
* @accl_fifo_enable: enable accel data output * @accl_fifo_enable: enable accel data output
* @gyro_fifo_enable: enable gyro data output * @gyro_fifo_enable: enable gyro data output
* @fifo_rate: FIFO update rate. * @fifo_rate: FIFO update rate.
...@@ -89,7 +89,6 @@ struct inv_mpu6050_chip_config { ...@@ -89,7 +89,6 @@ struct inv_mpu6050_chip_config {
unsigned int fsr:2; unsigned int fsr:2;
unsigned int lpf:3; unsigned int lpf:3;
unsigned int accl_fs:2; unsigned int accl_fs:2;
unsigned int enable:1;
unsigned int accl_fifo_enable:1; unsigned int accl_fifo_enable:1;
unsigned int gyro_fifo_enable:1; unsigned int gyro_fifo_enable:1;
u16 fifo_rate; u16 fifo_rate;
...@@ -112,6 +111,7 @@ struct inv_mpu6050_hw { ...@@ -112,6 +111,7 @@ struct inv_mpu6050_hw {
/* /*
* struct inv_mpu6050_state - Driver state variables. * struct inv_mpu6050_state - Driver state variables.
* @TIMESTAMP_FIFO_SIZE: fifo size for timestamp. * @TIMESTAMP_FIFO_SIZE: fifo size for timestamp.
* @lock: Chip access lock.
* @trig: IIO trigger. * @trig: IIO trigger.
* @chip_config: Cached attribute information. * @chip_config: Cached attribute information.
* @reg: Map of important registers. * @reg: Map of important registers.
...@@ -126,6 +126,7 @@ struct inv_mpu6050_hw { ...@@ -126,6 +126,7 @@ struct inv_mpu6050_hw {
*/ */
struct inv_mpu6050_state { struct inv_mpu6050_state {
#define TIMESTAMP_FIFO_SIZE 16 #define TIMESTAMP_FIFO_SIZE 16
struct mutex lock;
struct iio_trigger *trig; struct iio_trigger *trig;
struct inv_mpu6050_chip_config chip_config; struct inv_mpu6050_chip_config chip_config;
const struct inv_mpu6050_reg_map *reg; const struct inv_mpu6050_reg_map *reg;
......
...@@ -128,7 +128,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) ...@@ -128,7 +128,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
u16 fifo_count; u16 fifo_count;
s64 timestamp; s64 timestamp;
mutex_lock(&indio_dev->mlock); mutex_lock(&st->lock);
if (!(st->chip_config.accl_fifo_enable | if (!(st->chip_config.accl_fifo_enable |
st->chip_config.gyro_fifo_enable)) st->chip_config.gyro_fifo_enable))
goto end_session; goto end_session;
...@@ -178,7 +178,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) ...@@ -178,7 +178,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
} }
end_session: end_session:
mutex_unlock(&indio_dev->mlock); mutex_unlock(&st->lock);
iio_trigger_notify_done(indio_dev->trig); iio_trigger_notify_done(indio_dev->trig);
return IRQ_HANDLED; return IRQ_HANDLED;
...@@ -186,7 +186,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) ...@@ -186,7 +186,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
flush_fifo: flush_fifo:
/* Flush HW and SW FIFOs. */ /* Flush HW and SW FIFOs. */
inv_reset_fifo(indio_dev); inv_reset_fifo(indio_dev);
mutex_unlock(&indio_dev->mlock); mutex_unlock(&st->lock);
iio_trigger_notify_done(indio_dev->trig); iio_trigger_notify_done(indio_dev->trig);
return IRQ_HANDLED; return IRQ_HANDLED;
......
...@@ -90,7 +90,6 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable) ...@@ -90,7 +90,6 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
if (result) if (result)
return result; return result;
} }
st->chip_config.enable = enable;
return 0; return 0;
} }
...@@ -103,7 +102,15 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable) ...@@ -103,7 +102,15 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
static int inv_mpu_data_rdy_trigger_set_state(struct iio_trigger *trig, static int inv_mpu_data_rdy_trigger_set_state(struct iio_trigger *trig,
bool state) bool state)
{ {
return inv_mpu6050_set_enable(iio_trigger_get_drvdata(trig), state); struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
int result;
mutex_lock(&st->lock);
result = inv_mpu6050_set_enable(indio_dev, state);
mutex_unlock(&st->lock);
return result;
} }
static const struct iio_trigger_ops inv_mpu_trigger_ops = { static const struct iio_trigger_ops inv_mpu_trigger_ops = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment