Changes since last post
* Using generic blacklight class.
* Drop touchpad sysfs node. I will send patch again after knowing which
PM infrastructure for input device.
* More information for cfg bits in documents
* Let sysfs node invisible if no camera.
These patches are also available in the git repository at:
git://kernel.ubuntu.com/ikepanhc/ideapad-laptop.git ideapad-laptop
Ike Panhc (3):
ideapad: define cfg bits and create sysfs node for cfg
ideapad: let camera_power node invisiable if no camera
ideapad: add backlight driver
.../ABI/testing/sysfs-platform-ideapad-laptop | 17 ++
drivers/platform/x86/ideapad-laptop.c | 198 +++++++++++++++++---
2 files changed, 189 insertions(+), 26 deletions(-)
--
1.7.4.1
Create /sys/devices/platform/ideapad/cfg for showing cfg value.
Signed-off-by: Ike Panhc <[email protected]>
---
.../ABI/testing/sysfs-platform-ideapad-laptop | 17 +++++++
drivers/platform/x86/ideapad-laptop.c | 51 +++++++++++++------
2 files changed, 52 insertions(+), 16 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
index 807fca2..ff53183 100644
--- a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
+++ b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
@@ -4,3 +4,20 @@ KernelVersion: 2.6.37
Contact: "Ike Panhc <[email protected]>"
Description:
Control the power of camera module. 1 means on, 0 means off.
+
+What: /sys/devices/platform/ideapad/cfg
+Date: Jun 2011
+KernelVersion: 3.1
+Contact: "Ike Panhc <[email protected]>"
+Description:
+ Ideapad capability bits.
+ Bit 8-10: 1 - Intel graphic only
+ 2 - ATI graphic only
+ 3 - Nvidia graphic only
+ 4 - Intel and ATI graphic
+ 5 - Intel and Nvidia graphic
+ Bit 16: Bluetooth exist (1 for exist)
+ Bit 17: 3G exist (1 for exist)
+ Bit 18: Wifi exist (1 for exist)
+ Bit 19: Camera exist (1 for exist)
+
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index bfdda33..51d4144 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -35,10 +35,15 @@
#define IDEAPAD_RFKILL_DEV_NUM (3)
+#define CFG_BT_BIT (16)
+#define CFG_3G_BIT (17)
+#define CFG_WIFI_BIT (18)
+
struct ideapad_private {
struct rfkill *rfk[IDEAPAD_RFKILL_DEV_NUM];
struct platform_device *platform_device;
struct input_dev *inputdev;
+ unsigned long cfg;
};
static acpi_handle ideapad_handle;
@@ -155,7 +160,7 @@ static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data)
}
/*
- * camera power
+ * sysfs
*/
static ssize_t show_ideapad_cam(struct device *dev,
struct device_attribute *attr,
@@ -186,6 +191,27 @@ static ssize_t store_ideapad_cam(struct device *dev,
static DEVICE_ATTR(camera_power, 0644, show_ideapad_cam, store_ideapad_cam);
+static ssize_t show_ideapad_cfg(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ideapad_private *priv = dev_get_drvdata(dev);
+
+ return sprintf(buf, "0x%.8X\n", (unsigned int)(priv->cfg));
+}
+
+static DEVICE_ATTR(cfg, 0444, show_ideapad_cfg, NULL);
+
+static struct attribute *ideapad_attributes[] = {
+ &dev_attr_camera_power.attr,
+ &dev_attr_cfg.attr,
+ NULL
+};
+
+static struct attribute_group ideapad_attribute_group = {
+ .attrs = ideapad_attributes
+};
+
/*
* Rfkill
*/
@@ -197,9 +223,9 @@ struct ideapad_rfk_data {
};
const struct ideapad_rfk_data ideapad_rfk_data[] = {
- { "ideapad_wlan", 18, 0x15, RFKILL_TYPE_WLAN },
- { "ideapad_bluetooth", 16, 0x17, RFKILL_TYPE_BLUETOOTH },
- { "ideapad_3g", 17, 0x20, RFKILL_TYPE_WWAN },
+ { "ideapad_wlan", CFG_WIFI_BIT, 0x15, RFKILL_TYPE_WLAN },
+ { "ideapad_bluetooth", CFG_BT_BIT, 0x17, RFKILL_TYPE_BLUETOOTH },
+ { "ideapad_3g", CFG_3G_BIT, 0x20, RFKILL_TYPE_WWAN },
};
static int ideapad_rfk_set(void *data, bool blocked)
@@ -280,15 +306,6 @@ static void __devexit ideapad_unregister_rfkill(struct acpi_device *adevice,
/*
* 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(struct ideapad_private *priv)
{
int result;
@@ -393,10 +410,11 @@ MODULE_DEVICE_TABLE(acpi, ideapad_device_ids);
static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
{
- int ret, i, cfg;
+ int ret, i;
+ unsigned long cfg;
struct ideapad_private *priv;
- if (read_method_int(adevice->handle, "_CFG", &cfg))
+ if (read_method_int(adevice->handle, "_CFG", (int *)&cfg))
return -ENODEV;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
@@ -404,6 +422,7 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
return -ENOMEM;
dev_set_drvdata(&adevice->dev, priv);
ideapad_handle = adevice->handle;
+ priv->cfg = cfg;
ret = ideapad_platform_init(priv);
if (ret)
@@ -414,7 +433,7 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
goto input_failed;
for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++) {
- if (test_bit(ideapad_rfk_data[i].cfgbit, (unsigned long *)&cfg))
+ if (test_bit(ideapad_rfk_data[i].cfgbit, &cfg))
ideapad_register_rfkill(adevice, i);
else
priv->rfk[i] = NULL;
--
1.7.4.1
Signed-off-by: Ike Panhc <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 51d4144..678bbf7 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -38,6 +38,7 @@
#define CFG_BT_BIT (16)
#define CFG_3G_BIT (17)
#define CFG_WIFI_BIT (18)
+#define CFG_CAMERA_BIT (19)
struct ideapad_private {
struct rfkill *rfk[IDEAPAD_RFKILL_DEV_NUM];
@@ -208,7 +209,24 @@ static struct attribute *ideapad_attributes[] = {
NULL
};
+static mode_t ideapad_is_visible(struct kobject *kobj,
+ struct attribute *attr,
+ int idx)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct ideapad_private *priv = dev_get_drvdata(dev);
+ bool supported;
+
+ if (attr == &dev_attr_camera_power.attr)
+ supported = test_bit(CFG_CAMERA_BIT, &(priv->cfg));
+ else
+ supported = true;
+
+ return supported ? attr->mode : 0;
+}
+
static struct attribute_group ideapad_attribute_group = {
+ .is_visible = ideapad_is_visible,
.attrs = ideapad_attributes
};
--
1.7.4.1
When acpi_backlight=vendor in cmdline or no backlight support in acpi video
device, ideapad-laptop will register backlight device and control brightness
and backlight power via the command in VPC2004.
Signed-off-by: Ike Panhc <[email protected]>
---
drivers/platform/x86/ideapad-laptop.c | 129 ++++++++++++++++++++++++++++++---
1 files changed, 119 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 678bbf7..018c457 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -32,6 +32,8 @@
#include <linux/platform_device.h>
#include <linux/input.h>
#include <linux/input/sparse-keymap.h>
+#include <linux/backlight.h>
+#include <linux/fb.h>
#define IDEAPAD_RFKILL_DEV_NUM (3)
@@ -44,6 +46,7 @@ struct ideapad_private {
struct rfkill *rfk[IDEAPAD_RFKILL_DEV_NUM];
struct platform_device *platform_device;
struct input_dev *inputdev;
+ struct backlight_device *blightdev;
unsigned long cfg;
};
@@ -257,9 +260,8 @@ static struct rfkill_ops ideapad_rfk_ops = {
.set_block = ideapad_rfk_set,
};
-static void ideapad_sync_rfk_state(struct acpi_device *adevice)
+static void ideapad_sync_rfk_state(struct ideapad_private *priv)
{
- struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
unsigned long hw_blocked;
int i;
@@ -309,8 +311,7 @@ static int __devinit ideapad_register_rfkill(struct acpi_device *adevice,
return 0;
}
-static void __devexit ideapad_unregister_rfkill(struct acpi_device *adevice,
- int dev)
+static void ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)
{
struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
@@ -418,6 +419,97 @@ static void ideapad_input_report(struct ideapad_private *priv,
}
/*
+ * backlight
+ */
+static int ideapad_backlight_get_brightness(struct backlight_device *blightdev)
+{
+ unsigned long now;
+
+ if (read_ec_data(ideapad_handle, 0x12, &now))
+ return -EAGAIN;
+ return now;
+}
+
+static int ideapad_backlight_update_status(struct backlight_device *blightdev)
+{
+ if (write_ec_cmd(ideapad_handle, 0x13, blightdev->props.brightness))
+ return -EAGAIN;
+ if (write_ec_cmd(ideapad_handle, 0x33,
+ blightdev->props.power == FB_BLANK_POWERDOWN ? 0 : 1))
+ return -EAGAIN;
+
+ return 0;
+}
+
+static const struct backlight_ops ideapad_backlight_ops = {
+ .get_brightness = ideapad_backlight_get_brightness,
+ .update_status = ideapad_backlight_update_status,
+};
+
+static int ideapad_backlight_init(struct ideapad_private *priv)
+{
+ struct backlight_device *blightdev;
+ struct backlight_properties props;
+ unsigned long max, now, power;
+
+ if (read_ec_data(ideapad_handle, 0x11, &max))
+ return -EAGAIN;
+ if (read_ec_data(ideapad_handle, 0x12, &now))
+ return -EAGAIN;
+ if (read_ec_data(ideapad_handle, 0x18, &power))
+ return -EAGAIN;
+
+ memset(&props, 0, sizeof(struct backlight_properties));
+ props.max_brightness = max;
+ blightdev = backlight_device_register("ideapad",
+ &priv->platform_device->dev,
+ priv,
+ &ideapad_backlight_ops,
+ &props);
+ if (IS_ERR(blightdev)) {
+ pr_err("Could not register backlight device\n");
+ return PTR_ERR(blightdev);
+ }
+
+ priv->blightdev = blightdev;
+ blightdev->props.brightness = now;
+ blightdev->props.power = power ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
+ backlight_update_status(blightdev);
+
+ return 0;
+}
+
+static void ideapad_backlight_exit(struct ideapad_private *priv)
+{
+ if (priv->blightdev)
+ backlight_device_unregister(priv->blightdev);
+ priv->blightdev = NULL;
+}
+
+static void ideapad_backlight_notify_power(struct ideapad_private *priv)
+{
+ unsigned long power;
+ struct backlight_device *blightdev = priv->blightdev;
+
+ if (read_ec_data(ideapad_handle, 0x18, &power))
+ return;
+ blightdev->props.power = power ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
+}
+
+static void ideapad_backlight_notify_brightness(struct ideapad_private *priv)
+{
+ unsigned long now;
+
+ /* if we control brightness via acpi video driver */
+ if (priv->blightdev == NULL) {
+ read_ec_data(ideapad_handle, 0x12, &now);
+ return;
+ }
+
+ backlight_force_update(priv->blightdev, BACKLIGHT_UPDATE_HOTKEY);
+}
+
+/*
* module init/exit
*/
static const struct acpi_device_id ideapad_device_ids[] = {
@@ -456,10 +548,19 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
else
priv->rfk[i] = NULL;
}
- ideapad_sync_rfk_state(adevice);
+ ideapad_sync_rfk_state(priv);
+
+ if (!acpi_video_backlight_support()) {
+ ret = ideapad_backlight_init(priv);
+ if (ret && ret != -ENODEV)
+ goto backlight_failed;
+ }
return 0;
+backlight_failed:
+ for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
+ ideapad_unregister_rfkill(adevice, i);
input_failed:
ideapad_platform_exit(priv);
platform_failed:
@@ -472,6 +573,7 @@ static int __devexit ideapad_acpi_remove(struct acpi_device *adevice, int type)
struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
int i;
+ ideapad_backlight_exit(priv);
for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
ideapad_unregister_rfkill(adevice, i);
ideapad_input_exit(priv);
@@ -496,12 +598,19 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
vpc1 = (vpc2 << 8) | vpc1;
for (vpc_bit = 0; vpc_bit < 16; vpc_bit++) {
if (test_bit(vpc_bit, &vpc1)) {
- if (vpc_bit == 9)
- ideapad_sync_rfk_state(adevice);
- else if (vpc_bit == 4)
- read_ec_data(handle, 0x12, &vpc2);
- else
+ switch (vpc_bit) {
+ case 9:
+ ideapad_sync_rfk_state(priv);
+ break;
+ case 4:
+ ideapad_backlight_notify_brightness(priv);
+ break;
+ case 2:
+ ideapad_backlight_notify_power(priv);
+ break;
+ default:
ideapad_input_report(priv, vpc_bit);
+ }
}
}
}
--
1.7.4.1
On Wed, Jun 22, 2011 at 10:07 AM, Ike Panhc <[email protected]> wrote:
> When acpi_backlight=vendor in cmdline or no backlight support in acpi video
> device, ideapad-laptop will register backlight device and control brightness
> and backlight power via the command in VPC2004.
>
> Signed-off-by: Ike Panhc <[email protected]>
> ---
> drivers/platform/x86/ideapad-laptop.c | 129 ++++++++++++++++++++++++++++++---
> 1 files changed, 119 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 678bbf7..018c457 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -32,6 +32,8 @@
> #include <linux/platform_device.h>
> #include <linux/input.h>
> #include <linux/input/sparse-keymap.h>
> +#include <linux/backlight.h>
> +#include <linux/fb.h>
>
> #define IDEAPAD_RFKILL_DEV_NUM (3)
>
> @@ -44,6 +46,7 @@ struct ideapad_private {
> struct rfkill *rfk[IDEAPAD_RFKILL_DEV_NUM];
> struct platform_device *platform_device;
> struct input_dev *inputdev;
> + struct backlight_device *blightdev;
> unsigned long cfg;
> };
>
> @@ -257,9 +260,8 @@ static struct rfkill_ops ideapad_rfk_ops = {
> .set_block = ideapad_rfk_set,
> };
>
> -static void ideapad_sync_rfk_state(struct acpi_device *adevice)
> +static void ideapad_sync_rfk_state(struct ideapad_private *priv)
> {
> - struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
> unsigned long hw_blocked;
> int i;
>
> @@ -309,8 +311,7 @@ static int __devinit ideapad_register_rfkill(struct acpi_device *adevice,
> return 0;
> }
>
> -static void __devexit ideapad_unregister_rfkill(struct acpi_device *adevice,
> - int dev)
> +static void ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)
> {
> struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
>
> @@ -418,6 +419,97 @@ static void ideapad_input_report(struct ideapad_private *priv,
> }
>
> /*
> + * backlight
> + */
> +static int ideapad_backlight_get_brightness(struct backlight_device *blightdev)
> +{
> + unsigned long now;
> +
> + if (read_ec_data(ideapad_handle, 0x12, &now))
> + return -EAGAIN;
> + return now;
> +}
> +
> +static int ideapad_backlight_update_status(struct backlight_device *blightdev)
> +{
> + if (write_ec_cmd(ideapad_handle, 0x13, blightdev->props.brightness))
> + return -EAGAIN;
> + if (write_ec_cmd(ideapad_handle, 0x33,
> + blightdev->props.power == FB_BLANK_POWERDOWN ? 0 : 1))
> + return -EAGAIN;
> +
> + return 0;
> +}
I don't think -EAGAIN is a good idea here ... Why not -EIO ?
> +static const struct backlight_ops ideapad_backlight_ops = {
> + .get_brightness = ideapad_backlight_get_brightness,
> + .update_status = ideapad_backlight_update_status,
> +};
> +
> +static int ideapad_backlight_init(struct ideapad_private *priv)
> +{
> + struct backlight_device *blightdev;
> + struct backlight_properties props;
> + unsigned long max, now, power;
> +
> + if (read_ec_data(ideapad_handle, 0x11, &max))
> + return -EAGAIN;
> + if (read_ec_data(ideapad_handle, 0x12, &now))
> + return -EAGAIN;
> + if (read_ec_data(ideapad_handle, 0x18, &power))
> + return -EAGAIN;
Same
> + memset(&props, 0, sizeof(struct backlight_properties));
> + props.max_brightness = max;
You forgot to set backlight type, "props.type = BACKLIGHT_PLATFORM;"
> + blightdev = backlight_device_register("ideapad",
> + &priv->platform_device->dev,
> + priv,
> + &ideapad_backlight_ops,
> + &props);
> + if (IS_ERR(blightdev)) {
> + pr_err("Could not register backlight device\n");
> + return PTR_ERR(blightdev);
> + }
> +
> + priv->blightdev = blightdev;
> + blightdev->props.brightness = now;
> + blightdev->props.power = power ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
> + backlight_update_status(blightdev);
> +
> + return 0;
> +}
> +
> +static void ideapad_backlight_exit(struct ideapad_private *priv)
> +{
> + if (priv->blightdev)
> + backlight_device_unregister(priv->blightdev);
> + priv->blightdev = NULL;
> +}
> +
> +static void ideapad_backlight_notify_power(struct ideapad_private *priv)
> +{
> + unsigned long power;
> + struct backlight_device *blightdev = priv->blightdev;
> +
> + if (read_ec_data(ideapad_handle, 0x18, &power))
> + return;
> + blightdev->props.power = power ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
> +}
> +
> +static void ideapad_backlight_notify_brightness(struct ideapad_private *priv)
> +{
> + unsigned long now;
> +
> + /* if we control brightness via acpi video driver */
> + if (priv->blightdev == NULL) {
> + read_ec_data(ideapad_handle, 0x12, &now);
> + return;
> + }
> +
> + backlight_force_update(priv->blightdev, BACKLIGHT_UPDATE_HOTKEY);
> +}
> +
> +/*
> * module init/exit
> */
> static const struct acpi_device_id ideapad_device_ids[] = {
> @@ -456,10 +548,19 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
> else
> priv->rfk[i] = NULL;
> }
> - ideapad_sync_rfk_state(adevice);
> + ideapad_sync_rfk_state(priv);
> +
> + if (!acpi_video_backlight_support()) {
> + ret = ideapad_backlight_init(priv);
> + if (ret && ret != -ENODEV)
> + goto backlight_failed;
> + }
>
> return 0;
>
> +backlight_failed:
> + for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
> + ideapad_unregister_rfkill(adevice, i);
> input_failed:
> ideapad_platform_exit(priv);
> platform_failed:
> @@ -472,6 +573,7 @@ static int __devexit ideapad_acpi_remove(struct acpi_device *adevice, int type)
> struct ideapad_private *priv = dev_get_drvdata(&adevice->dev);
> int i;
>
> + ideapad_backlight_exit(priv);
> for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
> ideapad_unregister_rfkill(adevice, i);
> ideapad_input_exit(priv);
> @@ -496,12 +598,19 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
> vpc1 = (vpc2 << 8) | vpc1;
> for (vpc_bit = 0; vpc_bit < 16; vpc_bit++) {
> if (test_bit(vpc_bit, &vpc1)) {
> - if (vpc_bit == 9)
> - ideapad_sync_rfk_state(adevice);
> - else if (vpc_bit == 4)
> - read_ec_data(handle, 0x12, &vpc2);
> - else
> + switch (vpc_bit) {
> + case 9:
> + ideapad_sync_rfk_state(priv);
> + break;
> + case 4:
> + ideapad_backlight_notify_brightness(priv);
> + break;
> + case 2:
> + ideapad_backlight_notify_power(priv);
> + break;
> + default:
> ideapad_input_report(priv, vpc_bit);
> + }
> }
> }
> }
> --
> 1.7.4.1
>
>
Otherwise, looks ok
--
Corentin Chary
http://xf.iksaif.net
On Wed, Jun 22, 2011 at 04:05:43PM +0800, Ike Panhc wrote:
> static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
> {
> - int ret, i, cfg;
> + int ret, i;
> + unsigned long cfg;
You change this to an unsigned long, but you cast it to an int in
several places. Is that just to avoid casting to a long here?
> - if (test_bit(ideapad_rfk_data[i].cfgbit, (unsigned long *)&cfg))
> + if (test_bit(ideapad_rfk_data[i].cfgbit, &cfg))
I think it seems neater the other way around, but don't have terribly
strong opinions.
--
Matthew Garrett | [email protected]
On Wed, Jun 22, 2011 at 04:07:38PM +0800, Ike Panhc wrote:
> -static void ideapad_sync_rfk_state(struct acpi_device *adevice)
> +static void ideapad_sync_rfk_state(struct ideapad_private *priv)
> {
This should be a separate patch.
> -static void __devexit ideapad_unregister_rfkill(struct acpi_device *adevice,
> - int dev)
> +static void ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)
This also seems unrelated.
> +static int ideapad_backlight_get_brightness(struct backlight_device *blightdev)
> +{
> + unsigned long now;
> +
> + if (read_ec_data(ideapad_handle, 0x12, &now))
> + return -EAGAIN;
Description says you're using commands on the VPC2004 device, but it
looks like you're just poking the embedded controller? Are you sure the
EC offsets are stable?
--
Matthew Garrett | [email protected]
On 06/23/2011 03:40 AM, Matthew Garrett wrote:
>> +static int ideapad_backlight_get_brightness(struct backlight_device *blightdev)
>> +{
>> + unsigned long now;
>> +
>> + if (read_ec_data(ideapad_handle, 0x12, &now))
>> + return -EAGAIN;
>
> Description says you're using commands on the VPC2004 device, but it
> looks like you're just poking the embedded controller? Are you sure the
> EC offsets are stable?
>
Yes, they are stable, though on some machine, some functions are not fully implement.
For example, on s10-3, the brightness control is no function but bl_power works fine.
on B550, the brightness control works fine but only get acpi notify when bl_power
turning on.
function read_ec_data will access VPCW and VPCR methods that exist in every VPC2004
devices. In this case, VPCW will write 0x12 to VCMD and then VPCR read the current
brightness value from VDAT.
So far as I have seen, there are two design for VDAT and VCMD. One way (eg, ideapad
B550) is they are EC registers and embedded controller will do all things. The other
way (eg, ideapad Y530) is they are variables in DSDT, and CPU/DSDT will correct
informations into VDAT.
Both of the design are ok for ideapad-laptop because we only touch VPCW/VPCR methods
and let DSDT or EC do the rest.
I put DSDTs all I have at http://people.ubuntu.com/~ikepanhc/DSDTs
On 06/23/2011 03:40 AM, Matthew Garrett wrote:
>> -static void __devexit ideapad_unregister_rfkill(struct acpi_device *adevice,
>> - int dev)
>> +static void ideapad_unregister_rfkill(struct acpi_device *adevice, int dev)
>
> This also seems unrelated.
>
This is because ideapad_unregister_rfkill is called in ideapad_acpi_add
which has markup __devinit, so I get warning when building.
@@ -456,10 +548,19 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
else
priv->rfk[i] = NULL;
}
- ideapad_sync_rfk_state(adevice);
+ ideapad_sync_rfk_state(priv);
+
+ if (!acpi_video_backlight_support()) {
+ ret = ideapad_backlight_init(priv);
+ if (ret && ret != -ENODEV)
+ goto backlight_failed;
+ }
return 0;
+backlight_failed:
+ for (i = 0; i < IDEAPAD_RFKILL_DEV_NUM; i++)
+ ideapad_unregister_rfkill(adevice, i);
input_failed:
ideapad_platform_exit(priv);
platform_failed:
On 06/23/2011 03:35 AM, Matthew Garrett wrote:
> On Wed, Jun 22, 2011 at 04:05:43PM +0800, Ike Panhc wrote:
>> static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
>> {
>> - int ret, i, cfg;
>> + int ret, i;
>> + unsigned long cfg;
>
> You change this to an unsigned long, but you cast it to an int in
> several places. Is that just to avoid casting to a long here?
>
>> - if (test_bit(ideapad_rfk_data[i].cfgbit, (unsigned long *)&cfg))
>> + if (test_bit(ideapad_rfk_data[i].cfgbit, &cfg))
>
> I think it seems neater the other way around, but don't have terribly
> strong opinions.
>
I read again how cfg is used. They are 2 test_bit, one sprintf and
read_method_int.
test_bit is the major reason of changing from int to unsigned long. I will
change the format string in sprintf from %X to %lX. After the change there
will be only one cast left.