Re: [PATCH] iio: hid-sensors: Fix an error handling path in _hid_sensor_set_report_latency()

[Jonathan Cameron]

On Tue, 08 Oct 2024 10:21:50 -0700
srinivas pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxx>
> 
> 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);
> > >    
> >   
>