Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1423349 > unrolled thread

[PATCH v3 0/4] dell-wmi: Changes in WMI event code handling

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling Pali Rohár <pali.rohar@gmail.com> - 2016-06-15 21:50 +0200
    [PATCH v3 4/4] dell-wmi: Generate one sparse keymap for all machines Pali Rohár <pali.rohar@gmail.com> - 2016-06-15 21:50 +0200
    [PATCH v3 2/4] dell-wmi: Sort WMI event codes and update comments Pali Rohár <pali.rohar@gmail.com> - 2016-06-15 22:00 +0200
    [PATCH v3 3/4] dell-wmi: Add information about other WMI event codes Pali Rohár <pali.rohar@gmail.com> - 2016-06-15 22:00 +0200
    Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling Darren Hart <dvhart@infradead.org> - 2016-06-16 05:30 +0200
      Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling Pali Rohár <pali.rohar@gmail.com> - 2016-06-16 09:40 +0200
        Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling Gabriele Mazzotta <gabriele.mzt@gmail.com> - 2016-06-17 01:10 +0200
        Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling Darren Hart <dvhart@infradead.org> - 2016-06-21 18:30 +0200
        Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling Darren Hart <dvhart@infradead.org> - 2016-06-21 20:10 +0200
          RE: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling <Mario_Limonciello@Dell.com> - 2016-06-21 20:30 +0200
            Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling Pali Rohár <pali.rohar@gmail.com> - 2016-06-21 20:30 +0200
              RE: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling <Mario_Limonciello@Dell.com> - 2016-06-21 20:40 +0200
                Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling Pali Rohár <pali.rohar@gmail.com> - 2016-06-22 12:40 +0200
                  RE: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling <Mario_Limonciello@Dell.com> - 2016-06-22 15:30 +0200

#1423349 — [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling

First patch describe problem about 0xe045 code. Second and third are just
cosmetic and last rework code which processing WMI events. It should be
properly tested on more Dell machines, to check that everything is still
working correctly.

Changes since v2:
* Updated description for 2/4 and 4/4
* Fixed comments over 80 characters in 2/4 and 4/4
* Use switch fall-through in 4/4
* Added Michał's Reviewed-by line

Changes since v1:
* Fixed comments
* Fixed memory leak
* Added Tested-By lines
* Added event 0xe06e

Pali Rohár (4):
  dell-wmi: Ignore WMI event code 0xe045
  dell-wmi: Sort WMI event codes and update comments
  dell-wmi: Add information about other WMI event codes
  dell-wmi: Generate one sparse keymap for all machines

 drivers/platform/x86/dell-wmi.c |  290 ++++++++++++++++++++++-----------------
 1 file changed, 166 insertions(+), 124 deletions(-)

-- 
1.7.9.5

[toc] | [next] | [standalone]

#1423350 — [PATCH v3 4/4] dell-wmi: Generate one sparse keymap for all machines

This patch reworks code for generating sparse keymap and processing WMI
events. It unifies procedure for generating sparse keymap and also unifies
big switch code for processing WMI events of different types. After this
patch dell-wmi driver does not differ between "old" and "new" hotkey type.

It constructs sparse keymap table with all WMI codes. It is because on some
laptops (e.g. Dell Latitude E6440) ACPI/firmware send both event types (old
and new).

Each WMI code in sparse keymap table is prefixed by 16bit event type, so it
does not change functionality on laptops with "old" hotkey support (those
without scancodes in DMI).

This allow us to distinguish between same WMI codes with different types in
sparse keymap. Thanks to this WMI events of type 0x0011 were moved from big
switch into sparse keymap table too.

This patch also fixes possible bug in parsing WMI event buffer introduced
in commit 5ea2559726b7 ("dell-wmi: Add support for new Dell systems"). That
commit changed buffer type from int* to u16* without fixing code. More at:
http://lkml.iu.edu/hypermail/linux/kernel/1507.0/01950.html

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Michał Kępień <kernel@kempniu.pl>
Reviewed-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-wmi.c |  215 +++++++++++++++++++--------------------
 1 file changed, 104 insertions(+), 111 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 41ae79d..b8ad055 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -80,12 +80,13 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
 };
 
 /*
+ * Keymap for WMI events of type 0x0000
+ *
  * Certain keys are flagged as KE_IGNORE. All of these are either
  * notifications (rather than requests for change) or are also sent
  * via the keyboard controller so should not be sent again.
  */
