[PATCH] dell-wmi: Update code for processing WMI events

[PATCH] dell-wmi: Update code for processing WMI events

[Pali Rohár]

WMI buffer can contains more events. First value in buffer is length of event
followed by data of specified length. After that is next length and next data.
When length is zero then there is no more events in bufffer.

This patch adds support for processing all events in buffer (not only first)
and parse more event types (not only hotkey events). Because of variable length
of events sometimes BIOS fills more hotkeys (or other values) into single WMI
event. In this case this patch process also these multiple hotkeys (and not
only first one).

Some event types are just ignored because kernel is not interested for them
(e.g. NIC Link status, battery unplug, ...).

This patch is based on DSDT table from Dell Latitude E6440. Code should be
backward compatible so will process other events of old types same as before
this patch.

This patch also fixes problem when in kernel log are written messages about
unknown WMI events. Now all know events are parsed and those which are not
interesting for kernel are dropped without unknown message.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/platform/x86/dell-wmi.c |  157 +++++++++++++++++++++++++++++++--------
 1 file changed, 127 insertions(+), 30 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 25721bf..3d15949 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -145,11 +145,35 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
 
 static struct input_dev *dell_wmi_input_dev;
 
+static void dell_wmi_process_key(int reported_key)
+{
+	const struct key_entry *key;
+
+	key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
+						reported_key);
+	if (!key) {
+		pr_info("Unknown key %x pressed\n", reported_key);
+		return;
+	}
+
+	pr_debug("Key %x pressed\n", reported_key);
+
+	/* Don't report brightness notifications that will also come via ACPI */
+	if ((key->keycode == KEY_BRIGHTNESSUP ||
+	     key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video)
+		return;
+
+	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true);
+}
+
 static void dell_wmi_notify(u32 value, void *context)
 {
 	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *obj;
 	acpi_status status;
+	acpi_size buffer_size;
+	u16 *buffer_entry, *buffer_end;
+	int len, i;
 
 	status = wmi_get_event_data(value, &response);
 	if (status != AE_OK) {
@@ -158,44 +182,117 @@ static void dell_wmi_notify(u32 value, void *context)
 	}
 
 	obj = (union acpi_object *)response.pointer;
+	if (!obj) {
+		pr_info("no response\n");
+		return;
+	}
 
-	if (obj && obj->type == ACPI_TYPE_BUFFER) {
-		const struct key_entry *key;
-		int reported_key;
-		u16 *buffer_entry = (u16 *)obj->buffer.pointer;
-		int buffer_size = obj->buffer.length/2;
-
-		if (buffer_size >= 2 && dell_new_hk_type && buffer_entry[1] != 0x10) {
-			pr_info("Received unknown WMI event (0x%x)\n",
-				buffer_entry[1]);
-			kfree(obj);
-			return;
-		}
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		pr_info("bad response type %x\n", obj->type);
+		kfree(obj);
+		return;
+	}
+
+	pr_debug("Received WMI event (%*ph)\n",
+		obj->buffer.length, obj->buffer.pointer);
 
-		if (buffer_size >= 3 && (dell_new_hk_type || buffer_entry[1] == 0x0))
-			reported_key = (int)buffer_entry[2];
+	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)
-			reported_key = (int)buffer_entry[1] & 0xffff;
-		else {
+			dell_wmi_process_key(buffer_entry[1]);
+		else
 			pr_info("Received unknown WMI event\n");
-			kfree(obj);
-			return;
+		kfree(obj);
+		return;
+	}
+
+	buffer_end = buffer_entry + buffer_size;
+
+	while (buffer_entry < buffer_end) {
+
+		len = buffer_entry[0];
+		if (len == 0)
+			break;
+
+		len++;
+
+		if (buffer_entry+len > buffer_end) {
+			pr_info("Invalid length of WMI event\n");
+			break;
 		}
 
-		key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
-							reported_key);
-		if (!key) {
-			pr_info("Unknown key %x pressed\n", reported_key);
-		} else if ((key->keycode == KEY_BRIGHTNESSUP ||
-			    key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video) {
-			/* Don't report brightness notifications that will also
-			 * come via ACPI */
-			;
-		} else {
-			sparse_keymap_report_entry(dell_wmi_input_dev, key,
-						   1, true);
+		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;
+				}
+			}
+			break;
+		case 0x10:
+			/* Keys pressed */
+			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;
+				}
+			}
+			break;
+		default:
+			/* Unknown event */
+			pr_info("Unknown WMI event type 0x%x\n",
+				(int)buffer_entry[1]);
+			break;
 		}
+
+		buffer_entry += len;
+
 	}
+
 	kfree(obj);
 }
 
-- 
1.7.9.5

