[PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Ricardo Ribalda]

When the driver was originally created, it was decided that
sampling_frequency and hysteresis would be shared_per_type instead
of shared_by_all (even though it is internally shared by all). Eg:
in_proximity_raw
in_proximity_sampling_frequency

When we introduced support for more channels, we continued with
shared_by_type which. Eg:
in_proximity0_raw
in_proximity1_raw
in_proximity_sampling_frequency
in_attention_raw
in_attention_sampling_frequency

Ideally we should change to shared_by_all, but it is not an option,
because the current naming has been a stablished ABI by now. Luckily we
can use separate instead. That will be more consistent:
in_proximity0_raw
in_proximity0_sampling_frequency
in_proximity1_raw
in_proximity1_sampling_frequency
in_attention_raw
in_attention_sampling_frequency

Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for more channels")
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v2:
- Use separate
- Link to v1: https://lore.kernel.org/r/20241205-fix-hid-sensor-v1-1-9b789f39c220@chromium.org
---
 drivers/iio/light/hid-sensor-prox.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index c83acbd78275..71dcef3fbe57 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -49,9 +49,10 @@ static const u32 prox_sensitivity_addresses[] = {
 #define PROX_CHANNEL(_is_proximity, _channel) \
 	{\
 		.type = _is_proximity ? IIO_PROXIMITY : IIO_ATTENTION,\
-		.info_mask_separate = _is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
-				      BIT(IIO_CHAN_INFO_PROCESSED),\
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |\
+		.info_mask_separate = \
+		(_is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
+				BIT(IIO_CHAN_INFO_PROCESSED)) |\
+		BIT(IIO_CHAN_INFO_OFFSET) |\
 		BIT(IIO_CHAN_INFO_SCALE) |\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ) |\
 		BIT(IIO_CHAN_INFO_HYSTERESIS),\

---
base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
change-id: 20241203-fix-hid-sensor-62e1979ecd03

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

Re: [PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Jonathan Cameron]

On Mon, 16 Dec 2024 10:05:53 +0000
Ricardo Ribalda <ribalda@chromium.org> wrote:

> When the driver was originally created, it was decided that
> sampling_frequency and hysteresis would be shared_per_type instead
> of shared_by_all (even though it is internally shared by all). Eg:
> in_proximity_raw
> in_proximity_sampling_frequency
> 
> When we introduced support for more channels, we continued with
> shared_by_type which. Eg:
> in_proximity0_raw
> in_proximity1_raw
> in_proximity_sampling_frequency
> in_attention_raw
> in_attention_sampling_frequency
> 
> Ideally we should change to shared_by_all, but it is not an option,
> because the current naming has been a stablished ABI by now. Luckily we
> can use separate instead. That will be more consistent:
> in_proximity0_raw
> in_proximity0_sampling_frequency
> in_proximity1_raw
> in_proximity1_sampling_frequency
> in_attention_raw
> in_attention_sampling_frequency
> 
> Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for more channels")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

I got lost somewhere in the discussion.  This is still an ABI change compared
to original interface at the top (which is the one that has been there 
quite some time).

However we already had to make one of those to add the index that wasn't there
for _raw. (I'd missed that in earlier discussion - thanks for laying out the
steps here!)  Srinivas, Jiri, do you think we are better off just assuming users
of this will be using a library that correctly deals with sharing and just
jump to 
in_proximity0_raw
in_proximity1_raw
in_attention_raw
(should have indexed that but it may never matter) and
sampling_frequency

Which is what I think Ricardo originally asked.

Do we have any guarantee the sampling_frequency will be shared across the
sensor channels?  It may be the most common situation but I don't want to
wall us into a corner if it turns out someone runs separate sensors at
different rates (no particularly reason they should be one type of sensor
so this might make sense).  If we don't have that guarantee
then this patch is fine as far as I'm concerned.

Jonathan



> ---
> Changes in v2:
> - Use separate
> - Link to v1: https://lore.kernel.org/r/20241205-fix-hid-sensor-v1-1-9b789f39c220@chromium.org
> ---
>  drivers/iio/light/hid-sensor-prox.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index c83acbd78275..71dcef3fbe57 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c
> @@ -49,9 +49,10 @@ static const u32 prox_sensitivity_addresses[] = {
>  #define PROX_CHANNEL(_is_proximity, _channel) \
>  	{\
>  		.type = _is_proximity ? IIO_PROXIMITY : IIO_ATTENTION,\
> -		.info_mask_separate = _is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> -				      BIT(IIO_CHAN_INFO_PROCESSED),\
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |\
> +		.info_mask_separate = \
> +		(_is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> +				BIT(IIO_CHAN_INFO_PROCESSED)) |\
> +		BIT(IIO_CHAN_INFO_OFFSET) |\
>  		BIT(IIO_CHAN_INFO_SCALE) |\
>  		BIT(IIO_CHAN_INFO_SAMP_FREQ) |\
>  		BIT(IIO_CHAN_INFO_HYSTERESIS),\
> 
> ---
> base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
> change-id: 20241203-fix-hid-sensor-62e1979ecd03
> 
> Best regards,

Re: [PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Ricardo Ribalda]

Hi Jonathan

Happy new year!

Friendly ping about this patch so we can change the ABI before the
kernel release happens

On Thu, 19 Dec 2024 at 18:17, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 16 Dec 2024 10:05:53 +0000
> Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> > When the driver was originally created, it was decided that
> > sampling_frequency and hysteresis would be shared_per_type instead
> > of shared_by_all (even though it is internally shared by all). Eg:
> > in_proximity_raw
> > in_proximity_sampling_frequency
> >
> > When we introduced support for more channels, we continued with
> > shared_by_type which. Eg:
> > in_proximity0_raw
> > in_proximity1_raw
> > in_proximity_sampling_frequency
> > in_attention_raw
> > in_attention_sampling_frequency
> >
> > Ideally we should change to shared_by_all, but it is not an option,
> > because the current naming has been a stablished ABI by now. Luckily we
> > can use separate instead. That will be more consistent:
> > in_proximity0_raw
> > in_proximity0_sampling_frequency
> > in_proximity1_raw
> > in_proximity1_sampling_frequency
> > in_attention_raw
> > in_attention_sampling_frequency
> >
> > Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for more channels")
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>
> I got lost somewhere in the discussion.  This is still an ABI change compared
> to original interface at the top (which is the one that has been there
> quite some time).
>
> However we already had to make one of those to add the index that wasn't there
> for _raw. (I'd missed that in earlier discussion - thanks for laying out the
> steps here!)  Srinivas, Jiri, do you think we are better off just assuming users
> of this will be using a library that correctly deals with sharing and just
> jump to
> in_proximity0_raw
> in_proximity1_raw
> in_attention_raw
> (should have indexed that but it may never matter) and
> sampling_frequency
>
> Which is what I think Ricardo originally asked.
>
> Do we have any guarantee the sampling_frequency will be shared across the
> sensor channels?  It may be the most common situation but I don't want to
> wall us into a corner if it turns out someone runs separate sensors at
> different rates (no particularly reason they should be one type of sensor
> so this might make sense).  If we don't have that guarantee
> then this patch is fine as far as I'm concerned.
>
> Jonathan
>
>
>
> > ---
> > Changes in v2:
> > - Use separate
> > - Link to v1: https://lore.kernel.org/r/20241205-fix-hid-sensor-v1-1-9b789f39c220@chromium.org
> > ---
> >  drivers/iio/light/hid-sensor-prox.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> > index c83acbd78275..71dcef3fbe57 100644
> > --- a/drivers/iio/light/hid-sensor-prox.c
> > +++ b/drivers/iio/light/hid-sensor-prox.c
> > @@ -49,9 +49,10 @@ static const u32 prox_sensitivity_addresses[] = {
> >  #define PROX_CHANNEL(_is_proximity, _channel) \
> >       {\
> >               .type = _is_proximity ? IIO_PROXIMITY : IIO_ATTENTION,\
> > -             .info_mask_separate = _is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> > -                                   BIT(IIO_CHAN_INFO_PROCESSED),\
> > -             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |\
> > +             .info_mask_separate = \
> > +             (_is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> > +                             BIT(IIO_CHAN_INFO_PROCESSED)) |\
> > +             BIT(IIO_CHAN_INFO_OFFSET) |\
> >               BIT(IIO_CHAN_INFO_SCALE) |\
> >               BIT(IIO_CHAN_INFO_SAMP_FREQ) |\
> >               BIT(IIO_CHAN_INFO_HYSTERESIS),\
> >
> > ---
> > base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
> > change-id: 20241203-fix-hid-sensor-62e1979ecd03
> >
> > Best regards,
>


-- 
Ricardo Ribalda

Re: [PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Jonathan Cameron]

On Sat, 11 Jan 2025 10:17:27 +0100
Ricardo Ribalda <ribalda@chromium.org> wrote:

> Hi Jonathan
> 
> Happy new year!
> 
> Friendly ping about this patch so we can change the ABI before the
> kernel release happens

We might not quite make that :(  Srinivas, Jiri I'm looking for your
input on this.

I'm fine with us taking this as a fix that goes into an early point
release on basis only crazy people base products on a version that hasn't
gotten the fixes that inevitably only go in a few weeks later.

Jonathan

> 
> On Thu, 19 Dec 2024 at 18:17, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Mon, 16 Dec 2024 10:05:53 +0000
> > Ricardo Ribalda <ribalda@chromium.org> wrote:
> >  
> > > When the driver was originally created, it was decided that
> > > sampling_frequency and hysteresis would be shared_per_type instead
> > > of shared_by_all (even though it is internally shared by all). Eg:
> > > in_proximity_raw
> > > in_proximity_sampling_frequency
> > >
> > > When we introduced support for more channels, we continued with
> > > shared_by_type which. Eg:
> > > in_proximity0_raw
> > > in_proximity1_raw
> > > in_proximity_sampling_frequency
> > > in_attention_raw
> > > in_attention_sampling_frequency
> > >
> > > Ideally we should change to shared_by_all, but it is not an option,
> > > because the current naming has been a stablished ABI by now. Luckily we
> > > can use separate instead. That will be more consistent:
> > > in_proximity0_raw
> > > in_proximity0_sampling_frequency
> > > in_proximity1_raw
> > > in_proximity1_sampling_frequency
> > > in_attention_raw
> > > in_attention_sampling_frequency
> > >
> > > Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for more channels")
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>  
> >
> > I got lost somewhere in the discussion.  This is still an ABI change compared
> > to original interface at the top (which is the one that has been there
> > quite some time).
> >
> > However we already had to make one of those to add the index that wasn't there
> > for _raw. (I'd missed that in earlier discussion - thanks for laying out the
> > steps here!)  Srinivas, Jiri, do you think we are better off just assuming users
> > of this will be using a library that correctly deals with sharing and just
> > jump to
> > in_proximity0_raw
> > in_proximity1_raw
> > in_attention_raw
> > (should have indexed that but it may never matter) and
> > sampling_frequency
> >
> > Which is what I think Ricardo originally asked.
> >
> > Do we have any guarantee the sampling_frequency will be shared across the
> > sensor channels?  It may be the most common situation but I don't want to
> > wall us into a corner if it turns out someone runs separate sensors at
> > different rates (no particularly reason they should be one type of sensor
> > so this might make sense).  If we don't have that guarantee
> > then this patch is fine as far as I'm concerned.
> >
> > Jonathan
> >
> >
> >  
> > > ---
> > > Changes in v2:
> > > - Use separate
> > > - Link to v1: https://lore.kernel.org/r/20241205-fix-hid-sensor-v1-1-9b789f39c220@chromium.org
> > > ---
> > >  drivers/iio/light/hid-sensor-prox.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> > > index c83acbd78275..71dcef3fbe57 100644
> > > --- a/drivers/iio/light/hid-sensor-prox.c
> > > +++ b/drivers/iio/light/hid-sensor-prox.c
> > > @@ -49,9 +49,10 @@ static const u32 prox_sensitivity_addresses[] = {
> > >  #define PROX_CHANNEL(_is_proximity, _channel) \
> > >       {\
> > >               .type = _is_proximity ? IIO_PROXIMITY : IIO_ATTENTION,\
> > > -             .info_mask_separate = _is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> > > -                                   BIT(IIO_CHAN_INFO_PROCESSED),\
> > > -             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |\
> > > +             .info_mask_separate = \
> > > +             (_is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> > > +                             BIT(IIO_CHAN_INFO_PROCESSED)) |\
> > > +             BIT(IIO_CHAN_INFO_OFFSET) |\
> > >               BIT(IIO_CHAN_INFO_SCALE) |\
> > >               BIT(IIO_CHAN_INFO_SAMP_FREQ) |\
> > >               BIT(IIO_CHAN_INFO_HYSTERESIS),\
> > >
> > > ---
> > > base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
> > > change-id: 20241203-fix-hid-sensor-62e1979ecd03
> > >
> > > Best regards,  
> >  
> 
> 

Re: [PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Pandruvada, Srinivas]

On Sat, 2025-01-11 at 10:17 +0100, Ricardo Ribalda wrote:
> Hi Jonathan
> 
> Happy new year!
> 
> Friendly ping about this patch so we can change the ABI before the
> kernel release happens
> 
> On Thu, 19 Dec 2024 at 18:17, Jonathan Cameron <jic23@kernel.org>
> wrote:
> > 
> > On Mon, 16 Dec 2024 10:05:53 +0000
> > Ricardo Ribalda <ribalda@chromium.org> wrote:
> > 
> > > When the driver was originally created, it was decided that
> > > sampling_frequency and hysteresis would be shared_per_type
> > > instead
> > > of shared_by_all (even though it is internally shared by all).
> > > Eg:
> > > in_proximity_raw
> > > in_proximity_sampling_frequency
> > > 
> > > When we introduced support for more channels, we continued with
> > > shared_by_type which. Eg:
> > > in_proximity0_raw
> > > in_proximity1_raw
> > > in_proximity_sampling_frequency
> > > in_attention_raw
> > > in_attention_sampling_frequency
> > > 
> > > Ideally we should change to shared_by_all, but it is not an
> > > option,
> > > because the current naming has been a stablished ABI by now.
> > > Luckily we
> > > can use separate instead. That will be more consistent:
> > > in_proximity0_raw
> > > in_proximity0_sampling_frequency
> > > in_proximity1_raw
> > > in_proximity1_sampling_frequency
> > > in_attention_raw
> > > in_attention_sampling_frequency
> > > 
> > > Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for more
> > > channels")
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > 
> > I got lost somewhere in the discussion.  This is still an ABI
> > change compared
> > to original interface at the top (which is the one that has been
> > there
> > quite some time).
> > 
> > However we already had to make one of those to add the index that
> > wasn't there
> > for _raw. (I'd missed that in earlier discussion - thanks for
> > laying out the
> > steps here!) 

Didn't realize this. I don't see proximity sensor use in the mainline
Linux distro user space, so it will affect only some private user space
programs. 
Adding Mark to see if it affects Lenovo Sensing solution as there was
specific custom sensor added to this driver for Lenovo.

> >  Srinivas, Jiri, do you think we are better off just assuming users
> > of this will be using a library that correctly deals with sharing
> > and just
> > jump to
> > in_proximity0_raw
> > in_proximity1_raw
> > in_attention_raw
> > (should have indexed that but it may never matter) and
> > sampling_frequency
> > 
> > Which is what I think Ricardo originally asked.
> > 
> > Do we have any guarantee the sampling_frequency will be shared
> > across the
> > sensor channels?  It may be the most common situation but I don't
> > want to
> > wall us into a corner if it turns out someone runs separate sensors
> > at
> > different rates (no particularly reason they should be one type of
> > sensor
> > so this might make sense).  If we don't have that guarantee
> > then this patch is fine as far as I'm concerned.

From HID sensor spec, these are three different sensor usage IDs. So
you can have three different sensor properties like sampling frequency.

#define HID_USAGE_SENSOR_TYPE_BIOMETRIC_PRESENCE	0x11
#define HID_USAGE_SENSOR_TYPE_BIOMETRIC_PROXIMITY	0x12
#define HID_USAGE_SENSOR_TYPE_BIOMETRIC_TOUCH		0x13

This driver either loads on HID_USAGE_SENSOR_TYPE_BIOMETRIC_PRESENCE or
Lenovo custom sensor, which merge all channels, in that case they will
share one sampling frequency for all.

So there is no guarantee.

Thanks,
Srinivas


> > 
> > Jonathan
> > 
> > 
> > 
> > > ---
> > > Changes in v2:
> > > - Use separate
> > > - Link to v1:
> > > https://lore.kernel.org/r/20241205-fix-hid-sensor-v1-1-9b789f39c220@chromium.org
> > > ---
> > >  drivers/iio/light/hid-sensor-prox.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/iio/light/hid-sensor-prox.c
> > > b/drivers/iio/light/hid-sensor-prox.c
> > > index c83acbd78275..71dcef3fbe57 100644
> > > --- a/drivers/iio/light/hid-sensor-prox.c
> > > +++ b/drivers/iio/light/hid-sensor-prox.c
> > > @@ -49,9 +49,10 @@ static const u32 prox_sensitivity_addresses[]
> > > = {
> > >  #define PROX_CHANNEL(_is_proximity, _channel) \
> > >       {\
> > >               .type = _is_proximity ? IIO_PROXIMITY :
> > > IIO_ATTENTION,\
> > > -             .info_mask_separate = _is_proximity ?
> > > BIT(IIO_CHAN_INFO_RAW) :\
> > > -                                  
> > > BIT(IIO_CHAN_INFO_PROCESSED),\
> > > -             .info_mask_shared_by_type =
> > > BIT(IIO_CHAN_INFO_OFFSET) |\
> > > +             .info_mask_separate = \
> > > +             (_is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> > > +                             BIT(IIO_CHAN_INFO_PROCESSED)) |\
> > > +             BIT(IIO_CHAN_INFO_OFFSET) |\
> > >               BIT(IIO_CHAN_INFO_SCALE) |\
> > >               BIT(IIO_CHAN_INFO_SAMP_FREQ) |\
> > >               BIT(IIO_CHAN_INFO_HYSTERESIS),\
> > > 
> > > ---
> > > base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
> > > change-id: 20241203-fix-hid-sensor-62e1979ecd03
> > > 
> > > Best regards,
> > 
> 
> 

Re: [PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Mark Pearson]

Note - switched to my open-source friendly email account (avoid the Lenovo address, especially for mailing lists, it's Outlook based and can't cope).

On Mon, Jan 13, 2025, at 2:19 PM, Mark Pearson wrote:
> Subject: [External] Re: [PATCH v2] iio: hid-sensor-prox: Split 
> difference from multiple channels
>
> On Sat, 2025-01-11 at 10:17 +0100, Ricardo Ribalda wrote:
>> Hi Jonathan
>>
>> Happy new year!
>>
>> Friendly ping about this patch so we can change the ABI before the
>> kernel release happens
>>
>> On Thu, 19 Dec 2024 at 18:17, Jonathan Cameron <jic23@kernel.org>
>> wrote:
>> >
>> > On Mon, 16 Dec 2024 10:05:53 +0000
>> > Ricardo Ribalda <ribalda@chromium.org> wrote:
>> >
>> > > When the driver was originally created, it was decided that
>> > > sampling_frequency and hysteresis would be shared_per_type
>> > > instead
>> > > of shared_by_all (even though it is internally shared by all).
>> > > Eg:
>> > > in_proximity_raw
>> > > in_proximity_sampling_frequency
>> > >
>> > > When we introduced support for more channels, we continued with
>> > > shared_by_type which. Eg:
>> > > in_proximity0_raw
>> > > in_proximity1_raw
>> > > in_proximity_sampling_frequency
>> > > in_attention_raw
>> > > in_attention_sampling_frequency
>> > >
>> > > Ideally we should change to shared_by_all, but it is not an
>> > > option,
>> > > because the current naming has been a stablished ABI by now.
>> > > Luckily we
>> > > can use separate instead. That will be more consistent:
>> > > in_proximity0_raw
>> > > in_proximity0_sampling_frequency
>> > > in_proximity1_raw
>> > > in_proximity1_sampling_frequency
>> > > in_attention_raw
>> > > in_attention_sampling_frequency
>> > >
>> > > Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for more
>> > > channels")
>> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>> >
>> > I got lost somewhere in the discussion.  This is still an ABI
>> > change compared
>> > to original interface at the top (which is the one that has been
>> > there
>> > quite some time).
>> >
>> > However we already had to make one of those to add the index that
>> > wasn't there
>> > for _raw. (I'd missed that in earlier discussion - thanks for
>> > laying out the
>> > steps here!)
>
> Didn't realize this. I don't see proximity sensor use in the mainline
> Linux distro user space, so it will affect only some private user space
> programs.
> Adding Mark to see if it affects Lenovo Sensing solution as there was
> specific custom sensor added to this driver for Lenovo.
>

Can I get some pointers to what sensor that is please?
We've been asking for the HID support drivers, but it isn't available yet to my knowledge. Would the MIPI camera work tie into this?

If I can get details on what the sensor is I'll go and check what is impacted.

Thanks
Mark

Re: [PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Pandruvada, Srinivas]

On Mon, 2025-01-13 at 14:49 -0500, Mark Pearson wrote:
> Note - switched to my open-source friendly email account (avoid the
> Lenovo address, especially for mailing lists, it's Outlook based and
> can't cope).
> 
> On Mon, Jan 13, 2025, at 2:19 PM, Mark Pearson wrote:
> > Subject: [External] Re: [PATCH v2] iio: hid-sensor-prox: Split 
> > difference from multiple channels
> > 
> > On Sat, 2025-01-11 at 10:17 +0100, Ricardo Ribalda wrote:
> > > Hi Jonathan
> > > 
> > > Happy new year!
> > > 
> > > Friendly ping about this patch so we can change the ABI before
> > > the
> > > kernel release happens
> > > 
> > > On Thu, 19 Dec 2024 at 18:17, Jonathan Cameron <jic23@kernel.org>
> > > wrote:
> > > > 
> > > > On Mon, 16 Dec 2024 10:05:53 +0000
> > > > Ricardo Ribalda <ribalda@chromium.org> wrote:
> > > > 
> > > > > When the driver was originally created, it was decided that
> > > > > sampling_frequency and hysteresis would be shared_per_type
> > > > > instead
> > > > > of shared_by_all (even though it is internally shared by
> > > > > all).
> > > > > Eg:
> > > > > in_proximity_raw
> > > > > in_proximity_sampling_frequency
> > > > > 
> > > > > When we introduced support for more channels, we continued
> > > > > with
> > > > > shared_by_type which. Eg:
> > > > > in_proximity0_raw
> > > > > in_proximity1_raw
> > > > > in_proximity_sampling_frequency
> > > > > in_attention_raw
> > > > > in_attention_sampling_frequency
> > > > > 
> > > > > Ideally we should change to shared_by_all, but it is not an
> > > > > option,
> > > > > because the current naming has been a stablished ABI by now.
> > > > > Luckily we
> > > > > can use separate instead. That will be more consistent:
> > > > > in_proximity0_raw
> > > > > in_proximity0_sampling_frequency
> > > > > in_proximity1_raw
> > > > > in_proximity1_sampling_frequency
> > > > > in_attention_raw
> > > > > in_attention_sampling_frequency
> > > > > 
> > > > > Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for
> > > > > more
> > > > > channels")
> > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > 
> > > > I got lost somewhere in the discussion.  This is still an ABI
> > > > change compared
> > > > to original interface at the top (which is the one that has
> > > > been
> > > > there
> > > > quite some time).
> > > > 
> > > > However we already had to make one of those to add the index
> > > > that
> > > > wasn't there
> > > > for _raw. (I'd missed that in earlier discussion - thanks for
> > > > laying out the
> > > > steps here!)
> > 
> > Didn't realize this. I don't see proximity sensor use in the
> > mainline
> > Linux distro user space, so it will affect only some private user
> > space
> > programs.
> > Adding Mark to see if it affects Lenovo Sensing solution as there
> > was
> > specific custom sensor added to this driver for Lenovo.
> > 
> 
> Can I get some pointers to what sensor that is please?
> We've been asking for the HID support drivers, but it isn't available
> yet to my knowledge. Would the MIPI camera work tie into this?
No.

> 
> If I can get details on what the sensor is I'll go and check what is
> impacted.
> 
This is a custom sensor exported via Intel ISH

 /*
         * Lenovo Intelligent Sensing Solution (LISS)
         */


       {       /* human presence */
                .tag = "LISS",
                .luid = "0226000171AC0081",
                .model = "VL53L1_HOD Sensor",
                .manufacturer = "ST_MICRO",
                .check_dmi = true,
                .dmi.matches = {
                        DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
                }
        },

Question is what Lenovo user space is using this sensor?

Thanks,
Srinivas

> Thanks
> Mark

Re: [PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Philipp Jungkamp]

Hello,

that LISS sensor was from a patch that I had introduced. It's not
connected officially to Lenovo.

It's a HPD HID sensor on the Lenovo Yoga 9 14IAP7 that exports binary
values on the human presence HID usage that is monitored by
hid-sensor-prox.

I had tried to get it working with iio-sensor-proxy to expose it over
dbus and use it in e.g. a GNOME shell extension to lock the screen when
there is no human presence detected. But my MRs were never merged and
due to private struggles I lost interest in pushing them further.

https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/364
https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/364

Regards,
Philipp Jungkamp


On Mon, 2025-01-13 at 20:03 +0000, Pandruvada, Srinivas wrote:
> On Mon, 2025-01-13 at 14:49 -0500, Mark Pearson wrote:
> > Note - switched to my open-source friendly email account (avoid the
> > Lenovo address, especially for mailing lists, it's Outlook based
> > and
> > can't cope).
> > 
> > On Mon, Jan 13, 2025, at 2:19 PM, Mark Pearson wrote:
> > > Subject: [External] Re: [PATCH v2] iio: hid-sensor-prox: Split 
> > > difference from multiple channels
> > > 
> > > On Sat, 2025-01-11 at 10:17 +0100, Ricardo Ribalda wrote:
> > > > Hi Jonathan
> > > > 
> > > > Happy new year!
> > > > 
> > > > Friendly ping about this patch so we can change the ABI before
> > > > the
> > > > kernel release happens
> > > > 
> > > > On Thu, 19 Dec 2024 at 18:17, Jonathan Cameron
> > > > <jic23@kernel.org>
> > > > wrote:
> > > > > 
> > > > > On Mon, 16 Dec 2024 10:05:53 +0000
> > > > > Ricardo Ribalda <ribalda@chromium.org> wrote:
> > > > > 
> > > > > > When the driver was originally created, it was decided that
> > > > > > sampling_frequency and hysteresis would be shared_per_type
> > > > > > instead
> > > > > > of shared_by_all (even though it is internally shared by
> > > > > > all).
> > > > > > Eg:
> > > > > > in_proximity_raw
> > > > > > in_proximity_sampling_frequency
> > > > > > 
> > > > > > When we introduced support for more channels, we continued
> > > > > > with
> > > > > > shared_by_type which. Eg:
> > > > > > in_proximity0_raw
> > > > > > in_proximity1_raw
> > > > > > in_proximity_sampling_frequency
> > > > > > in_attention_raw
> > > > > > in_attention_sampling_frequency
> > > > > > 
> > > > > > Ideally we should change to shared_by_all, but it is not an
> > > > > > option,
> > > > > > because the current naming has been a stablished ABI by
> > > > > > now.
> > > > > > Luckily we
> > > > > > can use separate instead. That will be more consistent:
> > > > > > in_proximity0_raw
> > > > > > in_proximity0_sampling_frequency
> > > > > > in_proximity1_raw
> > > > > > in_proximity1_sampling_frequency
> > > > > > in_attention_raw
> > > > > > in_attention_sampling_frequency
> > > > > > 
> > > > > > Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for
> > > > > > more
> > > > > > channels")
> > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > 
> > > > > I got lost somewhere in the discussion.  This is still an ABI
> > > > > change compared
> > > > > to original interface at the top (which is the one that has
> > > > > been
> > > > > there
> > > > > quite some time).
> > > > > 
> > > > > However we already had to make one of those to add the index
> > > > > that
> > > > > wasn't there
> > > > > for _raw. (I'd missed that in earlier discussion - thanks for
> > > > > laying out the
> > > > > steps here!)
> > > 
> > > Didn't realize this. I don't see proximity sensor use in the
> > > mainline
> > > Linux distro user space, so it will affect only some private user
> > > space
> > > programs.
> > > Adding Mark to see if it affects Lenovo Sensing solution as there
> > > was
> > > specific custom sensor added to this driver for Lenovo.
> > > 
> > 
> > Can I get some pointers to what sensor that is please?
> > We've been asking for the HID support drivers, but it isn't
> > available
> > yet to my knowledge. Would the MIPI camera work tie into this?
> No.
> 
> > 
> > If I can get details on what the sensor is I'll go and check what
> > is
> > impacted.
> > 
> This is a custom sensor exported via Intel ISH
> 
>  /*
>          * Lenovo Intelligent Sensing Solution (LISS)
>          */
> 
> 
>        {       /* human presence */
>                 .tag = "LISS",
>                 .luid = "0226000171AC0081",
>                 .model = "VL53L1_HOD Sensor",
>                 .manufacturer = "ST_MICRO",
>                 .check_dmi = true,
>                 .dmi.matches = {
>                         DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>                 }
>         },
> 
> Question is what Lenovo user space is using this sensor?
> 
> Thanks,
> Srinivas
> 
> > Thanks
> > Mark
> 

Re: [PATCH v2] iio: hid-sensor-prox: Split difference from multiple channels

[Jonathan Cameron]

On Wed, 22 Jan 2025 14:55:07 +0100
Philipp Jungkamp <philipp@jungkamp.dev> wrote:

> Hello,
> 
> that LISS sensor was from a patch that I had introduced. It's not
> connected officially to Lenovo.
> 
> It's a HPD HID sensor on the Lenovo Yoga 9 14IAP7 that exports binary
> values on the human presence HID usage that is monitored by
> hid-sensor-prox.
> 
> I had tried to get it working with iio-sensor-proxy to expose it over
> dbus and use it in e.g. a GNOME shell extension to lock the screen when
> there is no human presence detected. But my MRs were never merged and
> due to private struggles I lost interest in pushing them further.
> 
> https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/364
> https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/364
> 
> Regards,
> Philipp Jungkamp
Thanks Philipp,

So my impression is no one (other than you!) will notice the ABI change.

I'll queue the patch for after the merge window.  That means
there is still time to shout if anyone disagrees!

Jonathan

> 
> 
> On Mon, 2025-01-13 at 20:03 +0000, Pandruvada, Srinivas wrote:
> > On Mon, 2025-01-13 at 14:49 -0500, Mark Pearson wrote:  
> > > Note - switched to my open-source friendly email account (avoid the
> > > Lenovo address, especially for mailing lists, it's Outlook based
> > > and
> > > can't cope).
> > > 
> > > On Mon, Jan 13, 2025, at 2:19 PM, Mark Pearson wrote:  
> > > > Subject: [External] Re: [PATCH v2] iio: hid-sensor-prox: Split 
> > > > difference from multiple channels
> > > > 
> > > > On Sat, 2025-01-11 at 10:17 +0100, Ricardo Ribalda wrote:  
> > > > > Hi Jonathan
> > > > > 
> > > > > Happy new year!
> > > > > 
> > > > > Friendly ping about this patch so we can change the ABI before
> > > > > the
> > > > > kernel release happens
> > > > > 
> > > > > On Thu, 19 Dec 2024 at 18:17, Jonathan Cameron
> > > > > <jic23@kernel.org>
> > > > > wrote:  
> > > > > > 
> > > > > > On Mon, 16 Dec 2024 10:05:53 +0000
> > > > > > Ricardo Ribalda <ribalda@chromium.org> wrote:
> > > > > >   
> > > > > > > When the driver was originally created, it was decided that
> > > > > > > sampling_frequency and hysteresis would be shared_per_type
> > > > > > > instead
> > > > > > > of shared_by_all (even though it is internally shared by
> > > > > > > all).
> > > > > > > Eg:
> > > > > > > in_proximity_raw
> > > > > > > in_proximity_sampling_frequency
> > > > > > > 
> > > > > > > When we introduced support for more channels, we continued
> > > > > > > with
> > > > > > > shared_by_type which. Eg:
> > > > > > > in_proximity0_raw
> > > > > > > in_proximity1_raw
> > > > > > > in_proximity_sampling_frequency
> > > > > > > in_attention_raw
> > > > > > > in_attention_sampling_frequency
> > > > > > > 
> > > > > > > Ideally we should change to shared_by_all, but it is not an
> > > > > > > option,
> > > > > > > because the current naming has been a stablished ABI by
> > > > > > > now.
> > > > > > > Luckily we
> > > > > > > can use separate instead. That will be more consistent:
> > > > > > > in_proximity0_raw
> > > > > > > in_proximity0_sampling_frequency
> > > > > > > in_proximity1_raw
> > > > > > > in_proximity1_sampling_frequency
> > > > > > > in_attention_raw
> > > > > > > in_attention_sampling_frequency
> > > > > > > 
> > > > > > > Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for
> > > > > > > more
> > > > > > > channels")
> > > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>  
> > > > > > 
> > > > > > I got lost somewhere in the discussion.  This is still an ABI
> > > > > > change compared
> > > > > > to original interface at the top (which is the one that has
> > > > > > been
> > > > > > there
> > > > > > quite some time).
> > > > > > 
> > > > > > However we already had to make one of those to add the index
> > > > > > that
> > > > > > wasn't there
> > > > > > for _raw. (I'd missed that in earlier discussion - thanks for
> > > > > > laying out the
> > > > > > steps here!)  
> > > > 
> > > > Didn't realize this. I don't see proximity sensor use in the
> > > > mainline
> > > > Linux distro user space, so it will affect only some private user
> > > > space
> > > > programs.
> > > > Adding Mark to see if it affects Lenovo Sensing solution as there
> > > > was
> > > > specific custom sensor added to this driver for Lenovo.
> > > >   
> > > 
> > > Can I get some pointers to what sensor that is please?
> > > We've been asking for the HID support drivers, but it isn't
> > > available
> > > yet to my knowledge. Would the MIPI camera work tie into this?  
> > No.
> >   
> > > 
> > > If I can get details on what the sensor is I'll go and check what
> > > is
> > > impacted.
> > >   
> > This is a custom sensor exported via Intel ISH
> > 
> >  /*
> >          * Lenovo Intelligent Sensing Solution (LISS)
> >          */
> > 
> > 
> >        {       /* human presence */
> >                 .tag = "LISS",
> >                 .luid = "0226000171AC0081",
> >                 .model = "VL53L1_HOD Sensor",
> >                 .manufacturer = "ST_MICRO",
> >                 .check_dmi = true,
> >                 .dmi.matches = {
> >                         DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> >                 }
> >         },
> > 
> > Question is what Lenovo user space is using this sensor?
> > 
> > Thanks,
> > Srinivas
> >   
> > > Thanks
> > > Mark  
> >   
>