linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
       [not found] ` <5a646527-7a1f-2fb9-7c09-8becdbff417b@lenovo.com>
@ 2020-10-07  8:36   ` Jonathan Cameron
  2020-10-07  9:51     ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Cameron @ 2020-10-07  8:36 UTC (permalink / raw)
  To: Mark Pearson
  Cc: Hans de Goede, linux-iio, Bastien Nocera, Nitin Joshi1,
	linux-input, dmitry.torokhov

On Mon, 5 Oct 2020 22:04:27 -0400
Mark Pearson <markpearson@lenovo.com> wrote:

> Adding Nitin, lead for this feature, to the thread

+CC linux-input and Dmitry for reasons that will become clear below.
> 
> On 2020-10-03 10:02 a.m., Hans de Goede wrote:
> > Hi All,
> > 
> > Modern laptops can have various sensors which are kinda
> > like proximity sensors, but not really (they are more
> > specific in which part of the laptop the user is
> > proximate to).
> > 
> > Specifically modern Thinkpad's have 2 readings which we
> > want to export to userspace, and I'm wondering if we
> > could use the IIO framework for this since these readings
> > are in essence sensor readings:
> > 
> > 1. These laptops have a sensor in the palm-rests to
> > check if a user is physically proximate to the device's
> > palm-rests. This info will be used by userspace for WWAN
> > functionality to control the transmission level safely.
> > 
> > A patch adding a thinkpad_acpi specific sysfs API for this
> > is currently pending:
> > https://patchwork.kernel.org/patch/11722127/
> > 
> > But I'm wondering if it would not be better to use
> > IIO to export this info.

My first thought on this is it sounds more like a key than a sensor
(simple proximity sensors fall into this category as well.)

Dmitry, any existing stuff like this in input?

If it does make sense to put it in IIO then rest of the questions
obviously relevant.

> > 
> > 2. These laptops have something called lap-mode, which
> > determines if the laptop's firmware thinks that it is on
> > a users lap, or sitting on a table. This influences the
> > max. allowed skin-temperature of the bottom of the laptop
> > and thus influences thermal management.  Like the palm-rest
> > snesors, this reading will likely also be used for
> > controlling wireless transmission levels in the future.
> > 
> > Note that AFAIK the lap_mode reading is not a single sensor
> > reading, it is a value derived from a bunch of sensor readings,
> > the raw values of which may or may not be available
> > separately.
> > 
> > So looking at existing IIO userspace API docs, focussing on
> > proximity sensors I see:
> > 
> > Documentation/ABI/testing/sysfs-bus-iio
> > Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
> > 
> > Where the latter seems to not really be relevant.

Indeed, that one is a very odd beast :) (lightning sensor)

> > 
> >  From the generic IO API doc, this bit is the most
> > interesting:
> > 
> > What:           /sys/.../iio:deviceX/in_proximity_raw
> > What:           /sys/.../iio:deviceX/in_proximity_input
> > What:           /sys/.../iio:deviceX/in_proximityY_raw
> > KernelVersion:  3.4
> > Contact:        linux-iio@vger.kernel.org
> > Description:
> >                  Proximity measurement indicating that some
> >                  object is near the sensor, usually by observing
> >                  reflectivity of infrared or ultrasound emitted.
> >                  Often these sensors are unit less and as such conversion
> >                  to SI units is not possible. Higher proximity measurements
> >                  indicate closer objects, and vice versa. Units after
> >                  application of scale and offset are meters.
> > 
> > This seems to be a reasonable match for the Thinkpad sensors
> > we are discussing here, although those report a simple
> > 0/1 value.

Given this is a bit of computed estimate rather than a true reading, I wonder
a bit if we should treat it as closer to an 'activity classification sensor'.

For those we use a percentage value to represent the output of some probabilistic
classifier.  In reality all the versions we've had so far aren't that clever though
so they only output 0 or 100%.  See in_activity_walking_input in the docs for
example.

> > 
> > What is missing for the ThinkPad case is something like this:
> > 
> > What:        /sys/.../iio:deviceX/proximity_sensor_location
> > KernelVersion:  5.11
> > Contact:        linux-iio@vger.kernel.org
> > Description:
> >          Specifies the location of the proximity sensor /
> >          specifies proximity to what the sensor is measuring.
> >          Reading this file returns a string describing this, valid values
> >          for this string are: "screen", "lap", "palmrest"
> >          Note the list of valid values may be extended in the
> >          future.
> > 
> > So what do you (IIO devs) think about this?
> > 
> > Would adding a proximity_sensor_location attribute be a reasonable
> > thing to do for this; and do you think that this would be a good idea ?

Absolutely fine.  There is precedence in cros_ec which has a generic
location sysfs attribute (not associated with a particular channel though
it is fine to do that as well). See Documentation/ABI/testing/sysfs-bus-iio-cros_ec
We haven't moved it to the general docs because there is only one device
providing it so far.  Hence we would move it with the introduction of
this second device.

> > 
> > Regards,
> > 
> > Hans
> >   



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-07  8:36   ` [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace? Jonathan Cameron
@ 2020-10-07  9:51     ` Hans de Goede
  2020-10-07 11:35       ` Bastien Nocera
  2020-11-12  6:23       ` Dmitry Torokhov
  0 siblings, 2 replies; 28+ messages in thread
From: Hans de Goede @ 2020-10-07  9:51 UTC (permalink / raw)
  To: Jonathan Cameron, Mark Pearson
  Cc: linux-iio, Bastien Nocera, Nitin Joshi1, linux-input, dmitry.torokhov

Hi,

On 10/7/20 10:36 AM, Jonathan Cameron wrote:
> On Mon, 5 Oct 2020 22:04:27 -0400
> Mark Pearson <markpearson@lenovo.com> wrote:
> 
>> Adding Nitin, lead for this feature, to the thread
> 
> +CC linux-input and Dmitry for reasons that will become clear below.
>>
>> On 2020-10-03 10:02 a.m., Hans de Goede wrote:
>>> Hi All,
>>>
>>> Modern laptops can have various sensors which are kinda
>>> like proximity sensors, but not really (they are more
>>> specific in which part of the laptop the user is
>>> proximate to).
>>>
>>> Specifically modern Thinkpad's have 2 readings which we
>>> want to export to userspace, and I'm wondering if we
>>> could use the IIO framework for this since these readings
>>> are in essence sensor readings:
>>>
>>> 1. These laptops have a sensor in the palm-rests to
>>> check if a user is physically proximate to the device's
>>> palm-rests. This info will be used by userspace for WWAN
>>> functionality to control the transmission level safely.
>>>
>>> A patch adding a thinkpad_acpi specific sysfs API for this
>>> is currently pending:
>>> https://patchwork.kernel.org/patch/11722127/
>>>
>>> But I'm wondering if it would not be better to use
>>> IIO to export this info.
> 
> My first thought on this is it sounds more like a key than a sensor
> (simple proximity sensors fall into this category as well.)

That is an interesting suggestion. Using the input/evdev API
would have some advantages such as being able to have a single
event node for all the proximity switches and then being able
to pass a fd to that from a privileged process to a non
privileged one, something which userspace already has
various infrastructure for.

So yes this might indeed be better. Dmitry any thoughts on
this / objections against using the input/evdev API for this?

Note: s/key/switch/ in "sounds more like a key" above I guess.

> Dmitry, any existing stuff like this in input?

There already is a SW_FRONT_PROXIMITY defined in
input-event-codes.h, which I guess means detection if
someone is sitting in front of the screen. So we could add:

SW_LAP_PROXIMITY
SW_PALMREST_PROXIMITY,

And then we have a pretty decent API for this I think.

> If it does make sense to put it in IIO then rest of the questions
> obviously relevant.

Ack, thank you for your input.

Regards,

Hans





>>> 2. These laptops have something called lap-mode, which
>>> determines if the laptop's firmware thinks that it is on
>>> a users lap, or sitting on a table. This influences the
>>> max. allowed skin-temperature of the bottom of the laptop
>>> and thus influences thermal management.  Like the palm-rest
>>> snesors, this reading will likely also be used for
>>> controlling wireless transmission levels in the future.
>>>
>>> Note that AFAIK the lap_mode reading is not a single sensor
>>> reading, it is a value derived from a bunch of sensor readings,
>>> the raw values of which may or may not be available
>>> separately.
>>>
>>> So looking at existing IIO userspace API docs, focussing on
>>> proximity sensors I see:
>>>
>>> Documentation/ABI/testing/sysfs-bus-iio
>>> Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935
>>>
>>> Where the latter seems to not really be relevant.
> 
> Indeed, that one is a very odd beast :) (lightning sensor)
> 
>>>
>>>   From the generic IO API doc, this bit is the most
>>> interesting:
>>>
>>> What:           /sys/.../iio:deviceX/in_proximity_raw
>>> What:           /sys/.../iio:deviceX/in_proximity_input
>>> What:           /sys/.../iio:deviceX/in_proximityY_raw
>>> KernelVersion:  3.4
>>> Contact:        linux-iio@vger.kernel.org
>>> Description:
>>>                   Proximity measurement indicating that some
>>>                   object is near the sensor, usually by observing
>>>                   reflectivity of infrared or ultrasound emitted.
>>>                   Often these sensors are unit less and as such conversion
>>>                   to SI units is not possible. Higher proximity measurements
>>>                   indicate closer objects, and vice versa. Units after
>>>                   application of scale and offset are meters.
>>>
>>> This seems to be a reasonable match for the Thinkpad sensors
>>> we are discussing here, although those report a simple
>>> 0/1 value.
> 
> Given this is a bit of computed estimate rather than a true reading, I wonder
> a bit if we should treat it as closer to an 'activity classification sensor'.
> 
> For those we use a percentage value to represent the output of some probabilistic
> classifier.  In reality all the versions we've had so far aren't that clever though
> so they only output 0 or 100%.  See in_activity_walking_input in the docs for
> example.
> 
>>>
>>> What is missing for the ThinkPad case is something like this:
>>>
>>> What:        /sys/.../iio:deviceX/proximity_sensor_location
>>> KernelVersion:  5.11
>>> Contact:        linux-iio@vger.kernel.org
>>> Description:
>>>           Specifies the location of the proximity sensor /
>>>           specifies proximity to what the sensor is measuring.
>>>           Reading this file returns a string describing this, valid values
>>>           for this string are: "screen", "lap", "palmrest"
>>>           Note the list of valid values may be extended in the
>>>           future.
>>>
>>> So what do you (IIO devs) think about this?
>>>
>>> Would adding a proximity_sensor_location attribute be a reasonable
>>> thing to do for this; and do you think that this would be a good idea ?
> 
> Absolutely fine.  There is precedence in cros_ec which has a generic
> location sysfs attribute (not associated with a particular channel though
> it is fine to do that as well). See Documentation/ABI/testing/sysfs-bus-iio-cros_ec
> We haven't moved it to the general docs because there is only one device
> providing it so far.  Hence we would move it with the introduction of
> this second device.
> 
>>>
>>> Regards,
>>>
>>> Hans
>>>    
> 
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-07  9:51     ` Hans de Goede
@ 2020-10-07 11:35       ` Bastien Nocera
  2020-10-07 13:08         ` Hans de Goede
  2020-11-12  6:23       ` Dmitry Torokhov
  1 sibling, 1 reply; 28+ messages in thread
From: Bastien Nocera @ 2020-10-07 11:35 UTC (permalink / raw)
  To: Hans de Goede, Jonathan Cameron, Mark Pearson
  Cc: linux-iio, Nitin Joshi1, linux-input, dmitry.torokhov

On Wed, 2020-10-07 at 11:51 +0200, Hans de Goede wrote:
> <snip>
> > Dmitry, any existing stuff like this in input?
> 
> There already is a SW_FRONT_PROXIMITY defined in
> input-event-codes.h, which I guess means detection if
> someone is sitting in front of the screen. So we could add:
> 
> SW_LAP_PROXIMITY
> SW_PALMREST_PROXIMITY,
> 
> And then we have a pretty decent API for this I think.

From the point of view of writing the consumer code for this API, it's
rather a lot of pain to open the device node (hoping that it's the
right one for what we need), getting the initial state, setting up
masks to avoid being woken up for every little event, and parsing those
events.

Where would the necessary bits of metadata for daemons to be able to
find that those switches need to get added?

If you go down that route, you'll definitely want a want to attach the
"palmrest" to the touchpad/keyboard that it corresponds to, otherwise
that might have weird interactions when using external keyboards and
touchpads. (I don't know what you'd use that proximity sensor for
though)

