Skip to contentSkip to conversation

dell-wmi: Support new hotkeys on the XPS 13 9350 (Skylake)

PATCH In review Andy Lutomirski 4 participants 9 messages Jan 19, 2016 – Jan 30, 2016

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v4 1/1 dell-wmi: Support new hotkeys on the XPS 13 9350 (Skylake)

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

---

This applies after the dmi-walk fix.

Notes:
Changes from v3:
- Rebase in top of interface version stuff. (This changes context but not any diff lines.)
Changes from v2:
- Factor check for already-known scancodes into a helper.
- Un-abbreviate comments.
- Fix off-by-one.
- Rebase on top of of dmi_walk fixes.

Changes from v1:
- The new hotkey code matches reality better.
- Don't send key events for the new hotkeys.

drivers/platform/x86/dell-wmi.c | 71 +++++++++++++++++++++++++++++++++++++----

 1 file changed, 64 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +64 -7
index 3e30f77b70e1..afb7150bf2c1 100644
@@ -168,6 +168,30 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
 	[255]	= KEY_PROG3,
 };
 
+/*
+ * These are applied if the 0xB2 DMI hotkey table is present and doesn't
+ * override them.
+ */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+	/* Fn-lock */
+	{ KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+	/* Change keyboard illumination */
+	{ KE_IGNORE, 0x152, { KEY_KBDILLUMTOGGLE } },
+
+	/*
+	 * Radio disable (notify only -- there is no model for which the
+	 * WMI event is supposed to trigger an action).
+	 */
+	{ KE_IGNORE, 0x153, { KEY_RFKILL } },
+
+	/* RGB keyboard backlight control */
+	{ KE_IGNORE, 0x154, { KEY_RESERVED } },
+
+	/* Stealth mode toggle */
+	{ KE_IGNORE, 0x155, { KEY_RESERVED } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
 static void dell_wmi_process_key(int reported_key)
@@ -339,13 +363,27 @@ static void dell_wmi_notify(u32 value, void *context)
 	kfree(obj);
 }
 
+static bool have_scancode(u32 scancode, const struct key_entry *keymap, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		if (keymap[i].code == scancode)
+			return true;
+
+	return false;
+}
+
 static void __init handle_dmi_entry(const struct dmi_header *dm,
+
 				    void *opaque)
+
 {
 	struct dell_dmi_results *results = opaque;
 	struct dell_bios_hotkey_table *table;
+	int hotkey_num, i, pos = 0;
 	struct key_entry *keymap;
-	int hotkey_num, i;
+	int num_bios_keys;
 
 	if (results->err || results->keymap)
 		return;		/* We already found the hotkey table. */
@@ -369,7 +407,8 @@ static void __init handle_dmi_entry(const struct dmi_header *dm,
 		return;
 	}
 
-	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap) + 1,
+			 sizeof(struct key_entry), GFP_KERNEL);
 	if (!keymap) {
 		results->err = -ENOMEM;
 		return;
@@ -397,14 +436,32 @@ static void __init handle_dmi_entry(const struct dmi_header *dm,
 		}
 
 		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++) {
+		const struct key_entry *entry = &dell_wmi_extra_keymap[i];
+
+		/*
+		 * 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.
+		 */
+		if (!have_scancode(entry->code, keymap, num_bios_keys)) {
+			keymap[pos] = *entry;
+			pos++;
+		}
 	}
 
-	keymap[hotkey_num].type = KE_END;
+	keymap[pos].type = KE_END;
 
 	results->keymap = keymap;
 }
Signature · 1 line
2.5.0
Reviewed-by on v4 1/1
#1 lore raw
Quoted 9 lines · On Tuesday 19 January 2016 15:07:47 Andy Lutomirski wrote:

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

Quoted 6 lines

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

Looks good,

Quoted 133 lines

---

This applies after the dmi-walk fix.

Notes:
Changes from v3:
- Rebase in top of interface version stuff. (This changes context but not any diff lines.)
Changes from v2:
- Factor check for already-known scancodes into a helper.
- Un-abbreviate comments.
- Fix off-by-one.
- Rebase on top of of dmi_walk fixes.

Changes from v1:
- The new hotkey code matches reality better.
- Don't send key events for the new hotkeys.

drivers/platform/x86/dell-wmi.c | 71 +++++++++++++++++++++++++++++++++++++----

 1 file changed, 64 insertions(+), 7 deletions(-)
drivers/platform/x86/dell-wmi.c +64 -7
index 3e30f77b70e1..afb7150bf2c1 100644
@@ -168,6 +168,30 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
 	[255]	= KEY_PROG3,
 };
 