-
-static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
+static const struct key_entry dell_wmi_keymap_type_0000[] __initconst = {
 	{ KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
 
 	/* Key code is followed by brightness level */
@@ -183,12 +184,8 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 	{ KE_IGNORE, 0xe0f7, { KEY_MUTE } },
 	{ KE_IGNORE, 0xe0f8, { KEY_VOLUMEDOWN } },
 	{ KE_IGNORE, 0xe0f9, { KEY_VOLUMEUP } },
-
-	{ KE_END, 0 }
 };
 
-static bool dell_new_hk_type;
-
 struct dell_bios_keymap_entry {
 	u16 scancode;
 	u16 keycode;
@@ -202,6 +199,7 @@ struct dell_bios_hotkey_table {
 
 struct dell_dmi_results {
 	int err;
+	int keymap_size;
 	struct key_entry *keymap;
 };
 
@@ -250,10 +248,12 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
 };
 
 /*
+ * Keymap for WMI events of type 0x0010
+ *
  * 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 = {
+static const struct key_entry dell_wmi_keymap_type_0010[] __initconst = {
 	/* Fn-lock */
 	{ KE_IGNORE, 0x151, { KEY_RESERVED } },
 
@@ -273,21 +273,39 @@ static const struct key_entry dell_wmi_extra_keymap[] __initconst = {
 	{ KE_IGNORE, 0x155, { KEY_RESERVED } },
 };
 
+/*
+ * Keymap for WMI events of type 0x0011
+ */
+static const struct key_entry dell_wmi_keymap_type_0011[] __initconst = {
+	/* Battery unplugged */
+	{ KE_IGNORE, 0xfff0, { KEY_RESERVED } },
+
+	/* Battery inserted */
+	{ KE_IGNORE, 0xfff1, { KEY_RESERVED } },
+
+	/* Keyboard backlight level changed */
+	{ KE_IGNORE, 0x01e1, { KEY_RESERVED } },
+	{ KE_IGNORE, 0x02ea, { KEY_RESERVED } },
+	{ KE_IGNORE, 0x02eb, { KEY_RESERVED } },
+	{ KE_IGNORE, 0x02ec, { KEY_RESERVED } },
+	{ KE_IGNORE, 0x02f6, { KEY_RESERVED } },
+};
+
 static struct input_dev *dell_wmi_input_dev;
 
-static void dell_wmi_process_key(int reported_key)
+static void dell_wmi_process_key(int type, int code)
 {
 	const struct key_entry *key;
 
 	key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
-						reported_key);
+						(type << 16) | code);
 	if (!key) {
-		pr_info("Unknown key with scancode 0x%x pressed\n",
-			reported_key);
+		pr_info("Unknown key with type 0x%04x and code 0x%04x pressed\n",
+			type, code);
 		return;
 	}
 
-	pr_debug("Key %x pressed\n", reported_key);
+	pr_debug("Key with type 0x%04x and code 0x%04x pressed\n", type, code);
 
 	/* Don't report brightness notifications that will also come via ACPI */
 	if ((key->keycode == KEY_BRIGHTNESSUP ||
@@ -295,7 +313,7 @@ static void dell_wmi_process_key(int reported_key)
 	    acpi_video_handles_brightness_key_presses())
 		return;
 
-	if (reported_key == 0xe025 && !wmi_requires_smbios_request)
+	if (type == 0x0000 && code == 0xe025 && !wmi_requires_smbios_request)
 		return;
 
 	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true);
@@ -333,18 +351,6 @@ static void dell_wmi_notify(u32 value, void *context)
 
 	buffer_entry = (u16 *)obj->buffer.pointer;
 	buffer_size = obj->buffer.length/2;
-
-	if (!dell_new_hk_type) {
-		if (buffer_size >= 3 && buffer_entry[1] == 0x0)
-			dell_wmi_process_key(buffer_entry[2]);
-		else if (buffer_size >= 2)
-			dell_wmi_process_key(buffer_entry[1]);
-		else
-			pr_info("Received unknown WMI event\n");
-		kfree(obj);
-		return;
-	}
-
 	buffer_end = buffer_entry + buffer_size;
 
 	/*
@@ -379,62 +385,18 @@ static void dell_wmi_notify(u32 value, void *context)
 		pr_debug("Process buffer (%*ph)\n", len*2, buffer_entry);
 
 		switch (buffer_entry[1]) {
-		case 0x00:
-			for (i = 2; i < len; ++i) {
-				switch (buffer_entry[i]) {
-				case 0xe043:
-					/* NIC Link is Up */
-					pr_debug("NIC Link is Up\n");
-					break;
-				case 0xe044:
-					/* NIC Link is Down */
-					pr_debug("NIC Link is Down\n");
-					break;
-				case 0xe045:
-					/* Unknown event but defined in DSDT */
-				default:
-					/* Unknown event */
-					pr_info("Unknown WMI event type 0x00: "
-						"0x%x\n", (int)buffer_entry[i]);
-					break;
-				}
-			}
+		case 0x0000: /* One key pressed or event occurred */
+			if (len > 2)
+				dell_wmi_process_key(0x0000, buffer_entry[2]);
+			/* Other entries could contain additional information */
 			break;
-		case 0x10:
-			/* Keys pressed */
+		case 0x0010: /* Sequence of keys pressed */
+		case 0x0011: /* Sequence of events occurred */
 			for (i = 2; i < len; ++i)
-				dell_wmi_process_key(buffer_entry[i]);
-			break;
-		case 0x11:
-			for (i = 2; i < len; ++i) {
-				switch (buffer_entry[i]) {
-				case 0xfff0:
-					/* Battery unplugged */
-					pr_debug("Battery unplugged\n");
-					break;
-				case 0xfff1:
-					/* Battery inserted */
-					pr_debug("Battery inserted\n");
-					break;
-				case 0x01e1:
-				case 0x02ea:
-				case 0x02eb:
-				case 0x02ec:
-				case 0x02f6:
-					/* Keyboard backlight level changed */
-					pr_debug("Keyboard backlight level "
-						 "changed\n");
-					break;
-				default:
-					/* Unknown event */
-					pr_info("Unknown WMI event type 0x11: "
-						"0x%x\n", (int)buffer_entry[i]);
-					break;
-				}
-			}
+				dell_wmi_process_key(buffer_entry[1],
+						     buffer_entry[i]);
 			break;
