* [PATCH AUTOSEL 4.4 17/35] iio: st_accel: fix iio_triggered_buffer_{pre,post}enable positions
[not found] <20190719041423.19322-1-sashal@kernel.org>
@ 2019-07-19 4:14 ` Sasha Levin
2019-07-21 17:23 ` Jonathan Cameron
2019-07-19 4:14 ` [PATCH AUTOSEL 4.4 18/35] iio: iio-utils: Fix possible incorrect mask calculation Sasha Levin
1 sibling, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2019-07-19 4:14 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexandru Ardelean, Denis Ciocca, Jonathan Cameron, Sasha Levin,
linux-iio
From: Alexandru Ardelean <alexandru.ardelean@analog.com>
[ Upstream commit 05b8bcc96278c9ef927a6f25a98e233e55de42e1 ]
The iio_triggered_buffer_{predisable,postenable} functions attach/detach
the poll functions.
For the predisable hook, the disable code should occur before detaching
the poll func, and for the postenable hook, the poll func should be
attached before the enable code.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Acked-by: Denis Ciocca <denis.ciocca@st.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/iio/accel/st_accel_buffer.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/accel/st_accel_buffer.c b/drivers/iio/accel/st_accel_buffer.c
index a1e642ee13d6..4f838277184a 100644
--- a/drivers/iio/accel/st_accel_buffer.c
+++ b/drivers/iio/accel/st_accel_buffer.c
@@ -46,17 +46,19 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
goto allocate_memory_error;
}
- err = st_sensors_set_axis_enable(indio_dev,
- (u8)indio_dev->active_scan_mask[0]);
+ err = iio_triggered_buffer_postenable(indio_dev);
if (err < 0)
goto st_accel_buffer_postenable_error;
- err = iio_triggered_buffer_postenable(indio_dev);
+ err = st_sensors_set_axis_enable(indio_dev,
+ (u8)indio_dev->active_scan_mask[0]);
if (err < 0)
- goto st_accel_buffer_postenable_error;
+ goto st_sensors_set_axis_enable_error;
return err;
+st_sensors_set_axis_enable_error:
+ iio_triggered_buffer_predisable(indio_dev);
st_accel_buffer_postenable_error:
kfree(adata->buffer_data);
allocate_memory_error:
@@ -65,20 +67,22 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
static int st_accel_buffer_predisable(struct iio_dev *indio_dev)
{
- int err;
+ int err, err2;
struct st_sensor_data *adata = iio_priv(indio_dev);
- err = iio_triggered_buffer_predisable(indio_dev);
- if (err < 0)
- goto st_accel_buffer_predisable_error;
-
err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
if (err < 0)
goto st_accel_buffer_predisable_error;
err = st_sensors_set_enable(indio_dev, false);
+ if (err < 0)
+ goto st_accel_buffer_predisable_error;
st_accel_buffer_predisable_error:
+ err2 = iio_triggered_buffer_predisable(indio_dev);
+ if (!err)
+ err = err2;
+
kfree(adata->buffer_data);
return err;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH AUTOSEL 4.4 18/35] iio: iio-utils: Fix possible incorrect mask calculation
[not found] <20190719041423.19322-1-sashal@kernel.org>
2019-07-19 4:14 ` [PATCH AUTOSEL 4.4 17/35] iio: st_accel: fix iio_triggered_buffer_{pre,post}enable positions Sasha Levin
@ 2019-07-19 4:14 ` Sasha Levin
2019-07-21 17:27 ` Jonathan Cameron
1 sibling, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2019-07-19 4:14 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Bastien Nocera, Jonathan Cameron, Sasha Levin, linux-iio
From: Bastien Nocera <hadess@hadess.net>
[ Upstream commit 208a68c8393d6041a90862992222f3d7943d44d6 ]
On some machines, iio-sensor-proxy was returning all 0's for IIO sensor
values. It turns out that the bits_used for this sensor is 32, which makes
the mask calculation:
*mask = (1 << 32) - 1;
If the compiler interprets the 1 literals as 32-bit ints, it generates
undefined behavior depending on compiler version and optimization level.
On my system, it optimizes out the shift, so the mask value becomes
*mask = (1) - 1;
With a mask value of 0, iio-sensor-proxy will always return 0 for every axis.
Avoid incorrect 0 values caused by compiler optimization.
See original fix by Brett Dutro <brett.dutro@gmail.com> in
iio-sensor-proxy:
https://github.com/hadess/iio-sensor-proxy/commit/9615ceac7c134d838660e209726cd86aa2064fd3
Signed-off-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
tools/iio/iio_utils.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c
index 5eb6793f3972..2d0dcd6fc64c 100644
--- a/tools/iio/iio_utils.c
+++ b/tools/iio/iio_utils.c
@@ -163,9 +163,9 @@ int iioutils_get_type(unsigned *is_signed, unsigned *bytes, unsigned *bits_used,
*be = (endianchar == 'b');
*bytes = padint / 8;
if (*bits_used == 64)
- *mask = ~0;
+ *mask = ~(0ULL);
else
- *mask = (1ULL << *bits_used) - 1;
+ *mask = (1ULL << *bits_used) - 1ULL;
*is_signed = (signchar == 's');
if (fclose(sysfsfp)) {
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH AUTOSEL 4.4 17/35] iio: st_accel: fix iio_triggered_buffer_{pre,post}enable positions
2019-07-19 4:14 ` [PATCH AUTOSEL 4.4 17/35] iio: st_accel: fix iio_triggered_buffer_{pre,post}enable positions Sasha Levin
@ 2019-07-21 17:23 ` Jonathan Cameron
2019-07-22 6:47 ` Ardelean, Alexandru
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2019-07-21 17:23 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, Alexandru Ardelean, Denis Ciocca,
Jonathan Cameron, linux-iio
On Fri, 19 Jul 2019 00:14:05 -0400
Sasha Levin <sashal@kernel.org> wrote:
> From: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> [ Upstream commit 05b8bcc96278c9ef927a6f25a98e233e55de42e1 ]
>
> The iio_triggered_buffer_{predisable,postenable} functions attach/detach
> the poll functions.
>
> For the predisable hook, the disable code should occur before detaching
> the poll func, and for the postenable hook, the poll func should be
> attached before the enable code.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> Acked-by: Denis Ciocca <denis.ciocca@st.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
Hi Sasha,
This should do any harm, but I deliberately didn't cc stable on
this one.
Alex, my assumption on this one is that it was fixing a logical
ordering problem, but one that had no visible impact.
Whilst the pollfunc will be attached too early, the trigger
will be disabled for the whole of this function anyway so
it shouldn't cause any visible problem. Is that a correct interpretation?
There are going to be a few more similar fixes in the near future
as Alex is trying to tidy up various paths so we can do a general
bit of refactoring.
If I'm too late for this, then not a problem, just noise
in the stable release.
Jonathan
> ---
> drivers/iio/accel/st_accel_buffer.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/accel/st_accel_buffer.c b/drivers/iio/accel/st_accel_buffer.c
> index a1e642ee13d6..4f838277184a 100644
> --- a/drivers/iio/accel/st_accel_buffer.c
> +++ b/drivers/iio/accel/st_accel_buffer.c
> @@ -46,17 +46,19 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
> goto allocate_memory_error;
> }
>
> - err = st_sensors_set_axis_enable(indio_dev,
> - (u8)indio_dev->active_scan_mask[0]);
> + err = iio_triggered_buffer_postenable(indio_dev);
> if (err < 0)
> goto st_accel_buffer_postenable_error;
>
> - err = iio_triggered_buffer_postenable(indio_dev);
> + err = st_sensors_set_axis_enable(indio_dev,
> + (u8)indio_dev->active_scan_mask[0]);
> if (err < 0)
> - goto st_accel_buffer_postenable_error;
> + goto st_sensors_set_axis_enable_error;
>
> return err;
>
> +st_sensors_set_axis_enable_error:
> + iio_triggered_buffer_predisable(indio_dev);
> st_accel_buffer_postenable_error:
> kfree(adata->buffer_data);
> allocate_memory_error:
> @@ -65,20 +67,22 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
>
> static int st_accel_buffer_predisable(struct iio_dev *indio_dev)
> {
> - int err;
> + int err, err2;
> struct st_sensor_data *adata = iio_priv(indio_dev);
>
> - err = iio_triggered_buffer_predisable(indio_dev);
> - if (err < 0)
> - goto st_accel_buffer_predisable_error;
> -
> err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> if (err < 0)
> goto st_accel_buffer_predisable_error;
>
> err = st_sensors_set_enable(indio_dev, false);
> + if (err < 0)
> + goto st_accel_buffer_predisable_error;
>
> st_accel_buffer_predisable_error:
> + err2 = iio_triggered_buffer_predisable(indio_dev);
> + if (!err)
> + err = err2;
> +
> kfree(adata->buffer_data);
> return err;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH AUTOSEL 4.4 18/35] iio: iio-utils: Fix possible incorrect mask calculation
2019-07-19 4:14 ` [PATCH AUTOSEL 4.4 18/35] iio: iio-utils: Fix possible incorrect mask calculation Sasha Levin
@ 2019-07-21 17:27 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2019-07-21 17:27 UTC (permalink / raw)
To: Sasha Levin
Cc: linux-kernel, stable, Bastien Nocera, Jonathan Cameron, linux-iio
On Fri, 19 Jul 2019 00:14:06 -0400
Sasha Levin <sashal@kernel.org> wrote:
> From: Bastien Nocera <hadess@hadess.net>
>
> [ Upstream commit 208a68c8393d6041a90862992222f3d7943d44d6 ]
>
> On some machines, iio-sensor-proxy was returning all 0's for IIO sensor
> values. It turns out that the bits_used for this sensor is 32, which makes
> the mask calculation:
>
> *mask = (1 << 32) - 1;
>
> If the compiler interprets the 1 literals as 32-bit ints, it generates
> undefined behavior depending on compiler version and optimization level.
> On my system, it optimizes out the shift, so the mask value becomes
>
> *mask = (1) - 1;
>
> With a mask value of 0, iio-sensor-proxy will always return 0 for every axis.
>
> Avoid incorrect 0 values caused by compiler optimization.
>
> See original fix by Brett Dutro <brett.dutro@gmail.com> in
> iio-sensor-proxy:
> https://github.com/hadess/iio-sensor-proxy/commit/9615ceac7c134d838660e209726cd86aa2064fd3
>
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
Good catch, I should have cc'd stable on this one.
Thanks,
Jonathan
> ---
> tools/iio/iio_utils.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c
> index 5eb6793f3972..2d0dcd6fc64c 100644
> --- a/tools/iio/iio_utils.c
> +++ b/tools/iio/iio_utils.c
> @@ -163,9 +163,9 @@ int iioutils_get_type(unsigned *is_signed, unsigned *bytes, unsigned *bits_used,
> *be = (endianchar == 'b');
> *bytes = padint / 8;
> if (*bits_used == 64)
> - *mask = ~0;
> + *mask = ~(0ULL);
> else
> - *mask = (1ULL << *bits_used) - 1;
> + *mask = (1ULL << *bits_used) - 1ULL;
>
> *is_signed = (signchar == 's');
> if (fclose(sysfsfp)) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH AUTOSEL 4.4 17/35] iio: st_accel: fix iio_triggered_buffer_{pre,post}enable positions
2019-07-21 17:23 ` Jonathan Cameron
@ 2019-07-22 6:47 ` Ardelean, Alexandru
2019-07-28 15:43 ` Sasha Levin
0 siblings, 1 reply; 6+ messages in thread
From: Ardelean, Alexandru @ 2019-07-22 6:47 UTC (permalink / raw)
To: sashal, jic23
Cc: stable, Jonathan.Cameron, linux-kernel, denis.ciocca, linux-iio
On Sun, 2019-07-21 at 18:23 +0100, Jonathan Cameron wrote:
> [External]
>
> On Fri, 19 Jul 2019 00:14:05 -0400
> Sasha Levin <sashal@kernel.org> wrote:
>
> > From: Alexandru Ardelean <alexandru.ardelean@analog.com>
> >
> > [ Upstream commit 05b8bcc96278c9ef927a6f25a98e233e55de42e1 ]
> >
> > The iio_triggered_buffer_{predisable,postenable} functions attach/detach
> > the poll functions.
> >
> > For the predisable hook, the disable code should occur before detaching
> > the poll func, and for the postenable hook, the poll func should be
> > attached before the enable code.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > Acked-by: Denis Ciocca <denis.ciocca@st.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> Hi Sasha,
>
> This should do any harm, but I deliberately didn't cc stable on
> this one.
>
> Alex, my assumption on this one is that it was fixing a logical
> ordering problem, but one that had no visible impact.
> Whilst the pollfunc will be attached too early, the trigger
> will be disabled for the whole of this function anyway so
> it shouldn't cause any visible problem. Is that a correct interpretation?
Yes.
That is correct.
Maybe I'll change the commit title format so that there is no confusion.
It's a fix, but not in the sense that something was broken.
Just something that is the realm of cleaning up.
Thanks
Alex
> There are going to be a few more similar fixes in the near future
> as Alex is trying to tidy up various paths so we can do a general
> bit of refactoring.
>
> If I'm too late for this, then not a problem, just noise
> in the stable release.
>
> Jonathan
>
>
> > ---
> > drivers/iio/accel/st_accel_buffer.c | 22 +++++++++++++---------
> > 1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/iio/accel/st_accel_buffer.c b/drivers/iio/accel/st_accel_buffer.c
> > index a1e642ee13d6..4f838277184a 100644
> > --- a/drivers/iio/accel/st_accel_buffer.c
> > +++ b/drivers/iio/accel/st_accel_buffer.c
> > @@ -46,17 +46,19 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
> > goto allocate_memory_error;
> > }
> >
> > - err = st_sensors_set_axis_enable(indio_dev,
> > - (u8)indio_dev->active_scan_mask[0]);
> > + err = iio_triggered_buffer_postenable(indio_dev);
> > if (err < 0)
> > goto st_accel_buffer_postenable_error;
> >
> > - err = iio_triggered_buffer_postenable(indio_dev);
> > + err = st_sensors_set_axis_enable(indio_dev,
> > + (u8)indio_dev->active_scan_mask[0]);
> > if (err < 0)
> > - goto st_accel_buffer_postenable_error;
> > + goto st_sensors_set_axis_enable_error;
> >
> > return err;
> >
> > +st_sensors_set_axis_enable_error:
> > + iio_triggered_buffer_predisable(indio_dev);
> > st_accel_buffer_postenable_error:
> > kfree(adata->buffer_data);
> > allocate_memory_error:
> > @@ -65,20 +67,22 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
> >
> > static int st_accel_buffer_predisable(struct iio_dev *indio_dev)
> > {
> > - int err;
> > + int err, err2;
> > struct st_sensor_data *adata = iio_priv(indio_dev);
> >
> > - err = iio_triggered_buffer_predisable(indio_dev);
> > - if (err < 0)
> > - goto st_accel_buffer_predisable_error;
> > -
> > err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
> > if (err < 0)
> > goto st_accel_buffer_predisable_error;
> >
> > err = st_sensors_set_enable(indio_dev, false);
> > + if (err < 0)
> > + goto st_accel_buffer_predisable_error;
> >
> > st_accel_buffer_predisable_error:
> > + err2 = iio_triggered_buffer_predisable(indio_dev);
> > + if (!err)
> > + err = err2;
> > +
> > kfree(adata->buffer_data);
> > return err;
> > }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH AUTOSEL 4.4 17/35] iio: st_accel: fix iio_triggered_buffer_{pre,post}enable positions
2019-07-22 6:47 ` Ardelean, Alexandru
@ 2019-07-28 15:43 ` Sasha Levin
0 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2019-07-28 15:43 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: jic23, stable, Jonathan.Cameron, linux-kernel, denis.ciocca, linux-iio
On Mon, Jul 22, 2019 at 06:47:47AM +0000, Ardelean, Alexandru wrote:
>On Sun, 2019-07-21 at 18:23 +0100, Jonathan Cameron wrote:
>> [External]
>>
>> On Fri, 19 Jul 2019 00:14:05 -0400
>> Sasha Levin <sashal@kernel.org> wrote:
>>
>> > From: Alexandru Ardelean <alexandru.ardelean@analog.com>
>> >
>> > [ Upstream commit 05b8bcc96278c9ef927a6f25a98e233e55de42e1 ]
>> >
>> > The iio_triggered_buffer_{predisable,postenable} functions attach/detach
>> > the poll functions.
>> >
>> > For the predisable hook, the disable code should occur before detaching
>> > the poll func, and for the postenable hook, the poll func should be
>> > attached before the enable code.
>> >
>> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>> > Acked-by: Denis Ciocca <denis.ciocca@st.com>
>> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> > Signed-off-by: Sasha Levin <sashal@kernel.org>
>> Hi Sasha,
>>
>> This should do any harm, but I deliberately didn't cc stable on
>> this one.
>>
>> Alex, my assumption on this one is that it was fixing a logical
>> ordering problem, but one that had no visible impact.
>> Whilst the pollfunc will be attached too early, the trigger
>> will be disabled for the whole of this function anyway so
>> it shouldn't cause any visible problem. Is that a correct interpretation?
>
>Yes.
>That is correct.
>
>Maybe I'll change the commit title format so that there is no confusion.
>It's a fix, but not in the sense that something was broken.
>Just something that is the realm of cleaning up.
I'll drop it from the queue, thanks!
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-28 15:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190719041423.19322-1-sashal@kernel.org>
2019-07-19 4:14 ` [PATCH AUTOSEL 4.4 17/35] iio: st_accel: fix iio_triggered_buffer_{pre,post}enable positions Sasha Levin
2019-07-21 17:23 ` Jonathan Cameron
2019-07-22 6:47 ` Ardelean, Alexandru
2019-07-28 15:43 ` Sasha Levin
2019-07-19 4:14 ` [PATCH AUTOSEL 4.4 18/35] iio: iio-utils: Fix possible incorrect mask calculation Sasha Levin
2019-07-21 17:27 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).