platform/x86: thinkpad_acpi: suppress warning about palm detection

PATCH In review David Herrmann 4 participants 10 messages Jan 12, 2018 – Feb 5, 2018 x86 platform drivers

Share & tools

David Herrmann

author

2018-01-12 11:04 UTC #0 lore raw

PATCH 1/1 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(+)

drivers/platform/x86/thinkpad_acpi.c +10 -0

index 117be48ff4de..128f91af716e 100644
@@ -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;

Signature · 1 line

2.15.1

Henrique de Moraes Holschuh

Acked-by on v1 1/1

2018-01-12 12:06 UTC #1 lore raw

Quoted 10 lines · David Herrmann

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.

Quoted 5 lines

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]>

Quoted 32 lines

---
 drivers/platform/x86/thinkpad_acpi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

drivers/platform/x86/thinkpad_acpi.c +10 -0

index 117be48ff4de..128f91af716e 100644
@@ -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;

Signature · 1 line

  Henrique Holschuh

Andy Shevchenko

2018-01-12 13:50 UTC #2 lore raw

In reply to David Herrmann

Quoted 10 lines · David Herrmann

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.

Quoted 5 lines

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?

drivers/platform/x86/thinkpad_acpi.c static bool hotkey_notify_6xxx(const u32 hkey, · line 4089

Quoted 8 lines

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

drivers/platform/x86/thinkpad_acpi.c +4 -0

+       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.
*/

Quoted 1 line

+ return true;

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

Signature · 2 lines

With Best Regards,
Andy Shevchenko

David Herrmann

author

2018-01-12 14:07 UTC #3 lore raw

Hi Andy

Quoted 11 lines · Andy Shevchenko

Quoted 10 lines · David Herrmann
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.

Quoted 7 lines

Quoted 5 lines
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.

Quoted 6 lines

Quoted 5 lines
Signed-off-by David Herrmann <[email protected]>
drivers/platform/x86/thinkpad_acpi.c +2 -0
+       case TP_HKEY_EV_PALM_DETECTED:
+       case TP_HKEY_EV_PALM_UNDETECTED:

Quoted 8 lines

drivers/platform/x86/thinkpad_acpi.c static bool hotkey_notify_6xxx(const u32 hkey, · line 4089
Quoted 2 lines
drivers/platform/x86/thinkpad_acpi.c +2 -0
+               /* 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

Matthew Thode

2018-01-13 06:33 UTC #4 lore raw

Quoted 22 lines · David Herrmann

Hi Andy
Quoted 4 lines · Andy Shevchenko
Quoted 1 line · David Herrmann
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.
Quoted 9 lines
Quoted 8 lines
Signed-off-by David Herrmann <[email protected]>
drivers/platform/x86/thinkpad_acpi.c +4 -0
+       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 */

Quoted 9 lines

Quoted 5 lines
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.

Signature · 1 line

Matthew Thode

Matthew Thode

2018-02-05 04:56 UTC #5 lore raw

Quoted 29 lines · Matthew Thode

Quoted 28 lines · David Herrmann
Hi Andy
Quoted 4 lines · Andy Shevchenko
Quoted 1 line · David Herrmann
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.
Quoted 14 lines
drivers/platform/x86/thinkpad_acpi.c static bool hotkey_notify_6xxx(const u32 hkey, · line 4089
Quoted 8 lines
Signed-off-by David Herrmann <[email protected]>
drivers/platform/x86/thinkpad_acpi.c +4 -0
+       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.
*/

Quoted 7 lines

Quoted 3 lines
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.

Signature · 1 line

Matthew Thode (prometheanfire)

Matthew Thode

2018-02-05 05:01 UTC #6 lore raw

In reply to Matthew Thode

Quoted 29 lines · Matthew Thode

Quoted 28 lines · David Herrmann
Hi Andy
Quoted 4 lines · Andy Shevchenko
Quoted 1 line · David Herrmann
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.
Quoted 14 lines
drivers/platform/x86/thinkpad_acpi.c static bool hotkey_notify_6xxx(const u32 hkey, · line 4089
Quoted 8 lines
Signed-off-by David Herrmann <[email protected]>
drivers/platform/x86/thinkpad_acpi.c +4 -0
+       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.
*/

Quoted 7 lines

Quoted 3 lines
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.

Signature · 1 line

Matthew Thode

Henrique de Moraes Holschuh

2018-02-05 11:34 UTC #7 lore raw

Quoted 5 lines

Quoted 2 lines
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?

Signature · 1 line

  Henrique Holschuh

Andy Shevchenko

2018-02-05 13:08 UTC #8 lore raw

Quoted 9 lines · Henrique de Moraes Holschuh

Quoted 5 lines
Quoted 2 lines
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...

Quoted 6 lines

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

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Matthew Thode

2018-02-05 14:27 UTC #9 lore raw

Quoted 16 lines · Andy Shevchenko

Quoted 13 lines · Henrique de Moraes Holschuh
Quoted 5 lines
Quoted 2 lines
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.

Quoted 5 lines

Quoted 1 line
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.

Signature · 1 line

Matthew Thode