[RFC] [PATCH] panasonic-laptop.c: add support for CD power management

[RFC] [PATCH] panasonic-laptop.c: add support for CD power management

[Martin Lucina]

Hello,

attached is a patch to the panasonic-laptop driver to enable power
management (i.e. on/off) of the built-in optical drive.  It creates a
sysfs interface /sys/devices/platform/panasonic/cdpower which can be
either queried or written to.  Writing "0" enables the device, "1"
disables it.

This code is based on reading the existing code[1][2],
examining the DSDT on my CF-W4 system and experimentation.

The presence of the CF-Y series in the DMI table is based on the
assumption that this method works on recent CF-Yx models.

I have a few questions I'd like to clear up before actually submitting
this for inclusion:

1) Is a platform device the right way to do this?

2) Rather than checking the DMI system vendor and product name would it
be a better approach to simply check for the presence of the \_SB.FBAY()
and \_SB.STAT() ACPI methods and enable the platform device on any
(Panasonic) system where these are present?  This would seem like a
more future-proof approach.

3) Please advise on the correct way to propagate an error result from
the functions that call the ACPI methods {get,set}optd_power_state() to
userspace in the sysfs interface.  I couldn't find a straightforward
example anywhere.

Thanks!

-mato

[1] Referenced here http://www.da-cha.jp/letsnote as "A patch to handle
CD on/off button(need to merge)" (now a dead link)
[2] http://www.netlab.is.tsukuba.ac.jp/~yokota/izumi/panasonic_acpi/,
see the 20070907 version ("includes small hack for CD-ROM drive")

---

--- linux-2.6.28/drivers/misc/panasonic-laptop.c.orig	2009-01-13 17:01:53.327106958 +0100
+++ linux-2.6.28/drivers/misc/panasonic-laptop.c	2009-01-13 17:04:04.911106197 +0100
@@ -24,6 +24,8 @@
  *---------------------------------------------------------------------------
  *
  * ChangeLog:
+ *	Jan.13, 2009    Martin Lucina <mato@kotelna.sk>
+ *		-v0.96  add support for optical drive power in Y and W series
  *	Sep.23, 2008	Harald Welte <laforge@gnumonks.org>
  *		-v0.95	rename driver from drivers/acpi/pcc_acpi.c to
  *			drivers/misc/panasonic-laptop.c
@@ -127,6 +129,8 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/input.h>
+#include <linux/platform_device.h>
+#include <linux/dmi.h>
 
 
 #ifndef ACPI_HOTKEY_COMPONENT
@@ -219,6 +223,7 @@ struct pcc_acpi {
 	struct acpi_device	*device;
 	struct input_dev	*input_dev;
 	struct backlight_device	*backlight;
+	struct platform_device  *platform;
 	int			keymap[KEYMAP_SIZE];
 };
 
@@ -226,6 +231,27 @@ struct pcc_keyinput {
 	struct acpi_hotkey      *hotkey;
 };
 
+/* known models with optical drive */
+static struct dmi_system_id pcc_platform_dmi_table[] = {
+	{
+		.ident = "Panasonic Toughbook W Series",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, 
+				"Matsushita Electric Industrial Co.,Ltd."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "CF-W"),
+		},
+	},
+	{
+		 .ident = "Panasonic Toughbook Y Series",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR,
+				"Matsushita Electric Industrial Co.,Ltd."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "CF-Y"),
+		},
+	},
+	{ }
+};
+
 /* method access functions */
 static int acpi_pcc_write_sset(struct pcc_acpi *pcc, int func, int val)
 {
@@ -360,6 +386,63 @@ static struct backlight_ops pcc_backligh
 	.update_status	= bl_set_status,
 };
 
