LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Enable Dell All-In-One volume up/down keys
@ 2011-02-14 13:02 Colin King
  2011-02-17  7:52 ` Dmitry Torokhov
  0 siblings, 1 reply; 6+ messages in thread
From: Colin King @ 2011-02-14 13:02 UTC (permalink / raw)
  To: platform-driver-x86, Matthew Garrett; +Cc: linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Enable volume up and down hotkeys on WMI events
GUID 284A0E6B-380E-472A-921F-E52786257FB and
GUID 02314822-307C-4F66-bf0E-48AEAEB26CC8.

Also works around a firmware bug where the _WED method
should return an integer containing the key code and in fact
the method returns the key code in element zero of a buffer.

BugLink: http://bugs.launchpad.net/bugs/701530
BugLink: http://bugs.launchpad.net/bugs/676997

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/platform/x86/Kconfig        |   12 +++
 drivers/platform/x86/Makefile       |    1 +
 drivers/platform/x86/dell-wmi-aio.c |  173 +++++++++++++++++++++++++++++++++++
 3 files changed, 186 insertions(+), 0 deletions(-)
 create mode 100644 drivers/platform/x86/dell-wmi-aio.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index d163bc2..b6fbb0c 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -101,6 +101,18 @@ config DELL_WMI
 	  To compile this driver as a module, choose M here: the module will
 	  be called dell-wmi.
 
+config DELL_WMI_AIO
+	tristate "WMI Hotkeys for Dell All-In-One series"
+	depends on ACPI_WMI
+	depends on INPUT
+	---help---
+	  Say Y here if you want to support WMI-based hotkeys on Dell
+	  All-In-One machines.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called dell-wmi.
+
+
 config FUJITSU_LAPTOP
 	tristate "Fujitsu Laptop Extras"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 4ec4ff8..26c6a57 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_ACPI_CMPC)		+= classmate-laptop.o
 obj-$(CONFIG_COMPAL_LAPTOP)	+= compal-laptop.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
+obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
 obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
 obj-$(CONFIG_ACERHDF)		+= acerhdf.o
 obj-$(CONFIG_HP_WMI)		+= hp-wmi.o
diff --git a/drivers/platform/x86/dell-wmi-aio.c b/drivers/platform/x86/dell-wmi-aio.c
new file mode 100644
index 0000000..3dccbaa
--- /dev/null
+++ b/drivers/platform/x86/dell-wmi-aio.c
@@ -0,0 +1,173 @@
+/*
+ *  WMI hotkeys support for Dell All-In-One series
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <acpi/acpi_drivers.h>
+#include <linux/acpi.h>
+#include <linux/string.h>
+
+#define AIO_PREFIX	"dell-wmi-aio: "
+
+MODULE_DESCRIPTION("WMI hotkeys driver for Dell All-In-One series");
+MODULE_LICENSE("GPL");
+
+#define EVENT_GUID1 "284A0E6B-380E-472A-921F-E52786257FB4"
+#define EVENT_GUID2 "02314822-307C-4F66-BF0E-48AEAEB26CC8"
+
+static char *dell_wmi_aio_guids[] = {
+	EVENT_GUID1,
+	EVENT_GUID2,
+	NULL
+};
+
+MODULE_ALIAS("wmi:"EVENT_GUID1);
+MODULE_ALIAS("wmi:"EVENT_GUID2);
+
+static struct key_entry dell_wmi_aio_keymap[] = {
+	{ KE_KEY, 0xc0, { KEY_VOLUMEUP } },
+	{ KE_KEY, 0xc1, { KEY_VOLUMEDOWN } },
+	{ KE_END, 0 }
+};
+
+static struct input_dev *dell_wmi_aio_input_dev;
+
+static void dell_wmi_aio_notify(u32 value, void *context)
+{
+	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	acpi_status status;
+
+	status = wmi_get_event_data(value, &response);
+	if (status != AE_OK) {
+		pr_info(AIO_PREFIX "bad event status 0x%x\n", status);
+		return;
+	}
+
+	obj = (union acpi_object *)response.pointer;
+	if (obj) {
+		unsigned int scancode;
+
+		switch (obj->type) {
+		case ACPI_TYPE_INTEGER:
+			/* Most All-In-One correctly return integer scancode */
+			scancode = obj->integer.value;
+			sparse_keymap_report_event(dell_wmi_aio_input_dev,
+				scancode, 1, true);
+			break;
+		case ACPI_TYPE_BUFFER:
+			/* Broken machines return the scancode in a buffer */
+			if (obj->buffer.pointer && obj->buffer.length > 0) {
+				scancode = obj->buffer.pointer[0];
+				sparse_keymap_report_event(
+					dell_wmi_aio_input_dev,
+					scancode, 1, true);
+			}
+			break;
+		}
+	}
+	kfree(obj);
+}
+
+static int __init dell_wmi_aio_input_setup(void)
+{
+	int err;
+
+	dell_wmi_aio_input_dev = input_allocate_device();
+
+	if (!dell_wmi_aio_input_dev)
+		return -ENOMEM;
+
+	dell_wmi_aio_input_dev->name = "Dell AIO WMI hotkeys";
+	dell_wmi_aio_input_dev->phys = "wmi/input0";
+	dell_wmi_aio_input_dev->id.bustype = BUS_HOST;
+
+	err = sparse_keymap_setup(dell_wmi_aio_input_dev,
+			dell_wmi_aio_keymap, NULL);
+	if (err) {
+		pr_err("Unable to setup input device keymap\n");
+		goto err_free_dev;
+	}
+	err = input_register_device(dell_wmi_aio_input_dev);
+	if (err) {
+		pr_info("Unable to register input device\n");
+		goto err_free_keymap;
+	}
+	return 0;
+
+err_free_keymap:
+	sparse_keymap_free(dell_wmi_aio_input_dev);
+err_free_dev:
+	input_free_device(dell_wmi_aio_input_dev);
+	return err;
+}
+
+static char *dell_wmi_aio_find(void)
+{
+	int i;
+
+	for (i = 0; dell_wmi_aio_guids[i] != NULL; i++)
+		if (wmi_has_guid(dell_wmi_aio_guids[i]))
+			return dell_wmi_aio_guids[i];
+
+	return NULL;
+}
+
+static int __init dell_wmi_aio_init(void)
+{
+	int err;
+	char *guid;
+
+	guid = dell_wmi_aio_find();
+	if (guid) {
+		err = dell_wmi_aio_input_setup();
+
+		if (err)
+			return err;
+
+		err = wmi_install_notify_handler(guid,
+			dell_wmi_aio_notify, NULL);
+		if (err) {
+			input_unregister_device(dell_wmi_aio_input_dev);
+			pr_err(AIO_PREFIX "Unable to register"
+			       " notify handler - %d\n", err);
+			return err;
+		}
+	} else
+		pr_warning(AIO_PREFIX "No known WMI GUID found\n");
+
+	return 0;
+}
+
+static void __exit dell_wmi_aio_exit(void)
+{
+	char *guid;
+
+	guid = dell_wmi_aio_find();
+	if (guid) {
+		wmi_remove_notify_handler(guid);
+		input_unregister_device(dell_wmi_aio_input_dev);
+	}
+}
+
+module_init(dell_wmi_aio_init);
+module_exit(dell_wmi_aio_exit);
-- 
1.7.2.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Dell All-In-One volume up/down keys
  2011-02-14 13:02 [PATCH] Enable Dell All-In-One volume up/down keys Colin King
@ 2011-02-17  7:52 ` Dmitry Torokhov
  2011-02-17 11:10   ` Colin Ian King
  2011-02-17 16:33   ` Matthew Garrett
  0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2011-02-17  7:52 UTC (permalink / raw)
  To: Colin King; +Cc: platform-driver-x86, Matthew Garrett, linux-kernel

