[PATCH v2 0/7] ideapad: hotkey enablement

[PATCH v2 0/7] ideapad: hotkey enablement

[Ike Panhc]

Here are the enablement patches for hotkey events on ideapads and these patches
are made against current checkout of mainline kernel.

After last time posting on LKML, there are several feedback and these patches
are modified.

>From Dmitry Torokhov, using markup on functions to let the compiler knows where
to put and passing private variable instead of using global vairable. The
acpi_handle is still a global variable becuase we need to pass handle pointer
to rfk_set with its opcode but only one data argument available.

>From Dave Hansen, select INPUT_SPARSEKMAP to fill the dependency.

These patches are available in the git repository at:
  git://kernel.ubuntu.com/ikepanhc/ideapad-laptop.git ideapad-laptop

Ike Panhc (7):
  ideapad: add platform driver for ideapad
  ideapad: let camera power control entry under platform driver
  ideapad: add hotkey support
  ideapad: select INPUT_SPARSEKMAP
  ideapad: add markups, unify comments and return result when init
  ideapad: pass ideapad_priv as argument (part 1)
  ideapad: pass ideapad_priv as argument (part 2)

 drivers/platform/x86/Kconfig          |    1 +
 drivers/platform/x86/ideapad-laptop.c |  255 ++++++++++++++++++++++----------
 2 files changed, 176 insertions(+), 80 deletions(-)

[PATCH 1/7] ideapad: add platform driver for ideapad

[Ike Panhc]

Create /sys/devices/platform/Ideapad for nodes of ideapad landing.

Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |   54 ++++++++++++++++++++++++++++----
 1 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 5ff1220..62901e9 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -27,6 +27,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/rfkill.h>
+#include <linux/platform_device.h>
 
 #define IDEAPAD_DEV_CAMERA	0
 #define IDEAPAD_DEV_WLAN	1
@@ -37,6 +38,7 @@
 struct ideapad_private {
 	acpi_handle handle;
 	struct rfkill *rfk[5];
+	struct platform_device *platform_device;
 } *ideapad_priv;
 
 static struct {
@@ -277,6 +279,35 @@ static void ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)
 	rfkill_destroy(priv->rfk[dev]);
 }
 
+/*
+ * Platform device
+ */
+static int __devinit ideapad_platform_init(void)
+{
+	int result;
+
+	ideapad_priv->platform_device = platform_device_alloc("Ideapad", -1);
+	if (!ideapad_priv->platform_device)
+		return -ENOMEM;
+	platform_set_drvdata(ideapad_priv->platform_device, ideapad_priv);
+
+	result = platform_device_add(ideapad_priv->platform_device);
+	if (result)
+		goto fail_platform_device;
+
+	return 0;
+
+fail_platform_device:
+	platform_device_put(ideapad_priv->platform_device);
+	return result;
+}
+
+static void ideapad_platform_exit(void)
+{
+	platform_device_unregister(ideapad_priv->platform_device);
+}
+/* the above is platform device */
+
 static const struct acpi_device_id ideapad_device_ids[] = {
 	{ "VPC2004", 0},
 	{ "", 0},
@@ -285,7 +316,7 @@ MODULE_DEVICE_TABLE(acpi, ideapad_device_ids);
 
 static int ideapad_acpi_add(struct acpi_device *adevice)
 {
-	int i, cfg;
+	int ret, i, cfg;
 	int devs_present[5];
 	struct ideapad_private *priv;
 
@@ -305,18 +336,20 @@ static int ideapad_acpi_add(struct acpi_device *adevice)
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
+	ideapad_priv = priv;
+
+	ret = ideapad_platform_init();
+	if (ret)
+		goto platform_failed;
 
 	if (devs_present[IDEAPAD_DEV_CAMERA]) {
-		int ret = device_create_file(&adevice->dev, &dev_attr_camera_power);
-		if (ret) {
-			kfree(priv);
-			return ret;
-		}
+		ret = device_create_file(&adevice->dev, &dev_attr_camera_power);
+		if (ret)
+			goto camera_failed;
 	}
 
 	priv->handle = adevice->handle;
 	dev_set_drvdata(&adevice->dev, priv);
-	ideapad_priv = priv;
 	for (i = IDEAPAD_DEV_WLAN; i <= IDEAPAD_DEV_KILLSW; i++) {
 		if (!devs_present[i])
 			continue;
@@ -325,6 +358,12 @@ static int ideapad_acpi_add(struct acpi_device *adevice)
 	}
 	ideapad_sync_rfk_state(adevice);
 	return 0;
+
+camera_failed:
+	ideapad_platform_exit();
+platform_failed:
+	kfree(priv);
+	return ret;
 }
 
 static int ideapad_acpi_remove(struct acpi_device *adevice, int type)
@@ -337,6 +376,7 @@ static int ideapad_acpi_remove(struct acpi_device *adevice, int type)
 	for (i = IDEAPAD_DEV_WLAN; i <= IDEAPAD_DEV_KILLSW; i++)
 		ideapad_unregister_rfkill(adevice, i);
 
+	ideapad_platform_exit();
 	dev_set_drvdata(&adevice->dev, NULL);
 	kfree(priv);
 	return 0;
-- 
1.7.1

Re: [PATCH 1/7] ideapad: add platform driver for ideapad

[Dave Hansen]

On Thu, 2010-12-09 at 15:56 +0800, Ike Panhc wrote:
> Create /sys/devices/platform/Ideapad for nodes of ideapad landing.

Any chance you want to make that 'ideapad'?  It would be a bit more
consistent with all the other stuff in that directory, and will surely
save me some trips over to the shift key. :)

