* [PATCH] iio: hid-sensors: Fix an error handling path in _hid_sensor_set_report_latency()
@ 2024-10-03 18:41 Christophe JAILLET
2024-10-05 18:06 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Christophe JAILLET @ 2024-10-03 18:41 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Srinivas Pandruvada
Cc: linux-kernel, kernel-janitors, Christophe JAILLET, linux-iio
If hid_sensor_set_report_latency() fails, the error code should be returned
instead of a value likely to be interpreted as 'success'.
Fixes: 138bc7969c24 ("iio: hid-sensor-hub: Implement batch mode")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This patch is speculative.
The code just *looks* wrong to me. No strong opinion, if it is done on
purpose or not.
---
drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index ad8910e6ad59..abb09fefc792 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -32,7 +32,7 @@ static ssize_t _hid_sensor_set_report_latency(struct device *dev,
latency = integer * 1000 + fract / 1000;
ret = hid_sensor_set_report_latency(attrb, latency);
if (ret < 0)
- return len;
+ return ret;
attrb->latency_ms = hid_sensor_get_report_latency(attrb);
--
2.46.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] iio: hid-sensors: Fix an error handling path in _hid_sensor_set_report_latency()
2024-10-03 18:41 [PATCH] iio: hid-sensors: Fix an error handling path in _hid_sensor_set_report_latency() Christophe JAILLET
@ 2024-10-05 18:06 ` Jonathan Cameron
2024-10-08 17:21 ` srinivas pandruvada
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2024-10-05 18:06 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Lars-Peter Clausen, Srinivas Pandruvada, linux-kernel,
kernel-janitors, linux-iio
On Thu, 3 Oct 2024 20:41:12 +0200
Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> If hid_sensor_set_report_latency() fails, the error code should be returned
> instead of a value likely to be interpreted as 'success'.
>
> Fixes: 138bc7969c24 ("iio: hid-sensor-hub: Implement batch mode")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> This patch is speculative.
>
> The code just *looks* wrong to me. No strong opinion, if it is done on
> purpose or not.
Agreed it smells :) But I'd like more eyes on this before I take the fix
as maybe there is something subtle going on.
J
> ---
> drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index ad8910e6ad59..abb09fefc792 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -32,7 +32,7 @@ static ssize_t _hid_sensor_set_report_latency(struct device *dev,
> latency = integer * 1000 + fract / 1000;
> ret = hid_sensor_set_report_latency(attrb, latency);
> if (ret < 0)
> - return len;
> + return ret;
>
> attrb->latency_ms = hid_sensor_get_report_latency(attrb);
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] iio: hid-sensors: Fix an error handling path in _hid_sensor_set_report_latency()
2024-10-05 18:06 ` Jonathan Cameron
@ 2024-10-08 17:21 ` srinivas pandruvada
2024-10-10 17:50 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: srinivas pandruvada @ 2024-10-08 17:21 UTC (permalink / raw)
To: Jonathan Cameron, Christophe JAILLET
Cc: Lars-Peter Clausen, linux-kernel, kernel-janitors, linux-iio
On Sat, 2024-10-05 at 19:06 +0100, Jonathan Cameron wrote:
> On Thu, 3 Oct 2024 20:41:12 +0200
> Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
>
> > If hid_sensor_set_report_latency() fails, the error code should be
> > returned
> > instead of a value likely to be interpreted as 'success'.
> >
> > Fixes: 138bc7969c24 ("iio: hid-sensor-hub: Implement batch mode")
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > ---
> > This patch is speculative.
> >
> > The code just *looks* wrong to me. No strong opinion, if it is done
> > on
> > purpose or not.
> Agreed it smells :) But I'd like more eyes on this before I take the
> fix
> as maybe there is something subtle going on.
>
The original HID sensor spec HUTRR39 didn't have this property (usage
ID 0x31B). This was added by update "HUTRR59" to support batch mode to
improve power.
This attribute will not be present on non batch mode supported system
and on supported system this attribute writes will not fail unless some
hardware error.
Returning error is fine.
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Thanks,
Srinivas
> J
> > ---
> > drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > index ad8910e6ad59..abb09fefc792 100644
> > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > @@ -32,7 +32,7 @@ static ssize_t
> > _hid_sensor_set_report_latency(struct device *dev,
> > latency = integer * 1000 + fract / 1000;
> > ret = hid_sensor_set_report_latency(attrb, latency);
> > if (ret < 0)
> > - return len;
> > + return ret;
> >
> > attrb->latency_ms = hid_sensor_get_report_latency(attrb);
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] iio: hid-sensors: Fix an error handling path in _hid_sensor_set_report_latency()
2024-10-08 17:21 ` srinivas pandruvada
@ 2024-10-10 17:50 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2024-10-10 17:50 UTC (permalink / raw)
To: srinivas pandruvada
Cc: Christophe JAILLET, Lars-Peter Clausen, linux-kernel,
kernel-janitors, linux-iio
On Tue, 08 Oct 2024 10:21:50 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> On Sat, 2024-10-05 at 19:06 +0100, Jonathan Cameron wrote:
> > On Thu, 3 Oct 2024 20:41:12 +0200
> > Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> >
> > > If hid_sensor_set_report_latency() fails, the error code should be
> > > returned
> > > instead of a value likely to be interpreted as 'success'.
> > >
> > > Fixes: 138bc7969c24 ("iio: hid-sensor-hub: Implement batch mode")
> > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > > ---
> > > This patch is speculative.
> > >
> > > The code just *looks* wrong to me. No strong opinion, if it is done
> > > on
> > > purpose or not.
> > Agreed it smells :) But I'd like more eyes on this before I take the
> > fix
> > as maybe there is something subtle going on.
> >
> The original HID sensor spec HUTRR39 didn't have this property (usage
> ID 0x31B). This was added by update "HUTRR59" to support batch mode to
> improve power.
> This attribute will not be present on non batch mode supported system
> and on supported system this attribute writes will not fail unless some
> hardware error.
>
> Returning error is fine.
>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>
> Thanks,
> Srinivas
>
Thanks and applied to the fixes-togreg branch of iio.git + marked
for stable.
Jonathan
>
>
>
>
> > J
> > > ---
> > > drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > > b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > > index ad8910e6ad59..abb09fefc792 100644
> > > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > > @@ -32,7 +32,7 @@ static ssize_t
> > > _hid_sensor_set_report_latency(struct device *dev,
> > > latency = integer * 1000 + fract / 1000;
> > > ret = hid_sensor_set_report_latency(attrb, latency);
> > > if (ret < 0)
> > > - return len;
> > > + return ret;
> > >
> > > attrb->latency_ms = hid_sensor_get_report_latency(attrb);
> > >
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-10-10 17:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-03 18:41 [PATCH] iio: hid-sensors: Fix an error handling path in _hid_sensor_set_report_latency() Christophe JAILLET
2024-10-05 18:06 ` Jonathan Cameron
2024-10-08 17:21 ` srinivas pandruvada
2024-10-10 17:50 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox