Skip to contentSkip to conversation

dell_wmi: XPS 13 Skylake support and misc stuff

PATCH Superseded Andy Lutomirski 6 participants 35 messages Nov 14, 2015 – Jan 22, 2016 1 message not in the archive x86 platform drivers

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

cover dell_wmi: XPS 13 Skylake support and misc stuff

The first patch adds three missing hotkeys. The second and third patches are just cleanup and future-proofing.

This is obviously rather late (I just got this laptop working at all two days ago), but it would be nice if patch 1 could squeeze in to 4.4. That way 4.4 would fully support this laptop. (4.3 is needed for graphics to work right, and 4.4 will support the wireless chip, but as it stands, Linus' tree is missing support for the wireless button.)

Andy Lutomirski (3):
dell_wmi: Support new hotkeys on the XPS 13 Skylake
dell_wmi: Use a C99-style array for bios_to_linux_keycode
dell_wmi: Improve unknown hotkey handling

 drivers/platform/x86/dell-wmi.c | 114 ++++++++++++++++++++++++++++++----------
 1 file changed, 86 insertions(+), 28 deletions(-)
Signature · 1 line
2.5.0
#1 lore raw

PATCH 1/3 dell_wmi: Support new hotkeys on the XPS 13 Skylake

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

This also adds the Fn-lock key as a KE_IGNORE entry.

---
 drivers/platform/x86/dell-wmi.c | 48 +++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +41 -7
index f2d77fe696ac..5be1abec4f64 100644
@@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
 	0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
 };
 
+/* These are applied if the hk table is present and doesn't override them. */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+	/* Fn-lock -- no action is required by the kernel. */
+	{ KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+	/* Keys that need our help (on XPS 13 Skylake and maybe others. */
+	{ KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
+	{ KE_KEY, 0x153, { KEY_RFKILL } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
 static void dell_wmi_process_key(int reported_key)
@@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 	int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
 				sizeof(struct dell_bios_keymap_entry);
 	struct key_entry *keymap;
-	int i;
+	int i, pos = 0, num_bios_keys;
 
-	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),
+			 sizeof(struct key_entry), GFP_KERNEL);
 	if (!keymap)
 		return NULL;
 
@@ -314,14 +325,37 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 				    KEY_RESERVED;
 
 		if (keycode == KEY_KBDILLUMTOGGLE)
-			keymap[i].type = KE_IGNORE;
+			keymap[pos].type = KE_IGNORE;
 		else
-			keymap[i].type = KE_KEY;
-		keymap[i].code = bios_entry->scancode;
-		keymap[i].keycode = keycode;
+			keymap[pos].type = KE_KEY;
+		keymap[pos].code = bios_entry->scancode;
+		keymap[pos].keycode = keycode;
+
+		pos++;
+	}
+
+	num_bios_keys = pos;
+
+	for (i = 0; i < ARRAY_SIZE(dell_wmi_extra_keymap); i++) {
+		int j;
+
+		/*
+		 * Check if we've already found this scancode.  This takes
+		 * quadratic time, but it doesn't matter unless the list
+		 * of extra keys gets very long.
+		 */
+		for (j = 0; j < num_bios_keys; j++)
+			if (keymap[j].code == dell_wmi_extra_keymap[i].code)
+				goto skip;
+
+		keymap[pos] = dell_wmi_extra_keymap[i];
+		pos++;
+
+skip:
+		;
 	}
 
-	keymap[hotkey_num].type = KE_END;
+	keymap[pos].type = KE_END;
 
 	return keymap;
 }
Signature · 1 line
2.5.0
#22 lore raw

PATCH 2/3 dell_wmi: Use a C99-style array for bios_to_linux_keycode

In reply to Andy Lutomirski

It's currently hard to follow what maps to what, and it's hard to edit the array. Redo it as a C99-style array.

I generated this using emacs regexes and a python one-liner.

---
 drivers/platform/x86/dell-wmi.c | 61 +++++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 21 deletions(-)
drivers/platform/x86/dell-wmi.c +40 -21
index 5be1abec4f64..92b0149fa4a7 100644
@@ -119,27 +119,46 @@ struct dell_bios_hotkey_table {
 static const struct dell_bios_hotkey_table *dell_bios_hotkey_table;
 
 static const u16 bios_to_linux_keycode[256] __initconst = {
-
-	KEY_MEDIA,	KEY_NEXTSONG,	KEY_PLAYPAUSE, KEY_PREVIOUSSONG,
-	KEY_STOPCD,	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_UNKNOWN,
-	KEY_WWW,	KEY_UNKNOWN,	KEY_VOLUMEDOWN, KEY_MUTE,
-	KEY_VOLUMEUP,	KEY_UNKNOWN,	KEY_BATTERY,	KEY_EJECTCD,
-	KEY_UNKNOWN,	KEY_SLEEP,	KEY_PROG1, KEY_BRIGHTNESSDOWN,
-	KEY_BRIGHTNESSUP,	KEY_UNKNOWN,	KEY_KBDILLUMTOGGLE,
-	KEY_UNKNOWN,	KEY_SWITCHVIDEOMODE,	KEY_UNKNOWN, KEY_UNKNOWN,
-	KEY_SWITCHVIDEOMODE,	KEY_UNKNOWN,	KEY_UNKNOWN, KEY_PROG2,
-	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_UNKNOWN,
-	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_MICMUTE,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
+	[0]	= KEY_MEDIA,
+	[1]	= KEY_NEXTSONG,
+	[2]	= KEY_PLAYPAUSE,
+	[3]	= KEY_PREVIOUSSONG,
+	[4]	= KEY_STOPCD,
+	[5]	= KEY_UNKNOWN,
+	[6]	= KEY_UNKNOWN,
+	[7]	= KEY_UNKNOWN,
+	[8]	= KEY_WWW,
+	[9]	= KEY_UNKNOWN,
+	[10]	= KEY_VOLUMEDOWN,
+	[11]	= KEY_MUTE,
+	[12]	= KEY_VOLUMEUP,
+	[13]	= KEY_UNKNOWN,
+	[14]	= KEY_BATTERY,
+	[15]	= KEY_EJECTCD,
+	[16]	= KEY_UNKNOWN,
+	[17]	= KEY_SLEEP,
+	[18]	= KEY_PROG1,
+	[19]	= KEY_BRIGHTNESSDOWN,
+	[20]	= KEY_BRIGHTNESSUP,
+	[21]	= KEY_UNKNOWN,
+	[22]	= KEY_KBDILLUMTOGGLE,
+	[23]	= KEY_UNKNOWN,
+	[24]	= KEY_SWITCHVIDEOMODE,
+	[25]	= KEY_UNKNOWN,
+	[26]	= KEY_UNKNOWN,
+	[27]	= KEY_SWITCHVIDEOMODE,
+	[28]	= KEY_UNKNOWN,
+	[29]	= KEY_UNKNOWN,
+	[30]	= KEY_PROG2,
+	[31]	= KEY_UNKNOWN,
+	[32]	= KEY_UNKNOWN,
+	[33]	= KEY_UNKNOWN,
+	[34]	= KEY_UNKNOWN,
+	[35]	= KEY_UNKNOWN,
+	[36]	= KEY_UNKNOWN,
+	[37]	= KEY_UNKNOWN,
+	[38]	= KEY_MICMUTE,
+	[255]	= KEY_PROG3,
 };
 
 /* These are applied if the hk table is present and doesn't override them. */
Signature · 1 line
2.5.0
#28 lore raw

PATCH 3/3 dell_wmi: Improve unknown hotkey handling

In reply to Andy Lutomirski

If DMI lists a hotkey that we don't recognize, log and ignore it instead of trying to map it to keycode 0. I haven't seen this happen, but it will help maintain the key map in the future and it will help avoid sending bogus events.

This also improves the message that we log when we get an unknown key event.

---
 drivers/platform/x86/dell-wmi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/platform/x86/dell-wmi.c +6 -1
index 92b0149fa4a7..e43228a35f6b 100644
@@ -180,7 +180,7 @@ static void dell_wmi_process_key(int reported_key)
 	key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
 						reported_key);
 	if (!key) {
-		pr_info("Unknown key %x pressed\n", reported_key);
+		pr_info("Unknown key with scancode 0x%x pressed\n", reported_key);
 		return;
 	}
 
@@ -343,6 +343,11 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 				    bios_to_linux_keycode[bios_entry->keycode] :
 				    KEY_RESERVED;
 
+		if (keycode == 0) {
+			pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);
+			continue;
+		}
+
 		if (keycode == KEY_KBDILLUMTOGGLE)
 			keymap[pos].type = KE_IGNORE;
 		else
Signature · 1 line
2.5.0
#2 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Andy Lutomirski
Quoted 6 lines · On Friday 13 November 2015 21:49:30 Andy Lutomirski wrote:

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

Do you have any (Dell) documentation which specify list of these wmi codes send to dell-wmi driver?

Quoted 13 lines

This also adds the Fn-lock key as a KE_IGNORE entry.

---
 drivers/platform/x86/dell-wmi.c | 48 +++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +0 -0
index f2d77fe696ac..5be1abec4f64 100644
@@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
 	0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3

drivers/platform/x86/dell-wmi.c static const u16 bios_to_linux_keycode[256] __initconst = { · line 152

Quoted 10 lines
drivers/platform/x86/dell-wmi.c +8 -0
 };
 
+/* These are applied if the hk table is present and doesn't override them. */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+	/* Fn-lock -- no action is required by the kernel. */
+	{ KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+	/* Keys that need our help (on XPS 13 Skylake and maybe others. */
+	{ KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
+	{ KE_KEY, 0x153, { KEY_RFKILL } },

On more Dell laptops rfkill events are handed by ACPI driver dell-rbtn.ko. Are you sure that dell-rbtn.ko does not send keypress event and you really need it from dell-wmi? We already masked KEY_RFKILL in dell-wmi to prevent double events...

Quoted 38 lines
drivers/platform/x86/dell-wmi.c +16 -6
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
 static void dell_wmi_process_key(int reported_key)
@@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 	int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
 				sizeof(struct dell_bios_keymap_entry);
 	struct key_entry *keymap;
-	int i;
+	int i, pos = 0, num_bios_keys;
 
-	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),
+			 sizeof(struct key_entry), GFP_KERNEL);
 	if (!keymap)
 		return NULL;
 
@@ -314,14 +325,37 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 				    KEY_RESERVED;
 
 		if (keycode == KEY_KBDILLUMTOGGLE)
-			keymap[i].type = KE_IGNORE;
+			keymap[pos].type = KE_IGNORE;
 		else
-			keymap[i].type = KE_KEY;
-		keymap[i].code = bios_entry->scancode;
-		keymap[i].keycode = keycode;
+			keymap[pos].type = KE_KEY;
+		keymap[pos].code = bios_entry->scancode;
+		keymap[pos].keycode = keycode;
+
+		pos++;
+	}
+
+	num_bios_keys = pos;
+
+	for (i = 0; i < ARRAY_SIZE(dell_wmi_extra_keymap); i++) {

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 349

Quoted 10 lines
drivers/platform/x86/dell-wmi.c +10 -0
+		int j;
+
+		/*
+		 * Check if we've already found this scancode.  This takes
+		 * quadratic time, but it doesn't matter unless the list
+		 * of extra keys gets very long.
+		 */
+		for (j = 0; j < num_bios_keys; j++)
+			if (keymap[j].code == dell_wmi_extra_keymap[i].code)
+				goto skip;

Rather move this code into separate boolean function and for return value here. This will prevent using hacky goto...

Quoted 13 lines
drivers/platform/x86/dell-wmi.c +7 -1
+
+		keymap[pos] = dell_wmi_extra_keymap[i];
+		pos++;
+
+skip:
+		;
 	}
 
-	keymap[hotkey_num].type = KE_END;
+	keymap[pos].type = KE_END;
 
 	return keymap;
 }
Signature · 2 lines
Pali Rohár
[email protected]
Acked-by on v1 2/3
#23 lore raw

Re: [PATCH 2/3] dell_wmi: Use a C99-style array for bios_to_linux_keycode

In reply to Andy Lutomirski
Quoted 6 lines · On Friday 13 November 2015 21:49:31 Andy Lutomirski wrote:

It's currently hard to follow what maps to what, and it's hard to edit the array. Redo it as a C99-style array.

I generated this using emacs regexes and a python one-liner.

There are no function changes in this patch, so

Signature · 2 lines
Pali Rohár
[email protected]
#29 lore raw

Re: [PATCH 3/3] dell_wmi: Improve unknown hotkey handling

In reply to Andy Lutomirski
Quoted 27 lines · On Friday 13 November 2015 21:49:32 Andy Lutomirski wrote:

If DMI lists a hotkey that we don't recognize, log and ignore it instead of trying to map it to keycode 0. I haven't seen this happen, but it will help maintain the key map in the future and it will help avoid sending bogus events.

This also improves the message that we log when we get an unknown key event.

---
 drivers/platform/x86/dell-wmi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/platform/x86/dell-wmi.c +1 -1
index 92b0149fa4a7..e43228a35f6b 100644
@@ -180,7 +180,7 @@ static void dell_wmi_process_key(int reported_key)
 	key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
 						reported_key);
 	if (!key) {
-		pr_info("Unknown key %x pressed\n", reported_key);
+		pr_info("Unknown key with scancode 0x%x pressed\n", reported_key);
 		return;
 	}
 

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 350

Quoted 8 lines
drivers/platform/x86/dell-wmi.c +5 -0
@@ -343,6 +343,11 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 				    bios_to_linux_keycode[bios_entry->keycode] :
 				    KEY_RESERVED;
 
+		if (keycode == 0) {
+			pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);
+			continue;
+		}
+

This line is too long and checkpatch.pl will probably blame you.

Anyway, have you already found some missing mapping which comes from bios/firmware (because your patches do not change that bios table)?

Quoted 3 lines
drivers/platform/x86/dell-wmi.c +0 -0
 		if (keycode == KEY_KBDILLUMTOGGLE)
 			keymap[pos].type = KE_IGNORE;
 		else
Signature · 2 lines
Pali Rohár
[email protected]
#3 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Pali Rohár
Quoted 9 lines · Pali Rohár
Quoted 6 lines · On Friday 13 November 2015 21:49:30 Andy Lutomirski wrote:

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

Quoted 3 lines

Do you have any (Dell) documentation which specify list of these wmi codes send to dell-wmi driver?

No. Do you know where to get that documentation?

Quoted 15 lines
Quoted 15 lines

This also adds the Fn-lock key as a KE_IGNORE entry.

---
 drivers/platform/x86/dell-wmi.c | 48 +++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +0 -0
index f2d77fe696ac..5be1abec4f64 100644
@@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
      0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
 };
 
Quoted 14 lines