-		default:
-			/* Unknown event */
+		default: /* Unknown event */
 			pr_info("Unknown WMI event type 0x%x\n",
 				(int)buffer_entry[1]);
 			break;
@@ -459,7 +421,6 @@ static bool have_scancode(u32 scancode, const struct key_entry *keymap, int len)
 }
 
 static void __init handle_dmi_entry(const struct dmi_header *dm,
-
 				    void *opaque)
 
 {
@@ -467,7 +428,6 @@ static void __init handle_dmi_entry(const struct dmi_header *dm,
 	struct dell_bios_hotkey_table *table;
 	int hotkey_num, i, pos = 0;
 	struct key_entry *keymap;
-	int num_bios_keys;
 
 	if (results->err || results->keymap)
 		return;		/* We already found the hotkey table. */
@@ -491,8 +451,7 @@ static void __init handle_dmi_entry(const struct dmi_header *dm,
 		return;
 	}
 
-	keymap = kcalloc(hotkey_num + ARRAY_SIZE(dell_wmi_extra_keymap) + 1,
-			 sizeof(struct key_entry), GFP_KERNEL);
+	keymap = kcalloc(hotkey_num, sizeof(struct key_entry), GFP_KERNEL);
 	if (!keymap) {
 		results->err = -ENOMEM;
 		return;
@@ -529,31 +488,15 @@ static void __init handle_dmi_entry(const struct dmi_header *dm,
 		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[pos].type = KE_END;
-
 	results->keymap = keymap;
+	results->keymap_size = pos;
 }
 
 static int __init dell_wmi_input_setup(void)
 {
 	struct dell_dmi_results dmi_results = {};
-	int err;
+	struct key_entry *keymap;
+	int err, i, pos = 0;
 
 	dell_wmi_input_dev = input_allocate_device();
 	if (!dell_wmi_input_dev)
@@ -577,21 +520,71 @@ static int __init dell_wmi_input_setup(void)
 		goto err_free_dev;
 	}
 
-	if (dmi_results.keymap) {
-		dell_new_hk_type = true;
+	keymap = kcalloc(dmi_results.keymap_size +
+			 ARRAY_SIZE(dell_wmi_keymap_type_0000) +
+			 ARRAY_SIZE(dell_wmi_keymap_type_0010) +
+			 ARRAY_SIZE(dell_wmi_keymap_type_0011) +
+			 1,
+			 sizeof(struct key_entry), GFP_KERNEL);
+	if (!keymap) {
+		kfree(dmi_results.keymap);
+		err = -ENOMEM;
+		goto err_free_dev;
+	}
 
-		err = sparse_keymap_setup(dell_wmi_input_dev,
-					  dmi_results.keymap, NULL);
+	/* Append table with events of type 0x0010 which comes from DMI */
+	for (i = 0; i < dmi_results.keymap_size; i++) {
+		keymap[pos] = dmi_results.keymap[i];
+		keymap[pos].code |= (0x0010 << 16);
+		pos++;
+	}
+
+	kfree(dmi_results.keymap);
+
+	/* Append table with extra events of type 0x0010 which are not in DMI */
+	for (i = 0; i < ARRAY_SIZE(dell_wmi_keymap_type_0010); i++) {
+		const struct key_entry *entry = &dell_wmi_keymap_type_0010[i];
 
 		/*
-		 * Sparse keymap library makes a copy of keymap so we
-		 * don't need the original one that was allocated.
+		 * 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.
 		 */
-		kfree(dmi_results.keymap);
-	} else {
-		err = sparse_keymap_setup(dell_wmi_input_dev,
-					  dell_wmi_legacy_keymap, NULL);
+		if (dmi_results.keymap_size &&
+		    have_scancode(entry->code | (0x0010 << 16),
+				  keymap, dmi_results.keymap_size)
+		   )
+			continue;
+
+		keymap[pos] = *entry;
+		keymap[pos].code |= (0x0010 << 16);
+		pos++;
+	}
+
+	/* Append table with events of type 0x0011 */
+	for (i = 0; i < ARRAY_SIZE(dell_wmi_keymap_type_0011); i++) {
+		keymap[pos] = dell_wmi_keymap_type_0011[i];
+		keymap[pos].code |= (0x0011 << 16);
+		pos++;
 	}
+
+	/*
+	 * Now append also table with "legacy" events of type 0x0000. Some of
+	 * them are reported also on laptops which have scancodes in DMI.
+	 */
+	for (i = 0; i < ARRAY_SIZE(dell_wmi_keymap_type_0000); i++) {
+		keymap[pos] = dell_wmi_keymap_type_0000[i];
+		pos++;
+	}
+
+	keymap[pos].type = KE_END;
+
+	err = sparse_keymap_setup(dell_wmi_input_dev, keymap, NULL);
+	/*
+	 * Sparse keymap library makes a copy of keymap so we don't need the
+	 * original one that was allocated.
+	 */
+	kfree(keymap);
 	if (err)
 		goto err_free_dev;
 
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1423351 — [PATCH v3 2/4] dell-wmi: Sort WMI event codes and update comments

For better readability of keymap table, sort events by codes and also
update comments for events to be more informative.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Michał Kępień <kernel@kempniu.pl>
Reviewed-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-wmi.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 4d23c91..b3b9970 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -88,29 +88,29 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
 static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 	{ KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
 
-	{ KE_KEY, 0xe009, { KEY_EJECTCD } },
-
-	/* These also contain the brightness level at offset 6 */
-	{ KE_KEY, 0xe006, { KEY_BRIGHTNESSUP } },
-	{ KE_KEY, 0xe005, { KEY_BRIGHTNESSDOWN } },
+	/* Key code is followed by brightness level */
+	{ KE_KEY,    0xe005, { KEY_BRIGHTNESSDOWN } },
+	{ KE_KEY,    0xe006, { KEY_BRIGHTNESSUP } },
 
 	/* Battery health status button */
-	{ KE_KEY, 0xe007, { KEY_BATTERY } },
+	{ KE_KEY,    0xe007, { KEY_BATTERY } },
 
-	/* Radio devices state change */
+	/* Radio devices state change, key code is followed by other values */
 	{ KE_IGNORE, 0xe008, { KEY_RFKILL } },
 
-	/* The next device is at offset 6, the active devices are at
-	   offset 8 and the attached devices at offset 10 */
-	{ KE_KEY, 0xe00b, { KEY_SWITCHVIDEOMODE } },
+	{ KE_KEY,    0xe009, { KEY_EJECTCD } },
+
+	/* Key code is followed by: next, active and attached devices */
+	{ KE_KEY,    0xe00b, { KEY_SWITCHVIDEOMODE } },
 
+	/* Key code is followed by keyboard illumination level */
 	{ KE_IGNORE, 0xe00c, { KEY_KBDILLUMTOGGLE } },
 
 	/* BIOS error detected */
 	{ KE_IGNORE, 0xe00d, { KEY_RESERVED } },
 
 	/* Wifi Catcher */
-	{ KE_KEY, 0xe011, {KEY_PROG2 } },
+	{ KE_KEY,    0xe011, { KEY_PROG2 } },
 
 	/* Ambient light sensor toggle */
 	{ KE_IGNORE, 0xe013, { KEY_RESERVED } },
@@ -118,12 +118,14 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
 
 	/* Dell Instant Launch key */
-	{ KE_KEY, 0xe025, { KEY_PROG4 } },
-	{ KE_KEY, 0xe029, { KEY_PROG4 } },
+	{ KE_KEY,    0xe025, { KEY_PROG4 } },
 
 	/* Audio panel key */
 	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
 
+	/* Dell Instant Launch key */
+	{ KE_KEY,    0xe029, { KEY_PROG4 } },
+
 	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
 	{ KE_IGNORE, 0xe030, { KEY_VOLUMEUP } },
 	{ KE_IGNORE, 0xe033, { KEY_KBDILLUMUP } },
@@ -146,6 +148,7 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 	{ KE_IGNORE, 0xe0f7, { KEY_MUTE } },
 	{ KE_IGNORE, 0xe0f8, { KEY_VOLUMEDOWN } },
 	{ KE_IGNORE, 0xe0f9, { KEY_VOLUMEUP } },
+
 	{ KE_END, 0 }
 };
 
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1423355 — [PATCH v3 3/4] dell-wmi: Add information about other WMI event codes

ACPI DSDT tables have defined other WMI codes, but does not contain any
description when those codes are emitted. Some other codes can be found in
logs on internet. In this patch are all which I saw, but lot of them are
not tested properly (e.g. for duplicate events with AT keyboard). Now we
have all WMI event codes at one place and in future after proper testing
those codes can be correctly enabled or disabled...

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Michał Kępień <kernel@kempniu.pl>
Tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Reviewed-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-wmi.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index b3b9970..41ae79d 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -109,6 +109,9 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 	/* BIOS error detected */
 	{ KE_IGNORE, 0xe00d, { KEY_RESERVED } },
 
+	/* Unknown, defined in ACPI DSDT */
+	/* { KE_IGNORE, 0xe00e, { KEY_RESERVED } }, */
+
 	/* Wifi Catcher */
 	{ KE_KEY,    0xe011, { KEY_PROG2 } },
 
@@ -117,21 +120,45 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 
 	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
 
+	/* Unknown, defined in ACPI DSDT */
+	/* { KE_IGNORE, 0xe023, { KEY_RESERVED } }, */
+
+	/* Untested, Dell Instant Launch key on Inspiron 7520 */
+	/* { KE_IGNORE, 0xe024, { KEY_RESERVED } }, */
+
 	/* Dell Instant Launch key */
 	{ KE_KEY,    0xe025, { KEY_PROG4 } },
 
 	/* Audio panel key */
 	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
 
+	/* Untested, Multimedia key on Dell Vostro 3560 */
+	/* { KE_IGNORE, 0xe028, { KEY_RESERVED } }, */
+
 	/* Dell Instant Launch key */
 	{ KE_KEY,    0xe029, { KEY_PROG4 } },
 
+	/* Untested, Windows Mobility Center button on Inspiron 7520 */
+	/* { KE_IGNORE, 0xe02a, { KEY_RESERVED } }, */
+
+	/* Unknown, defined in ACPI DSDT */
+	/* { KE_IGNORE, 0xe02b, { KEY_RESERVED } }, */
+
+	/* Untested, Dell Audio With Preset Switch button on Inspiron 7520 */
+	/* { KE_IGNORE, 0xe02c, { KEY_RESERVED } }, */
+
 	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
 	{ KE_IGNORE, 0xe030, { KEY_VOLUMEUP } },
 	{ KE_IGNORE, 0xe033, { KEY_KBDILLUMUP } },
 	{ KE_IGNORE, 0xe034, { KEY_KBDILLUMDOWN } },
 	{ KE_IGNORE, 0xe03a, { KEY_CAPSLOCK } },
 
+	/* NIC Link is Up */
+	{ KE_IGNORE, 0xe043, { KEY_RESERVED } },
+
+	/* NIC Link is Down */
+	{ KE_IGNORE, 0xe044, { KEY_RESERVED } },
+
 	/*
 	 * This entry is very suspicious!
 	 * Originally Matthew Garrett created this dell-wmi driver specially for
@@ -144,7 +171,15 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 	 */
 	{ KE_IGNORE, 0xe045, { KEY_NUMLOCK } },
 
+	/* Scroll lock and also going to tablet mode on portable devices */
 	{ KE_IGNORE, 0xe046, { KEY_SCROLLLOCK } },
+
+	/* Untested, going from tablet mode on portable devices */
+	/* { KE_IGNORE, 0xe047, { KEY_RESERVED } }, */
+
+	/* Dell Support Center key */
+	{ KE_IGNORE, 0xe06e, { KEY_RESERVED } },
+
 	{ KE_IGNORE, 0xe0f7, { KEY_MUTE } },
 	{ KE_IGNORE, 0xe0f8, { KEY_VOLUMEDOWN } },
 	{ KE_IGNORE, 0xe0f9, { KEY_VOLUMEUP } },
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]

#1423661

On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
> First patch describe problem about 0xe045 code. Second and third are just
> cosmetic and last rework code which processing WMI events. It should be
> properly tested on more Dell machines, to check that everything is still
> working correctly.

Is this "should be properly tested on more Dell machines" still the case? Are
you ready for this to go into linux-next?

> 
> Changes since v2:
> * Updated description for 2/4 and 4/4
> * Fixed comments over 80 characters in 2/4 and 4/4
> * Use switch fall-through in 4/4
> * Added Michał's Reviewed-by line
> 
> Changes since v1:
> * Fixed comments
> * Fixed memory leak
> * Added Tested-By lines
> * Added event 0xe06e
> 
> Pali Rohár (4):
>   dell-wmi: Ignore WMI event code 0xe045
>   dell-wmi: Sort WMI event codes and update comments
>   dell-wmi: Add information about other WMI event codes
>   dell-wmi: Generate one sparse keymap for all machines
> 
>  drivers/platform/x86/dell-wmi.c |  290 ++++++++++++++++++++++-----------------
>  1 file changed, 166 insertions(+), 124 deletions(-)
> 
> -- 
> 1.7.9.5
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1423750

On Wednesday 15 June 2016 20:19:58 Darren Hart wrote:
> On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
> > First patch describe problem about 0xe045 code. Second and third are just
> > cosmetic and last rework code which processing WMI events. It should be
> > properly tested on more Dell machines, to check that everything is still
> > working correctly.
> 
> Is this "should be properly tested on more Dell machines" still the case? Are
> you ready for this to go into linux-next?

Series should be OK, but I would like to see if someone else test this
series... Gabriele, Alex or Andy? Do you have time?

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1424525

On 16/06/2016 09:33, Pali Rohár wrote:
> On Wednesday 15 June 2016 20:19:58 Darren Hart wrote:
>> On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
>>> First patch describe problem about 0xe045 code. Second and third are just
>>> cosmetic and last rework code which processing WMI events. It should be
>>> properly tested on more Dell machines, to check that everything is still
>>> working correctly.
>>
>> Is this "should be properly tested on more Dell machines" still the case? Are
>> you ready for this to go into linux-next?
> 
> Series should be OK, but I would like to see if someone else test this
> series... Gabriele, Alex or Andy? Do you have time?

I tested this series and everything seems to be working fine here.

Tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>

[toc] | [prev] | [next] | [standalone]

#1427930

On Thu, Jun 16, 2016 at 09:33:02AM +0200, Pali Rohár wrote:
> On Wednesday 15 June 2016 20:19:58 Darren Hart wrote:
> > On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
> > > First patch describe problem about 0xe045 code. Second and third are just
> > > cosmetic and last rework code which processing WMI events. It should be
> > > properly tested on more Dell machines, to check that everything is still
> > > working correctly.
> > 
> > Is this "should be properly tested on more Dell machines" still the case? Are
> > you ready for this to go into linux-next?
> 
> Series should be OK, but I would like to see if someone else test this
> series... Gabriele, Alex or Andy? Do you have time?

I've queued these to for-next to ensure we get several weeks there.

-- 
Darren Hart
Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1428039

On Thu, Jun 16, 2016 at 09:33:02AM +0200, Pali Rohár wrote:
> On Wednesday 15 June 2016 20:19:58 Darren Hart wrote:
> > On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
> > > First patch describe problem about 0xe045 code. Second and third are just
> > > cosmetic and last rework code which processing WMI events. It should be
> > > properly tested on more Dell machines, to check that everything is still
> > > working correctly.
> > 
> > Is this "should be properly tested on more Dell machines" still the case? Are
> > you ready for this to go into linux-next?
> 
> Series should be OK, but I would like to see if someone else test this
> series... Gabriele, Alex or Andy? Do you have time?

Tested on a Dell XPS 13 2016 (9350). All hotkeys appear to work without warning
messages. I didn't get anything out of Fn-F8 which has a picture of a laptop and
white screen behind it. Not sure what that is supposed to do - if it was meant
to blank the screen, it did not, perhaps it is meant to toggle screen outputs...
will test that when I have access to an external display.

> 
> -- 
> Pali Rohár
> pali.rohar@gmail.com
> 

-- 
Darren Hart
Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1428045

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Tuesday, June 21, 2016 1:06 PM
> To: Pali Rohár <pali.rohar@gmail.com>
> Cc: Gabriele Mazzotta <gabriele.mzt@gmail.com>; Andy Lutomirski
> <luto@kernel.org>; Alex Hung <alex.hung@canonical.com>; Matthew
> Garrett <mjg59@srcf.ucam.org>; Michał Kępień <kernel@kempniu.pl>;
> Limonciello, Mario <Mario_Limonciello@Dell.com>; platform-driver-
> x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling
> 
> On Thu, Jun 16, 2016 at 09:33:02AM +0200, Pali Rohár wrote:
> > On Wednesday 15 June 2016 20:19:58 Darren Hart wrote:
> > > On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
> > > > First patch describe problem about 0xe045 code. Second and third are
> just
> > > > cosmetic and last rework code which processing WMI events. It should
> be
> > > > properly tested on more Dell machines, to check that everything is still
> > > > working correctly.
> > >
> > > Is this "should be properly tested on more Dell machines" still the case?
> Are
> > > you ready for this to go into linux-next?
> >
> > Series should be OK, but I would like to see if someone else test this
> > series... Gabriele, Alex or Andy? Do you have time?
> 
> Tested on a Dell XPS 13 2016 (9350). All hotkeys appear to work without
> warning
> messages. I didn't get anything out of Fn-F8 which has a picture of a laptop
> and
> white screen behind it. Not sure what that is supposed to do - if it was meant
> to blank the screen, it did not, perhaps it is meant to toggle screen outputs...
> will test that when I have access to an external display.

That key is meant to toggle screen outputs.  I believe it's still done by the EC
emitting <super> + p.  If your WM doesn't recognize that, it won't do much,
but you can see in xev the key combinations.

[toc] | [prev] | [next] | [standalone]

#1428053

[Multipart message — attachments visible in raw view] — view raw

On Tuesday 21 June 2016 20:16:09 Mario_Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Darren Hart [mailto:dvhart@infradead.org]
> > Sent: Tuesday, June 21, 2016 1:06 PM
> > To: Pali Rohár <pali.rohar@gmail.com>
> > Cc: Gabriele Mazzotta <gabriele.mzt@gmail.com>; Andy Lutomirski
> > <luto@kernel.org>; Alex Hung <alex.hung@canonical.com>; Matthew
> > Garrett <mjg59@srcf.ucam.org>; Michał Kępień <kernel@kempniu.pl>;
> > Limonciello, Mario <Mario_Limonciello@Dell.com>; platform-driver-
> > x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code
> > handling
> > 
> > On Thu, Jun 16, 2016 at 09:33:02AM +0200, Pali Rohár wrote:
> > > On Wednesday 15 June 2016 20:19:58 Darren Hart wrote:
> > > > On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
> > > > > First patch describe problem about 0xe045 code. Second and
> > > > > third are
> > 
> > just
> > 
> > > > > cosmetic and last rework code which processing WMI events. It
> > > > > should
> > 
> > be
> > 
> > > > > properly tested on more Dell machines, to check that
> > > > > everything is still working correctly.
> > > > 
> > > > Is this "should be properly tested on more Dell machines" still
> > > > the case?
> > 
> > Are
> > 
> > > > you ready for this to go into linux-next?
> > > 
> > > Series should be OK, but I would like to see if someone else test
> > > this series... Gabriele, Alex or Andy? Do you have time?
> > 
> > Tested on a Dell XPS 13 2016 (9350). All hotkeys appear to work
> > without warning
> > messages. I didn't get anything out of Fn-F8 which has a picture of
> > a laptop and
> > white screen behind it. Not sure what that is supposed to do - if
> > it was meant to blank the screen, it did not, perhaps it is meant
> > to toggle screen outputs... will test that when I have access to
> > an external display.
> 
> That key is meant to toggle screen outputs.  I believe it's still
> done by the EC emitting <super> + p.  If your WM doesn't recognize
> that, it won't do much, but you can see in xev the key combinations.

I still do not understand this stupidity, pressing *one* key cause 
emitting two keys to OS and then OS needs to handle combinations of keys 
and acts on it correctly.... (like windows manager)

Is there some way to disable this insane nonsense activity of BIOS, 
firmware or whatever it is doing in HW to send *one* key scancode when 
pressing *one* key?

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1428056

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, June 21, 2016 1:29 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; gabriele.mzt@gmail.com; luto@kernel.org;
> alex.hung@canonical.com; mjg59@srcf.ucam.org; kernel@kempniu.pl;
> platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling
> 
> On Tuesday 21 June 2016 20:16:09 Mario_Limonciello@dell.com wrote:
> > > -----Original Message-----
> > > From: Darren Hart [mailto:dvhart@infradead.org]
> > > Sent: Tuesday, June 21, 2016 1:06 PM
> > > To: Pali Rohár <pali.rohar@gmail.com>
> > > Cc: Gabriele Mazzotta <gabriele.mzt@gmail.com>; Andy Lutomirski
> > > <luto@kernel.org>; Alex Hung <alex.hung@canonical.com>; Matthew
> > > Garrett <mjg59@srcf.ucam.org>; Michał Kępień <kernel@kempniu.pl>;
> > > Limonciello, Mario <Mario_Limonciello@Dell.com>; platform-driver-
> > > x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code
> > > handling
> > >
> > > On Thu, Jun 16, 2016 at 09:33:02AM +0200, Pali Rohár wrote:
> > > > On Wednesday 15 June 2016 20:19:58 Darren Hart wrote:
> > > > > On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
> > > > > > First patch describe problem about 0xe045 code. Second and
> > > > > > third are
> > >
> > > just
> > >
> > > > > > cosmetic and last rework code which processing WMI events. It
> > > > > > should
> > >
> > > be
> > >
> > > > > > properly tested on more Dell machines, to check that
> > > > > > everything is still working correctly.
> > > > >
> > > > > Is this "should be properly tested on more Dell machines" still
> > > > > the case?
> > >
> > > Are
> > >
> > > > > you ready for this to go into linux-next?
> > > >
> > > > Series should be OK, but I would like to see if someone else test
> > > > this series... Gabriele, Alex or Andy? Do you have time?
> > >
> > > Tested on a Dell XPS 13 2016 (9350). All hotkeys appear to work
> > > without warning
> > > messages. I didn't get anything out of Fn-F8 which has a picture of
> > > a laptop and
> > > white screen behind it. Not sure what that is supposed to do - if
> > > it was meant to blank the screen, it did not, perhaps it is meant
> > > to toggle screen outputs... will test that when I have access to
> > > an external display.
> >
> > That key is meant to toggle screen outputs.  I believe it's still
> > done by the EC emitting <super> + p.  If your WM doesn't recognize
> > that, it won't do much, but you can see in xev the key combinations.
> 
> I still do not understand this stupidity, pressing *one* key cause
> emitting two keys to OS and then OS needs to handle combinations of keys
> and acts on it correctly.... (like windows manager)
> 
> Is there some way to disable this insane nonsense activity of BIOS,
> firmware or whatever it is doing in HW to send *one* key scancode when
> pressing *one* key?
> 

I talked to the EC team about this a while back when it was first implemented.
That's not possible without _OSI detection of Linux.  _OSI detection could be 
used to relay to the EC to behave differently, but otherwise the EC will have 
no idea what OS it's on for which way to behave.

I don't remember all the history behind the switch over from a single scan code 
To <super> + P, but I think it's along the lines of Windows 8/Windows 10 allow
you to iterate the display selection menu based upon holding super and pressing
P multiple times and waiting for you to stop.  

Sending a single scan code will change displays immediately, so having the 
EC send super+p unifies the behavior of fn-f8 and super+p.

[toc] | [prev] | [next] | [standalone]

#1428671

On Tuesday 21 June 2016 18:37:19 Mario_Limonciello@Dell.com wrote:
> > -----Original Message-----
> > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > Sent: Tuesday, June 21, 2016 1:29 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: dvhart@infradead.org; gabriele.mzt@gmail.com; luto@kernel.org;
> > alex.hung@canonical.com; mjg59@srcf.ucam.org; kernel@kempniu.pl;
> > platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code handling
> > 
> > On Tuesday 21 June 2016 20:16:09 Mario_Limonciello@dell.com wrote:
> > > > -----Original Message-----
> > > > From: Darren Hart [mailto:dvhart@infradead.org]
> > > > Sent: Tuesday, June 21, 2016 1:06 PM
> > > > To: Pali Rohár <pali.rohar@gmail.com>
> > > > Cc: Gabriele Mazzotta <gabriele.mzt@gmail.com>; Andy Lutomirski
> > > > <luto@kernel.org>; Alex Hung <alex.hung@canonical.com>; Matthew
> > > > Garrett <mjg59@srcf.ucam.org>; Michał Kępień <kernel@kempniu.pl>;
> > > > Limonciello, Mario <Mario_Limonciello@Dell.com>; platform-driver-
> > > > x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v3 0/4] dell-wmi: Changes in WMI event code
> > > > handling
> > > >
> > > > On Thu, Jun 16, 2016 at 09:33:02AM +0200, Pali Rohár wrote:
> > > > > On Wednesday 15 June 2016 20:19:58 Darren Hart wrote:
> > > > > > On Wed, Jun 15, 2016 at 09:49:09PM +0200, Pali Rohár wrote:
> > > > > > > First patch describe problem about 0xe045 code. Second and
> > > > > > > third are
> > > >
> > > > just
> > > >
> > > > > > > cosmetic and last rework code which processing WMI events. It
> > > > > > > should
> > > >
> > > > be
> > > >
> > > > > > > properly tested on more Dell machines, to check that
> > > > > > > everything is still working correctly.
> > > > > >
> > > > > > Is this "should be properly tested on more Dell machines" still
> > > > > > the case?
> > > >
> > > > Are
> > > >
> > > > > > you ready for this to go into linux-next?
> > > > >
> > > > > Series should be OK, but I would like to see if someone else test
> > > > > this series... Gabriele, Alex or Andy? Do you have time?
> > > >
> > > > Tested on a Dell XPS 13 2016 (9350). All hotkeys appear to work
> > > > without warning
> > > > messages. I didn't get anything out of Fn-F8 which has a picture of
> > > > a laptop and
> > > > white screen behind it. Not sure what that is supposed to do - if
> > > > it was meant to blank the screen, it did not, perhaps it is meant
> > > > to toggle screen outputs... will test that when I have access to
> > > > an external display.
> > >
> > > That key is meant to toggle screen outputs.  I believe it's still
> > > done by the EC emitting <super> + p.  If your WM doesn't recognize
> > > that, it won't do much, but you can see in xev the key combinations.
> > 
> > I still do not understand this stupidity, pressing *one* key cause
> > emitting two keys to OS and then OS needs to handle combinations of keys
> > and acts on it correctly.... (like windows manager)
> > 
> > Is there some way to disable this insane nonsense activity of BIOS,
> > firmware or whatever it is doing in HW to send *one* key scancode when
> > pressing *one* key?
> > 
> 
> I talked to the EC team about this a while back when it was first implemented.
> That's not possible without _OSI detection of Linux.  _OSI detection could be 
> used to relay to the EC to behave differently, but otherwise the EC will have 
> no idea what OS it's on for which way to behave.

ACPI code should not behave differently for different operating systems.
If there is bug in kernel, report bug to kernel, subtree maintainer for
fixing it. And not doing workaround and hacks in ACPI.

In this case there could be (standardized) ACPI function for turning off
this nonsense functionality and supported kernel could call it.

Is not there such ACPI function? Or Dell specific SMBIOS call?

> I don't remember all the history behind the switch over from a single scan code 
> To <super> + P, but I think it's along the lines of Windows 8/Windows 10 allow
> you to iterate the display selection menu based upon holding super and pressing
> P multiple times and waiting for you to stop.  

Windows systems doing stupid things and fixing their bugs in ACPI is
wrong. It broke for example this key on all other systems (Linux too).

> Sending a single scan code will change displays immediately, so having the 
> EC send super+p unifies the behavior of fn-f8 and super+p.

And due to this OS/kernel cannot distinguish between Fn-F8 and Super+p
keys...

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1428786

> > I talked to the EC team about this a while back when it was first implemented.
> > That's not possible without _OSI detection of Linux.  _OSI detection
> > could be used to relay to the EC to behave differently, but otherwise
> > the EC will have no idea what OS it's on for which way to behave.
> 
> ACPI code should not behave differently for different operating systems.
> If there is bug in kernel, report bug to kernel, subtree maintainer for fixing it.
> And not doing workaround and hacks in ACPI.

This isn't ACPI code, this is EC code.

> 
> In this case there could be (standardized) ACPI function for turning off this
> nonsense functionality and supported kernel could call it.
> 

I think you might have interpreted my response differently than I intended.
I know that the Linux kernel has chosen to respond as the latest Windows
version for _OSI, and that's why it's not possible to do a different behavior
for Linux and Windows.

If there is a desire to go down the route of having different behavior for 
what the EC does in different OS'es, _OSI is only way to accomplish this.

> Is not there such ACPI function? Or Dell specific SMBIOS call?
> 

I'm not aware of any standard ACPI function or Dell function for this type 
of request.  Last time this was discussed I was told the EC would emit 
single display scan code for Windows < 7 (as Windows Vista and earlier
doesn't support super + p).  Windows > 7 (as detected  by _OSI and 
passed to EC) will emit super + p.

> > I don't remember all the history behind the switch over from a single
> > scan code To <super> + P, but I think it's along the lines of Windows
> > 8/Windows 10 allow you to iterate the display selection menu based
> > upon holding super and pressing P multiple times and waiting for you to stop.
> 
> Windows systems doing stupid things and fixing their bugs in ACPI is wrong. It
> broke for example this key on all other systems (Linux too).
> 

There's no bug in this behavior, it's intended behavior.

Like I said, previously display switch hotkey would immediately cycle outputs.
The behavior followed with super + p allows for OS to toggle through a menu
of outputs in this OS.

" Toggle through the projection mode (new with Windows 7)."
https://msdn.microsoft.com/en-us/library/ms971323.aspx

> > Sending a single scan code will change displays immediately, so having
> > the EC send super+p unifies the behavior of fn-f8 and super+p.
> 
> And due to this OS/kernel cannot distinguish between Fn-F8 and Super+p keys...

Which is intended behavior from system designer's perspective.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web