All of lore.kernel.org
 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: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-03 14:02 Using IIO to export laptop palm-sensor and lap-mode info to userspace? Hans de Goede
2020-10-06  2:04 ` [External] " Mark Pearson
2020-10-07  8:36   ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.