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:

quoted

Hi,

On 10/9/20 4:19 AM, Jeff LaBundy wrote:

quoted

Hi Hans,

<snip>

quoted

quoted

quoted

quoted

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.

quoted

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