2018-01-12 11:08:49

by David Herrmann

[permalink] [raw]
Subject: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

This patch prevents the thinkpad_acpi driver from warning about 2 event
codes returned for keyboard palm-detection. No behavioral changes,
other than suppressing the warning in the kernel log. The events are
still forwarded via acpi-netlink channels.

We could, optionally, decide to forward the event through a
input-switch on the tpacpi input device. However, so far no suitable
input-code exists, and no similar drivers report such events. Hence,
leave it an acpi event for now.

Note that the event-codes are named based on empirical studies. On the
ThinkPad X1 5th Gen the sensor can be found underneath the arrow key.

Cc: Matthew Thode <[email protected]>
Signed-off-by: David Herrmann <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 117be48ff4de..128f91af716e 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -214,6 +214,10 @@ enum tpacpi_hkey_event_t {
/* AC-related events */
TP_HKEY_EV_AC_CHANGED = 0x6040, /* AC status changed */

+ /* Further user-interface events */
+ TP_HKEY_EV_PALM_DETECTED = 0x60b0, /* palm hoveres keyboard */
+ TP_HKEY_EV_PALM_UNDETECTED = 0x60b1, /* palm removed */
+
/* Misc */
TP_HKEY_EV_RFKILL_CHANGED = 0x7000, /* rfkill switch changed */
};
@@ -4079,6 +4083,12 @@ static bool hotkey_notify_6xxx(const u32 hkey,
*send_acpi_ev = false;
break;

+ case TP_HKEY_EV_PALM_DETECTED:
+ case TP_HKEY_EV_PALM_UNDETECTED:
+ /* palm detected hovering the keyboard, forward to user-space
+ * via netlink for consumption */
+ return true;
+
default:
pr_warn("unknown possible thermal alarm or keyboard event received\n");
known = false;
--
2.15.1


Subject: Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

On Fri, 12 Jan 2018, David Herrmann wrote:
> This patch prevents the thinkpad_acpi driver from warning about 2 event
> codes returned for keyboard palm-detection. No behavioral changes,
> other than suppressing the warning in the kernel log. The events are
> still forwarded via acpi-netlink channels.
>
> We could, optionally, decide to forward the event through a
> input-switch on the tpacpi input device. However, so far no suitable
> input-code exists, and no similar drivers report such events. Hence,
> leave it an acpi event for now.
>
> Note that the event-codes are named based on empirical studies. On the
> ThinkPad X1 5th Gen the sensor can be found underneath the arrow key.
>
> Cc: Matthew Thode <[email protected]>
> Signed-off-by: David Herrmann <[email protected]>

Well, we certainly should drop the warning, and I am fine with the
current name for the events (they are not ABI in any way, so we can
change that at any time if we find out it should be named differently).

Acked-by: Henrique de Moraes Holschuh <[email protected]>

> ---
> drivers/platform/x86/thinkpad_acpi.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 117be48ff4de..128f91af716e 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -214,6 +214,10 @@ enum tpacpi_hkey_event_t {
> /* AC-related events */
> TP_HKEY_EV_AC_CHANGED = 0x6040, /* AC status changed */
>
> + /* Further user-interface events */
> + TP_HKEY_EV_PALM_DETECTED = 0x60b0, /* palm hoveres keyboard */
> + TP_HKEY_EV_PALM_UNDETECTED = 0x60b1, /* palm removed */
> +
> /* Misc */
> TP_HKEY_EV_RFKILL_CHANGED = 0x7000, /* rfkill switch changed */
> };
> @@ -4079,6 +4083,12 @@ static bool hotkey_notify_6xxx(const u32 hkey,
> *send_acpi_ev = false;
> break;
>
> + case TP_HKEY_EV_PALM_DETECTED:
> + case TP_HKEY_EV_PALM_UNDETECTED:
> + /* palm detected hovering the keyboard, forward to user-space
> + * via netlink for consumption */
> + return true;
> +
> default:
> pr_warn("unknown possible thermal alarm or keyboard event received\n");
> known = false;

--
Henrique Holschuh

2018-01-12 13:50:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <[email protected]> wrote:
> This patch prevents the thinkpad_acpi driver from warning about 2 event
> codes returned for keyboard palm-detection. No behavioral changes,
> other than suppressing the warning in the kernel log. The events are
> still forwarded via acpi-netlink channels.
>
> We could, optionally, decide to forward the event through a
> input-switch on the tpacpi input device. However, so far no suitable
> input-code exists, and no similar drivers report such events. Hence,
> leave it an acpi event for now.
>
> Note that the event-codes are named based on empirical studies. On the
> ThinkPad X1 5th Gen the sensor can be found underneath the arrow key.
>

> Cc: Matthew Thode <[email protected]>

Shouldn't be Suggested-by or even Signed-off-by?

> Signed-off-by: David Herrmann <[email protected]>


> + case TP_HKEY_EV_PALM_DETECTED:
> + case TP_HKEY_EV_PALM_UNDETECTED:

> + /* palm detected hovering the keyboard, forward to user-space
> + * via netlink for consumption */

Comment style is
/*
* Multi line comment.
* This is an example.
*/

> + return true;

I can fix the latter, but I have to be sure all credits are given properly.

--
With Best Regards,
Andy Shevchenko

2018-01-12 14:07:18

by David Herrmann

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

Hi Andy

On Fri, Jan 12, 2018 at 2:50 PM, Andy Shevchenko
<[email protected]> wrote:
> On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <[email protected]> wrote:
>> This patch prevents the thinkpad_acpi driver from warning about 2 event
>> codes returned for keyboard palm-detection. No behavioral changes,
>> other than suppressing the warning in the kernel log. The events are
>> still forwarded via acpi-netlink channels.
>>
>> We could, optionally, decide to forward the event through a
>> input-switch on the tpacpi input device. However, so far no suitable
>> input-code exists, and no similar drivers report such events. Hence,
>> leave it an acpi event for now.
>>
>> Note that the event-codes are named based on empirical studies. On the
>> ThinkPad X1 5th Gen the sensor can be found underneath the arrow key.
>>
>
>> Cc: Matthew Thode <[email protected]>
>
> Shouldn't be Suggested-by or even Signed-off-by?

The patch is different (Matthew originally suppressed the ACPI event),
so I did not copy the sign-off. Please add Suggested-by, if that is an
acceptable tag.

>> Signed-off-by: David Herrmann <[email protected]>
>
>
>> + case TP_HKEY_EV_PALM_DETECTED:
>> + case TP_HKEY_EV_PALM_UNDETECTED:
>
>> + /* palm detected hovering the keyboard, forward to user-space
>> + * via netlink for consumption */
>
> Comment style is
> /*
> * Multi line comment.
> * This is an example.
> */

All other 6 comments in this function follow the style I used here, so
I tried to be consistent. But feel free to amend this change.

Thanks a lot!
David

2018-01-13 06:39:23

by Matthew Thode

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

On 18-01-12 15:07:12, David Herrmann wrote:
> Hi Andy
>
> On Fri, Jan 12, 2018 at 2:50 PM, Andy Shevchenko
> <[email protected]> wrote:
> > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <[email protected]> wrote:
> >> Cc: Matthew Thode <[email protected]>
> >
> > Shouldn't be Suggested-by or even Signed-off-by?
>
> The patch is different (Matthew originally suppressed the ACPI event),
> so I did not copy the sign-off. Please add Suggested-by, if that is an
> acceptable tag.
>
> >> Signed-off-by: David Herrmann <[email protected]>
> >
> >
> >> + case TP_HKEY_EV_PALM_DETECTED:
> >> + case TP_HKEY_EV_PALM_UNDETECTED:
> >
> >> + /* palm detected hovering the keyboard, forward to user-space
> >> + * via netlink for consumption */
> >
> > Comment style is
> > /*
> > * Multi line comment.
> > * This is an example.
> > */
>
> All other 6 comments in this function follow the style I used here, so
> I tried to be consistent. But feel free to amend this change.
>

I'm fine with a signed-off-by or tested-by or suggested-by. There is a
spelling mistake though, 'hoveres' should be hovers.

--
Matthew Thode


Attachments:
(No filename) (1.15 kB)
signature.asc (833.00 B)
Download all attachments

2018-02-05 05:03:30

by Matthew Thode

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

On 18-01-13 00:33:09, Matthew Thode wrote:
> On 18-01-12 15:07:12, David Herrmann wrote:
> > Hi Andy
> >
> > On Fri, Jan 12, 2018 at 2:50 PM, Andy Shevchenko
> > <[email protected]> wrote:
> > > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <[email protected]> wrote:
> > >> Cc: Matthew Thode <[email protected]>
> > >
> > > Shouldn't be Suggested-by or even Signed-off-by?
> >
> > The patch is different (Matthew originally suppressed the ACPI event),
> > so I did not copy the sign-off. Please add Suggested-by, if that is an
> > acceptable tag.
> >
> > >> Signed-off-by: David Herrmann <[email protected]>
> > >
> > >
> > >> + case TP_HKEY_EV_PALM_DETECTED:
> > >> + case TP_HKEY_EV_PALM_UNDETECTED:
> > >
> > >> + /* palm detected hovering the keyboard, forward to user-space
> > >> + * via netlink for consumption */
> > >
> > > Comment style is
> > > /*
> > > * Multi line comment.
> > > * This is an example.
> > > */
> >
> > All other 6 comments in this function follow the style I used here, so
> > I tried to be consistent. But feel free to amend this change.
> >
>
> I'm fine with a signed-off-by or tested-by or suggested-by. There is a
> spelling mistake though, 'hoveres' should be hovers.
>

Is there anything else I can do here? I'm not sure any of us have the
ability to find out what lenovo actualy wants this for.

Sorry for the double reply, dmarc can be hell.

--
Matthew Thode


Attachments:
(No filename) (1.48 kB)
signature.asc (849.00 B)
Download all attachments
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

> > I'm fine with a signed-off-by or tested-by or suggested-by. There is a
> > spelling mistake though, 'hoveres' should be hovers.
>
> Is there anything else I can do here? I'm not sure any of us have the
> ability to find out what lenovo actualy wants this for.

I have already acked the patch, so Darren or Andy should pick it up
sooner or later for merging into the subsystem tree...

But if you wouldn't mind doing so, you could send a V2 with all acks,
tested-by/suggested-by/whatever added, as well as that minor spelling
mistake correction... I am not sure this is necessary, though.

Andy, Daren?

--
Henrique Holschuh

2018-02-05 13:10:46

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

On Mon, Feb 5, 2018 at 1:34 PM, Henrique de Moraes Holschuh
<[email protected]> wrote:
>> > I'm fine with a signed-off-by or tested-by or suggested-by. There is a
>> > spelling mistake though, 'hoveres' should be hovers.
>>
>> Is there anything else I can do here? I'm not sure any of us have the
>> ability to find out what lenovo actualy wants this for.
>
> I have already acked the patch, so Darren or Andy should pick it up
> sooner or later for merging into the subsystem tree...
>
> But if you wouldn't mind doing so, you could send a V2 with all acks,
> tested-by/suggested-by/whatever added, as well as that minor spelling
> mistake correction... I am not sure this is necessary, though.
>

> Andy, Daren?

The following one already in for-next, meaning it's about to be upstream soon
587d8628fb71 platform/x86: thinkpad_acpi: suppress warning about palm detection


--
With Best Regards,
Andy Shevchenko

2018-02-05 14:29:01

by Matthew Thode

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection

On 18-02-05 15:08:25, Andy Shevchenko wrote:
> On Mon, Feb 5, 2018 at 1:34 PM, Henrique de Moraes Holschuh
> <[email protected]> wrote:
> >> > I'm fine with a signed-off-by or tested-by or suggested-by. There is a
> >> > spelling mistake though, 'hoveres' should be hovers.
> >>
> >> Is there anything else I can do here? I'm not sure any of us have the
> >> ability to find out what lenovo actualy wants this for.
> >
> > I have already acked the patch, so Darren or Andy should pick it up
> > sooner or later for merging into the subsystem tree...
> >
> > But if you wouldn't mind doing so, you could send a V2 with all acks,
> > tested-by/suggested-by/whatever added, as well as that minor spelling
> > mistake correction... I am not sure this is necessary, though.
> >
>
> > Andy, Daren?
>
> The following one already in for-next, meaning it's about to be upstream soon
> 587d8628fb71 platform/x86: thinkpad_acpi: suppress warning about palm detection
>

Thanks, if there's nothing left for me to do I'll just leave it then.

--
Matthew Thode


Attachments:
(No filename) (1.05 kB)
signature.asc (849.00 B)
Download all attachments