* [PATCH v2] iio: imu: st_lsm6dsx: do not apply ODR configuration in write_raw handler
@ 2017-03-27 21:42 Lorenzo Bianconi
2017-04-02 10:33 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Bianconi @ 2017-03-27 21:42 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, lorenzo.bianconi
This patch allows to avoid a transitory that occurs when a given sensor
has been already enabled (e.g. gyroscope) and the user is configuring
the sample frequency of the other one (e.g. accelerometer).
Until the accelerometer is enabled gyroscope ODR is modified as well.
Fix it introducing st_lsm6dsx_check_odr() routine to check ODR consistency
in write_raw handler in order to apply frequency configuration just
in st_lsm6dsx_set_odr()
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
---
Changes since v1:
- Rename st_lsm6dsx_get_odr_val() in st_lsm6dsx_check_odr()
---
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 41 ++++++++++++++++++----------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index c433223..f80a3d4 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -308,32 +308,40 @@ static int st_lsm6dsx_set_full_scale(struct st_lsm6dsx_sensor *sensor,
return 0;
}
-static int st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u16 odr)
+static int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u16 odr,
+ u8 *val)
{
- enum st_lsm6dsx_sensor_id id = sensor->id;
- int i, err;
- u8 val;
+ int i;
for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++)
- if (st_lsm6dsx_odr_table[id].odr_avl[i].hz == odr)
+ if (st_lsm6dsx_odr_table[sensor->id].odr_avl[i].hz == odr)
break;
if (i == ST_LSM6DSX_ODR_LIST_SIZE)
return -EINVAL;
- val = st_lsm6dsx_odr_table[id].odr_avl[i].val;
- err = st_lsm6dsx_write_with_mask(sensor->hw,
- st_lsm6dsx_odr_table[id].reg.addr,
- st_lsm6dsx_odr_table[id].reg.mask,
- val);
- if (err < 0)
- return err;
-
+ *val = st_lsm6dsx_odr_table[sensor->id].odr_avl[i].val;
sensor->odr = odr;
return 0;
}
+static int st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u16 odr)
+{
+ enum st_lsm6dsx_sensor_id id = sensor->id;
+ int err;
+ u8 val;
+
+ err = st_lsm6dsx_check_odr(sensor, odr, &val);
+ if (err < 0)
+ return err;
+
+ return st_lsm6dsx_write_with_mask(sensor->hw,
+ st_lsm6dsx_odr_table[id].reg.addr,
+ st_lsm6dsx_odr_table[id].reg.mask,
+ val);
+}
+
int st_lsm6dsx_sensor_enable(struct st_lsm6dsx_sensor *sensor)
{
int err;
@@ -436,9 +444,12 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
case IIO_CHAN_INFO_SCALE:
err = st_lsm6dsx_set_full_scale(sensor, val2);
break;
- case IIO_CHAN_INFO_SAMP_FREQ:
- err = st_lsm6dsx_set_odr(sensor, val);
+ case IIO_CHAN_INFO_SAMP_FREQ: {
+ u8 data;
+
+ err = st_lsm6dsx_check_odr(sensor, val, &data);
break;
+ }
default:
err = -EINVAL;
break;
--
2.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: imu: st_lsm6dsx: do not apply ODR configuration in write_raw handler
2017-03-27 21:42 [PATCH v2] iio: imu: st_lsm6dsx: do not apply ODR configuration in write_raw handler Lorenzo Bianconi
@ 2017-04-02 10:33 ` Jonathan Cameron
2017-04-02 11:55 ` Lorenzo Bianconi
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2017-04-02 10:33 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, lorenzo.bianconi
On 27/03/17 22:42, Lorenzo Bianconi wrote:
> This patch allows to avoid a transitory that occurs when a given sensor
> has been already enabled (e.g. gyroscope) and the user is configuring
> the sample frequency of the other one (e.g. accelerometer).
> Until the accelerometer is enabled gyroscope ODR is modified as well.
> Fix it introducing st_lsm6dsx_check_odr() routine to check ODR consistency
> in write_raw handler in order to apply frequency configuration just
> in st_lsm6dsx_set_odr()
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
Hi Lorenzo,
Patch is great, but to make my life easier could you add a fixes
tag + give me some idea of how important the issue is.
I need to know if I should be rushing it into the kernel or the
next merge window is fine.
Thanks,
Jonathan
> ---
> Changes since v1:
> - Rename st_lsm6dsx_get_odr_val() in st_lsm6dsx_check_odr()
> ---
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 41 ++++++++++++++++++----------
> 1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index c433223..f80a3d4 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -308,32 +308,40 @@ static int st_lsm6dsx_set_full_scale(struct st_lsm6dsx_sensor *sensor,
> return 0;
> }
>
> -static int st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u16 odr)
> +static int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u16 odr,
> + u8 *val)
> {
> - enum st_lsm6dsx_sensor_id id = sensor->id;
> - int i, err;
> - u8 val;
> + int i;
>
> for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++)
> - if (st_lsm6dsx_odr_table[id].odr_avl[i].hz == odr)
> + if (st_lsm6dsx_odr_table[sensor->id].odr_avl[i].hz == odr)
> break;
>
> if (i == ST_LSM6DSX_ODR_LIST_SIZE)
> return -EINVAL;
>
> - val = st_lsm6dsx_odr_table[id].odr_avl[i].val;
> - err = st_lsm6dsx_write_with_mask(sensor->hw,
> - st_lsm6dsx_odr_table[id].reg.addr,
> - st_lsm6dsx_odr_table[id].reg.mask,
> - val);
> - if (err < 0)
> - return err;
> -
> + *val = st_lsm6dsx_odr_table[sensor->id].odr_avl[i].val;
> sensor->odr = odr;
>
> return 0;
> }
>
> +static int st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u16 odr)
> +{
> + enum st_lsm6dsx_sensor_id id = sensor->id;
> + int err;
> + u8 val;
> +
> + err = st_lsm6dsx_check_odr(sensor, odr, &val);
> + if (err < 0)
> + return err;
> +
> + return st_lsm6dsx_write_with_mask(sensor->hw,
> + st_lsm6dsx_odr_table[id].reg.addr,
> + st_lsm6dsx_odr_table[id].reg.mask,
> + val);
> +}
> +
> int st_lsm6dsx_sensor_enable(struct st_lsm6dsx_sensor *sensor)
> {
> int err;
> @@ -436,9 +444,12 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
> case IIO_CHAN_INFO_SCALE:
> err = st_lsm6dsx_set_full_scale(sensor, val2);
> break;
> - case IIO_CHAN_INFO_SAMP_FREQ:
> - err = st_lsm6dsx_set_odr(sensor, val);
> + case IIO_CHAN_INFO_SAMP_FREQ: {
> + u8 data;
> +
> + err = st_lsm6dsx_check_odr(sensor, val, &data);
> break;
> + }
> default:
> err = -EINVAL;
> break;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: imu: st_lsm6dsx: do not apply ODR configuration in write_raw handler
2017-04-02 10:33 ` Jonathan Cameron
@ 2017-04-02 11:55 ` Lorenzo Bianconi
0 siblings, 0 replies; 3+ messages in thread
From: Lorenzo Bianconi @ 2017-04-02 11:55 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Lorenzo BIANCONI
> On 27/03/17 22:42, Lorenzo Bianconi wrote:
>> This patch allows to avoid a transitory that occurs when a given sensor
>> has been already enabled (e.g. gyroscope) and the user is configuring
>> the sample frequency of the other one (e.g. accelerometer).
>> Until the accelerometer is enabled gyroscope ODR is modified as well.
>> Fix it introducing st_lsm6dsx_check_odr() routine to check ODR consistency
>> in write_raw handler in order to apply frequency configuration just
>> in st_lsm6dsx_set_odr()
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
> Hi Lorenzo,
Hi Jonathan,
>
> Patch is great, but to make my life easier could you add a fixes
> tag + give me some idea of how important the issue is.
>
> I need to know if I should be rushing it into the kernel or the
> next merge window is fine.
>
Ack. The issue is not particularly severe (no crash or whatever). You
can notice it if a sensor has been already enabled and you just
configure the ODR of the other one without enabling it. I guess next
merge window is fine.
Anyway, v3 on the way.
Regards,
Lorenzo
> Thanks,
>
> Jonathan
>> ---
>> Changes since v1:
>> - Rename st_lsm6dsx_get_odr_val() in st_lsm6dsx_check_odr()
>> ---
>> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 41 ++++++++++++++++++----------
>> 1 file changed, 26 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> index c433223..f80a3d4 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> @@ -308,32 +308,40 @@ static int st_lsm6dsx_set_full_scale(struct st_lsm6dsx_sensor *sensor,
>> return 0;
>> }
>>
>> -static int st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u16 odr)
>> +static int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u16 odr,
>> + u8 *val)
>> {
>> - enum st_lsm6dsx_sensor_id id = sensor->id;
>> - int i, err;
>> - u8 val;
>> + int i;
>>
>> for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++)
>> - if (st_lsm6dsx_odr_table[id].odr_avl[i].hz == odr)
>> + if (st_lsm6dsx_odr_table[sensor->id].odr_avl[i].hz == odr)
>> break;
>>
>> if (i == ST_LSM6DSX_ODR_LIST_SIZE)
>> return -EINVAL;
>>
>> - val = st_lsm6dsx_odr_table[id].odr_avl[i].val;
>> - err = st_lsm6dsx_write_with_mask(sensor->hw,
>> - st_lsm6dsx_odr_table[id].reg.addr,
>> - st_lsm6dsx_odr_table[id].reg.mask,
>> - val);
>> - if (err < 0)
>> - return err;
>> -
>> + *val = st_lsm6dsx_odr_table[sensor->id].odr_avl[i].val;
>> sensor->odr = odr;
>>
>> return 0;
>> }
>>
>> +static int st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u16 odr)
>> +{
>> + enum st_lsm6dsx_sensor_id id = sensor->id;
>> + int err;
>> + u8 val;
>> +
>> + err = st_lsm6dsx_check_odr(sensor, odr, &val);
>> + if (err < 0)
>> + return err;
>> +
>> + return st_lsm6dsx_write_with_mask(sensor->hw,
>> + st_lsm6dsx_odr_table[id].reg.addr,
>> + st_lsm6dsx_odr_table[id].reg.mask,
>> + val);
>> +}
>> +
>> int st_lsm6dsx_sensor_enable(struct st_lsm6dsx_sensor *sensor)
>> {
>> int err;
>> @@ -436,9 +444,12 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
>> case IIO_CHAN_INFO_SCALE:
>> err = st_lsm6dsx_set_full_scale(sensor, val2);
>> break;
>> - case IIO_CHAN_INFO_SAMP_FREQ:
>> - err = st_lsm6dsx_set_odr(sensor, val);
>> + case IIO_CHAN_INFO_SAMP_FREQ: {
>> + u8 data;
>> +
>> + err = st_lsm6dsx_check_odr(sensor, val, &data);
>> break;
>> + }
>> default:
>> err = -EINVAL;
>> break;
>>
>
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-02 11:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 21:42 [PATCH v2] iio: imu: st_lsm6dsx: do not apply ODR configuration in write_raw handler Lorenzo Bianconi
2017-04-02 10:33 ` Jonathan Cameron
2017-04-02 11:55 ` Lorenzo Bianconi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.