Hi Colin,

On Mon, Feb 14, 2011 at 01:02:04PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Enable volume up and down hotkeys on WMI events
> GUID 284A0E6B-380E-472A-921F-E52786257FB and
> GUID 02314822-307C-4F66-bf0E-48AEAEB26CC8.
> 
> Also works around a firmware bug where the _WED method
> should return an integer containing the key code and in fact
> the method returns the key code in element zero of a buffer.
> 
> BugLink: http://bugs.launchpad.net/bugs/701530
> BugLink: http://bugs.launchpad.net/bugs/676997

Looks generally good, one question though - should't it be part of
dell-wmi driver?

> +config DELL_WMI_AIO
> +       tristate "WMI Hotkeys for Dell All-In-One series"
> +       depends on ACPI_WMI
> +       depends on INPUT

select INPUT_SPARSEKMAP

> +
> +#define AIO_PREFIX	"dell-wmi-aio: "

Just use

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

and drop AIO_PREFIX from messages.

> +
> +MODULE_DESCRIPTION("WMI hotkeys driver for Dell All-In-One series");
> +MODULE_LICENSE("GPL");
> +
> +#define EVENT_GUID1 "284A0E6B-380E-472A-921F-E52786257FB4"
> +#define EVENT_GUID2 "02314822-307C-4F66-BF0E-48AEAEB26CC8"
> +
> +static char *dell_wmi_aio_guids[] = {
> +	EVENT_GUID1,
> +	EVENT_GUID2,
> +	NULL
> +};
> +
> +MODULE_ALIAS("wmi:"EVENT_GUID1);
> +MODULE_ALIAS("wmi:"EVENT_GUID2);
> +
> +static struct key_entry dell_wmi_aio_keymap[] = {

const

> +
> +static char *dell_wmi_aio_find(void)
> +{
> +	int i;
> +
> +	for (i = 0; dell_wmi_aio_guids[i] != NULL; i++)
> +		if (wmi_has_guid(dell_wmi_aio_guids[i]))
> +			return dell_wmi_aio_guids[i];
> +
> +	return NULL;
> +}
> +
> +static int __init dell_wmi_aio_init(void)
> +{
> +	int err;
> +	char *guid;
> +
> +	guid = dell_wmi_aio_find();
> +	if (guid) {
> +		err = dell_wmi_aio_input_setup();
> +
> +		if (err)
> +			return err;
> +
> +		err = wmi_install_notify_handler(guid,
> +			dell_wmi_aio_notify, NULL);
> +		if (err) {
> +			input_unregister_device(dell_wmi_aio_input_dev);
> +			pr_err(AIO_PREFIX "Unable to register"
> +			       " notify handler - %d\n", err);

Do not break the strings - 80-columr rule does not apply to messages.

> +			return err;
> +		}
> +	} else
> +		pr_warning(AIO_PREFIX "No known WMI GUID found\n");
> +
> +	return 0;

If you did not find known guid you shold return -ENXIO so that the
driver aborts loading into memory (if it is compiled as a module -
likely).

> +}
> +
> +static void __exit dell_wmi_aio_exit(void)
> +{
> +	char *guid;
> +
> +	guid = dell_wmi_aio_find();

And if you fail loading you do not need to check it here.

> +	if (guid) {
> +		wmi_remove_notify_handler(guid);
> +		input_unregister_device(dell_wmi_aio_input_dev);
> +	}
> +}
> +
> +module_init(dell_wmi_aio_init);
> +module_exit(dell_wmi_aio_exit);

Thanks,

-- 
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Dell All-In-One volume up/down keys
  2011-02-17  7:52 ` Dmitry Torokhov
@ 2011-02-17 11:10   ` Colin Ian King
  2011-02-17 17:21     ` Dmitry Torokhov
  2011-02-17 16:33   ` Matthew Garrett
  1 sibling, 1 reply; 6+ messages in thread
From: Colin Ian King @ 2011-02-17 11:10 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: platform-driver-x86, Matthew Garrett, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4040 bytes --]