Re: [PATCH] dell-wmi: Update code for processing WMI events

[Darren Hart]

On Tue, Oct 21, 2014 at 12:15:24AM +0200, Pali Rohár wrote:
> WMI buffer can contains more events. First value in buffer is length of event
> followed by data of specified length. After that is next length and next data.
> When length is zero then there is no more events in bufffer.
> 
> This patch adds support for processing all events in buffer (not only first)
> and parse more event types (not only hotkey events). Because of variable length
> of events sometimes BIOS fills more hotkeys (or other values) into single WMI
> event. In this case this patch process also these multiple hotkeys (and not
> only first one).
> 
> Some event types are just ignored because kernel is not interested for them
> (e.g. NIC Link status, battery unplug, ...).
> 
> This patch is based on DSDT table from Dell Latitude E6440. Code should be
> backward compatible so will process other events of old types same as before
> this patch.
> 
> This patch also fixes problem when in kernel log are written messages about
> unknown WMI events. Now all know events are parsed and those which are not
> interesting for kernel are dropped without unknown message.

This should probably be done in a separate patch.

> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Tested-by: Pali Rohár <pali.rohar@gmail.com>

Well yes, I should hope so ;-)

> ---
>  drivers/platform/x86/dell-wmi.c |  157 +++++++++++++++++++++++++++++++--------
>  1 file changed, 127 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 25721bf..3d15949 100644
> --- a/drivers/platform/x86/dell-wmi.c
> +++ b/drivers/platform/x86/dell-wmi.c
> @@ -145,11 +145,35 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
>  
>  static struct input_dev *dell_wmi_input_dev;
>  
> +static void dell_wmi_process_key(int reported_key)
> +{
> +	const struct key_entry *key;
> +
> +	key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
> +						reported_key);
> +	if (!key) {
> +		pr_info("Unknown key %x pressed\n", reported_key);
> +		return;
> +	}
> +
> +	pr_debug("Key %x pressed\n", reported_key);
> +
> +	/* Don't report brightness notifications that will also come via ACPI */
> +	if ((key->keycode == KEY_BRIGHTNESSUP ||
> +	     key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video)
> +		return;
> +
> +	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true);
> +}
> +
>  static void dell_wmi_notify(u32 value, void *context)
>  {
>  	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
>  	union acpi_object *obj;
>  	acpi_status status;
> +	acpi_size buffer_size;
> +	u16 *buffer_entry, *buffer_end;
> +	int len, i;
>  
>  	status = wmi_get_event_data(value, &response);
>  	if (status != AE_OK) {
> @@ -158,44 +182,117 @@ static void dell_wmi_notify(u32 value, void *context)
>  	}
>  
>  	obj = (union acpi_object *)response.pointer;
> +	if (!obj) {
> +		pr_info("no response\n");
> +		return;
> +	}


If you intend to print this, it should probably be a bit more informative. Is
"info" the right level here? I would imagine either WARN if this was a bad
thing, or DEBUG if this is more for debugging the driver.


> -	if (obj && obj->type == ACPI_TYPE_BUFFER) {
> -		const struct key_entry *key;
> -		int reported_key;
> -		u16 *buffer_entry = (u16 *)obj->buffer.pointer;
> -		int buffer_size = obj->buffer.length/2;
> -
> -		if (buffer_size >= 2 && dell_new_hk_type && buffer_entry[1] != 0x10) {
> -			pr_info("Received unknown WMI event (0x%x)\n",
> -				buffer_entry[1]);
> -			kfree(obj);
> -			return;
> -		}
> +	if (obj->type != ACPI_TYPE_BUFFER) {
> +		pr_info("bad response type %x\n", obj->type);
> +		kfree(obj);
> +		return;
> +	}
> +
> +	pr_debug("Received WMI event (%*ph)\n",
> +		obj->buffer.length, obj->buffer.pointer);
>  
> -		if (buffer_size >= 3 && (dell_new_hk_type || buffer_entry[1] == 0x0))
> -			reported_key = (int)buffer_entry[2];
> +	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)
> -			reported_key = (int)buffer_entry[1] & 0xffff;
> -		else {
> +			dell_wmi_process_key(buffer_entry[1]);


Why can we drop the 0xffff mask now?


> +		else
>  			pr_info("Received unknown WMI event\n");
> -			kfree(obj);
> -			return;
> +		kfree(obj);
> +		return;
> +	}
> +
> +	buffer_end = buffer_entry + buffer_size;
> +
> +	while (buffer_entry < buffer_end) {
> +
> +		len = buffer_entry[0];
> +		if (len == 0)
> +			break;
> +
> +		len++;
> +


Why increment len here? Are you trying to avoid a "len + 1" in the comparisons
below? If so, is using "len * 2" in the debug message below correct? Please
clarify.


> +		if (buffer_entry+len > buffer_end) {


See coding style documentation on operators. Please run patches through
checkpatch.


> +			pr_info("Invalid length of WMI event\n");

info doesn't see correct here either.

> +			break;
>  		}
>  
> -		key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
> -							reported_key);
> -		if (!key) {
> -			pr_info("Unknown key %x pressed\n", reported_key);
> -		} else if ((key->keycode == KEY_BRIGHTNESSUP ||
> -			    key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video) {
> -			/* Don't report brightness notifications that will also
> -			 * come via ACPI */
> -			;
> -		} else {
> -			sparse_keymap_report_entry(dell_wmi_input_dev, key,
> -						   1, true);
> +		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;
> +				}
> +			}
> +			break;
> +		case 0x10:
> +			/* Keys pressed */
> +			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;
> +				}
> +			}
> +			break;
> +		default:
> +			/* Unknown event */
> +			pr_info("Unknown WMI event type 0x%x\n",
> +				(int)buffer_entry[1]);
> +			break;
>  		}
> +
> +		buffer_entry += len;
> +
>  	}
> +
>  	kfree(obj);
>  }
>  
> -- 
> 1.7.9.5
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH] dell-wmi: Update code for processing WMI events

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 8854 bytes --]