+/* get optical driver power state */
+
+static int get_optd_power_state(void)
+{
+	acpi_status status;
+	unsigned long long state;
+	int result;
+	
+	status = acpi_evaluate_integer(NULL, "\\_SB.STAT", NULL, &state);
+	if (ACPI_FAILURE(status)) {
+		result = -EIO;
+		goto out;
+	}
+	switch (state) {
+	case 0: /* power off */
+		result = 1;
+		break;
+	case 0x0f: /* power on */
+		result = 0;
+		break;
+	default:
+		result = -EIO;
+		break;
+	}
+out:
+	return result;
+}
+
+/* set optical drive power state */
+
+static int set_optd_power_state(int new_state)
+{
+	int state, result;
+	acpi_status status;
+
+	state = get_optd_power_state();
+	if (new_state == state)
+		return 0;
+
+	result = 0;
+	switch (new_state) {
+	case 0: /* power on */
+		status = acpi_evaluate_object (NULL, "\\_SB.FBAY", NULL, NULL);
+		if (ACPI_FAILURE(status))
+			result = -EIO;
+		break;
+	case 1: /* power off */
+		status = acpi_evaluate_object (NULL, "\\_SB.CDDI", NULL, NULL);
+		if (ACPI_FAILURE(status))
+			result = -EIO;
+		break;
+	default:
+		result = -EINVAL;
+		break;
+	}
+	return result;
+}
 
 /* sysfs user interface functions */
 
@@ -427,10 +510,29 @@ static ssize_t set_sticky(struct device 
 	return count;
 }
 
+static ssize_t show_cdpower(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", get_optd_power_state());
+}
+
+static ssize_t set_cdpower(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	int value;
+
+	if (count) {
+		value = simple_strtoul(buf, NULL, 10);
+		set_optd_power_state (value);
+	}
+	return count;
+}
+
 static DEVICE_ATTR(numbatt, S_IRUGO, show_numbatt, NULL);
 static DEVICE_ATTR(lcdtype, S_IRUGO, show_lcdtype, NULL);
 static DEVICE_ATTR(mute, S_IRUGO, show_mute, NULL);
 static DEVICE_ATTR(sticky_key, S_IRUGO | S_IWUSR, show_sticky, set_sticky);