+/*
+ * These are applied if the 0xB2 DMI hotkey table is present and doesn't
+ * override them.
+ */
+static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
+	/* Fn-lock */
+	{ KE_IGNORE, 0x151, { KEY_RESERVED } },
+
+	/* Change keyboard illumination */
+	{ KE_IGNORE, 0x152, { KEY_KBDILLUMTOGGLE } },
+
+	/*
+	 * Radio disable (notify only -- there is no model for which the
+	 * WMI event is supposed to trigger an action).
+	 */
+	{ KE_IGNORE, 0x153, { KEY_RFKILL } },
+
+	/* RGB keyboard backlight control */
+	{ KE_IGNORE, 0x154, { KEY_RESERVED } },
+
+	/* Stealth mode toggle */
+	{ KE_IGNORE, 0x155, { KEY_RESERVED } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
 static void dell_wmi_process_key(int reported_key)
@@ -339,13 +363,27 @@ static void dell_wmi_notify(u32 value, void *context)
 	kfree(obj);
 }
 
+static bool have_scancode(u32 scancode, const struct key_entry *keymap, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		if (keymap[i].code == scancode)
+			return true;
+
+	return false;
+}
+
 static void __init handle_dmi_entry(const struct dmi_header *dm,
+
 				    void *opaque)
+
 {
 	struct dell_dmi_results *results = opaque;
 	struct dell_bios_hotkey_table *table;
+	int hotkey_num, i, pos = 0;
 	struct key_entry *keymap;
-	int hotkey_num, i;
+	int num_bios_keys;
 
 	if (results->err || results->keymap)
 		return;		/* We already found the hotkey table. */
@@ -369,7 +407,8 @@ static void __init handle_dmi_entry(const struct dmi_header *dm,
 		return;
 	}
 
-	keymap = kcalloc(hotkey_num + 1, sizeof(struct key_entry), GFP_KERNEL);
+	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap) + 1,
+			 sizeof(struct key_entry), GFP_KERNEL);
 	if (!keymap) {
 		results->err = -ENOMEM;
 		return;
@@ -397,14 +436,32 @@ static void __init handle_dmi_entry(const struct dmi_header *dm,
 		}
 
 		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++) {
+		const struct key_entry *entry = &dell_wmi_extra_keymap[i];
+
+		/*
+		 * 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.
+		 */
+		if (!have_scancode(entry->code, keymap, num_bios_keys)) {
+			keymap[pos] = *entry;
+			pos++;
+		}
 	}
 
-	keymap[hotkey_num].type = KE_END;
+	keymap[pos].type = KE_END;
 
 	results->keymap = keymap;
 }
Signature · 2 lines
Pali Rohár
[email protected]
#2 lore raw
Quoted 15 lines · Pali Rohár
Quoted 14 lines · On Tuesday 19 January 2016 15:07:47 Andy Lutomirski wrote:

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

Quoted 5 lines
Quoted 1 line

Looks good,

Darren, if/when you apply this, can you fix up the last paragraph of the commit message?

This doesn't send keypress events for any of the new events, as no models appear to require it.

I wrote the old text before intel-hid landed.

--Andy

#3 lore raw
Quoted 22 lines · Andy Lutomirski
Quoted 15 lines · Pali Rohár
Quoted 14 lines · On Tuesday 19 January 2016 15:07:47 Andy Lutomirski wrote:

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

Quoted 5 lines
Quoted 1 line

Looks good,

Quoted 7 lines

Darren, if/when you apply this, can you fix up the last paragraph of the commit message?

This doesn't send keypress events for any of the new events, as no models appear to require it.

I wrote the old text before intel-hid landed.

Planning on queueing for 4.6 as soon as this window closes, likely tomorrow. I want this to have some time in -next. Thanks!

Quoted 3 lines

--Andy

Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#4 lore raw
In reply to Andy Lutomirski
Quoted 25 lines · Andy Lutomirski
Quoted 15 lines · Pali Rohár
Quoted 14 lines · On Tuesday 19 January 2016 15:07:47 Andy Lutomirski wrote:

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

Quoted 5 lines
Quoted 1 line

Looks good,

Darren, if/when you apply this, can you fix up the last paragraph of the commit message?

Quoted 6 lines

This doesn't send keypress events for any of the new events, as no models appear to require it.

I wrote the old text before intel-hid landed.

--Andy

This had some problems applying to 4.5-rc1 (refactoring of handle_dmi_...). I've done the merge, it builds, but I can't test it. Please have a look at the dell-wmi branch and let me know if this is correct and working as expected.

Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#5 lore raw
Quoted 29 lines · Darren Hart
Quoted 28 lines · Andy Lutomirski
Quoted 15 lines · Pali Rohár
Quoted 14 lines · On Tuesday 19 January 2016 15:07:47 Andy Lutomirski wrote:

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

Quoted 5 lines
Quoted 1 line

Looks good,

Darren, if/when you apply this, can you fix up the last paragraph of the commit message?

This doesn't send keypress events for any of the new events, as no models appear to require it.

Quoted 8 lines
Quoted 3 lines

I wrote the old text before intel-hid landed.

--Andy

This had some problems applying to 4.5-rc1 (refactoring of handle_dmi_...). I've done the merge, it builds, but I can't test it. Please have a look at the dell-wmi branch and let me know if this is correct and working as expected.