drivers/platform/x86/dell-wmi.c static const u16 bios_to_linux_keycode[256] __initconst = { · line 152

Quoted 8 lines
drivers/platform/x86/dell-wmi.c +8 -0
+/* These are applied if the hk table is present and doesn't override them. */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+     /* Fn-lock -- no action is required by the kernel. */
+     { KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+     /* Keys that need our help (on XPS 13 Skylake and maybe others. */
+     { KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
+     { KE_KEY, 0x153, { KEY_RFKILL } },

On more Dell laptops rfkill events are handed by ACPI driver dell-rbtn.ko. Are you sure that dell-rbtn.ko does not send keypress event and you really need it from dell-wmi? We already masked KEY_RFKILL in dell-wmi to prevent double events...

Hmm, interesting. I have DELLABC6, not DELLABCE. I'll play around with it a bit. Are there Dell docs for this?

Regardless, we'll need something like this, but maybe with KE_IGNORE, just to silence the warnings.

Quoted 10 lines

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 349

Quoted 9 lines
drivers/platform/x86/dell-wmi.c +9 -0
+
+             /*
+              * Check if we've already found this scancode.  This takes
+              * quadratic time, but it doesn't matter unless the list
+              * of extra keys gets very long.
+              */
+             for (j = 0; j < num_bios_keys; j++)
+                     if (keymap[j].code == dell_wmi_extra_keymap[i].code)
+                             goto skip;
Quoted 3 lines

Rather move this code into separate boolean function and for return value here. This will prevent using hacky goto...

I'll do that in v2.

--Andy

#4 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 8 lines · On Saturday 14 November 2015 16:48:25 Andy Lutomirski wrote:
Quoted 7 lines · Pali Rohár
Quoted 5 lines · On Friday 13 November 2015 21:49:30 Andy Lutomirski wrote:

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

Quoted 5 lines
Quoted 2 lines

Do you have any (Dell) documentation which specify list of these wmi codes send to dell-wmi driver?

No. Do you know where to get that documentation?

Time to time Dell release some documentation or example code. You could ask Dell people on LKML (e.g. Mario Limonciello is active) or on smbios mailing list [email protected].

But currently there there are open questions about WMI hotkeys on Dell Vostro V131 which we cannot fix yet :-(

Quoted 36 lines
Quoted 21 lines
Quoted 21 lines

This also adds the Fn-lock key as a KE_IGNORE entry.

---

 drivers/platform/x86/dell-wmi.c | 48

+++++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/drivers/platform/x86/dell-wmi.c index
f2d77fe696ac..5be1abec4f64 100644

drivers/platform/x86/dell-wmi.c +0 -0
@@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256]
 __initconst = {
 
      0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
 
 };
 
Quoted 14 lines
Quoted 9 lines
drivers/platform/x86/dell-wmi.c +1 -0
+/* These are applied if the hk table is present and doesn't

override them. */ +static const struct key_entry
dell_wmi_extra_keymap[] __initconst = { + /* Fn-lock -- no
action is required by the kernel. */ + { KE_IGNORE, 0x151, {
KEY_RESERVED } },

drivers/platform/x86/dell-wmi.c +2 -0
+
+     /* Keys that need our help (on XPS 13 Skylake and maybe

others. */ + { KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
+ { KE_KEY, 0x153, { KEY_RFKILL } },

On more Dell laptops rfkill events are handed by ACPI driver dell-rbtn.ko. Are you sure that dell-rbtn.ko does not send keypress event and you really need it from dell-wmi? We already masked KEY_RFKILL in dell-wmi to prevent double events...

Quoted 3 lines

Hmm, interesting. I have DELLABC6, not DELLABCE. I'll play around with it a bit. Are there Dell docs for this?

Decompiling ACPI table is documentation :-) Probably DELLABC6 will have similar (or maybe same) ACPI interface. Post relevant ACPI ASL code, maybe I could help with it.

Anyway first version of DELLABCE driver was written by Alex Hung (from Canonical), so it is possible that also non-Dell could help with documentation/behaviour as well.

Quoted 3 lines

Regardless, we'll need something like this, but maybe with KE_IGNORE, just to silence the warnings.

Yes, with KE_IGNORE we will have documented behaviour in code.

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
drivers/platform/x86/dell-wmi.c +3 -0
+
+             /*
+              * Check if we've already found this scancode. 

This takes + * quadratic time, but it doesn't
matter unless the list + * of extra keys gets very
long.

drivers/platform/x86/dell-wmi.c +3 -0
+              */
+             for (j = 0; j < num_bios_keys; j++)
+                     if (keymap[j].code ==

dell_wmi_extra_keymap[i].code) +
goto skip;

Quoted 6 lines
Quoted 2 lines

Rather move this code into separate boolean function and for return value here. This will prevent using hacky goto...

I'll do that in v2.

--Andy

OK.

Signature · 2 lines
Pali Rohár
[email protected]
#30 lore raw

Re: [PATCH 3/3] dell_wmi: Improve unknown hotkey handling

In reply to Pali Rohár
Quoted 31 lines · Pali Rohár
Quoted 30 lines · On Friday 13 November 2015 21:49:32 Andy Lutomirski wrote:

If DMI lists a hotkey that we don't recognize, log and ignore it instead of trying to map it to keycode 0. I haven't seen this happen, but it will help maintain the key map in the future and it will help avoid sending bogus events.

This also improves the message that we log when we get an unknown key event.

---
 drivers/platform/x86/dell-wmi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/platform/x86/dell-wmi.c +1 -1
index 92b0149fa4a7..e43228a35f6b 100644
@@ -180,7 +180,7 @@ static void dell_wmi_process_key(int reported_key)
      key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
                                              reported_key);
      if (!key) {
-             pr_info("Unknown key %x pressed\n", reported_key);
+             pr_info("Unknown key with scancode 0x%x pressed\n", reported_key);
              return;
      }
 
@@ -343,6 +343,11 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
                                  bios_to_linux_keycode[bios_entry->keycode] :
                                  KEY_RESERVED;
 
Quoted 7 lines

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 350

Quoted 5 lines
drivers/platform/x86/dell-wmi.c +5 -0
+             if (keycode == 0) {
+                     pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);
+                     continue;
+             }
+

This line is too long and checkpatch.pl will probably blame you.

I split it. The first bit (with the quoted string) is still a bit above 80, but that's better than splitting the string itself and breaking grep.

Quoted 4 lines

Anyway, have you already found some missing mapping which comes from bios/firmware (because your patches do not change that bios table)?

I don't see a DMI entry that maps to something outside the table, though, so this warning doesn't trigger. My best guess is that Dell simply didn't bother to update the DMI table and has it hardcoded in their driver. Admittedly, I've never played with the official driver at all.

--Andy

#6 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to a message not in the archive
Quoted 36 lines · On Saturday 14 November 2015 18:07:57 Andy Lutomirski wrote:

[lots of people added]

Quoted 12 lines · Pali Rohár
Quoted 11 lines · On Saturday 14 November 2015 16:48:25 Andy Lutomirski wrote:
Quoted 9 lines · Pali Rohár
Quoted 5 lines · On Friday 13 November 2015 21:49:30 Andy Lutomirski wrote:

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

Do you have any (Dell) documentation which specify list of these wmi codes send to dell-wmi driver?

Quoted 8 lines
Quoted 1 line

No. Do you know where to get that documentation?

Time to time Dell release some documentation or example code. You could ask Dell people on LKML (e.g. Mario Limonciello is active) or on smbios mailing list [email protected].

But currently there there are open questions about WMI hotkeys on Dell Vostro V131 which we cannot fix yet :-(

On the Dell XPS 13 Skylake (XPS 13 9350), upstream Linux doesn't support the rfkill button.

There seem to be three WMI events that aren't reflected in the OEM type 178 DMI table:

0x151: Fn-lock (no action needed by OS)
0x152: Switch video mode (should send KEY_SWITCHVIDEOMODE, but currently just warns)
0x153: rfkill -- currently warns, and see below.

Quoted 6 lines

On several Dell models, there's the dell_rbtn (DELRBTN / DELLABCE) device. It's here in the DSDT, too, but it seems to be disabled if _OSI reports "Windows 2012" or "Windows 2013", so _STA returns zero. (It also shows up as DELLRBC6, but I haven't tried all the _OSI hackery that seems to be needed in order to test the driver.)

Hi!

In your ASL code is:

            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                If ((OIDE () < One))
                {
                    Return (0x0F)
                }

                Return (Zero)
            }

OIDE() returns 1 for Windows 8.

This is quite interesting, on my Latitude E6440 is this ASL code:

            Method (_STA, 0, NotSerialized)
            {
                If (LLess (OIDE (), One))
                {
                    Return (Zero)
                }

                Return (0x0F)
            }

And again OIDE() returns 1 for Windows 8. So behaviour is negated.

Can you check if you have latest version of BIOS? Maybe Dell written that condition incorrectly?

Can you try to add "DELLRBC6" into dell-rbtn.c acpi table and boot kernel with acpi_osi="!Windows 2012" acpi_osi="!Windows 2013" what happens?

Quoted 6 lines

My proposal is to modify dell_wmi to handle 0x151 (ignore), 0x152 (send KEY_SWITCHVIDEOMODE), and 0x153 (send KEY_RFKILL), but only if there isn't something else mapped to them in the DMI table.

I've attached dmidecode output and the DSDT.

Can you please provide debug output from dell-wmi module when you press those hotkeys? Specially I want to see wmi buffer for each pressed hotkey. In debug dmesg it starts with "Received WMI event" and "Process buffer". Look into dell-wmi.c source code.

Signature · 2 lines
Pali Rohár
[email protected]
#7 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 73 lines · Pali Rohár
Quoted 36 lines · On Saturday 14 November 2015 18:07:57 Andy Lutomirski wrote:

[lots of people added]

Quoted 12 lines · Pali Rohár
Quoted 11 lines · On Saturday 14 November 2015 16:48:25 Andy Lutomirski wrote:
Quoted 9 lines · Pali Rohár
Quoted 5 lines · On Friday 13 November 2015 21:49:30 Andy Lutomirski wrote:

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

Do you have any (Dell) documentation which specify list of these wmi codes send to dell-wmi driver?

Quoted 8 lines
Quoted 1 line

No. Do you know where to get that documentation?

Time to time Dell release some documentation or example code. You could ask Dell people on LKML (e.g. Mario Limonciello is active) or on smbios mailing list [email protected].

But currently there there are open questions about WMI hotkeys on Dell Vostro V131 which we cannot fix yet :-(

On the Dell XPS 13 Skylake (XPS 13 9350), upstream Linux doesn't support the rfkill button.

There seem to be three WMI events that aren't reflected in the OEM type 178 DMI table:

0x151: Fn-lock (no action needed by OS)
0x152: Switch video mode (should send KEY_SWITCHVIDEOMODE, but currently just warns)
0x153: rfkill -- currently warns, and see below.

Quoted 6 lines

On several Dell models, there's the dell_rbtn (DELRBTN / DELLABCE) device. It's here in the DSDT, too, but it seems to be disabled if _OSI reports "Windows 2012" or "Windows 2013", so _STA returns zero. (It also shows up as DELLRBC6, but I haven't tried all the _OSI hackery that seems to be needed in order to test the driver.)

Hi!

In your ASL code is:

            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                If ((OIDE () < One))
                {
                    Return (0x0F)
                }

                Return (Zero)
            }

OIDE() returns 1 for Windows 8.

This is quite interesting, on my Latitude E6440 is this ASL code:

            Method (_STA, 0, NotSerialized)
            {
                If (LLess (OIDE (), One))
                {
                    Return (Zero)
                }

                Return (0x0F)
            }
Quoted 5 lines

And again OIDE() returns 1 for Windows 8. So behaviour is negated.

Can you check if you have latest version of BIOS? Maybe Dell written that condition incorrectly?

I'll check later on. There's one newer version.

Quoted 4 lines

Can you try to add "DELLRBC6" into dell-rbtn.c acpi table and boot kernel with acpi_osi="!Windows 2012" acpi_osi="!Windows 2013" what happens?

I did exactly that. The dell_rbtn driver partially worked, but, when I pushed the rfkill button to turn off wireless and then turned it back on using NetworkManager's menu, NM thought it was back on but it didn't seem to work until I pushed the rfkill button again. dell-rbtn does very strange things with device creation and deletion, and maybe that's related.

Quoted 11 lines
Quoted 6 lines

My proposal is to modify dell_wmi to handle 0x151 (ignore), 0x152 (send KEY_SWITCHVIDEOMODE), and 0x153 (send KEY_RFKILL), but only if there isn't something else mapped to them in the DMI table.

I've attached dmidecode output and the DSDT.

Can you please provide debug output from dell-wmi module when you press those hotkeys? Specially I want to see wmi buffer for each pressed hotkey. In debug dmesg it starts with "Received WMI event" and "Process buffer". Look into dell-wmi.c source code.

It looks like this:

[ 7822.601910] dell_wmi: Received WMI event (02 00 10 00 53 01 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00)
[ 7822.601913] dell_wmi: Process buffer (02 00 10 00 53 01)
[ 7822.601915] dell_wmi: Key 153 pressed

--Andy

#8 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 74 lines · Andy Lutomirski
Quoted 73 lines · Pali Rohár
Quoted 36 lines · On Saturday 14 November 2015 18:07:57 Andy Lutomirski wrote:

[lots of people added]

Quoted 12 lines · Pali Rohár
Quoted 11 lines · On Saturday 14 November 2015 16:48:25 Andy Lutomirski wrote:
Quoted 9 lines · Pali Rohár
Quoted 5 lines · On Friday 13 November 2015 21:49:30 Andy Lutomirski wrote:

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

Do you have any (Dell) documentation which specify list of these wmi codes send to dell-wmi driver?

Quoted 8 lines
Quoted 1 line

No. Do you know where to get that documentation?

Time to time Dell release some documentation or example code. You could ask Dell people on LKML (e.g. Mario Limonciello is active) or on smbios mailing list [email protected].

But currently there there are open questions about WMI hotkeys on Dell Vostro V131 which we cannot fix yet :-(

On the Dell XPS 13 Skylake (XPS 13 9350), upstream Linux doesn't support the rfkill button.

There seem to be three WMI events that aren't reflected in the OEM type 178 DMI table:

0x151: Fn-lock (no action needed by OS)
0x152: Switch video mode (should send KEY_SWITCHVIDEOMODE, but currently just warns)
0x153: rfkill -- currently warns, and see below.

Quoted 6 lines

On several Dell models, there's the dell_rbtn (DELRBTN / DELLABCE) device. It's here in the DSDT, too, but it seems to be disabled if _OSI reports "Windows 2012" or "Windows 2013", so _STA returns zero. (It also shows up as DELLRBC6, but I haven't tried all the _OSI hackery that seems to be needed in order to test the driver.)

Hi!

In your ASL code is:

            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                If ((OIDE () < One))
                {
                    Return (0x0F)
                }

                Return (Zero)
            }

OIDE() returns 1 for Windows 8.

This is quite interesting, on my Latitude E6440 is this ASL code:

            Method (_STA, 0, NotSerialized)
            {
                If (LLess (OIDE (), One))
                {
                    Return (Zero)
                }

                Return (0x0F)
            }
Quoted 7 lines
Quoted 5 lines

And again OIDE() returns 1 for Windows 8. So behaviour is negated.

Can you check if you have latest version of BIOS? Maybe Dell written that condition incorrectly?

I'll check later on. There's one newer version.

No relevant changes in 1.0.4.

--Andy

#9 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Andy Lutomirski
Quoted 5 lines · Andy Lutomirski

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

Consider this withdrawn entirely for now. Apparently there's an entirely new mechanism for this in the era of Windows 10.

--Andy

Quoted 88 lines

This also adds the Fn-lock key as a KE_IGNORE entry.

---
 drivers/platform/x86/dell-wmi.c | 48 +++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +41 -8
index f2d77fe696ac..5be1abec4f64 100644
@@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
        0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
 };
 
+/* These are applied if the hk table is present and doesn't override them. */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+       /* Fn-lock -- no action is required by the kernel. */
+       { KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+       /* Keys that need our help (on XPS 13 Skylake and maybe others. */
+       { KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
+       { KE_KEY, 0x153, { KEY_RFKILL } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
 static void dell_wmi_process_key(int reported_key)
@@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
        int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
                                sizeof(struct dell_bios_keymap_entry);
        struct key_entry *keymap;
-       int i;
+       int i, pos = 0, num_bios_keys;
 
-       keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+       keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),
+                        sizeof(struct key_entry), GFP_KERNEL);
        if (!keymap)
                return NULL;
 
@@ -314,14 +325,37 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
                                    KEY_RESERVED;
 
                if (keycode == KEY_KBDILLUMTOGGLE)
-                       keymap[i].type = KE_IGNORE;
+                       keymap[pos].type = KE_IGNORE;
                else
-                       keymap[i].type = KE_KEY;
-               keymap[i].code = bios_entry->scancode;
-               keymap[i].keycode = keycode;
+                       keymap[pos].type = KE_KEY;
+               keymap[pos].code = bios_entry->scancode;
+               keymap[pos].keycode = keycode;
+
+               pos++;
+       }
+
+       num_bios_keys = pos;
+
+       for (i = 0; i < ARRAY_SIZE(dell_wmi_extra_keymap); i++) {
+               int j;
+
+               /*
+                * Check if we've already found this scancode.  This takes
+                * quadratic time, but it doesn't matter unless the list
+                * of extra keys gets very long.
+                */
+               for (j = 0; j < num_bios_keys; j++)
+                       if (keymap[j].code == dell_wmi_extra_keymap[i].code)
+                               goto skip;
+
+               keymap[pos] = dell_wmi_extra_keymap[i];
+               pos++;
+
+skip:
+               ;
        }
 
-       keymap[hotkey_num].type = KE_END;
+       keymap[pos].type = KE_END;
 
        return keymap;
 }
--

2.5.0

Signature · 2 lines
Andy Lutomirski
AMA Capital Management, LLC
#10 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 10 lines · On Wednesday 18 November 2015 13:24:33 Andy Lutomirski wrote:
Quoted 5 lines · Andy Lutomirski

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

Consider this withdrawn entirely for now. Apparently there's an entirely new mechanism for this in the era of Windows 10.

Can you provide more information? I'm planning to cleanup dell-wmi.c driver and if there are needed changes for Windows 10, please let me know what... Thanks!

Signature · 2 lines
Pali Rohár
[email protected]
#11 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Andy Lutomirski
Quoted 35 lines · Andy Lutomirski

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

This also adds the Fn-lock key as a KE_IGNORE entry.

---
 drivers/platform/x86/dell-wmi.c | 48 +++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +10 -0
index f2d77fe696ac..5be1abec4f64 100644
@@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
 	0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
 };
 
+/* These are applied if the hk table is present and doesn't override them. */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+	/* Fn-lock -- no action is required by the kernel. */
+	{ KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+	/* Keys that need our help (on XPS 13 Skylake and maybe others. */
+	{ KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
+	{ KE_KEY, 0x153, { KEY_RFKILL } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
 static void dell_wmi_process_key(int reported_key)

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 313

Quoted 5 lines
drivers/platform/x86/dell-wmi.c +1 -1
@@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 	int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
 				sizeof(struct dell_bios_keymap_entry);
 	struct key_entry *keymap;
-	int i;
+	int i, pos = 0, num_bios_keys;

Just a nit, "reverse christmas tree" order (longest line length first) please. (only if you resend after this review for other reasons)

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 315

Quoted 3 lines
drivers/platform/x86/dell-wmi.c +1 -1
-	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),

This previously allocated kotkey_num + 1, but you dropeed the +1, making it eactly the size of hotkey_num + the new entries you added.

Why don't we need the +1 anymore? (it isn't clear to me why we needed to before actually, but I want to confirm you considered it).

Thanks,

Quoted 55 lines
drivers/platform/x86/dell-wmi.c +29 -5
+			 sizeof(struct key_entry), GFP_KERNEL);
 	if (!keymap)
 		return NULL;
 
@@ -314,14 +325,37 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 				    KEY_RESERVED;
 
 		if (keycode == KEY_KBDILLUMTOGGLE)
-			keymap[i].type = KE_IGNORE;
+			keymap[pos].type = KE_IGNORE;
 		else
-			keymap[i].type = KE_KEY;
-		keymap[i].code = bios_entry->scancode;
-		keymap[i].keycode = keycode;
+			keymap[pos].type = KE_KEY;
+		keymap[pos].code = bios_entry->scancode;
+		keymap[pos].keycode = keycode;
+
+		pos++;
+	}
+
+	num_bios_keys = pos;
+
+	for (i = 0; i < ARRAY_SIZE(dell_wmi_extra_keymap); i++) {
+		int j;
+
+		/*
+		 * Check if we've already found this scancode.  This takes
+		 * quadratic time, but it doesn't matter unless the list
+		 * of extra keys gets very long.
+		 */
+		for (j = 0; j < num_bios_keys; j++)
+			if (keymap[j].code == dell_wmi_extra_keymap[i].code)
+				goto skip;
+
+		keymap[pos] = dell_wmi_extra_keymap[i];
+		pos++;
+
+skip:
+		;
 	}
 
-	keymap[hotkey_num].type = KE_END;
+	keymap[pos].type = KE_END;
 
 	return keymap;
 }
