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 > 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 signature.asc Description: PGP signature
Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection
On Mon, Feb 5, 2018 at 1:34 PM, Henrique de Moraes Holschuh 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
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
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 > > wrote: > > > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann > > > wrote: > > >> Cc: Matthew Thode > > > > > > 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 > > > > > > > > >> + 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 signature.asc Description: PGP signature
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 > > wrote: > > > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann > > > wrote: > > >> Cc: Matthew Thode > > > > > > 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 > > > > > > > > >> + 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. -- Matthew Thode (prometheanfire) signature.asc Description: PGP signature
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 > wrote: > > On Fri, Jan 12, 2018 at 1:04 PM, David Herrmann > > wrote: > >> Cc: Matthew Thode > > > > 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 > > > > > >> + 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 signature.asc Description: PGP signature
Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection
Hi Andy On Fri, Jan 12, 2018 at 2:50 PM, Andy Shevchenko wrote: > On Fri, Jan 12, 2018 at 1:04 PM, 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 > > 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 > > >> + 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
Re: [PATCH] platform/x86: thinkpad_acpi: suppress warning about palm detection
On Fri, Jan 12, 2018 at 1:04 PM, 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 Shouldn't be Suggested-by or even Signed-off-by? > Signed-off-by: David Herrmann > + 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
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
> Signed-off-by: David Herrmann
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
> ---
> 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