Cheers


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-07 11:35       ` Bastien Nocera
@ 2020-10-07 13:08         ` Hans de Goede
  2020-10-07 13:29           ` Bastien Nocera
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2020-10-07 13:08 UTC (permalink / raw)
  To: Bastien Nocera, Jonathan Cameron, Mark Pearson
  Cc: linux-iio, Nitin Joshi1, linux-input, dmitry.torokhov

Hi,

On 10/7/20 1:35 PM, Bastien Nocera wrote:
> On Wed, 2020-10-07 at 11:51 +0200, Hans de Goede wrote:
>> <snip>
>>> Dmitry, any existing stuff like this in input?
>>
>> There already is a SW_FRONT_PROXIMITY defined in
>> input-event-codes.h, which I guess means detection if
>> someone is sitting in front of the screen. So we could add:
>>
>> SW_LAP_PROXIMITY
>> SW_PALMREST_PROXIMITY,
>>
>> And then we have a pretty decent API for this I think.
> 
>  From the point of view of writing the consumer code for this API, it's
> rather a lot of pain to open the device node (hoping that it's the
> right one for what we need), getting the initial state, setting up
> masks to avoid being woken up for every little event, and parsing those
> events.

There is not much difference with the iio sysfs API though, you would
also need to iterate over all the iio devices and test if they
are a proximity sensor (and read the new location sysfs file discussed).

> Where would the necessary bits of metadata for daemons to be able to
> find that those switches need to get added?

evdev files export info on which events they can report. Not only
the types of events (EV_SW in this case) but also a bitmask for
which event-codes they can report within that type.

> If you go down that route, you'll definitely want a want to attach the
> "palmrest" to the touchpad/keyboard that it corresponds to, otherwise
> that might have weird interactions when using external keyboards and
> touchpads. (I don't know what you'd use that proximity sensor for
> though)

The proximity sensor is primarily for deciding how strong a signal
wireless devices inside the laptop may emit.

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-07 13:08         ` Hans de Goede
@ 2020-10-07 13:29           ` Bastien Nocera
  2020-10-07 13:32             ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Bastien Nocera @ 2020-10-07 13:29 UTC (permalink / raw)
  To: Hans de Goede, Jonathan Cameron, Mark Pearson
  Cc: linux-iio, Nitin Joshi1, linux-input, dmitry.torokhov

On Wed, 2020-10-07 at 15:08 +0200, Hans de Goede wrote:
> Hi,
> 
> On 10/7/20 1:35 PM, Bastien Nocera wrote:
> > On Wed, 2020-10-07 at 11:51 +0200, Hans de Goede wrote:
> > > <snip>
> > > > Dmitry, any existing stuff like this in input?
> > > 
> > > There already is a SW_FRONT_PROXIMITY defined in
> > > input-event-codes.h, which I guess means detection if
> > > someone is sitting in front of the screen. So we could add:
> > > 
> > > SW_LAP_PROXIMITY
> > > SW_PALMREST_PROXIMITY,
> > > 
> > > And then we have a pretty decent API for this I think.
> > 
> >  From the point of view of writing the consumer code for this API,
> > it's
> > rather a lot of pain to open the device node (hoping that it's the
> > right one for what we need), getting the initial state, setting up
> > masks to avoid being woken up for every little event, and parsing
> > those
> > events.
> 
> There is not much difference with the iio sysfs API though, you would
> also need to iterate over all the iio devices and test if they
> are a proximity sensor (and read the new location sysfs file
> discussed).
> 
> > Where would the necessary bits of metadata for daemons to be able
> > to
> > find that those switches need to get added?
> 
> evdev files export info on which events they can report. Not only
> the types of events (EV_SW in this case) but also a bitmask for
> which event-codes they can report within that type.

I know that, I've written enough of that type of code ;)

I meant a way to avoid having to go open each evdev, read its
capabilities, and close them when it doesn't, and re-do that every time
a new event device appears. In other words, can you please make sure
there will be a udev property that we can use to enumerate and filter
devices with those capabilities?

> > If you go down that route, you'll definitely want a want to attach
> > the
> > "palmrest" to the touchpad/keyboard that it corresponds to,
> > otherwise
> > that might have weird interactions when using external keyboards
> > and
> > touchpads. (I don't know what you'd use that proximity sensor for
> > though)
> 
> The proximity sensor is primarily for deciding how strong a signal
> wireless devices inside the laptop may emit.

So we'll need a way to know what wireless device is inside the laptop
so that policy only applies to that one. This was already fun when it
was just event devices that needed grouping ;)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-07 13:29           ` Bastien Nocera
@ 2020-10-07 13:32             ` Hans de Goede
  2020-10-08  0:14               ` Jeff LaBundy
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2020-10-07 13:32 UTC (permalink / raw)
  To: Bastien Nocera, Jonathan Cameron, Mark Pearson
  Cc: linux-iio, Nitin Joshi1, linux-input, dmitry.torokhov

Hi,

On 10/7/20 3:29 PM, Bastien Nocera wrote:
> On Wed, 2020-10-07 at 15:08 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 10/7/20 1:35 PM, Bastien Nocera wrote:
>>> On Wed, 2020-10-07 at 11:51 +0200, Hans de Goede wrote:
>>>> <snip>
>>>>> Dmitry, any existing stuff like this in input?
>>>>
>>>> There already is a SW_FRONT_PROXIMITY defined in
>>>> input-event-codes.h, which I guess means detection if
>>>> someone is sitting in front of the screen. So we could add:
>>>>
>>>> SW_LAP_PROXIMITY
>>>> SW_PALMREST_PROXIMITY,
>>>>
>>>> And then we have a pretty decent API for this I think.
>>>
>>>   From the point of view of writing the consumer code for this API,
>>> it's
>>> rather a lot of pain to open the device node (hoping that it's the
>>> right one for what we need), getting the initial state, setting up
>>> masks to avoid being woken up for every little event, and parsing
>>> those
>>> events.
>>
>> There is not much difference with the iio sysfs API though, you would
>> also need to iterate over all the iio devices and test if they
>> are a proximity sensor (and read the new location sysfs file
>> discussed).
>>
>>> Where would the necessary bits of metadata for daemons to be able
>>> to
>>> find that those switches need to get added?
>>
>> evdev files export info on which events they can report. Not only
>> the types of events (EV_SW in this case) but also a bitmask for
>> which event-codes they can report within that type.
> 
> I know that, I've written enough of that type of code ;)
> 
> I meant a way to avoid having to go open each evdev, read its
> capabilities, and close them when it doesn't, and re-do that every time
> a new event device appears. In other words, can you please make sure
> there will be a udev property that we can use to enumerate and filter
> devices with those capabilities?

Ah I see, yes that should not be a problem since we already run
a helper on all new evdev-s anyways (assuming we go this route).

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-07 13:32             ` Hans de Goede
@ 2020-10-08  0:14               ` Jeff LaBundy
  2020-10-08  7:10                 ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Jeff LaBundy @ 2020-10-08  0:14 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Bastien Nocera, Jonathan Cameron, Mark Pearson, linux-iio,
	Nitin Joshi1, linux-input, dmitry.torokhov

Hi all,

On Wed, Oct 07, 2020 at 03:32:07PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10/7/20 3:29 PM, Bastien Nocera wrote:
> >On Wed, 2020-10-07 at 15:08 +0200, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 10/7/20 1:35 PM, Bastien Nocera wrote:
> >>>On Wed, 2020-10-07 at 11:51 +0200, Hans de Goede wrote:
> >>>><snip>
> >>>>>Dmitry, any existing stuff like this in input?

It seems we are talking about "specific absorption rate" (SAR) type
devices that signal the WLAN controller to reduce transmitted power
while a user is nearby.

I just wanted to chime in and confirm that we do have at least one
precedent for these being in input (keyboard/iqs62x-keys) and not
iio so I agree with Jonathan here. My argument is that we want to
signal binary events (user grabbed onto or let go of the handset)
rather than deliver continuous data.

The example I've shown reports events as keycodes since some of the
events it can express represent momentary conditions. In hindsight,
however, an argument can be made to express some of this information
as a switch (user is or is not near the device) and the new event
codes proposed here seem like a step in the right direction.

> >>>>
> >>>>There already is a SW_FRONT_PROXIMITY defined in
> >>>>input-event-codes.h, which I guess means detection if
> >>>>someone is sitting in front of the screen. So we could add:
> >>>>
> >>>>SW_LAP_PROXIMITY
> >>>>SW_PALMREST_PROXIMITY,
> >>>>
> >>>>And then we have a pretty decent API for this I think.
> >>>
> >>>  From the point of view of writing the consumer code for this API,
> >>>it's
> >>>rather a lot of pain to open the device node (hoping that it's the
> >>>right one for what we need), getting the initial state, setting up
> >>>masks to avoid being woken up for every little event, and parsing
> >>>those
> >>>events.
> >>
> >>There is not much difference with the iio sysfs API though, you would
> >>also need to iterate over all the iio devices and test if they
> >>are a proximity sensor (and read the new location sysfs file
> >>discussed).
> >>
> >>>Where would the necessary bits of metadata for daemons to be able
> >>>to
> >>>find that those switches need to get added?
> >>
> >>evdev files export info on which events they can report. Not only
> >>the types of events (EV_SW in this case) but also a bitmask for
> >>which event-codes they can report within that type.
> >
> >I know that, I've written enough of that type of code ;)
> >
> >I meant a way to avoid having to go open each evdev, read its
> >capabilities, and close them when it doesn't, and re-do that every time
> >a new event device appears. In other words, can you please make sure
> >there will be a udev property that we can use to enumerate and filter
> >devices with those capabilities?
> 
> Ah I see, yes that should not be a problem since we already run
> a helper on all new evdev-s anyways (assuming we go this route).
> 
> Regards,
> 
> Hans
> 

Kind regards,
Jeff LaBundy

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-08  0:14               ` Jeff LaBundy
@ 2020-10-08  7:10                 ` Hans de Goede
  2020-10-09  2:19                   ` Jeff LaBundy
  2020-10-12 12:36                   ` Enrico Weigelt, metux IT consult
  0 siblings, 2 replies; 28+ messages in thread
From: Hans de Goede @ 2020-10-08  7:10 UTC (permalink / raw)
  To: Jeff LaBundy
  Cc: Bastien Nocera, Jonathan Cameron, Mark Pearson, linux-iio,
	Nitin Joshi1, linux-input, dmitry.torokhov

Hi,

On 10/8/20 2:14 AM, Jeff LaBundy wrote:
> Hi all,
> 
> On Wed, Oct 07, 2020 at 03:32:07PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 10/7/20 3:29 PM, Bastien Nocera wrote:
>>> On Wed, 2020-10-07 at 15:08 +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 10/7/20 1:35 PM, Bastien Nocera wrote:
>>>>> On Wed, 2020-10-07 at 11:51 +0200, Hans de Goede wrote:
>>>>>> <snip>
>>>>>>> Dmitry, any existing stuff like this in input?
> 
> It seems we are talking about "specific absorption rate" (SAR) type
> devices that signal the WLAN controller to reduce transmitted power
> while a user is nearby.

Yes and no. At least the lap-mode detection (laptop on someones
lap rather then sitting on a table) is currently used by the
embedded-controller for thermal management decisions, basically
when on someones lap the configurable TPD of the CPU is set lower
to keep the laptop's bottom skin temperate < 45 degrees Celsius
(I think it is 45 but the exact number does not matter).

The lap-mode info is currently exported with a thinkpad_acpi specific
sysfs attribute with the idea that userspace could potentially use
this to indicate to the user that turbo clocks will be lower
because of this.

With upcoming WLAN cards with configurable transmit power,
this will also be used as what you call a SAR device.

AFAIK the palmrest case is mostly a SAR device.

Note I'm explaining the alternative lap-mode use-case to make
sure everyone is on the same page. I completely agree with the
gist of your email :)

> I just wanted to chime in and confirm that we do have at least one
> precedent for these being in input (keyboard/iqs62x-keys) and not
> iio so I agree with Jonathan here. My argument is that we want to
> signal binary events (user grabbed onto or let go of the handset)
> rather than deliver continuous data.

I was curious what keycode you are using for this, but I see
that the keycodes come from devicetree, so I guess I should
just ask: what keycode are you using for this ?

> The example I've shown reports events as keycodes since some of the
> events it can express represent momentary conditions. In hindsight,
> however, an argument can be made to express some of this information
> as a switch (user is or is not near the device) and the new event
> codes proposed here seem like a step in the right direction.

I'm glad that you like the new proposed switch event-codes.

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-08  7:10                 ` Hans de Goede
@ 2020-10-09  2:19                   ` Jeff LaBundy
  2020-10-12 12:13                     ` Hans de Goede
  2020-10-12 12:36                   ` Enrico Weigelt, metux IT consult
  1 sibling, 1 reply; 28+ messages in thread
From: Jeff LaBundy @ 2020-10-09  2:19 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Bastien Nocera, Jonathan Cameron, Mark Pearson, linux-iio,
	Nitin Joshi1, linux-input, dmitry.torokhov

Hi Hans,

On Thu, Oct 08, 2020 at 09:10:19AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10/8/20 2:14 AM, Jeff LaBundy wrote:
> >Hi all,
> >
> >On Wed, Oct 07, 2020 at 03:32:07PM +0200, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 10/7/20 3:29 PM, Bastien Nocera wrote:
> >>>On Wed, 2020-10-07 at 15:08 +0200, Hans de Goede wrote:
> >>>>Hi,
> >>>>
> >>>>On 10/7/20 1:35 PM, Bastien Nocera wrote:
> >>>>>On Wed, 2020-10-07 at 11:51 +0200, Hans de Goede wrote:
> >>>>>><snip>
> >>>>>>>Dmitry, any existing stuff like this in input?
> >
> >It seems we are talking about "specific absorption rate" (SAR) type
> >devices that signal the WLAN controller to reduce transmitted power
> >while a user is nearby.
> 
> Yes and no. At least the lap-mode detection (laptop on someones
> lap rather then sitting on a table) is currently used by the
> embedded-controller for thermal management decisions, basically
> when on someones lap the configurable TPD of the CPU is set lower
> to keep the laptop's bottom skin temperate < 45 degrees Celsius
> (I think it is 45 but the exact number does not matter).

This is a much-appreciated feature. :)

> 
> The lap-mode info is currently exported with a thinkpad_acpi specific
> sysfs attribute with the idea that userspace could potentially use
> this to indicate to the user that turbo clocks will be lower
> because of this.
> 
> With upcoming WLAN cards with configurable transmit power,
> this will also be used as what you call a SAR device.
> 
> AFAIK the palmrest case is mostly a SAR device.
> 
> Note I'm explaining the alternative lap-mode use-case to make
> sure everyone is on the same page. I completely agree with the
> gist of your email :)

Acknowledged on all counts; thank you for this additional background
information.

> 
> >I just wanted to chime in and confirm that we do have at least one
> >precedent for these being in input (keyboard/iqs62x-keys) and not
> >iio so I agree with Jonathan here. My argument is that we want to
> >signal binary events (user grabbed onto or let go of the handset)
> >rather than deliver continuous data.
> 
> I was curious what keycode you are using for this, but I see
> that the keycodes come from devicetree, so I guess I should
> just ask: what keycode are you using for this ?

The idea here was that a vendor might implement their own daemon
that interprets any keycode of their choice, hence leaving the
keycodes assignable via devicetree.

This particular device also acts as a capacitive/inductive button
sensor, and these applications were the primary motivation for it
landing in input with its status bits mapped to keycodes.

