diff mbox

[1/2] platform/x86: thinkpad_acpi: guard generic hotkey case

Message ID 20170225182030.19232-2-ckellner@redhat.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Christian Kellner Feb. 25, 2017, 6:20 p.m. UTC 
Currently when dispatching hotkeys we check if the scancode is in
the range of 0 and TPACPI_HOTKEY_MAP_LEN, although the bottom 20
entries in the hotkey keymap are already adaptive keycodes.
Therefore we introduce a TP_ACPI_HOTKEYSCAN_ADAPTIVE_START and
ensure that we are in the range 0 and ADAPTIVE_START for the generic
keycode case.

Signed-off-by: Christian Kellner <ckellner@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Henrique de Moraes Holschuh Feb. 26, 2017, 6:44 p.m. UTC | #1 
On Sat, 25 Feb 2017, Christian Kellner wrote:
> @@ -1923,7 +1923,8 @@ enum {	/* hot key scan codes (derived from ACPI DSDT) */
>  	TP_ACPI_HOTKEYSCAN_UNK7,
>  	TP_ACPI_HOTKEYSCAN_UNK8,
>  
> -	TP_ACPI_HOTKEYSCAN_MUTE2,
> +	TP_ACPI_HOTKEYSCAN_ADAPTIVE_START = 32,
> +	TP_ACPI_HOTKEYSCAN_MUTE2 = 32,
>  	TP_ACPI_HOTKEYSCAN_BRIGHTNESS_ZERO,
>  	TP_ACPI_HOTKEYSCAN_CLIPPING_TOOL,
>  	TP_ACPI_HOTKEYSCAN_CLOUD,

This works, but...

enum {
	TP_ACPI_HOTKEYSCAN_ADAPTIVE_START,
	TP_ACPI_HOTKEYSCAN_MUTE2 = TP_ACPI_HOTKEYSCAN_ADAPTIVE_START,
	TP_ACPI_HOTKEYSCAN_BRIGHTNESS_ZERO,
...
} 

or something to that effect might be better.  Either that or an
compile-time assert that the block boundaries are where we expect them
to be.

Feel free to use formatting tricks with whitespace to make it create
visual blocks in the enum {}  definition, or use comments as spacers...
;-)

> @@ -3657,7 +3658,6 @@ static const int adaptive_keyboard_modes[] = {
>  #define DFR_CHANGE_ROW			0x101
>  #define DFR_SHOW_QUICKVIEW_ROW		0x102
>  #define FIRST_ADAPTIVE_KEY		0x103
> -#define ADAPTIVE_KEY_OFFSET		0x020
>  
>  /* press Fn key a while second, it will switch to Function Mode. Then
>   * release Fn key, previous mode be restored.
> @@ -3748,12 +3748,13 @@ static bool adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode)
>  	default:
>  		if (scancode < FIRST_ADAPTIVE_KEY ||
>  		    scancode >= FIRST_ADAPTIVE_KEY + TPACPI_HOTKEY_MAP_LEN -
> -				ADAPTIVE_KEY_OFFSET) {
> +				TP_ACPI_HOTKEYSCAN_ADAPTIVE_START) {
>  			pr_info("Unhandled adaptive keyboard key: 0x%x\n",
>  					scancode);
>  			return false;
>  		}
> -		keycode = hotkey_keycode_map[scancode - FIRST_ADAPTIVE_KEY + ADAPTIVE_KEY_OFFSET];
> +		keycode = hotkey_keycode_map[scancode - FIRST_ADAPTIVE_KEY +
> +					     TP_ACPI_HOTKEYSCAN_ADAPTIVE_START];
>  		if (keycode != KEY_RESERVED) {
>  			mutex_lock(&tpacpi_inputdev_send_mutex);
>  
> @@ -3779,7 +3780,7 @@ static bool hotkey_notify_hotkey(const u32 hkey,
>  	*ignore_acpi_ev = false;
>  
>  	/* HKEY event 0x1001 is scancode 0x00 */
> -	if (scancode > 0 && scancode <= TPACPI_HOTKEY_MAP_LEN) {
> +	if (scancode > 0 && scancode <= TP_ACPI_HOTKEYSCAN_ADAPTIVE_START) {
>  		scancode--;
>  		if (!(hotkey_source_mask & (1 << scancode))) {
>  			tpacpi_input_send_key_masked(scancode);

Other than that, I like the idea.
diff mbox

Patch

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index cacb43fb1df7..83294b28933e 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -1923,7 +1923,8 @@  enum {	/* hot key scan codes (derived from ACPI DSDT) */
 	TP_ACPI_HOTKEYSCAN_UNK7,
 	TP_ACPI_HOTKEYSCAN_UNK8,
 
-	TP_ACPI_HOTKEYSCAN_MUTE2,
+	TP_ACPI_HOTKEYSCAN_ADAPTIVE_START = 32,
+	TP_ACPI_HOTKEYSCAN_MUTE2 = 32,
 	TP_ACPI_HOTKEYSCAN_BRIGHTNESS_ZERO,
 	TP_ACPI_HOTKEYSCAN_CLIPPING_TOOL,
 	TP_ACPI_HOTKEYSCAN_CLOUD,
@@ -3657,7 +3658,6 @@  static const int adaptive_keyboard_modes[] = {
 #define DFR_CHANGE_ROW			0x101
 #define DFR_SHOW_QUICKVIEW_ROW		0x102
 #define FIRST_ADAPTIVE_KEY		0x103
-#define ADAPTIVE_KEY_OFFSET		0x020
 
 /* press Fn key a while second, it will switch to Function Mode. Then
  * release Fn key, previous mode be restored.
@@ -3748,12 +3748,13 @@  static bool adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode)
 	default:
 		if (scancode < FIRST_ADAPTIVE_KEY ||
 		    scancode >= FIRST_ADAPTIVE_KEY + TPACPI_HOTKEY_MAP_LEN -
-				ADAPTIVE_KEY_OFFSET) {
+				TP_ACPI_HOTKEYSCAN_ADAPTIVE_START) {
 			pr_info("Unhandled adaptive keyboard key: 0x%x\n",
 					scancode);
 			return false;
 		}
-		keycode = hotkey_keycode_map[scancode - FIRST_ADAPTIVE_KEY + ADAPTIVE_KEY_OFFSET];
+		keycode = hotkey_keycode_map[scancode - FIRST_ADAPTIVE_KEY +
+					     TP_ACPI_HOTKEYSCAN_ADAPTIVE_START];
 		if (keycode != KEY_RESERVED) {
 			mutex_lock(&tpacpi_inputdev_send_mutex);
 
@@ -3779,7 +3780,7 @@  static bool hotkey_notify_hotkey(const u32 hkey,
 	*ignore_acpi_ev = false;
 
 	/* HKEY event 0x1001 is scancode 0x00 */
-	if (scancode > 0 && scancode <= TPACPI_HOTKEY_MAP_LEN) {
+	if (scancode > 0 && scancode <= TP_ACPI_HOTKEYSCAN_ADAPTIVE_START) {
 		scancode--;
 		if (!(hotkey_source_mask & (1 << scancode))) {
 			tpacpi_input_send_key_masked(scancode);