Signature · 6 lines
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#24 lore raw

Re: [PATCH 2/3] dell_wmi: Use a C99-style array for bios_to_linux_keycode

In reply to Andy Lutomirski
Quoted 6 lines · Andy Lutomirski

It's currently hard to follow what maps to what, and it's hard to edit the array. Redo it as a C99-style array.

I generated this using emacs regexes and a python one-liner.

Indeed, this is much nicer.

Please include all the maintainers listed in MAINTAINERS in the future for a faster response.

Pali or Matthew, do either of you care to comment? I'm queueing to testing, will merge to next shortly if I don't hear from you.

Thanks,

Quoted 84 lines
---
 drivers/platform/x86/dell-wmi.c | 61 +++++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 21 deletions(-)
drivers/platform/x86/dell-wmi.c +40 -21
index 5be1abec4f64..92b0149fa4a7 100644
@@ -119,27 +119,46 @@ struct dell_bios_hotkey_table {
 static const struct dell_bios_hotkey_table *dell_bios_hotkey_table;
 
 static const u16 bios_to_linux_keycode[256] __initconst = {
-
-	KEY_MEDIA,	KEY_NEXTSONG,	KEY_PLAYPAUSE, KEY_PREVIOUSSONG,
-	KEY_STOPCD,	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_UNKNOWN,
-	KEY_WWW,	KEY_UNKNOWN,	KEY_VOLUMEDOWN, KEY_MUTE,
-	KEY_VOLUMEUP,	KEY_UNKNOWN,	KEY_BATTERY,	KEY_EJECTCD,
-	KEY_UNKNOWN,	KEY_SLEEP,	KEY_PROG1, KEY_BRIGHTNESSDOWN,
-	KEY_BRIGHTNESSUP,	KEY_UNKNOWN,	KEY_KBDILLUMTOGGLE,
-	KEY_UNKNOWN,	KEY_SWITCHVIDEOMODE,	KEY_UNKNOWN, KEY_UNKNOWN,
-	KEY_SWITCHVIDEOMODE,	KEY_UNKNOWN,	KEY_UNKNOWN, KEY_PROG2,
-	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_UNKNOWN,
-	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_UNKNOWN,	KEY_MICMUTE,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
+	[0]	= KEY_MEDIA,
+	[1]	= KEY_NEXTSONG,
+	[2]	= KEY_PLAYPAUSE,
+	[3]	= KEY_PREVIOUSSONG,
+	[4]	= KEY_STOPCD,
+	[5]	= KEY_UNKNOWN,
+	[6]	= KEY_UNKNOWN,
+	[7]	= KEY_UNKNOWN,
+	[8]	= KEY_WWW,
+	[9]	= KEY_UNKNOWN,
+	[10]	= KEY_VOLUMEDOWN,
+	[11]	= KEY_MUTE,
+	[12]	= KEY_VOLUMEUP,
+	[13]	= KEY_UNKNOWN,
+	[14]	= KEY_BATTERY,
+	[15]	= KEY_EJECTCD,
+	[16]	= KEY_UNKNOWN,
+	[17]	= KEY_SLEEP,
+	[18]	= KEY_PROG1,
+	[19]	= KEY_BRIGHTNESSDOWN,
+	[20]	= KEY_BRIGHTNESSUP,
+	[21]	= KEY_UNKNOWN,
+	[22]	= KEY_KBDILLUMTOGGLE,
+	[23]	= KEY_UNKNOWN,
+	[24]	= KEY_SWITCHVIDEOMODE,
+	[25]	= KEY_UNKNOWN,
+	[26]	= KEY_UNKNOWN,
+	[27]	= KEY_SWITCHVIDEOMODE,
+	[28]	= KEY_UNKNOWN,
+	[29]	= KEY_UNKNOWN,
+	[30]	= KEY_PROG2,
+	[31]	= KEY_UNKNOWN,
+	[32]	= KEY_UNKNOWN,
+	[33]	= KEY_UNKNOWN,
+	[34]	= KEY_UNKNOWN,
+	[35]	= KEY_UNKNOWN,
+	[36]	= KEY_UNKNOWN,
+	[37]	= KEY_UNKNOWN,
+	[38]	= KEY_MICMUTE,
+	[255]	= KEY_PROG3,
 };
 
 /* These are applied if the hk table is present and doesn't override them. */
Signature · 6 lines
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#12 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Darren Hart
Quoted 10 lines · On Saturday 21 November 2015 01:06:36 Darren Hart wrote:
Quoted 3 lines
(quoted diff) +0 -1
-	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry),