I don't think there are any keycodes that exist today that would
universally work for this application. The couple that seem most
closely related (e.g. KEY_WLAN or KEY_RFKILL) are typically used
for disabling the adapter entirely or for airplane mode (please
correct me if I'm wrong).

To that end, I'm keen to see how this interface unfolds because
SAR detection tends to be an available mode of operation for
several of the capacitive touch devices I've been working with.

> 
> >The example I've shown reports events as keycodes since some of the
> >events it can express represent momentary conditions. In hindsight,
> >however, an argument can be made to express some of this information
> >as a switch (user is or is not near the device) and the new event
> >codes proposed here seem like a step in the right direction.
> 
> I'm glad that you like the new proposed switch event-codes.
> 
> Regards,
> 
> Hans
> 

Kind regards,
Jeff LaBundy

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-09  2:19                   ` Jeff LaBundy
@ 2020-10-12 12:13                     ` Hans de Goede
  2020-10-13 21:59                       ` Mark Pearson
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2020-10-12 12:13 UTC (permalink / raw)
  To: Jeff LaBundy
  Cc: Bastien Nocera, Jonathan Cameron, Mark Pearson, linux-iio,
	Nitin Joshi1, linux-input, dmitry.torokhov

Hi,

On 10/9/20 4:19 AM, Jeff LaBundy wrote:
> Hi Hans,
> 
> On Thu, Oct 08, 2020 at 09:10:19AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 10/8/20 2:14 AM, Jeff LaBundy wrote:
>>> Hi all,
>>>
>>> On Wed, Oct 07, 2020 at 03:32:07PM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 10/7/20 3:29 PM, Bastien Nocera wrote:
>>>>> On Wed, 2020-10-07 at 15:08 +0200, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 10/7/20 1:35 PM, Bastien Nocera wrote:
>>>>>>> On Wed, 2020-10-07 at 11:51 +0200, Hans de Goede wrote:
>>>>>>>> <snip>
>>>>>>>>> Dmitry, any existing stuff like this in input?
>>>
>>> It seems we are talking about "specific absorption rate" (SAR) type
>>> devices that signal the WLAN controller to reduce transmitted power
>>> while a user is nearby.
>>
>> Yes and no. At least the lap-mode detection (laptop on someones
>> lap rather then sitting on a table) is currently used by the
>> embedded-controller for thermal management decisions, basically
>> when on someones lap the configurable TPD of the CPU is set lower
>> to keep the laptop's bottom skin temperate < 45 degrees Celsius
>> (I think it is 45 but the exact number does not matter).
> 
> This is a much-appreciated feature. :)
> 
>>
>> The lap-mode info is currently exported with a thinkpad_acpi specific
>> sysfs attribute with the idea that userspace could potentially use
>> this to indicate to the user that turbo clocks will be lower
>> because of this.
>>
>> With upcoming WLAN cards with configurable transmit power,
>> this will also be used as what you call a SAR device.
>>
>> AFAIK the palmrest case is mostly a SAR device.
>>
>> Note I'm explaining the alternative lap-mode use-case to make
>> sure everyone is on the same page. I completely agree with the
>> gist of your email :)
> 
> Acknowledged on all counts; thank you for this additional background
> information.
> 
>>
>>> I just wanted to chime in and confirm that we do have at least one
>>> precedent for these being in input (keyboard/iqs62x-keys) and not
>>> iio so I agree with Jonathan here. My argument is that we want to
>>> signal binary events (user grabbed onto or let go of the handset)
>>> rather than deliver continuous data.
>>
>> I was curious what keycode you are using for this, but I see
>> that the keycodes come from devicetree, so I guess I should
>> just ask: what keycode are you using for this ?
> 
> The idea here was that a vendor might implement their own daemon
> that interprets any keycode of their choice, hence leaving the
> keycodes assignable via devicetree.
> 
> This particular device also acts as a capacitive/inductive button
> sensor, and these applications were the primary motivation for it
> landing in input with its status bits mapped to keycodes.
> 
> I don't think there are any keycodes that exist today that would
> universally work for this application. The couple that seem most
> closely related (e.g. KEY_WLAN or KEY_RFKILL) are typically used
> for disabling the adapter entirely or for airplane mode (please
> correct me if I'm wrong).

You're right (aka not wrong), KEY_WLAN and KEY_RFKILL are used to
toggle wireless radios on/off and re-using them for some SAR
purpose would lead to nothing but confusion. We really need to
define some standard *new* event-codes for this, such as e.g.
the proposed SW_LAP_PROXIMITY and SW_PALMREST_PROXIMITY.

> To that end, I'm keen to see how this interface unfolds because
> SAR detection tends to be an available mode of operation for
> several of the capacitive touch devices I've been working with.

I guess that for touchscreens at least (which are on the front),
using the existing SW_FRONT_PROXIMITY would make the most sense.

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-08  7:10                 ` Hans de Goede
  2020-10-09  2:19                   ` Jeff LaBundy
@ 2020-10-12 12:36                   ` Enrico Weigelt, metux IT consult
  2020-10-13  1:12                     ` Mark Pearson
  2020-10-13  8:38                     ` Hans de Goede
  1 sibling, 2 replies; 28+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-10-12 12:36 UTC (permalink / raw)
  To: Hans de Goede, Jeff LaBundy
  Cc: Bastien Nocera, Jonathan Cameron, Mark Pearson, linux-iio,
	Nitin Joshi1, linux-input, dmitry.torokhov

On 08.10.20 09:10, Hans de Goede wrote:

Hi folks,

> Yes and no. At least the lap-mode detection (laptop on someones
> lap rather then sitting on a table) is currently used by the
> embedded-controller for thermal management decisions, basically
> when on someones lap the configurable TPD of the CPU is set lower
> to keep the laptop's bottom skin temperate < 45 degrees Celsius
> (I think it is 45 but the exact number does not matter).

Am I the only one who thinks the whole concept is a pretty weird
idea ?

IIRC the machine becomes slower when it *thinks* its on my lap,
but runs faster - and becomes hotter - when it's laying around
somewhere, eg. ontop of some papers ?

Where can I get the drugs that these guys took ? :o

> With upcoming WLAN cards with configurable transmit power,
> this will also be used as what you call a SAR device.

Same fun. Once a person comes near, the signal gets weaker and
potentially connection breaks. Great fun for debugging.

Back to the technical side: IMHO we should first work out what the
actual purpose of these sensors could be - are they useful for
anything else than just these specific cases ? If not, I'm not
sure whether it makes sense to put them into IIO at all, but using
a specific board driver instead.

Okay, maybe we find these sensors somewhere else (maybe some embedded
stuff), for completely different purpose - in that case having one
standard driver (for the sensor itself) could make sense.

But that leads me to bigger topic: we've got several cases of some
sensors/chips used in different subsystems, eg. simple one-shot
ADCs, eeprom's, etc. ... maybe we should move them to separate
subsystems, which then can be wired to other (more specific) ones
in a very generic way ? ... just some quick+dirty thoughs,


--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-12 12:36                   ` Enrico Weigelt, metux IT consult
@ 2020-10-13  1:12                     ` Mark Pearson
  2020-10-13  8:38                     ` Hans de Goede
  1 sibling, 0 replies; 28+ messages in thread
From: Mark Pearson @ 2020-10-13  1:12 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, Hans de Goede, Jeff LaBundy
  Cc: Bastien Nocera, Jonathan Cameron, linux-iio, Nitin Joshi1,
	linux-input, dmitry.torokhov

Hi,

On 2020-10-12 8:36 a.m., Enrico Weigelt, metux IT consult wrote:
> On 08.10.20 09:10, Hans de Goede wrote:
> 
> Hi folks,
> 
>> Yes and no. At least the lap-mode detection (laptop on someones
>> lap rather then sitting on a table) is currently used by the
>> embedded-controller for thermal management decisions, basically
>> when on someones lap the configurable TPD of the CPU is set lower
>> to keep the laptop's bottom skin temperate < 45 degrees Celsius
>> (I think it is 45 but the exact number does not matter).
> 
> Am I the only one who thinks the whole concept is a pretty weird
> idea ?
> 
> IIRC the machine becomes slower when it *thinks* its on my lap,
> but runs faster - and becomes hotter - when it's laying around
> somewhere, eg. ontop of some papers ?
> 
> Where can I get the drugs that these guys took ? :o
This made me smile :) But I think it's safe to so no dubious substances 
were involved and it's really not that weird. We try very hard to not 
burn our customers, many of them appreciate that. We haven't yet 
implemented a paper sensor so that feature isn't available yet.

A lot of Linux users, quite reasonably, want to be able to access the 
maximum power available from their unit and so the logical conclusion is 
you can have max power (and therefore temperatures) when it's not on 
your lap, but in the interest of making the device safe and comfortable 
when it's on your lap the power rating drops.
I think this implementation is pretty common across all vendors these 
days - we're just exposing the lapmode sensor to user space to make it 
more obvious to users *why* the power dropped. We will also use both the 
lapmode and palm sensor for WWAN.

> 
>> With upcoming WLAN cards with configurable transmit power,
>> this will also be used as what you call a SAR device.
Minor correction - we're using this for WWAN

> 
> Same fun. Once a person comes near, the signal gets weaker and
> potentially connection breaks. Great fun for debugging.

My understanding is it's the way it's done on Windows and it is a FCC 
legal requirement so we can't get away from it. We could do what I think 
most vendors do and only provide the low power mode, but we're trying to 
give full and equivalent support to Linux users, so they can have full 
power when possible, and that means these proximity sensors being 
available to user space.

I hear you on the debugging but Windows seems to have managed OK.

> 
> Back to the technical side: IMHO we should first work out what the
> actual purpose of these sensors could be - are they useful for
> anything else than just these specific cases ? If not, I'm not
> sure whether it makes sense to put them into IIO at all, but using
> a specific board driver instead.

Hopefully the above helps explain the purpose of them a bit.

 From my point of view, I'm pretty new to the kernel contribution side 
of things so want to do whatever is recommended from the kernel 
community but gets these sensor states to user space so we can give 
Linux users a better experience on Lenovo platforms.

I think we've settled on using the input system instead of iio so maybe 
this thread is moot - but I wanted to respond in case details were 
useful or interesting.

> 
> Okay, maybe we find these sensors somewhere else (maybe some embedded
> stuff), for completely different purpose - in that case having one
> standard driver (for the sensor itself) could make sense.

It's hard to comment here as I only know about Lenovo implementations, 
but I wouldn't be hugely surprised if other vendors wanted to do 
similar. For now, to my knowledge, it is just a Lenovo implementation 
and the user-space consumer is a Lenovo application.

> 
> But that leads me to bigger topic: we've got several cases of some
> sensors/chips used in different subsystems, eg. simple one-shot
> ADCs, eeprom's, etc. ... maybe we should move them to separate
> subsystems, which then can be wired to other (more specific) ones
> in a very generic way ? ... just some quick+dirty thoughs,
> 
> 
> --mtx
> 

Mark

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-12 12:36                   ` Enrico Weigelt, metux IT consult
  2020-10-13  1:12                     ` Mark Pearson
@ 2020-10-13  8:38                     ` Hans de Goede
  1 sibling, 0 replies; 28+ messages in thread
From: Hans de Goede @ 2020-10-13  8:38 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, Jeff LaBundy
  Cc: Bastien Nocera, Jonathan Cameron, Mark Pearson, linux-iio,
	Nitin Joshi1, linux-input, dmitry.torokhov

Hi,

Enrico, thank you for your input.

See Mark's excellent email for answers to most of your questions,
I just have one little thing to add.

On 10/12/20 2:36 PM, Enrico Weigelt, metux IT consult wrote:
> On 08.10.20 09:10, Hans de Goede wrote:

<snip>

> Back to the technical side: IMHO we should first work out what the
> actual purpose of these sensors could be - are they useful for
> anything else than just these specific cases ? If not, I'm not
> sure whether it makes sense to put them into IIO at all, but using
> a specific board driver instead.

Right, also note that although there are doubtlessly sensors involved
we don't actually get any meaningful / direct access to these sensors.

We only get to talk to firmware which basically gives us:

Laptop is on someone's lap: yes/no
Someone is resting on the palmrest: yes/no

The lack of direct sensor access also makes this a less then
ideal case for using iio. So I believe that the suggestion
to extend the existing evdev/input SW_FRONT_PROXIMITY support
with 2 new SW_LAP_PROXIMITY and SW_PALMREST_PROXIMITY suggestion
makes a ton of sense. Switches are binary and given that this
really is a derived value and not raw sensor access using the
input system seems a better match.

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-12 12:13                     ` Hans de Goede
@ 2020-10-13 21:59                       ` Mark Pearson
  2020-10-14  4:47                         ` Jeff LaBundy
  2020-10-14  8:16                         ` Hans de Goede
  0 siblings, 2 replies; 28+ messages in thread
From: Mark Pearson @ 2020-10-13 21:59 UTC (permalink / raw)
  To: Hans de Goede, Jeff LaBundy
  Cc: Bastien Nocera, Jonathan Cameron, Nitin Joshi1, linux-input,
	dmitry.torokhov

Hi

On 2020-10-12 8:13 a.m., Hans de Goede wrote:
> Hi,
> 
> On 10/9/20 4:19 AM, Jeff LaBundy wrote:
>> Hi Hans,
>>
<snip>
>>>
>>>> I just wanted to chime in and confirm that we do have at least one
>>>> precedent for these being in input (keyboard/iqs62x-keys) and not
>>>> iio so I agree with Jonathan here. My argument is that we want to
>>>> signal binary events (user grabbed onto or let go of the handset)
>>>> rather than deliver continuous data.
>>>
>>> I was curious what keycode you are using for this, but I see
>>> that the keycodes come from devicetree, so I guess I should
>>> just ask: what keycode are you using for this ?
>>
>> The idea here was that a vendor might implement their own daemon
>> that interprets any keycode of their choice, hence leaving the
>> keycodes assignable via devicetree.
>>
>> This particular device also acts as a capacitive/inductive button
>> sensor, and these applications were the primary motivation for it
>> landing in input with its status bits mapped to keycodes.
>>
>> I don't think there are any keycodes that exist today that would
>> universally work for this application. The couple that seem most
>> closely related (e.g. KEY_WLAN or KEY_RFKILL) are typically used
>> for disabling the adapter entirely or for airplane mode (please
>> correct me if I'm wrong).
> 
> You're right (aka not wrong), KEY_WLAN and KEY_RFKILL are used to
> toggle wireless radios on/off and re-using them for some SAR
> purpose would lead to nothing but confusion. We really need to
> define some standard *new* event-codes for this, such as e.g.
> the proposed SW_LAP_PROXIMITY and SW_PALMREST_PROXIMITY.
> 
>> To that end, I'm keen to see how this interface unfolds because
>> SAR detection tends to be an available mode of operation for
>> several of the capacitive touch devices I've been working with.
> 
> I guess that for touchscreens at least (which are on the front),
> using the existing SW_FRONT_PROXIMITY would make the most sense.
> 

I've been looking at implementing this and I'm missing something - and I 
think it's probably something obvious so hoping someone can short cut me 
to the answer. Hope it's OK to do that in this thread (I removed the 
linux-iio list as I'm assuming they won't be interested)

I've added the new event codes to input-event-codes.h and updated 
mode_devicetable.h

In the thinkpad_acpi.c driver I initialise the device:

    tpacpi_sw_dev = input_allocate_device();
    if (!tpacpi_sw_dev)
            return -ENOMEM;
    tpacpi_sw_dev->name = "Thinkpad proximity switches";
    tpacpi_sw_dev->phys = TPACPI_DRVR_NAME "/input1";
    tpacpi_sw_dev->id.bustype = BUS_HOST;
    tpacpi_sw_dev->id.vendor = thinkpad_id.vendor;
    tpacpi_sw_dev->id.product = TPACPI_HKEY_INPUT_PRODUCT;
    tpacpi_sw_dev->id.version = TPACPI_HKEY_INPUT_VERSION;
    tpacpi_sw_dev->dev.parent = &tpacpi_pdev->dev;

    if (has_palmsensor) {
       input_set_capability(tpacpi_sw_dev, EV_SW, SW_PALMREST_PROXIMITY);
       input_report_switch(tpacpi_sw_dev,SW_PALMREST_PROXIMITY, 
palmsensor_state);
    }

    if (has_lapsensor) {
         input_set_capability(tpacpi_sw_dev, EV_SW, SW_LAP_PROXIMITY);
         input_report_switch(tpacpi_sw_dev, SW_LAP_PROXIMITY, 
lapsensor_state);
    }
    err = input_register_device(tpacpi_sw_dev);

If the sensor triggers I update the inputdevice with:
    input_report_switch(tpacpi_sw_dev, SW_PALMREST_PROXIMITY, new_state);
    input_sync(tpacpi_sw_dev);
<similar for lapmode>

However I'm not seeing the change when I look under evtest, though I do 
see the new sensors show up:

    [banther@localhost linux]$ sudo evtest
    No device specified, trying to scan all of /dev/input/event*
    Available devices:
    /dev/input/event0:	Sleep Button
    /dev/input/event1:	Lid Switch
    /dev/input/event2:	Power Button
    /dev/input/event3:	AT Translated Set 2 keyboard
    /dev/input/event4:	TPPS/2 Elan TrackPoint
    /dev/input/event5:	SYNA8004:00 06CB:CD8B Mouse
    /dev/input/event6:	SYNA8004:00 06CB:CD8B Touchpad
    /dev/input/event7:	Video Bus
    /dev/input/event8:	Thinkpad proximity switches
    /dev/input/event9:	PC Speaker
    /dev/input/event10:	Integrated Camera: Integrated C
    /dev/input/event11:	sof-hda-dsp Headset Jack
    /dev/input/event12:	sof-hda-dsp Mic
    /dev/input/event13:	sof-hda-dsp Headphone
    /dev/input/event14:	sof-hda-dsp HDMI/DP,pcm=3
    /dev/input/event15:	sof-hda-dsp HDMI/DP,pcm=4
    /dev/input/event16:	sof-hda-dsp HDMI/DP,pcm=5
    /dev/input/event17:	ThinkPad Extra Buttons
    Select the device event number [0-17]: 8
    Input driver version is 1.0.1
    Input device ID: bus 0x19 vendor 0x17aa product 0x5054 version 0x4101
    Input device name: "Thinkpad proximity switches"
    Supported events:
      Event type 0 (EV_SYN)
      Event type 5 (EV_SW)
        Event code 17 (?) state 0
        Event code 18 (?) state 0
    Properties:
    Testing ... (interrupt to exit)

The state for both sensors is supposed to be 1.
I did download and rebuild evtest and fixed the (?), but haven't figured 
out why the state is wrong. It seemed related to the number of keys 
which I found odd.

Any suggestions from what I'm missing, or have done wrong, or where I 
should dig next? What's the recommended way of testing my implementation?

Thanks
Mark


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-13 21:59                       ` Mark Pearson
@ 2020-10-14  4:47                         ` Jeff LaBundy
  2020-10-14  8:16                         ` Hans de Goede
  1 sibling, 0 replies; 28+ messages in thread
From: Jeff LaBundy @ 2020-10-14  4:47 UTC (permalink / raw)
  To: Mark Pearson
  Cc: Hans de Goede, Bastien Nocera, Jonathan Cameron, Nitin Joshi1,
	linux-input, dmitry.torokhov

Hi Mark,

Thank you for this additional background information about how these
types of sensors are used in a practical application.

I'll throw in my couple of debug tips below.

On Tue, Oct 13, 2020 at 05:59:18PM -0400, Mark Pearson wrote:
> Hi
> 
> On 2020-10-12 8:13 a.m., Hans de Goede wrote:
> >Hi,
> >
> >On 10/9/20 4:19 AM, Jeff LaBundy wrote:
> >>Hi Hans,
> >>
> <snip>
> >>>
> >>>>I just wanted to chime in and confirm that we do have at least one
> >>>>precedent for these being in input (keyboard/iqs62x-keys) and not
> >>>>iio so I agree with Jonathan here. My argument is that we want to
> >>>>signal binary events (user grabbed onto or let go of the handset)
> >>>>rather than deliver continuous data.
> >>>
> >>>I was curious what keycode you are using for this, but I see
> >>>that the keycodes come from devicetree, so I guess I should
> >>>just ask: what keycode are you using for this ?
> >>
> >>The idea here was that a vendor might implement their own daemon
> >>that interprets any keycode of their choice, hence leaving the
> >>keycodes assignable via devicetree.
> >>
> >>This particular device also acts as a capacitive/inductive button
> >>sensor, and these applications were the primary motivation for it
> >>landing in input with its status bits mapped to keycodes.
> >>
> >>I don't think there are any keycodes that exist today that would
> >>universally work for this application. The couple that seem most
> >>closely related (e.g. KEY_WLAN or KEY_RFKILL) are typically used
> >>for disabling the adapter entirely or for airplane mode (please
> >>correct me if I'm wrong).
> >
> >You're right (aka not wrong), KEY_WLAN and KEY_RFKILL are used to
> >toggle wireless radios on/off and re-using them for some SAR
> >purpose would lead to nothing but confusion. We really need to
> >define some standard *new* event-codes for this, such as e.g.
> >the proposed SW_LAP_PROXIMITY and SW_PALMREST_PROXIMITY.
> >
> >>To that end, I'm keen to see how this interface unfolds because
> >>SAR detection tends to be an available mode of operation for
> >>several of the capacitive touch devices I've been working with.
> >
> >I guess that for touchscreens at least (which are on the front),
> >using the existing SW_FRONT_PROXIMITY would make the most sense.
> >
> 
> I've been looking at implementing this and I'm missing something - and I
> think it's probably something obvious so hoping someone can short cut me to
> the answer. Hope it's OK to do that in this thread (I removed the linux-iio
> list as I'm assuming they won't be interested)
> 
> I've added the new event codes to input-event-codes.h and updated
> mode_devicetable.h
> 
> In the thinkpad_acpi.c driver I initialise the device:
> 
>    tpacpi_sw_dev = input_allocate_device();
>    if (!tpacpi_sw_dev)
>            return -ENOMEM;
>    tpacpi_sw_dev->name = "Thinkpad proximity switches";
>    tpacpi_sw_dev->phys = TPACPI_DRVR_NAME "/input1";
>    tpacpi_sw_dev->id.bustype = BUS_HOST;
>    tpacpi_sw_dev->id.vendor = thinkpad_id.vendor;
>    tpacpi_sw_dev->id.product = TPACPI_HKEY_INPUT_PRODUCT;
>    tpacpi_sw_dev->id.version = TPACPI_HKEY_INPUT_VERSION;
>    tpacpi_sw_dev->dev.parent = &tpacpi_pdev->dev;
> 
>    if (has_palmsensor) {
>       input_set_capability(tpacpi_sw_dev, EV_SW, SW_PALMREST_PROXIMITY);
>       input_report_switch(tpacpi_sw_dev,SW_PALMREST_PROXIMITY,
> palmsensor_state);
>    }
> 
>    if (has_lapsensor) {
>         input_set_capability(tpacpi_sw_dev, EV_SW, SW_LAP_PROXIMITY);
>         input_report_switch(tpacpi_sw_dev, SW_LAP_PROXIMITY,
> lapsensor_state);
>    }
>    err = input_register_device(tpacpi_sw_dev);
> 
> If the sensor triggers I update the inputdevice with:
>    input_report_switch(tpacpi_sw_dev, SW_PALMREST_PROXIMITY, new_state);
>    input_sync(tpacpi_sw_dev);
> <similar for lapmode>
> 
> However I'm not seeing the change when I look under evtest, though I do see
> the new sensors show up:

Have you proven that the sensor is actually signaling a change in state? I
would try printing new_state from your interrupt handler just to make sure
that the hardware is saying what you think it's saying. Maybe an interrupt
is masked within the sensor's register map, etc.

> 
>    [banther@localhost linux]$ sudo evtest
>    No device specified, trying to scan all of /dev/input/event*
>    Available devices:
>    /dev/input/event0:	Sleep Button
>    /dev/input/event1:	Lid Switch
>    /dev/input/event2:	Power Button
>    /dev/input/event3:	AT Translated Set 2 keyboard
>    /dev/input/event4:	TPPS/2 Elan TrackPoint
>    /dev/input/event5:	SYNA8004:00 06CB:CD8B Mouse
>    /dev/input/event6:	SYNA8004:00 06CB:CD8B Touchpad
>    /dev/input/event7:	Video Bus
>    /dev/input/event8:	Thinkpad proximity switches
>    /dev/input/event9:	PC Speaker
>    /dev/input/event10:	Integrated Camera: Integrated C
>    /dev/input/event11:	sof-hda-dsp Headset Jack
>    /dev/input/event12:	sof-hda-dsp Mic
>    /dev/input/event13:	sof-hda-dsp Headphone
>    /dev/input/event14:	sof-hda-dsp HDMI/DP,pcm=3
>    /dev/input/event15:	sof-hda-dsp HDMI/DP,pcm=4
>    /dev/input/event16:	sof-hda-dsp HDMI/DP,pcm=5
>    /dev/input/event17:	ThinkPad Extra Buttons
>    Select the device event number [0-17]: 8
>    Input driver version is 1.0.1
>    Input device ID: bus 0x19 vendor 0x17aa product 0x5054 version 0x4101
>    Input device name: "Thinkpad proximity switches"
>    Supported events:
>      Event type 0 (EV_SYN)
>      Event type 5 (EV_SW)
>        Event code 17 (?) state 0
>        Event code 18 (?) state 0
>    Properties:
>    Testing ... (interrupt to exit)

When you added new switch codes 0x11 and 0x12 to input-event-codes.h, did you
also increase SW_MAX to 0x12?

> 
> The state for both sensors is supposed to be 1.

I would recommend printing both palmsensor_state and lapsensor_state during
initialization to make sure the hardware is reporting what you're expecting.

> I did download and rebuild evtest and fixed the (?), but haven't figured out
> why the state is wrong. It seemed related to the number of keys which I
> found odd.

Can you clarify this observation? What changed as keys were added or removed?

> 
> Any suggestions from what I'm missing, or have done wrong, or where I should
> dig next? What's the recommended way of testing my implementation?
> 
> Thanks
> Mark
> 

Kind regards,
Jeff LaBundy

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-13 21:59                       ` Mark Pearson
  2020-10-14  4:47                         ` Jeff LaBundy
@ 2020-10-14  8:16                         ` Hans de Goede
  2020-10-14 14:26                           ` Mark Pearson
  1 sibling, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2020-10-14  8:16 UTC (permalink / raw)
  To: Mark Pearson, Jeff LaBundy
  Cc: Bastien Nocera, Jonathan Cameron, Nitin Joshi1, linux-input,
	dmitry.torokhov

Hi Mark,

On 10/13/20 11:59 PM, Mark Pearson wrote:
> Hi
> 
> On 2020-10-12 8:13 a.m., Hans de Goede wrote:
>> Hi,
>>
>> On 10/9/20 4:19 AM, Jeff LaBundy wrote:
>>> Hi Hans,
>>>
> <snip>
>>>>
>>>>> I just wanted to chime in and confirm that we do have at least one
>>>>> precedent for these being in input (keyboard/iqs62x-keys) and not
>>>>> iio so I agree with Jonathan here. My argument is that we want to
>>>>> signal binary events (user grabbed onto or let go of the handset)
>>>>> rather than deliver continuous data.
>>>>
>>>> I was curious what keycode you are using for this, but I see
>>>> that the keycodes come from devicetree, so I guess I should
>>>> just ask: what keycode are you using for this ?
>>>
>>> The idea here was that a vendor might implement their own daemon
>>> that interprets any keycode of their choice, hence leaving the
>>> keycodes assignable via devicetree.
>>>
>>> This particular device also acts as a capacitive/inductive button
>>> sensor, and these applications were the primary motivation for it
>>> landing in input with its status bits mapped to keycodes.
>>>
>>> I don't think there are any keycodes that exist today that would
>>> universally work for this application. The couple that seem most
>>> closely related (e.g. KEY_WLAN or KEY_RFKILL) are typically used
>>> for disabling the adapter entirely or for airplane mode (please
>>> correct me if I'm wrong).
>>
>> You're right (aka not wrong), KEY_WLAN and KEY_RFKILL are used to
>> toggle wireless radios on/off and re-using them for some SAR
>> purpose would lead to nothing but confusion. We really need to
>> define some standard *new* event-codes for this, such as e.g.
>> the proposed SW_LAP_PROXIMITY and SW_PALMREST_PROXIMITY.
>>
>>> To that end, I'm keen to see how this interface unfolds because
>>> SAR detection tends to be an available mode of operation for
>>> several of the capacitive touch devices I've been working with.
>>
>> I guess that for touchscreens at least (which are on the front),
>> using the existing SW_FRONT_PROXIMITY would make the most sense.
>>
> 
> I've been looking at implementing this and I'm missing something - and I think it's probably something obvious so hoping someone can short cut me to the answer. Hope it's OK to do that in this thread (I removed the linux-iio list as I'm assuming they won't be interested)
> 
> I've added the new event codes to input-event-codes.h and updated mode_devicetable.h
> 
> In the thinkpad_acpi.c driver I initialise the device:
> 
>     tpacpi_sw_dev = input_allocate_device();
>     if (!tpacpi_sw_dev)
>             return -ENOMEM;
>     tpacpi_sw_dev->name = "Thinkpad proximity switches";
>     tpacpi_sw_dev->phys = TPACPI_DRVR_NAME "/input1";
>     tpacpi_sw_dev->id.bustype = BUS_HOST;
>     tpacpi_sw_dev->id.vendor = thinkpad_id.vendor;
>     tpacpi_sw_dev->id.product = TPACPI_HKEY_INPUT_PRODUCT;
>     tpacpi_sw_dev->id.version = TPACPI_HKEY_INPUT_VERSION;
>     tpacpi_sw_dev->dev.parent = &tpacpi_pdev->dev;
> 
>     if (has_palmsensor) {
>        input_set_capability(tpacpi_sw_dev, EV_SW, SW_PALMREST_PROXIMITY);
>        input_report_switch(tpacpi_sw_dev,SW_PALMREST_PROXIMITY, palmsensor_state);
>     }
> 
>     if (has_lapsensor) {
>          input_set_capability(tpacpi_sw_dev, EV_SW, SW_LAP_PROXIMITY);
>          input_report_switch(tpacpi_sw_dev, SW_LAP_PROXIMITY, lapsensor_state);
>     }
>     err = input_register_device(tpacpi_sw_dev);
> 
> If the sensor triggers I update the inputdevice with:
>     input_report_switch(tpacpi_sw_dev, SW_PALMREST_PROXIMITY, new_state);
>     input_sync(tpacpi_sw_dev);
> <similar for lapmode>
> 
> However I'm not seeing the change when I look under evtest, though I do see the new sensors show up:
> 
>     [banther@localhost linux]$ sudo evtest
>     No device specified, trying to scan all of /dev/input/event*
>     Available devices:
>     /dev/input/event0:    Sleep Button
>     /dev/input/event1:    Lid Switch
>     /dev/input/event2:    Power Button
>     /dev/input/event3:    AT Translated Set 2 keyboard
>     /dev/input/event4:    TPPS/2 Elan TrackPoint
>     /dev/input/event5:    SYNA8004:00 06CB:CD8B Mouse
>     /dev/input/event6:    SYNA8004:00 06CB:CD8B Touchpad
>     /dev/input/event7:    Video Bus
>     /dev/input/event8:    Thinkpad proximity switches
>     /dev/input/event9:    PC Speaker
>     /dev/input/event10:    Integrated Camera: Integrated C
>     /dev/input/event11:    sof-hda-dsp Headset Jack
>     /dev/input/event12:    sof-hda-dsp Mic
>     /dev/input/event13:    sof-hda-dsp Headphone
>     /dev/input/event14:    sof-hda-dsp HDMI/DP,pcm=3
>     /dev/input/event15:    sof-hda-dsp HDMI/DP,pcm=4
>     /dev/input/event16:    sof-hda-dsp HDMI/DP,pcm=5
>     /dev/input/event17:    ThinkPad Extra Buttons
>     Select the device event number [0-17]: 8
>     Input driver version is 1.0.1
>     Input device ID: bus 0x19 vendor 0x17aa product 0x5054 version 0x4101
>     Input device name: "Thinkpad proximity switches"
>     Supported events:
>       Event type 0 (EV_SYN)
>       Event type 5 (EV_SW)
>         Event code 17 (?) state 0
>         Event code 18 (?) state 0
>     Properties:
>     Testing ... (interrupt to exit)
> 
> The state for both sensors is supposed to be 1.
> I did download and rebuild evtest and fixed the (?), but haven't figured out why the state is wrong. It seemed related to the number of keys which I found odd.
> 
> Any suggestions from what I'm missing, or have done wrong, or where I should dig next? What's the recommended way of testing my implementation?

A couple suggestions:

1. What Jeff said, add a printf to make sure that the input_report_switch + sync actually get called.
2. Did you rebuild your entire kernel after adding the new SW_ definitions to input-event-codes.h ?  The core may very well be using a check for SW_MAX / SW_CNT
3. Did you rebuild the evtest tool against the latest headers, including an updated SW_MAX / SW_CNT.

To rule out issues with evtest still using an old SW_MAX somewhere you could (as a hack) replace the 2 new SW_... codes with 2 existing ones and see if that makes things work.

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-14  8:16                         ` Hans de Goede
@ 2020-10-14 14:26                           ` Mark Pearson
  0 siblings, 0 replies; 28+ messages in thread
From: Mark Pearson @ 2020-10-14 14:26 UTC (permalink / raw)
  To: Hans de Goede, Jeff LaBundy
  Cc: Bastien Nocera, Jonathan Cameron, Nitin Joshi1, linux-input,
	dmitry.torokhov



On 2020-10-14 4:16 a.m., Hans de Goede wrote:
> Hi Mark,
> 
> On 10/13/20 11:59 PM, Mark Pearson wrote:
>> Hi
>>
>> On 2020-10-12 8:13 a.m., Hans de Goede wrote:
>>> Hi,
>>>
>>> On 10/9/20 4:19 AM, Jeff LaBundy wrote:
>>>> Hi Hans,
>>>>
>> <snip>
>>>>>
>>>>>> I just wanted to chime in and confirm that we do have at least one
>>>>>> precedent for these being in input (keyboard/iqs62x-keys) and not
>>>>>> iio so I agree with Jonathan here. My argument is that we want to
>>>>>> signal binary events (user grabbed onto or let go of the handset)
>>>>>> rather than deliver continuous data.
>>>>>
>>>>> I was curious what keycode you are using for this, but I see
>>>>> that the keycodes come from devicetree, so I guess I should
>>>>> just ask: what keycode are you using for this ?
>>>>
>>>> The idea here was that a vendor might implement their own daemon
>>>> that interprets any keycode of their choice, hence leaving the
>>>> keycodes assignable via devicetree.
>>>>
>>>> This particular device also acts as a capacitive/inductive button
>>>> sensor, and these applications were the primary motivation for it
>>>> landing in input with its status bits mapped to keycodes.
>>>>
>>>> I don't think there are any keycodes that exist today that would
>>>> universally work for this application. The couple that seem most
>>>> closely related (e.g. KEY_WLAN or KEY_RFKILL) are typically used
>>>> for disabling the adapter entirely or for airplane mode (please
>>>> correct me if I'm wrong).
>>>
>>> You're right (aka not wrong), KEY_WLAN and KEY_RFKILL are used to
>>> toggle wireless radios on/off and re-using them for some SAR
>>> purpose would lead to nothing but confusion. We really need to
>>> define some standard *new* event-codes for this, such as e.g.
>>> the proposed SW_LAP_PROXIMITY and SW_PALMREST_PROXIMITY.
>>>
>>>> To that end, I'm keen to see how this interface unfolds because
>>>> SAR detection tends to be an available mode of operation for
>>>> several of the capacitive touch devices I've been working with.
>>>
>>> I guess that for touchscreens at least (which are on the front),
>>> using the existing SW_FRONT_PROXIMITY would make the most sense.
>>>
>>
>> I've been looking at implementing this and I'm missing something - and 
>> I think it's probably something obvious so hoping someone can short 
>> cut me to the answer. Hope it's OK to do that in this thread (I 
>> removed the linux-iio list as I'm assuming they won't be interested)
>>
>> I've added the new event codes to input-event-codes.h and updated 
>> mode_devicetable.h
>>
>> In the thinkpad_acpi.c driver I initialise the device:
>>
>>     tpacpi_sw_dev = input_allocate_device();
>>     if (!tpacpi_sw_dev)
>>             return -ENOMEM;
>>     tpacpi_sw_dev->name = "Thinkpad proximity switches";
>>     tpacpi_sw_dev->phys = TPACPI_DRVR_NAME "/input1";
>>     tpacpi_sw_dev->id.bustype = BUS_HOST;
>>     tpacpi_sw_dev->id.vendor = thinkpad_id.vendor;
>>     tpacpi_sw_dev->id.product = TPACPI_HKEY_INPUT_PRODUCT;
>>     tpacpi_sw_dev->id.version = TPACPI_HKEY_INPUT_VERSION;
>>     tpacpi_sw_dev->dev.parent = &tpacpi_pdev->dev;
>>
>>     if (has_palmsensor) {
>>        input_set_capability(tpacpi_sw_dev, EV_SW, SW_PALMREST_PROXIMITY);
>>        input_report_switch(tpacpi_sw_dev,SW_PALMREST_PROXIMITY, 
>> palmsensor_state);
>>     }
>>
>>     if (has_lapsensor) {
>>          input_set_capability(tpacpi_sw_dev, EV_SW, SW_LAP_PROXIMITY);
>>          input_report_switch(tpacpi_sw_dev, SW_LAP_PROXIMITY, 
>> lapsensor_state);
>>     }
>>     err = input_register_device(tpacpi_sw_dev);
>>
>> If the sensor triggers I update the inputdevice with:
>>     input_report_switch(tpacpi_sw_dev, SW_PALMREST_PROXIMITY, new_state);
>>     input_sync(tpacpi_sw_dev);
>> <similar for lapmode>
>>
>> However I'm not seeing the change when I look under evtest, though I 
>> do see the new sensors show up:
>>
>>     [banther@localhost linux]$ sudo evtest
>>     No device specified, trying to scan all of /dev/input/event*
>>     Available devices:
>>     /dev/input/event0:    Sleep Button
>>     /dev/input/event1:    Lid Switch
>>     /dev/input/event2:    Power Button
>>     /dev/input/event3:    AT Translated Set 2 keyboard
>>     /dev/input/event4:    TPPS/2 Elan TrackPoint
>>     /dev/input/event5:    SYNA8004:00 06CB:CD8B Mouse
>>     /dev/input/event6:    SYNA8004:00 06CB:CD8B Touchpad
>>     /dev/input/event7:    Video Bus
>>     /dev/input/event8:    Thinkpad proximity switches
>>     /dev/input/event9:    PC Speaker
>>     /dev/input/event10:    Integrated Camera: Integrated C
>>     /dev/input/event11:    sof-hda-dsp Headset Jack
>>     /dev/input/event12:    sof-hda-dsp Mic
>>     /dev/input/event13:    sof-hda-dsp Headphone
>>     /dev/input/event14:    sof-hda-dsp HDMI/DP,pcm=3
>>     /dev/input/event15:    sof-hda-dsp HDMI/DP,pcm=4
>>     /dev/input/event16:    sof-hda-dsp HDMI/DP,pcm=5
>>     /dev/input/event17:    ThinkPad Extra Buttons
>>     Select the device event number [0-17]: 8
>>     Input driver version is 1.0.1
>>     Input device ID: bus 0x19 vendor 0x17aa product 0x5054 version 0x4101
>>     Input device name: "Thinkpad proximity switches"
>>     Supported events:
>>       Event type 0 (EV_SYN)
>>       Event type 5 (EV_SW)
>>         Event code 17 (?) state 0
>>         Event code 18 (?) state 0
>>     Properties:
>>     Testing ... (interrupt to exit)
>>
>> The state for both sensors is supposed to be 1.
>> I did download and rebuild evtest and fixed the (?), but haven't 
>> figured out why the state is wrong. It seemed related to the number of 
>> keys which I found odd.
>>
>> Any suggestions from what I'm missing, or have done wrong, or where I 
>> should dig next? What's the recommended way of testing my implementation?
> 
> A couple suggestions:
> 
> 1. What Jeff said, add a printf to make sure that the 
> input_report_switch + sync actually get called.
> 2. Did you rebuild your entire kernel after adding the new SW_ 
> definitions to input-event-codes.h ?  The core may very well be using a 
> check for SW_MAX / SW_CNT
> 3. Did you rebuild the evtest tool against the latest headers, including 
> an updated SW_MAX / SW_CNT.
> 
> To rule out issues with evtest still using an old SW_MAX somewhere you 
> could (as a hack) replace the 2 new SW_... codes with 2 existing ones 
> and see if that makes things work.
> 

Looks like rebuilding the entire kernel did the trick - guess I needed 
to update the input driver (which now I write that does seem 
obvious....sigh)

I'm seeing the events show up in evtest which is very cool

Thanks to all for the comments and help - much appreciated. I'll work on 
getting a patch out for this soon for review

Mark

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-10-07  9:51     ` Hans de Goede
  2020-10-07 11:35       ` Bastien Nocera
@ 2020-11-12  6:23       ` Dmitry Torokhov
  2020-11-12  9:50         ` Hans de Goede
  2020-11-19 15:16         ` Bastien Nocera
  1 sibling, 2 replies; 28+ messages in thread
From: Dmitry Torokhov @ 2020-11-12  6:23 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Jonathan Cameron, Mark Pearson, linux-iio, Bastien Nocera,
	Nitin Joshi1, linux-input

On Wed, Oct 07, 2020 at 11:51:05AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10/7/20 10:36 AM, Jonathan Cameron wrote:
> > On Mon, 5 Oct 2020 22:04:27 -0400
> > Mark Pearson <markpearson@lenovo.com> wrote:
> > 
> > > Adding Nitin, lead for this feature, to the thread
> > 
> > +CC linux-input and Dmitry for reasons that will become clear below.
> > > 
> > > On 2020-10-03 10:02 a.m., Hans de Goede wrote:
> > > > Hi All,
> > > > 
> > > > Modern laptops can have various sensors which are kinda
> > > > like proximity sensors, but not really (they are more
> > > > specific in which part of the laptop the user is
> > > > proximate to).
> > > > 
> > > > Specifically modern Thinkpad's have 2 readings which we
> > > > want to export to userspace, and I'm wondering if we
> > > > could use the IIO framework for this since these readings
> > > > are in essence sensor readings:
> > > > 
> > > > 1. These laptops have a sensor in the palm-rests to
> > > > check if a user is physically proximate to the device's
> > > > palm-rests. This info will be used by userspace for WWAN
> > > > functionality to control the transmission level safely.
> > > > 
> > > > A patch adding a thinkpad_acpi specific sysfs API for this
> > > > is currently pending:
> > > > https://patchwork.kernel.org/patch/11722127/
> > > > 
> > > > But I'm wondering if it would not be better to use
> > > > IIO to export this info.
> > 
> > My first thought on this is it sounds more like a key than a sensor
> > (simple proximity sensors fall into this category as well.)

[ sorry for sitting on this thread for so long ]

So I think the important question here is if we only ever want yes/no
answer, or if we can consider adjusting behavior of the system based on
the "closeness" of an object to the device, in which case I think IIO is
more flexible.

FWIW in Chrome OS land we name IIO proximity sensors using a scheme
"proximity-lte", "proximity-wifi", "proximity-wifi-left",
"proximity-wifi-right", etc, and then userspace implements various
policies (SAR, etc) based off it.

> 
> That is an interesting suggestion. Using the input/evdev API
> would have some advantages such as being able to have a single
> event node for all the proximity switches and then being able
> to pass a fd to that from a privileged process to a non
> privileged one, something which userspace already has
> various infrastructure for.

I am not sure if multiplexing all proximity switches into one evdev node
is that great option, as I am sure we'll soon have devices with 2x
palmrest switches and being capable finely adjusting transmit power,
etc.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-12  6:23       ` Dmitry Torokhov
@ 2020-11-12  9:50         ` Hans de Goede
  2020-11-13  6:58           ` Dmitry Torokhov
  2020-11-19 15:16         ` Bastien Nocera
  1 sibling, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2020-11-12  9:50 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jonathan Cameron, Mark Pearson, linux-iio, Bastien Nocera,
	Nitin Joshi1, linux-input

Hi,

On 11/12/20 7:23 AM, Dmitry Torokhov wrote:
> On Wed, Oct 07, 2020 at 11:51:05AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 10/7/20 10:36 AM, Jonathan Cameron wrote:
>>> On Mon, 5 Oct 2020 22:04:27 -0400
>>> Mark Pearson <markpearson@lenovo.com> wrote:
>>>
>>>> Adding Nitin, lead for this feature, to the thread
>>>
>>> +CC linux-input and Dmitry for reasons that will become clear below.
>>>>
>>>> On 2020-10-03 10:02 a.m., Hans de Goede wrote:
>>>>> Hi All,
>>>>>
>>>>> Modern laptops can have various sensors which are kinda
>>>>> like proximity sensors, but not really (they are more
>>>>> specific in which part of the laptop the user is
>>>>> proximate to).
>>>>>
>>>>> Specifically modern Thinkpad's have 2 readings which we
>>>>> want to export to userspace, and I'm wondering if we
>>>>> could use the IIO framework for this since these readings
>>>>> are in essence sensor readings:
>>>>>
>>>>> 1. These laptops have a sensor in the palm-rests to
>>>>> check if a user is physically proximate to the device's
>>>>> palm-rests. This info will be used by userspace for WWAN
>>>>> functionality to control the transmission level safely.
>>>>>
>>>>> A patch adding a thinkpad_acpi specific sysfs API for this
>>>>> is currently pending:
>>>>> https://patchwork.kernel.org/patch/11722127/
>>>>>
>>>>> But I'm wondering if it would not be better to use
>>>>> IIO to export this info.
>>>
>>> My first thought on this is it sounds more like a key than a sensor
>>> (simple proximity sensors fall into this category as well.)
> 
> [ sorry for sitting on this thread for so long ]
> 
> So I think the important question here is if we only ever want yes/no
> answer, or if we can consider adjusting behavior of the system based on
> the "closeness" of an object to the device, in which case I think IIO is
> more flexible.
> 
> FWIW in Chrome OS land we name IIO proximity sensors using a scheme
> "proximity-lte", "proximity-wifi", "proximity-wifi-left",
> "proximity-wifi-right", etc, and then userspace implements various
> policies (SAR, etc) based off it.

Interesting, so 2 questions:

1. So your encoding the location in the sensor's parent-device name
instead of using a new sysfs attribute for this ?

2. Do these sensors just give a boolean value atm, or do they already
report a range ?  IIRC one of the objections from the iio folks in
the Lenovo case was that booleans are not really a good fit for iio
(IIRC they also said we could still use iio for this).

Perhaps you can provide an URL to the kernel code implementing these ?

>> That is an interesting suggestion. Using the input/evdev API
>> would have some advantages such as being able to have a single
>> event node for all the proximity switches and then being able
>> to pass a fd to that from a privileged process to a non
>> privileged one, something which userspace already has
>> various infrastructure for.
> 
> I am not sure if multiplexing all proximity switches into one evdev node
> is that great option, as I am sure we'll soon have devices with 2x
> palmrest switches and being capable finely adjusting transmit power,
> etc.

Right, so going with iio, together with a naming scheme like used
on ChromeOS might indeed be more future proof (and make things
easier for running ChromeOS on non ChromeOS hardware and the other
way around).

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-12  9:50         ` Hans de Goede
@ 2020-11-13  6:58           ` Dmitry Torokhov
  2020-11-19 15:39             ` Hans de Goede
  0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Torokhov @ 2020-11-13  6:58 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Jonathan Cameron, Mark Pearson, linux-iio, Bastien Nocera,
	Nitin Joshi1, linux-input

On Thu, Nov 12, 2020 at 10:50:12AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 11/12/20 7:23 AM, Dmitry Torokhov wrote:
> > On Wed, Oct 07, 2020 at 11:51:05AM +0200, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 10/7/20 10:36 AM, Jonathan Cameron wrote:
> >>> On Mon, 5 Oct 2020 22:04:27 -0400
> >>> Mark Pearson <markpearson@lenovo.com> wrote:
> >>>
> >>>> Adding Nitin, lead for this feature, to the thread
> >>>
> >>> +CC linux-input and Dmitry for reasons that will become clear below.
> >>>>
> >>>> On 2020-10-03 10:02 a.m., Hans de Goede wrote:
> >>>>> Hi All,
> >>>>>
> >>>>> Modern laptops can have various sensors which are kinda
> >>>>> like proximity sensors, but not really (they are more
> >>>>> specific in which part of the laptop the user is
> >>>>> proximate to).
> >>>>>
> >>>>> Specifically modern Thinkpad's have 2 readings which we
> >>>>> want to export to userspace, and I'm wondering if we
> >>>>> could use the IIO framework for this since these readings
> >>>>> are in essence sensor readings:
> >>>>>
> >>>>> 1. These laptops have a sensor in the palm-rests to
> >>>>> check if a user is physically proximate to the device's
> >>>>> palm-rests. This info will be used by userspace for WWAN
> >>>>> functionality to control the transmission level safely.
> >>>>>
> >>>>> A patch adding a thinkpad_acpi specific sysfs API for this
> >>>>> is currently pending:
> >>>>> https://patchwork.kernel.org/patch/11722127/
> >>>>>
> >>>>> But I'm wondering if it would not be better to use
> >>>>> IIO to export this info.
> >>>
> >>> My first thought on this is it sounds more like a key than a sensor
> >>> (simple proximity sensors fall into this category as well.)
> > 
> > [ sorry for sitting on this thread for so long ]
> > 
> > So I think the important question here is if we only ever want yes/no
> > answer, or if we can consider adjusting behavior of the system based on
> > the "closeness" of an object to the device, in which case I think IIO is
> > more flexible.
> > 
> > FWIW in Chrome OS land we name IIO proximity sensors using a scheme
> > "proximity-lte", "proximity-wifi", "proximity-wifi-left",
> > "proximity-wifi-right", etc, and then userspace implements various
> > policies (SAR, etc) based off it.
> 
> Interesting, so 2 questions:
> 
> 1. So your encoding the location in the sensor's parent-device name
> instead of using a new sysfs attribute for this ?

I think it depends on the kernel we use and architecture. On x86 I think
we rely on udev, like this:

https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-nocturne/chromeos-base/chromeos-bsp-nocturne/files/udev/99-cros-sx-proximity.rules

DEVPATH=="*/pci0000:00/0000:00:15.1/*", SYMLINK+="proximity-wifi-right"
DEVPATH=="*/pci0000:00/0000:00:19.1/*", SYMLINK+="proximity-wifi-left"
ATTR{events/in_proximity1_USE_CS1_thresh_either_en}="1"

On newer ARM we use "label" attribute in DTS:

arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi

        ap_sar_sensor: proximity@28 {
                compatible = "semtech,sx9310";
                reg = <0x28>;
                #io-channel-cells = <1>;
                pinctrl-names = "default";
                pinctrl-0 = <&p_sensor_int_l>;

                interrupt-parent = <&tlmm>;
                interrupts = <24 IRQ_TYPE_LEVEL_LOW>;

                vdd-supply = <&pp3300_a>;
                svdd-supply = <&pp1800_prox>;

                status = "disabled";
                label = "proximity-wifi";
        };

> 
> 2. Do these sensors just give a boolean value atm, or do they already
> report a range ?  IIRC one of the objections from the iio folks in
> the Lenovo case was that booleans are not really a good fit for iio
> (IIRC they also said we could still use iio for this).

One of the sensors we use is sx9310 that I believe can report range, but
I think we configure them to trigger when a threshold is crossed.

Events are handled by our powerd:

https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/powerd/system/sar_watcher.cc

> 
> Perhaps you can provide an URL to the kernel code implementing these ?

drivers/iio/proximity/sx9310.c

Also sx932x - https://lore.kernel.org/patchwork/patch/1005708/

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-12  6:23       ` Dmitry Torokhov
  2020-11-12  9:50         ` Hans de Goede
@ 2020-11-19 15:16         ` Bastien Nocera
  2020-11-19 15:24           ` Hans de Goede
  1 sibling, 1 reply; 28+ messages in thread
From: Bastien Nocera @ 2020-11-19 15:16 UTC (permalink / raw)
  To: Dmitry Torokhov, Hans de Goede
  Cc: Jonathan Cameron, Mark Pearson, linux-iio, Nitin Joshi1, linux-input

On Wed, 2020-11-11 at 22:23 -0800, Dmitry Torokhov wrote:
> <snip>
> I am not sure if multiplexing all proximity switches into one evdev
> node
> is that great option, as I am sure we'll soon have devices with 2x
> palmrest switches and being capable finely adjusting transmit power,
> etc.

Hans, Mark, so is there a consensus to how we should export the "lap-
mode"?

I had nearly finished working on updated code and all the test suite
changes needed to use an input device with switches when IIO started
being discussed, so I stopped in my tracks.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-19 15:16         ` Bastien Nocera
@ 2020-11-19 15:24           ` Hans de Goede
  2020-11-19 15:58             ` Bastien Nocera
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2020-11-19 15:24 UTC (permalink / raw)
  To: Bastien Nocera, Dmitry Torokhov
  Cc: Jonathan Cameron, Mark Pearson, linux-iio, Nitin Joshi1, linux-input

Hi,

On 11/19/20 4:16 PM, Bastien Nocera wrote:
> On Wed, 2020-11-11 at 22:23 -0800, Dmitry Torokhov wrote:
>> <snip>
>> I am not sure if multiplexing all proximity switches into one evdev
>> node
>> is that great option, as I am sure we'll soon have devices with 2x
>> palmrest switches and being capable finely adjusting transmit power,
>> etc.
> 
> Hans, Mark, so is there a consensus to how we should export the "lap-
> mode"?

Given Dmitry's input itl ooks like we need to go back to using iio
for this. Probably with something like my initial proposal wherre we
add an in_proximity_location sysfs attribute to the iio-devices which
represent the lap-mode and palmrest sensors. But ChromeOS is doing
something different to figure out which sensor is which, so this needs
a bit more discussion.

I'll go and reply to Dmitry's latest mail on this now and then we will
see from there.

> I had nearly finished working on updated code and all the test suite
> changes needed to use an input device with switches when IIO started
> being discussed, so I stopped in my tracks.

Ouch, sorry about this.

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-13  6:58           ` Dmitry Torokhov
@ 2020-11-19 15:39             ` Hans de Goede
  2020-11-19 16:11               ` Bastien Nocera
  2020-11-20  9:59               ` Jonathan Cameron
  0 siblings, 2 replies; 28+ messages in thread
From: Hans de Goede @ 2020-11-19 15:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jonathan Cameron, Mark Pearson, linux-iio, Bastien Nocera,
	Nitin Joshi1, linux-input

Hi,

On 11/13/20 7:58 AM, Dmitry Torokhov wrote:
> On Thu, Nov 12, 2020 at 10:50:12AM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 11/12/20 7:23 AM, Dmitry Torokhov wrote:
>>> On Wed, Oct 07, 2020 at 11:51:05AM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 10/7/20 10:36 AM, Jonathan Cameron wrote:
>>>>> On Mon, 5 Oct 2020 22:04:27 -0400
>>>>> Mark Pearson <markpearson@lenovo.com> wrote:
>>>>>
>>>>>> Adding Nitin, lead for this feature, to the thread
>>>>>
>>>>> +CC linux-input and Dmitry for reasons that will become clear below.
>>>>>>
>>>>>> On 2020-10-03 10:02 a.m., Hans de Goede wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Modern laptops can have various sensors which are kinda
>>>>>>> like proximity sensors, but not really (they are more
>>>>>>> specific in which part of the laptop the user is
>>>>>>> proximate to).
>>>>>>>
>>>>>>> Specifically modern Thinkpad's have 2 readings which we
>>>>>>> want to export to userspace, and I'm wondering if we
>>>>>>> could use the IIO framework for this since these readings
>>>>>>> are in essence sensor readings:
>>>>>>>
>>>>>>> 1. These laptops have a sensor in the palm-rests to
>>>>>>> check if a user is physically proximate to the device's
>>>>>>> palm-rests. This info will be used by userspace for WWAN
>>>>>>> functionality to control the transmission level safely.
>>>>>>>
>>>>>>> A patch adding a thinkpad_acpi specific sysfs API for this
>>>>>>> is currently pending:
>>>>>>> https://patchwork.kernel.org/patch/11722127/
>>>>>>>
>>>>>>> But I'm wondering if it would not be better to use
>>>>>>> IIO to export this info.
>>>>>
>>>>> My first thought on this is it sounds more like a key than a sensor
>>>>> (simple proximity sensors fall into this category as well.)
>>>
>>> [ sorry for sitting on this thread for so long ]
>>>
>>> So I think the important question here is if we only ever want yes/no
>>> answer, or if we can consider adjusting behavior of the system based on
>>> the "closeness" of an object to the device, in which case I think IIO is
>>> more flexible.
>>>
>>> FWIW in Chrome OS land we name IIO proximity sensors using a scheme
>>> "proximity-lte", "proximity-wifi", "proximity-wifi-left",
>>> "proximity-wifi-right", etc, and then userspace implements various
>>> policies (SAR, etc) based off it.
>>
>> Interesting, so 2 questions:
>>
>> 1. So your encoding the location in the sensor's parent-device name
>> instead of using a new sysfs attribute for this ?
> 
> I think it depends on the kernel we use and architecture. On x86 I think
> we rely on udev, like this:
> 
> https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-nocturne/chromeos-base/chromeos-bsp-nocturne/files/udev/99-cros-sx-proximity.rules
> 
> DEVPATH=="*/pci0000:00/0000:00:15.1/*", SYMLINK+="proximity-wifi-right"
> DEVPATH=="*/pci0000:00/0000:00:19.1/*", SYMLINK+="proximity-wifi-left"
> ATTR{events/in_proximity1_USE_CS1_thresh_either_en}="1"

So that results in a symlink under /dev, right ? That seems like
it is not really compatible with how most modern userspace discovers
hw (through udev). Although I guess code using udev could still
lookup the symlink in the udev per device data, this just not feel
like a good way forward.

> On newer ARM we use "label" attribute in DTS:
> 
> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> 
>         ap_sar_sensor: proximity@28 {
>                 compatible = "semtech,sx9310";
>                 reg = <0x28>;
>                 #io-channel-cells = <1>;
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&p_sensor_int_l>;
> 
>                 interrupt-parent = <&tlmm>;
>                 interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
> 
>                 vdd-supply = <&pp3300_a>;
>                 svdd-supply = <&pp1800_prox>;
> 
>                 status = "disabled";
>                 label = "proximity-wifi";
>         };

Hmm, interesting. I did not know iio-devices could
have a label sysfs attribute (nor that that could be
set through device-tree). I was thinking about adding
an in_proximity_location sysfs attribute. But using
labels (and standardizing a set of label names) will
work nicely too.

I have no real preference for this either way, so
I guess we might as well go with labels to avoid
having any unnecessary discrepancies between ChromeOS
and whatever we do for the Thinkpad sensors.

Is there a know set of labels which ChromeOS is currently
using? If we are going to use labels for this it would
be good IMHO to define a set of standard labels for
this in say Documentation/ABI/testing/sysfs-bus-iio-labels.

>> 2. Do these sensors just give a boolean value atm, or do they already
>> report a range ?  IIRC one of the objections from the iio folks in
>> the Lenovo case was that booleans are not really a good fit for iio
>> (IIRC they also said we could still use iio for this).
> 
> One of the sensors we use is sx9310 that I believe can report range, but
> I think we configure them to trigger when a threshold is crossed.
> 
> Events are handled by our powerd:
> 
> https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/powerd/system/sar_watcher.cc
> 
>>
>> Perhaps you can provide an URL to the kernel code implementing these ?
> 
> drivers/iio/proximity/sx9310.c

If I'm reading that correctly the it exports a raw "distance"
reading and a suggested threshold value for the code interpreting
the reading to use.

So that would be a bit different then the Thinkpad sensors, but
exporting just a 0-1 range for the in_proximity_raw value for the
Thinkpad case should not be a problem. Or we could just make it
repot 0 and 100 and export a fixed in_proximity_nearlevel of 50,
that would make the userspace API more like other proximity sensors.

Regards,

Hans





^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-19 15:24           ` Hans de Goede
@ 2020-11-19 15:58             ` Bastien Nocera
  0 siblings, 0 replies; 28+ messages in thread
From: Bastien Nocera @ 2020-11-19 15:58 UTC (permalink / raw)
  To: Hans de Goede, Dmitry Torokhov
  Cc: Jonathan Cameron, Mark Pearson, linux-iio, Nitin Joshi1, linux-input

On Thu, 2020-11-19 at 16:24 +0100, Hans de Goede wrote:
> <snip>
> > I had nearly finished working on updated code and all the test
> > suite
> > changes needed to use an input device with switches when IIO
> > started
> > being discussed, so I stopped in my tracks.
> 
> Ouch, sorry about this.

It happens, and the biggest problem I had was generating mock events
through umockdev.

Looks like I might have to finally implement:
https://github.com/martinpitt/umockdev/issues/43
which has the benefit of also helping me with iio-sensor-proxy.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-19 15:39             ` Hans de Goede
@ 2020-11-19 16:11               ` Bastien Nocera
  2020-11-20  9:59               ` Jonathan Cameron
  1 sibling, 0 replies; 28+ messages in thread
From: Bastien Nocera @ 2020-11-19 16:11 UTC (permalink / raw)
  To: Hans de Goede, Dmitry Torokhov
  Cc: Jonathan Cameron, Mark Pearson, linux-iio, Nitin Joshi1, linux-input

On Thu, 2020-11-19 at 16:39 +0100, Hans de Goede wrote:
> Hi,
> 
> On 11/13/20 7:58 AM, Dmitry Torokhov wrote:
> > On Thu, Nov 12, 2020 at 10:50:12AM +0100, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 11/12/20 7:23 AM, Dmitry Torokhov wrote:
> > > > On Wed, Oct 07, 2020 at 11:51:05AM +0200, Hans de Goede wrote:
> > > > > Hi,
> > > > > 
> > > > > On 10/7/20 10:36 AM, Jonathan Cameron wrote:
> > > > > > On Mon, 5 Oct 2020 22:04:27 -0400
> > > > > > Mark Pearson <markpearson@lenovo.com> wrote:
> > > > > > 
> > > > > > > Adding Nitin, lead for this feature, to the thread
> > > > > > 
> > > > > > +CC linux-input and Dmitry for reasons that will become
> > > > > > clear below.
> > > > > > > 
> > > > > > > On 2020-10-03 10:02 a.m., Hans de Goede wrote:
> > > > > > > > Hi All,
> > > > > > > > 
> > > > > > > > Modern laptops can have various sensors which are kinda
> > > > > > > > like proximity sensors, but not really (they are more
> > > > > > > > specific in which part of the laptop the user is
> > > > > > > > proximate to).
> > > > > > > > 
> > > > > > > > Specifically modern Thinkpad's have 2 readings which we
> > > > > > > > want to export to userspace, and I'm wondering if we
> > > > > > > > could use the IIO framework for this since these
> > > > > > > > readings
> > > > > > > > are in essence sensor readings:
> > > > > > > > 
> > > > > > > > 1. These laptops have a sensor in the palm-rests to
> > > > > > > > check if a user is physically proximate to the device's
> > > > > > > > palm-rests. This info will be used by userspace for
> > > > > > > > WWAN
> > > > > > > > functionality to control the transmission level safely.
> > > > > > > > 
> > > > > > > > A patch adding a thinkpad_acpi specific sysfs API for
> > > > > > > > this
> > > > > > > > is currently pending:
> > > > > > > > https://patchwork.kernel.org/patch/11722127/
> > > > > > > > 
> > > > > > > > But I'm wondering if it would not be better to use
> > > > > > > > IIO to export this info.
> > > > > > 
> > > > > > My first thought on this is it sounds more like a key than
> > > > > > a sensor
> > > > > > (simple proximity sensors fall into this category as well.)
> > > > 
> > > > [ sorry for sitting on this thread for so long ]
> > > > 
> > > > So I think the important question here is if we only ever want
> > > > yes/no
> > > > answer, or if we can consider adjusting behavior of the system
> > > > based on
> > > > the "closeness" of an object to the device, in which case I
> > > > think IIO is
> > > > more flexible.
> > > > 
> > > > FWIW in Chrome OS land we name IIO proximity sensors using a
> > > > scheme
> > > > "proximity-lte", "proximity-wifi", "proximity-wifi-left",
> > > > "proximity-wifi-right", etc, and then userspace implements
> > > > various
> > > > policies (SAR, etc) based off it.
> > > 
> > > Interesting, so 2 questions:
> > > 
> > > 1. So your encoding the location in the sensor's parent-device
> > > name
> > > instead of using a new sysfs attribute for this ?
> > 
> > I think it depends on the kernel we use and architecture. On x86 I
> > think
> > we rely on udev, like this:
> > 
> > https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-nocturne/chromeos-base/chromeos-bsp-nocturne/files/udev/99-cros-sx-proximity.rules
> > 
> > DEVPATH=="*/pci0000:00/0000:00:15.1/*", SYMLINK+="proximity-wifi-
> > right"
> > DEVPATH=="*/pci0000:00/0000:00:19.1/*", SYMLINK+="proximity-wifi-
> > left"
> > ATTR{events/in_proximity1_USE_CS1_thresh_either_en}="1"
> 
> So that results in a symlink under /dev, right ? That seems like
> it is not really compatible with how most modern userspace discovers
> hw (through udev). Although I guess code using udev could still
> lookup the symlink in the udev per device data, this just not feel
> like a good way forward.

We can tag it, the metadata will be readable in where we need it,
through libudev, so that's not a big bother.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-19 15:39             ` Hans de Goede
  2020-11-19 16:11               ` Bastien Nocera
@ 2020-11-20  9:59               ` Jonathan Cameron
  2020-11-23 12:16                 ` Hans de Goede
  1 sibling, 1 reply; 28+ messages in thread
From: Jonathan Cameron @ 2020-11-20  9:59 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Dmitry Torokhov, Mark Pearson, linux-iio, Bastien Nocera,
	Nitin Joshi1, linux-input

On Thu, 19 Nov 2020 16:39:07 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 11/13/20 7:58 AM, Dmitry Torokhov wrote:
> > On Thu, Nov 12, 2020 at 10:50:12AM +0100, Hans de Goede wrote:  
> >> Hi,
> >>
> >> On 11/12/20 7:23 AM, Dmitry Torokhov wrote:  
> >>> On Wed, Oct 07, 2020 at 11:51:05AM +0200, Hans de Goede wrote:  
> >>>> Hi,
> >>>>
> >>>> On 10/7/20 10:36 AM, Jonathan Cameron wrote:  
> >>>>> On Mon, 5 Oct 2020 22:04:27 -0400
> >>>>> Mark Pearson <markpearson@lenovo.com> wrote:
> >>>>>  
> >>>>>> Adding Nitin, lead for this feature, to the thread  
> >>>>>
> >>>>> +CC linux-input and Dmitry for reasons that will become clear below.  
> >>>>>>
> >>>>>> On 2020-10-03 10:02 a.m., Hans de Goede wrote:  
> >>>>>>> Hi All,
> >>>>>>>
> >>>>>>> Modern laptops can have various sensors which are kinda
> >>>>>>> like proximity sensors, but not really (they are more
> >>>>>>> specific in which part of the laptop the user is
> >>>>>>> proximate to).
> >>>>>>>
> >>>>>>> Specifically modern Thinkpad's have 2 readings which we
> >>>>>>> want to export to userspace, and I'm wondering if we
> >>>>>>> could use the IIO framework for this since these readings
> >>>>>>> are in essence sensor readings:
> >>>>>>>
> >>>>>>> 1. These laptops have a sensor in the palm-rests to
> >>>>>>> check if a user is physically proximate to the device's
> >>>>>>> palm-rests. This info will be used by userspace for WWAN
> >>>>>>> functionality to control the transmission level safely.
> >>>>>>>
> >>>>>>> A patch adding a thinkpad_acpi specific sysfs API for this
> >>>>>>> is currently pending:
> >>>>>>> https://patchwork.kernel.org/patch/11722127/
> >>>>>>>
> >>>>>>> But I'm wondering if it would not be better to use
> >>>>>>> IIO to export this info.  
> >>>>>
> >>>>> My first thought on this is it sounds more like a key than a sensor
> >>>>> (simple proximity sensors fall into this category as well.)  
> >>>
> >>> [ sorry for sitting on this thread for so long ]
> >>>
> >>> So I think the important question here is if we only ever want yes/no
> >>> answer, or if we can consider adjusting behavior of the system based on
> >>> the "closeness" of an object to the device, in which case I think IIO is
> >>> more flexible.
> >>>
> >>> FWIW in Chrome OS land we name IIO proximity sensors using a scheme
> >>> "proximity-lte", "proximity-wifi", "proximity-wifi-left",
> >>> "proximity-wifi-right", etc, and then userspace implements various
> >>> policies (SAR, etc) based off it.  
> >>
> >> Interesting, so 2 questions:
> >>
> >> 1. So your encoding the location in the sensor's parent-device name
> >> instead of using a new sysfs attribute for this ?  
> > 
> > I think it depends on the kernel we use and architecture. On x86 I think
> > we rely on udev, like this:
> > 
> > https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-nocturne/chromeos-base/chromeos-bsp-nocturne/files/udev/99-cros-sx-proximity.rules
> > 
> > DEVPATH=="*/pci0000:00/0000:00:15.1/*", SYMLINK+="proximity-wifi-right"
> > DEVPATH=="*/pci0000:00/0000:00:19.1/*", SYMLINK+="proximity-wifi-left"
> > ATTR{events/in_proximity1_USE_CS1_thresh_either_en}="1"  
> 
> So that results in a symlink under /dev, right ? That seems like
> it is not really compatible with how most modern userspace discovers
> hw (through udev). Although I guess code using udev could still
> lookup the symlink in the udev per device data, this just not feel
> like a good way forward.
> 
> > On newer ARM we use "label" attribute in DTS:
> > 
> > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > 
> >         ap_sar_sensor: proximity@28 {
> >                 compatible = "semtech,sx9310";
> >                 reg = <0x28>;
> >                 #io-channel-cells = <1>;
> >                 pinctrl-names = "default";
> >                 pinctrl-0 = <&p_sensor_int_l>;
> > 
> >                 interrupt-parent = <&tlmm>;
> >                 interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
> > 
> >                 vdd-supply = <&pp3300_a>;
> >                 svdd-supply = <&pp1800_prox>;
> > 
> >                 status = "disabled";
> >                 label = "proximity-wifi";
> >         };  
> 
> Hmm, interesting. I did not know iio-devices could
> have a label sysfs attribute (nor that that could be
> set through device-tree). I was thinking about adding
> an in_proximity_location sysfs attribute. But using
> labels (and standardizing a set of label names) will
> work nicely too.

It's fairly new.   Note we also have per channel labels
though they are 'very new'.  Might be handy if the sensors
appear as a single device despite being spread over the
laptop.

> 
> I have no real preference for this either way, so
> I guess we might as well go with labels to avoid
> having any unnecessary discrepancies between ChromeOS
> and whatever we do for the Thinkpad sensors.
> 
> Is there a know set of labels which ChromeOS is currently
> using? If we are going to use labels for this it would
> be good IMHO to define a set of standard labels for
> this in say Documentation/ABI/testing/sysfs-bus-iio-labels.

If you do want to do this, please just put it under sysfs-bus-iio
doc.  I want this to be in the top level doc and there is an issue
we are currently trying to sort out with autogenerated docs and
repeats of a given filename in the ABI docs.
(basically it doesn't work and generates lots of warnings!)

Thanks,

Jonathan

> 
> >> 2. Do these sensors just give a boolean value atm, or do they already
> >> report a range ?  IIRC one of the objections from the iio folks in
> >> the Lenovo case was that booleans are not really a good fit for iio
> >> (IIRC they also said we could still use iio for this).  
> > 
> > One of the sensors we use is sx9310 that I believe can report range, but
> > I think we configure them to trigger when a threshold is crossed.
> > 
> > Events are handled by our powerd:
> > 
> > https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/powerd/system/sar_watcher.cc
> >   
> >>
> >> Perhaps you can provide an URL to the kernel code implementing these ?  
> > 
> > drivers/iio/proximity/sx9310.c  
> 
> If I'm reading that correctly the it exports a raw "distance"
> reading and a suggested threshold value for the code interpreting
> the reading to use.
> 
> So that would be a bit different then the Thinkpad sensors, but
> exporting just a 0-1 range for the in_proximity_raw value for the
> Thinkpad case should not be a problem. Or we could just make it
> repot 0 and 100 and export a fixed in_proximity_nearlevel of 50,
> that would make the userspace API more like other proximity sensors.
> 
> Regards,
> 
> Hans
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-20  9:59               ` Jonathan Cameron
@ 2020-11-23 12:16                 ` Hans de Goede
  2020-11-23 16:07                   ` Jonathan Cameron
  0 siblings, 1 reply; 28+ messages in thread
From: Hans de Goede @ 2020-11-23 12:16 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Dmitry Torokhov, Mark Pearson, linux-iio, Bastien Nocera,
	Nitin Joshi1, linux-input

Hi,

On 11/20/20 10:59 AM, Jonathan Cameron wrote:
> On Thu, 19 Nov 2020 16:39:07 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:

>>>>>>>> On 2020-10-03 10:02 a.m., Hans de Goede wrote:  
>>>>>>>>> Hi All,
>>>>>>>>>
>>>>>>>>> Modern laptops can have various sensors which are kinda
>>>>>>>>> like proximity sensors, but not really (they are more
>>>>>>>>> specific in which part of the laptop the user is
>>>>>>>>> proximate to).
>>>>>>>>>
>>>>>>>>> Specifically modern Thinkpad's have 2 readings which we
>>>>>>>>> want to export to userspace, and I'm wondering if we
>>>>>>>>> could use the IIO framework for this since these readings
>>>>>>>>> are in essence sensor readings:
>>>>>>>>>
>>>>>>>>> 1. These laptops have a sensor in the palm-rests to
>>>>>>>>> check if a user is physically proximate to the device's
>>>>>>>>> palm-rests. This info will be used by userspace for WWAN
>>>>>>>>> functionality to control the transmission level safely.
>>>>>>>>>
>>>>>>>>> A patch adding a thinkpad_acpi specific sysfs API for this
>>>>>>>>> is currently pending:
>>>>>>>>> https://patchwork.kernel.org/patch/11722127/
>>>>>>>>>
>>>>>>>>> But I'm wondering if it would not be better to use
>>>>>>>>> IIO to export this info.  

<snip>

>>> On newer ARM we use "label" attribute in DTS:
>>>
>>> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>>>
>>>         ap_sar_sensor: proximity@28 {
>>>                 compatible = "semtech,sx9310";
>>>                 reg = <0x28>;
>>>                 #io-channel-cells = <1>;
>>>                 pinctrl-names = "default";
>>>                 pinctrl-0 = <&p_sensor_int_l>;
>>>
>>>                 interrupt-parent = <&tlmm>;
>>>                 interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
>>>
>>>                 vdd-supply = <&pp3300_a>;
>>>                 svdd-supply = <&pp1800_prox>;
>>>
>>>                 status = "disabled";
>>>                 label = "proximity-wifi";
>>>         };  
>>
>> Hmm, interesting. I did not know iio-devices could
>> have a label sysfs attribute (nor that that could be
>> set through device-tree). I was thinking about adding
>> an in_proximity_location sysfs attribute. But using
>> labels (and standardizing a set of label names) will
>> work nicely too.
> 
> It's fairly new.   Note we also have per channel labels
> though they are 'very new'.  Might be handy if the sensors
> appear as a single device despite being spread over the
> laptop.

Interesting, the thinkpad_acpi stuff currently has 2
proximity(ish) sensors:

1. Laptop is close to (on) someones lap
2. Someone's arms are resting on or close to the palmrest

Ideally we would indeed register 1 iio-dev with separate
channels for this, rather then having to register 2
(and the future maybe even more) iio-devs for this.

Can you give a pointer to docs / examples of using a
label per channel ?

>> Is there a know set of labels which ChromeOS is currently
>> using? If we are going to use labels for this it would
>> be good IMHO to define a set of standard labels for
>> this in say Documentation/ABI/testing/sysfs-bus-iio-labels.
> 
> If you do want to do this, please just put it under sysfs-bus-iio
> doc.  I want this to be in the top level doc.

Ok, ack.

Dmitry, can you perhaps dig up a full-list of labels
which ChromeOS is currently using to identify
proximity sensors for e.g. SAR related use?

Regards,

Hans


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
  2020-11-23 12:16                 ` Hans de Goede
@ 2020-11-23 16:07                   ` Jonathan Cameron
  0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Cameron @ 2020-11-23 16:07 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Dmitry Torokhov, Mark Pearson, linux-iio, Bastien Nocera,
	Nitin Joshi1, linux-input

On Mon, 23 Nov 2020 13:16:20 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 11/20/20 10:59 AM, Jonathan Cameron wrote:
> > On Thu, 19 Nov 2020 16:39:07 +0100
> > Hans de Goede <hdegoede@redhat.com> wrote:  
> 
> >>>>>>>> On 2020-10-03 10:02 a.m., Hans de Goede wrote:    
> >>>>>>>>> Hi All,
> >>>>>>>>>
> >>>>>>>>> Modern laptops can have various sensors which are kinda
> >>>>>>>>> like proximity sensors, but not really (they are more
> >>>>>>>>> specific in which part of the laptop the user is
> >>>>>>>>> proximate to).
> >>>>>>>>>
> >>>>>>>>> Specifically modern Thinkpad's have 2 readings which we
> >>>>>>>>> want to export to userspace, and I'm wondering if we
> >>>>>>>>> could use the IIO framework for this since these readings
> >>>>>>>>> are in essence sensor readings:
> >>>>>>>>>
> >>>>>>>>> 1. These laptops have a sensor in the palm-rests to
> >>>>>>>>> check if a user is physically proximate to the device's
> >>>>>>>>> palm-rests. This info will be used by userspace for WWAN
> >>>>>>>>> functionality to control the transmission level safely.
> >>>>>>>>>
> >>>>>>>>> A patch adding a thinkpad_acpi specific sysfs API for this
> >>>>>>>>> is currently pending:
> >>>>>>>>> https://patchwork.kernel.org/patch/11722127/
> >>>>>>>>>
> >>>>>>>>> But I'm wondering if it would not be better to use
> >>>>>>>>> IIO to export this info.    
> 
> <snip>
> 
> >>> On newer ARM we use "label" attribute in DTS:
> >>>
> >>> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> >>>
> >>>         ap_sar_sensor: proximity@28 {
> >>>                 compatible = "semtech,sx9310";
> >>>                 reg = <0x28>;
> >>>                 #io-channel-cells = <1>;
> >>>                 pinctrl-names = "default";
> >>>                 pinctrl-0 = <&p_sensor_int_l>;
> >>>
> >>>                 interrupt-parent = <&tlmm>;
> >>>                 interrupts = <24 IRQ_TYPE_LEVEL_LOW>;
> >>>
> >>>                 vdd-supply = <&pp3300_a>;
> >>>                 svdd-supply = <&pp1800_prox>;
> >>>
> >>>                 status = "disabled";
> >>>                 label = "proximity-wifi";
> >>>         };    
> >>
> >> Hmm, interesting. I did not know iio-devices could
> >> have a label sysfs attribute (nor that that could be
> >> set through device-tree). I was thinking about adding
> >> an in_proximity_location sysfs attribute. But using
> >> labels (and standardizing a set of label names) will
> >> work nicely too.  
> > 
> > It's fairly new.   Note we also have per channel labels
> > though they are 'very new'.  Might be handy if the sensors
> > appear as a single device despite being spread over the
> > laptop.  
> 
> Interesting, the thinkpad_acpi stuff currently has 2
> proximity(ish) sensors:
> 
> 1. Laptop is close to (on) someones lap
> 2. Someone's arms are resting on or close to the palmrest
> 
> Ideally we would indeed register 1 iio-dev with separate
> channels for this, rather then having to register 2
> (and the future maybe even more) iio-devs for this.
> 
> Can you give a pointer to docs / examples of using a
> label per channel ?

Docs if done with DT binding are at:
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=7f79711533a96b02e1e24e2e36a29b08734e36e2
ABI Docs
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=3079188f821cfbdbb0b12f668335931a87eb14c6
An example using it directly rather than via DT (it's not really
DT related though in some cases the label may come from there)
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=1f4877218f7e2c2b914aeb69a8a0f47d59c74717

Those will probably be in Linux-next later this week.

Jonathan

> 
> >> Is there a know set of labels which ChromeOS is currently
> >> using? If we are going to use labels for this it would
> >> be good IMHO to define a set of standard labels for
> >> this in say Documentation/ABI/testing/sysfs-bus-iio-labels.  
> > 
> > If you do want to do this, please just put it under sysfs-bus-iio
> > doc.  I want this to be in the top level doc.  
> 
> Ok, ack.
> 
> Dmitry, can you perhaps dig up a full-list of labels
> which ChromeOS is currently using to identify
> proximity sensors for e.g. SAR related use?
> 
> Regards,
> 
> Hans
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2020-11-23 16:08 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <9f9b0ff6-3bf1-63c4-eb36-901cecd7c4d9@redhat.com>
     [not found] ` <5a646527-7a1f-2fb9-7c09-8becdbff417b@lenovo.com>
2020-10-07  8:36   ` [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace? Jonathan Cameron
2020-10-07  9:51     ` Hans de Goede
2020-10-07 11:35       ` Bastien Nocera
2020-10-07 13:08         ` Hans de Goede
2020-10-07 13:29           ` Bastien Nocera
2020-10-07 13:32             ` Hans de Goede
2020-10-08  0:14               ` Jeff LaBundy
2020-10-08  7:10                 ` Hans de Goede
2020-10-09  2:19                   ` Jeff LaBundy
2020-10-12 12:13                     ` Hans de Goede
2020-10-13 21:59                       ` Mark Pearson
2020-10-14  4:47                         ` Jeff LaBundy
2020-10-14  8:16                         ` Hans de Goede
2020-10-14 14:26                           ` Mark Pearson
2020-10-12 12:36                   ` Enrico Weigelt, metux IT consult
2020-10-13  1:12                     ` Mark Pearson
2020-10-13  8:38                     ` Hans de Goede
2020-11-12  6:23       ` Dmitry Torokhov
2020-11-12  9:50         ` Hans de Goede
2020-11-13  6:58           ` Dmitry Torokhov
2020-11-19 15:39             ` Hans de Goede
2020-11-19 16:11               ` Bastien Nocera
2020-11-20  9:59               ` Jonathan Cameron
2020-11-23 12:16                 ` Hans de Goede
2020-11-23 16:07                   ` Jonathan Cameron
2020-11-19 15:16         ` Bastien Nocera
2020-11-19 15:24           ` Hans de Goede
2020-11-19 15:58             ` Bastien Nocera

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).