linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff LaBundy <jeff@labundy.com>
To: Mark Pearson <markpearson@lenovo.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	Bastien Nocera <hadess@hadess.net>,
	Jonathan Cameron <Jonathan.Cameron@Huawei.com>,
	Nitin Joshi1 <njoshi1@lenovo.com>,
	linux-input@vger.kernel.org, dmitry.torokhov@gmail.com
Subject: Re: [External] Using IIO to export laptop palm-sensor and lap-mode info to userspace?
Date: Tue, 13 Oct 2020 23:47:50 -0500	[thread overview]
Message-ID: <20201014044750.GA20190@labundy.com> (raw)
In-Reply-To: <075a5f57-3330-78fe-669b-01570d43d9c0@lenovo.com>

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

  reply	other threads:[~2020-10-14  4:48 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201014044750.GA20190@labundy.com \
    --to=jeff@labundy.com \
    --cc=Jonathan.Cameron@Huawei.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=hadess@hadess.net \
    --cc=hdegoede@redhat.com \
    --cc=linux-input@vger.kernel.org \
    --cc=markpearson@lenovo.com \
    --cc=njoshi1@lenovo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).