GFP_KERNEL); + keymap = kcalloc(hotkey_num +
ARRAY_SIZE(dell_wmi_extra_keymap),

This previously allocated kotkey_num + 1, but you dropeed the +1, making it eactly the size of hotkey_num + the new entries you added.

Why don't we need the +1 anymore? (it isn't clear to me why we needed to before actually, but I want to confirm you considered it).

Last entry in keymap array must be KE_END, right? So this is that +1.

Signature · 2 lines
Pali Rohár
[email protected]
#13 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Darren Hart
Quoted 34 lines · Darren Hart
Quoted 33 lines · Andy Lutomirski

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

This also adds the Fn-lock key as a KE_IGNORE entry.

---
 drivers/platform/x86/dell-wmi.c | 48 +++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +10 -0
index f2d77fe696ac..5be1abec4f64 100644
@@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
      0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
 };
 
+/* These are applied if the hk table is present and doesn't override them. */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+     /* Fn-lock -- no action is required by the kernel. */
+     { KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+     /* Keys that need our help (on XPS 13 Skylake and maybe others. */
+     { KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
+     { KE_KEY, 0x153, { KEY_RFKILL } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
Quoted 10 lines

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 313

Quoted 7 lines
drivers/platform/x86/dell-wmi.c +1 -1
 static void dell_wmi_process_key(int reported_key)
@@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
      int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
                              sizeof(struct dell_bios_keymap_entry);
      struct key_entry *keymap;
-     int i;
+     int i, pos = 0, num_bios_keys;

Just a nit, "reverse christmas tree" order (longest line length first) please. (only if you resend after this review for other reasons)

Will do if I resubmit.

Quoted 10 lines

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 315

Quoted 4 lines
drivers/platform/x86/dell-wmi.c +1 -1
-     keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+     keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),

This previously allocated kotkey_num + 1, but you dropeed the +1, making it eactly the size of hotkey_num + the new entries you added.

Why don't we need the +1 anymore? (it isn't clear to me why we needed to before actually, but I want to confirm you considered it).

Whoops! It's for the KE_END entry.

Jared, want to give us some guidance as to whether this code is correct at all and, if so, whether we should actually send a KEY_RFKILL event from dell-wmi when the key is pressed?

IOW, should we allow dell-wmi to handle rfkill or should we wait for the other mechanism?

--Andy

#35 lore raw

Re: [PATCH 3/3] dell_wmi: Improve unknown hotkey handling

In reply to Andy Lutomirski
Quoted 9 lines · Andy Lutomirski

If DMI lists a hotkey that we don't recognize, log and ignore it instead of trying to map it to keycode 0. I haven't seen this happen, but it will help maintain the key map in the future and it will help avoid sending bogus events.

This also improves the message that we log when we get an unknown key event.

Matthew? Pali? Any concerns?

Quoted 18 lines
---
 drivers/platform/x86/dell-wmi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/platform/x86/dell-wmi.c +1 -1
index 92b0149fa4a7..e43228a35f6b 100644
@@ -180,7 +180,7 @@ static void dell_wmi_process_key(int reported_key)
 	key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
 						reported_key);
 	if (!key) {
-		pr_info("Unknown key %x pressed\n", reported_key);
+		pr_info("Unknown key with scancode 0x%x pressed\n", reported_key);
 		return;
 	}
 

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 347