On Tuesday 21 October 2014 23:32:12 Darren Hart wrote:
> On Tue, Oct 21, 2014 at 12:15:24AM +0200, Pali Rohár wrote:
> > WMI buffer can contains more events. First value in buffer
> > is length of event followed by data of specified length.
> > After that is next length and next data. When length is
> > zero then there is no more events in bufffer.
> > 
> > This patch adds support for processing all events in buffer
> > (not only first) and parse more event types (not only
> > hotkey events). Because of variable length of events
> > sometimes BIOS fills more hotkeys (or other values) into
> > single WMI event. In this case this patch process also
> > these multiple hotkeys (and not only first one).
> > 
> > Some event types are just ignored because kernel is not
> > interested for them (e.g. NIC Link status, battery unplug,
> > ...).
> > 
> > This patch is based on DSDT table from Dell Latitude E6440.
> > Code should be backward compatible so will process other
> > events of old types same as before this patch.
> > 
> > This patch also fixes problem when in kernel log are written
> > messages about unknown WMI events. Now all know events are
> > parsed and those which are not interesting for kernel are
> > dropped without unknown message.
> 
> This should probably be done in a separate patch.
> 

It is not possible, because my patch rewrite code for handling 
events. Kernel does not print "unknown event" messages when it 
parse WMI event and understand specified part.

> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > Tested-by: Pali Rohár <pali.rohar@gmail.com>
> 
> Well yes, I should hope so ;-)
> 
> > ---
> > 
> >  drivers/platform/x86/dell-wmi.c |  157
> >  +++++++++++++++++++++++++++++++-------- 1 file changed,
> >  127 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/dell-wmi.c
> > b/drivers/platform/x86/dell-wmi.c index 25721bf..3d15949
> > 100644
> > --- a/drivers/platform/x86/dell-wmi.c
> > +++ b/drivers/platform/x86/dell-wmi.c
> > @@ -145,11 +145,35 @@ static const u16
> > bios_to_linux_keycode[256] __initconst = {
> > 
> >  static struct input_dev *dell_wmi_input_dev;
> > 
> > +static void dell_wmi_process_key(int reported_key)
> > +{
> > +	const struct key_entry *key;
> > +
> > +	key =
> > sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
> > +						reported_key);
> > +	if (!key) {
> > +		pr_info("Unknown key %x pressed\n", reported_key);
> > +		return;
> > +	}
> > +
> > +	pr_debug("Key %x pressed\n", reported_key);
> > +
> > +	/* Don't report brightness notifications that will also
> > come via ACPI */ +	if ((key->keycode == KEY_BRIGHTNESSUP ||
> > +	     key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video)
> > +		return;
> > +
> > +	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1,
> > true); +}
> > +
> > 
> >  static void dell_wmi_notify(u32 value, void *context)
> >  {
> >  
> >  	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL
> >  	}; union acpi_object *obj;
> >  	acpi_status status;
> > 
> > +	acpi_size buffer_size;
> > +	u16 *buffer_entry, *buffer_end;
> > +	int len, i;
> > 
> >  	status = wmi_get_event_data(value, &response);
> >  	if (status != AE_OK) {
> > 
> > @@ -158,44 +182,117 @@ static void dell_wmi_notify(u32
> > value, void *context)
> > 
> >  	}
> >  	
> >  	obj = (union acpi_object *)response.pointer;
> > 
> > +	if (!obj) {
> > +		pr_info("no response\n");
> > +		return;
> > +	}
> 
> If you intend to print this, it should probably be a bit more
> informative. Is "info" the right level here? I would imagine
> either WARN if this was a bad thing, or DEBUG if this is more
> for debugging the driver.
> 