It's probably okay, but I think you applied it out of order. This patch was intended to be applied after the handle_dmi_xyz fixes. If you back this one out, apply those, and then re-apply this, it should apply cleanly.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw
Quoted 39 lines · Andy Lutomirski
Quoted 29 lines · Darren Hart
Quoted 28 lines · Andy Lutomirski
Quoted 15 lines · Pali Rohár
Quoted 14 lines · On Tuesday 19 January 2016 15:07:47 Andy Lutomirski wrote:

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

Quoted 5 lines
Quoted 1 line

Looks good,

Darren, if/when you apply this, can you fix up the last paragraph of the commit message?

This doesn't send keypress events for any of the new events, as no models appear to require it.

Quoted 8 lines
Quoted 3 lines

I wrote the old text before intel-hid landed.

--Andy

This had some problems applying to 4.5-rc1 (refactoring of handle_dmi_...). I've done the merge, it builds, but I can't test it. Please have a look at the dell-wmi branch and let me know if this is correct and working as expected.

Quoted 6 lines

It's probably okay, but I think you applied it out of order. This patch was intended to be applied after the handle_dmi_xyz fixes. If you back this one out, apply those, and then re-apply this, it should apply cleanly.

--Andy

Ah, thanks. Sometimes that gets lost in the INBOX, I was starting to think the same thing as I went through the patchwork list though. I'll back it out and do that. Thanks!

Quoted 1 line
Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#7 lore raw
Quoted 45 lines · Darren Hart
Quoted 44 lines · Andy Lutomirski
Quoted 29 lines · Darren Hart
Quoted 28 lines · Andy Lutomirski
Quoted 15 lines · Pali Rohár
Quoted 14 lines · On Tuesday 19 January 2016 15:07:47 Andy Lutomirski wrote:

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

Quoted 5 lines
Quoted 1 line

Looks good,

Darren, if/when you apply this, can you fix up the last paragraph of the commit message?

This doesn't send keypress events for any of the new events, as no models appear to require it.

Quoted 8 lines
Quoted 3 lines

I wrote the old text before intel-hid landed.

--Andy

This had some problems applying to 4.5-rc1 (refactoring of handle_dmi_...). I've done the merge, it builds, but I can't test it. Please have a look at the dell-wmi branch and let me know if this is correct and working as expected.

It's probably okay, but I think you applied it out of order. This patch was intended to be applied after the handle_dmi_xyz fixes. If you back this one out, apply those, and then re-apply this, it should apply cleanly.

Quoted 5 lines
Quoted 1 line

--Andy

Ah, thanks. Sometimes that gets lost in the INBOX, I was starting to think the same thing as I went through the patchwork list though. I'll back it out and do that. Thanks!

Turns out I wasn't Cc'd on the dependent dmi fixes series. Please include me on Cc for patches where I show up as a maintainer.

Signature · 2 lines
Darren Hart
Intel Open Source Technology Center
#8 lore raw
Quoted 46 lines · Darren Hart
Quoted 45 lines · Darren Hart
Quoted 44 lines · Andy Lutomirski
Quoted 29 lines · Darren Hart
Quoted 28 lines · Andy Lutomirski
Quoted 15 lines · Pali Rohár
Quoted 14 lines · On Tuesday 19 January 2016 15:07:47 Andy Lutomirski wrote:

The XPS 13 9350 sends WMI keypress events that aren't enumerated in the DMI table. Add a table listing them. To avoid breaking things that worked before, these un-enumerated hotkeys won't be used if the DMI table maps them to something else.

FWIW, it appears that the DMI table may be a legacy thing and we might want to rethink how we handle events in general. As an example, a whole lot of things map to KEY_PROG3 via the DMI table.

So far, this doesn't send keypress events for any of the new events. Depnding on whether we figure out exactly what needs to happen to get the wireless button working in time for Linux 4.5, we might want to temporarily handle it in dell-wmi.

Quoted 5 lines
Quoted 1 line

Looks good,

Darren, if/when you apply this, can you fix up the last paragraph of the commit message?

This doesn't send keypress events for any of the new events, as no models appear to require it.

Quoted 8 lines
Quoted 3 lines

I wrote the old text before intel-hid landed.

--Andy

This had some problems applying to 4.5-rc1 (refactoring of handle_dmi_...). I've done the merge, it builds, but I can't test it. Please have a look at the dell-wmi branch and let me know if this is correct and working as expected.

It's probably okay, but I think you applied it out of order. This patch was intended to be applied after the handle_dmi_xyz fixes. If you back this one out, apply those, and then re-apply this, it should apply cleanly.

Quoted 9 lines
Quoted 5 lines
Quoted 1 line

--Andy

Ah, thanks. Sometimes that gets lost in the INBOX, I was starting to think the same thing as I went through the patchwork list though. I'll back it out and do that. Thanks!

Turns out I wasn't Cc'd on the dependent dmi fixes series. Please include me on Cc for patches where I show up as a maintainer.

Oops. I cc'd [email protected] and assumed that was sufficient. Sorry.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html