Quoted 5 lines
drivers/platform/x86/dell-wmi.c +2 -0
@@ -343,6 +343,11 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
 				    bios_to_linux_keycode[bios_entry->keycode] :
 				    KEY_RESERVED;
 
+		if (keycode == 0) {
+			pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);

We don't split up strings for line length, but these arguments can certainly go on the next line. I've taken care of this, no need to resend.

Queued for testing.

Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#26 lore raw

Re: [PATCH 2/3] dell_wmi: Use a C99-style array for bios_to_linux_keycode

Quoted 6 lines · Pali Rohár
Quoted 1 line · On Saturday 21 November 2015 01:09:39 Darren Hart wrote:

Pali or Matthew, do either of you care to comment?

Already commented, email is in archive, see:

http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/7936/focus=7941

Right, sorry, I picked this up from my procmail auto-populated patches mbox since I wasn't Cc'd - missed your response.

This is queued to testing.

Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#27 lore raw

Re: [PATCH 2/3] dell_wmi: Use a C99-style array for bios_to_linux_keycode

Quoted 10 lines · Darren Hart
Quoted 6 lines · Pali Rohár
Quoted 1 line · On Saturday 21 November 2015 01:09:39 Darren Hart wrote:

Pali or Matthew, do either of you care to comment?

Already commented, email is in archive, see:

http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/7936/focus=7941

Right, sorry, I picked this up from my procmail auto-populated patches mbox since I wasn't Cc'd - missed your response.

Sorry, I parsed MAINTAINERS by hand incorrectly.

--Andy

#31 lore raw

Re: [PATCH 3/3] dell_wmi: Improve unknown hotkey handling

In reply to Andy Lutomirski
Quoted 40 lines · Andy Lutomirski
Quoted 31 lines · Pali Rohár
Quoted 30 lines · On Friday 13 November 2015 21:49:32 Andy Lutomirski wrote:

If DMI lists a hotkey that we don't recognize, log and ignore it instead of trying to map it to keycode 0. I haven't seen this happen, but it will help maintain the key map in the future and it will help avoid sending bogus events.

This also improves the message that we log when we get an unknown key event.

---
 drivers/platform/x86/dell-wmi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/platform/x86/dell-wmi.c +1 -1
index 92b0149fa4a7..e43228a35f6b 100644
@@ -180,7 +180,7 @@ static void dell_wmi_process_key(int reported_key)
      key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
                                              reported_key);
      if (!key) {
-             pr_info("Unknown key %x pressed\n", reported_key);
+             pr_info("Unknown key with scancode 0x%x pressed\n", reported_key);
              return;
      }
 
@@ -343,6 +343,11 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
                                  bios_to_linux_keycode[bios_entry->keycode] :
                                  KEY_RESERVED;
 
Quoted 7 lines

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 350