+static DEVICE_ATTR(cdpower, S_IRUGO | S_IWUSR, show_cdpower, set_cdpower);
 
 static struct attribute *pcc_sysfs_entries[] = {
 	&dev_attr_numbatt.attr,
@@ -691,8 +793,26 @@ static int acpi_pcc_hotkey_add(struct ac
 	if (result)
 		goto out_backlight;
 
+	/* platform device initialization */
+	if (!dmi_check_system(pcc_platform_dmi_table)) {
+		pcc->platform = NULL;
+		goto out_no_optd;
+	}
+	pcc->platform = platform_device_register_simple("panasonic", -1,
+		NULL, 0);
+	if (IS_ERR(pcc->platform)) {
+		result = PTR_ERR(pcc->platform);
+		goto out_backlight;
+	}
+	result = device_create_file(&pcc->platform->dev, &dev_attr_cdpower);
+	if (result)
+		goto out_platform;
+
+out_no_optd:
 	return 0;
 
+out_platform:
+	platform_device_unregister(pcc->platform);
 out_backlight:
 	backlight_device_unregister(pcc->backlight);
 out_notify:
@@ -738,6 +858,11 @@ static int acpi_pcc_hotkey_remove(struct
 	if (!device || !pcc)
 		return -EINVAL;
 
+	if (pcc->platform) {
+		device_remove_file(&pcc->platform->dev, &dev_attr_cdpower);
+		platform_device_unregister(pcc->platform);
+	}
+
 	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
 
 	backlight_device_unregister(pcc->backlight);

Re: [RFC] [PATCH] panasonic-laptop.c: add support for CD power management

[Karthik Gopalakrishnan]

Hi Martin.

To an end user, writing "0" to enable the device & "1" to disable it
sounds counter-intuitive, especially when the sysfs entry is called
"cdpower". Is there a reason to not flip that?

Regards,
Karthik

On Tue, Jan 13, 2009 at 11:32 AM, Martin Lucina <mato@kotelna.sk> wrote:
> Hello,
>
> attached is a patch to the panasonic-laptop driver to enable power
> management (i.e. on/off) of the built-in optical drive.  It creates a
> sysfs interface /sys/devices/platform/panasonic/cdpower which can be
> either queried or written to.  Writing "0" enables the device, "1"
> disables it.
>
> This code is based on reading the existing code[1][2],
> examining the DSDT on my CF-W4 system and experimentation.
>
> The presence of the CF-Y series in the DMI table is based on the
> assumption that this method works on recent CF-Yx models.
>
> I have a few questions I'd like to clear up before actually submitting
> this for inclusion:
>
> 1) Is a platform device the right way to do this?
>
> 2) Rather than checking the DMI system vendor and product name would it
> be a better approach to simply check for the presence of the \_SB.FBAY()
> and \_SB.STAT() ACPI methods and enable the platform device on any
> (Panasonic) system where these are present?  This would seem like a
> more future-proof approach.
>
> 3) Please advise on the correct way to propagate an error result from
> the functions that call the ACPI methods {get,set}optd_power_state() to
> userspace in the sysfs interface.  I couldn't find a straightforward
> example anywhere.
>
> Thanks!
>
> -mato
>
> [1] Referenced here http://www.da-cha.jp/letsnote as "A patch to handle
> CD on/off button(need to merge)" (now a dead link)
> [2] http://www.netlab.is.tsukuba.ac.jp/~yokota/izumi/panasonic_acpi/,
> see the 20070907 version ("includes small hack for CD-ROM drive")
>
> ---
>
> --- linux-2.6.28/drivers/misc/panasonic-laptop.c.orig   2009-01-13 17:01:53.327106958 +0100
> +++ linux-2.6.28/drivers/misc/panasonic-laptop.c        2009-01-13 17:04:04.911106197 +0100
> @@ -24,6 +24,8 @@
>  *---------------------------------------------------------------------------
>  *
>  * ChangeLog:
> + *     Jan.13, 2009    Martin Lucina <mato@kotelna.sk>
> + *             -v0.96  add support for optical drive power in Y and W series
>  *     Sep.23, 2008    Harald Welte <laforge@gnumonks.org>
>  *             -v0.95  rename driver from drivers/acpi/pcc_acpi.c to
>  *                     drivers/misc/panasonic-laptop.c
> @@ -127,6 +129,8 @@
>  #include <acpi/acpi_bus.h>
>  #include <acpi/acpi_drivers.h>
>  #include <linux/input.h>
> +#include <linux/platform_device.h>
> +#include <linux/dmi.h>
>
>
>  #ifndef ACPI_HOTKEY_COMPONENT
> @@ -219,6 +223,7 @@ struct pcc_acpi {
>        struct acpi_device      *device;
>        struct input_dev        *input_dev;
>        struct backlight_device *backlight;
> +       struct platform_device  *platform;
>        int                     keymap[KEYMAP_SIZE];
>  };
>
> @@ -226,6 +231,27 @@ struct pcc_keyinput {
>        struct acpi_hotkey      *hotkey;
>  };
>
> +/* known models with optical drive */
> +static struct dmi_system_id pcc_platform_dmi_table[] = {
> +       {
> +               .ident = "Panasonic Toughbook W Series",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR,
> +                               "Matsushita Electric Industrial Co.,Ltd."),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "CF-W"),
> +               },
> +       },
> +       {
> +                .ident = "Panasonic Toughbook Y Series",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR,
> +                               "Matsushita Electric Industrial Co.,Ltd."),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "CF-Y"),
> +               },
> +       },
> +       { }
> +};
> +
>  /* method access functions */
>  static int acpi_pcc_write_sset(struct pcc_acpi *pcc, int func, int val)
>  {
> @@ -360,6 +386,63 @@ static struct backlight_ops pcc_backligh
>        .update_status  = bl_set_status,
>  };
>
> +/* get optical driver power state */
> +
> +static int get_optd_power_state(void)
> +{
> +       acpi_status status;
> +       unsigned long long state;
> +       int result;
> +
> +       status = acpi_evaluate_integer(NULL, "\\_SB.STAT", NULL, &state);
> +       if (ACPI_FAILURE(status)) {
> +               result = -EIO;
> +               goto out;
> +       }
> +       switch (state) {
> +       case 0: /* power off */
> +               result = 1;
> +               break;
> +       case 0x0f: /* power on */
> +               result = 0;
> +               break;
> +       default:
> +               result = -EIO;
> +               break;
> +       }
> +out:
> +       return result;
> +}
> +
> +/* set optical drive power state */
> +
> +static int set_optd_power_state(int new_state)
> +{
> +       int state, result;
> +       acpi_status status;
> +
> +       state = get_optd_power_state();
> +       if (new_state == state)
> +               return 0;
> +
> +       result = 0;
> +       switch (new_state) {
> +       case 0: /* power on */
> +               status = acpi_evaluate_object (NULL, "\\_SB.FBAY", NULL, NULL);
> +               if (ACPI_FAILURE(status))
> +                       result = -EIO;
> +               break;
> +       case 1: /* power off */
> +               status = acpi_evaluate_object (NULL, "\\_SB.CDDI", NULL, NULL);
> +               if (ACPI_FAILURE(status))
> +                       result = -EIO;
> +               break;
> +       default:
> +               result = -EINVAL;
> +               break;
> +       }
> +       return result;
> +}
>
>  /* sysfs user interface functions */
>
> @@ -427,10 +510,29 @@ static ssize_t set_sticky(struct device
>        return count;
>  }
>
> +static ssize_t show_cdpower(struct device *dev, struct device_attribute *attr,
> +                           char *buf)
> +{
> +       return snprintf(buf, PAGE_SIZE, "%d\n", get_optd_power_state());
> +}
> +
> +static ssize_t set_cdpower(struct device *dev, struct device_attribute *attr,
> +                          const char *buf, size_t count)
> +{
> +       int value;
> +
> +       if (count) {
> +               value = simple_strtoul(buf, NULL, 10);
> +               set_optd_power_state (value);
> +       }
> +       return count;
> +}
> +
>  static DEVICE_ATTR(numbatt, S_IRUGO, show_numbatt, NULL);
>  static DEVICE_ATTR(lcdtype, S_IRUGO, show_lcdtype, NULL);
>  static DEVICE_ATTR(mute, S_IRUGO, show_mute, NULL);
>  static DEVICE_ATTR(sticky_key, S_IRUGO | S_IWUSR, show_sticky, set_sticky);
> +static DEVICE_ATTR(cdpower, S_IRUGO | S_IWUSR, show_cdpower, set_cdpower);
>
>  static struct attribute *pcc_sysfs_entries[] = {
>        &dev_attr_numbatt.attr,
> @@ -691,8 +793,26 @@ static int acpi_pcc_hotkey_add(struct ac
>        if (result)
>                goto out_backlight;
>
> +       /* platform device initialization */
> +       if (!dmi_check_system(pcc_platform_dmi_table)) {
> +               pcc->platform = NULL;
> +               goto out_no_optd;
> +       }
> +       pcc->platform = platform_device_register_simple("panasonic", -1,
> +               NULL, 0);
> +       if (IS_ERR(pcc->platform)) {
> +               result = PTR_ERR(pcc->platform);
> +               goto out_backlight;
> +       }
> +       result = device_create_file(&pcc->platform->dev, &dev_attr_cdpower);
> +       if (result)
> +               goto out_platform;
> +
> +out_no_optd:
>        return 0;
>
> +out_platform:
> +       platform_device_unregister(pcc->platform);
>  out_backlight:
>        backlight_device_unregister(pcc->backlight);
>  out_notify:
> @@ -738,6 +858,11 @@ static int acpi_pcc_hotkey_remove(struct
>        if (!device || !pcc)
>                return -EINVAL;
>
> +       if (pcc->platform) {
> +               device_remove_file(&pcc->platform->dev, &dev_attr_cdpower);
> +               platform_device_unregister(pcc->platform);
> +       }
> +
>        sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
>
>        backlight_device_unregister(pcc->backlight);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [RFC] [PATCH] panasonic-laptop.c: add support for CD power management

[Martin Lucina]

Karthik,

karthik.g.krishnan@gmail.com said:
> To an end user, writing "0" to enable the device & "1" to disable it
> sounds counter-intuitive, especially when the sysfs entry is called
> "cdpower". Is there a reason to not flip that?

/me goes off to look at how other people do it...  sony-laptop uses "1"
for on and "0" for off, so I guess you're right.  Not sure what I was
thinking there, I'll flip it in the final version of the patch.  Thanks!

Which brings me to a related question someone here may be able to
answer:  Which bit of userspace would be responsible for managing the
drive's power state?  To me the most intuitive automatic mode of
operation would be that the system powers off the drive after some time
iff there is no medium in the drive.

On these laptops there is no need to actually power the drive back up,
since it will do so automatically if a user attempts to insert a disc.

-mato

Re: [RFC] [PATCH] panasonic-laptop.c: add support for CD power management

[Harald Welte]

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

Hi Martin,

thanks for your patch, here some comments:

* please provide Signed-off-by line
* please invert the logic (as discussed earlier in the thread)
* please don't use the goto construct in pcc_hotkey_add(), since it
  makes it easy to introduce future bugs while adding more code,
  just check for the ODD drive presence and put the entire block
  in an 'if ()  { }' construct.
* I agree, checking for _SB.FBAY and _SB.STAT is probably the better
  solution.  I would accept both versions, though.
* Please run your patch through checkpatch.pl, I think there were some
  indentation errors in it (just spotted them with my eye, didn't run
  the script)

> 3) Please advise on the correct way to propagate an error result from
> the functions that call the ACPI methods {get,set}optd_power_state() to
> userspace in the sysfs interface.  I couldn't find a straightforward
> example anywhere.

I cannot help with that either, sorry.  I'm not the ACPI expert here, just
happening to have merged the panasonic driver ;)

Oh, and with regard to 'who in userspace is responsible': This is actually a
good question.  I've had many of this kind of cases in embedded development,
where you had to explicitly have to enable the power to a certain peripheral
before using.  I personally believe this doesn't belong into userspace, and
the kernel should provide hooks for it, i.e. issue some kind of
event/notifier/... to call any power-up hanlers before using, and then doing
the inverse after use.

Regards,
-- 
- Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH] panasonic-laptop.c: add support for optical drive power control

