Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?

[Hans de Goede <hdegoede@redhat.com>]

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