So what you (or somebody else) prefer? warn or debug?

> > -	if (obj && obj->type == ACPI_TYPE_BUFFER) {
> > -		const struct key_entry *key;
> > -		int reported_key;
> > -		u16 *buffer_entry = (u16 *)obj->buffer.pointer;
> > -		int buffer_size = obj->buffer.length/2;
> > -
> > -		if (buffer_size >= 2 && dell_new_hk_type &&
> > buffer_entry[1] != 0x10) { -			pr_info("Received 
unknown
> > WMI event (0x%x)\n",
> > -				buffer_entry[1]);
> > -			kfree(obj);
> > -			return;
> > -		}
> > +	if (obj->type != ACPI_TYPE_BUFFER) {
> > +		pr_info("bad response type %x\n", obj->type);
> > +		kfree(obj);
> > +		return;
> > +	}
> > +
> > +	pr_debug("Received WMI event (%*ph)\n",
> > +		obj->buffer.length, obj->buffer.pointer);
> > 
> > -		if (buffer_size >= 3 && (dell_new_hk_type ||
> > buffer_entry[1] == 0x0)) -			reported_key =
> > (int)buffer_entry[2];
> > +	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)
> > 
> > -			reported_key = (int)buffer_entry[1] & 0xffff;
> > -		else {
> > +			dell_wmi_process_key(buffer_entry[1]);
> 
> Why can we drop the 0xffff mask now?
> 

Because it is useless (or correct me if not!). Variable 
buffer_entry has type u16* so operation "AND 0xFFFF" on 16bit 
integer do nothing.

> > +		else
> > 
> >  			pr_info("Received unknown WMI event\n");
> > 
> > -			kfree(obj);
> > -			return;
> > +		kfree(obj);
> > +		return;
> > +	}
> > +
> > +	buffer_end = buffer_entry + buffer_size;
> > +
> > +	while (buffer_entry < buffer_end) {
> > +
> > +		len = buffer_entry[0];
> > +		if (len == 0)
> > +			break;
> > +
> > +		len++;
> > +
> 
> Why increment len here? Are you trying to avoid a "len + 1" in
> the comparisons below? If so, is using "len * 2" in the debug
> message below correct? Please clarify.
> 

in buffer_entry[0] (16 bit integer) is stored length of event (in 
16bit units) without first (length) value. And "%*ph" takes size 
in bytes (u8). So length in bytes (u8) units is 2 * length in u16 
units.

> > +		if (buffer_entry+len > buffer_end) {
> 
> See coding style documentation on operators. Please run
> patches through checkpatch.
> 

checkpatch.pl does not show any problem for these lines.

> > +			pr_info("Invalid length of WMI event\n");
> 
> info doesn't see correct here either.
> 

debug or warn?

> > +			break;
> > 
> >  		}
> > 
> > -		key =
> > sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
> > -							reported_key);
> > -		if (!key) {
> > -			pr_info("Unknown key %x pressed\n", reported_key);
> > -		} else if ((key->keycode == KEY_BRIGHTNESSUP ||
> > -			    key->keycode == KEY_BRIGHTNESSDOWN) && 
acpi_video) {
> > -			/* Don't report brightness notifications that will 
also
> > -			 * come via ACPI */
> > -			;
> > -		} else {
> > -			sparse_keymap_report_entry(dell_wmi_input_dev, 
key,
> > -						   1, true);
> > +		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;
> > +				}
> > +			}
> > +			break;
> > +		case 0x10:
> > +			/* Keys pressed */
> > +			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;
> > +				}
> > +			}
> > +			break;
> > +		default:
> > +			/* Unknown event */
> > +			pr_info("Unknown WMI event type 0x%x\n",
> > +				(int)buffer_entry[1]);
> > +			break;
> > 
> >  		}
> > 
> > +
> > +		buffer_entry += len;
> > +
> > 
> >  	}
> > 
> > +
> > 
> >  	kfree(obj);
> >  
> >  }

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] dell-wmi: Update code for processing WMI events

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 9558 bytes --]

