[PATCH 0/3 v2] ideapad: add backlight driver and changes of sysfs nodes

[PATCH 0/3 v2] ideapad: add backlight driver and changes of sysfs nodes

[Ike Panhc]

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

[PATCH 1/3] ideapad: define cfg bits and create sysfs node for cfg

[Ike Panhc]

Create /sys/devices/platform/ideapad/cfg for showing cfg value.

Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 .../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 <ike.pan@canonical.com>"
 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 <ike.pan@canonical.com>"
+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

Re: [PATCH 1/3] ideapad: define cfg bits and create sysfs node for cfg

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

Re: [PATCH 1/3] ideapad: define cfg bits and create sysfs node for cfg

[Ike Panhc]

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.

[PATCH 2/3] ideapad: let camera_power node invisiable if no camera

[Ike Panhc]

Signed-off-by: Ike Panhc <ike.pan@canonical.com>
---
 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

[PATCH 3/3] ideapad: add backlight driver

[Ike Panhc]

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 <ike.pan@canonical.com>
---
 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

Re: [PATCH 3/3] ideapad: add backlight driver

[Corentin Chary]

On Wed, Jun 22, 2011 at 10:07 AM, Ike Panhc <ike.pan@canonical.com> 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 <ike.pan@canonical.com>
> ---
>  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

Re: [PATCH 3/3] ideapad: add backlight driver

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

Re: [PATCH 3/3] ideapad: add backlight driver

[Ike Panhc]

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  

Re: [PATCH 3/3] ideapad: add backlight driver

[Ike Panhc]

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:

[PATCH 0/3 v3] ideapad: add backlight driver and changes of sysfs nodes

[Ike Panhc]

Changes after last post:
 * In show_ideapad_cfg, Using %lX to avoid one cast.
 * When fail from reading EC, return -EIO
 * Not to change the parameter of ideapad_sync_rfk_state
 * Add backlight type

Patches are 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              |  192 +++++++++++++++++---
 2 files changed, 187 insertions(+), 22 deletions(-)

-- 
1.7.4.1

[PATCH 3/3] ideapad: add backlight driver

[Ike Panhc]

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 <ike.pan@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |  123 +++++++++++++++++++++++++++++++--
 1 files changed, 117 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 1612abd..8811c68 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;
 };
 
@@ -309,8 +312,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 +420,98 @@ 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 -EIO;
+	return now;
+}
+
+static int ideapad_backlight_update_status(struct backlight_device *blightdev)
+{
+	if (write_ec_cmd(ideapad_handle, 0x13, blightdev->props.brightness))
+		return -EIO;
+	if (write_ec_cmd(ideapad_handle, 0x33,
+			 blightdev->props.power == FB_BLANK_POWERDOWN ? 0 : 1))
+		return -EIO;
+
+	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 -EIO;
+	if (read_ec_data(ideapad_handle, 0x12, &now))
+		return -EIO;
+	if (read_ec_data(ideapad_handle, 0x18, &power))
+		return -EIO;
+
+	memset(&props, 0, sizeof(struct backlight_properties));
+	props.max_brightness = max;
+	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[] = {
@@ -458,8 +552,17 @@ static int __devinit ideapad_acpi_add(struct acpi_device *adevice)
 	}
 	ideapad_sync_rfk_state(adevice);
 
+	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 +575,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 +600,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)
+			switch (vpc_bit) {
+			case 9:
 				ideapad_sync_rfk_state(adevice);
-			else if (vpc_bit == 4)
-				read_ec_data(handle, 0x12, &vpc2);
-			else
+				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