Thanks for your feedback Dmitry,

On Wed, 2011-02-16 at 23:52 -0800, Dmitry Torokhov wrote:
> Hi Colin,
> 
> On Mon, Feb 14, 2011 at 01:02:04PM +0000, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> > 
> > Enable volume up and down hotkeys on WMI events
> > GUID 284A0E6B-380E-472A-921F-E52786257FB and
> > GUID 02314822-307C-4F66-bf0E-48AEAEB26CC8.
> > 
> > Also works around a firmware bug where the _WED method
> > should return an integer containing the key code and in fact
> > the method returns the key code in element zero of a buffer.
> > 
> > BugLink: http://bugs.launchpad.net/bugs/701530
> > BugLink: http://bugs.launchpad.net/bugs/676997
> 
> Looks generally good, one question though - should't it be part of
> dell-wmi driver?

Good question.  A couple of points:

1. The Dell All-In-One WMI hotkey mechanism is a little different from
the
normal Dell WMI hotkey mechanism, so I didn't really want to clutter up
the
Dell WMI driver with exceptions for these particular GUIDs and notifier
IDs.

2. These keys appear on a very limited subset of Dell machines, so
keeping
this code out of the Dell WMI driver for the majority of Dell machines
will
keep the original Dell WMI driver smaller. 

3. I expect further additions in functionality for these machines, which
makes sense to keep it specific to this limited All-In-One driver.

As it stands, this All-In-One interface has already been implemented by
two different BIOS vendors and already there are two different
implementations. I'd like to keep this all in one specific driver for
this class of machine if possible.