Quoted 5 lines
drivers/platform/x86/dell-wmi.c +5 -0
+             if (keycode == 0) {
+                     pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);
+                     continue;
+             }
+

This line is too long and checkpatch.pl will probably blame you.

Quoted 3 lines

I split it. The first bit (with the quoted string) is still a bit above 80, but that's better than splitting the string itself and breaking grep.

So I've taken care of this in my branch (and the one above too).

Quoted 11 lines
Quoted 5 lines

Anyway, have you already found some missing mapping which comes from bios/firmware (because your patches do not change that bios table)?

I don't see a DMI entry that maps to something outside the table, though, so this warning doesn't trigger. My best guess is that Dell simply didn't bother to update the DMI table and has it hardcoded in their driver. Admittedly, I've never played with the official driver at all.

Pali, was this something you wanted to discuss more before I merge it?

Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#32 lore raw

Re: [PATCH 3/3] dell_wmi: Improve unknown hotkey handling

Quoted 53 lines · Darren Hart
Quoted 40 lines · Andy Lutomirski
Quoted 31 lines · Pali Rohár
Quoted 30 lines · On Friday 13 November 2015 21:49:32 Andy Lutomirski wrote:

If DMI lists a hotkey that we don't recognize, log and ignore it instead of trying to map it to keycode 0. I haven't seen this happen, but it will help maintain the key map in the future and it will help avoid sending bogus events.

This also improves the message that we log when we get an unknown key event.

---
 drivers/platform/x86/dell-wmi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/platform/x86/dell-wmi.c +1 -1
index 92b0149fa4a7..e43228a35f6b 100644
@@ -180,7 +180,7 @@ static void dell_wmi_process_key(int reported_key)
      key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
                                              reported_key);
      if (!key) {
-             pr_info("Unknown key %x pressed\n", reported_key);
+             pr_info("Unknown key with scancode 0x%x pressed\n", reported_key);
              return;
      }
 
@@ -343,6 +343,11 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
                                  bios_to_linux_keycode[bios_entry->keycode] :
                                  KEY_RESERVED;
 
Quoted 7 lines

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 350

Quoted 5 lines
drivers/platform/x86/dell-wmi.c +5 -0
+             if (keycode == 0) {
+                     pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);
+                     continue;
+             }
+

This line is too long and checkpatch.pl will probably blame you.

Quoted 3 lines

I split it. The first bit (with the quoted string) is still a bit above 80, but that's better than splitting the string itself and breaking grep.

So I've taken care of this in my branch (and the one above too).

Quoted 6 lines
Quoted 5 lines

Anyway, have you already found some missing mapping which comes from bios/firmware (because your patches do not change that bios table)?

Quoted 7 lines
Quoted 5 lines

I don't see a DMI entry that maps to something outside the table, though, so this warning doesn't trigger. My best guess is that Dell simply didn't bother to update the DMI table and has it hardcoded in their driver. Admittedly, I've never played with the official driver at all.

Pali, was this something you wanted to discuss more before I merge it?

FWIW, my current partial understanding of what changed between previous laptops and the 9350 is that there's Yet Another New Hotkey Mechanism (tm). I'm not sure whether I should comment on exactly what it does, but if you dig around in the DSDT, it's not all that hard to get a decent idea.

The upshot is that, on Windows, it's probably still the case that all of the hotkeys that are supposed to be handled through WMI on the 9350 are, indeed, in the DMI table. Presumably the Windows driver doesn't visibly warn about unexpected scancodes.

Since we *do* warn about unexpected scancodes, we need to silence the warnings. We could do it with a fancy table-driven approach that can actually generate keystrokes as in this patch (which gives us full support for the 9350's rfkill key), or we could just make a little list of scancodes that aren't worth warning about and add a while new platform driver for the new mechanism. Either way works.

--Andy

#33 lore raw

Re: [PATCH 3/3] dell_wmi: Improve unknown hotkey handling

In reply to Darren Hart
Quoted 2 lines · On Saturday 21 November 2015 01:29:04 Darren Hart wrote:

Pali, was this something you wanted to discuss more before I merge it?

Just one note:

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 346

Quoted 1 line
drivers/platform/x86/dell-wmi.c +1 -0
+		if (keycode == 0) {

What about using define KEY_RESERVED instead hardcoded constant 0?

Quoted 3 lines
drivers/platform/x86/dell-wmi.c +3 -0
+			pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);
+			continue;
+		}
Signature · 2 lines
Pali Rohár
[email protected]
#34 lore raw

Re: [PATCH 3/3] dell_wmi: Improve unknown hotkey handling

Quoted 14 lines · Pali Rohár
Quoted 2 lines · On Saturday 21 November 2015 01:29:04 Darren Hart wrote:

Pali, was this something you wanted to discuss more before I merge it?

Just one note:

Quoted 1 line

+ if (keycode == 0) {

What about using define KEY_RESERVED instead hardcoded constant 0?

Quoted 3 lines
drivers/platform/x86/dell-wmi.c +3 -0
+                     pr_info("firmware scancode %d maps to unrecognized keycode %d\n", bios_entry->keycode, bios_entry->scancode);
+                     continue;
+             }

That would work, but only because KEY_RESERVED == 0. Let me send a better patch.

--Andy

#14 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Andy Lutomirski
Quoted 61 lines · Andy Lutomirski
Quoted 34 lines · Darren Hart
Quoted 33 lines · Andy Lutomirski

The XPS 13 Skylake has an rfkill button and a switchvideomode button that aren't enumerated in the DMI table AFAICT. Add a table listing extra un-enumerated hotkeys. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

This also adds the Fn-lock key as a KE_IGNORE entry.

---
 drivers/platform/x86/dell-wmi.c | 48 +++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +10 -0
index f2d77fe696ac..5be1abec4f64 100644
@@ -142,6 +142,16 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
      0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_PROG3
 };
 
+/* These are applied if the hk table is present and doesn't override them. */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+     /* Fn-lock -- no action is required by the kernel. */
+     { KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+     /* Keys that need our help (on XPS 13 Skylake and maybe others. */
+     { KE_KEY, 0x152, { KEY_SWITCHVIDEOMODE } },
+     { KE_KEY, 0x153, { KEY_RFKILL } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
Quoted 10 lines

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 313

Quoted 7 lines
drivers/platform/x86/dell-wmi.c +1 -1
 static void dell_wmi_process_key(int reported_key)
@@ -300,9 +310,10 @@ static const struct key_entry * __init dell_wmi_prepare_new_keymap(void)
      int hotkey_num = (dell_bios_hotkey_table->header.length - 4) /
                              sizeof(struct dell_bios_keymap_entry);
      struct key_entry *keymap;
-     int i;
+     int i, pos = 0, num_bios_keys;

Just a nit, "reverse christmas tree" order (longest line length first) please. (only if you resend after this review for other reasons)

Will do if I resubmit.

Quoted 10 lines

drivers/platform/x86/dell-wmi.c static const struct key_entry * __init dell_wmi_prepare_new_keymap(void) · line 315

Quoted 4 lines
drivers/platform/x86/dell-wmi.c +1 -1
-     keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+     keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap),

This previously allocated kotkey_num + 1, but you dropeed the +1, making it eactly the size of hotkey_num + the new entries you added.

Why don't we need the +1 anymore? (it isn't clear to me why we needed to before actually, but I want to confirm you considered it).

Whoops! It's for the KE_END entry.

Quoted 8 lines

Jared, want to give us some guidance as to whether this code is correct at all and, if so, whether we should actually send a KEY_RFKILL event from dell-wmi when the key is pressed?

IOW, should we allow dell-wmi to handle rfkill or should we wait for the other mechanism?

--Andy

I'm adding Mario since he's worked on our client systems much longer and knows more about the correct behavior for our hotkeys. I've not yet read through any of our documentation about our hotkey behavior so wouldn't be as qualified to answer that. Note that both Mario and I are on vacation until the 30th, so he may not reply until then.