On Wednesday 22 October 2014 12:51:17 Pali Rohár wrote:
> On Tuesday 21 October 2014 23:32:12 Darren Hart wrote:
> > On Tue, Oct 21, 2014 at 12:15:24AM +0200, Pali Rohár wrote:
> > > WMI buffer can contains more events. First value in buffer
> > > is length of event followed by data of specified length.
> > > After that is next length and next data. When length is
> > > zero then there is no more events in bufffer.
> > > 
> > > This patch adds support for processing all events in
> > > buffer (not only first) and parse more event types (not
> > > only hotkey events). Because of variable length of events
> > > sometimes BIOS fills more hotkeys (or other values) into
> > > single WMI event. In this case this patch process also
> > > these multiple hotkeys (and not only first one).
> > > 
> > > Some event types are just ignored because kernel is not
> > > interested for them (e.g. NIC Link status, battery unplug,
> > > ...).
> > > 
> > > This patch is based on DSDT table from Dell Latitude
> > > E6440. Code should be backward compatible so will process
> > > other events of old types same as before this patch.
> > > 
> > > This patch also fixes problem when in kernel log are
> > > written messages about unknown WMI events. Now all know
> > > events are parsed and those which are not interesting for
> > > kernel are dropped without unknown message.
> > 
> > This should probably be done in a separate patch.
> 
> It is not possible, because my patch rewrite code for handling
> events. Kernel does not print "unknown event" messages when it
> parse WMI event and understand specified part.
> 
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > Tested-by: Pali Rohár <pali.rohar@gmail.com>
> > 
> > Well yes, I should hope so ;-)
> > 
> > > ---
> > > 
> > >  drivers/platform/x86/dell-wmi.c |  157
> > >  +++++++++++++++++++++++++++++++-------- 1 file changed,
> > >  127 insertions(+), 30 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/dell-wmi.c
> > > b/drivers/platform/x86/dell-wmi.c index 25721bf..3d15949
> > > 100644
> > > --- a/drivers/platform/x86/dell-wmi.c
> > > +++ b/drivers/platform/x86/dell-wmi.c
> > > @@ -145,11 +145,35 @@ static const u16
> > > bios_to_linux_keycode[256] __initconst = {
> > > 
> > >  static struct input_dev *dell_wmi_input_dev;
> > > 
> > > +static void dell_wmi_process_key(int reported_key)
> > > +{
> > > +	const struct key_entry *key;
> > > +
> > > +	key =
> > > sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
> > > +						reported_key);
> > > +	if (!key) {
> > > +		pr_info("Unknown key %x pressed\n", reported_key);
> > > +		return;
> > > +	}
> > > +
> > > +	pr_debug("Key %x pressed\n", reported_key);
> > > +
> > > +	/* Don't report brightness notifications that will also
> > > come via ACPI */ +	if ((key->keycode == KEY_BRIGHTNESSUP
> > > || +	     key->keycode == KEY_BRIGHTNESSDOWN) &&
> > > acpi_video) +		return;
> > > +
> > > +	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1,
> > > true); +}
> > > +
> > > 
> > >  static void dell_wmi_notify(u32 value, void *context)
> > >  {
> > >  
> > >  	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER,
> > >  	NULL }; union acpi_object *obj;
> > >  	acpi_status status;
> > > 
> > > +	acpi_size buffer_size;
> > > +	u16 *buffer_entry, *buffer_end;
> > > +	int len, i;
> > > 
> > >  	status = wmi_get_event_data(value, &response);
> > >  	if (status != AE_OK) {
> > > 
> > > @@ -158,44 +182,117 @@ static void dell_wmi_notify(u32
> > > value, void *context)
> > > 
> > >  	}
> > >  	
> > >  	obj = (union acpi_object *)response.pointer;
> > > 
> > > +	if (!obj) {
> > > +		pr_info("no response\n");
> > > +		return;
> > > +	}
> > 
> > If you intend to print this, it should probably be a bit
> > more informative. Is "info" the right level here? I would
> > imagine either WARN if this was a bad thing, or DEBUG if
> > this is more for debugging the driver.
> 
> So what you (or somebody else) prefer? warn or debug?
> 
> > > -	if (obj && obj->type == ACPI_TYPE_BUFFER) {
> > > -		const struct key_entry *key;
> > > -		int reported_key;
> > > -		u16 *buffer_entry = (u16 *)obj->buffer.pointer;
> > > -		int buffer_size = obj->buffer.length/2;
> > > -
> > > -		if (buffer_size >= 2 && dell_new_hk_type &&
> > > buffer_entry[1] != 0x10) { -			pr_info("Received
> 
> unknown
> 
> > > WMI event (0x%x)\n",
> > > -				buffer_entry[1]);
> > > -			kfree(obj);
> > > -			return;
> > > -		}
> > > +	if (obj->type != ACPI_TYPE_BUFFER) {
> > > +		pr_info("bad response type %x\n", obj->type);
> > > +		kfree(obj);
> > > +		return;
> > > +	}
> > > +
> > > +	pr_debug("Received WMI event (%*ph)\n",
> > > +		obj->buffer.length, obj->buffer.pointer);
> > > 
> > > -		if (buffer_size >= 3 && (dell_new_hk_type ||
> > > buffer_entry[1] == 0x0)) -			reported_key =
> > > (int)buffer_entry[2];
> > > +	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)
> > > 
> > > -			reported_key = (int)buffer_entry[1] & 0xffff;
> > > -		else {
> > > +			dell_wmi_process_key(buffer_entry[1]);
> > 
> > Why can we drop the 0xffff mask now?
> 
> Because it is useless (or correct me if not!). Variable
> buffer_entry has type u16* so operation "AND 0xFFFF" on 16bit
> integer do nothing.
> 
> > > +		else
> > > 
> > >  			pr_info("Received unknown WMI event\n");
> > > 
> > > -			kfree(obj);
> > > -			return;
> > > +		kfree(obj);
> > > +		return;
> > > +	}
> > > +
> > > +	buffer_end = buffer_entry + buffer_size;
> > > +
> > > +	while (buffer_entry < buffer_end) {
> > > +
> > > +		len = buffer_entry[0];
> > > +		if (len == 0)
> > > +			break;
> > > +
> > > +		len++;
> > > +
> > 
> > Why increment len here? Are you trying to avoid a "len + 1"
> > in the comparisons below? If so, is using "len * 2" in the
> > debug message below correct? Please clarify.
> 
> in buffer_entry[0] (16 bit integer) is stored length of event
> (in 16bit units) without first (length) value. And "%*ph"
> takes size in bytes (u8). So length in bytes (u8) units is 2
> * length in u16 units.
> 
> > > +		if (buffer_entry+len > buffer_end) {
> > 
> > See coding style documentation on operators. Please run
> > patches through checkpatch.
> 
> checkpatch.pl does not show any problem for these lines.
> 
> > > +			pr_info("Invalid length of WMI event\n");
> > 
> > info doesn't see correct here either.
> 
> debug or warn?
> 
> > > +			break;
> > > 
> > >  		}
> > > 
> > > -		key =
> > > sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
> > > -							reported_key);
> > > -		if (!key) {
> > > -			pr_info("Unknown key %x pressed\n", 
reported_key);
> > > -		} else if ((key->keycode == KEY_BRIGHTNESSUP ||
> > > -			    key->keycode == KEY_BRIGHTNESSDOWN) &&
> 
> acpi_video) {
> 
> > > -			/* Don't report brightness notifications that 
will
> 
> also
> 
> > > -			 * come via ACPI */
> > > -			;
> > > -		} else {
> > > -			sparse_keymap_report_entry(dell_wmi_input_dev,
> 
> key,
> 
> > > -						   1, true);
> > > +		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;
> > > +				}
> > > +			}
> > > +			break;
> > > +		case 0x10:
> > > +			/* Keys pressed */
> > > +			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;
> > > +				}
> > > +			}
> > > +			break;
> > > +		default:
> > > +			/* Unknown event */
> > > +			pr_info("Unknown WMI event type 0x%x\n",
> > > +				(int)buffer_entry[1]);
> > > +			break;
> > > 
> > >  		}
> > > 
> > > +
> > > +		buffer_entry += len;
> > > +
> > > 
> > >  	}
> > > 
> > > +
> > > 
> > >  	kfree(obj);
> > >  
> > >  }

