diff mbox series

[v2,15/24] platform/x86: thinkpad_acpi: Do not send ACPI netlink events for unknown hotkeys

Message ID 20240424122834.19801-16-hdegoede@redhat.com (mailing list archive)
State Accepted, archived
Headers show
Series platform/x86: thinkpad_acpi: Refactor hotkey handling and add support for some new hotkeys | expand

Commit Message

Hans de Goede April 24, 2024, 12:28 p.m. UTC
Do not send ACPI netlink events for unknown hotkeys, to avoid userspace
starting to rely on them. Instead these should be added to the keymap to
send evdev events.

This should not cause a behavior change for existing laptop models since
all currently known 0x1xxx events have a mapping.

In hindsight the ACPI netlink events should have been suppressed for
the adaptive keyboard and extended hotkeys events too. But the kernel has
been sending ACPI netlink events for those for a long time now, so we
cannot just stop sending them without potentially causing issues for
existing users who may depend on these.

Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Ilpo Järvinen April 25, 2024, 8:51 a.m. UTC | #1
On Wed, 24 Apr 2024, Hans de Goede wrote:

> Do not send ACPI netlink events for unknown hotkeys, to avoid userspace
> starting to rely on them. Instead these should be added to the keymap to
> send evdev events.
> 
> This should not cause a behavior change for existing laptop models since
> all currently known 0x1xxx events have a mapping.
> 
> In hindsight the ACPI netlink events should have been suppressed for
> the adaptive keyboard and extended hotkeys events too. But the kernel has
> been sending ACPI netlink events for those for a long time now, so we
> cannot just stop sending them without potentially causing issues for
> existing users who may depend on these.
> 
> Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 9b38770412ab..952bac635a18 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -2250,7 +2250,7 @@ static void tpacpi_input_send_tabletsw(void)
>  	}
>  }
>  
> -static bool tpacpi_input_send_key(const u32 hkey)
> +static bool tpacpi_input_send_key(const u32 hkey, bool *send_acpi_ev)
>  {
>  	unsigned int keycode, scancode;
>  
> @@ -2271,6 +2271,14 @@ static bool tpacpi_input_send_key(const u32 hkey)
>  		scancode = hkey - TP_HKEY_EV_EXTENDED_KEY_START +
>  			   TP_ACPI_HOTKEYSCAN_EXTENDED_START;
>  	} else {
> +		/*
> +		 * Do not send ACPI netlink events for unknown hotkeys, to
> +		 * avoid userspace starting to rely on them. Instead these
> +		 * should be added to the keymap to send evdev events.
> +		 */
> +		if (send_acpi_ev)
> +			*send_acpi_ev = false;

Hi,

I'd consider dropping the check and passing a pointer to dummy variable 
instead of NULL, and comment at the caller defining the dummy variable why 
the value is getting ignored. But you know this code better than me so I 
leave it up to you what you want to do.

> +
>  		return false;
>  	}
>  
> @@ -2298,7 +2306,7 @@ static struct tp_acpi_drv_struct ibm_hotkey_acpidriver;
>  /* Do NOT call without validating scancode first */
>  static void tpacpi_hotkey_send_key(unsigned int scancode)
>  {
> -	tpacpi_input_send_key(TP_HKEY_EV_ORIG_KEY_START + scancode);
> +	tpacpi_input_send_key(TP_HKEY_EV_ORIG_KEY_START + scancode, NULL);
>  }
diff mbox series

Patch

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 9b38770412ab..952bac635a18 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2250,7 +2250,7 @@  static void tpacpi_input_send_tabletsw(void)
 	}
 }
 
-static bool tpacpi_input_send_key(const u32 hkey)
+static bool tpacpi_input_send_key(const u32 hkey, bool *send_acpi_ev)
 {
 	unsigned int keycode, scancode;
 
@@ -2271,6 +2271,14 @@  static bool tpacpi_input_send_key(const u32 hkey)
 		scancode = hkey - TP_HKEY_EV_EXTENDED_KEY_START +
 			   TP_ACPI_HOTKEYSCAN_EXTENDED_START;
 	} else {
+		/*
+		 * Do not send ACPI netlink events for unknown hotkeys, to
+		 * avoid userspace starting to rely on them. Instead these
+		 * should be added to the keymap to send evdev events.
+		 */
+		if (send_acpi_ev)
+			*send_acpi_ev = false;
+
 		return false;
 	}
 
@@ -2298,7 +2306,7 @@  static struct tp_acpi_drv_struct ibm_hotkey_acpidriver;
 /* Do NOT call without validating scancode first */
 static void tpacpi_hotkey_send_key(unsigned int scancode)
 {
-	tpacpi_input_send_key(TP_HKEY_EV_ORIG_KEY_START + scancode);
+	tpacpi_input_send_key(TP_HKEY_EV_ORIG_KEY_START + scancode, NULL);
 }
 
 static void hotkey_read_nvram(struct tp_nvram_state *n, const u32 m)
@@ -3734,7 +3742,7 @@  static bool hotkey_notify_hotkey(const u32 hkey, bool *send_acpi_ev)
 			return true;
 	}
 
-	return tpacpi_input_send_key(hkey);
+	return tpacpi_input_send_key(hkey, send_acpi_ev);
 }
 
 /* 0x2000-0x2FFF: Wakeup reason */