-- Dave

[PATCH 2/7] ideapad: let camera power control entry under platform driver

[Ike Panhc]

The entry was at /sys/devices/LNXSYSTM:00/../VPC2004:00/camera_power
move to /sys/devices/platform/Ideapad/camera_power

Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |   55 +++++++++++++++------------------
 1 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 62901e9..a6d3e04 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -282,6 +282,15 @@ static void ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)
 /*
  * Platform device
  */
+static struct attribute *ideapad_attributes[] = {
+	&dev_attr_camera_power.attr,
+	NULL
+};
+
+static struct attribute_group ideapad_attribute_group = {
+	.attrs = ideapad_attributes
+};
+
 static int __devinit ideapad_platform_init(void)
 {
 	int result;
@@ -295,8 +304,14 @@ static int __devinit ideapad_platform_init(void)
 	if (result)
 		goto fail_platform_device;
 
+	result = sysfs_create_group(&ideapad_priv->platform_device->dev.kobj,
+				    &ideapad_attribute_group);
+	if (result)
+		goto fail_sysfs;
 	return 0;
 
+fail_sysfs:
+	platform_device_del(ideapad_priv->platform_device);
 fail_platform_device:
 	platform_device_put(ideapad_priv->platform_device);
 	return result;
@@ -304,6 +319,8 @@ fail_platform_device:
 
 static void ideapad_platform_exit(void)
 {
+	sysfs_remove_group(&ideapad_priv->platform_device->dev.kobj,
+			   &ideapad_attribute_group);
 	platform_device_unregister(ideapad_priv->platform_device);
 }
 /* the above is platform device */
@@ -317,50 +334,30 @@ MODULE_DEVICE_TABLE(acpi, ideapad_device_ids);
 static int ideapad_acpi_add(struct acpi_device *adevice)
 {
 	int ret, i, cfg;
-	int devs_present[5];
 	struct ideapad_private *priv;
 
 	if (read_method_int(adevice->handle, "_CFG", &cfg))
 		return -ENODEV;
 
-	for (i = IDEAPAD_DEV_CAMERA; i < IDEAPAD_DEV_KILLSW; i++) {
-		if (test_bit(ideapad_rfk_data[i].cfgbit, (unsigned long *)&cfg))
-			devs_present[i] = 1;
-		else
-			devs_present[i] = 0;
-	}
-
-	/* The hardware switch is always present */
-	devs_present[IDEAPAD_DEV_KILLSW] = 1;
-
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 	ideapad_priv = priv;
+	priv->handle = adevice->handle;
+	dev_set_drvdata(&adevice->dev, priv);
 
 	ret = ideapad_platform_init();
 	if (ret)
 		goto platform_failed;
 
-	if (devs_present[IDEAPAD_DEV_CAMERA]) {
-		ret = device_create_file(&adevice->dev, &dev_attr_camera_power);
-		if (ret)
-			goto camera_failed;
-	}
-
-	priv->handle = adevice->handle;
-	dev_set_drvdata(&adevice->dev, priv);
-	for (i = IDEAPAD_DEV_WLAN; i <= IDEAPAD_DEV_KILLSW; i++) {
-		if (!devs_present[i])
-			continue;
-
-		ideapad_register_rfkill(adevice, i);
+	for (i = IDEAPAD_DEV_WLAN; i < IDEAPAD_DEV_KILLSW; i++) {
+		if (test_bit(ideapad_rfk_data[i].cfgbit, (unsigned long *)&cfg))
+			ideapad_register_rfkill(adevice, i);
 	}
 	ideapad_sync_rfk_state(adevice);
+
 	return 0;
 
-camera_failed:
-	ideapad_platform_exit();
 platform_failed:
 	kfree(priv);
 	return ret;
@@ -371,14 +368,12 @@ static int ideapad_acpi_remove(struct acpi_device *adevice, int type)
 	struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
 	int i;
 
-	device_remove_file(&adevice->dev, &dev_attr_camera_power);
-
-	for (i = IDEAPAD_DEV_WLAN; i <= IDEAPAD_DEV_KILLSW; i++)
+	for (i = IDEAPAD_DEV_WLAN; i < IDEAPAD_DEV_KILLSW; i++)
 		ideapad_unregister_rfkill(adevice, i);
-
 	ideapad_platform_exit();
 	dev_set_drvdata(&adevice->dev, NULL);
 	kfree(priv);
+
 	return 0;
 }
 
-- 
1.7.1

[PATCH 3/7] ideapad: add hotkey support

[Ike Panhc]

Hotkey enabled by this patch:
  Fn+F3: Video mode switch
  Fn+F5: software rfkill for wifi

For some ideapad when push Fn+F3, hardware generates Super-P keys, those key
will not be enabled by this patch.

Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |   72 +++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index a6d3e04..fdb5fde 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -28,6 +28,8 @@
 #include <acpi/acpi_drivers.h>
 #include <linux/rfkill.h>
 #include <linux/platform_device.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
 
 #define IDEAPAD_DEV_CAMERA	0
 #define IDEAPAD_DEV_WLAN	1
@@ -39,6 +41,7 @@ struct ideapad_private {
 	acpi_handle handle;
 	struct rfkill *rfk[5];
 	struct platform_device *platform_device;
+	struct input_dev *inputdev;
 } *ideapad_priv;
 
 static struct {
@@ -325,6 +328,66 @@ static void ideapad_platform_exit(void)
 }
 /* the above is platform device */
 
+/*
+ * input device
+ */
+static const struct key_entry ideapad_keymap[] = {
+	{ KE_KEY, 0x06, { KEY_SWITCHVIDEOMODE } },
+	{ KE_KEY, 0x0D, { KEY_WLAN } },
+	{ KE_END, 0 },
+};
+
+static int __devinit ideapad_input_init(void)
+{
+	struct input_dev *inputdev;
+	int error;
+
+	inputdev = input_allocate_device();
+	if (!inputdev) {
+		pr_info("Unable to allocate input device\n");
+		return -ENOMEM;
+	}
+
+	inputdev->name = "Ideapad extra buttons";
+	inputdev->phys = "ideapad/input0";
+	inputdev->id.bustype = BUS_HOST;
+	inputdev->dev.parent = &ideapad_priv->platform_device->dev;
+
+	error = sparse_keymap_setup(inputdev, ideapad_keymap, NULL);
+	if (error) {
+		pr_err("Unable to setup input device keymap\n");
+		goto err_free_dev;
+	}
+
+	error = input_register_device(inputdev);
+	if (error) {
+		pr_err("Unable to register input device\n");
+		goto err_free_keymap;
+	}
+
+	ideapad_priv->inputdev = inputdev;
+	return 0;
+
+err_free_keymap:
+	sparse_keymap_free(inputdev);
+err_free_dev:
+	input_free_device(inputdev);
+	return error;
+}
+
+static void __devexit ideapad_input_exit(void)
+{
+	sparse_keymap_free(ideapad_priv->inputdev);
+	input_unregister_device(ideapad_priv->inputdev);
+	ideapad_priv->inputdev = NULL;
+}
+
+static void ideapad_input_report(unsigned long scancode)
+{
+	sparse_keymap_report_event(ideapad_priv->inputdev, scancode, 1, true);
+}
+/* the above is input device */
+
 static const struct acpi_device_id ideapad_device_ids[] = {
 	{ "VPC2004", 0},
 	{ "", 0},
@@ -350,6 +413,10 @@ static int ideapad_acpi_add(struct acpi_device *adevice)
 	if (ret)
 		goto platform_failed;
 
+	ret = ideapad_input_init();
+	if (ret)
+		goto input_failed;
+
 	for (i = IDEAPAD_DEV_WLAN; i < IDEAPAD_DEV_KILLSW; i++) {
 		if (test_bit(ideapad_rfk_data[i].cfgbit, (unsigned long *)&cfg))
 			ideapad_register_rfkill(adevice, i);
@@ -358,6 +425,8 @@ static int ideapad_acpi_add(struct acpi_device *adevice)
 
 	return 0;
 
+input_failed:
+	ideapad_platform_exit();
 platform_failed:
 	kfree(priv);
 	return ret;
@@ -370,6 +439,7 @@ static int ideapad_acpi_remove(struct acpi_device *adevice, int type)
 
 	for (i = IDEAPAD_DEV_WLAN; i < IDEAPAD_DEV_KILLSW; i++)
 		ideapad_unregister_rfkill(adevice, i);
+	ideapad_input_exit();
 	ideapad_platform_exit();
 	dev_set_drvdata(&adevice->dev, NULL);
 	kfree(priv);
@@ -392,6 +462,8 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
 		if (test_bit(vpc_bit, &vpc1)) {
 			if (vpc_bit == 9)
 				ideapad_sync_rfk_state(adevice);
+			else
+				ideapad_input_report(vpc_bit);
 		}
 	}
 }
-- 
1.7.1

[PATCH 4/7] ideapad: select INPUT_SPARSEKMAP

[Ike Panhc]

Thanks for Dave Hansen report the problem. If CONFIG_INPUT_SPARSEKMAP is not
set, when building, you will have error message:

  ERROR: "sparse_keymap_setup" [drivers/platform/x86/ideapad-laptop.ko] undefined!
  ERROR: "sparse_keymap_free" [drivers/platform/x86/ideapad-laptop.ko] undefined!
  ERROR: "sparse_keymap_report_event" [drivers/platform/x86/ideapad-laptop.ko] undefined!

To select INPUT_SPARSEKMAP solve this issue.

Ref: http://lkml.org/lkml/2010/12/2/340

Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 drivers/platform/x86/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index faec777..879d266 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -226,6 +226,7 @@ config IDEAPAD_LAPTOP
 	tristate "Lenovo IdeaPad Laptop Extras"
 	depends on ACPI
 	depends on RFKILL
+	select INPUT_SPARSEKMAP
 	help
 	  This is a driver for the rfkill switches on Lenovo IdeaPad netbooks.
 
-- 
1.7.1

Re: [PATCH 4/7] ideapad: select INPUT_SPARSEKMAP

[Dmitry Torokhov]

On Thu, Dec 09, 2010 at 03:57:20PM +0800, Ike Panhc wrote:
> Thanks for Dave Hansen report the problem. If CONFIG_INPUT_SPARSEKMAP is not
> set, when building, you will have error message:
> 
>   ERROR: "sparse_keymap_setup" [drivers/platform/x86/ideapad-laptop.ko] undefined!
>   ERROR: "sparse_keymap_free" [drivers/platform/x86/ideapad-laptop.ko] undefined!
>   ERROR: "sparse_keymap_report_event" [drivers/platform/x86/ideapad-laptop.ko] undefined!
> 
> To select INPUT_SPARSEKMAP solve this issue.
> 
> Ref: http://lkml.org/lkml/2010/12/2/340
>

I'd recommend folding this patch together with the patch that introduces
sparse keymap usage. Leaving them separated served only to risk compile
errors during bisecting.

Thanks.

-- 
Dmitry

[PATCH 5/7] ideapad: add markups, unify comments and return result when init

[Ike Panhc]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3558 bytes --]

1. Add markups on init and exit functions
2. Unify the comments in the same style
3. Return result when module initial

Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index fdb5fde..2c4830c 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1,5 +1,5 @@
 /*
- *  ideapad_acpi.c - Lenovo IdeaPad ACPI Extras
+ *  ideapad-laptop.c - Lenovo IdeaPad ACPI Extras
  *
  *  Copyright © 2010 Intel Corporation
  *  Copyright © 2010 David Woodhouse <dwmw2@infradead.org>
@@ -168,8 +168,10 @@ static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data)
 	pr_err("timeout in write_ec_cmd\n");
 	return -1;
 }
-/* the above is ACPI helpers */
 
+/*
+ * camera power
+ */
 static ssize_t show_ideapad_cam(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
@@ -203,6 +205,9 @@ static ssize_t store_ideapad_cam(struct device *dev,
 
 static DEVICE_ATTR(camera_power, 0644, show_ideapad_cam, store_ideapad_cam);
 
+/*
+ * Rfkill
+ */
 static int ideapad_rfk_set(void *data, bool blocked)
 {
 	int device = (unsigned long)data;
@@ -235,7 +240,7 @@ static void ideapad_sync_rfk_state(struct acpi_device *adevice)
 			rfkill_set_hw_state(priv->rfk[i], hw_blocked);
 }
 
-static int ideapad_register_rfkill(struct acpi_device *adevice, int dev)
+static int __devinit ideapad_register_rfkill(struct acpi_device *adevice, int dev)
 {
 	struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
 	int ret;
@@ -271,7 +276,7 @@ static int ideapad_register_rfkill(struct acpi_device *adevice, int dev)
 	return 0;
 }
 
-static void ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)
+static void __devexit ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)
 {
 	struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
 
@@ -326,7 +331,6 @@ static void ideapad_platform_exit(void)
 			   &ideapad_attribute_group);
 	platform_device_unregister(ideapad_priv->platform_device);
 }