Darren: PING. See my comments and questions.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] dell-wmi: Update code for processing WMI events

[Darren Hart]

On Sun, Nov 09, 2014 at 04:21:33PM +0100, Pali Rohár wrote:
> On Wednesday 22 October 2014 12:51:17 Pali Rohár wrote:
> > On Tuesday 21 October 2014 23:32:12 Darren Hart wrote:
> > > On Tue, Oct 21, 2014 at 12:15:24AM +0200, Pali Rohár wrote:

Hi Pali,

...
> > > > @@ -158,44 +182,117 @@ static void dell_wmi_notify(u32
> > > > value, void *context)
> > > > 
> > > >  	}
> > > >  	
> > > >  	obj = (union acpi_object *)response.pointer;
> > > > 
> > > > +	if (!obj) {
> > > > +		pr_info("no response\n");
> > > > +		return;
> > > > +	}
> > > 
> > > If you intend to print this, it should probably be a bit
> > > more informative. Is "info" the right level here? I would
> > > imagine either WARN if this was a bad thing, or DEBUG if
> > > this is more for debugging the driver.
> > 
> > So what you (or somebody else) prefer? warn or debug?

I was leaving that up to you based on your interpretation of severity and why
you added this. If it was just for debug purporses, the I suggest debug. If this
will impact the user in an unexpected way, then WARN.

