| Message ID | 20180112110445.1876-1-dh.herrmann@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Andy Shevchenko |
| Headers | show |
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 <mthode@mthode.org> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> 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 <hmh@hmh.eng.br> > --- > 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;
On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <dh.herrmann@gmail.com> 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 <mthode@mthode.org> Shouldn't be Suggested-by or even Signed-off-by? > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> > + 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.
Hi Andy On Fri, Jan 12, 2018 at 2:50 PM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <dh.herrmann@gmail.com> 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 <mthode@mthode.org> > > 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 <dh.herrmann@gmail.com> > > >> + 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
On 18-01-12 15:07:12, David Herrmann wrote: > Hi Andy > > On Fri, Jan 12, 2018 at 2:50 PM, Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <dh.herrmann@gmail.com> wrote: > >> Cc: Matthew Thode <mthode@mthode.org> > > > > 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 <dh.herrmann@gmail.com> > > > > > >> + 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.
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 > > <andy.shevchenko@gmail.com> wrote: > > > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <dh.herrmann@gmail.com> wrote: > > >> Cc: Matthew Thode <mthode@mthode.org> > > > > > > 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 <dh.herrmann@gmail.com> > > > > > > > > >> + 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 actually wants this for.
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 > > <andy.shevchenko@gmail.com> wrote: > > > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann <dh.herrmann@gmail.com> wrote: > > >> Cc: Matthew Thode <mthode@mthode.org> > > > > > > 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 <dh.herrmann@gmail.com> > > > > > > > > >> + 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.
> > 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?
On Mon, Feb 5, 2018 at 1:34 PM, Henrique de Moraes Holschuh <hmh@hmh.eng.br> 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
On 18-02-05 15:08:25, Andy Shevchenko wrote: > On Mon, Feb 5, 2018 at 1:34 PM, Henrique de Moraes Holschuh > <hmh@hmh.eng.br> 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.
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;
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 <mthode@mthode.org> Signed-off-by: David Herrmann <dh.herrmann@gmail.com> --- drivers/platform/x86/thinkpad_acpi.c | 10 ++++++++++ 1 file changed, 10 insertions(+)