* [PATCH v2 01/13] iio: imu: inv_mpu6050: enable i2c aux mux bypass only once
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:17 ` Jonathan Cameron
2020-02-19 14:39 ` [PATCH v2 02/13] iio: imu: inv_mpu6050: delete useless check Jean-Baptiste Maneyrol
` (11 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
i2c auxiliary mux is done by analog switches. You do not need to
set them for every i2c transfer.
Just set i2c bypass bit at init and do noting in i2c de/select.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 71 +++++++++--------------
1 file changed, 28 insertions(+), 43 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index 1363d3776523..24df880248f2 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -20,38 +20,6 @@ static const struct regmap_config inv_mpu_regmap_config = {
static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
{
- struct iio_dev *indio_dev = i2c_mux_priv(muxc);
- struct inv_mpu6050_state *st = iio_priv(indio_dev);
- int ret;
-
- mutex_lock(&st->lock);
-
- ret = inv_mpu6050_set_power_itg(st, true);
- if (ret)
- goto error_unlock;
-
- ret = regmap_write(st->map, st->reg->int_pin_cfg,
- st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
-
-error_unlock:
- mutex_unlock(&st->lock);
-
- return ret;
-}
-
-static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
-{
- struct iio_dev *indio_dev = i2c_mux_priv(muxc);
- struct inv_mpu6050_state *st = iio_priv(indio_dev);
-
- mutex_lock(&st->lock);
-
- /* It doesn't really matter if any of the calls fail */
- regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
- inv_mpu6050_set_power_itg(st, false);
-
- mutex_unlock(&st->lock);
-
return 0;
}
@@ -79,19 +47,20 @@ static bool inv_mpu_i2c_aux_bus(struct device *dev)
}
}
-/*
- * MPU9xxx magnetometer support requires to disable i2c auxiliary bus support.
- * To ensure backward compatibility with existing setups, do not disable
- * i2c auxiliary bus if it used.
- * Check for i2c-gate node in devicetree and set magnetometer disabled.
- * Only MPU6500 is supported by ACPI, no need to check.
- */
-static int inv_mpu_magn_disable(struct iio_dev *indio_dev)
+static int inv_mpu_i2c_aux_setup(struct iio_dev *indio_dev)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
struct device *dev = indio_dev->dev.parent;
struct device_node *mux_node;
+ int ret;
+ /*
+ * MPU9xxx magnetometer support requires to disable i2c auxiliary bus.
+ * To ensure backward compatibility with existing setups, do not disable
+ * i2c auxiliary bus if it used.
+ * Check for i2c-gate node in devicetree and set magnetometer disabled.
+ * Only MPU6500 is supported by ACPI, no need to check.
+ */
switch (st->chip_type) {
case INV_MPU9150:
case INV_MPU9250:
@@ -107,7 +76,24 @@ static int inv_mpu_magn_disable(struct iio_dev *indio_dev)
break;
}
+ /* enable i2c bypass when using i2c auxiliary bus */
+ if (inv_mpu_i2c_aux_bus(dev)) {
+ ret = inv_mpu6050_set_power_itg(st, true);
+ if (ret)
+ return ret;
+ ret = regmap_write(st->map, st->reg->int_pin_cfg,
+ st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
+ if (ret)
+ goto error;
+ ret = inv_mpu6050_set_power_itg(st, false);
+ if (ret)
+ goto error;
+ }
+
return 0;
+error:
+ inv_mpu6050_set_power_itg(st, false);
+ return ret;
}
/**
@@ -151,7 +137,7 @@ static int inv_mpu_probe(struct i2c_client *client,
}
result = inv_mpu_core_probe(regmap, client->irq, name,
- inv_mpu_magn_disable, chip_type);
+ inv_mpu_i2c_aux_setup, chip_type);
if (result < 0)
return result;
@@ -160,8 +146,7 @@ static int inv_mpu_probe(struct i2c_client *client,
/* declare i2c auxiliary bus */
st->muxc = i2c_mux_alloc(client->adapter, &client->dev,
1, 0, I2C_MUX_LOCKED | I2C_MUX_GATE,
- inv_mpu6050_select_bypass,
- inv_mpu6050_deselect_bypass);
+ inv_mpu6050_select_bypass, NULL);
if (!st->muxc)
return -ENOMEM;
st->muxc->priv = dev_get_drvdata(&client->dev);
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 01/13] iio: imu: inv_mpu6050: enable i2c aux mux bypass only once
2020-02-19 14:39 ` [PATCH v2 01/13] iio: imu: inv_mpu6050: enable i2c aux mux bypass only once Jean-Baptiste Maneyrol
@ 2020-02-21 11:17 ` Jonathan Cameron
2020-02-21 17:01 ` Peter Rosin
0 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:17 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio, Peter Rosin
On Wed, 19 Feb 2020 15:39:46 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> i2c auxiliary mux is done by analog switches. You do not need to
> set them for every i2c transfer.
> Just set i2c bypass bit at init and do noting in i2c de/select.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
OK. Given we are keeping the i2c mux stuff mostly for backwards
compatibility I'll take this. However, there is still a bit
of time if Peter want's to comment before I push this out as
a non rebasing tree.
Please make sure to cc peter on anything touching i2c mux.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 71 +++++++++--------------
> 1 file changed, 28 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index 1363d3776523..24df880248f2 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -20,38 +20,6 @@ static const struct regmap_config inv_mpu_regmap_config = {
>
> static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
> {
> - struct iio_dev *indio_dev = i2c_mux_priv(muxc);
> - struct inv_mpu6050_state *st = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(&st->lock);
> -
> - ret = inv_mpu6050_set_power_itg(st, true);
> - if (ret)
> - goto error_unlock;
> -
> - ret = regmap_write(st->map, st->reg->int_pin_cfg,
> - st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
> -
> -error_unlock:
> - mutex_unlock(&st->lock);
> -
> - return ret;
> -}
> -
> -static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
> -{
> - struct iio_dev *indio_dev = i2c_mux_priv(muxc);
> - struct inv_mpu6050_state *st = iio_priv(indio_dev);
> -
> - mutex_lock(&st->lock);
> -
> - /* It doesn't really matter if any of the calls fail */
> - regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
> - inv_mpu6050_set_power_itg(st, false);
> -
> - mutex_unlock(&st->lock);
> -
> return 0;
> }
>
> @@ -79,19 +47,20 @@ static bool inv_mpu_i2c_aux_bus(struct device *dev)
> }
> }
>
> -/*
> - * MPU9xxx magnetometer support requires to disable i2c auxiliary bus support.
> - * To ensure backward compatibility with existing setups, do not disable
> - * i2c auxiliary bus if it used.
> - * Check for i2c-gate node in devicetree and set magnetometer disabled.
> - * Only MPU6500 is supported by ACPI, no need to check.
> - */
> -static int inv_mpu_magn_disable(struct iio_dev *indio_dev)
> +static int inv_mpu_i2c_aux_setup(struct iio_dev *indio_dev)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> struct device *dev = indio_dev->dev.parent;
> struct device_node *mux_node;
> + int ret;
>
> + /*
> + * MPU9xxx magnetometer support requires to disable i2c auxiliary bus.
> + * To ensure backward compatibility with existing setups, do not disable
> + * i2c auxiliary bus if it used.
> + * Check for i2c-gate node in devicetree and set magnetometer disabled.
> + * Only MPU6500 is supported by ACPI, no need to check.
> + */
> switch (st->chip_type) {
> case INV_MPU9150:
> case INV_MPU9250:
> @@ -107,7 +76,24 @@ static int inv_mpu_magn_disable(struct iio_dev *indio_dev)
> break;
> }
>
> + /* enable i2c bypass when using i2c auxiliary bus */
> + if (inv_mpu_i2c_aux_bus(dev)) {
> + ret = inv_mpu6050_set_power_itg(st, true);
> + if (ret)
> + return ret;
> + ret = regmap_write(st->map, st->reg->int_pin_cfg,
> + st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
> + if (ret)
> + goto error;
> + ret = inv_mpu6050_set_power_itg(st, false);
> + if (ret)
> + goto error;
> + }
> +
> return 0;
> +error:
> + inv_mpu6050_set_power_itg(st, false);
> + return ret;
> }
>
> /**
> @@ -151,7 +137,7 @@ static int inv_mpu_probe(struct i2c_client *client,
> }
>
> result = inv_mpu_core_probe(regmap, client->irq, name,
> - inv_mpu_magn_disable, chip_type);
> + inv_mpu_i2c_aux_setup, chip_type);
> if (result < 0)
> return result;
>
> @@ -160,8 +146,7 @@ static int inv_mpu_probe(struct i2c_client *client,
> /* declare i2c auxiliary bus */
> st->muxc = i2c_mux_alloc(client->adapter, &client->dev,
> 1, 0, I2C_MUX_LOCKED | I2C_MUX_GATE,
> - inv_mpu6050_select_bypass,
> - inv_mpu6050_deselect_bypass);
> + inv_mpu6050_select_bypass, NULL);
> if (!st->muxc)
> return -ENOMEM;
> st->muxc->priv = dev_get_drvdata(&client->dev);
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 01/13] iio: imu: inv_mpu6050: enable i2c aux mux bypass only once
2020-02-21 11:17 ` Jonathan Cameron
@ 2020-02-21 17:01 ` Peter Rosin
0 siblings, 0 replies; 30+ messages in thread
From: Peter Rosin @ 2020-02-21 17:01 UTC (permalink / raw)
To: Jonathan Cameron, Jean-Baptiste Maneyrol; +Cc: linux-iio
On 2020-02-21 12:17, Jonathan Cameron wrote:
> On Wed, 19 Feb 2020 15:39:46 +0100
> Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
>
>> i2c auxiliary mux is done by analog switches. You do not need to
>> set them for every i2c transfer.
>> Just set i2c bypass bit at init and do noting in i2c de/select.
Nits:
i2c -> I2C (in comments, docs, commit messages, etc)
noting -> nothing
>>
>> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> OK. Given we are keeping the i2c mux stuff mostly for backwards
> compatibility I'll take this. However, there is still a bit
> of time if Peter want's to comment before I push this out as
> a non rebasing tree.
Even though I have not tried it, I see no conceptual problem to have
an I2C mux (or gate as in this case) that does "nothing". You do get
the backwards compatible extra I2C bus for the other side of the gate,
keeping the expected I2C adapter tree intact. But do note that a I2C
mux that does "nothing" still triggers locking and various extra
indirections and operations so it's not a complete NOP.
So,
Acked-by: Peter Rosin <peda@axentia.se>
(one more nit below)
> Please make sure to cc peter on anything touching i2c mux.
>
> Applied to the togreg branch of iio.git and pushed out as testing for
> the autobuilders to play with it.
>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 71 +++++++++--------------
>> 1 file changed, 28 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> index 1363d3776523..24df880248f2 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>> @@ -20,38 +20,6 @@ static const struct regmap_config inv_mpu_regmap_config = {
>>
>> static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>> {
>> - struct iio_dev *indio_dev = i2c_mux_priv(muxc);
>> - struct inv_mpu6050_state *st = iio_priv(indio_dev);
>> - int ret;
>> -
>> - mutex_lock(&st->lock);
>> -
>> - ret = inv_mpu6050_set_power_itg(st, true);
>> - if (ret)
>> - goto error_unlock;
>> -
>> - ret = regmap_write(st->map, st->reg->int_pin_cfg,
>> - st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
>> -
>> -error_unlock:
>> - mutex_unlock(&st->lock);
>> -
>> - return ret;
>> -}
>> -
>> -static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>> -{
>> - struct iio_dev *indio_dev = i2c_mux_priv(muxc);
>> - struct inv_mpu6050_state *st = iio_priv(indio_dev);
>> -
>> - mutex_lock(&st->lock);
>> -
>> - /* It doesn't really matter if any of the calls fail */
>> - regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
>> - inv_mpu6050_set_power_itg(st, false);
>> -
>> - mutex_unlock(&st->lock);
>> -
>> return 0;
>> }
>>
>> @@ -79,19 +47,20 @@ static bool inv_mpu_i2c_aux_bus(struct device *dev)
>> }
>> }
>>
>> -/*
>> - * MPU9xxx magnetometer support requires to disable i2c auxiliary bus support.
>> - * To ensure backward compatibility with existing setups, do not disable
>> - * i2c auxiliary bus if it used.
>> - * Check for i2c-gate node in devicetree and set magnetometer disabled.
>> - * Only MPU6500 is supported by ACPI, no need to check.
>> - */
>> -static int inv_mpu_magn_disable(struct iio_dev *indio_dev)
>> +static int inv_mpu_i2c_aux_setup(struct iio_dev *indio_dev)
>> {
>> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>> struct device *dev = indio_dev->dev.parent;
>> struct device_node *mux_node;
>> + int ret;
>>
>> + /*
>> + * MPU9xxx magnetometer support requires to disable i2c auxiliary bus.
>> + * To ensure backward compatibility with existing setups, do not disable
>> + * i2c auxiliary bus if it used.
it used -> it is used
Cheers,
Peter
>> + * Check for i2c-gate node in devicetree and set magnetometer disabled.
>> + * Only MPU6500 is supported by ACPI, no need to check.
>> + */
>> switch (st->chip_type) {
>> case INV_MPU9150:
>> case INV_MPU9250:
>> @@ -107,7 +76,24 @@ static int inv_mpu_magn_disable(struct iio_dev *indio_dev)
>> break;
>> }
>>
>> + /* enable i2c bypass when using i2c auxiliary bus */
>> + if (inv_mpu_i2c_aux_bus(dev)) {
>> + ret = inv_mpu6050_set_power_itg(st, true);
>> + if (ret)
>> + return ret;
>> + ret = regmap_write(st->map, st->reg->int_pin_cfg,
>> + st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
>> + if (ret)
>> + goto error;
>> + ret = inv_mpu6050_set_power_itg(st, false);
>> + if (ret)
>> + goto error;
>> + }
>> +
>> return 0;
>> +error:
>> + inv_mpu6050_set_power_itg(st, false);
>> + return ret;
>> }
>>
>> /**
>> @@ -151,7 +137,7 @@ static int inv_mpu_probe(struct i2c_client *client,
>> }
>>
>> result = inv_mpu_core_probe(regmap, client->irq, name,
>> - inv_mpu_magn_disable, chip_type);
>> + inv_mpu_i2c_aux_setup, chip_type);
>> if (result < 0)
>> return result;
>>
>> @@ -160,8 +146,7 @@ static int inv_mpu_probe(struct i2c_client *client,
>> /* declare i2c auxiliary bus */
>> st->muxc = i2c_mux_alloc(client->adapter, &client->dev,
>> 1, 0, I2C_MUX_LOCKED | I2C_MUX_GATE,
>> - inv_mpu6050_select_bypass,
>> - inv_mpu6050_deselect_bypass);
>> + inv_mpu6050_select_bypass, NULL);
>> if (!st->muxc)
>> return -ENOMEM;
>> st->muxc->priv = dev_get_drvdata(&client->dev);
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 02/13] iio: imu: inv_mpu6050: delete useless check
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
2020-02-19 14:39 ` [PATCH v2 01/13] iio: imu: inv_mpu6050: enable i2c aux mux bypass only once Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:19 ` Jonathan Cameron
2020-02-19 14:39 ` [PATCH v2 03/13] iio: imu: inv_mpu6050: set power on/off only once during all init Jean-Baptiste Maneyrol
` (10 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
If we are here it means we have fifo enabled for 1 sensor
at least. And interrupt is always required for using trigger.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index f9fdf4302a91..d7397705974e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -123,14 +123,11 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
goto reset_fifo_fail;
/* enable interrupt */
- if (st->chip_config.accl_fifo_enable ||
- st->chip_config.gyro_fifo_enable ||
- st->chip_config.magn_fifo_enable) {
- result = regmap_write(st->map, st->reg->int_enable,
- INV_MPU6050_BIT_DATA_RDY_EN);
- if (result)
- return result;
- }
+ result = regmap_write(st->map, st->reg->int_enable,
+ INV_MPU6050_BIT_DATA_RDY_EN);
+ if (result)
+ return result;
+
/* enable FIFO reading */
d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_EN;
result = regmap_write(st->map, st->reg->user_ctrl, d);
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 02/13] iio: imu: inv_mpu6050: delete useless check
2020-02-19 14:39 ` [PATCH v2 02/13] iio: imu: inv_mpu6050: delete useless check Jean-Baptiste Maneyrol
@ 2020-02-21 11:19 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:19 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Wed, 19 Feb 2020 15:39:47 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> If we are here it means we have fifo enabled for 1 sensor
> at least. And interrupt is always required for using trigger.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index f9fdf4302a91..d7397705974e 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -123,14 +123,11 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
> goto reset_fifo_fail;
>
> /* enable interrupt */
> - if (st->chip_config.accl_fifo_enable ||
> - st->chip_config.gyro_fifo_enable ||
> - st->chip_config.magn_fifo_enable) {
> - result = regmap_write(st->map, st->reg->int_enable,
> - INV_MPU6050_BIT_DATA_RDY_EN);
> - if (result)
> - return result;
> - }
> + result = regmap_write(st->map, st->reg->int_enable,
> + INV_MPU6050_BIT_DATA_RDY_EN);
> + if (result)
> + return result;
> +
> /* enable FIFO reading */
> d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_EN;
> result = regmap_write(st->map, st->reg->user_ctrl, d);
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 03/13] iio: imu: inv_mpu6050: set power on/off only once during all init
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
2020-02-19 14:39 ` [PATCH v2 01/13] iio: imu: inv_mpu6050: enable i2c aux mux bypass only once Jean-Baptiste Maneyrol
2020-02-19 14:39 ` [PATCH v2 02/13] iio: imu: inv_mpu6050: delete useless check Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-19 14:39 ` [PATCH v2 04/13] iio: imu: inv_mpu6050: simplify polling magnetometer Jean-Baptiste Maneyrol
` (9 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
This way there is no need anymore to export the power function to
i2c and spi modules.
Bus setup is done inside init when power is on and the result is
now checked.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 41 ++++++++++++----------
drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 11 +-----
drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c | 10 +-----
3 files changed, 24 insertions(+), 38 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index c4db9086775c..0b06d6aa6469 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -301,7 +301,6 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
return 0;
}
-EXPORT_SYMBOL_GPL(inv_mpu6050_set_power_itg);
static int inv_mpu6050_set_gyro_fsr(struct inv_mpu6050_state *st,
enum inv_mpu6050_fsr_e val)
@@ -371,27 +370,23 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
u8 d;
struct inv_mpu6050_state *st = iio_priv(indio_dev);
- result = inv_mpu6050_set_power_itg(st, true);
- if (result)
- return result;
-
result = inv_mpu6050_set_gyro_fsr(st, INV_MPU6050_FSR_2000DPS);
if (result)
- goto error_power_off;
+ return result;
result = inv_mpu6050_set_lpf_regs(st, INV_MPU6050_FILTER_20HZ);
if (result)
- goto error_power_off;
+ return result;
d = INV_MPU6050_FIFO_RATE_TO_DIVIDER(INV_MPU6050_INIT_FIFO_RATE);
result = regmap_write(st->map, st->reg->sample_rate_div, d);
if (result)
- goto error_power_off;
+ return result;
d = (INV_MPU6050_FS_02G << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
result = regmap_write(st->map, st->reg->accl_config, d);
if (result)
- goto error_power_off;
+ return result;
result = regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
if (result)
@@ -410,13 +405,9 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
/* magn chip init, noop if not present in the chip */
result = inv_mpu_magn_probe(st);
if (result)
- goto error_power_off;
-
- return inv_mpu6050_set_power_itg(st, false);
+ return result;
-error_power_off:
- inv_mpu6050_set_power_itg(st, false);
- return result;
+ return 0;
}
static int inv_mpu6050_sensor_set(struct inv_mpu6050_state *st, int reg,
@@ -1176,7 +1167,7 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
if (result)
goto error_power_off;
- return inv_mpu6050_set_power_itg(st, false);
+ return 0;
error_power_off:
inv_mpu6050_set_power_itg(st, false);
@@ -1341,7 +1332,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
result = inv_mpu6050_init_config(indio_dev);
if (result) {
dev_err(dev, "Could not initialize device.\n");
- return result;
+ goto error_power_off;
}
dev_set_drvdata(dev, indio_dev);
@@ -1353,8 +1344,16 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
indio_dev->name = dev_name(dev);
/* requires parent device set in indio_dev */
- if (inv_mpu_bus_setup)
- inv_mpu_bus_setup(indio_dev);
+ if (inv_mpu_bus_setup) {
+ result = inv_mpu_bus_setup(indio_dev);
+ if (result)
+ goto error_power_off;
+ }
+
+ /* chip init is done, turning off */
+ result = inv_mpu6050_set_power_itg(st, false);
+ if (result)
+ return result;
switch (chip_type) {
case INV_MPU9150:
@@ -1413,6 +1412,10 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
}
return 0;
+
+error_power_off:
+ inv_mpu6050_set_power_itg(st, false);
+ return result;
}
EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index 24df880248f2..6993d3b87bb0 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -78,22 +78,13 @@ static int inv_mpu_i2c_aux_setup(struct iio_dev *indio_dev)
/* enable i2c bypass when using i2c auxiliary bus */
if (inv_mpu_i2c_aux_bus(dev)) {
- ret = inv_mpu6050_set_power_itg(st, true);
- if (ret)
- return ret;
ret = regmap_write(st->map, st->reg->int_pin_cfg,
st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
if (ret)
- goto error;
- ret = inv_mpu6050_set_power_itg(st, false);
- if (ret)
- goto error;
+ return ret;
}
return 0;
-error:
- inv_mpu6050_set_power_itg(st, false);
- return ret;
}
/**
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
index bc351dd58c53..673b198e6368 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
@@ -21,10 +21,6 @@ static int inv_mpu_i2c_disable(struct iio_dev *indio_dev)
struct inv_mpu6050_state *st = iio_priv(indio_dev);
int ret = 0;
- ret = inv_mpu6050_set_power_itg(st, true);
- if (ret)
- return ret;
-
if (st->reg->i2c_if) {
ret = regmap_write(st->map, st->reg->i2c_if,
INV_ICM20602_BIT_I2C_IF_DIS);
@@ -33,12 +29,8 @@ static int inv_mpu_i2c_disable(struct iio_dev *indio_dev)
ret = regmap_write(st->map, st->reg->user_ctrl,
st->chip_config.user_ctrl);
}
- if (ret) {
- inv_mpu6050_set_power_itg(st, false);
- return ret;
- }
- return inv_mpu6050_set_power_itg(st, false);
+ return ret;
}
static int inv_mpu_probe(struct spi_device *spi)
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 04/13] iio: imu: inv_mpu6050: simplify polling magnetometer
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (2 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 03/13] iio: imu: inv_mpu6050: set power on/off only once during all init Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-19 14:39 ` [PATCH v2 05/13] iio: imu: inv_mpu6050: early init of chip_config for use at setup Jean-Baptiste Maneyrol
` (8 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Do not change the sampling rate value. Let userspace decide what
is the sampling rate to use.
Read only the requested axis.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c | 34 +++++++++-------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
index 4f192352521e..607104a2631e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
@@ -319,36 +319,36 @@ int inv_mpu_magn_set_orient(struct inv_mpu6050_state *st)
int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val)
{
unsigned int user_ctrl, status;
- __be16 data[3];
+ __be16 data;
uint8_t addr;
- uint8_t d;
- unsigned int period_ms;
+ unsigned int freq_hz, period_ms;
int ret;
/* quit if chip is not supported */
if (!inv_magn_supported(st))
return -ENODEV;
- /* Mag data: X - Y - Z */
+ /* Mag data: XH,XL,YH,YL,ZH,ZL */
switch (axis) {
case IIO_MOD_X:
addr = 0;
break;
case IIO_MOD_Y:
- addr = 1;
+ addr = 2;
break;
case IIO_MOD_Z:
- addr = 2;
+ addr = 4;
break;
default:
return -EINVAL;
}
+ addr += INV_MPU6050_REG_EXT_SENS_DATA;
- /* set sample rate to max mag freq */
- d = INV_MPU6050_FIFO_RATE_TO_DIVIDER(INV_MPU_MAGN_FREQ_HZ_MAX);
- ret = regmap_write(st->map, st->reg->sample_rate_div, d);
- if (ret)
- return ret;
+ /* compute period depending on current sampling rate */
+ freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
+ if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX)
+ freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
+ period_ms = 1000 / freq_hz;
/* start i2c master, wait for xfer, stop */
user_ctrl = st->chip_config.user_ctrl | INV_MPU6050_BIT_I2C_MST_EN;
@@ -357,19 +357,12 @@ int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val)
return ret;
/* need to wait 2 periods + half-period margin */
- period_ms = 1000 / INV_MPU_MAGN_FREQ_HZ_MAX;
msleep(period_ms * 2 + period_ms / 2);
user_ctrl = st->chip_config.user_ctrl;
ret = regmap_write(st->map, st->reg->user_ctrl, user_ctrl);
if (ret)
return ret;
- /* restore sample rate */
- d = st->chip_config.divider;
- ret = regmap_write(st->map, st->reg->sample_rate_div, d);
- if (ret)
- return ret;
-
/* check i2c status and read raw data */
ret = regmap_read(st->map, INV_MPU6050_REG_I2C_MST_STATUS, &status);
if (ret)
@@ -379,12 +372,11 @@ int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val)
status & INV_MPU6050_BIT_I2C_SLV1_NACK)
return -EIO;
- ret = regmap_bulk_read(st->map, INV_MPU6050_REG_EXT_SENS_DATA,
- data, sizeof(data));
+ ret = regmap_bulk_read(st->map, addr, &data, sizeof(data));
if (ret)
return ret;
- *val = (int16_t)be16_to_cpu(data[addr]);
+ *val = (int16_t)be16_to_cpu(data);
return IIO_VAL_INT;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 05/13] iio: imu: inv_mpu6050: early init of chip_config for use at setup
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (3 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 04/13] iio: imu: inv_mpu6050: simplify polling magnetometer Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-19 14:39 ` [PATCH v2 06/13] iio: imu: inv_mpu6050: add all signal path resets at init Jean-Baptiste Maneyrol
` (7 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Init chip_config early and use its values for initial setup.
More coherent, prevent possible mistakes.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 15 +++++++--------
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 1 -
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 0b06d6aa6469..85872e55154f 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -101,7 +101,7 @@ static const struct inv_mpu6050_reg_map reg_set_6050 = {
static const struct inv_mpu6050_chip_config chip_config_6050 = {
.fsr = INV_MPU6050_FSR_2000DPS,
.lpf = INV_MPU6050_FILTER_20HZ,
- .divider = INV_MPU6050_FIFO_RATE_TO_DIVIDER(INV_MPU6050_INIT_FIFO_RATE),
+ .divider = INV_MPU6050_FIFO_RATE_TO_DIVIDER(50),
.gyro_fifo_enable = false,
.accl_fifo_enable = false,
.temp_fifo_enable = false,
@@ -370,20 +370,20 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
u8 d;
struct inv_mpu6050_state *st = iio_priv(indio_dev);
- result = inv_mpu6050_set_gyro_fsr(st, INV_MPU6050_FSR_2000DPS);
+ result = inv_mpu6050_set_gyro_fsr(st, st->chip_config.fsr);
if (result)
return result;
- result = inv_mpu6050_set_lpf_regs(st, INV_MPU6050_FILTER_20HZ);
+ result = inv_mpu6050_set_lpf_regs(st, st->chip_config.lpf);
if (result)
return result;
- d = INV_MPU6050_FIFO_RATE_TO_DIVIDER(INV_MPU6050_INIT_FIFO_RATE);
+ d = st->chip_config.divider;
result = regmap_write(st->map, st->reg->sample_rate_div, d);
if (result)
return result;
- d = (INV_MPU6050_FS_02G << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
+ d = (st->chip_config.accl_fs << INV_MPU6050_ACCL_CONFIG_FSR_SHIFT);
result = regmap_write(st->map, st->reg->accl_config, d);
if (result)
return result;
@@ -392,9 +392,6 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
if (result)
return result;
- memcpy(&st->chip_config, hw_info[st->chip_type].config,
- sizeof(struct inv_mpu6050_chip_config));
-
/*
* Internal chip period is 1ms (1kHz).
* Let's use at the beginning the theorical value before measuring
@@ -1116,6 +1113,8 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
st->hw = &hw_info[st->chip_type];
st->reg = hw_info[st->chip_type].reg;
+ memcpy(&st->chip_config, hw_info[st->chip_type].config,
+ sizeof(st->chip_config));
/* check chip self-identification */
result = regmap_read(st->map, INV_MPU6050_REG_WHOAMI, ®val);
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index 9a81098a8b4d..d5edf903c076 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -321,7 +321,6 @@ struct inv_mpu6050_state {
#define INV_MPU6050_TS_PERIOD_JITTER 4
/* init parameters */
-#define INV_MPU6050_INIT_FIFO_RATE 50
#define INV_MPU6050_MAX_FIFO_RATE 1000
#define INV_MPU6050_MIN_FIFO_RATE 4
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 06/13] iio: imu: inv_mpu6050: add all signal path resets at init
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (4 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 05/13] iio: imu: inv_mpu6050: early init of chip_config for use at setup Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-19 14:39 ` [PATCH v2 07/13] iio: imu: inv_mpu6050: fix sleep time when turning regulators on Jean-Baptiste Maneyrol
` (6 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Old chips using spi require for a full reset to manually reset
all signal path. This does not harm when using i2c so do it
inconditionally. Exclude i2c only chips.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 18 ++++++++++++++++++
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 6 ++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 85872e55154f..3502b996671c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1146,6 +1146,24 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
if (result)
return result;
msleep(INV_MPU6050_POWER_UP_TIME);
+ switch (st->chip_type) {
+ case INV_MPU6000:
+ case INV_MPU6500:
+ case INV_MPU6515:
+ case INV_MPU9250:
+ case INV_MPU9255:
+ /* reset signal path (required for spi connection) */
+ regval = INV_MPU6050_BIT_TEMP_RST | INV_MPU6050_BIT_ACCEL_RST |
+ INV_MPU6050_BIT_GYRO_RST;
+ result = regmap_write(st->map, INV_MPU6050_REG_SIGNAL_PATH_RESET,
+ regval);
+ if (result)
+ return result;
+ msleep(INV_MPU6050_POWER_UP_TIME);
+ break;
+ default:
+ break;
+ }
/*
* Turn power on. After reset, the sleep bit could be on
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index d5edf903c076..17f1f6a15f95 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -245,7 +245,13 @@ struct inv_mpu6050_state {
#define INV_MPU6050_BIT_I2C_SLV3_DLY_EN 0x08
#define INV_MPU6050_BIT_DELAY_ES_SHADOW 0x80
+#define INV_MPU6050_REG_SIGNAL_PATH_RESET 0x68
+#define INV_MPU6050_BIT_TEMP_RST BIT(0)
+#define INV_MPU6050_BIT_ACCEL_RST BIT(1)
+#define INV_MPU6050_BIT_GYRO_RST BIT(2)
+
#define INV_MPU6050_REG_USER_CTRL 0x6A
+#define INV_MPU6050_BIT_SIG_COND_RST 0x01
#define INV_MPU6050_BIT_FIFO_RST 0x04
#define INV_MPU6050_BIT_DMP_RST 0x08
#define INV_MPU6050_BIT_I2C_MST_EN 0x20
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v2 07/13] iio: imu: inv_mpu6050: fix sleep time when turning regulators on
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (5 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 06/13] iio: imu: inv_mpu6050: add all signal path resets at init Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:29 ` Jonathan Cameron
2020-02-19 14:39 ` [PATCH v2 08/13] iio: imu: inv_mpu6050: rewrite power and engine management Jean-Baptiste Maneyrol
` (5 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Turning vdd regulator on requires a consequent sleep for the
chip to power on correctly.
Turning vddio regulator is much faster.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 3 ++-
1 file changed, 2 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 3502b996671c..63cdde20df7e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1201,7 +1201,7 @@ static int inv_mpu_core_enable_regulator_vddio(struct inv_mpu6050_state *st)
"Failed to enable vddio regulator: %d\n", result);
} else {
/* Give the device a little bit of time to start up. */
- usleep_range(35000, 70000);
+ usleep_range(3000, 5000);
}
return result;
@@ -1321,6 +1321,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
dev_err(dev, "Failed to enable vdd regulator: %d\n", result);
return result;
}
+ msleep(INV_MPU6050_POWER_UP_TIME);
result = inv_mpu_core_enable_regulator_vddio(st);
if (result) {
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 07/13] iio: imu: inv_mpu6050: fix sleep time when turning regulators on
2020-02-19 14:39 ` [PATCH v2 07/13] iio: imu: inv_mpu6050: fix sleep time when turning regulators on Jean-Baptiste Maneyrol
@ 2020-02-21 11:29 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:29 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Wed, 19 Feb 2020 15:39:52 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Turning vdd regulator on requires a consequent sleep for the
> chip to power on correctly.
> Turning vddio regulator is much faster.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
I'm going to retitle this as reduced sleep time rather than fix.
The reason is to avoid it getting picked up as something we need
to backport.
Applied this and the ones before it I didn't reply to, to the togreg
branch of iio.git and pushed out as testing.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 3 ++-
> 1 file changed, 2 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 3502b996671c..63cdde20df7e 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -1201,7 +1201,7 @@ static int inv_mpu_core_enable_regulator_vddio(struct inv_mpu6050_state *st)
> "Failed to enable vddio regulator: %d\n", result);
> } else {
> /* Give the device a little bit of time to start up. */
> - usleep_range(35000, 70000);
> + usleep_range(3000, 5000);
> }
>
> return result;
> @@ -1321,6 +1321,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> dev_err(dev, "Failed to enable vdd regulator: %d\n", result);
> return result;
> }
> + msleep(INV_MPU6050_POWER_UP_TIME);
>
> result = inv_mpu_core_enable_regulator_vddio(st);
> if (result) {
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 08/13] iio: imu: inv_mpu6050: rewrite power and engine management
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (6 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 07/13] iio: imu: inv_mpu6050: fix sleep time when turning regulators on Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:31 ` Jonathan Cameron
2020-02-19 14:39 ` [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface Jean-Baptiste Maneyrol
` (4 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Rewrite clock management to use automatic clock switching
present since MPU6500.
Sensors engine management can now turn on or off a batch of
sensors which simplifies usage a lot.
Temperature sensor is now turned on/off depending on usage.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 264 +++++++++++++-----
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 24 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c | 12 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h | 2 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 91 +++---
5 files changed, 262 insertions(+), 131 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 63cdde20df7e..a51efc4c941b 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -99,9 +99,31 @@ static const struct inv_mpu6050_reg_map reg_set_6050 = {
};
static const struct inv_mpu6050_chip_config chip_config_6050 = {
+ .clk = INV_CLK_INTERNAL,
.fsr = INV_MPU6050_FSR_2000DPS,
.lpf = INV_MPU6050_FILTER_20HZ,
.divider = INV_MPU6050_FIFO_RATE_TO_DIVIDER(50),
+ .gyro_en = true,
+ .accl_en = true,
+ .temp_en = true,
+ .magn_en = false,
+ .gyro_fifo_enable = false,
+ .accl_fifo_enable = false,
+ .temp_fifo_enable = false,
+ .magn_fifo_enable = false,
+ .accl_fs = INV_MPU6050_FS_02G,
+ .user_ctrl = 0,
+};
+
+static const struct inv_mpu6050_chip_config chip_config_6500 = {
+ .clk = INV_CLK_PLL,
+ .fsr = INV_MPU6050_FSR_2000DPS,
+ .lpf = INV_MPU6050_FILTER_20HZ,
+ .divider = INV_MPU6050_FIFO_RATE_TO_DIVIDER(50),
+ .gyro_en = true,
+ .accl_en = true,
+ .temp_en = true,
+ .magn_en = false,
.gyro_fifo_enable = false,
.accl_fifo_enable = false,
.temp_fifo_enable = false,
@@ -124,7 +146,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_MPU6500_WHOAMI_VALUE,
.name = "MPU6500",
.reg = ®_set_6500,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 512,
.temp = {INV_MPU6500_TEMP_OFFSET, INV_MPU6500_TEMP_SCALE},
},
@@ -132,7 +154,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_MPU6515_WHOAMI_VALUE,
.name = "MPU6515",
.reg = ®_set_6500,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 512,
.temp = {INV_MPU6500_TEMP_OFFSET, INV_MPU6500_TEMP_SCALE},
},
@@ -156,7 +178,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_MPU9250_WHOAMI_VALUE,
.name = "MPU9250",
.reg = ®_set_6500,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 512,
.temp = {INV_MPU6500_TEMP_OFFSET, INV_MPU6500_TEMP_SCALE},
},
@@ -164,7 +186,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_MPU9255_WHOAMI_VALUE,
.name = "MPU9255",
.reg = ®_set_6500,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 512,
.temp = {INV_MPU6500_TEMP_OFFSET, INV_MPU6500_TEMP_SCALE},
},
@@ -172,7 +194,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_ICM20608_WHOAMI_VALUE,
.name = "ICM20608",
.reg = ®_set_6500,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 512,
.temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
},
@@ -180,7 +202,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_ICM20609_WHOAMI_VALUE,
.name = "ICM20609",
.reg = ®_set_6500,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 4 * 1024,
.temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
},
@@ -188,7 +210,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_ICM20689_WHOAMI_VALUE,
.name = "ICM20689",
.reg = ®_set_6500,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 4 * 1024,
.temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
},
@@ -196,15 +218,15 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_ICM20602_WHOAMI_VALUE,
.name = "ICM20602",
.reg = ®_set_icm20602,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 1008,
.temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
},
{
.whoami = INV_ICM20690_WHOAMI_VALUE,
.name = "ICM20690",
- .reg = ®_set_icm20602,
- .config = &chip_config_6050,
+ .reg = ®_set_6500,
+ .config = &chip_config_6500,
.fifo_size = 1024,
.temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
},
@@ -212,61 +234,162 @@ static const struct inv_mpu6050_hw hw_info[] = {
.whoami = INV_IAM20680_WHOAMI_VALUE,
.name = "IAM20680",
.reg = ®_set_6500,
- .config = &chip_config_6050,
+ .config = &chip_config_6500,
.fifo_size = 512,
.temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
},
};
-int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
+static int inv_mpu6050_pwr_mgmt_1_write(struct inv_mpu6050_state *st, bool sleep,
+ int clock, int temp_dis)
{
- unsigned int d, mgmt_1;
- int result;
- /*
- * switch clock needs to be careful. Only when gyro is on, can
- * clock source be switched to gyro. Otherwise, it must be set to
- * internal clock
- */
- if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
- result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
- if (result)
- return result;
+ u8 val;
+
+ if (clock < 0)
+ clock = st->chip_config.clk;
+ if (temp_dis < 0)
+ temp_dis = !st->chip_config.temp_en;
+
+ val = clock & INV_MPU6050_BIT_CLK_MASK;
+ if (temp_dis)
+ val |= INV_MPU6050_BIT_TEMP_DIS;
+ if (sleep)
+ val |= INV_MPU6050_BIT_SLEEP;
+
+ dev_dbg(regmap_get_device(st->map), "pwr_mgmt_1: 0x%x\n", val);
+ return regmap_write(st->map, st->reg->pwr_mgmt_1, val);
+}
+
+static int inv_mpu6050_clock_switch(struct inv_mpu6050_state *st,
+ unsigned int clock)
+{
+ int ret;
+
+ switch (st->chip_type) {
+ case INV_MPU6050:
+ case INV_MPU6000:
+ case INV_MPU9150:
+ /* old chips: switch clock manually */
+ ret = inv_mpu6050_pwr_mgmt_1_write(st, false, clock, -1);
+ if (ret)
+ return ret;
+ st->chip_config.clk = clock;
+ break;
+ default:
+ /* automatic clock switching, nothing to do */
+ break;
+ }
+
+ return 0;
+}
- mgmt_1 &= ~INV_MPU6050_BIT_CLK_MASK;
+int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
+ unsigned int mask)
+{
+ unsigned int sleep;
+ u8 pwr_mgmt2, user_ctrl;
+ int ret;
+
+ /* delete useless requests */
+ if (mask & INV_MPU6050_SENSOR_ACCL && en == st->chip_config.accl_en)
+ mask &= ~INV_MPU6050_SENSOR_ACCL;
+ if (mask & INV_MPU6050_SENSOR_GYRO && en == st->chip_config.gyro_en)
+ mask &= ~INV_MPU6050_SENSOR_GYRO;
+ if (mask & INV_MPU6050_SENSOR_TEMP && en == st->chip_config.temp_en)
+ mask &= ~INV_MPU6050_SENSOR_TEMP;
+ if (mask & INV_MPU6050_SENSOR_MAGN && en == st->chip_config.magn_en)
+ mask &= ~INV_MPU6050_SENSOR_MAGN;
+ if (mask == 0)
+ return 0;
+
+ /* turn on/off temperature sensor */
+ if (mask & INV_MPU6050_SENSOR_TEMP) {
+ ret = inv_mpu6050_pwr_mgmt_1_write(st, false, -1, !en);
+ if (ret)
+ return ret;
+ st->chip_config.temp_en = en;
}
- if ((mask == INV_MPU6050_BIT_PWR_GYRO_STBY) && (!en)) {
- /*
- * turning off gyro requires switch to internal clock first.
- * Then turn off gyro engine
- */
- mgmt_1 |= INV_CLK_INTERNAL;
- result = regmap_write(st->map, st->reg->pwr_mgmt_1, mgmt_1);
- if (result)
- return result;
+ /* update user_crtl for driving magnetometer */
+ if (mask & INV_MPU6050_SENSOR_MAGN) {
+ user_ctrl = st->chip_config.user_ctrl;
+ if (en)
+ user_ctrl |= INV_MPU6050_BIT_I2C_MST_EN;
+ else
+ user_ctrl &= ~INV_MPU6050_BIT_I2C_MST_EN;
+ ret = regmap_write(st->map, st->reg->user_ctrl, user_ctrl);
+ if (ret)
+ return ret;
+ st->chip_config.user_ctrl = user_ctrl;
+ st->chip_config.magn_en = en;
}
- result = regmap_read(st->map, st->reg->pwr_mgmt_2, &d);
- if (result)
- return result;
- if (en)
- d &= ~mask;
- else
- d |= mask;
- result = regmap_write(st->map, st->reg->pwr_mgmt_2, d);
- if (result)
- return result;
+ /* manage accel & gyro engines */
+ if (mask & (INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_GYRO)) {
+ /* compute power management 2 current value */
+ pwr_mgmt2 = 0;
+ if (!st->chip_config.accl_en)
+ pwr_mgmt2 |= INV_MPU6050_BIT_PWR_ACCL_STBY;
+ if (!st->chip_config.gyro_en)
+ pwr_mgmt2 |= INV_MPU6050_BIT_PWR_GYRO_STBY;
+
+ /* update to new requested value */
+ if (mask & INV_MPU6050_SENSOR_ACCL) {
+ if (en)
+ pwr_mgmt2 &= ~INV_MPU6050_BIT_PWR_ACCL_STBY;
+ else
+ pwr_mgmt2 |= INV_MPU6050_BIT_PWR_ACCL_STBY;
+ }
+ if (mask & INV_MPU6050_SENSOR_GYRO) {
+ if (en)
+ pwr_mgmt2 &= ~INV_MPU6050_BIT_PWR_GYRO_STBY;
+ else
+ pwr_mgmt2 |= INV_MPU6050_BIT_PWR_GYRO_STBY;
+ }
- if (en) {
- /* Wait for output to stabilize */
- msleep(INV_MPU6050_TEMP_UP_TIME);
- if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
- /* switch internal clock to PLL */
- mgmt_1 |= INV_CLK_PLL;
- result = regmap_write(st->map,
- st->reg->pwr_mgmt_1, mgmt_1);
- if (result)
- return result;
+ /* switch clock to internal when turning gyro off */
+ if (mask & INV_MPU6050_SENSOR_GYRO && !en) {
+ ret = inv_mpu6050_clock_switch(st, INV_CLK_INTERNAL);
+ if (ret)
+ return ret;
+ }
+
+ /* update sensors engine */
+ dev_dbg(regmap_get_device(st->map), "pwr_mgmt_2: 0x%x\n",
+ pwr_mgmt2);
+ ret = regmap_write(st->map, st->reg->pwr_mgmt_2, pwr_mgmt2);
+ if (ret)
+ return ret;
+ if (mask & INV_MPU6050_SENSOR_ACCL)
+ st->chip_config.accl_en = en;
+ if (mask & INV_MPU6050_SENSOR_GYRO)
+ st->chip_config.gyro_en = en;
+
+ /* compute required time to have sensors stabilized */
+ sleep = 0;
+ if (en) {
+ if (mask & INV_MPU6050_SENSOR_ACCL) {
+ if (sleep < INV_MPU6050_ACCEL_UP_TIME)
+ sleep = INV_MPU6050_ACCEL_UP_TIME;
+ }
+ if (mask & INV_MPU6050_SENSOR_GYRO) {
+ if (sleep < INV_MPU6050_GYRO_UP_TIME)
+ sleep = INV_MPU6050_GYRO_UP_TIME;
+ }
+ } else {
+ if (mask & INV_MPU6050_SENSOR_GYRO) {
+ if (sleep < INV_MPU6050_GYRO_DOWN_TIME)
+ sleep = INV_MPU6050_GYRO_DOWN_TIME;
+ }
+ }
+ if (sleep)
+ msleep(sleep);
+
+ /* switch clock to PLL when turning gyro on */
+ if (mask & INV_MPU6050_SENSOR_GYRO && en) {
+ ret = inv_mpu6050_clock_switch(st, INV_CLK_PLL);
+ if (ret)
+ return ret;
}
}
@@ -279,7 +402,7 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
if (power_on) {
if (!st->powerup_count) {
- result = regmap_write(st->map, st->reg->pwr_mgmt_1, 0);
+ result = inv_mpu6050_pwr_mgmt_1_write(st, false, -1, -1);
if (result)
return result;
usleep_range(INV_MPU6050_REG_UP_TIME_MIN,
@@ -288,8 +411,7 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
st->powerup_count++;
} else {
if (st->powerup_count == 1) {
- result = regmap_write(st->map, st->reg->pwr_mgmt_1,
- INV_MPU6050_BIT_SLEEP);
+ result = inv_mpu6050_pwr_mgmt_1_write(st, true, -1, -1);
if (result)
return result;
}
@@ -451,33 +573,41 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
switch (chan->type) {
case IIO_ANGL_VEL:
result = inv_mpu6050_switch_engine(st, true,
- INV_MPU6050_BIT_PWR_GYRO_STBY);
+ INV_MPU6050_SENSOR_GYRO);
if (result)
goto error_power_off;
ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
chan->channel2, val);
result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_BIT_PWR_GYRO_STBY);
+ INV_MPU6050_SENSOR_GYRO);
if (result)
goto error_power_off;
break;
case IIO_ACCEL:
result = inv_mpu6050_switch_engine(st, true,
- INV_MPU6050_BIT_PWR_ACCL_STBY);
+ INV_MPU6050_SENSOR_ACCL);
if (result)
goto error_power_off;
ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
chan->channel2, val);
result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_BIT_PWR_ACCL_STBY);
+ INV_MPU6050_SENSOR_ACCL);
if (result)
goto error_power_off;
break;
case IIO_TEMP:
+ result = inv_mpu6050_switch_engine(st, true,
+ INV_MPU6050_SENSOR_TEMP);
+ if (result)
+ goto error_power_off;
/* wait for stablization */
- msleep(INV_MPU6050_SENSOR_UP_TIME);
+ msleep(INV_MPU6050_TEMP_UP_TIME);
ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
IIO_MOD_X, val);
+ result = inv_mpu6050_switch_engine(st, false,
+ INV_MPU6050_SENSOR_TEMP);
+ if (result)
+ goto error_power_off;
break;
case IIO_MAGN:
ret = inv_mpu_magn_read(st, chan->channel2, val);
@@ -1108,7 +1238,7 @@ static const struct iio_info mpu_info = {
static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
{
int result;
- unsigned int regval;
+ unsigned int regval, mask;
int i;
st->hw = &hw_info[st->chip_type];
@@ -1174,13 +1304,9 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
result = inv_mpu6050_set_power_itg(st, true);
if (result)
return result;
-
- result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_BIT_PWR_ACCL_STBY);
- if (result)
- goto error_power_off;
- result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_BIT_PWR_GYRO_STBY);
+ mask = INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_GYRO |
+ INV_MPU6050_SENSOR_TEMP | INV_MPU6050_SENSOR_MAGN;
+ result = inv_mpu6050_switch_engine(st, false, mask);
if (result)
goto error_power_off;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index 17f1f6a15f95..a578789c9210 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -83,11 +83,22 @@ enum inv_devices {
INV_NUM_PARTS
};
+/* chip sensors mask: accelerometer, gyroscope, temperature, magnetometer */
+#define INV_MPU6050_SENSOR_ACCL BIT(0)
+#define INV_MPU6050_SENSOR_GYRO BIT(1)
+#define INV_MPU6050_SENSOR_TEMP BIT(2)
+#define INV_MPU6050_SENSOR_MAGN BIT(3)
+
/**
* struct inv_mpu6050_chip_config - Cached chip configuration data.
+ * @clk: selected chip clock
* @fsr: Full scale range.
* @lpf: Digital low pass filter frequency.
* @accl_fs: accel full scale range.
+ * @accl_en: accel engine enabled
+ * @gyro_en: gyro engine enabled
+ * @temp_en: temperature sensor enabled
+ * @magn_en: magn engine (i2c master) enabled
* @accl_fifo_enable: enable accel data output
* @gyro_fifo_enable: enable gyro data output
* @temp_fifo_enable: enable temp data output
@@ -95,9 +106,14 @@ enum inv_devices {
* @divider: chip sample rate divider (sample rate divider - 1)
*/
struct inv_mpu6050_chip_config {
+ unsigned int clk:3;
unsigned int fsr:2;
unsigned int lpf:3;
unsigned int accl_fs:2;
+ unsigned int accl_en:1;
+ unsigned int gyro_en:1;
+ unsigned int temp_en:1;
+ unsigned int magn_en:1;
unsigned int accl_fifo_enable:1;
unsigned int gyro_fifo_enable:1;
unsigned int temp_fifo_enable:1;
@@ -262,6 +278,7 @@ struct inv_mpu6050_state {
#define INV_MPU6050_REG_PWR_MGMT_1 0x6B
#define INV_MPU6050_BIT_H_RESET 0x80
#define INV_MPU6050_BIT_SLEEP 0x40
+#define INV_MPU6050_BIT_TEMP_DIS 0x08
#define INV_MPU6050_BIT_CLK_MASK 0x7
#define INV_MPU6050_REG_PWR_MGMT_2 0x6C
@@ -292,7 +309,9 @@ struct inv_mpu6050_state {
/* delay time in milliseconds */
#define INV_MPU6050_POWER_UP_TIME 100
#define INV_MPU6050_TEMP_UP_TIME 100
-#define INV_MPU6050_SENSOR_UP_TIME 30
+#define INV_MPU6050_ACCEL_UP_TIME 20
+#define INV_MPU6050_GYRO_UP_TIME 35
+#define INV_MPU6050_GYRO_DOWN_TIME 150
/* delay time in microseconds */
#define INV_MPU6050_REG_UP_TIME_MIN 5000
@@ -417,7 +436,8 @@ enum inv_mpu6050_clock_sel_e {
irqreturn_t inv_mpu6050_read_fifo(int irq, void *p);
int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type);
int inv_reset_fifo(struct iio_dev *indio_dev);
-int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask);
+int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
+ unsigned int mask);
int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 val);
int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on);
int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
index 607104a2631e..3f402fa56d95 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
@@ -316,9 +316,9 @@ int inv_mpu_magn_set_orient(struct inv_mpu6050_state *st)
*
* Returns 0 on success, a negative error code otherwise
*/
-int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val)
+int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
{
- unsigned int user_ctrl, status;
+ unsigned int status;
__be16 data;
uint8_t addr;
unsigned int freq_hz, period_ms;
@@ -350,16 +350,14 @@ int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val)
freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
period_ms = 1000 / freq_hz;
- /* start i2c master, wait for xfer, stop */
- user_ctrl = st->chip_config.user_ctrl | INV_MPU6050_BIT_I2C_MST_EN;
- ret = regmap_write(st->map, st->reg->user_ctrl, user_ctrl);
+ ret = inv_mpu6050_switch_engine(st, true, INV_MPU6050_SENSOR_MAGN);
if (ret)
return ret;
/* need to wait 2 periods + half-period margin */
msleep(period_ms * 2 + period_ms / 2);
- user_ctrl = st->chip_config.user_ctrl;
- ret = regmap_write(st->map, st->reg->user_ctrl, user_ctrl);
+
+ ret = inv_mpu6050_switch_engine(st, false, INV_MPU6050_SENSOR_MAGN);
if (ret)
return ret;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
index b41bd0578478..f7ad50ca6c1b 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
@@ -31,6 +31,6 @@ int inv_mpu_magn_set_rate(const struct inv_mpu6050_state *st, int fifo_rate);
int inv_mpu_magn_set_orient(struct inv_mpu6050_state *st);
-int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val);
+int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val);
#endif /* INV_MPU_MAGN_H_ */
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index 5199fe790c30..cfd7243159f6 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -5,9 +5,10 @@
#include "inv_mpu_iio.h"
-static void inv_scan_query_mpu6050(struct iio_dev *indio_dev)
+static unsigned int inv_scan_query_mpu6050(struct iio_dev *indio_dev)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
+ unsigned int mask;
st->chip_config.gyro_fifo_enable =
test_bit(INV_MPU6050_SCAN_GYRO_X,
@@ -27,17 +28,28 @@ static void inv_scan_query_mpu6050(struct iio_dev *indio_dev)
st->chip_config.temp_fifo_enable =
test_bit(INV_MPU6050_SCAN_TEMP, indio_dev->active_scan_mask);
+
+ mask = 0;
+ if (st->chip_config.gyro_fifo_enable)
+ mask |= INV_MPU6050_SENSOR_GYRO;
+ if (st->chip_config.accl_fifo_enable)
+ mask |= INV_MPU6050_SENSOR_ACCL;
+ if (st->chip_config.temp_fifo_enable)
+ mask |= INV_MPU6050_SENSOR_TEMP;
+
+ return mask;
}
-static void inv_scan_query_mpu9x50(struct iio_dev *indio_dev)
+static unsigned int inv_scan_query_mpu9x50(struct iio_dev *indio_dev)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
+ unsigned int mask;
- inv_scan_query_mpu6050(indio_dev);
+ mask = inv_scan_query_mpu6050(indio_dev);
/* no magnetometer if i2c auxiliary bus is used */
if (st->magn_disabled)
- return;
+ return mask;
st->chip_config.magn_fifo_enable =
test_bit(INV_MPU9X50_SCAN_MAGN_X,
@@ -46,9 +58,13 @@ static void inv_scan_query_mpu9x50(struct iio_dev *indio_dev)
indio_dev->active_scan_mask) ||
test_bit(INV_MPU9X50_SCAN_MAGN_Z,
indio_dev->active_scan_mask);
+ if (st->chip_config.magn_fifo_enable)
+ mask |= INV_MPU6050_SENSOR_MAGN;
+
+ return mask;
}
-static void inv_scan_query(struct iio_dev *indio_dev)
+static unsigned int inv_scan_query(struct iio_dev *indio_dev)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
@@ -92,62 +108,40 @@ static unsigned int inv_compute_skip_samples(const struct inv_mpu6050_state *st)
static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
- uint8_t d;
+ unsigned int scan;
int result;
+ scan = inv_scan_query(indio_dev);
+
if (enable) {
result = inv_mpu6050_set_power_itg(st, true);
if (result)
return result;
- inv_scan_query(indio_dev);
- if (st->chip_config.gyro_fifo_enable) {
- result = inv_mpu6050_switch_engine(st, true,
- INV_MPU6050_BIT_PWR_GYRO_STBY);
- if (result)
- goto error_power_off;
- }
- if (st->chip_config.accl_fifo_enable) {
- result = inv_mpu6050_switch_engine(st, true,
- INV_MPU6050_BIT_PWR_ACCL_STBY);
- if (result)
- goto error_gyro_off;
- }
- if (st->chip_config.magn_fifo_enable) {
- d = st->chip_config.user_ctrl |
- INV_MPU6050_BIT_I2C_MST_EN;
- result = regmap_write(st->map, st->reg->user_ctrl, d);
- if (result)
- goto error_accl_off;
- st->chip_config.user_ctrl = d;
- }
+ result = inv_mpu6050_switch_engine(st, true, scan);
+ if (result)
+ goto error_power_off;
st->skip_samples = inv_compute_skip_samples(st);
result = inv_reset_fifo(indio_dev);
if (result)
- goto error_magn_off;
+ goto error_sensors_off;
} else {
result = regmap_write(st->map, st->reg->fifo_en, 0);
if (result)
- goto error_magn_off;
+ goto error_fifo_off;
result = regmap_write(st->map, st->reg->int_enable, 0);
if (result)
- goto error_magn_off;
-
- d = st->chip_config.user_ctrl & ~INV_MPU6050_BIT_I2C_MST_EN;
- result = regmap_write(st->map, st->reg->user_ctrl, d);
- if (result)
- goto error_magn_off;
- st->chip_config.user_ctrl = d;
+ goto error_fifo_off;
- result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_BIT_PWR_ACCL_STBY);
+ /* restore user_ctrl for disabling FIFO reading */
+ result = regmap_write(st->map, st->reg->user_ctrl,
+ st->chip_config.user_ctrl);
if (result)
- goto error_accl_off;
+ goto error_sensors_off;
- result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_BIT_PWR_GYRO_STBY);
+ result = inv_mpu6050_switch_engine(st, false, scan);
if (result)
- goto error_gyro_off;
+ goto error_power_off;
result = inv_mpu6050_set_power_itg(st, false);
if (result)
@@ -156,18 +150,11 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
return 0;
-error_magn_off:
+error_fifo_off:
/* always restore user_ctrl to disable fifo properly */
- st->chip_config.user_ctrl &= ~INV_MPU6050_BIT_I2C_MST_EN;
regmap_write(st->map, st->reg->user_ctrl, st->chip_config.user_ctrl);
-error_accl_off:
- if (st->chip_config.accl_fifo_enable)
- inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_BIT_PWR_ACCL_STBY);
-error_gyro_off:
- if (st->chip_config.gyro_fifo_enable)
- inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_BIT_PWR_GYRO_STBY);
+error_sensors_off:
+ inv_mpu6050_switch_engine(st, false, scan);
error_power_off:
inv_mpu6050_set_power_itg(st, false);
return result;
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 08/13] iio: imu: inv_mpu6050: rewrite power and engine management
2020-02-19 14:39 ` [PATCH v2 08/13] iio: imu: inv_mpu6050: rewrite power and engine management Jean-Baptiste Maneyrol
@ 2020-02-21 11:31 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:31 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Wed, 19 Feb 2020 15:39:53 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Rewrite clock management to use automatic clock switching
> present since MPU6500.
> Sensors engine management can now turn on or off a batch of
> sensors which simplifies usage a lot.
> Temperature sensor is now turned on/off depending on usage.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 264 +++++++++++++-----
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 24 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c | 12 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h | 2 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 91 +++---
> 5 files changed, 262 insertions(+), 131 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 63cdde20df7e..a51efc4c941b 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -99,9 +99,31 @@ static const struct inv_mpu6050_reg_map reg_set_6050 = {
> };
>
> static const struct inv_mpu6050_chip_config chip_config_6050 = {
> + .clk = INV_CLK_INTERNAL,
> .fsr = INV_MPU6050_FSR_2000DPS,
> .lpf = INV_MPU6050_FILTER_20HZ,
> .divider = INV_MPU6050_FIFO_RATE_TO_DIVIDER(50),
> + .gyro_en = true,
> + .accl_en = true,
> + .temp_en = true,
> + .magn_en = false,
> + .gyro_fifo_enable = false,
> + .accl_fifo_enable = false,
> + .temp_fifo_enable = false,
> + .magn_fifo_enable = false,
> + .accl_fs = INV_MPU6050_FS_02G,
> + .user_ctrl = 0,
> +};
> +
> +static const struct inv_mpu6050_chip_config chip_config_6500 = {
> + .clk = INV_CLK_PLL,
> + .fsr = INV_MPU6050_FSR_2000DPS,
> + .lpf = INV_MPU6050_FILTER_20HZ,
> + .divider = INV_MPU6050_FIFO_RATE_TO_DIVIDER(50),
> + .gyro_en = true,
> + .accl_en = true,
> + .temp_en = true,
> + .magn_en = false,
> .gyro_fifo_enable = false,
> .accl_fifo_enable = false,
> .temp_fifo_enable = false,
> @@ -124,7 +146,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_MPU6500_WHOAMI_VALUE,
> .name = "MPU6500",
> .reg = ®_set_6500,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 512,
> .temp = {INV_MPU6500_TEMP_OFFSET, INV_MPU6500_TEMP_SCALE},
> },
> @@ -132,7 +154,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_MPU6515_WHOAMI_VALUE,
> .name = "MPU6515",
> .reg = ®_set_6500,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 512,
> .temp = {INV_MPU6500_TEMP_OFFSET, INV_MPU6500_TEMP_SCALE},
> },
> @@ -156,7 +178,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_MPU9250_WHOAMI_VALUE,
> .name = "MPU9250",
> .reg = ®_set_6500,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 512,
> .temp = {INV_MPU6500_TEMP_OFFSET, INV_MPU6500_TEMP_SCALE},
> },
> @@ -164,7 +186,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_MPU9255_WHOAMI_VALUE,
> .name = "MPU9255",
> .reg = ®_set_6500,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 512,
> .temp = {INV_MPU6500_TEMP_OFFSET, INV_MPU6500_TEMP_SCALE},
> },
> @@ -172,7 +194,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_ICM20608_WHOAMI_VALUE,
> .name = "ICM20608",
> .reg = ®_set_6500,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 512,
> .temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
> },
> @@ -180,7 +202,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_ICM20609_WHOAMI_VALUE,
> .name = "ICM20609",
> .reg = ®_set_6500,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 4 * 1024,
> .temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
> },
> @@ -188,7 +210,7 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_ICM20689_WHOAMI_VALUE,
> .name = "ICM20689",
> .reg = ®_set_6500,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 4 * 1024,
> .temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
> },
> @@ -196,15 +218,15 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_ICM20602_WHOAMI_VALUE,
> .name = "ICM20602",
> .reg = ®_set_icm20602,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 1008,
> .temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
> },
> {
> .whoami = INV_ICM20690_WHOAMI_VALUE,
> .name = "ICM20690",
> - .reg = ®_set_icm20602,
> - .config = &chip_config_6050,
> + .reg = ®_set_6500,
> + .config = &chip_config_6500,
> .fifo_size = 1024,
> .temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
> },
> @@ -212,61 +234,162 @@ static const struct inv_mpu6050_hw hw_info[] = {
> .whoami = INV_IAM20680_WHOAMI_VALUE,
> .name = "IAM20680",
> .reg = ®_set_6500,
> - .config = &chip_config_6050,
> + .config = &chip_config_6500,
> .fifo_size = 512,
> .temp = {INV_ICM20608_TEMP_OFFSET, INV_ICM20608_TEMP_SCALE},
> },
> };
>
> -int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
> +static int inv_mpu6050_pwr_mgmt_1_write(struct inv_mpu6050_state *st, bool sleep,
> + int clock, int temp_dis)
> {
> - unsigned int d, mgmt_1;
> - int result;
> - /*
> - * switch clock needs to be careful. Only when gyro is on, can
> - * clock source be switched to gyro. Otherwise, it must be set to
> - * internal clock
> - */
> - if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
> - result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
> - if (result)
> - return result;
> + u8 val;
> +
> + if (clock < 0)
> + clock = st->chip_config.clk;
> + if (temp_dis < 0)
> + temp_dis = !st->chip_config.temp_en;
> +
> + val = clock & INV_MPU6050_BIT_CLK_MASK;
> + if (temp_dis)
> + val |= INV_MPU6050_BIT_TEMP_DIS;
> + if (sleep)
> + val |= INV_MPU6050_BIT_SLEEP;
> +
> + dev_dbg(regmap_get_device(st->map), "pwr_mgmt_1: 0x%x\n", val);
> + return regmap_write(st->map, st->reg->pwr_mgmt_1, val);
> +}
> +
> +static int inv_mpu6050_clock_switch(struct inv_mpu6050_state *st,
> + unsigned int clock)
> +{
> + int ret;
> +
> + switch (st->chip_type) {
> + case INV_MPU6050:
> + case INV_MPU6000:
> + case INV_MPU9150:
> + /* old chips: switch clock manually */
> + ret = inv_mpu6050_pwr_mgmt_1_write(st, false, clock, -1);
> + if (ret)
> + return ret;
> + st->chip_config.clk = clock;
> + break;
> + default:
> + /* automatic clock switching, nothing to do */
> + break;
> + }
> +
> + return 0;
> +}
>
> - mgmt_1 &= ~INV_MPU6050_BIT_CLK_MASK;
> +int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
> + unsigned int mask)
> +{
> + unsigned int sleep;
> + u8 pwr_mgmt2, user_ctrl;
> + int ret;
> +
> + /* delete useless requests */
> + if (mask & INV_MPU6050_SENSOR_ACCL && en == st->chip_config.accl_en)
> + mask &= ~INV_MPU6050_SENSOR_ACCL;
> + if (mask & INV_MPU6050_SENSOR_GYRO && en == st->chip_config.gyro_en)
> + mask &= ~INV_MPU6050_SENSOR_GYRO;
> + if (mask & INV_MPU6050_SENSOR_TEMP && en == st->chip_config.temp_en)
> + mask &= ~INV_MPU6050_SENSOR_TEMP;
> + if (mask & INV_MPU6050_SENSOR_MAGN && en == st->chip_config.magn_en)
> + mask &= ~INV_MPU6050_SENSOR_MAGN;
> + if (mask == 0)
> + return 0;
> +
> + /* turn on/off temperature sensor */
> + if (mask & INV_MPU6050_SENSOR_TEMP) {
> + ret = inv_mpu6050_pwr_mgmt_1_write(st, false, -1, !en);
> + if (ret)
> + return ret;
> + st->chip_config.temp_en = en;
> }
>
> - if ((mask == INV_MPU6050_BIT_PWR_GYRO_STBY) && (!en)) {
> - /*
> - * turning off gyro requires switch to internal clock first.
> - * Then turn off gyro engine
> - */
> - mgmt_1 |= INV_CLK_INTERNAL;
> - result = regmap_write(st->map, st->reg->pwr_mgmt_1, mgmt_1);
> - if (result)
> - return result;
> + /* update user_crtl for driving magnetometer */
> + if (mask & INV_MPU6050_SENSOR_MAGN) {
> + user_ctrl = st->chip_config.user_ctrl;
> + if (en)
> + user_ctrl |= INV_MPU6050_BIT_I2C_MST_EN;
> + else
> + user_ctrl &= ~INV_MPU6050_BIT_I2C_MST_EN;
> + ret = regmap_write(st->map, st->reg->user_ctrl, user_ctrl);
> + if (ret)
> + return ret;
> + st->chip_config.user_ctrl = user_ctrl;
> + st->chip_config.magn_en = en;
> }
>
> - result = regmap_read(st->map, st->reg->pwr_mgmt_2, &d);
> - if (result)
> - return result;
> - if (en)
> - d &= ~mask;
> - else
> - d |= mask;
> - result = regmap_write(st->map, st->reg->pwr_mgmt_2, d);
> - if (result)
> - return result;
> + /* manage accel & gyro engines */
> + if (mask & (INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_GYRO)) {
> + /* compute power management 2 current value */
> + pwr_mgmt2 = 0;
> + if (!st->chip_config.accl_en)
> + pwr_mgmt2 |= INV_MPU6050_BIT_PWR_ACCL_STBY;
> + if (!st->chip_config.gyro_en)
> + pwr_mgmt2 |= INV_MPU6050_BIT_PWR_GYRO_STBY;
> +
> + /* update to new requested value */
> + if (mask & INV_MPU6050_SENSOR_ACCL) {
> + if (en)
> + pwr_mgmt2 &= ~INV_MPU6050_BIT_PWR_ACCL_STBY;
> + else
> + pwr_mgmt2 |= INV_MPU6050_BIT_PWR_ACCL_STBY;
> + }
> + if (mask & INV_MPU6050_SENSOR_GYRO) {
> + if (en)
> + pwr_mgmt2 &= ~INV_MPU6050_BIT_PWR_GYRO_STBY;
> + else
> + pwr_mgmt2 |= INV_MPU6050_BIT_PWR_GYRO_STBY;
> + }
>
> - if (en) {
> - /* Wait for output to stabilize */
> - msleep(INV_MPU6050_TEMP_UP_TIME);
> - if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
> - /* switch internal clock to PLL */
> - mgmt_1 |= INV_CLK_PLL;
> - result = regmap_write(st->map,
> - st->reg->pwr_mgmt_1, mgmt_1);
> - if (result)
> - return result;
> + /* switch clock to internal when turning gyro off */
> + if (mask & INV_MPU6050_SENSOR_GYRO && !en) {
> + ret = inv_mpu6050_clock_switch(st, INV_CLK_INTERNAL);
> + if (ret)
> + return ret;
> + }
> +
> + /* update sensors engine */
> + dev_dbg(regmap_get_device(st->map), "pwr_mgmt_2: 0x%x\n",
> + pwr_mgmt2);
> + ret = regmap_write(st->map, st->reg->pwr_mgmt_2, pwr_mgmt2);
> + if (ret)
> + return ret;
> + if (mask & INV_MPU6050_SENSOR_ACCL)
> + st->chip_config.accl_en = en;
> + if (mask & INV_MPU6050_SENSOR_GYRO)
> + st->chip_config.gyro_en = en;
> +
> + /* compute required time to have sensors stabilized */
> + sleep = 0;
> + if (en) {
> + if (mask & INV_MPU6050_SENSOR_ACCL) {
> + if (sleep < INV_MPU6050_ACCEL_UP_TIME)
> + sleep = INV_MPU6050_ACCEL_UP_TIME;
> + }
> + if (mask & INV_MPU6050_SENSOR_GYRO) {
> + if (sleep < INV_MPU6050_GYRO_UP_TIME)
> + sleep = INV_MPU6050_GYRO_UP_TIME;
> + }
> + } else {
> + if (mask & INV_MPU6050_SENSOR_GYRO) {
> + if (sleep < INV_MPU6050_GYRO_DOWN_TIME)
> + sleep = INV_MPU6050_GYRO_DOWN_TIME;
> + }
> + }
> + if (sleep)
> + msleep(sleep);
> +
> + /* switch clock to PLL when turning gyro on */
> + if (mask & INV_MPU6050_SENSOR_GYRO && en) {
> + ret = inv_mpu6050_clock_switch(st, INV_CLK_PLL);
> + if (ret)
> + return ret;
> }
> }
>
> @@ -279,7 +402,7 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
>
> if (power_on) {
> if (!st->powerup_count) {
> - result = regmap_write(st->map, st->reg->pwr_mgmt_1, 0);
> + result = inv_mpu6050_pwr_mgmt_1_write(st, false, -1, -1);
> if (result)
> return result;
> usleep_range(INV_MPU6050_REG_UP_TIME_MIN,
> @@ -288,8 +411,7 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
> st->powerup_count++;
> } else {
> if (st->powerup_count == 1) {
> - result = regmap_write(st->map, st->reg->pwr_mgmt_1,
> - INV_MPU6050_BIT_SLEEP);
> + result = inv_mpu6050_pwr_mgmt_1_write(st, true, -1, -1);
> if (result)
> return result;
> }
> @@ -451,33 +573,41 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> switch (chan->type) {
> case IIO_ANGL_VEL:
> result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_BIT_PWR_GYRO_STBY);
> + INV_MPU6050_SENSOR_GYRO);
> if (result)
> goto error_power_off;
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
> chan->channel2, val);
> result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_BIT_PWR_GYRO_STBY);
> + INV_MPU6050_SENSOR_GYRO);
> if (result)
> goto error_power_off;
> break;
> case IIO_ACCEL:
> result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_BIT_PWR_ACCL_STBY);
> + INV_MPU6050_SENSOR_ACCL);
> if (result)
> goto error_power_off;
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
> chan->channel2, val);
> result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_BIT_PWR_ACCL_STBY);
> + INV_MPU6050_SENSOR_ACCL);
> if (result)
> goto error_power_off;
> break;
> case IIO_TEMP:
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_TEMP);
> + if (result)
> + goto error_power_off;
> /* wait for stablization */
> - msleep(INV_MPU6050_SENSOR_UP_TIME);
> + msleep(INV_MPU6050_TEMP_UP_TIME);
> ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
> IIO_MOD_X, val);
> + result = inv_mpu6050_switch_engine(st, false,
> + INV_MPU6050_SENSOR_TEMP);
> + if (result)
> + goto error_power_off;
> break;
> case IIO_MAGN:
> ret = inv_mpu_magn_read(st, chan->channel2, val);
> @@ -1108,7 +1238,7 @@ static const struct iio_info mpu_info = {
> static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
> {
> int result;
> - unsigned int regval;
> + unsigned int regval, mask;
> int i;
>
> st->hw = &hw_info[st->chip_type];
> @@ -1174,13 +1304,9 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
> result = inv_mpu6050_set_power_itg(st, true);
> if (result)
> return result;
> -
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_BIT_PWR_ACCL_STBY);
> - if (result)
> - goto error_power_off;
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_BIT_PWR_GYRO_STBY);
> + mask = INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_GYRO |
> + INV_MPU6050_SENSOR_TEMP | INV_MPU6050_SENSOR_MAGN;
> + result = inv_mpu6050_switch_engine(st, false, mask);
> if (result)
> goto error_power_off;
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 17f1f6a15f95..a578789c9210 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -83,11 +83,22 @@ enum inv_devices {
> INV_NUM_PARTS
> };
>
> +/* chip sensors mask: accelerometer, gyroscope, temperature, magnetometer */
> +#define INV_MPU6050_SENSOR_ACCL BIT(0)
> +#define INV_MPU6050_SENSOR_GYRO BIT(1)
> +#define INV_MPU6050_SENSOR_TEMP BIT(2)
> +#define INV_MPU6050_SENSOR_MAGN BIT(3)
> +
> /**
> * struct inv_mpu6050_chip_config - Cached chip configuration data.
> + * @clk: selected chip clock
> * @fsr: Full scale range.
> * @lpf: Digital low pass filter frequency.
> * @accl_fs: accel full scale range.
> + * @accl_en: accel engine enabled
> + * @gyro_en: gyro engine enabled
> + * @temp_en: temperature sensor enabled
> + * @magn_en: magn engine (i2c master) enabled
> * @accl_fifo_enable: enable accel data output
> * @gyro_fifo_enable: enable gyro data output
> * @temp_fifo_enable: enable temp data output
> @@ -95,9 +106,14 @@ enum inv_devices {
> * @divider: chip sample rate divider (sample rate divider - 1)
> */
> struct inv_mpu6050_chip_config {
> + unsigned int clk:3;
> unsigned int fsr:2;
> unsigned int lpf:3;
> unsigned int accl_fs:2;
> + unsigned int accl_en:1;
> + unsigned int gyro_en:1;
> + unsigned int temp_en:1;
> + unsigned int magn_en:1;
> unsigned int accl_fifo_enable:1;
> unsigned int gyro_fifo_enable:1;
> unsigned int temp_fifo_enable:1;
> @@ -262,6 +278,7 @@ struct inv_mpu6050_state {
> #define INV_MPU6050_REG_PWR_MGMT_1 0x6B
> #define INV_MPU6050_BIT_H_RESET 0x80
> #define INV_MPU6050_BIT_SLEEP 0x40
> +#define INV_MPU6050_BIT_TEMP_DIS 0x08
> #define INV_MPU6050_BIT_CLK_MASK 0x7
>
> #define INV_MPU6050_REG_PWR_MGMT_2 0x6C
> @@ -292,7 +309,9 @@ struct inv_mpu6050_state {
> /* delay time in milliseconds */
> #define INV_MPU6050_POWER_UP_TIME 100
> #define INV_MPU6050_TEMP_UP_TIME 100
> -#define INV_MPU6050_SENSOR_UP_TIME 30
> +#define INV_MPU6050_ACCEL_UP_TIME 20
> +#define INV_MPU6050_GYRO_UP_TIME 35
> +#define INV_MPU6050_GYRO_DOWN_TIME 150
>
> /* delay time in microseconds */
> #define INV_MPU6050_REG_UP_TIME_MIN 5000
> @@ -417,7 +436,8 @@ enum inv_mpu6050_clock_sel_e {
> irqreturn_t inv_mpu6050_read_fifo(int irq, void *p);
> int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type);
> int inv_reset_fifo(struct iio_dev *indio_dev);
> -int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask);
> +int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
> + unsigned int mask);
> int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 val);
> int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on);
> int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> index 607104a2631e..3f402fa56d95 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> @@ -316,9 +316,9 @@ int inv_mpu_magn_set_orient(struct inv_mpu6050_state *st)
> *
> * Returns 0 on success, a negative error code otherwise
> */
> -int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val)
> +int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
> {
> - unsigned int user_ctrl, status;
> + unsigned int status;
> __be16 data;
> uint8_t addr;
> unsigned int freq_hz, period_ms;
> @@ -350,16 +350,14 @@ int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val)
> freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> period_ms = 1000 / freq_hz;
>
> - /* start i2c master, wait for xfer, stop */
> - user_ctrl = st->chip_config.user_ctrl | INV_MPU6050_BIT_I2C_MST_EN;
> - ret = regmap_write(st->map, st->reg->user_ctrl, user_ctrl);
> + ret = inv_mpu6050_switch_engine(st, true, INV_MPU6050_SENSOR_MAGN);
> if (ret)
> return ret;
>
> /* need to wait 2 periods + half-period margin */
> msleep(period_ms * 2 + period_ms / 2);
> - user_ctrl = st->chip_config.user_ctrl;
> - ret = regmap_write(st->map, st->reg->user_ctrl, user_ctrl);
> +
> + ret = inv_mpu6050_switch_engine(st, false, INV_MPU6050_SENSOR_MAGN);
> if (ret)
> return ret;
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> index b41bd0578478..f7ad50ca6c1b 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> @@ -31,6 +31,6 @@ int inv_mpu_magn_set_rate(const struct inv_mpu6050_state *st, int fifo_rate);
>
> int inv_mpu_magn_set_orient(struct inv_mpu6050_state *st);
>
> -int inv_mpu_magn_read(const struct inv_mpu6050_state *st, int axis, int *val);
> +int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val);
>
> #endif /* INV_MPU_MAGN_H_ */
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index 5199fe790c30..cfd7243159f6 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -5,9 +5,10 @@
>
> #include "inv_mpu_iio.h"
>
> -static void inv_scan_query_mpu6050(struct iio_dev *indio_dev)
> +static unsigned int inv_scan_query_mpu6050(struct iio_dev *indio_dev)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + unsigned int mask;
>
> st->chip_config.gyro_fifo_enable =
> test_bit(INV_MPU6050_SCAN_GYRO_X,
> @@ -27,17 +28,28 @@ static void inv_scan_query_mpu6050(struct iio_dev *indio_dev)
>
> st->chip_config.temp_fifo_enable =
> test_bit(INV_MPU6050_SCAN_TEMP, indio_dev->active_scan_mask);
> +
> + mask = 0;
> + if (st->chip_config.gyro_fifo_enable)
> + mask |= INV_MPU6050_SENSOR_GYRO;
> + if (st->chip_config.accl_fifo_enable)
> + mask |= INV_MPU6050_SENSOR_ACCL;
> + if (st->chip_config.temp_fifo_enable)
> + mask |= INV_MPU6050_SENSOR_TEMP;
> +
> + return mask;
> }
>
> -static void inv_scan_query_mpu9x50(struct iio_dev *indio_dev)
> +static unsigned int inv_scan_query_mpu9x50(struct iio_dev *indio_dev)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + unsigned int mask;
>
> - inv_scan_query_mpu6050(indio_dev);
> + mask = inv_scan_query_mpu6050(indio_dev);
>
> /* no magnetometer if i2c auxiliary bus is used */
> if (st->magn_disabled)
> - return;
> + return mask;
>
> st->chip_config.magn_fifo_enable =
> test_bit(INV_MPU9X50_SCAN_MAGN_X,
> @@ -46,9 +58,13 @@ static void inv_scan_query_mpu9x50(struct iio_dev *indio_dev)
> indio_dev->active_scan_mask) ||
> test_bit(INV_MPU9X50_SCAN_MAGN_Z,
> indio_dev->active_scan_mask);
> + if (st->chip_config.magn_fifo_enable)
> + mask |= INV_MPU6050_SENSOR_MAGN;
> +
> + return mask;
> }
>
> -static void inv_scan_query(struct iio_dev *indio_dev)
> +static unsigned int inv_scan_query(struct iio_dev *indio_dev)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>
> @@ -92,62 +108,40 @@ static unsigned int inv_compute_skip_samples(const struct inv_mpu6050_state *st)
> static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> - uint8_t d;
> + unsigned int scan;
> int result;
>
> + scan = inv_scan_query(indio_dev);
> +
> if (enable) {
> result = inv_mpu6050_set_power_itg(st, true);
> if (result)
> return result;
> - inv_scan_query(indio_dev);
> - if (st->chip_config.gyro_fifo_enable) {
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_BIT_PWR_GYRO_STBY);
> - if (result)
> - goto error_power_off;
> - }
> - if (st->chip_config.accl_fifo_enable) {
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_BIT_PWR_ACCL_STBY);
> - if (result)
> - goto error_gyro_off;
> - }
> - if (st->chip_config.magn_fifo_enable) {
> - d = st->chip_config.user_ctrl |
> - INV_MPU6050_BIT_I2C_MST_EN;
> - result = regmap_write(st->map, st->reg->user_ctrl, d);
> - if (result)
> - goto error_accl_off;
> - st->chip_config.user_ctrl = d;
> - }
> + result = inv_mpu6050_switch_engine(st, true, scan);
> + if (result)
> + goto error_power_off;
> st->skip_samples = inv_compute_skip_samples(st);
> result = inv_reset_fifo(indio_dev);
> if (result)
> - goto error_magn_off;
> + goto error_sensors_off;
> } else {
> result = regmap_write(st->map, st->reg->fifo_en, 0);
> if (result)
> - goto error_magn_off;
> + goto error_fifo_off;
>
> result = regmap_write(st->map, st->reg->int_enable, 0);
> if (result)
> - goto error_magn_off;
> -
> - d = st->chip_config.user_ctrl & ~INV_MPU6050_BIT_I2C_MST_EN;
> - result = regmap_write(st->map, st->reg->user_ctrl, d);
> - if (result)
> - goto error_magn_off;
> - st->chip_config.user_ctrl = d;
> + goto error_fifo_off;
>
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_BIT_PWR_ACCL_STBY);
> + /* restore user_ctrl for disabling FIFO reading */
> + result = regmap_write(st->map, st->reg->user_ctrl,
> + st->chip_config.user_ctrl);
> if (result)
> - goto error_accl_off;
> + goto error_sensors_off;
>
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_BIT_PWR_GYRO_STBY);
> + result = inv_mpu6050_switch_engine(st, false, scan);
> if (result)
> - goto error_gyro_off;
> + goto error_power_off;
>
> result = inv_mpu6050_set_power_itg(st, false);
> if (result)
> @@ -156,18 +150,11 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
>
> return 0;
>
> -error_magn_off:
> +error_fifo_off:
> /* always restore user_ctrl to disable fifo properly */
> - st->chip_config.user_ctrl &= ~INV_MPU6050_BIT_I2C_MST_EN;
> regmap_write(st->map, st->reg->user_ctrl, st->chip_config.user_ctrl);
> -error_accl_off:
> - if (st->chip_config.accl_fifo_enable)
> - inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_BIT_PWR_ACCL_STBY);
> -error_gyro_off:
> - if (st->chip_config.gyro_fifo_enable)
> - inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_BIT_PWR_GYRO_STBY);
> +error_sensors_off:
> + inv_mpu6050_switch_engine(st, false, scan);
> error_power_off:
> inv_mpu6050_set_power_itg(st, false);
> return result;
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (7 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 08/13] iio: imu: inv_mpu6050: rewrite power and engine management Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:34 ` Jonathan Cameron
2020-02-19 14:39 ` [PATCH v2 10/13] iio: imu: inv_mpu6050: factorize fifo enable/disable Jean-Baptiste Maneyrol
` (3 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
When reading data with the polling interface, we need to wait
at 1 sampling period to have a sample.
For gyroscope and magnetometer, we need to wait for 2 periods
before having a correct sample.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 36 ++++++++++++++++++++--
drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c | 21 -------------
drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h | 3 ++
3 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index a51efc4c941b..aeee39696d3a 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -563,9 +563,14 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
int *val)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
+ unsigned int freq_hz, period_us, min_sleep_us, max_sleep_us;
int result;
int ret;
+ /* compute sample period */
+ freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
+ period_us = 1000000 / freq_hz;
+
result = inv_mpu6050_set_power_itg(st, true);
if (result)
return result;
@@ -576,6 +581,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
INV_MPU6050_SENSOR_GYRO);
if (result)
goto error_power_off;
+ /* need to wait 2 periods to have first valid sample */
+ min_sleep_us = 2 * period_us;
+ max_sleep_us = 2 * (period_us + period_us / 2);
+ usleep_range(min_sleep_us, max_sleep_us);
ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
chan->channel2, val);
result = inv_mpu6050_switch_engine(st, false,
@@ -588,6 +597,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
INV_MPU6050_SENSOR_ACCL);
if (result)
goto error_power_off;
+ /* wait 1 period for first sample availability */
+ min_sleep_us = period_us;
+ max_sleep_us = period_us + period_us / 2;
+ usleep_range(min_sleep_us, max_sleep_us);
ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
chan->channel2, val);
result = inv_mpu6050_switch_engine(st, false,
@@ -600,8 +613,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
INV_MPU6050_SENSOR_TEMP);
if (result)
goto error_power_off;
- /* wait for stablization */
- msleep(INV_MPU6050_TEMP_UP_TIME);
+ /* wait 1 period for first sample availability */
+ min_sleep_us = period_us;
+ max_sleep_us = period_us + period_us / 2;
+ usleep_range(min_sleep_us, max_sleep_us);
ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
IIO_MOD_X, val);
result = inv_mpu6050_switch_engine(st, false,
@@ -610,7 +625,24 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
goto error_power_off;
break;
case IIO_MAGN:
+ result = inv_mpu6050_switch_engine(st, true,
+ INV_MPU6050_SENSOR_MAGN);
+ if (result)
+ goto error_power_off;
+ /* frequency is limited for magnetometer */
+ if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
+ freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
+ period_us = 1000000 / freq_hz;
+ }
+ /* need to wait 2 periods to have first valid sample */
+ min_sleep_us = 2 * period_us;
+ max_sleep_us = 2 * (period_us + period_us / 2);
+ usleep_range(min_sleep_us, max_sleep_us);
ret = inv_mpu_magn_read(st, chan->channel2, val);
+ result = inv_mpu6050_switch_engine(st, false,
+ INV_MPU6050_SENSOR_MAGN);
+ if (result)
+ goto error_power_off;
break;
default:
ret = -EINVAL;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
index 3f402fa56d95..f282e9cc34c5 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
@@ -44,9 +44,6 @@
#define INV_MPU_MAGN_REG_ASAY 0x11
#define INV_MPU_MAGN_REG_ASAZ 0x12
-/* Magnetometer maximum frequency */
-#define INV_MPU_MAGN_FREQ_HZ_MAX 50
-
static bool inv_magn_supported(const struct inv_mpu6050_state *st)
{
switch (st->chip_type) {
@@ -321,7 +318,6 @@ int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
unsigned int status;
__be16 data;
uint8_t addr;
- unsigned int freq_hz, period_ms;
int ret;
/* quit if chip is not supported */
@@ -344,23 +340,6 @@ int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
}
addr += INV_MPU6050_REG_EXT_SENS_DATA;
- /* compute period depending on current sampling rate */
- freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
- if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX)
- freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
- period_ms = 1000 / freq_hz;
-
- ret = inv_mpu6050_switch_engine(st, true, INV_MPU6050_SENSOR_MAGN);
- if (ret)
- return ret;
-
- /* need to wait 2 periods + half-period margin */
- msleep(period_ms * 2 + period_ms / 2);
-
- ret = inv_mpu6050_switch_engine(st, false, INV_MPU6050_SENSOR_MAGN);
- if (ret)
- return ret;
-
/* check i2c status and read raw data */
ret = regmap_read(st->map, INV_MPU6050_REG_I2C_MST_STATUS, &status);
if (ret)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
index f7ad50ca6c1b..185c000c697c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
@@ -10,6 +10,9 @@
#include "inv_mpu_iio.h"
+/* Magnetometer maximum frequency */
+#define INV_MPU_MAGN_FREQ_HZ_MAX 50
+
int inv_mpu_magn_probe(struct inv_mpu6050_state *st);
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface
2020-02-19 14:39 ` [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface Jean-Baptiste Maneyrol
@ 2020-02-21 11:34 ` Jonathan Cameron
2020-02-21 14:03 ` Jean-Baptiste Maneyrol
0 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:34 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Wed, 19 Feb 2020 15:39:54 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> When reading data with the polling interface, we need to wait
> at 1 sampling period to have a sample.
> For gyroscope and magnetometer, we need to wait for 2 periods
> before having a correct sample.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
I should have raised this before, but is this something we might want to
backport? I don't really want this whole cleanup series getting backported,
so is there a more minimal change for older kernels? (probably just sleep too
long in all cases!)
Applied,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 36 ++++++++++++++++++++--
> drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c | 21 -------------
> drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h | 3 ++
> 3 files changed, 37 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index a51efc4c941b..aeee39696d3a 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -563,9 +563,14 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> int *val)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + unsigned int freq_hz, period_us, min_sleep_us, max_sleep_us;
> int result;
> int ret;
>
> + /* compute sample period */
> + freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
> + period_us = 1000000 / freq_hz;
> +
> result = inv_mpu6050_set_power_itg(st, true);
> if (result)
> return result;
> @@ -576,6 +581,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> INV_MPU6050_SENSOR_GYRO);
> if (result)
> goto error_power_off;
> + /* need to wait 2 periods to have first valid sample */
> + min_sleep_us = 2 * period_us;
> + max_sleep_us = 2 * (period_us + period_us / 2);
> + usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
> chan->channel2, val);
> result = inv_mpu6050_switch_engine(st, false,
> @@ -588,6 +597,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> INV_MPU6050_SENSOR_ACCL);
> if (result)
> goto error_power_off;
> + /* wait 1 period for first sample availability */
> + min_sleep_us = period_us;
> + max_sleep_us = period_us + period_us / 2;
> + usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
> chan->channel2, val);
> result = inv_mpu6050_switch_engine(st, false,
> @@ -600,8 +613,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> INV_MPU6050_SENSOR_TEMP);
> if (result)
> goto error_power_off;
> - /* wait for stablization */
> - msleep(INV_MPU6050_TEMP_UP_TIME);
> + /* wait 1 period for first sample availability */
> + min_sleep_us = period_us;
> + max_sleep_us = period_us + period_us / 2;
> + usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
> IIO_MOD_X, val);
> result = inv_mpu6050_switch_engine(st, false,
> @@ -610,7 +625,24 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> goto error_power_off;
> break;
> case IIO_MAGN:
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_MAGN);
> + if (result)
> + goto error_power_off;
> + /* frequency is limited for magnetometer */
> + if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
> + freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> + period_us = 1000000 / freq_hz;
> + }
> + /* need to wait 2 periods to have first valid sample */
> + min_sleep_us = 2 * period_us;
> + max_sleep_us = 2 * (period_us + period_us / 2);
> + usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu_magn_read(st, chan->channel2, val);
> + result = inv_mpu6050_switch_engine(st, false,
> + INV_MPU6050_SENSOR_MAGN);
> + if (result)
> + goto error_power_off;
> break;
> default:
> ret = -EINVAL;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> index 3f402fa56d95..f282e9cc34c5 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> @@ -44,9 +44,6 @@
> #define INV_MPU_MAGN_REG_ASAY 0x11
> #define INV_MPU_MAGN_REG_ASAZ 0x12
>
> -/* Magnetometer maximum frequency */
> -#define INV_MPU_MAGN_FREQ_HZ_MAX 50
> -
> static bool inv_magn_supported(const struct inv_mpu6050_state *st)
> {
> switch (st->chip_type) {
> @@ -321,7 +318,6 @@ int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
> unsigned int status;
> __be16 data;
> uint8_t addr;
> - unsigned int freq_hz, period_ms;
> int ret;
>
> /* quit if chip is not supported */
> @@ -344,23 +340,6 @@ int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
> }
> addr += INV_MPU6050_REG_EXT_SENS_DATA;
>
> - /* compute period depending on current sampling rate */
> - freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
> - if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX)
> - freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> - period_ms = 1000 / freq_hz;
> -
> - ret = inv_mpu6050_switch_engine(st, true, INV_MPU6050_SENSOR_MAGN);
> - if (ret)
> - return ret;
> -
> - /* need to wait 2 periods + half-period margin */
> - msleep(period_ms * 2 + period_ms / 2);
> -
> - ret = inv_mpu6050_switch_engine(st, false, INV_MPU6050_SENSOR_MAGN);
> - if (ret)
> - return ret;
> -
> /* check i2c status and read raw data */
> ret = regmap_read(st->map, INV_MPU6050_REG_I2C_MST_STATUS, &status);
> if (ret)
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> index f7ad50ca6c1b..185c000c697c 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> @@ -10,6 +10,9 @@
>
> #include "inv_mpu_iio.h"
>
> +/* Magnetometer maximum frequency */
> +#define INV_MPU_MAGN_FREQ_HZ_MAX 50
> +
> int inv_mpu_magn_probe(struct inv_mpu6050_state *st);
>
> /**
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface
2020-02-21 11:34 ` Jonathan Cameron
@ 2020-02-21 14:03 ` Jean-Baptiste Maneyrol
2020-02-21 15:29 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-21 14:03 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio
Hi Jonathan,
this is not something we want to backport. Mainly because it is heavly dependant on the rework of the power and sensor engines.
And polling interface without pm_runtime autosuspend is not really relevent.
Thanks,
JB
From: linux-iio-owner@vger.kernel.org <linux-iio-owner@vger.kernel.org> on behalf of Jonathan Cameron <jic23@kernel.org>
Sent: Friday, February 21, 2020 12:34
To: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
Subject: Re: [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface
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 Wed, 19 Feb 2020 15:39:54 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> When reading data with the polling interface, we need to wait
> at 1 sampling period to have a sample.
> For gyroscope and magnetometer, we need to wait for 2 periods
> before having a correct sample.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
I should have raised this before, but is this something we might want to
backport? I don't really want this whole cleanup series getting backported,
so is there a more minimal change for older kernels? (probably just sleep too
long in all cases!)
Applied,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 36 ++++++++++++++++++++--
> drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c | 21 -------------
> drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h | 3 ++
> 3 files changed, 37 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index a51efc4c941b..aeee39696d3a 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -563,9 +563,14 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> int *val)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + unsigned int freq_hz, period_us, min_sleep_us, max_sleep_us;
> int result;
> int ret;
>
> + /* compute sample period */
> + freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
> + period_us = 1000000 / freq_hz;
> +
> result = inv_mpu6050_set_power_itg(st, true);
> if (result)
> return result;
> @@ -576,6 +581,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> INV_MPU6050_SENSOR_GYRO);
> if (result)
> goto error_power_off;
> + /* need to wait 2 periods to have first valid sample */
> + min_sleep_us = 2 * period_us;
> + max_sleep_us = 2 * (period_us + period_us / 2);
> + usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
> chan->channel2, val);
> result = inv_mpu6050_switch_engine(st, false,
> @@ -588,6 +597,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> INV_MPU6050_SENSOR_ACCL);
> if (result)
> goto error_power_off;
> + /* wait 1 period for first sample availability */
> + min_sleep_us = period_us;
> + max_sleep_us = period_us + period_us / 2;
> + usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
> chan->channel2, val);
> result = inv_mpu6050_switch_engine(st, false,
> @@ -600,8 +613,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> INV_MPU6050_SENSOR_TEMP);
> if (result)
> goto error_power_off;
> - /* wait for stablization */
> - msleep(INV_MPU6050_TEMP_UP_TIME);
> + /* wait 1 period for first sample availability */
> + min_sleep_us = period_us;
> + max_sleep_us = period_us + period_us / 2;
> + usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
> IIO_MOD_X, val);
> result = inv_mpu6050_switch_engine(st, false,
> @@ -610,7 +625,24 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> goto error_power_off;
> break;
> case IIO_MAGN:
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_MAGN);
> + if (result)
> + goto error_power_off;
> + /* frequency is limited for magnetometer */
> + if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
> + freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> + period_us = 1000000 / freq_hz;
> + }
> + /* need to wait 2 periods to have first valid sample */
> + min_sleep_us = 2 * period_us;
> + max_sleep_us = 2 * (period_us + period_us / 2);
> + usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu_magn_read(st, chan->channel2, val);
> + result = inv_mpu6050_switch_engine(st, false,
> + INV_MPU6050_SENSOR_MAGN);
> + if (result)
> + goto error_power_off;
> break;
> default:
> ret = -EINVAL;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> index 3f402fa56d95..f282e9cc34c5 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
> @@ -44,9 +44,6 @@
> #define INV_MPU_MAGN_REG_ASAY 0x11
> #define INV_MPU_MAGN_REG_ASAZ 0x12
>
> -/* Magnetometer maximum frequency */
> -#define INV_MPU_MAGN_FREQ_HZ_MAX 50
> -
> static bool inv_magn_supported(const struct inv_mpu6050_state *st)
> {
> switch (st->chip_type) {
> @@ -321,7 +318,6 @@ int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
> unsigned int status;
> __be16 data;
> uint8_t addr;
> - unsigned int freq_hz, period_ms;
> int ret;
>
> /* quit if chip is not supported */
> @@ -344,23 +340,6 @@ int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
> }
> addr += INV_MPU6050_REG_EXT_SENS_DATA;
>
> - /* compute period depending on current sampling rate */
> - freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
> - if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX)
> - freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> - period_ms = 1000 / freq_hz;
> -
> - ret = inv_mpu6050_switch_engine(st, true, INV_MPU6050_SENSOR_MAGN);
> - if (ret)
> - return ret;
> -
> - /* need to wait 2 periods + half-period margin */
> - msleep(period_ms * 2 + period_ms / 2);
> -
> - ret = inv_mpu6050_switch_engine(st, false, INV_MPU6050_SENSOR_MAGN);
> - if (ret)
> - return ret;
> -
> /* check i2c status and read raw data */
> ret = regmap_read(st->map, INV_MPU6050_REG_I2C_MST_STATUS, &status);
> if (ret)
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> index f7ad50ca6c1b..185c000c697c 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
> @@ -10,6 +10,9 @@
>
> #include "inv_mpu_iio.h"
>
> +/* Magnetometer maximum frequency */
> +#define INV_MPU_MAGN_FREQ_HZ_MAX 50
> +
> int inv_mpu_magn_probe(struct inv_mpu6050_state *st);
>
> /**
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface
2020-02-21 14:03 ` Jean-Baptiste Maneyrol
@ 2020-02-21 15:29 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 15:29 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Fri, 21 Feb 2020 14:03:39 +0000
Jean-Baptiste Maneyrol <JManeyrol@invensense.com> wrote:
> Hi Jonathan,
>
> this is not something we want to backport. Mainly because it is heavly dependant on the rework of the power and sensor engines.
>
> And polling interface without pm_runtime autosuspend is not really relevent.
Fair enough. I'll add a note to try and prevent it getting automatically
picked up as a fix.
Thanks,
Jonathan
>
> Thanks,
> JB
>
>
> From: linux-iio-owner@vger.kernel.org <linux-iio-owner@vger.kernel.org> on behalf of Jonathan Cameron <jic23@kernel.org>
>
> Sent: Friday, February 21, 2020 12:34
>
> To: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
>
> Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
>
> Subject: Re: [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface
>
>
>
>
> 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 Wed, 19 Feb 2020 15:39:54 +0100
>
> Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
>
>
>
> > When reading data with the polling interface, we need to wait
>
> > at 1 sampling period to have a sample.
>
> > For gyroscope and magnetometer, we need to wait for 2 periods
>
> > before having a correct sample.
>
> >
>
> > Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
>
>
>
> I should have raised this before, but is this something we might want to
>
> backport? I don't really want this whole cleanup series getting backported,
>
> so is there a more minimal change for older kernels? (probably just sleep too
>
> long in all cases!)
>
>
>
> Applied,
>
>
>
> Jonathan
>
>
>
> > ---
>
> > drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 36 ++++++++++++++++++++--
>
> > drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c | 21 -------------
>
> > drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h | 3 ++
>
> > 3 files changed, 37 insertions(+), 23 deletions(-)
>
> >
>
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>
> > index a51efc4c941b..aeee39696d3a 100644
>
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>
> > @@ -563,9 +563,14 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
>
> > int *val)
>
> > {
>
> > struct inv_mpu6050_state *st = iio_priv(indio_dev);
>
> > + unsigned int freq_hz, period_us, min_sleep_us, max_sleep_us;
>
> > int result;
>
> > int ret;
>
> >
>
> > + /* compute sample period */
>
> > + freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
>
> > + period_us = 1000000 / freq_hz;
>
> > +
>
> > result = inv_mpu6050_set_power_itg(st, true);
>
> > if (result)
>
> > return result;
>
> > @@ -576,6 +581,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
>
> > INV_MPU6050_SENSOR_GYRO);
>
> > if (result)
>
> > goto error_power_off;
>
> > + /* need to wait 2 periods to have first valid sample */
>
> > + min_sleep_us = 2 * period_us;
>
> > + max_sleep_us = 2 * (period_us + period_us / 2);
>
> > + usleep_range(min_sleep_us, max_sleep_us);
>
> > ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
>
> > chan->channel2, val);
>
> > result = inv_mpu6050_switch_engine(st, false,
>
> > @@ -588,6 +597,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
>
> > INV_MPU6050_SENSOR_ACCL);
>
> > if (result)
>
> > goto error_power_off;
>
> > + /* wait 1 period for first sample availability */
>
> > + min_sleep_us = period_us;
>
> > + max_sleep_us = period_us + period_us / 2;
>
> > + usleep_range(min_sleep_us, max_sleep_us);
>
> > ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
>
> > chan->channel2, val);
>
> > result = inv_mpu6050_switch_engine(st, false,
>
> > @@ -600,8 +613,10 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
>
> > INV_MPU6050_SENSOR_TEMP);
>
> > if (result)
>
> > goto error_power_off;
>
> > - /* wait for stablization */
>
> > - msleep(INV_MPU6050_TEMP_UP_TIME);
>
> > + /* wait 1 period for first sample availability */
>
> > + min_sleep_us = period_us;
>
> > + max_sleep_us = period_us + period_us / 2;
>
> > + usleep_range(min_sleep_us, max_sleep_us);
>
> > ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
>
> > IIO_MOD_X, val);
>
> > result = inv_mpu6050_switch_engine(st, false,
>
> > @@ -610,7 +625,24 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
>
> > goto error_power_off;
>
> > break;
>
> > case IIO_MAGN:
>
> > + result = inv_mpu6050_switch_engine(st, true,
>
> > + INV_MPU6050_SENSOR_MAGN);
>
> > + if (result)
>
> > + goto error_power_off;
>
> > + /* frequency is limited for magnetometer */
>
> > + if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
>
> > + freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
>
> > + period_us = 1000000 / freq_hz;
>
> > + }
>
> > + /* need to wait 2 periods to have first valid sample */
>
> > + min_sleep_us = 2 * period_us;
>
> > + max_sleep_us = 2 * (period_us + period_us / 2);
>
> > + usleep_range(min_sleep_us, max_sleep_us);
>
> > ret = inv_mpu_magn_read(st, chan->channel2, val);
>
> > + result = inv_mpu6050_switch_engine(st, false,
>
> > + INV_MPU6050_SENSOR_MAGN);
>
> > + if (result)
>
> > + goto error_power_off;
>
> > break;
>
> > default:
>
> > ret = -EINVAL;
>
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
>
> > index 3f402fa56d95..f282e9cc34c5 100644
>
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
>
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.c
>
> > @@ -44,9 +44,6 @@
>
> > #define INV_MPU_MAGN_REG_ASAY 0x11
>
> > #define INV_MPU_MAGN_REG_ASAZ 0x12
>
> >
>
> > -/* Magnetometer maximum frequency */
>
> > -#define INV_MPU_MAGN_FREQ_HZ_MAX 50
>
> > -
>
> > static bool inv_magn_supported(const struct inv_mpu6050_state *st)
>
> > {
>
> > switch (st->chip_type) {
>
> > @@ -321,7 +318,6 @@ int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
>
> > unsigned int status;
>
> > __be16 data;
>
> > uint8_t addr;
>
> > - unsigned int freq_hz, period_ms;
>
> > int ret;
>
> >
>
> > /* quit if chip is not supported */
>
> > @@ -344,23 +340,6 @@ int inv_mpu_magn_read(struct inv_mpu6050_state *st, int axis, int *val)
>
> > }
>
> > addr += INV_MPU6050_REG_EXT_SENS_DATA;
>
> >
>
> > - /* compute period depending on current sampling rate */
>
> > - freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
>
> > - if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX)
>
> > - freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
>
> > - period_ms = 1000 / freq_hz;
>
> > -
>
> > - ret = inv_mpu6050_switch_engine(st, true, INV_MPU6050_SENSOR_MAGN);
>
> > - if (ret)
>
> > - return ret;
>
> > -
>
> > - /* need to wait 2 periods + half-period margin */
>
> > - msleep(period_ms * 2 + period_ms / 2);
>
> > -
>
> > - ret = inv_mpu6050_switch_engine(st, false, INV_MPU6050_SENSOR_MAGN);
>
> > - if (ret)
>
> > - return ret;
>
> > -
>
> > /* check i2c status and read raw data */
>
> > ret = regmap_read(st->map, INV_MPU6050_REG_I2C_MST_STATUS, &status);
>
> > if (ret)
>
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
>
> > index f7ad50ca6c1b..185c000c697c 100644
>
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
>
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_magn.h
>
> > @@ -10,6 +10,9 @@
>
> >
>
> > #include "inv_mpu_iio.h"
>
> >
>
> > +/* Magnetometer maximum frequency */
>
> > +#define INV_MPU_MAGN_FREQ_HZ_MAX 50
>
> > +
>
> > int inv_mpu_magn_probe(struct inv_mpu6050_state *st);
>
> >
>
> > /**
>
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 10/13] iio: imu: inv_mpu6050: factorize fifo enable/disable
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (8 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 09/13] iio: imu: inv_mpu6050: fix data polling interface Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:37 ` Jonathan Cameron
2020-02-19 14:39 ` [PATCH v2 11/13] iio: imu: inv_mpu6050: dynamic sampling rate change Jean-Baptiste Maneyrol
` (2 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Rework fifo enable/disable in a separate function.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 54 ++-------------
drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 66 ++++++++++++++-----
3 files changed, 55 insertions(+), 67 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index a578789c9210..e328c98e362c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -435,7 +435,7 @@ enum inv_mpu6050_clock_sel_e {
irqreturn_t inv_mpu6050_read_fifo(int irq, void *p);
int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type);
-int inv_reset_fifo(struct iio_dev *indio_dev);
+int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable);
int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
unsigned int mask);
int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 val);
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index d7397705974e..9511e4715e2c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -90,60 +90,14 @@ static s64 inv_mpu6050_get_timestamp(struct inv_mpu6050_state *st)
return ts;
}
-int inv_reset_fifo(struct iio_dev *indio_dev)
+static int inv_reset_fifo(struct iio_dev *indio_dev)
{
int result;
- u8 d;
struct inv_mpu6050_state *st = iio_priv(indio_dev);
- /* reset it timestamp validation */
- st->it_timestamp = 0;
-
- /* disable interrupt */
- result = regmap_write(st->map, st->reg->int_enable, 0);
- if (result) {
- dev_err(regmap_get_device(st->map), "int_enable failed %d\n",
- result);
- return result;
- }
- /* disable the sensor output to FIFO */
- result = regmap_write(st->map, st->reg->fifo_en, 0);
- if (result)
- goto reset_fifo_fail;
- /* disable fifo reading */
- result = regmap_write(st->map, st->reg->user_ctrl,
- st->chip_config.user_ctrl);
- if (result)
- goto reset_fifo_fail;
-
- /* reset FIFO*/
- d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_RST;
- result = regmap_write(st->map, st->reg->user_ctrl, d);
- if (result)
- goto reset_fifo_fail;
-
- /* enable interrupt */
- result = regmap_write(st->map, st->reg->int_enable,
- INV_MPU6050_BIT_DATA_RDY_EN);
- if (result)
- return result;
-
- /* enable FIFO reading */
- d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_EN;
- result = regmap_write(st->map, st->reg->user_ctrl, d);
- if (result)
- goto reset_fifo_fail;
- /* enable sensor output to FIFO */
- d = 0;
- if (st->chip_config.gyro_fifo_enable)
- d |= INV_MPU6050_BITS_GYRO_OUT;
- if (st->chip_config.accl_fifo_enable)
- d |= INV_MPU6050_BIT_ACCEL_OUT;
- if (st->chip_config.temp_fifo_enable)
- d |= INV_MPU6050_BIT_TEMP_OUT;
- if (st->chip_config.magn_fifo_enable)
- d |= INV_MPU6050_BIT_SLAVE_0;
- result = regmap_write(st->map, st->reg->fifo_en, d);
+ /* disable fifo and reenable it */
+ inv_mpu6050_prepare_fifo(st, false);
+ result = inv_mpu6050_prepare_fifo(st, true);
if (result)
goto reset_fifo_fail;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index cfd7243159f6..f53f50d08b9e 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -100,6 +100,54 @@ static unsigned int inv_compute_skip_samples(const struct inv_mpu6050_state *st)
return skip_samples;
}
+int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable)
+{
+ uint8_t d;
+ int ret;
+
+ if (enable) {
+ st->it_timestamp = 0;
+ /* reset FIFO */
+ d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_RST;
+ ret = regmap_write(st->map, st->reg->user_ctrl, d);
+ if (ret)
+ return ret;
+ /* enable sensor output to FIFO */
+ d = 0;
+ if (st->chip_config.gyro_fifo_enable)
+ d |= INV_MPU6050_BITS_GYRO_OUT;
+ if (st->chip_config.accl_fifo_enable)
+ d |= INV_MPU6050_BIT_ACCEL_OUT;
+ if (st->chip_config.temp_fifo_enable)
+ d |= INV_MPU6050_BIT_TEMP_OUT;
+ if (st->chip_config.magn_fifo_enable)
+ d |= INV_MPU6050_BIT_SLAVE_0;
+ ret = regmap_write(st->map, st->reg->fifo_en, d);
+ if (ret)
+ return ret;
+ /* enable FIFO reading */
+ d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_EN;
+ ret = regmap_write(st->map, st->reg->user_ctrl, d);
+ if (ret)
+ return ret;
+ /* enable interrupt */
+ ret = regmap_write(st->map, st->reg->int_enable,
+ INV_MPU6050_BIT_DATA_RDY_EN);
+ } else {
+ ret = regmap_write(st->map, st->reg->int_enable, 0);
+ if (ret)
+ return ret;
+ ret = regmap_write(st->map, st->reg->fifo_en, 0);
+ if (ret)
+ return ret;
+ /* restore user_ctrl for disabling FIFO reading */
+ ret = regmap_write(st->map, st->reg->user_ctrl,
+ st->chip_config.user_ctrl);
+ }
+
+ return ret;
+}
+
/**
* inv_mpu6050_set_enable() - enable chip functions.
* @indio_dev: Device driver instance.
@@ -121,24 +169,13 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
if (result)
goto error_power_off;
st->skip_samples = inv_compute_skip_samples(st);
- result = inv_reset_fifo(indio_dev);
+ result = inv_mpu6050_prepare_fifo(st, true);
if (result)
goto error_sensors_off;
} else {
- result = regmap_write(st->map, st->reg->fifo_en, 0);
- if (result)
- goto error_fifo_off;
-
- result = regmap_write(st->map, st->reg->int_enable, 0);
- if (result)
- goto error_fifo_off;
-
- /* restore user_ctrl for disabling FIFO reading */
- result = regmap_write(st->map, st->reg->user_ctrl,
- st->chip_config.user_ctrl);
+ result = inv_mpu6050_prepare_fifo(st, false);
if (result)
goto error_sensors_off;
-
result = inv_mpu6050_switch_engine(st, false, scan);
if (result)
goto error_power_off;
@@ -150,9 +187,6 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
return 0;
-error_fifo_off:
- /* always restore user_ctrl to disable fifo properly */
- regmap_write(st->map, st->reg->user_ctrl, st->chip_config.user_ctrl);
error_sensors_off:
inv_mpu6050_switch_engine(st, false, scan);
error_power_off:
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 10/13] iio: imu: inv_mpu6050: factorize fifo enable/disable
2020-02-19 14:39 ` [PATCH v2 10/13] iio: imu: inv_mpu6050: factorize fifo enable/disable Jean-Baptiste Maneyrol
@ 2020-02-21 11:37 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:37 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Wed, 19 Feb 2020 15:39:55 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Rework fifo enable/disable in a separate function.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 2 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 54 ++-------------
> drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 66 ++++++++++++++-----
> 3 files changed, 55 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index a578789c9210..e328c98e362c 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -435,7 +435,7 @@ enum inv_mpu6050_clock_sel_e {
>
> irqreturn_t inv_mpu6050_read_fifo(int irq, void *p);
> int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type);
> -int inv_reset_fifo(struct iio_dev *indio_dev);
> +int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable);
> int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
> unsigned int mask);
> int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 val);
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index d7397705974e..9511e4715e2c 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -90,60 +90,14 @@ static s64 inv_mpu6050_get_timestamp(struct inv_mpu6050_state *st)
> return ts;
> }
>
> -int inv_reset_fifo(struct iio_dev *indio_dev)
> +static int inv_reset_fifo(struct iio_dev *indio_dev)
> {
> int result;
> - u8 d;
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
>
> - /* reset it timestamp validation */
> - st->it_timestamp = 0;
> -
> - /* disable interrupt */
> - result = regmap_write(st->map, st->reg->int_enable, 0);
> - if (result) {
> - dev_err(regmap_get_device(st->map), "int_enable failed %d\n",
> - result);
> - return result;
> - }
> - /* disable the sensor output to FIFO */
> - result = regmap_write(st->map, st->reg->fifo_en, 0);
> - if (result)
> - goto reset_fifo_fail;
> - /* disable fifo reading */
> - result = regmap_write(st->map, st->reg->user_ctrl,
> - st->chip_config.user_ctrl);
> - if (result)
> - goto reset_fifo_fail;
> -
> - /* reset FIFO*/
> - d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_RST;
> - result = regmap_write(st->map, st->reg->user_ctrl, d);
> - if (result)
> - goto reset_fifo_fail;
> -
> - /* enable interrupt */
> - result = regmap_write(st->map, st->reg->int_enable,
> - INV_MPU6050_BIT_DATA_RDY_EN);
> - if (result)
> - return result;
> -
> - /* enable FIFO reading */
> - d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_EN;
> - result = regmap_write(st->map, st->reg->user_ctrl, d);
> - if (result)
> - goto reset_fifo_fail;
> - /* enable sensor output to FIFO */
> - d = 0;
> - if (st->chip_config.gyro_fifo_enable)
> - d |= INV_MPU6050_BITS_GYRO_OUT;
> - if (st->chip_config.accl_fifo_enable)
> - d |= INV_MPU6050_BIT_ACCEL_OUT;
> - if (st->chip_config.temp_fifo_enable)
> - d |= INV_MPU6050_BIT_TEMP_OUT;
> - if (st->chip_config.magn_fifo_enable)
> - d |= INV_MPU6050_BIT_SLAVE_0;
> - result = regmap_write(st->map, st->reg->fifo_en, d);
> + /* disable fifo and reenable it */
> + inv_mpu6050_prepare_fifo(st, false);
> + result = inv_mpu6050_prepare_fifo(st, true);
> if (result)
> goto reset_fifo_fail;
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index cfd7243159f6..f53f50d08b9e 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -100,6 +100,54 @@ static unsigned int inv_compute_skip_samples(const struct inv_mpu6050_state *st)
> return skip_samples;
> }
>
> +int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable)
> +{
> + uint8_t d;
> + int ret;
> +
> + if (enable) {
> + st->it_timestamp = 0;
> + /* reset FIFO */
> + d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_RST;
> + ret = regmap_write(st->map, st->reg->user_ctrl, d);
> + if (ret)
> + return ret;
> + /* enable sensor output to FIFO */
> + d = 0;
> + if (st->chip_config.gyro_fifo_enable)
> + d |= INV_MPU6050_BITS_GYRO_OUT;
> + if (st->chip_config.accl_fifo_enable)
> + d |= INV_MPU6050_BIT_ACCEL_OUT;
> + if (st->chip_config.temp_fifo_enable)
> + d |= INV_MPU6050_BIT_TEMP_OUT;
> + if (st->chip_config.magn_fifo_enable)
> + d |= INV_MPU6050_BIT_SLAVE_0;
> + ret = regmap_write(st->map, st->reg->fifo_en, d);
> + if (ret)
> + return ret;
> + /* enable FIFO reading */
> + d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_EN;
> + ret = regmap_write(st->map, st->reg->user_ctrl, d);
> + if (ret)
> + return ret;
> + /* enable interrupt */
> + ret = regmap_write(st->map, st->reg->int_enable,
> + INV_MPU6050_BIT_DATA_RDY_EN);
> + } else {
> + ret = regmap_write(st->map, st->reg->int_enable, 0);
> + if (ret)
> + return ret;
> + ret = regmap_write(st->map, st->reg->fifo_en, 0);
> + if (ret)
> + return ret;
> + /* restore user_ctrl for disabling FIFO reading */
> + ret = regmap_write(st->map, st->reg->user_ctrl,
> + st->chip_config.user_ctrl);
> + }
> +
> + return ret;
> +}
> +
> /**
> * inv_mpu6050_set_enable() - enable chip functions.
> * @indio_dev: Device driver instance.
> @@ -121,24 +169,13 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
> if (result)
> goto error_power_off;
> st->skip_samples = inv_compute_skip_samples(st);
> - result = inv_reset_fifo(indio_dev);
> + result = inv_mpu6050_prepare_fifo(st, true);
> if (result)
> goto error_sensors_off;
> } else {
> - result = regmap_write(st->map, st->reg->fifo_en, 0);
> - if (result)
> - goto error_fifo_off;
> -
> - result = regmap_write(st->map, st->reg->int_enable, 0);
> - if (result)
> - goto error_fifo_off;
> -
> - /* restore user_ctrl for disabling FIFO reading */
> - result = regmap_write(st->map, st->reg->user_ctrl,
> - st->chip_config.user_ctrl);
> + result = inv_mpu6050_prepare_fifo(st, false);
> if (result)
> goto error_sensors_off;
> -
> result = inv_mpu6050_switch_engine(st, false, scan);
> if (result)
> goto error_power_off;
> @@ -150,9 +187,6 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
>
> return 0;
>
> -error_fifo_off:
> - /* always restore user_ctrl to disable fifo properly */
> - regmap_write(st->map, st->reg->user_ctrl, st->chip_config.user_ctrl);
> error_sensors_off:
> inv_mpu6050_switch_engine(st, false, scan);
> error_power_off:
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 11/13] iio: imu: inv_mpu6050: dynamic sampling rate change
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (9 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 10/13] iio: imu: inv_mpu6050: factorize fifo enable/disable Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:38 ` Jonathan Cameron
2020-02-19 14:39 ` [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend Jean-Baptiste Maneyrol
2020-02-19 14:39 ` [PATCH v2 13/13] iio: imu: inv_mpu6050: temperature only work with accel/gyro Jean-Baptiste Maneyrol
12 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Sampling rate can be changed while the chip is running. It can
be useful thus do not prevent it.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index aeee39696d3a..9076b6bb099c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -910,10 +910,6 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
return -EINVAL;
- result = iio_device_claim_direct_mode(indio_dev);
- if (result)
- return result;
-
/* compute the chip sample rate divider */
d = INV_MPU6050_FIFO_RATE_TO_DIVIDER(fifo_rate);
/* compute back the fifo rate to handle truncation cases */
@@ -946,7 +942,6 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
result |= inv_mpu6050_set_power_itg(st, false);
fifo_rate_fail_unlock:
mutex_unlock(&st->lock);
- iio_device_release_direct_mode(indio_dev);
if (result)
return result;
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 11/13] iio: imu: inv_mpu6050: dynamic sampling rate change
2020-02-19 14:39 ` [PATCH v2 11/13] iio: imu: inv_mpu6050: dynamic sampling rate change Jean-Baptiste Maneyrol
@ 2020-02-21 11:38 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:38 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Wed, 19 Feb 2020 15:39:56 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Sampling rate can be changed while the chip is running. It can
> be useful thus do not prevent it.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index aeee39696d3a..9076b6bb099c 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -910,10 +910,6 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
> return -EINVAL;
>
> - result = iio_device_claim_direct_mode(indio_dev);
> - if (result)
> - return result;
> -
> /* compute the chip sample rate divider */
> d = INV_MPU6050_FIFO_RATE_TO_DIVIDER(fifo_rate);
> /* compute back the fifo rate to handle truncation cases */
> @@ -946,7 +942,6 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> result |= inv_mpu6050_set_power_itg(st, false);
> fifo_rate_fail_unlock:
> mutex_unlock(&st->lock);
> - iio_device_release_direct_mode(indio_dev);
> if (result)
> return result;
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (10 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 11/13] iio: imu: inv_mpu6050: dynamic sampling rate change Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:54 ` Jonathan Cameron
2020-03-24 21:24 ` Dmitry Osipenko
2020-02-19 14:39 ` [PATCH v2 13/13] iio: imu: inv_mpu6050: temperature only work with accel/gyro Jean-Baptiste Maneyrol
12 siblings, 2 replies; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Use runtime power management for handling chip power and
sensor engines on/off. Simplifies things a lot since pm
runtime already has reference counter.
Usage of autosuspend reduces the number of power on/off. This
makes polling interface now usable to get data at low
frequency.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 267 ++++++++++++------
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 5 +-
drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 33 ++-
3 files changed, 194 insertions(+), 111 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 9076b6bb099c..750fbc2614f0 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -16,6 +16,8 @@
#include <linux/acpi.h>
#include <linux/platform_device.h>
#include <linux/regulator/consumer.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
#include "inv_mpu_iio.h"
#include "inv_mpu_magn.h"
@@ -400,26 +402,13 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
{
int result;
- if (power_on) {
- if (!st->powerup_count) {
- result = inv_mpu6050_pwr_mgmt_1_write(st, false, -1, -1);
- if (result)
- return result;
- usleep_range(INV_MPU6050_REG_UP_TIME_MIN,
- INV_MPU6050_REG_UP_TIME_MAX);
- }
- st->powerup_count++;
- } else {
- if (st->powerup_count == 1) {
- result = inv_mpu6050_pwr_mgmt_1_write(st, true, -1, -1);
- if (result)
- return result;
- }
- st->powerup_count--;
- }
+ result = inv_mpu6050_pwr_mgmt_1_write(st, !power_on, -1, -1);
+ if (result)
+ return result;
- dev_dbg(regmap_get_device(st->map), "set power %d, count=%u\n",
- power_on, st->powerup_count);
+ if (power_on)
+ usleep_range(INV_MPU6050_REG_UP_TIME_MIN,
+ INV_MPU6050_REG_UP_TIME_MAX);
return 0;
}
@@ -563,6 +552,7 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
int *val)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
+ struct device *pdev = regmap_get_device(st->map);
unsigned int freq_hz, period_us, min_sleep_us, max_sleep_us;
int result;
int ret;
@@ -571,92 +561,85 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
period_us = 1000000 / freq_hz;
- result = inv_mpu6050_set_power_itg(st, true);
- if (result)
+ result = pm_runtime_get_sync(pdev);
+ if (result < 0) {
+ pm_runtime_put_noidle(pdev);
return result;
+ }
switch (chan->type) {
case IIO_ANGL_VEL:
- result = inv_mpu6050_switch_engine(st, true,
- INV_MPU6050_SENSOR_GYRO);
- if (result)
- goto error_power_off;
- /* need to wait 2 periods to have first valid sample */
- min_sleep_us = 2 * period_us;
- max_sleep_us = 2 * (period_us + period_us / 2);
- usleep_range(min_sleep_us, max_sleep_us);
+ if (!st->chip_config.gyro_en) {
+ result = inv_mpu6050_switch_engine(st, true,
+ INV_MPU6050_SENSOR_GYRO);
+ if (result)
+ goto error_power_off;
+ /* need to wait 2 periods to have first valid sample */
+ min_sleep_us = 2 * period_us;
+ max_sleep_us = 2 * (period_us + period_us / 2);
+ usleep_range(min_sleep_us, max_sleep_us);
+ }
ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
chan->channel2, val);
- result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_SENSOR_GYRO);
- if (result)
- goto error_power_off;
break;
case IIO_ACCEL:
- result = inv_mpu6050_switch_engine(st, true,
- INV_MPU6050_SENSOR_ACCL);
- if (result)
- goto error_power_off;
- /* wait 1 period for first sample availability */
- min_sleep_us = period_us;
- max_sleep_us = period_us + period_us / 2;
- usleep_range(min_sleep_us, max_sleep_us);
+ if (!st->chip_config.accl_en) {
+ result = inv_mpu6050_switch_engine(st, true,
+ INV_MPU6050_SENSOR_ACCL);
+ if (result)
+ goto error_power_off;
+ /* wait 1 period for first sample availability */
+ min_sleep_us = period_us;
+ max_sleep_us = period_us + period_us / 2;
+ usleep_range(min_sleep_us, max_sleep_us);
+ }
ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
chan->channel2, val);
- result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_SENSOR_ACCL);
- if (result)
- goto error_power_off;
break;
case IIO_TEMP:
- result = inv_mpu6050_switch_engine(st, true,
- INV_MPU6050_SENSOR_TEMP);
- if (result)
- goto error_power_off;
- /* wait 1 period for first sample availability */
- min_sleep_us = period_us;
- max_sleep_us = period_us + period_us / 2;
- usleep_range(min_sleep_us, max_sleep_us);
+ if (!st->chip_config.temp_en) {
+ result = inv_mpu6050_switch_engine(st, true,
+ INV_MPU6050_SENSOR_TEMP);
+ if (result)
+ goto error_power_off;
+ /* wait 1 period for first sample availability */
+ min_sleep_us = period_us;
+ max_sleep_us = period_us + period_us / 2;
+ usleep_range(min_sleep_us, max_sleep_us);
+ }
ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
IIO_MOD_X, val);
- result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_SENSOR_TEMP);
- if (result)
- goto error_power_off;
break;
case IIO_MAGN:
- result = inv_mpu6050_switch_engine(st, true,
- INV_MPU6050_SENSOR_MAGN);
- if (result)
- goto error_power_off;
- /* frequency is limited for magnetometer */
- if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
- freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
- period_us = 1000000 / freq_hz;
+ if (!st->chip_config.magn_en) {
+ result = inv_mpu6050_switch_engine(st, true,
+ INV_MPU6050_SENSOR_MAGN);
+ if (result)
+ goto error_power_off;
+ /* frequency is limited for magnetometer */
+ if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
+ freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
+ period_us = 1000000 / freq_hz;
+ }
+ /* need to wait 2 periods to have first valid sample */
+ min_sleep_us = 2 * period_us;
+ max_sleep_us = 2 * (period_us + period_us / 2);
+ usleep_range(min_sleep_us, max_sleep_us);
}
- /* need to wait 2 periods to have first valid sample */
- min_sleep_us = 2 * period_us;
- max_sleep_us = 2 * (period_us + period_us / 2);
- usleep_range(min_sleep_us, max_sleep_us);
ret = inv_mpu_magn_read(st, chan->channel2, val);
- result = inv_mpu6050_switch_engine(st, false,
- INV_MPU6050_SENSOR_MAGN);
- if (result)
- goto error_power_off;
break;
default:
ret = -EINVAL;
break;
}
- result = inv_mpu6050_set_power_itg(st, false);
- if (result)
- goto error_power_off;
+ pm_runtime_mark_last_busy(pdev);
+ pm_runtime_put_autosuspend(pdev);
return ret;
error_power_off:
- inv_mpu6050_set_power_itg(st, false);
+ pm_runtime_put_autosuspend(pdev);
return result;
}
@@ -795,6 +778,7 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
int val, int val2, long mask)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
+ struct device *pdev = regmap_get_device(st->map);
int result;
/*
@@ -806,9 +790,11 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
return result;
mutex_lock(&st->lock);
- result = inv_mpu6050_set_power_itg(st, true);
- if (result)
+ result = pm_runtime_get_sync(pdev);
+ if (result < 0) {
+ pm_runtime_put_noidle(pdev);
goto error_write_raw_unlock;
+ }
switch (mask) {
case IIO_CHAN_INFO_SCALE:
@@ -846,7 +832,8 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
break;
}
- result |= inv_mpu6050_set_power_itg(st, false);
+ pm_runtime_mark_last_busy(pdev);
+ pm_runtime_put_autosuspend(pdev);
error_write_raw_unlock:
mutex_unlock(&st->lock);
iio_device_release_direct_mode(indio_dev);
@@ -903,6 +890,7 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
int result;
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct inv_mpu6050_state *st = iio_priv(indio_dev);
+ struct device *pdev = regmap_get_device(st->map);
if (kstrtoint(buf, 10, &fifo_rate))
return -EINVAL;
@@ -920,9 +908,11 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
result = 0;
goto fifo_rate_fail_unlock;
}
- result = inv_mpu6050_set_power_itg(st, true);
- if (result)
+ result = pm_runtime_get_sync(pdev);
+ if (result < 0) {
+ pm_runtime_put_noidle(pdev);
goto fifo_rate_fail_unlock;
+ }
result = regmap_write(st->map, st->reg->sample_rate_div, d);
if (result)
@@ -938,8 +928,9 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
if (result)
goto fifo_rate_fail_power_off;
+ pm_runtime_mark_last_busy(pdev);
fifo_rate_fail_power_off:
- result |= inv_mpu6050_set_power_itg(st, false);
+ pm_runtime_put_autosuspend(pdev);
fifo_rate_fail_unlock:
mutex_unlock(&st->lock);
if (result)
@@ -1385,6 +1376,14 @@ static void inv_mpu_core_disable_regulator_action(void *_data)
inv_mpu_core_disable_regulator_vddio(st);
}
+static void inv_mpu_pm_disable(void *data)
+{
+ struct device *dev = data;
+
+ pm_runtime_put_sync_suspend(dev);
+ pm_runtime_disable(dev);
+}
+
int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type)
{
@@ -1409,7 +1408,6 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
st = iio_priv(indio_dev);
mutex_init(&st->lock);
st->chip_type = chip_type;
- st->powerup_count = 0;
st->irq = irq;
st->map = regmap;
@@ -1521,8 +1519,16 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
goto error_power_off;
}
- /* chip init is done, turning off */
- result = inv_mpu6050_set_power_itg(st, false);
+ /* chip init is done, turning on runtime power management */
+ result = pm_runtime_set_active(dev);
+ if (result)
+ goto error_power_off;
+ pm_runtime_get_noresume(dev);
+ pm_runtime_enable(dev);
+ pm_runtime_set_autosuspend_delay(dev, INV_MPU6050_SUSPEND_DELAY_MS);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_put(dev);
+ result = devm_add_action_or_reset(dev, inv_mpu_pm_disable, dev);
if (result)
return result;
@@ -1590,11 +1596,10 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
}
EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
-#ifdef CONFIG_PM_SLEEP
-
-static int inv_mpu_resume(struct device *dev)
+static int __maybe_unused inv_mpu_resume(struct device *dev)
{
- struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct inv_mpu6050_state *st = iio_priv(indio_dev);
int result;
mutex_lock(&st->lock);
@@ -1603,27 +1608,101 @@ static int inv_mpu_resume(struct device *dev)
goto out_unlock;
result = inv_mpu6050_set_power_itg(st, true);
+ if (result)
+ goto out_unlock;
+
+ result = inv_mpu6050_switch_engine(st, true, st->suspended_sensors);
+ if (result)
+ goto out_unlock;
+
+ if (iio_buffer_enabled(indio_dev))
+ result = inv_mpu6050_prepare_fifo(st, true);
+
out_unlock:
mutex_unlock(&st->lock);
return result;
}
-static int inv_mpu_suspend(struct device *dev)
+static int __maybe_unused inv_mpu_suspend(struct device *dev)
{
- struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct inv_mpu6050_state *st = iio_priv(indio_dev);
int result;
mutex_lock(&st->lock);
+
+ if (iio_buffer_enabled(indio_dev)) {
+ result = inv_mpu6050_prepare_fifo(st, false);
+ if (result)
+ goto out_unlock;
+ }
+
+ st->suspended_sensors = 0;
+ if (st->chip_config.accl_en)
+ st->suspended_sensors |= INV_MPU6050_SENSOR_ACCL;
+ if (st->chip_config.gyro_en)
+ st->suspended_sensors |= INV_MPU6050_SENSOR_GYRO;
+ if (st->chip_config.temp_en)
+ st->suspended_sensors |= INV_MPU6050_SENSOR_TEMP;
+ if (st->chip_config.magn_en)
+ st->suspended_sensors |= INV_MPU6050_SENSOR_MAGN;
+ result = inv_mpu6050_switch_engine(st, false, st->suspended_sensors);
+ if (result)
+ goto out_unlock;
+
result = inv_mpu6050_set_power_itg(st, false);
+ if (result)
+ goto out_unlock;
+
inv_mpu_core_disable_regulator_vddio(st);
+out_unlock:
mutex_unlock(&st->lock);
return result;
}
-#endif /* CONFIG_PM_SLEEP */
-SIMPLE_DEV_PM_OPS(inv_mpu_pmops, inv_mpu_suspend, inv_mpu_resume);
+static int __maybe_unused inv_mpu_runtime_suspend(struct device *dev)
+{
+ struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
+ unsigned int sensors;
+ int ret;
+
+ mutex_lock(&st->lock);
+
+ sensors = INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_GYRO |
+ INV_MPU6050_SENSOR_TEMP | INV_MPU6050_SENSOR_MAGN;
+ ret = inv_mpu6050_switch_engine(st, false, sensors);
+ if (ret)
+ goto out_unlock;
+
+ ret = inv_mpu6050_set_power_itg(st, false);
+ if (ret)
+ goto out_unlock;
+
+ inv_mpu_core_disable_regulator_vddio(st);
+
+out_unlock:
+ mutex_unlock(&st->lock);
+ return ret;
+}
+
+static int __maybe_unused inv_mpu_runtime_resume(struct device *dev)
+{
+ struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
+ int ret;
+
+ ret = inv_mpu_core_enable_regulator_vddio(st);
+ if (ret)
+ return ret;
+
+ return inv_mpu6050_set_power_itg(st, true);
+}
+
+const struct dev_pm_ops inv_mpu_pmops = {
+ SET_SYSTEM_SLEEP_PM_OPS(inv_mpu_suspend, inv_mpu_resume)
+ SET_RUNTIME_PM_OPS(inv_mpu_runtime_suspend, inv_mpu_runtime_resume, NULL)
+};
EXPORT_SYMBOL_GPL(inv_mpu_pmops);
MODULE_AUTHOR("Invensense Corporation");
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e328c98e362c..cd38b3fccc7b 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -164,6 +164,7 @@ struct inv_mpu6050_hw {
* @magn_disabled: magnetometer disabled for backward compatibility reason.
* @magn_raw_to_gauss: coefficient to convert mag raw value to Gauss.
* @magn_orient: magnetometer sensor chip orientation if available.
+ * @suspended_sensors: sensors mask of sensors turned off for suspend
*/
struct inv_mpu6050_state {
struct mutex lock;
@@ -174,7 +175,6 @@ struct inv_mpu6050_state {
enum inv_devices chip_type;
struct i2c_mux_core *muxc;
struct i2c_client *mux_client;
- unsigned int powerup_count;
struct inv_mpu6050_platform_data plat_data;
struct iio_mount_matrix orientation;
struct regmap *map;
@@ -189,6 +189,7 @@ struct inv_mpu6050_state {
bool magn_disabled;
s32 magn_raw_to_gauss[3];
struct iio_mount_matrix magn_orient;
+ unsigned int suspended_sensors;
};
/*register and associated bit definition*/
@@ -312,6 +313,7 @@ struct inv_mpu6050_state {
#define INV_MPU6050_ACCEL_UP_TIME 20
#define INV_MPU6050_GYRO_UP_TIME 35
#define INV_MPU6050_GYRO_DOWN_TIME 150
+#define INV_MPU6050_SUSPEND_DELAY_MS 2000
/* delay time in microseconds */
#define INV_MPU6050_REG_UP_TIME_MIN 5000
@@ -439,7 +441,6 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable);
int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
unsigned int mask);
int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 val);
-int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on);
int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index f53f50d08b9e..f7b5a70be30f 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -3,6 +3,7 @@
* Copyright (C) 2012 Invensense, Inc.
*/
+#include <linux/pm_runtime.h>
#include "inv_mpu_iio.h"
static unsigned int inv_scan_query_mpu6050(struct iio_dev *indio_dev)
@@ -156,41 +157,43 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable)
static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
{
struct inv_mpu6050_state *st = iio_priv(indio_dev);
+ struct device *pdev = regmap_get_device(st->map);
unsigned int scan;
int result;
- scan = inv_scan_query(indio_dev);
-
if (enable) {
- result = inv_mpu6050_set_power_itg(st, true);
- if (result)
+ scan = inv_scan_query(indio_dev);
+ result = pm_runtime_get_sync(pdev);
+ if (result < 0) {
+ pm_runtime_put_noidle(pdev);
return result;
+ }
+ /*
+ * In case autosuspend didn't trigger, turn off first not
+ * required sensors.
+ */
+ result = inv_mpu6050_switch_engine(st, false, ~scan);
+ if (result)
+ goto error_power_off;
result = inv_mpu6050_switch_engine(st, true, scan);
if (result)
goto error_power_off;
st->skip_samples = inv_compute_skip_samples(st);
result = inv_mpu6050_prepare_fifo(st, true);
if (result)
- goto error_sensors_off;
+ goto error_power_off;
} else {
result = inv_mpu6050_prepare_fifo(st, false);
- if (result)
- goto error_sensors_off;
- result = inv_mpu6050_switch_engine(st, false, scan);
- if (result)
- goto error_power_off;
-
- result = inv_mpu6050_set_power_itg(st, false);
if (result)
goto error_power_off;
+ pm_runtime_mark_last_busy(pdev);
+ pm_runtime_put_autosuspend(pdev);
}
return 0;
-error_sensors_off:
- inv_mpu6050_switch_engine(st, false, scan);
error_power_off:
- inv_mpu6050_set_power_itg(st, false);
+ pm_runtime_put_autosuspend(pdev);
return result;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend
2020-02-19 14:39 ` [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend Jean-Baptiste Maneyrol
@ 2020-02-21 11:54 ` Jonathan Cameron
2020-02-21 14:04 ` Jean-Baptiste Maneyrol
2020-03-24 21:24 ` Dmitry Osipenko
1 sibling, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:54 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Wed, 19 Feb 2020 15:39:57 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Use runtime power management for handling chip power and
> sensor engines on/off. Simplifies things a lot since pm
> runtime already has reference counter.
> Usage of autosuspend reduces the number of power on/off. This
> makes polling interface now usable to get data at low
> frequency.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Missing static marking after reducing scope of one function.
See inline. I've fixed that up and applied.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 267 ++++++++++++------
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 5 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 33 ++-
> 3 files changed, 194 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 9076b6bb099c..750fbc2614f0 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -16,6 +16,8 @@
> #include <linux/acpi.h>
> #include <linux/platform_device.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> #include "inv_mpu_iio.h"
> #include "inv_mpu_magn.h"
>
> @@ -400,26 +402,13 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
> {
> int result;
>
> - if (power_on) {
> - if (!st->powerup_count) {
> - result = inv_mpu6050_pwr_mgmt_1_write(st, false, -1, -1);
> - if (result)
> - return result;
> - usleep_range(INV_MPU6050_REG_UP_TIME_MIN,
> - INV_MPU6050_REG_UP_TIME_MAX);
> - }
> - st->powerup_count++;
> - } else {
> - if (st->powerup_count == 1) {
> - result = inv_mpu6050_pwr_mgmt_1_write(st, true, -1, -1);
> - if (result)
> - return result;
> - }
> - st->powerup_count--;
> - }
> + result = inv_mpu6050_pwr_mgmt_1_write(st, !power_on, -1, -1);
> + if (result)
> + return result;
>
> - dev_dbg(regmap_get_device(st->map), "set power %d, count=%u\n",
> - power_on, st->powerup_count);
> + if (power_on)
> + usleep_range(INV_MPU6050_REG_UP_TIME_MIN,
> + INV_MPU6050_REG_UP_TIME_MAX);
>
> return 0;
> }
> @@ -563,6 +552,7 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> int *val)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct device *pdev = regmap_get_device(st->map);
> unsigned int freq_hz, period_us, min_sleep_us, max_sleep_us;
> int result;
> int ret;
> @@ -571,92 +561,85 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
> period_us = 1000000 / freq_hz;
>
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> + result = pm_runtime_get_sync(pdev);
> + if (result < 0) {
> + pm_runtime_put_noidle(pdev);
> return result;
> + }
>
> switch (chan->type) {
> case IIO_ANGL_VEL:
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_SENSOR_GYRO);
> - if (result)
> - goto error_power_off;
> - /* need to wait 2 periods to have first valid sample */
> - min_sleep_us = 2 * period_us;
> - max_sleep_us = 2 * (period_us + period_us / 2);
> - usleep_range(min_sleep_us, max_sleep_us);
> + if (!st->chip_config.gyro_en) {
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_GYRO);
> + if (result)
> + goto error_power_off;
> + /* need to wait 2 periods to have first valid sample */
> + min_sleep_us = 2 * period_us;
> + max_sleep_us = 2 * (period_us + period_us / 2);
> + usleep_range(min_sleep_us, max_sleep_us);
> + }
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
> chan->channel2, val);
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_SENSOR_GYRO);
> - if (result)
> - goto error_power_off;
> break;
> case IIO_ACCEL:
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_SENSOR_ACCL);
> - if (result)
> - goto error_power_off;
> - /* wait 1 period for first sample availability */
> - min_sleep_us = period_us;
> - max_sleep_us = period_us + period_us / 2;
> - usleep_range(min_sleep_us, max_sleep_us);
> + if (!st->chip_config.accl_en) {
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_ACCL);
> + if (result)
> + goto error_power_off;
> + /* wait 1 period for first sample availability */
> + min_sleep_us = period_us;
> + max_sleep_us = period_us + period_us / 2;
> + usleep_range(min_sleep_us, max_sleep_us);
> + }
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
> chan->channel2, val);
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_SENSOR_ACCL);
> - if (result)
> - goto error_power_off;
> break;
> case IIO_TEMP:
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_SENSOR_TEMP);
> - if (result)
> - goto error_power_off;
> - /* wait 1 period for first sample availability */
> - min_sleep_us = period_us;
> - max_sleep_us = period_us + period_us / 2;
> - usleep_range(min_sleep_us, max_sleep_us);
> + if (!st->chip_config.temp_en) {
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_TEMP);
> + if (result)
> + goto error_power_off;
> + /* wait 1 period for first sample availability */
> + min_sleep_us = period_us;
> + max_sleep_us = period_us + period_us / 2;
> + usleep_range(min_sleep_us, max_sleep_us);
> + }
> ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
> IIO_MOD_X, val);
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_SENSOR_TEMP);
> - if (result)
> - goto error_power_off;
> break;
> case IIO_MAGN:
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_SENSOR_MAGN);
> - if (result)
> - goto error_power_off;
> - /* frequency is limited for magnetometer */
> - if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
> - freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> - period_us = 1000000 / freq_hz;
> + if (!st->chip_config.magn_en) {
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_MAGN);
> + if (result)
> + goto error_power_off;
> + /* frequency is limited for magnetometer */
> + if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
> + freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> + period_us = 1000000 / freq_hz;
> + }
> + /* need to wait 2 periods to have first valid sample */
> + min_sleep_us = 2 * period_us;
> + max_sleep_us = 2 * (period_us + period_us / 2);
> + usleep_range(min_sleep_us, max_sleep_us);
> }
> - /* need to wait 2 periods to have first valid sample */
> - min_sleep_us = 2 * period_us;
> - max_sleep_us = 2 * (period_us + period_us / 2);
> - usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu_magn_read(st, chan->channel2, val);
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_SENSOR_MAGN);
> - if (result)
> - goto error_power_off;
> break;
> default:
> ret = -EINVAL;
> break;
> }
>
> - result = inv_mpu6050_set_power_itg(st, false);
> - if (result)
> - goto error_power_off;
> + pm_runtime_mark_last_busy(pdev);
> + pm_runtime_put_autosuspend(pdev);
>
> return ret;
>
> error_power_off:
> - inv_mpu6050_set_power_itg(st, false);
> + pm_runtime_put_autosuspend(pdev);
> return result;
> }
>
> @@ -795,6 +778,7 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> int val, int val2, long mask)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct device *pdev = regmap_get_device(st->map);
> int result;
>
> /*
> @@ -806,9 +790,11 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> return result;
>
> mutex_lock(&st->lock);
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> + result = pm_runtime_get_sync(pdev);
> + if (result < 0) {
> + pm_runtime_put_noidle(pdev);
> goto error_write_raw_unlock;
> + }
>
> switch (mask) {
> case IIO_CHAN_INFO_SCALE:
> @@ -846,7 +832,8 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> break;
> }
>
> - result |= inv_mpu6050_set_power_itg(st, false);
> + pm_runtime_mark_last_busy(pdev);
> + pm_runtime_put_autosuspend(pdev);
> error_write_raw_unlock:
> mutex_unlock(&st->lock);
> iio_device_release_direct_mode(indio_dev);
> @@ -903,6 +890,7 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> int result;
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct device *pdev = regmap_get_device(st->map);
>
> if (kstrtoint(buf, 10, &fifo_rate))
> return -EINVAL;
> @@ -920,9 +908,11 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> result = 0;
> goto fifo_rate_fail_unlock;
> }
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> + result = pm_runtime_get_sync(pdev);
> + if (result < 0) {
> + pm_runtime_put_noidle(pdev);
> goto fifo_rate_fail_unlock;
> + }
>
> result = regmap_write(st->map, st->reg->sample_rate_div, d);
> if (result)
> @@ -938,8 +928,9 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> if (result)
> goto fifo_rate_fail_power_off;
>
> + pm_runtime_mark_last_busy(pdev);
> fifo_rate_fail_power_off:
> - result |= inv_mpu6050_set_power_itg(st, false);
> + pm_runtime_put_autosuspend(pdev);
> fifo_rate_fail_unlock:
> mutex_unlock(&st->lock);
> if (result)
> @@ -1385,6 +1376,14 @@ static void inv_mpu_core_disable_regulator_action(void *_data)
> inv_mpu_core_disable_regulator_vddio(st);
> }
>
> +static void inv_mpu_pm_disable(void *data)
> +{
> + struct device *dev = data;
> +
> + pm_runtime_put_sync_suspend(dev);
> + pm_runtime_disable(dev);
> +}
> +
> int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type)
> {
> @@ -1409,7 +1408,6 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> st = iio_priv(indio_dev);
> mutex_init(&st->lock);
> st->chip_type = chip_type;
> - st->powerup_count = 0;
> st->irq = irq;
> st->map = regmap;
>
> @@ -1521,8 +1519,16 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> goto error_power_off;
> }
>
> - /* chip init is done, turning off */
> - result = inv_mpu6050_set_power_itg(st, false);
> + /* chip init is done, turning on runtime power management */
> + result = pm_runtime_set_active(dev);
> + if (result)
> + goto error_power_off;
> + pm_runtime_get_noresume(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_set_autosuspend_delay(dev, INV_MPU6050_SUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_put(dev);
> + result = devm_add_action_or_reset(dev, inv_mpu_pm_disable, dev);
> if (result)
> return result;
>
> @@ -1590,11 +1596,10 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> }
> EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
>
> -#ifdef CONFIG_PM_SLEEP
> -
> -static int inv_mpu_resume(struct device *dev)
> +static int __maybe_unused inv_mpu_resume(struct device *dev)
> {
> - struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct inv_mpu6050_state *st = iio_priv(indio_dev);
> int result;
>
> mutex_lock(&st->lock);
> @@ -1603,27 +1608,101 @@ static int inv_mpu_resume(struct device *dev)
> goto out_unlock;
>
> result = inv_mpu6050_set_power_itg(st, true);
> + if (result)
> + goto out_unlock;
> +
> + result = inv_mpu6050_switch_engine(st, true, st->suspended_sensors);
> + if (result)
> + goto out_unlock;
> +
> + if (iio_buffer_enabled(indio_dev))
> + result = inv_mpu6050_prepare_fifo(st, true);
> +
> out_unlock:
> mutex_unlock(&st->lock);
>
> return result;
> }
>
> -static int inv_mpu_suspend(struct device *dev)
> +static int __maybe_unused inv_mpu_suspend(struct device *dev)
> {
> - struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct inv_mpu6050_state *st = iio_priv(indio_dev);
> int result;
>
> mutex_lock(&st->lock);
> +
> + if (iio_buffer_enabled(indio_dev)) {
> + result = inv_mpu6050_prepare_fifo(st, false);
> + if (result)
> + goto out_unlock;
> + }
> +
> + st->suspended_sensors = 0;
> + if (st->chip_config.accl_en)
> + st->suspended_sensors |= INV_MPU6050_SENSOR_ACCL;
> + if (st->chip_config.gyro_en)
> + st->suspended_sensors |= INV_MPU6050_SENSOR_GYRO;
> + if (st->chip_config.temp_en)
> + st->suspended_sensors |= INV_MPU6050_SENSOR_TEMP;
> + if (st->chip_config.magn_en)
> + st->suspended_sensors |= INV_MPU6050_SENSOR_MAGN;
> + result = inv_mpu6050_switch_engine(st, false, st->suspended_sensors);
> + if (result)
> + goto out_unlock;
> +
> result = inv_mpu6050_set_power_itg(st, false);
> + if (result)
> + goto out_unlock;
> +
> inv_mpu_core_disable_regulator_vddio(st);
> +out_unlock:
> mutex_unlock(&st->lock);
>
> return result;
> }
> -#endif /* CONFIG_PM_SLEEP */
>
> -SIMPLE_DEV_PM_OPS(inv_mpu_pmops, inv_mpu_suspend, inv_mpu_resume);
> +static int __maybe_unused inv_mpu_runtime_suspend(struct device *dev)
> +{
> + struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> + unsigned int sensors;
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + sensors = INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_GYRO |
> + INV_MPU6050_SENSOR_TEMP | INV_MPU6050_SENSOR_MAGN;
> + ret = inv_mpu6050_switch_engine(st, false, sensors);
> + if (ret)
> + goto out_unlock;
> +
> + ret = inv_mpu6050_set_power_itg(st, false);
> + if (ret)
> + goto out_unlock;
> +
> + inv_mpu_core_disable_regulator_vddio(st);
> +
> +out_unlock:
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +static int __maybe_unused inv_mpu_runtime_resume(struct device *dev)
> +{
> + struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> + int ret;
> +
> + ret = inv_mpu_core_enable_regulator_vddio(st);
> + if (ret)
> + return ret;
> +
> + return inv_mpu6050_set_power_itg(st, true);
> +}
> +
> +const struct dev_pm_ops inv_mpu_pmops = {
> + SET_SYSTEM_SLEEP_PM_OPS(inv_mpu_suspend, inv_mpu_resume)
> + SET_RUNTIME_PM_OPS(inv_mpu_runtime_suspend, inv_mpu_runtime_resume, NULL)
> +};
> EXPORT_SYMBOL_GPL(inv_mpu_pmops);
>
> MODULE_AUTHOR("Invensense Corporation");
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e328c98e362c..cd38b3fccc7b 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -164,6 +164,7 @@ struct inv_mpu6050_hw {
> * @magn_disabled: magnetometer disabled for backward compatibility reason.
> * @magn_raw_to_gauss: coefficient to convert mag raw value to Gauss.
> * @magn_orient: magnetometer sensor chip orientation if available.
> + * @suspended_sensors: sensors mask of sensors turned off for suspend
> */
> struct inv_mpu6050_state {
> struct mutex lock;
> @@ -174,7 +175,6 @@ struct inv_mpu6050_state {
> enum inv_devices chip_type;
> struct i2c_mux_core *muxc;
> struct i2c_client *mux_client;
> - unsigned int powerup_count;
> struct inv_mpu6050_platform_data plat_data;
> struct iio_mount_matrix orientation;
> struct regmap *map;
> @@ -189,6 +189,7 @@ struct inv_mpu6050_state {
> bool magn_disabled;
> s32 magn_raw_to_gauss[3];
> struct iio_mount_matrix magn_orient;
> + unsigned int suspended_sensors;
> };
>
> /*register and associated bit definition*/
> @@ -312,6 +313,7 @@ struct inv_mpu6050_state {
> #define INV_MPU6050_ACCEL_UP_TIME 20
> #define INV_MPU6050_GYRO_UP_TIME 35
> #define INV_MPU6050_GYRO_DOWN_TIME 150
> +#define INV_MPU6050_SUSPEND_DELAY_MS 2000
>
> /* delay time in microseconds */
> #define INV_MPU6050_REG_UP_TIME_MIN 5000
> @@ -439,7 +441,6 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable);
> int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
> unsigned int mask);
> int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 val);
> -int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on);
Having done this, function should be marked static in _core.c
I've done so.
> int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
> void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
> int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index f53f50d08b9e..f7b5a70be30f 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -3,6 +3,7 @@
> * Copyright (C) 2012 Invensense, Inc.
> */
>
> +#include <linux/pm_runtime.h>
> #include "inv_mpu_iio.h"
>
> static unsigned int inv_scan_query_mpu6050(struct iio_dev *indio_dev)
> @@ -156,41 +157,43 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable)
> static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct device *pdev = regmap_get_device(st->map);
> unsigned int scan;
> int result;
>
> - scan = inv_scan_query(indio_dev);
> -
> if (enable) {
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> + scan = inv_scan_query(indio_dev);
> + result = pm_runtime_get_sync(pdev);
> + if (result < 0) {
> + pm_runtime_put_noidle(pdev);
> return result;
> + }
> + /*
> + * In case autosuspend didn't trigger, turn off first not
> + * required sensors.
> + */
> + result = inv_mpu6050_switch_engine(st, false, ~scan);
> + if (result)
> + goto error_power_off;
> result = inv_mpu6050_switch_engine(st, true, scan);
> if (result)
> goto error_power_off;
> st->skip_samples = inv_compute_skip_samples(st);
> result = inv_mpu6050_prepare_fifo(st, true);
> if (result)
> - goto error_sensors_off;
> + goto error_power_off;
> } else {
> result = inv_mpu6050_prepare_fifo(st, false);
> - if (result)
> - goto error_sensors_off;
> - result = inv_mpu6050_switch_engine(st, false, scan);
> - if (result)
> - goto error_power_off;
> -
> - result = inv_mpu6050_set_power_itg(st, false);
> if (result)
> goto error_power_off;
> + pm_runtime_mark_last_busy(pdev);
> + pm_runtime_put_autosuspend(pdev);
> }
>
> return 0;
>
> -error_sensors_off:
> - inv_mpu6050_switch_engine(st, false, scan);
> error_power_off:
> - inv_mpu6050_set_power_itg(st, false);
> + pm_runtime_put_autosuspend(pdev);
> return result;
> }
>
poer
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend
2020-02-21 11:54 ` Jonathan Cameron
@ 2020-02-21 14:04 ` Jean-Baptiste Maneyrol
0 siblings, 0 replies; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-21 14:04 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio
Thanks for catching this up.
JB
From: Jonathan Cameron <jic23@kernel.org>
Sent: Friday, February 21, 2020 12:54
To: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
Subject: Re: [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend
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 Wed, 19 Feb 2020 15:39:57 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Use runtime power management for handling chip power and
> sensor engines on/off. Simplifies things a lot since pm
> runtime already has reference counter.
> Usage of autosuspend reduces the number of power on/off. This
> makes polling interface now usable to get data at low
> frequency.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Missing static marking after reducing scope of one function.
See inline. I've fixed that up and applied.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 267 ++++++++++++------
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 5 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 33 ++-
> 3 files changed, 194 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 9076b6bb099c..750fbc2614f0 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -16,6 +16,8 @@
> #include <linux/acpi.h>
> #include <linux/platform_device.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> #include "inv_mpu_iio.h"
> #include "inv_mpu_magn.h"
>
> @@ -400,26 +402,13 @@ int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on)
> {
> int result;
>
> - if (power_on) {
> - if (!st->powerup_count) {
> - result = inv_mpu6050_pwr_mgmt_1_write(st, false, -1, -1);
> - if (result)
> - return result;
> - usleep_range(INV_MPU6050_REG_UP_TIME_MIN,
> - INV_MPU6050_REG_UP_TIME_MAX);
> - }
> - st->powerup_count++;
> - } else {
> - if (st->powerup_count == 1) {
> - result = inv_mpu6050_pwr_mgmt_1_write(st, true, -1, -1);
> - if (result)
> - return result;
> - }
> - st->powerup_count--;
> - }
> + result = inv_mpu6050_pwr_mgmt_1_write(st, !power_on, -1, -1);
> + if (result)
> + return result;
>
> - dev_dbg(regmap_get_device(st->map), "set power %d, count=%u\n",
> - power_on, st->powerup_count);
> + if (power_on)
> + usleep_range(INV_MPU6050_REG_UP_TIME_MIN,
> + INV_MPU6050_REG_UP_TIME_MAX);
>
> return 0;
> }
> @@ -563,6 +552,7 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> int *val)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct device *pdev = regmap_get_device(st->map);
> unsigned int freq_hz, period_us, min_sleep_us, max_sleep_us;
> int result;
> int ret;
> @@ -571,92 +561,85 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> freq_hz = INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
> period_us = 1000000 / freq_hz;
>
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> + result = pm_runtime_get_sync(pdev);
> + if (result < 0) {
> + pm_runtime_put_noidle(pdev);
> return result;
> + }
>
> switch (chan->type) {
> case IIO_ANGL_VEL:
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_SENSOR_GYRO);
> - if (result)
> - goto error_power_off;
> - /* need to wait 2 periods to have first valid sample */
> - min_sleep_us = 2 * period_us;
> - max_sleep_us = 2 * (period_us + period_us / 2);
> - usleep_range(min_sleep_us, max_sleep_us);
> + if (!st->chip_config.gyro_en) {
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_GYRO);
> + if (result)
> + goto error_power_off;
> + /* need to wait 2 periods to have first valid sample */
> + min_sleep_us = 2 * period_us;
> + max_sleep_us = 2 * (period_us + period_us / 2);
> + usleep_range(min_sleep_us, max_sleep_us);
> + }
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
> chan->channel2, val);
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_SENSOR_GYRO);
> - if (result)
> - goto error_power_off;
> break;
> case IIO_ACCEL:
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_SENSOR_ACCL);
> - if (result)
> - goto error_power_off;
> - /* wait 1 period for first sample availability */
> - min_sleep_us = period_us;
> - max_sleep_us = period_us + period_us / 2;
> - usleep_range(min_sleep_us, max_sleep_us);
> + if (!st->chip_config.accl_en) {
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_ACCL);
> + if (result)
> + goto error_power_off;
> + /* wait 1 period for first sample availability */
> + min_sleep_us = period_us;
> + max_sleep_us = period_us + period_us / 2;
> + usleep_range(min_sleep_us, max_sleep_us);
> + }
> ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
> chan->channel2, val);
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_SENSOR_ACCL);
> - if (result)
> - goto error_power_off;
> break;
> case IIO_TEMP:
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_SENSOR_TEMP);
> - if (result)
> - goto error_power_off;
> - /* wait 1 period for first sample availability */
> - min_sleep_us = period_us;
> - max_sleep_us = period_us + period_us / 2;
> - usleep_range(min_sleep_us, max_sleep_us);
> + if (!st->chip_config.temp_en) {
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_TEMP);
> + if (result)
> + goto error_power_off;
> + /* wait 1 period for first sample availability */
> + min_sleep_us = period_us;
> + max_sleep_us = period_us + period_us / 2;
> + usleep_range(min_sleep_us, max_sleep_us);
> + }
> ret = inv_mpu6050_sensor_show(st, st->reg->temperature,
> IIO_MOD_X, val);
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_SENSOR_TEMP);
> - if (result)
> - goto error_power_off;
> break;
> case IIO_MAGN:
> - result = inv_mpu6050_switch_engine(st, true,
> - INV_MPU6050_SENSOR_MAGN);
> - if (result)
> - goto error_power_off;
> - /* frequency is limited for magnetometer */
> - if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
> - freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> - period_us = 1000000 / freq_hz;
> + if (!st->chip_config.magn_en) {
> + result = inv_mpu6050_switch_engine(st, true,
> + INV_MPU6050_SENSOR_MAGN);
> + if (result)
> + goto error_power_off;
> + /* frequency is limited for magnetometer */
> + if (freq_hz > INV_MPU_MAGN_FREQ_HZ_MAX) {
> + freq_hz = INV_MPU_MAGN_FREQ_HZ_MAX;
> + period_us = 1000000 / freq_hz;
> + }
> + /* need to wait 2 periods to have first valid sample */
> + min_sleep_us = 2 * period_us;
> + max_sleep_us = 2 * (period_us + period_us / 2);
> + usleep_range(min_sleep_us, max_sleep_us);
> }
> - /* need to wait 2 periods to have first valid sample */
> - min_sleep_us = 2 * period_us;
> - max_sleep_us = 2 * (period_us + period_us / 2);
> - usleep_range(min_sleep_us, max_sleep_us);
> ret = inv_mpu_magn_read(st, chan->channel2, val);
> - result = inv_mpu6050_switch_engine(st, false,
> - INV_MPU6050_SENSOR_MAGN);
> - if (result)
> - goto error_power_off;
> break;
> default:
> ret = -EINVAL;
> break;
> }
>
> - result = inv_mpu6050_set_power_itg(st, false);
> - if (result)
> - goto error_power_off;
> + pm_runtime_mark_last_busy(pdev);
> + pm_runtime_put_autosuspend(pdev);
>
> return ret;
>
> error_power_off:
> - inv_mpu6050_set_power_itg(st, false);
> + pm_runtime_put_autosuspend(pdev);
> return result;
> }
>
> @@ -795,6 +778,7 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> int val, int val2, long mask)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct device *pdev = regmap_get_device(st->map);
> int result;
>
> /*
> @@ -806,9 +790,11 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> return result;
>
> mutex_lock(&st->lock);
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> + result = pm_runtime_get_sync(pdev);
> + if (result < 0) {
> + pm_runtime_put_noidle(pdev);
> goto error_write_raw_unlock;
> + }
>
> switch (mask) {
> case IIO_CHAN_INFO_SCALE:
> @@ -846,7 +832,8 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> break;
> }
>
> - result |= inv_mpu6050_set_power_itg(st, false);
> + pm_runtime_mark_last_busy(pdev);
> + pm_runtime_put_autosuspend(pdev);
> error_write_raw_unlock:
> mutex_unlock(&st->lock);
> iio_device_release_direct_mode(indio_dev);
> @@ -903,6 +890,7 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> int result;
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct device *pdev = regmap_get_device(st->map);
>
> if (kstrtoint(buf, 10, &fifo_rate))
> return -EINVAL;
> @@ -920,9 +908,11 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> result = 0;
> goto fifo_rate_fail_unlock;
> }
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> + result = pm_runtime_get_sync(pdev);
> + if (result < 0) {
> + pm_runtime_put_noidle(pdev);
> goto fifo_rate_fail_unlock;
> + }
>
> result = regmap_write(st->map, st->reg->sample_rate_div, d);
> if (result)
> @@ -938,8 +928,9 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> if (result)
> goto fifo_rate_fail_power_off;
>
> + pm_runtime_mark_last_busy(pdev);
> fifo_rate_fail_power_off:
> - result |= inv_mpu6050_set_power_itg(st, false);
> + pm_runtime_put_autosuspend(pdev);
> fifo_rate_fail_unlock:
> mutex_unlock(&st->lock);
> if (result)
> @@ -1385,6 +1376,14 @@ static void inv_mpu_core_disable_regulator_action(void *_data)
> inv_mpu_core_disable_regulator_vddio(st);
> }
>
> +static void inv_mpu_pm_disable(void *data)
> +{
> + struct device *dev = data;
> +
> + pm_runtime_put_sync_suspend(dev);
> + pm_runtime_disable(dev);
> +}
> +
> int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type)
> {
> @@ -1409,7 +1408,6 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> st = iio_priv(indio_dev);
> mutex_init(&st->lock);
> st->chip_type = chip_type;
> - st->powerup_count = 0;
> st->irq = irq;
> st->map = regmap;
>
> @@ -1521,8 +1519,16 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> goto error_power_off;
> }
>
> - /* chip init is done, turning off */
> - result = inv_mpu6050_set_power_itg(st, false);
> + /* chip init is done, turning on runtime power management */
> + result = pm_runtime_set_active(dev);
> + if (result)
> + goto error_power_off;
> + pm_runtime_get_noresume(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_set_autosuspend_delay(dev, INV_MPU6050_SUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_put(dev);
> + result = devm_add_action_or_reset(dev, inv_mpu_pm_disable, dev);
> if (result)
> return result;
>
> @@ -1590,11 +1596,10 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> }
> EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
>
> -#ifdef CONFIG_PM_SLEEP
> -
> -static int inv_mpu_resume(struct device *dev)
> +static int __maybe_unused inv_mpu_resume(struct device *dev)
> {
> - struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct inv_mpu6050_state *st = iio_priv(indio_dev);
> int result;
>
> mutex_lock(&st->lock);
> @@ -1603,27 +1608,101 @@ static int inv_mpu_resume(struct device *dev)
> goto out_unlock;
>
> result = inv_mpu6050_set_power_itg(st, true);
> + if (result)
> + goto out_unlock;
> +
> + result = inv_mpu6050_switch_engine(st, true, st->suspended_sensors);
> + if (result)
> + goto out_unlock;
> +
> + if (iio_buffer_enabled(indio_dev))
> + result = inv_mpu6050_prepare_fifo(st, true);
> +
> out_unlock:
> mutex_unlock(&st->lock);
>
> return result;
> }
>
> -static int inv_mpu_suspend(struct device *dev)
> +static int __maybe_unused inv_mpu_suspend(struct device *dev)
> {
> - struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct inv_mpu6050_state *st = iio_priv(indio_dev);
> int result;
>
> mutex_lock(&st->lock);
> +
> + if (iio_buffer_enabled(indio_dev)) {
> + result = inv_mpu6050_prepare_fifo(st, false);
> + if (result)
> + goto out_unlock;
> + }
> +
> + st->suspended_sensors = 0;
> + if (st->chip_config.accl_en)
> + st->suspended_sensors |= INV_MPU6050_SENSOR_ACCL;
> + if (st->chip_config.gyro_en)
> + st->suspended_sensors |= INV_MPU6050_SENSOR_GYRO;
> + if (st->chip_config.temp_en)
> + st->suspended_sensors |= INV_MPU6050_SENSOR_TEMP;
> + if (st->chip_config.magn_en)
> + st->suspended_sensors |= INV_MPU6050_SENSOR_MAGN;
> + result = inv_mpu6050_switch_engine(st, false, st->suspended_sensors);
> + if (result)
> + goto out_unlock;
> +
> result = inv_mpu6050_set_power_itg(st, false);
> + if (result)
> + goto out_unlock;
> +
> inv_mpu_core_disable_regulator_vddio(st);
> +out_unlock:
> mutex_unlock(&st->lock);
>
> return result;
> }
> -#endif /* CONFIG_PM_SLEEP */
>
> -SIMPLE_DEV_PM_OPS(inv_mpu_pmops, inv_mpu_suspend, inv_mpu_resume);
> +static int __maybe_unused inv_mpu_runtime_suspend(struct device *dev)
> +{
> + struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> + unsigned int sensors;
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + sensors = INV_MPU6050_SENSOR_ACCL | INV_MPU6050_SENSOR_GYRO |
> + INV_MPU6050_SENSOR_TEMP | INV_MPU6050_SENSOR_MAGN;
> + ret = inv_mpu6050_switch_engine(st, false, sensors);
> + if (ret)
> + goto out_unlock;
> +
> + ret = inv_mpu6050_set_power_itg(st, false);
> + if (ret)
> + goto out_unlock;
> +
> + inv_mpu_core_disable_regulator_vddio(st);
> +
> +out_unlock:
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +static int __maybe_unused inv_mpu_runtime_resume(struct device *dev)
> +{
> + struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(dev));
> + int ret;
> +
> + ret = inv_mpu_core_enable_regulator_vddio(st);
> + if (ret)
> + return ret;
> +
> + return inv_mpu6050_set_power_itg(st, true);
> +}
> +
> +const struct dev_pm_ops inv_mpu_pmops = {
> + SET_SYSTEM_SLEEP_PM_OPS(inv_mpu_suspend, inv_mpu_resume)
> + SET_RUNTIME_PM_OPS(inv_mpu_runtime_suspend, inv_mpu_runtime_resume, NULL)
> +};
> EXPORT_SYMBOL_GPL(inv_mpu_pmops);
>
> MODULE_AUTHOR("Invensense Corporation");
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e328c98e362c..cd38b3fccc7b 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -164,6 +164,7 @@ struct inv_mpu6050_hw {
> * @magn_disabled: magnetometer disabled for backward compatibility reason.
> * @magn_raw_to_gauss: coefficient to convert mag raw value to Gauss.
> * @magn_orient: magnetometer sensor chip orientation if available.
> + * @suspended_sensors: sensors mask of sensors turned off for suspend
> */
> struct inv_mpu6050_state {
> struct mutex lock;
> @@ -174,7 +175,6 @@ struct inv_mpu6050_state {
> enum inv_devices chip_type;
> struct i2c_mux_core *muxc;
> struct i2c_client *mux_client;
> - unsigned int powerup_count;
> struct inv_mpu6050_platform_data plat_data;
> struct iio_mount_matrix orientation;
> struct regmap *map;
> @@ -189,6 +189,7 @@ struct inv_mpu6050_state {
> bool magn_disabled;
> s32 magn_raw_to_gauss[3];
> struct iio_mount_matrix magn_orient;
> + unsigned int suspended_sensors;
> };
>
> /*register and associated bit definition*/
> @@ -312,6 +313,7 @@ struct inv_mpu6050_state {
> #define INV_MPU6050_ACCEL_UP_TIME 20
> #define INV_MPU6050_GYRO_UP_TIME 35
> #define INV_MPU6050_GYRO_DOWN_TIME 150
> +#define INV_MPU6050_SUSPEND_DELAY_MS 2000
>
> /* delay time in microseconds */
> #define INV_MPU6050_REG_UP_TIME_MIN 5000
> @@ -439,7 +441,6 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable);
> int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en,
> unsigned int mask);
> int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 val);
> -int inv_mpu6050_set_power_itg(struct inv_mpu6050_state *st, bool power_on);
Having done this, function should be marked static in _core.c
I've done so.
> int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
> void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
> int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index f53f50d08b9e..f7b5a70be30f 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -3,6 +3,7 @@
> * Copyright (C) 2012 Invensense, Inc.
> */
>
> +#include <linux/pm_runtime.h>
> #include "inv_mpu_iio.h"
>
> static unsigned int inv_scan_query_mpu6050(struct iio_dev *indio_dev)
> @@ -156,41 +157,43 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable)
> static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
> {
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> + struct device *pdev = regmap_get_device(st->map);
> unsigned int scan;
> int result;
>
> - scan = inv_scan_query(indio_dev);
> -
> if (enable) {
> - result = inv_mpu6050_set_power_itg(st, true);
> - if (result)
> + scan = inv_scan_query(indio_dev);
> + result = pm_runtime_get_sync(pdev);
> + if (result < 0) {
> + pm_runtime_put_noidle(pdev);
> return result;
> + }
> + /*
> + * In case autosuspend didn't trigger, turn off first not
> + * required sensors.
> + */
> + result = inv_mpu6050_switch_engine(st, false, ~scan);
> + if (result)
> + goto error_power_off;
> result = inv_mpu6050_switch_engine(st, true, scan);
> if (result)
> goto error_power_off;
> st->skip_samples = inv_compute_skip_samples(st);
> result = inv_mpu6050_prepare_fifo(st, true);
> if (result)
> - goto error_sensors_off;
> + goto error_power_off;
> } else {
> result = inv_mpu6050_prepare_fifo(st, false);
> - if (result)
> - goto error_sensors_off;
> - result = inv_mpu6050_switch_engine(st, false, scan);
> - if (result)
> - goto error_power_off;
> -
> - result = inv_mpu6050_set_power_itg(st, false);
> if (result)
> goto error_power_off;
> + pm_runtime_mark_last_busy(pdev);
> + pm_runtime_put_autosuspend(pdev);
> }
>
> return 0;
>
> -error_sensors_off:
> - inv_mpu6050_switch_engine(st, false, scan);
> error_power_off:
> - inv_mpu6050_set_power_itg(st, false);
> + pm_runtime_put_autosuspend(pdev);
> return result;
> }
>
poer
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend
2020-02-19 14:39 ` [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend Jean-Baptiste Maneyrol
2020-02-21 11:54 ` Jonathan Cameron
@ 2020-03-24 21:24 ` Dmitry Osipenko
2020-03-25 19:21 ` Jean-Baptiste Maneyrol
1 sibling, 1 reply; 30+ messages in thread
From: Dmitry Osipenko @ 2020-03-24 21:24 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol, jic23, linux-iio
19.02.2020 17:39, Jean-Baptiste Maneyrol пишет:
> Use runtime power management for handling chip power and
> sensor engines on/off. Simplifies things a lot since pm
> runtime already has reference counter.
> Usage of autosuspend reduces the number of power on/off. This
> makes polling interface now usable to get data at low
> frequency.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 267 ++++++++++++------
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 5 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 33 ++-
> 3 files changed, 194 insertions(+), 111 deletions(-)
Hello,
This patch introduces a system suspend-resume regressing, I'm seeing the
following on entering into suspend:
vdd_gen1v8: Underflow of regulator enable count
inv-mpu6050-i2c 1-0068: Failed to disable vddio regulator: -22
Apparently RPM disables the regulator and then it's erroneously disabled
second time on suspend. Please fix it, thanks in advance.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend
2020-03-24 21:24 ` Dmitry Osipenko
@ 2020-03-25 19:21 ` Jean-Baptiste Maneyrol
2020-03-25 19:49 ` Dmitry Osipenko
0 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-03-25 19:21 UTC (permalink / raw)
To: Dmitry Osipenko, jic23, linux-iio
Hello Dmitry,
that's right, thanks for finding it.
Hopefully this will not prevent system suspend since we are not checking on the return value.
System suspend/resume callbacks need to be reworked to work correctly with pm_runtime.
I will send a fix patch when it is ready.
Thanks,
JB
From: Dmitry Osipenko <digetx@gmail.com>
Sent: Tuesday, March 24, 2020 22:24
To: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>; jic23@kernel.org <jic23@kernel.org>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
Subject: Re: [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend
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.
19.02.2020 17:39, Jean-Baptiste Maneyrol пишет:
> Use runtime power management for handling chip power and
> sensor engines on/off. Simplifies things a lot since pm
> runtime already has reference counter.
> Usage of autosuspend reduces the number of power on/off. This
> makes polling interface now usable to get data at low
> frequency.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 267 ++++++++++++------
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 5 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 33 ++-
> 3 files changed, 194 insertions(+), 111 deletions(-)
Hello,
This patch introduces a system suspend-resume regressing, I'm seeing the
following on entering into suspend:
vdd_gen1v8: Underflow of regulator enable count
inv-mpu6050-i2c 1-0068: Failed to disable vddio regulator: -22
Apparently RPM disables the regulator and then it's erroneously disabled
second time on suspend. Please fix it, thanks in advance.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend
2020-03-25 19:21 ` Jean-Baptiste Maneyrol
@ 2020-03-25 19:49 ` Dmitry Osipenko
0 siblings, 0 replies; 30+ messages in thread
From: Dmitry Osipenko @ 2020-03-25 19:49 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol, jic23, linux-iio
25.03.2020 22:21, Jean-Baptiste Maneyrol пишет:
> Hello Dmitry,
>
> that's right, thanks for finding it.
>
> Hopefully this will not prevent system suspend since we are not checking on the return value.
Probably should be okay.
> System suspend/resume callbacks need to be reworked to work correctly with pm_runtime.
> I will send a fix patch when it is ready.
Thank you very much.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 13/13] iio: imu: inv_mpu6050: temperature only work with accel/gyro
2020-02-19 14:39 [PATCH v2 00/13] Rework sensors engines and power management Jean-Baptiste Maneyrol
` (11 preceding siblings ...)
2020-02-19 14:39 ` [PATCH v2 12/13] iio: imu: inv_mpu6050: use runtime pm with autosuspend Jean-Baptiste Maneyrol
@ 2020-02-19 14:39 ` Jean-Baptiste Maneyrol
2020-02-21 11:56 ` Jonathan Cameron
12 siblings, 1 reply; 30+ messages in thread
From: Jean-Baptiste Maneyrol @ 2020-02-19 14:39 UTC (permalink / raw)
To: jic23, linux-iio; +Cc: Jean-Baptiste Maneyrol
Temperature sensor works correctly only when accel and/or gyro
is turned on. Prevent polling value if they are not running.
Anyway it doesn't make sense to use it without sensor engines
on.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 750fbc2614f0..94a989368b94 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -597,6 +597,11 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
chan->channel2, val);
break;
case IIO_TEMP:
+ /* temperature sensor work only with accel and/or gyro */
+ if (!st->chip_config.accl_en && !st->chip_config.gyro_en) {
+ result = -EBUSY;
+ goto error_power_off;
+ }
if (!st->chip_config.temp_en) {
result = inv_mpu6050_switch_engine(st, true,
INV_MPU6050_SENSOR_TEMP);
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v2 13/13] iio: imu: inv_mpu6050: temperature only work with accel/gyro
2020-02-19 14:39 ` [PATCH v2 13/13] iio: imu: inv_mpu6050: temperature only work with accel/gyro Jean-Baptiste Maneyrol
@ 2020-02-21 11:56 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2020-02-21 11:56 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol; +Cc: linux-iio
On Wed, 19 Feb 2020 15:39:58 +0100
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Temperature sensor works correctly only when accel and/or gyro
> is turned on. Prevent polling value if they are not running.
> Anyway it doesn't make sense to use it without sensor engines
> on.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Applied.
Thanks
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 750fbc2614f0..94a989368b94 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -597,6 +597,11 @@ static int inv_mpu6050_read_channel_data(struct iio_dev *indio_dev,
> chan->channel2, val);
> break;
> case IIO_TEMP:
> + /* temperature sensor work only with accel and/or gyro */
> + if (!st->chip_config.accl_en && !st->chip_config.gyro_en) {
> + result = -EBUSY;
> + goto error_power_off;
> + }
> if (!st->chip_config.temp_en) {
> result = inv_mpu6050_switch_engine(st, true,
> INV_MPU6050_SENSOR_TEMP);
^ permalink raw reply [flat|nested] 30+ messages in thread