> > +config DELL_WMI_AIO
> > +       tristate "WMI Hotkeys for Dell All-In-One series"
> > +       depends on ACPI_WMI
> > +       depends on INPUT
> 
> select INPUT_SPARSEKMAP
> 
> > +
> > +#define AIO_PREFIX	"dell-wmi-aio: "
> 
> Just use
> 
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> and drop AIO_PREFIX from messages.
> 
> > +
> > +MODULE_DESCRIPTION("WMI hotkeys driver for Dell All-In-One series");
> > +MODULE_LICENSE("GPL");
> > +
> > +#define EVENT_GUID1 "284A0E6B-380E-472A-921F-E52786257FB4"
> > +#define EVENT_GUID2 "02314822-307C-4F66-BF0E-48AEAEB26CC8"
> > +
> > +static char *dell_wmi_aio_guids[] = {
> > +	EVENT_GUID1,
> > +	EVENT_GUID2,
> > +	NULL
> > +};
> > +
> > +MODULE_ALIAS("wmi:"EVENT_GUID1);
> > +MODULE_ALIAS("wmi:"EVENT_GUID2);
> > +
> > +static struct key_entry dell_wmi_aio_keymap[] = {
> 
> const
> 
> > +
> > +static char *dell_wmi_aio_find(void)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; dell_wmi_aio_guids[i] != NULL; i++)
> > +		if (wmi_has_guid(dell_wmi_aio_guids[i]))
> > +			return dell_wmi_aio_guids[i];
> > +
> > +	return NULL;
> > +}
> > +
> > +static int __init dell_wmi_aio_init(void)
> > +{
> > +	int err;
> > +	char *guid;
> > +
> > +	guid = dell_wmi_aio_find();
> > +	if (guid) {
> > +		err = dell_wmi_aio_input_setup();
> > +
> > +		if (err)
> > +			return err;
> > +
> > +		err = wmi_install_notify_handler(guid,
> > +			dell_wmi_aio_notify, NULL);
> > +		if (err) {
> > +			input_unregister_device(dell_wmi_aio_input_dev);
> > +			pr_err(AIO_PREFIX "Unable to register"
> > +			       " notify handler - %d\n", err);
> 
> Do not break the strings - 80-columr rule does not apply to messages.
> 
> > +			return err;
> > +		}
> > +	} else
> > +		pr_warning(AIO_PREFIX "No known WMI GUID found\n");
> > +
> > +	return 0;
> 
> If you did not find known guid you shold return -ENXIO so that the
> driver aborts loading into memory (if it is compiled as a module -
> likely).
> 
> > +}
> > +
> > +static void __exit dell_wmi_aio_exit(void)
> > +{
> > +	char *guid;
> > +
> > +	guid = dell_wmi_aio_find();
> 
> And if you fail loading you do not need to check it here.
> 
> > +	if (guid) {
> > +		wmi_remove_notify_handler(guid);
> > +		input_unregister_device(dell_wmi_aio_input_dev);
> > +	}
> > +}
> > +
> > +module_init(dell_wmi_aio_init);
> > +module_exit(dell_wmi_aio_exit);
> 
> Thanks,
> 
I took your recommendations and attached is the updated patch.

Thanks,

Colin



[-- Attachment #2: 0001-PATCH-Enable-Dell-All-In-One-volume-up-down-keys.patch --]
[-- Type: text/x-patch, Size: 6857 bytes --]

>From 7a4c2b706317df9be5b7514934e87412fa64eaf4 Mon Sep 17 00:00:00 2001
From: Colin Ian King <colin.king@canonical.com>
Date: Thu, 17 Feb 2011 10:49:30 +0000
Subject: [PATCH] Enable Dell All-In-One volume up/down keys

Enable volume up and down hotkeys on WMI events
GUID 284A0E6B-380E-472A-921F-E52786257FB4 and
GUID 02314822-307C-4F66-bf0E-48AEAEB26CC8.

Also works around a firmware bug where the _WED method
should return an integer containing the key code and in fact
the method returns the key code in element zero of a buffer.

BugLink: http://bugs.launchpad.net/bugs/701530
BugLink: http://bugs.launchpad.net/bugs/676997

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/platform/x86/Kconfig        |   13 +++
 drivers/platform/x86/Makefile       |    1 +
 drivers/platform/x86/dell-wmi-aio.c |  172 +++++++++++++++++++++++++++++++++++
 3 files changed, 186 insertions(+), 0 deletions(-)
 create mode 100644 drivers/platform/x86/dell-wmi-aio.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index d163bc2..b2e8ce0 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -101,6 +101,19 @@ config DELL_WMI
 	  To compile this driver as a module, choose M here: the module will
 	  be called dell-wmi.
 
+config DELL_WMI_AIO
+	tristate "WMI Hotkeys for Dell All-In-One series"
+	depends on ACPI_WMI
+	depends on INPUT
+	select INPUT_SPARSEKMAP
+	---help---
+	  Say Y here if you want to support WMI-based hotkeys on Dell
+	  All-In-One machines.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called dell-wmi.
+
+
 config FUJITSU_LAPTOP
 	tristate "Fujitsu Laptop Extras"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 4ec4ff8..26c6a57 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_ACPI_CMPC)		+= classmate-laptop.o
 obj-$(CONFIG_COMPAL_LAPTOP)	+= compal-laptop.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
+obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
 obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
 obj-$(CONFIG_ACERHDF)		+= acerhdf.o
 obj-$(CONFIG_HP_WMI)		+= hp-wmi.o