[Martin Lucina]

Add support for power control of the built in optical drive on models
with the required ACPI methods present.  Tested on Panasonic CF-W4.

Creates an interface in /sys/devices/platform/panasonic/cdpower, to
which you can write "1" to switch the drive on, "0" to switch it off
or read from to query the current state.

Signed-off-by: Martin Lucina <mato@kotelna.sk>

---

Harald, this should address all the comments in this thread.  I've
removed the DMI table and enable the code if _SB.{STAT,FBAY,CDDI} are
all present.

I've not figured out how to return an actual error from the sysfs
show/store functions so I've at least added ACPI_DEBUG_PRINT to print an
error if the relevant methods fail.

-mato

--- linux-2.6.28/drivers/misc/panasonic-laptop.c.orig	2009-01-14 19:10:33.626598152 +0100
+++ linux-2.6.28/drivers/misc/panasonic-laptop.c	2009-01-14 19:22:06.500598198 +0100
@@ -24,6 +24,9 @@
  *---------------------------------------------------------------------------
  *
  * ChangeLog:
+ *	Jan.14, 2009    Martin Lucina <mato@kotelna.sk>
+ *		-v0.96  add support for optical drive power control
+ *			via /sys/devices/platform/panasonic/cdpower
  *	Sep.23, 2008	Harald Welte <laforge@gnumonks.org>
  *		-v0.95	rename driver from drivers/acpi/pcc_acpi.c to
  *			drivers/misc/panasonic-laptop.c