--Jared

Signature · 4 lines
Jared Domínguez
OS Architect
Linux Engineering
Dell | Client Software Group
#15 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 10 lines · On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
Quoted 8 lines · Andy Lutomirski

Jared, want to give us some guidance as to whether this code is correct at all and, if so, whether we should actually send a KEY_RFKILL event from dell-wmi when the key is pressed?

IOW, should we allow dell-wmi to handle rfkill or should we wait for the other mechanism?

--Andy

Quoted 8 lines

I'm adding Mario since he's worked on our client systems much longer and knows more about the correct behavior for our hotkeys. I've not yet read through any of our documentation about our hotkey behavior so wouldn't be as qualified to answer that. Note that both Mario and I are on vacation until the 30th, so he may not reply until then.

--Jared

Jared, thanks for your input! When you or Mario are back from vacation, can you look at it? Or if you do not have much time, can you provide documentation (or some snippets), so somebody else can update driver to work correctly?

Signature · 2 lines
Pali Rohár
[email protected]
#16 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 17 lines · On Monday 23 November 2015 15:53:51 Pali Rohár wrote:
Quoted 16 lines · On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
Quoted 8 lines · Andy Lutomirski

Jared, want to give us some guidance as to whether this code is correct at all and, if so, whether we should actually send a KEY_RFKILL event from dell-wmi when the key is pressed?

IOW, should we allow dell-wmi to handle rfkill or should we wait for the other mechanism?

--Andy

I'm adding Mario since he's worked on our client systems much longer and knows more about the correct behavior for our hotkeys. I've not yet read through any of our documentation about our hotkey behavior so wouldn't be as qualified to answer that. Note that both Mario and I are on vacation until the 30th, so he may not reply until then.

Quoted 7 lines
Quoted 2 lines

--Jared

Jared, thanks for your input! When you or Mario are back from vacation, can you look at it? Or if you do not have much time, can you provide documentation (or some snippets), so somebody else can update driver to work correctly?

Hi Jared & Mario, I believe you are back from vacation, can you look at it?

Signature · 2 lines
Pali Rohár
[email protected]
#18 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Mario Limonciello
Quoted 28 lines · Mario Limonciello
Quoted 17 lines · Pali Rohár
Quoted 16 lines · On Monday 23 November 2015 15:53:51 Pali Rohár wrote:
Quoted 15 lines · On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
Quoted 8 lines · Andy Lutomirski

Jared, want to give us some guidance as to whether this code is correct at all and, if so, whether we should actually send a KEY_RFKILL event from dell-wmi when the key is pressed?

IOW, should we allow dell-wmi to handle rfkill or should we wait for the other mechanism?

--Andy

I'm adding Mario since he's worked on our client systems much longer and knows more about the correct behavior for our hotkeys. I've not yet read through any of our documentation about our hotkey behavior so wouldn't be as qualified to answer that. Note that both Mario and I are on vacation until the 30th, so he may not reply until then.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

--Jared

Jared, thanks for your input! When you or Mario are back from vacation, can you look at it? Or if you do not have much time, can you provide documentation (or some snippets), so somebody else can update driver to work correctly?

Hi Jared & Mario, I believe you are back from vacation, can you look at it?

Quoted 9 lines

Pali,

I understand that intel-hid will be making it in at this point. If I'm not mistaken, it should be handling this portion.

Andy,

Is that right now? Or is this on a different topic?

I think that's right. And intel-hid is in Linus' tree now.

--Andy

#17 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

In reply to Pali Rohár
Quoted 17 lines · Pali Rohár
Quoted 16 lines · On Monday 23 November 2015 15:53:51 Pali Rohár wrote:
Quoted 15 lines · On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
Quoted 8 lines · Andy Lutomirski

Jared, want to give us some guidance as to whether this code is correct at all and, if so, whether we should actually send a KEY_RFKILL event from dell-wmi when the key is pressed?

IOW, should we allow dell-wmi to handle rfkill or should we wait for the other mechanism?

--Andy

I'm adding Mario since he's worked on our client systems much longer and knows more about the correct behavior for our hotkeys. I've not yet read through any of our documentation about our hotkey behavior so wouldn't be as qualified to answer that. Note that both Mario and I are on vacation until the 30th, so he may not reply until then.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

--Jared

Jared, thanks for your input! When you or Mario are back from vacation, can you look at it? Or if you do not have much time, can you provide documentation (or some snippets), so somebody else can update driver to work correctly?

Hi Jared & Mario, I believe you are back from vacation, can you look at it?

Pali,

I understand that intel-hid will be making it in at this point. If I'm not mistaken, it should be handling this portion.

Andy,

Is that right now? Or is this on a different topic?

#19 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 26 lines · On Thursday 21 January 2016 18:08:38 Mario Limonciello wrote:
Quoted 23 lines · Pali Rohár
Quoted 22 lines · On Monday 23 November 2015 15:53:51 Pali Rohár wrote:
Quoted 8 lines · On Saturday 21 November 2015 20:04:15 D. Jared Dominguez wrote:
Quoted 7 lines · Andy Lutomirski

Jared, want to give us some guidance as to whether this code is correct at all and, if so, whether we should actually send a KEY_RFKILL event from dell-wmi when the key is pressed?

IOW, should we allow dell-wmi to handle rfkill or should we wait for the other mechanism?

Quoted 9 lines
Quoted 1 line

--Andy

I'm adding Mario since he's worked on our client systems much longer and knows more about the correct behavior for our hotkeys. I've not yet read through any of our documentation about our hotkey behavior so wouldn't be as qualified to answer that. Note that both Mario and I are on vacation until the 30th, so he may not reply until then.

--Jared

Jared, thanks for your input! When you or Mario are back from vacation, can you look at it? Or if you do not have much time, can you provide documentation (or some snippets), so somebody else can update driver to work correctly?

Quoted 6 lines
Quoted 2 lines

Hi Jared & Mario, I believe you are back from vacation, can you look at it?

Pali,

I understand that intel-hid will be making it in at this point. If I'm not mistaken, it should be handling this portion.

Hi Mario! Thats truth, but we still needs to know all codes which dell-wmi should ignore (because they will be handled by intel-hid).

Currently Andy's patch adds three codes which are not specified in vendor-specific DMI table.

Do you such table of key events which can ACPI/BIOS/WMI send to OS?

Signature · 2 lines
Pali Rohár
[email protected]
#20 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 8 lines · Pali Rohár

On Thursday 21 January 2016 18:08:38 Mario Limonciello wrote:
Hi Mario! Thats truth, but we still needs to know all codes which dell-wmi should ignore (because they will be handled by intel-hid).

Currently Andy's patch adds three codes which are not specified in vendor-specific DMI table.

Do you such table of key events which can ACPI/BIOS/WMI send to OS?

Hi Pali,

Andy conversed with us privately on this, and we double checked with our BIOS team on the EC source code for these items that aren't mentioned in the DMI table.
This should now be complete as of current EC feature support.

Thanks,

#21 lore raw

Re: [PATCH 1/3] dell_wmi: Support new hotkeys on the XPS 13 Skylake

Quoted 10 lines · On Friday 22 January 2016 16:58:02 Mario Limonciello wrote:
Quoted 8 lines · Pali Rohár

On Thursday 21 January 2016 18:08:38 Mario Limonciello wrote:
Hi Mario! Thats truth, but we still needs to know all codes which dell-wmi should ignore (because they will be handled by intel-hid).

Currently Andy's patch adds three codes which are not specified in vendor-specific DMI table.

Do you such table of key events which can ACPI/BIOS/WMI send to OS?

Quoted 8 lines

Hi Pali,

Andy conversed with us privately on this, and we double checked with our BIOS team on the EC source code for these items that aren't mentioned in the DMI table.
This should now be complete as of current EC feature support.

Thanks,

Great! Thank you for info.

Signature · 2 lines
Pali Rohár
[email protected]