...
> > > > +	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)
> > > > 
> > > > -			reported_key = (int)buffer_entry[1] & 0xffff;
> > > > -		else {
> > > > +			dell_wmi_process_key(buffer_entry[1]);
> > > 
> > > Why can we drop the 0xffff mask now?
> > 
> > Because it is useless (or correct me if not!). Variable
> > buffer_entry has type u16* so operation "AND 0xFFFF" on 16bit
> > integer do nothing.

Right, of course. Thanks,

> > > > +		else
> > > > 
> > > >  			pr_info("Received unknown WMI event\n");
> > > > 
> > > > -			kfree(obj);
> > > > -			return;
> > > > +		kfree(obj);
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	buffer_end = buffer_entry + buffer_size;
> > > > +
> > > > +	while (buffer_entry < buffer_end) {
> > > > +
> > > > +		len = buffer_entry[0];
> > > > +		if (len == 0)
> > > > +			break;
> > > > +
> > > > +		len++;
> > > > +
> > > 
> > > Why increment len here? Are you trying to avoid a "len + 1"
> > > in the comparisons below? If so, is using "len * 2" in the
> > > debug message below correct? Please clarify.
> > 
> > in buffer_entry[0] (16 bit integer) is stored length of event
> > (in 16bit units) without first (length) value. And "%*ph"
> > takes size in bytes (u8). So length in bytes (u8) units is 2
> > * length in u16 units.

Right, got it - thanks.

> > > > +		if (buffer_entry+len > buffer_end) {
> > > 
> > > See coding style documentation on operators. Please run
> > > patches through checkpatch.
> > 
> > checkpatch.pl does not show any problem for these lines.

I thought we checked for that. Please see Documentation/CodingStyle 3.1: Spaces
with regard to spacing around binary and ternary operators. (one space on each
side of).

> Darren: PING. See my comments and questions.

Thanks for the ping :)

So please choose a loglevel and correct the whitespace and we should be good.

-- 
Darren Hart
Intel Open Source Technology Center

[PATCH v2] dell-wmi: Update code for processing WMI events

[Pali Rohár]

WMI buffer can contains more events. First value in buffer is length of event
followed by data of specified length. After that is next length and next data.
When length is zero then there is no more events in bufffer.

This patch adds support for processing all events in buffer (not only first)
and parse more event types (not only hotkey events). Because of variable length
of events sometimes BIOS fills more hotkeys (or other values) into single WMI
event. In this case this patch process also these multiple hotkeys (and not
only first one).

Some event types are just ignored because kernel is not interested for them
(e.g. NIC Link status, battery unplug, ...).

This patch is based on DSDT table from Dell Latitude E6440. Code should be
backward compatible so will process other events of old types same as before
this patch.

This patch also fixes problem when in kernel log are written messages about
unknown WMI events. Now all know events are parsed and those which are not
interesting for kernel are dropped without unknown message.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Tested-by: Pali Rohár <pali.rohar@gmail.com>
---
Changes since v1:
* change some pr_info calls with pr_warn
* fix coding style
---
 drivers/platform/x86/dell-wmi.c |  159 +++++++++++++++++++++++++++++++--------
 1 file changed, 128 insertions(+), 31 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 25721bf..e2b6a64 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -145,57 +145,154 @@ static const u16 bios_to_linux_keycode[256] __initconst = {
 
 static struct input_dev *dell_wmi_input_dev;
 
+static void dell_wmi_process_key(int reported_key)
+{
+	const struct key_entry *key;
+
+	key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
+						reported_key);
+	if (!key) {
+		pr_info("Unknown key %x pressed\n", reported_key);
+		return;
+	}
+
+	pr_debug("Key %x pressed\n", reported_key);
+
+	/* Don't report brightness notifications that will also come via ACPI */
+	if ((key->keycode == KEY_BRIGHTNESSUP ||
+	     key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video)
+		return;
+
+	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true);
+}
+
 static void dell_wmi_notify(u32 value, void *context)
 {
 	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *obj;
 	acpi_status status;
+	acpi_size buffer_size;
+	u16 *buffer_entry, *buffer_end;
+	int len, i;
 
 	status = wmi_get_event_data(value, &response);
 	if (status != AE_OK) {
-		pr_info("bad event status 0x%x\n", status);
+		pr_warn("bad event status 0x%x\n", status);
 		return;
 	}
 
 	obj = (union acpi_object *)response.pointer;
+	if (!obj) {
+		pr_warn("no response\n");
+		return;
+	}
 
-	if (obj && obj->type == ACPI_TYPE_BUFFER) {
-		const struct key_entry *key;
-		int reported_key;
-		u16 *buffer_entry = (u16 *)obj->buffer.pointer;
-		int buffer_size = obj->buffer.length/2;
-
-		if (buffer_size >= 2 && dell_new_hk_type && buffer_entry[1] != 0x10) {
-			pr_info("Received unknown WMI event (0x%x)\n",
-				buffer_entry[1]);
-			kfree(obj);
-			return;
-		}
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		pr_warn("bad response type %x\n", obj->type);
+		kfree(obj);
+		return;
+	}
+
+	pr_debug("Received WMI event (%*ph)\n",
+		obj->buffer.length, obj->buffer.pointer);
 
-		if (buffer_size >= 3 && (dell_new_hk_type || buffer_entry[1] == 0x0))
-			reported_key = (int)buffer_entry[2];
+	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)
-			reported_key = (int)buffer_entry[1] & 0xffff;
-		else {
+			dell_wmi_process_key(buffer_entry[1]);
+		else
 			pr_info("Received unknown WMI event\n");
-			kfree(obj);
-			return;
+		kfree(obj);
+		return;
+	}
+
+	buffer_end = buffer_entry + buffer_size;
+
+	while (buffer_entry < buffer_end) {
+
+		len = buffer_entry[0];
+		if (len == 0)
+			break;
+
+		len++;
+
+		if (buffer_entry + len > buffer_end) {
+			pr_warn("Invalid length of WMI event\n");
+			break;
 		}
 
-		key = sparse_keymap_entry_from_scancode(dell_wmi_input_dev,
-							reported_key);
-		if (!key) {
-			pr_info("Unknown key %x pressed\n", reported_key);
-		} else if ((key->keycode == KEY_BRIGHTNESSUP ||
-			    key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video) {
-			/* Don't report brightness notifications that will also
-			 * come via ACPI */
-			;
-		} else {
-			sparse_keymap_report_entry(dell_wmi_input_dev, key,
-						   1, true);
+		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;
+				}
+			}
+			break;
+		case 0x10:
+			/* Keys pressed */
+			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;
+				}
+			}
+			break;
+		default:
+			/* Unknown event */
+			pr_info("Unknown WMI event type 0x%x\n",
+				(int)buffer_entry[1]);
+			break;
 		}