@@ -127,6 +130,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <linux/input.h>
+#include <linux/platform_device.h>
 
 
 #ifndef ACPI_HOTKEY_COMPONENT
@@ -219,6 +223,7 @@ struct pcc_acpi {
 	struct acpi_device	*device;
 	struct input_dev	*input_dev;
 	struct backlight_device	*backlight;
+	struct platform_device  *platform;
 	int			keymap[KEYMAP_SIZE];
 };
 
@@ -360,6 +365,96 @@ static struct backlight_ops pcc_backligh
 	.update_status	= bl_set_status,
 };
 
+/* returns ACPI_SUCCESS if methods to control optical drive are present */
+
+static acpi_status check_optd_present(void)
+{
+	acpi_status status = AE_OK;
+	acpi_handle handle;
+
+	status = acpi_get_handle(NULL, "\\_SB.STAT", &handle);
+	if (ACPI_FAILURE(status))
+		goto out;
+	status = acpi_get_handle(NULL, "\\_SB.FBAY", &handle);
+	if (ACPI_FAILURE(status))
+		goto out;
+	status = acpi_get_handle(NULL, "\\_SB.CDDI", &handle);
+	if (ACPI_FAILURE(status))
+		goto out;
+
+out:
+	return status;
+}
+
+/* get optical driver power state */
+
+static int get_optd_power_state(void)
+{
+	acpi_status status;
+	unsigned long long state;
+	int result;
+
+	status = acpi_evaluate_integer(NULL, "\\_SB.STAT", NULL, &state);
+	if (ACPI_FAILURE(status)) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+				  "evaluation error _SB.STAT\n"));
+		result = -EIO;
+		goto out;
+	}
+	switch (state) {
+	case 0: /* power off */
+		result = 0;
+		break;
+	case 0x0f: /* power on */
+		result = 1;
+		break;
+	default:
+		result = -EIO;
+		break;
+	}
+
+out:
+	return result;
+}
+
+/* set optical drive power state */
+
+static int set_optd_power_state(int new_state)
+{
+	int result;
+	acpi_status status;
+
+	result = get_optd_power_state();
+	if (result < 0)
+		goto out;
+	if (new_state == result)
+		goto out;
+
+	switch (new_state) {
+	case 0: /* power off */
+		status = acpi_evaluate_object(NULL, "\\_SB.CDDI", NULL, NULL);
+		if (ACPI_FAILURE(status)) {
+			ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+					  "evaluation error _SB.CDDI\n"));
+			result = -EIO;
+		}
+		break;
+	case 1: /* power on */
+		status = acpi_evaluate_object(NULL, "\\_SB.FBAY", NULL, NULL);
+		if (ACPI_FAILURE(status)) {
+			ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+					  "evaluation error _SB.FBAY\n"));
+			result = -EIO;
+		}
+		break;
+	default:
+		result = -EINVAL;
+		break;
+	}
+
+out:
+	return result;
+}
 
 /* sysfs user interface functions */
 
