Re: [PATCH] IIO: hid-sensor-prox: add missing scale attribute

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 17 Oct 2023 18:56:38 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> On Tue, 2023-10-17 at 20:11 +0100, Jonathan Cameron wrote:
> > On Mon, 16 Oct 2023 07:28:36 -0700
> > srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> >   
> > > On Mon, 2023-10-16 at 08:44 +0100, Jonathan Cameron wrote:  
> > > > On Sun, 15 Oct 2023 12:04:48 +0100
> > > > Jonathan Cameron <jic23@kernel.org> wrote:
> > > >     
> > > > > On Sat, 14 Oct 2023 19:56:26 -0700
> > > > > srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
> > > > > wrote:
> > > > >     
> > > > > > On Sat, 2023-10-14 at 17:52 +0100, Jonathan Cameron wrote:  
> > > > > >     
> > > > >     
> > > 
> > > [...]
> > >   
> > > > > Phillipp, if a backport makes sense you can request that after
> > > > > this
> > > > > goes upstream.    
> > > > Whilst typing up a pull request I saw this again and thought a
> > > > bit
> > > > more on it.
> > > > 
> > > > This fix is probably wrong approach.  Proximity sensors are often
> > > > scale free
> > > > because they depend on reflectance off something or a capacitance
> > > > changing etc
> > > > so we don't know the scaling.  So the right response then is not
> > > > to
> > > > return a scale
> > > > value of 1.0 but to not provide the attribute at all.  Is that
> > > > something that
> > > > could be easily done here?    
> > > 
> > > I think so. But hope that iio-sensor-proxy can handle absence of
> > > scale
> > > attribute.
> > > 
> > > git diff drivers/iio/light/hid-sensor-prox.c
> > > diff --git a/drivers/iio/light/hid-sensor-prox.c
> > > b/drivers/iio/light/hid-sensor-prox.c
> > > index a47591e1bad9..e4b81fa948f5 100644
> > > --- a/drivers/iio/light/hid-sensor-prox.c
> > > +++ b/drivers/iio/light/hid-sensor-prox.c
> > > @@ -36,7 +36,6 @@ static const struct iio_chan_spec prox_channels[]
> > > = {
> > >                 .type = IIO_PROXIMITY,
> > >                 .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> > >                 .info_mask_shared_by_type =
> > > BIT(IIO_CHAN_INFO_OFFSET) |
> > > -               BIT(IIO_CHAN_INFO_SCALE) |
> > >                 BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> > >                 BIT(IIO_CHAN_INFO_HYSTERESIS),
> > >                 .scan_index = CHANNEL_SCAN_INDEX_PRESENCE,
> > > 
> > > Thanks,
> > > Srinivas  
> > 
> > Just to check. Are we guaranteed that there is never a scale
> > parameter?
> > Some proximity sensors do have absolute units (time of flight sensors
> > for example).
> >   
> This driver is implementing:
> Biometric: Human Presence (Usage ID 0x11) from HID sensor hub
> specification.
> 
> "Biometric: Human
> PresenceCA,CP – An application-level or physical collection that
> identifies
> a device that detects human presence (Boolean yes or no)."
> 
> 
> It is not implementing Biometric: Human Proximity (Usage ID 0x12).
> This has range of values, then unit will be applicable.

Makes sense. In that case, formal version of the patch above?

Thanks,

Jonathan

> 
> Thanks,
> Srinivas
> 
> > Jonathan
> >   
> > >   
> > > > 
> > > > For now I'm dropping the patch. Sorry I wasn't paying enough
> > > > attention to notice
> > > > this was a proximity sensor.
> > > > 
> > > > Jonathan
> > > >     
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > Jonathan
> > > > >     
> > > > > > > 
> > > > > > > Phillipp - this sounds like a fix to me.  Fixes tag?
> > > > > > > 
> > > > > > > Thanks,
> > > > > > > 
> > > > > > > Jonathan        
> > > > > >       
> > > > >     
> > > >     
> > >   
> >   
>