+
+		buffer_entry += len;
+
 	}
+
 	kfree(obj);
 }
 
-- 
1.7.9.5

Re: [PATCH v2] dell-wmi: Update code for processing WMI events

[Darren Hart]

On Tue, Nov 11, 2014 at 08:21:22PM +0100, Pali Rohár wrote:
> WMI buffer can contains more events. First value in buffer is length of event
> followed by data of specified length. After that is next length and next data.
> When length is zero then there is no more events in bufffer.
> 
> This patch adds support for processing all events in buffer (not only first)
> and parse more event types (not only hotkey events). Because of variable length
> of events sometimes BIOS fills more hotkeys (or other values) into single WMI
> event. In this case this patch process also these multiple hotkeys (and not
> only first one).
> 
> Some event types are just ignored because kernel is not interested for them
> (e.g. NIC Link status, battery unplug, ...).
> 
> This patch is based on DSDT table from Dell Latitude E6440. Code should be
> backward compatible so will process other events of old types same as before
> this patch.
> 
> This patch also fixes problem when in kernel log are written messages about
> unknown WMI events. Now all know events are parsed and those which are not
> interesting for kernel are dropped without unknown message.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> Tested-by: Pali Rohár <pali.rohar@gmail.com>

Queued, thanks. I took the liberty of cleaning up the commit message just a bit:

    dell-wmi: Update code for processing WMI events

    The WMI buffer can contain multiple events. First value in buffer is
    length of event followed by data of specified length. After that is next
    length and next data.  When length is zero then there is no more events
    in bufffer.

    This patch adds support for processing all events in buffer (not only
    first) and parse more event types (not only hotkey events). Because of
    variable length of events sometimes BIOS fills more hotkeys (or other
    values) into single WMI event. In this case this patch also processes
    these multiple hotkeys (and not only first one).

    Some event types are just ignored because kernel is not interested in
    them (e.g. NIC Link status, battery unplug, ...).

    This patch is based on DSDT table from Dell Latitude E6440. Code should
    be backward compatible so will process other events of old types same as
    before this patch.

    This patch also fixes a problem with unknown WMI event messages being
    written to the log. Now all known events are parsed and those which are
    not interesting to the kernel are dropped without an unknown WMI event
    message.

    Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
    Tested-by: Pali Rohár <pali.rohar@gmail.com>
    Signed-off-by: Darren Hart <dvhart@linux.intel.com>

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center