@@ -427,10 +522,29 @@ static ssize_t set_sticky(struct device 
 	return count;
 }
 
+static ssize_t show_cdpower(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", get_optd_power_state());
+}
+
+static ssize_t set_cdpower(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	int value;
+
+	if (count) {
+		value = simple_strtoul(buf, NULL, 10);
+		set_optd_power_state(value);
+	}
+	return count;
+}
+
 static DEVICE_ATTR(numbatt, S_IRUGO, show_numbatt, NULL);
 static DEVICE_ATTR(lcdtype, S_IRUGO, show_lcdtype, NULL);
 static DEVICE_ATTR(mute, S_IRUGO, show_mute, NULL);
 static DEVICE_ATTR(sticky_key, S_IRUGO | S_IWUSR, show_sticky, set_sticky);
+static DEVICE_ATTR(cdpower, S_IRUGO | S_IWUSR, show_cdpower, set_cdpower);
 
 static struct attribute *pcc_sysfs_entries[] = {
 	&dev_attr_numbatt.attr,
@@ -691,8 +805,26 @@ static int acpi_pcc_hotkey_add(struct ac
 	if (result)
 		goto out_backlight;
 
+	/* optical drive initialization */
+	if (ACPI_SUCCESS(check_optd_present())) {
+		pcc->platform = platform_device_register_simple("panasonic",
+			-1, NULL, 0);
+		if (IS_ERR(pcc->platform)) {
+			result = PTR_ERR(pcc->platform);
+			goto out_backlight;
+		}
+		result = device_create_file(&pcc->platform->dev,
+			&dev_attr_cdpower);
+		if (result)
+			goto out_platform;
+	} else {
+		pcc->platform = NULL;
+	}
+
 	return 0;
 
+out_platform:
+	platform_device_unregister(pcc->platform);
 out_backlight:
 	backlight_device_unregister(pcc->backlight);
 out_notify:
@@ -738,6 +870,11 @@ static int acpi_pcc_hotkey_remove(struct
 	if (!device || !pcc)
 		return -EINVAL;
 
+	if (pcc->platform) {
+		device_remove_file(&pcc->platform->dev, &dev_attr_cdpower);
+		platform_device_unregister(pcc->platform);
+	}
+
 	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
 
 	backlight_device_unregister(pcc->backlight);

Re: [PATCH] panasonic-laptop.c: add support for optical drive power control

[Len Brown]

On Wed, 14 Jan 2009, Martin Lucina wrote:

> Add support for power control of the built in optical drive on models
> with the required ACPI methods present.  Tested on Panasonic CF-W4.
> 
> Creates an interface in /sys/devices/platform/panasonic/cdpower, to
> which you can write "1" to switch the drive on, "0" to switch it off
> or read from to query the current state.
> 
> Signed-off-by: Martin Lucina <mato@kotelna.sk>
> 
> ---
> 
> Harald, this should address all the comments in this thread.  I've
> removed the DMI table and enable the code if _SB.{STAT,FBAY,CDDI} are
> all present.
> 
> I've not figured out how to return an actual error from the sysfs
> show/store functions so I've at least added ACPI_DEBUG_PRINT to print an
> error if the relevant methods fail.
> 
> -mato

Harald,
It's your driver.  What do you what to do with this patch.

I'd delegate http://patchwork.kernel.org/patch/2372/
to you, but you don't seem to have a patchwork account yet.

thanks,
-Len Brown, Intel Open Source Technology Center

Re: [PATCH] panasonic-laptop.c: add support for optical drive power control

[Martin Lucina]

Harald, Len,

> Harald,
> It's your driver.  What do you what to do with this patch.
> 
> I'd delegate http://patchwork.kernel.org/patch/2372/
> to you, but you don't seem to have a patchwork account yet.

Please don't apply this just yet.  I've recently managed to get hold of
some DSDT dumps from a Panasonic CF-W5 and they've changed the CD power
management methods so as it stands the patch will only work on a CF-W4 (and
possibly older models).

I'm currently trying to get a contact at Panasonic in Japan to see if they
can provide some documentation.  If not, at the very least a more generic
approach that would work would be to call _EJ0 on the OPTD device for power
off, but I've not been able to figure out from the ACPICA documentation how
I can search the device tree for an OPTD device.

Thanks,

-mato

Re: [PATCH] panasonic-laptop.c: add support for optical drive power control

[Len Brown]

On Mon, 6 Apr 2009, Martin Lucina wrote:
> 
> Please don't apply this just yet.

okay.
http://patchwork.kernel.org/patch/2372/
is marked superceded.

http://patchwork.kernel.org/project/linux-acpi/list/
is now down to 6 patches:-)

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH] panasonic-laptop.c: add support for optical drive power control

[Harald Welte]

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

On Sun, Apr 05, 2009 at 12:53:31AM -0400, Len Brown wrote:
> On Wed, 14 Jan 2009, Martin Lucina wrote:
> 
> > Add support for power control of the built in optical drive on models
> > with the required ACPI methods present.  Tested on Panasonic CF-W4.
> > 
> > Creates an interface in /sys/devices/platform/panasonic/cdpower, to
> > which you can write "1" to switch the drive on, "0" to switch it off
> > or read from to query the current state.
> > 
> > Signed-off-by: Martin Lucina <mato@kotelna.sk>
> > 
> > ---
> > 
> > Harald, this should address all the comments in this thread.  I've
> > removed the DMI table and enable the code if _SB.{STAT,FBAY,CDDI} are
> > all present.
> > 
> > I've not figured out how to return an actual error from the sysfs
> > show/store functions so I've at least added ACPI_DEBUG_PRINT to print an
> > error if the relevant methods fail.
> > 
> > -mato
> 
> Harald,
> It's your driver.  What do you what to do with this patch.
> 
> I'd delegate http://patchwork.kernel.org/patch/2372/
> to you, but you don't seem to have a patchwork account yet.

sorry, I was offline throughout last week and am now slowly catching up with
mails.

Will create a patchwork account and get back to you asap.

-- 
- Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]