2010-12-09 07:56:29

by Ike Panhc

[permalink] [raw]
Subject: [PATCH v2 0/7] ideapad: hotkey enablement

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(-)


2010-12-09 07:56:57

by Ike Panhc

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

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

Signed-off-by: Ike Panhc <[email protected]>
---
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

2010-12-09 07:57:09

by Ike Panhc

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

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 <[email protected]>
---
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

2010-12-09 07:57:21

by Ike Panhc

[permalink] [raw]
Subject: [PATCH 3/7] ideapad: add hotkey support

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 <[email protected]>
---
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

2010-12-09 07:57:31

by Ike Panhc

[permalink] [raw]
Subject: [PATCH 4/7] ideapad: select INPUT_SPARSEKMAP

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 <[email protected]>
Signed-off-by: Ike Panhc <[email protected]>
---
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

2010-12-09 07:57:39

by Ike Panhc

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

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 <[email protected]>
---
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 <[email protected]>
@@ -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 <[email protected]>");
--
1.7.1

2010-12-09 07:57:48

by Ike Panhc

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

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 <[email protected]>
---
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

2010-12-09 07:57:59

by Ike Panhc

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

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 <[email protected]>
---
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

2010-12-09 08:13:39

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 4/7] ideapad: select INPUT_SPARSEKMAP

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

2010-12-09 08:39:49

by Corentin Chary

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

On Thu, Dec 9, 2010 at 8:56 AM, Ike Panhc <[email protected]> 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

2010-12-09 17:05:30

by Dave Hansen

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

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

2010-12-13 08:49:43

by Ike Panhc

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

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.