-/* the above is platform device */
 
 /*
  * input device
@@ -386,15 +390,17 @@ static void ideapad_input_report(unsigned long scancode)
 {
 	sparse_keymap_report_event(ideapad_priv->inputdev, scancode, 1, true);
 }
-/* the above is input device */
 
+/*
+ * module init/exit
+ */
 static const struct acpi_device_id ideapad_device_ids[] = {
 	{ "VPC2004", 0},
 	{ "", 0},
 };
 MODULE_DEVICE_TABLE(acpi, ideapad_device_ids);
 
-static int ideapad_acpi_add(struct acpi_device *adevice)
+static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
 {
 	int ret, i, cfg;
 	struct ideapad_private *priv;
@@ -432,7 +438,7 @@ platform_failed:
 	return ret;
 }
 
-static int ideapad_acpi_remove(struct acpi_device *adevice, int type)
+static int __devexit ideapad_acpi_remove(struct acpi_device *adevice, int type)
 {
 	struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
 	int i;
@@ -478,19 +484,14 @@ static struct acpi_driver ideapad_acpi_driver = {
 	.owner = THIS_MODULE,
 };
 
-
 static int __init ideapad_acpi_module_init(void)
 {
-	acpi_bus_register_driver(&ideapad_acpi_driver);
-
-	return 0;
+	return acpi_bus_register_driver(&ideapad_acpi_driver);
 }
 
-
 static void __exit ideapad_acpi_module_exit(void)
 {
 	acpi_bus_unregister_driver(&ideapad_acpi_driver);
-
 }
 
 MODULE_AUTHOR("David Woodhouse <dwmw2@infradead.org>");
-- 
1.7.1

[PATCH 6/7] ideapad: pass ideapad_priv as argument (part 1)

[Ike Panhc]

Passing ideapad_priv as argument and try not to using too much global variable.
This is part 1 for platform driver and input device.

Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |   54 +++++++++++++++++----------------
 1 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 2c4830c..eda7267 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -299,37 +299,37 @@ static struct attribute_group ideapad_attribute_group = {
 	.attrs = ideapad_attributes
 };
 
-static int __devinit ideapad_platform_init(void)
+static int __devinit ideapad_platform_init(struct ideapad_private *priv)
 {
 	int result;
 
-	ideapad_priv->platform_device = platform_device_alloc("Ideapad", -1);
-	if (!ideapad_priv->platform_device)
+	priv->platform_device = platform_device_alloc("Ideapad", -1);
+	if (!priv->platform_device)
 		return -ENOMEM;
-	platform_set_drvdata(ideapad_priv->platform_device, ideapad_priv);
+	platform_set_drvdata(priv->platform_device, priv);
 
-	result = platform_device_add(ideapad_priv->platform_device);
+	result = platform_device_add(priv->platform_device);
 	if (result)
 		goto fail_platform_device;
 
-	result = sysfs_create_group(&ideapad_priv->platform_device->dev.kobj,
+	result = sysfs_create_group(&priv->platform_device->dev.kobj,
 				    &ideapad_attribute_group);
 	if (result)
 		goto fail_sysfs;
 	return 0;
 
 fail_sysfs:
-	platform_device_del(ideapad_priv->platform_device);
+	platform_device_del(priv->platform_device);
 fail_platform_device:
-	platform_device_put(ideapad_priv->platform_device);
+	platform_device_put(priv->platform_device);
 	return result;
 }
 
-static void ideapad_platform_exit(void)
+static void ideapad_platform_exit(struct ideapad_private *priv)
 {
-	sysfs_remove_group(&ideapad_priv->platform_device->dev.kobj,
+	sysfs_remove_group(&priv->platform_device->dev.kobj,
 			   &ideapad_attribute_group);
-	platform_device_unregister(ideapad_priv->platform_device);
+	platform_device_unregister(priv->platform_device);
 }
 
 /*
@@ -341,7 +341,7 @@ static const struct key_entry ideapad_keymap[] = {
 	{ KE_END, 0 },
 };
 
-static int __devinit ideapad_input_init(void)
+static int __devinit ideapad_input_init(struct ideapad_private *priv)
 {
 	struct input_dev *inputdev;
 	int error;
@@ -355,7 +355,7 @@ static int __devinit ideapad_input_init(void)
 	inputdev->name = "Ideapad extra buttons";
 	inputdev->phys = "ideapad/input0";
 	inputdev->id.bustype = BUS_HOST;
-	inputdev->dev.parent = &ideapad_priv->platform_device->dev;
+	inputdev->dev.parent = &priv->platform_device->dev;
 
 	error = sparse_keymap_setup(inputdev, ideapad_keymap, NULL);
 	if (error) {
@@ -369,7 +369,7 @@ static int __devinit ideapad_input_init(void)
 		goto err_free_keymap;
 	}
 
-	ideapad_priv->inputdev = inputdev;
+	priv->inputdev = inputdev;
 	return 0;
 
 err_free_keymap:
@@ -379,16 +379,17 @@ err_free_dev:
 	return error;
 }
 
-static void __devexit ideapad_input_exit(void)
+static void __devexit ideapad_input_exit(struct ideapad_private *priv)
 {
-	sparse_keymap_free(ideapad_priv->inputdev);
-	input_unregister_device(ideapad_priv->inputdev);
-	ideapad_priv->inputdev = NULL;
+	sparse_keymap_free(priv->inputdev);
+	input_unregister_device(priv->inputdev);
+	priv->inputdev = NULL;
 }
 
-static void ideapad_input_report(unsigned long scancode)
+static void ideapad_input_report(struct ideapad_private *priv,
+				 unsigned long scancode)
 {
-	sparse_keymap_report_event(ideapad_priv->inputdev, scancode, 1, true);
+	sparse_keymap_report_event(priv->inputdev, scancode, 1, true);
 }
 
 /*
@@ -415,11 +416,11 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
 	priv->handle = adevice->handle;
 	dev_set_drvdata(&adevice->dev, priv);
 
-	ret = ideapad_platform_init();
+	ret = ideapad_platform_init(priv);
 	if (ret)
 		goto platform_failed;
 
-	ret = ideapad_input_init();
+	ret = ideapad_input_init(priv);
 	if (ret)
 		goto input_failed;
 
@@ -432,7 +433,7 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
 	return 0;
 
 input_failed:
-	ideapad_platform_exit();
+	ideapad_platform_exit(priv);
 platform_failed:
 	kfree(priv);
 	return ret;
@@ -445,8 +446,8 @@ static int __devexit ideapad_acpi_remove(struct acpi_device *adevice, int type)
 
 	for (i = IDEAPAD_DEV_WLAN; i < IDEAPAD_DEV_KILLSW; i++)
 		ideapad_unregister_rfkill(adevice, i);
-	ideapad_input_exit();
-	ideapad_platform_exit();
+	ideapad_input_exit(priv);
+	ideapad_platform_exit(priv);
 	dev_set_drvdata(&adevice->dev, NULL);
 	kfree(priv);
 
@@ -455,6 +456,7 @@ static int __devexit ideapad_acpi_remove(struct acpi_device *adevice, int type)
 
 static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
 {
+	struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
 	acpi_handle handle = adevice->handle;
 	unsigned long vpc1, vpc2, vpc_bit;
 
@@ -469,7 +471,7 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
 			if (vpc_bit == 9)
 				ideapad_sync_rfk_state(adevice);
 			else
-				ideapad_input_report(vpc_bit);
+				ideapad_input_report(priv, vpc_bit);
 		}
 	}
 }
-- 
1.7.1

[PATCH 7/7] ideapad: pass ideapad_priv as argument (part 2)

[Ike Panhc]

Passing ideapad_priv as argument and try not to using too much global variable.
This is part 2 for rfkill.

Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |   69 +++++++++++++--------------------
 1 files changed, 27 insertions(+), 42 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index eda7267..31556cb 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -31,32 +31,15 @@
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
 
-#define IDEAPAD_DEV_CAMERA	0
-#define IDEAPAD_DEV_WLAN	1
-#define IDEAPAD_DEV_BLUETOOTH	2
-#define IDEAPAD_DEV_3G		3
-#define IDEAPAD_DEV_KILLSW	4
+#define IDEAPAD_RFKILL_DEV_NUM	(3)
 
 struct ideapad_private {
-	acpi_handle handle;
-	struct rfkill *rfk[5];
+	struct rfkill *rfk[IDEAPAD_RFKILL_DEV_NUM];
 	struct platform_device *platform_device;
 	struct input_dev *inputdev;
-} *ideapad_priv;
-
-static struct {
-	char *name;
-	int cfgbit;
-	int opcode;
-	int type;
-} ideapad_rfk_data[] = {
-	{ "ideapad_camera",	19, 0x1E, NUM_RFKILL_TYPES },
-	{ "ideapad_wlan",	18, 0x15, RFKILL_TYPE_WLAN },
-	{ "ideapad_bluetooth",	16, 0x17, RFKILL_TYPE_BLUETOOTH },
-	{ "ideapad_3g",		17, 0x20, RFKILL_TYPE_WWAN },
-	{ "ideapad_killsw",	0,  0,    RFKILL_TYPE_WLAN }
 };
 
+static acpi_handle ideapad_handle;
 static bool no_bt_rfkill;
 module_param(no_bt_rfkill, bool, 0444);
 MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
@@ -176,11 +159,9 @@ static ssize_t show_ideapad_cam(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
 {
-	struct ideapad_private *priv = dev_get_drvdata(dev);
-	acpi_handle handle = priv->handle;
 	unsigned long result;
 
-	if (read_ec_data(handle, 0x1D, &result))
+	if (read_ec_data(ideapad_handle, 0x1D, &result))
 		return sprintf(buf, "-1\n");
 	return sprintf(buf, "%lu\n", result);
 }
@@ -189,15 +170,13 @@ static ssize_t store_ideapad_cam(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
 {
-	struct ideapad_private *priv = dev_get_drvdata(dev);
-	acpi_handle handle = priv->handle;
 	int ret, state;
 
 	if (!count)
 		return 0;
 	if (sscanf(buf, "%i", &state) != 1)
 		return -EINVAL;
-	ret = write_ec_cmd(handle, 0x1E, state);
+	ret = write_ec_cmd(ideapad_handle, 0x1E, state);
 	if (ret < 0)
 		return ret;
 	return count;
@@ -208,16 +187,22 @@ static DEVICE_ATTR(camera_power, 0644, show_ideapad_cam, store_ideapad_cam);
 /*
  * Rfkill
  */
+const static struct {
+	char *name;
+	int cfgbit;
+	int opcode;
+	int type;
+} ideapad_rfk_data[] = {
+	{ "ideapad_wlan",	18, 0x15, RFKILL_TYPE_WLAN },
+	{ "ideapad_bluetooth",	16, 0x17, RFKILL_TYPE_BLUETOOTH },
+	{ "ideapad_3g",		17, 0x20, RFKILL_TYPE_WWAN },
+};
+
 static int ideapad_rfk_set(void *data, bool blocked)
 {
-	int device = (unsigned long)data;
-
-	if (device == IDEAPAD_DEV_KILLSW)
-		return -EINVAL;
+	unsigned long opcode = (unsigned long)data;
 
-	return write_ec_cmd(ideapad_priv->handle,
-			    ideapad_rfk_data[device].opcode,
-			    !blocked);
+	return write_ec_cmd(ideapad_handle, opcode, !blocked);
 }
 
 static struct rfkill_ops ideapad_rfk_ops = {
@@ -227,15 +212,14 @@ static struct rfkill_ops ideapad_rfk_ops = {
 static void ideapad_sync_rfk_state(struct acpi_device *adevice)
 {
 	struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
-	acpi_handle handle = priv->handle;
 	unsigned long hw_blocked;
 	int i;
 
-	if (read_ec_data(handle, 0x23, &hw_blocked))
+	if (read_ec_data(ideapad_handle, 0x23, &hw_blocked))
 		return;
 	hw_blocked = !hw_blocked;
 
-	for (i = IDEAPAD_DEV_WLAN; i <= IDEAPAD_DEV_KILLSW; i++)
+	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
 		if (priv->rfk[i])
 			rfkill_set_hw_state(priv->rfk[i], hw_blocked);
 }
@@ -249,7 +233,7 @@ static int __devinit ideapad_register_rfkill(struct acpi_device *adevice, int de
 	if (no_bt_rfkill &&
 	    (ideapad_rfk_data[dev].type == RFKILL_TYPE_BLUETOOTH)) {
 		/* Force to enable bluetooth when no_bt_rfkill=1 */
-		write_ec_cmd(ideapad_priv->handle,
+		write_ec_cmd(ideapad_handle,
 			     ideapad_rfk_data[dev].opcode, 1);
 		return 0;
 	}
@@ -260,7 +244,7 @@ static int __devinit ideapad_register_rfkill(struct acpi_device *adevice, int de
 	if (!priv->rfk[dev])
 		return -ENOMEM;
 
-	if (read_ec_data(ideapad_priv->handle, ideapad_rfk_data[dev].opcode-1,
+	if (read_ec_data(ideapad_handle, ideapad_rfk_data[dev].opcode-1,
 			 &sw_blocked)) {
 		rfkill_init_sw_state(priv->rfk[dev], 0);
 	} else {
@@ -412,9 +396,8 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
-	ideapad_priv = priv;
-	priv->handle = adevice->handle;
 	dev_set_drvdata(&adevice->dev, priv);
+	ideapad_handle = adevice->handle;
 
 	ret = ideapad_platform_init(priv);
 	if (ret)
@@ -424,9 +407,11 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
 	if (ret)
 		goto input_failed;
 
-	for (i = IDEAPAD_DEV_WLAN; i < IDEAPAD_DEV_KILLSW; i++) {
+	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++) {
 		if (test_bit(ideapad_rfk_data[i].cfgbit, (unsigned long *)&cfg))
 			ideapad_register_rfkill(adevice, i);
+		else
+			priv->rfk[i] = NULL;
 	}
 	ideapad_sync_rfk_state(adevice);
 
@@ -444,7 +429,7 @@ static int __devexit ideapad_acpi_remove(struct acpi_device *adevice, int type)
 	struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
 	int i;
 
-	for (i = IDEAPAD_DEV_WLAN; i < IDEAPAD_DEV_KILLSW; i++)
+	for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
 		ideapad_unregister_rfkill(adevice, i);
 	ideapad_input_exit(priv);
 	ideapad_platform_exit(priv);
-- 
1.7.1

Re: [PATCH v2 0/7] ideapad: hotkey enablement

[Corentin Chary]

On Thu, Dec 9, 2010 at 8:56 AM, Ike Panhc <ike.pan@canonical.com> wrote:
> Here are the enablement patches for hotkey events on ideapads and these patches
> are made against current checkout of mainline kernel.
>
> After last time posting on LKML, there are several feedback and these patches
> are modified.
>
> From Dmitry Torokhov, using markup on functions to let the compiler knows where
> to put and passing private variable instead of using global vairable. The

Great,

> acpi_handle is still a global variable becuase we need to pass handle pointer
> to rfk_set with its opcode but only one data argument available.

Of course there is only one data argument available, but since it's a
void *, you
can use a struct with your handle and the opcode :).

> From Dave Hansen, select INPUT_SPARSEKMAP to fill the dependency.
>
> These patches are available in the git repository at:
>  git://kernel.ubuntu.com/ikepanhc/ideapad-laptop.git ideapad-laptop
>
> Ike Panhc (7):
>  ideapad: add platform driver for ideapad
>  ideapad: let camera power control entry under platform driver

Maybe you could document that ? See
Documentation/ABI/testing/sysfs-platform-* files.

>  ideapad: add hotkey support
>  ideapad: select INPUT_SPARSEKMAP
>  ideapad: add markups, unify comments and return result when init
>  ideapad: pass ideapad_priv as argument (part 1)
>  ideapad: pass ideapad_priv as argument (part 2)
>
>  drivers/platform/x86/Kconfig          |    1 +
>  drivers/platform/x86/ideapad-laptop.c |  255 ++++++++++++++++++++++----------
>  2 files changed, 176 insertions(+), 80 deletions(-)


-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH v2 0/7] ideapad: hotkey enablement

[Ike Panhc]

On 12/09/2010 04:39 PM, Corentin Chary wrote:
>> acpi_handle is still a global variable becuase we need to pass handle pointer
>> to rfk_set with its opcode but only one data argument available.
> 
> Of course there is only one data argument available, but since it's a
> void *, you
> can use a struct with your handle and the opcode :).
> 

I have tried it. It needs to copy the same handle three times to each struture.
Since we only have an acpi handle, I think to have it global will let driver
easy read.