diff --git a/drivers/platform/x86/dell-wmi-aio.c b/drivers/platform/x86/dell-wmi-aio.c
new file mode 100644
index 0000000..7304640
--- /dev/null
+++ b/drivers/platform/x86/dell-wmi-aio.c
@@ -0,0 +1,172 @@
+/*
+ *  WMI hotkeys support for Dell All-In-One series
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <acpi/acpi_drivers.h>
+#include <linux/acpi.h>
+#include <linux/string.h>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+MODULE_DESCRIPTION("WMI hotkeys driver for Dell All-In-One series");
+MODULE_LICENSE("GPL");
+
+#define EVENT_GUID1 "284A0E6B-380E-472A-921F-E52786257FB4"
+#define EVENT_GUID2 "02314822-307C-4F66-BF0E-48AEAEB26CC8"
+
+static const char *dell_wmi_aio_guids[] = {
+	EVENT_GUID1,
+	EVENT_GUID2,
+	NULL
+};
+
+MODULE_ALIAS("wmi:"EVENT_GUID1);
+MODULE_ALIAS("wmi:"EVENT_GUID2);
+
+static const struct key_entry dell_wmi_aio_keymap[] = {
+	{ KE_KEY, 0xc0, { KEY_VOLUMEUP } },
+	{ KE_KEY, 0xc1, { KEY_VOLUMEDOWN } },
+	{ KE_END, 0 }
+};
+
+static struct input_dev *dell_wmi_aio_input_dev;
+
+static void dell_wmi_aio_notify(u32 value, void *context)
+{
+	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	acpi_status status;
+
+	status = wmi_get_event_data(value, &response);
+	if (status != AE_OK) {
+		pr_info("bad event status 0x%x\n", status);
+		return;
+	}
+
+	obj = (union acpi_object *)response.pointer;
+	if (obj) {
+		unsigned int scancode;
+
+		switch (obj->type) {
+		case ACPI_TYPE_INTEGER:
+			/* Most All-In-One correctly return integer scancode */
+			scancode = obj->integer.value;
+			sparse_keymap_report_event(dell_wmi_aio_input_dev,
+				scancode, 1, true);
+			break;
+		case ACPI_TYPE_BUFFER:
+			/* Broken machines return the scancode in a buffer */
+			if (obj->buffer.pointer && obj->buffer.length > 0) {
+				scancode = obj->buffer.pointer[0];
+				sparse_keymap_report_event(
+					dell_wmi_aio_input_dev,
+					scancode, 1, true);
+			}
+			break;
+		}
+	}
+	kfree(obj);
+}
+
+static int __init dell_wmi_aio_input_setup(void)
+{
+	int err;
+
+	dell_wmi_aio_input_dev = input_allocate_device();
+
+	if (!dell_wmi_aio_input_dev)
+		return -ENOMEM;
+
+	dell_wmi_aio_input_dev->name = "Dell AIO WMI hotkeys";
+	dell_wmi_aio_input_dev->phys = "wmi/input0";
+	dell_wmi_aio_input_dev->id.bustype = BUS_HOST;
+
+	err = sparse_keymap_setup(dell_wmi_aio_input_dev,
+			dell_wmi_aio_keymap, NULL);
+	if (err) {
+		pr_err("Unable to setup input device keymap\n");
+		goto err_free_dev;
+	}
+	err = input_register_device(dell_wmi_aio_input_dev);
+	if (err) {
+		pr_info("Unable to register input device\n");
+		goto err_free_keymap;
+	}
+	return 0;
+
+err_free_keymap:
+	sparse_keymap_free(dell_wmi_aio_input_dev);
+err_free_dev:
+	input_free_device(dell_wmi_aio_input_dev);
+	return err;
+}
+
+static char *dell_wmi_aio_find(void)
+{
+	int i;
+
+	for (i = 0; dell_wmi_aio_guids[i] != NULL; i++)
+		if (wmi_has_guid(dell_wmi_aio_guids[i]))
+			return dell_wmi_aio_guids[i];
+
+	return NULL;
+}
+
+static int __init dell_wmi_aio_init(void)
+{
+	int err;
+	char *guid;
+
+	guid = dell_wmi_aio_find();
+	if (guid) {
+		err = dell_wmi_aio_input_setup();
+
+		if (err)
+			return err;
+
+		err = wmi_install_notify_handler(guid,
+			dell_wmi_aio_notify, NULL);
+		if (err) {
+			input_unregister_device(dell_wmi_aio_input_dev);
+			pr_err("Unable to register notify handler - %d\n", err);
+			return err;
+		}
+	} else {
+		pr_warning("No known WMI GUID found\n");
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
+static void __exit dell_wmi_aio_exit(void)
+{
+	char *guid;
+
+	guid = dell_wmi_aio_find();
+	wmi_remove_notify_handler(guid);
+	input_unregister_device(dell_wmi_aio_input_dev);
+}
+
+module_init(dell_wmi_aio_init);
+module_exit(dell_wmi_aio_exit);
-- 
1.7.2.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Dell All-In-One volume up/down keys
  2011-02-17  7:52 ` Dmitry Torokhov
  2011-02-17 11:10   ` Colin Ian King
@ 2011-02-17 16:33   ` Matthew Garrett
  1 sibling, 0 replies; 6+ messages in thread
From: Matthew Garrett @ 2011-02-17 16:33 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Colin King, platform-driver-x86, linux-kernel

On Wed, Feb 16, 2011 at 11:52:56PM -0800, Dmitry Torokhov wrote:

> Looks generally good, one question though - should't it be part of
> dell-wmi driver?

It uses a different WMI GUID and has a different behaviour model to dell 
laptops that dell-wmi drives, so I think it's fair for it to be a 
separate driver.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Dell All-In-One volume up/down keys
  2011-02-17 11:10   ` Colin Ian King
@ 2011-02-17 17:21     ` Dmitry Torokhov
  2011-02-17 18:44       ` Colin Ian King
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2011-02-17 17:21 UTC (permalink / raw)
  To: Colin Ian King; +Cc: platform-driver-x86, Matthew Garrett, linux-kernel

On Thu, Feb 17, 2011 at 11:10:11AM +0000, Colin Ian King wrote:
> Thanks for your feedback Dmitry,
> 
> On Wed, 2011-02-16 at 23:52 -0800, Dmitry Torokhov wrote:
> > Hi Colin,
> > 
> > On Mon, Feb 14, 2011 at 01:02:04PM +0000, Colin King wrote:
> > > From: Colin Ian King <colin.king@canonical.com>
> > > 
> > > Enable volume up and down hotkeys on WMI events
> > > GUID 284A0E6B-380E-472A-921F-E52786257FB and
> > > GUID 02314822-307C-4F66-bf0E-48AEAEB26CC8.
> > > 
> > > Also works around a firmware bug where the _WED method
> > > should return an integer containing the key code and in fact
> > > the method returns the key code in element zero of a buffer.
> > > 
> > > BugLink: http://bugs.launchpad.net/bugs/701530
> > > BugLink: http://bugs.launchpad.net/bugs/676997
> > 
> > Looks generally good, one question though - should't it be part of
> > dell-wmi driver?
> 
> Good question.  A couple of points:
> 
> 1. The Dell All-In-One WMI hotkey mechanism is a little different from
> the
> normal Dell WMI hotkey mechanism, so I didn't really want to clutter up
> the
> Dell WMI driver with exceptions for these particular GUIDs and notifier
> IDs.
> 
> 2. These keys appear on a very limited subset of Dell machines, so
> keeping
> this code out of the Dell WMI driver for the majority of Dell machines
> will
> keep the original Dell WMI driver smaller. 
> 
> 3. I expect further additions in functionality for these machines, which
> makes sense to keep it specific to this limited All-In-One driver.
> 
> As it stands, this All-In-One interface has already been implemented by
> two different BIOS vendors and already there are two different
> implementations. I'd like to keep this all in one specific driver for
> this class of machine if possible.

OK, fair enough.

> From 7a4c2b706317df9be5b7514934e87412fa64eaf4 Mon Sep 17 00:00:00 2001
> From: Colin Ian King <colin.king@canonical.com>
> Date: Thu, 17 Feb 2011 10:49:30 +0000
> Subject: [PATCH] Enable Dell All-In-One volume up/down keys
> 
> Enable volume up and down hotkeys on WMI events
> GUID 284A0E6B-380E-472A-921F-E52786257FB4 and
> GUID 02314822-307C-4F66-bf0E-48AEAEB26CC8.
> 
> Also works around a firmware bug where the _WED method
> should return an integer containing the key code and in fact
> the method returns the key code in element zero of a buffer.
> 
> BugLink: http://bugs.launchpad.net/bugs/701530
> BugLink: http://bugs.launchpad.net/bugs/676997
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Looks much better, but I just noticed that we are missing calls to
sparse_keymap_free(dell_wmi_aio_input_dev) in both probe error handling
path and in remove method (they shoudl be called before unregistering
the device).

Also, can we have probe routine more streamlined, like this:

static int __init dell_wmi_aio_init(void)
{
	int err;
	char *guid;

	guid = dell_wmi_aio_find();
	if (!guid) {
		pr_error("No known WMI GUID found\n");
		return -ENXIO;
	}

	err = dell_wmi_aio_input_setup();
	if (err)
		return err;

	err = wmi_install_notify_handler(guid, dell_wmi_aio_notify, NULL);
	if (err) {
		pr_err("Unable to register notify handler - %d\n", err);
		sparse_keymap_free(dell_wmi_aio_input_dev);
		input_unregister_device(dell_wmi_aio_input_dev);
		return err;
	}

	return 0;
}

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Dell All-In-One volume up/down keys
  2011-02-17 17:21     ` Dmitry Torokhov
@ 2011-02-17 18:44       ` Colin Ian King
  0 siblings, 0 replies; 6+ messages in thread
From: Colin Ian King @ 2011-02-17 18:44 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: platform-driver-x86, Matthew Garrett, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

On Thu, 2011-02-17 at 09:21 -0800, Dmitry Torokhov wrote:
> On Thu, Feb 17, 2011 at 11:10:11AM +0000, Colin Ian King wrote:
> > Thanks for your feedback Dmitry,
> > 
[ snip ]
> Looks much better, but I just noticed that we are missing calls to
> sparse_keymap_free(dell_wmi_aio_input_dev) in both probe error handling
> path and in remove method (they shoudl be called before unregistering
> the device).
> 
Indeed. Thanks for spotting that.  I've also incorporated your
suggestions below too.  Much appreciated.

Attached is the re-worked patch.

Colin

> Also, can we have probe routine more streamlined, like this:
> 
> static int __init dell_wmi_aio_init(void)
> {
> 	int err;
> 	char *guid;
> 
> 	guid = dell_wmi_aio_find();
> 	if (!guid) {
> 		pr_error("No known WMI GUID found\n");
> 		return -ENXIO;
> 	}
> 
> 	err = dell_wmi_aio_input_setup();
> 	if (err)
> 		return err;
> 
> 	err = wmi_install_notify_handler(guid, dell_wmi_aio_notify, NULL);
> 	if (err) {
> 		pr_err("Unable to register notify handler - %d\n", err);
> 		sparse_keymap_free(dell_wmi_aio_input_dev);
> 		input_unregister_device(dell_wmi_aio_input_dev);
> 		return err;
> 	}
> 
> 	return 0;
> }
> 
> Thanks.
> 


[-- Attachment #2: 0001-Enable-Dell-All-In-One-volume-up-down-keys.patch --]
[-- Type: text/x-patch, Size: 6927 bytes --]

>From 89005aa15b03ab269a1368eec6868833d478b4e3 Mon Sep 17 00:00:00 2001
From: Colin Ian King <colin.king@canonical.com>
Date: Thu, 17 Feb 2011 18:36:02 +0000
Subject: [PATCH] Enable Dell All-In-One volume up/down keys

Enable volume up and down hotkeys on WMI events
GUID 284A0E6B-380E-472A-921F-E52786257FB4 and
GUID 02314822-307C-4F66-bf0E-48AEAEB26CC8.

Also works around a firmware bug where the _WED method
should return an integer containing the key code and in fact
the method returns the key code in element zero of a buffer.

BugLink: http://bugs.launchpad.net/bugs/701530
BugLink: http://bugs.launchpad.net/bugs/676997

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/platform/x86/Kconfig        |   13 +++
 drivers/platform/x86/Makefile       |    1 +
 drivers/platform/x86/dell-wmi-aio.c |  172 +++++++++++++++++++++++++++++++++++
 3 files changed, 186 insertions(+), 0 deletions(-)
 create mode 100644 drivers/platform/x86/dell-wmi-aio.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index d163bc2..b2e8ce0 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -101,6 +101,19 @@ config DELL_WMI
 	  To compile this driver as a module, choose M here: the module will
 	  be called dell-wmi.
 
+config DELL_WMI_AIO
+	tristate "WMI Hotkeys for Dell All-In-One series"
+	depends on ACPI_WMI
+	depends on INPUT
+	select INPUT_SPARSEKMAP
+	---help---
+	  Say Y here if you want to support WMI-based hotkeys on Dell
+	  All-In-One machines.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called dell-wmi.
+
+
 config FUJITSU_LAPTOP
 	tristate "Fujitsu Laptop Extras"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 4ec4ff8..26c6a57 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_ACPI_CMPC)		+= classmate-laptop.o
 obj-$(CONFIG_COMPAL_LAPTOP)	+= compal-laptop.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
+obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
 obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
 obj-$(CONFIG_ACERHDF)		+= acerhdf.o
 obj-$(CONFIG_HP_WMI)		+= hp-wmi.o
diff --git a/drivers/platform/x86/dell-wmi-aio.c b/drivers/platform/x86/dell-wmi-aio.c
new file mode 100644
index 0000000..3efe788
--- /dev/null
+++ b/drivers/platform/x86/dell-wmi-aio.c
@@ -0,0 +1,172 @@
+/*
+ *  WMI hotkeys support for Dell All-In-One series
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <acpi/acpi_drivers.h>
+#include <linux/acpi.h>
+#include <linux/string.h>
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+MODULE_DESCRIPTION("WMI hotkeys driver for Dell All-In-One series");
+MODULE_LICENSE("GPL");
+
+#define EVENT_GUID1 "284A0E6B-380E-472A-921F-E52786257FB4"
+#define EVENT_GUID2 "02314822-307C-4F66-BF0E-48AEAEB26CC8"
+
+static const char *dell_wmi_aio_guids[] = {
+	EVENT_GUID1,
+	EVENT_GUID2,
+	NULL
+};
+
+MODULE_ALIAS("wmi:"EVENT_GUID1);
+MODULE_ALIAS("wmi:"EVENT_GUID2);
+
+static const struct key_entry dell_wmi_aio_keymap[] = {
+	{ KE_KEY, 0xc0, { KEY_VOLUMEUP } },
+	{ KE_KEY, 0xc1, { KEY_VOLUMEDOWN } },
+	{ KE_END, 0 }
+};
+
+static struct input_dev *dell_wmi_aio_input_dev;
+
+static void dell_wmi_aio_notify(u32 value, void *context)
+{
+	struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	acpi_status status;
+
+	status = wmi_get_event_data(value, &response);
+	if (status != AE_OK) {
+		pr_info("bad event status 0x%x\n", status);
+		return;
+	}
+
+	obj = (union acpi_object *)response.pointer;
+	if (obj) {
+		unsigned int scancode;
+
+		switch (obj->type) {
+		case ACPI_TYPE_INTEGER:
+			/* Most All-In-One correctly return integer scancode */
+			scancode = obj->integer.value;
+			sparse_keymap_report_event(dell_wmi_aio_input_dev,
+				scancode, 1, true);
+			break;
+		case ACPI_TYPE_BUFFER:
+			/* Broken machines return the scancode in a buffer */
+			if (obj->buffer.pointer && obj->buffer.length > 0) {
+				scancode = obj->buffer.pointer[0];
+				sparse_keymap_report_event(
+					dell_wmi_aio_input_dev,
+					scancode, 1, true);
+			}
+			break;
+		}
+	}
+	kfree(obj);
+}
+
+static int __init dell_wmi_aio_input_setup(void)
+{
+	int err;
+
+	dell_wmi_aio_input_dev = input_allocate_device();
+
+	if (!dell_wmi_aio_input_dev)
+		return -ENOMEM;
+
+	dell_wmi_aio_input_dev->name = "Dell AIO WMI hotkeys";
+	dell_wmi_aio_input_dev->phys = "wmi/input0";
+	dell_wmi_aio_input_dev->id.bustype = BUS_HOST;
+
+	err = sparse_keymap_setup(dell_wmi_aio_input_dev,
+			dell_wmi_aio_keymap, NULL);
+	if (err) {
+		pr_err("Unable to setup input device keymap\n");
+		goto err_free_dev;
+	}
+	err = input_register_device(dell_wmi_aio_input_dev);
+	if (err) {
+		pr_info("Unable to register input device\n");
+		goto err_free_keymap;
+	}
+	return 0;
+
+err_free_keymap:
+	sparse_keymap_free(dell_wmi_aio_input_dev);
+err_free_dev:
+	input_free_device(dell_wmi_aio_input_dev);
+	return err;
+}
+
+static char *dell_wmi_aio_find(void)
+{
+	int i;
+
+	for (i = 0; dell_wmi_aio_guids[i] != NULL; i++)
+		if (wmi_has_guid(dell_wmi_aio_guids[i]))
+			return dell_wmi_aio_guids[i];
+
+	return NULL;
+}
+
+static int __init dell_wmi_aio_init(void)
+{
+	int err;
+	char *guid;
+
+	guid = dell_wmi_aio_find();
+	if (!guid) {
+		pr_warning("No known WMI GUID found\n");
+		return -ENXIO;
+	}
+
+	err = dell_wmi_aio_input_setup();
+	if (err)
+		return err;
+
+	err = wmi_install_notify_handler(guid, dell_wmi_aio_notify, NULL);
+	if (err) {
+		pr_err("Unable to register notify handler - %d\n", err);
+		sparse_keymap_free(dell_wmi_aio_input_dev);
+		input_unregister_device(dell_wmi_aio_input_dev);
+		return err;
+	}
+
+	return 0;
+}
+
+static void __exit dell_wmi_aio_exit(void)
+{
+	char *guid;
+
+	guid = dell_wmi_aio_find();
+	wmi_remove_notify_handler(guid);
+	sparse_keymap_free(dell_wmi_aio_input_dev);
+	input_unregister_device(dell_wmi_aio_input_dev);
+}
+
+module_init(dell_wmi_aio_init);
+module_exit(dell_wmi_aio_exit);
-- 
1.7.2.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-02-17 18:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-14 13:02 [PATCH] Enable Dell All-In-One volume up/down keys Colin King
2011-02-17  7:52 ` Dmitry Torokhov
2011-02-17 11:10   ` Colin Ian King
2011-02-17 17:21     ` Dmitry Torokhov
2011-02-17 18:44       ` Colin Ian King
2011-02-17 16:33   ` Matthew Garrett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).