[PATCH RFC v2] support SATA ODD zero power

[PATCH RFC v2] support SATA ODD zero power

[su henry]

>From 53f3108a156e69da4bae4c2f2b9a189e2456cdbb Mon Sep 17 00:00:00 2001
From: henry su <henry.su.ati@gmail.com>
Date: Fri, 9 Jul 2010 19:39:46 +0800
Subject: [PATCH RFC v2] support sata odd zero power

In order to extend the battery life of Mobile PC system, the host
should remove the power supply to the Optical Disc Drive (logical
unit) when it detects the logical unit with no media and tray closed,
and the host starts the power supply to the logical unit when it
detects user action to the logical unit that the power supply is
omitted.

The patch evaluates the _PS3 method to remove the power supply to the
ODD if the host detects MEDIUM NOT PRESENT - TRAY CLOSED is reported
by REQUEST SENSE command for a Drawer, Tray or Pop-up type drive, or
MEDIUM NOT PRESENT - TRAY CLOSED for a slot/caddy type drive; on the
other hand, the patch evaluates the _PS0 method to restart the power
supply to the ODD when the user presses the button on a tray type
drive or inserts a CD to a slot/caddy type drive.

PLDS DS-8A5S, pansonic UJ892 and UJ897 are the ODD samples used to
test the patch on AMD SB800 platforms.

Signed-off-by: Henry Su <henry.su.ati@gmail.com>
---
 drivers/scsi/Kconfig   |   17 ++++
 drivers/scsi/Makefile  |    2 +-
 drivers/scsi/sr.c      |    4 +
 drivers/scsi/sr.h      |   12 +++
 drivers/scsi/sr_acpi.c |  215 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 249 insertions(+), 1 deletions(-)
 create mode 100644 drivers/scsi/sr_acpi.c

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 75f2336..5634424 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -146,6 +146,23 @@ config BLK_DEV_SR_VENDOR
 	  drives (and HP Writers). If you have such a drive and get the first
 	  session only, try saying Y here; everybody else says N.

+config BLK_DEV_SR_ZERO_POWER
+	bool "Enable SATA Zero Power ODD support (for SCSI CDROM)"
+	depends on BLK_DEV_SR && ACPI
+	help
+	  When the host detects the SCSI CDROM with no media and tray closed in
+	  case of tray/drawer loading mechanism, the host remove the power
+	  supply to the CDROM. When host detect the user action to the CDROM
+	  that the power supply is ommited, the host will start the power
+	  supply to the CDROM. Host can achive Zero Power consumption on such
+	  empty CDROM.
+
+	  See more information in appendix K in Mt. Fuji commands for
+	  Multimedia devices version7.
+ 	
+	  If you want to support sata odd zero power, and the drive can do zero
+	  power, try saying Y here; else says N.
+
 config CHR_DEV_SG
 	tristate "SCSI generic support"
 	depends on SCSI
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 1c7ac49..cc9ec84 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -169,7 +169,7 @@ scsi_tgt-y			+= scsi_tgt_lib.o scsi_tgt_if.o
 sd_mod-objs	:= sd.o
 sd_mod-$(CONFIG_BLK_DEV_INTEGRITY) += sd_dif.o

-sr_mod-objs	:= sr.o sr_ioctl.o sr_vendor.o
+sr_mod-objs	:= sr.o sr_ioctl.o sr_vendor.o sr_acpi.o
 ncr53c8xx-flags-$(CONFIG_SCSI_ZALON) \
 		:= -DCONFIG_NCR53C8XX_PREFETCH -DSCSI_NCR_BIG_ENDIAN \
 			-DCONFIG_SCSI_NCR53C8XX_NO_WORD_TRANSFERS
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 0a90abc..7755658 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -631,6 +631,7 @@ static int sr_probe(struct device *dev)
 	get_capabilities(cd);
 	blk_queue_prep_rq(sdev->request_queue, sr_prep_fn);
 	sr_vendor_init(cd);
+	sr_acpi_init(cd);

 	disk->driverfs_dev = &sdev->sdev_gendev;
 	set_capacity(disk, cd->capacity);
@@ -789,6 +790,7 @@ static void get_capabilities(struct scsi_cd *cd)
 	cd->cdi.speed = ((buffer[n + 8] << 8) + buffer[n + 9]) / 176;
 	cd->readcd_known = 1;
 	cd->readcd_cdda = buffer[n + 5] & 0x01;
+	cd->type = buffer[n + 6] >> 5;
 	/* print some capability bits */
 	printk("%s: scsi3-mmc drive: %dx/%dx %s%s%s%s%s%s\n", cd->cdi.name,
 	       ((buffer[n + 14] << 8) + buffer[n + 15]) / 176,
@@ -894,6 +896,7 @@ static int sr_remove(struct device *dev)
 	mutex_lock(&sr_ref_mutex);
 	kref_put(&cd->kref, sr_kref_release);
 	mutex_unlock(&sr_ref_mutex);
+	sr_acpi_remove(dev);

 	return 0;
 }
@@ -916,6 +919,7 @@ static void __exit exit_sr(void)
 {
 	scsi_unregister_driver(&sr_template.gendrv);
 	unregister_blkdev(SCSI_CDROM_MAJOR, "sr");
+	sr_acpi_unregister();
 }

 module_init(init_sr);
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 1e144df..96d85e0 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -46,6 +46,13 @@ typedef struct scsi_cd {
 	 * the refs on this kref to decide when to release them */
 	struct kref kref;
 	struct gendisk *disk;
+
+#ifdef CONFIG_BLK_DEV_SR_ZERO_POWER
+	mechtype_t type;
+	unsigned int no_media_retry;
+	struct timer_list zero_power_timer;
+	struct work_struct sr_set_power;
+#endif
 } Scsi_CD;

 int sr_do_ioctl(Scsi_CD *, struct packet_command *);
@@ -68,4 +75,9 @@ void sr_vendor_init(Scsi_CD *);
 int sr_cd_check(struct cdrom_device_info *);
 int sr_set_blocklength(Scsi_CD *, int blocklength);

+/* sr_acpi.c */
+void sr_acpi_init(Scsi_CD *);
+int sr_acpi_remove(struct device *dev);
+void sr_acpi_unregister(void);
+
 #endif
diff --git a/drivers/scsi/sr_acpi.c b/drivers/scsi/sr_acpi.c
new file mode 100644
index 0000000..b5a5c66
--- /dev/null
+++ b/drivers/scsi/sr_acpi.c
@@ -0,0 +1,215 @@
+
+#include <linux/cdrom.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/bcd.h>
+#include <linux/blkdev.h>
+#include <linux/slab.h>
+
+#include <scsi/scsi.h>
+#include <scsi/scsi_dbg.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_driver.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_eh.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_ioctl.h>	/* For the door lock/unlock commands */
+
+#include <acpi/acpi_bus.h>
+#include "sr.h"
+
+#ifdef CONFIG_BLK_DEV_SR_ZERO_POWER
+#define SR_CHECK_STATE_TIMEOUT	(10*HZ)
+#define SR_NO_MEDIA_RETRY	10	
+#define SR_ZPODD_REGISTERED	0x01
+#define SR_ZPODD_NEED_EJECT	0x02
+
+static struct sr_zero_power {
+	struct workqueue_struct *wq;
+	u32 flags;
+	acpi_handle handle;
+	spinlock_t lock;
+	struct scsi_cd *cd;
+	struct work_struct set_power;
+} sr_zpodd;
+
+/* Remove the power supply to the drive if detecting no media */
+void sr_acpi_remove_power(struct work_struct *work)
+{
+	struct scsi_cd *cd = sr_zpodd.cd;
+	struct scsi_sense_hdr *sshdr;
+	int isvalid;
+
+	sshdr =  kzalloc(sizeof(*sshdr), GFP_KERNEL);
+	sr_test_unit_ready(sr_zpodd.cd->device, sshdr);
+	isvalid = scsi_sense_valid(sshdr);
+
+	/* 0x3a is media not present */
+	if ((isvalid && (sshdr->asc  == 0x3a &&
+		/* tray opened for caddy/slot type drive*/
+		((cd->type == mechtype_caddy && sshdr->ascq == 0x02) ||
+		/*tray closed for tray/drawer/pop-up for type drive*/
+		sshdr->ascq == 0x01)))) {
+			if (cd->no_media_retry < SR_NO_MEDIA_RETRY) {
+				cd->no_media_retry += 1;
+			} else {
+				del_timer_sync(&cd->zero_power_timer);
+				acpi_bus_set_power(sr_zpodd.handle,
+							ACPI_STATE_D3);
+			}
+	} else
+		cd->no_media_retry = 1;
+
+	kfree(sshdr);
+
+}
+
+void sr_acpi_queue_work(unsigned long data)
+{
+	int isvalid;
+	unsigned long flags;
+	struct scsi_cd *cd = sr_zpodd.cd;
+
+	spin_lock_irqsave(&sr_zpodd.lock, flags);
+	isvalid = queue_work(sr_zpodd.wq, &sr_zpodd.set_power);
+	BUG_ON(!isvalid);
+
+	mod_timer(&cd->zero_power_timer, jiffies+SR_CHECK_STATE_TIMEOUT);
+	spin_unlock_irqrestore(&sr_zpodd.lock, flags);
+}
+
+/*
+ * Handle ACPI event notification when pressing the button on a
+ * tray type drive or inserting a CD to a caddy/slot type drive
+ */
+static void sr_acpi_handle_events(acpi_handle handle, u32 event, void *context)
+{
+	unsigned int state;
+
+	switch (event) {
+	/* FIXME: handle both wake and 0x80 notifications? */
+	case 0x02:
+	case 0x80:
+		acpi_bus_get_power(sr_zpodd.handle, &state);
+		if (state == ACPI_STATE_D3) {
+			acpi_bus_set_power(sr_zpodd.handle,
+						ACPI_STATE_D0);
+			udelay(100);
+
+			/* Eject the tray after starting
+			 * the power supply to tray type ODD
+			 */
+			if (sr_zpodd.cd->type == mechtype_tray)
+				sr_zpodd.flags |= SR_ZPODD_NEED_EJECT;
+		}
+	}
+	return;
+}
+
+/*
+ * find the zero power odd device in ACPI namespace
+ * and install the notification handler for the device
+ */
+static acpi_status __init sr_acpi_find_zpodd(acpi_handle handle,
+		u32 lvl, void *context, void **rv)
+{
+	acpi_status status;
+	char name[ACPI_PATH_SEGMENT_LENGTH];
+	struct acpi_buffer buffer = { sizeof(name), &name };
+	acpi_handle *phandle = (acpi_handle *)context;
+
+	/* FIXME: search the device by object name or _HID? */
+	status = acpi_get_name(handle, ACPI_SINGLE_NAME, &buffer);
+
+	if (ACPI_SUCCESS(status) && !strncmp("ODDZ", name, sizeof(name) - 1)) {
+		*phandle = handle;
+
+		/* returning non-zero causes the search to stop
+		 * and returns this value to the caller of
+		 * acpi_walk_namespace.
+		 */
+		return 1;
+
+	} else
+		return 0;
+}
+#endif
+
+void sr_acpi_init(Scsi_CD *cd)
+{
+#ifdef CONFIG_BLK_DEV_SR_ZERO_POWER
+	acpi_status status;
+
+	/* Walk the namespace only once*/
+	if (!(sr_zpodd.flags & SR_ZPODD_REGISTERED)) {
+		if (acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+				ACPI_UINT32_MAX, sr_acpi_find_zpodd, NULL,
+				&sr_zpodd.handle, NULL) != 1) {
+			printk(KERN_WARNING
+				 "The SCSI CD doesn't support zero power\n");
+			goto init_return;
+		}
+
+		status = acpi_install_notify_handler(sr_zpodd.handle,
+				ACPI_DEVICE_NOTIFY, sr_acpi_handle_events,
+				NULL);
+		if (ACPI_FAILURE(status)) {
+			printk(KERN_WARNING
+				 "failed to register notification handler\n");
+			goto init_return;
+		} else
+			sr_zpodd.flags |= SR_ZPODD_REGISTERED;
+
+		sr_zpodd.wq = create_singlethread_workqueue("sr_zpodd.wq");
+		if (!sr_zpodd.wq) {
+			acpi_remove_notify_handler(
+				sr_zpodd.handle,
+				ACPI_DEVICE_NOTIFY,
+				sr_acpi_handle_events);
+			goto init_return;
+		}
+	}
+
+	INIT_WORK(&sr_zpodd.set_power, sr_acpi_remove_power);
+
+	spin_lock_init(&sr_zpodd.lock);
+	init_timer(&cd->zero_power_timer);
+	cd->zero_power_timer.data = (unsigned long) cd;
+	cd->zero_power_timer.function = sr_acpi_queue_work;
+	cd->zero_power_timer.expires = jiffies + SR_CHECK_STATE_TIMEOUT;
+	add_timer(&cd->zero_power_timer);
+
+	cd->no_media_retry = 1;
+	sr_zpodd.cd = cd;
+
+	if (sr_zpodd.flags & SR_ZPODD_NEED_EJECT) {
+		sr_zpodd.flags &= ~SR_ZPODD_NEED_EJECT;
+		sr_tray_move(&cd->cdi, 1);
+	}
+
+init_return:
+#endif
+	return;
+}
+
+void sr_acpi_unregister(void)
+{
+#ifdef CONFIG_BLK_DEV_SR_ZERO_POWER
+	if (sr_zpodd.flags & SR_ZPODD_REGISTERED) {
+		destroy_workqueue(sr_zpodd.wq);
+		acpi_remove_notify_handler(sr_zpodd.handle,
+					ACPI_DEVICE_NOTIFY,
+					sr_acpi_handle_events);
+	}
+#endif
+}
+
+int sr_acpi_remove(struct device *dev)
+{
+#ifdef CONFIG_BLK_DEV_SR_ZERO_POWER
+	struct scsi_cd *cd = dev_get_drvdata(dev);
+	if (sr_zpodd.flags & SR_ZPODD_REGISTERED)
+		del_timer_sync(&cd->zero_power_timer);
+#endif
+	return 0;
+}
-- 
1.5.6.3

Re: [PATCH RFC v2] support SATA ODD zero power

[Joey Lee]

Dear all expert, 

Hi, m... I just google the SATA ODD zero power support and found Henry
already contribute the patch:

 http://kerneltrap.org/mailarchive/linux-scsi/2010/6/25/6885558

And, after first round review, 
The second version is here:
 http://www.spinics.net/lists/linux-scsi/msg44925.html

But, I didn't see this patch merged in kernel maintain, does there have
any problem with this patch? or actually the patch already in queue
waiting to merge to mainline?

I appreciate if can have any comment to me.


Thank's a lot!
Joey Lee

Re: [PATCH RFC v2] support SATA ODD zero power

[Tejun Heo]

(cc'ing linux-ide)

Hello,

On 12/08/2010 04:36 PM, Joey Lee wrote:
> Dear all expert, 
> 
> Hi, m... I just google the SATA ODD zero power support and found Henry
> already contribute the patch:
> 
>  http://kerneltrap.org/mailarchive/linux-scsi/2010/6/25/6885558
> 
> And, after first round review, 
> The second version is here:
>  http://www.spinics.net/lists/linux-scsi/msg44925.html
> 
> But, I didn't see this patch merged in kernel maintain, does there have
> any problem with this patch? or actually the patch already in queue
> waiting to merge to mainline?
> 
> I appreciate if can have any comment to me.

IIRC, at the time, the problem was that nobody had the hardware and
the implementation was solely based on the specification only.  I
didn't want it to go in without any actual testing, so it was put on
hold for the time being.  Do you have access to a device with zsero
power support?  If so, are you willing to test the implementation?

Hmmm... at the same time, I'm a little bit doubtful about its
usefulness.  How much power does it save?  Furthermore, ODDs are often
omitted on mobile devices these days.  So, I have a slight reluctance
against including it but if the implementation is clean enough and
there is enough benefits it shouldn't be too difficult to overcome.

Thanks.

-- 
tejun

Re: [PATCH RFC v2] support SATA ODD zero power

[Joey Lee]

Hi Tejun, 

Nice can talk with you! :-)

於 三，2010-12-08 於 18:25 +0100，Tejun Heo 提到：
> (cc'ing linux-ide)
> 
> Hello,
> 
> On 12/08/2010 04:36 PM, Joey Lee wrote:
> > Dear all expert, 
> > 
> > Hi, m... I just google the SATA ODD zero power support and found Henry
> > already contribute the patch:
> > 
> >  http://kerneltrap.org/mailarchive/linux-scsi/2010/6/25/6885558
> > 
> > And, after first round review, 
> > The second version is here:
> >  http://www.spinics.net/lists/linux-scsi/msg44925.html
> > 
> > But, I didn't see this patch merged in kernel maintain, does there have
> > any problem with this patch? or actually the patch already in queue
> > waiting to merge to mainline?
> > 
> > I appreciate if can have any comment to me.
> 
> IIRC, at the time, the problem was that nobody had the hardware and
> the implementation was solely based on the specification only.  I
> didn't want it to go in without any actual testing, so it was put on
> hold for the time being.  Do you have access to a device with zsero
> power support?  If so, are you willing to test the implementation?
> 

Got it!
I will do the testing if I get hardware can support this function.

> Hmmm... at the same time, I'm a little bit doubtful about its
> usefulness.  How much power does it save?  Furthermore, ODDs are often

I am not sure how many watts can be save, it's need use 2-channel power
meter or NetDAQ to verify. I know how to do that, just need the sample
machine.

> omitted on mobile devices these days.  So, I have a slight reluctance
> against including it but if the implementation is clean enough and
> there is enough benefits it shouldn't be too difficult to overcome.

Fully understood!


Thank's a lot!
Joey Lee

Re: [PATCH RFC v2] support SATA ODD zero power

[Joey Lee]

Hi Henry, 

於 三，2010-12-08 於 17:56 +0000，joeyli(Joey Lee) 提到：
> Hi Tejun, 
> 
> Nice can talk with you! :-)
> 
> 於 三，2010-12-08 於 18:25 +0100，Tejun Heo 提到：
> > (cc'ing linux-ide)
> > 
> > Hello,
> > 
> > On 12/08/2010 04:36 PM, Joey Lee wrote:
> > > Dear all expert, 
> > > 
> > > Hi, m... I just google the SATA ODD zero power support and found Henry
> > > already contribute the patch:
> > > 
> > >  http://kerneltrap.org/mailarchive/linux-scsi/2010/6/25/6885558
> > > 
> > > And, after first round review, 
> > > The second version is here:
> > >  http://www.spinics.net/lists/linux-scsi/msg44925.html
> > > 
> > > But, I didn't see this patch merged in kernel maintain, does there have
> > > any problem with this patch? or actually the patch already in queue
> > > waiting to merge to mainline?
> > > 
> > > I appreciate if can have any comment to me.
> > 
> > IIRC, at the time, the problem was that nobody had the hardware and
> > the implementation was solely based on the specification only.  I
> > didn't want it to go in without any actual testing, so it was put on
> > hold for the time being.  Do you have access to a device with zsero
> > power support?  If so, are you willing to test the implementation?
> > 
> 
> Got it!
> I will do the testing if I get hardware can support this function.
> 

I received a AMD notebook, there have _PS3 method in DSDT table. After
test, your SATA ODD zero power patch works to me. The /dev/sr0 was
removed when there have no media in tray, patch set ODD power to zero.

But I have a bit questions:

- I suggest add the kernel config check in drivers/scsi/sr.c like the
following, it can avoid compiler error when we disable
BLK_DEV_SR_ZERO_POWER:

+#ifdef CONFIG_BLK_DEV_SR_ZERO_POWER
+       cd->type = buffer[n + 6] >> 5;
+#endif

> > Hmmm... at the same time, I'm a little bit doubtful about its
> > usefulness.  How much power does it save?  Furthermore, ODDs are often
> 
> I am not sure how many watts can be save, it's need use 2-channel power
> meter or NetDAQ to verify. I know how to do that, just need the sample
> machine.
> 

- After hardware guys test power consumption by 2 channel power-metter,
the patch can save 0.1 - 0.2 W (depend on different ODD hardware) when
ODD power set to zero. This number is not high.

My question is:
What's the different between "ODD Zero power" with "SATA Link Power
Management on ODD"?

(Sorry for I cann't publish more detail power-metter result because it's
from my customer's lab.)

> > omitted on mobile devices these days.  So, I have a slight reluctance
> > against including it but if the implementation is clean enough and
> > there is enough benefits it shouldn't be too difficult to overcome.
> 
> Fully understood!
> 
> 
> Thank's a lot!
> Joey Lee
 
The last question is:

- This Linux patch's behavior is different with Windows platform:
When ODD go to "Zero power" mode, if I want to open tray then I need
press ODD's tray button TWICE: The first time is enable ODD power and
second time for open the tray.
But, on Windows, just need press tray button ONE time, then the tray can
be opened. 

Does it possible modify your patch to enable ODD power then direct open
tray?



Thank's a lot!
Joey Lee

Re: [PATCH RFC v2] support SATA ODD zero power

[Joey Lee]

Hi Henry,

於 三，2010-12-08 於 17:56 +0000，joeyli(Joey Lee) 提到：
> Hi Tejun, 
> 
> Nice can talk with you! :-)
> 
> 於 三，2010-12-08 於 18:25 +0100，Tejun Heo 提到：
> > (cc'ing linux-ide)
> > 
> > Hello,
> > 
> > On 12/08/2010 04:36 PM, Joey Lee wrote:
> > > Dear all expert, 
> > > 
> > > Hi, m... I just google the SATA ODD zero power support and found Henry
> > > already contribute the patch:
> > > 
> > >  http://kerneltrap.org/mailarchive/linux-scsi/2010/6/25/6885558
> > > 
> > > And, after first round review, 
> > > The second version is here:
> > >  http://www.spinics.net/lists/linux-scsi/msg44925.html
> > > 
> > > But, I didn't see this patch merged in kernel maintain, does there have
> > > any problem with this patch? or actually the patch already in queue
> > > waiting to merge to mainline?
> > > 
> > > I appreciate if can have any comment to me.
> > 
> > IIRC, at the time, the problem was that nobody had the hardware and
> > the implementation was solely based on the specification only.  I
> > didn't want it to go in without any actual testing, so it was put on
> > hold for the time being.  Do you have access to a device with zsero
> > power support?  If so, are you willing to test the implementation?
> > 
> 
> Got it!
> I will do the testing if I get hardware can support this function.
> 

I received a AMD notebook, there have _PS3 method in DSDT table. After
test, your SATA ODD zero power patch works to me. The /dev/sr0 was
removed when there have no media in tray, patch set ODD power to zero.

But I have a bit questions:

- I suggest add the kernel config check in drivers/scsi/sr.c like the
following, it can avoid compiler error when we disable
BLK_DEV_SR_ZERO_POWER:

+#ifdef CONFIG_BLK_DEV_SR_ZERO_POWER
+       cd->type = buffer[n + 6] >> 5;
+#endif


> > Hmmm... at the same time, I'm a little bit doubtful about its
> > usefulness.  How much power does it save?  Furthermore, ODDs are often
> 
> I am not sure how many watts can be save, it's need use 2-channel power
> meter or NetDAQ to verify. I know how to do that, just need the sample
> machine.
> 

- After hardware guys test power consumption by 2 channel power-metter,
the patch can save 0.1 - 0.2 W (depend on different ODD hardware) when
ODD power set to zero. This number is not high.

My question is:
What's the different between "ODD Zero power" with "SATA Link Power
Management on ODD"?

(Sorry for I cann't publish more detail power-metter result because it's
from my customer's lab.)


> > omitted on mobile devices these days.  So, I have a slight reluctance
> > against including it but if the implementation is clean enough and
> > there is enough benefits it shouldn't be too difficult to overcome.
> 
> Fully understood!
> 
> 
> Thank's a lot!
> Joey Lee

The last question is:

- This Linux patch's behavior is different with Windows platform:
When ODD go to "Zero power" mode, if I want to open tray then I need
press ODD's tray button TWICE: The first time is enable ODD power and
second time for open the tray.
But, on Windows, just need press tray button ONE time, then the tray can
be opened. 

Does it possible modify your patch to enable ODD power then direct open
tray?



Thank's a lot!
Joey Lee


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC v2] support SATA ODD zero power

[Joey Lee]

Hi Tejun, 

Sorry bother you for about support SATA ODD zero power in kernel.

Henry didn't reply me. 
Could you please kindly give any comment for this topic?

Looks the SATA ODD zero power didn't have good power reserve number,
does that mean support it in Linux kernel is not worthwhile?


Thank's a lot!
Joey Lee

於 二，2011-02-22 於 10:48 +0000，joeyli(Joey Lee) 提到：
> Hi Henry,
> 
> 於 三，2010-12-08 於 17:56 +0000，joeyli(Joey Lee) 提到：
> > Hi Tejun, 
> > 
> > Nice can talk with you! :-)
> > 
> > 於 三，2010-12-08 於 18:25 +0100，Tejun Heo 提到：
> > > (cc'ing linux-ide)
> > > 
> > > Hello,
> > > 
> > > On 12/08/2010 04:36 PM, Joey Lee wrote:
> > > > Dear all expert, 
> > > > 
> > > > Hi, m... I just google the SATA ODD zero power support and found Henry
> > > > already contribute the patch:
> > > > 
> > > >  http://kerneltrap.org/mailarchive/linux-scsi/2010/6/25/6885558
> > > > 
> > > > And, after first round review, 
> > > > The second version is here:
> > > >  http://www.spinics.net/lists/linux-scsi/msg44925.html
> > > > 
> > > > But, I didn't see this patch merged in kernel maintain, does there have
> > > > any problem with this patch? or actually the patch already in queue
> > > > waiting to merge to mainline?
> > > > 
> > > > I appreciate if can have any comment to me.
> > > 
> > > IIRC, at the time, the problem was that nobody had the hardware and
> > > the implementation was solely based on the specification only.  I
> > > didn't want it to go in without any actual testing, so it was put on
> > > hold for the time being.  Do you have access to a device with zsero
> > > power support?  If so, are you willing to test the implementation?
> > > 
> > 
> > Got it!
> > I will do the testing if I get hardware can support this function.
> > 
> 
> I received a AMD notebook, there have _PS3 method in DSDT table. After
> test, your SATA ODD zero power patch works to me. The /dev/sr0 was
> removed when there have no media in tray, patch set ODD power to zero.
> 
> But I have a bit questions:
> 
> - I suggest add the kernel config check in drivers/scsi/sr.c like the
> following, it can avoid compiler error when we disable
> BLK_DEV_SR_ZERO_POWER:
> 
> +#ifdef CONFIG_BLK_DEV_SR_ZERO_POWER
> +       cd->type = buffer[n + 6] >> 5;
> +#endif
> 
> 
> > > Hmmm... at the same time, I'm a little bit doubtful about its
> > > usefulness.  How much power does it save?  Furthermore, ODDs are often
> > 
> > I am not sure how many watts can be save, it's need use 2-channel power
> > meter or NetDAQ to verify. I know how to do that, just need the sample
> > machine.
> > 
> 
> - After hardware guys test power consumption by 2 channel power-metter,
> the patch can save 0.1 - 0.2 W (depend on different ODD hardware) when
> ODD power set to zero. This number is not high.
> 
> My question is:
> What's the different between "ODD Zero power" with "SATA Link Power
> Management on ODD"?
> 
> (Sorry for I cann't publish more detail power-metter result because it's
> from my customer's lab.)
> 
> 
> > > omitted on mobile devices these days.  So, I have a slight reluctance
> > > against including it but if the implementation is clean enough and
> > > there is enough benefits it shouldn't be too difficult to overcome.
> > 
> > Fully understood!
> > 
> > 
> > Thank's a lot!
> > Joey Lee
> 
> The last question is:
> 
> - This Linux patch's behavior is different with Windows platform:
> When ODD go to "Zero power" mode, if I want to open tray then I need
> press ODD's tray button TWICE: The first time is enable ODD power and
> second time for open the tray.
> But, on Windows, just need press tray button ONE time, then the tray can
> be opened. 
> 
> Does it possible modify your patch to enable ODD power then direct open
> tray?
> 
> 
> 
> Thank's a lot!
> Joey Lee
> 

Re: [PATCH RFC v2] support SATA ODD zero power

[Tejun Heo]

Hello,

On Mon, Mar 21, 2011 at 12:07:21AM -0600, Joey Lee wrote:
> Sorry bother you for about support SATA ODD zero power in kernel.
> 
> Henry didn't reply me. 
> Could you please kindly give any comment for this topic?
> 
> Looks the SATA ODD zero power didn't have good power reserve number,
> does that mean support it in Linux kernel is not worthwhile?

Well, 0.1-0.2w is not nothing.  It could be meaningful I guess, but
the concern that I have are...

* How wildly is it gonna be deployed?  Most ultraportables don't have
  ODDs to begin with and extra features in ATA land tend to be very
  cumbersome to deploy.  It usually ends up like - half of them
  reporting the supportq incorrectly, quarter of them malfunction when
  enabled and so on.

* Why is software involved in this at all?  The device needs to
  process the eject button to begin with.  I don't really see why ACPI
  or operating system needs to be involved here at all.  What prevents
  ODD from implementing it itself?  Is there any fundamental reason
  why OS should be involved?

  The thing is that if it isn't essential, people aren't gonna
  implement it anyway and we just end up with unused code.

Thanks.

-- 
tejun

Re: [PATCH RFC v2] support SATA ODD zero power

[Joey Lee]

Hi Tejun, 

於 一，2011-03-21 於 18:27 +0100，Tejun Heo 提到：
> Hello,
> 
> On Mon, Mar 21, 2011 at 12:07:21AM -0600, Joey Lee wrote:
> > Sorry bother you for about support SATA ODD zero power in kernel.
> > 
> > Henry didn't reply me. 
> > Could you please kindly give any comment for this topic?
> > 
> > Looks the SATA ODD zero power didn't have good power reserve number,
> > does that mean support it in Linux kernel is not worthwhile?
> 
> Well, 0.1-0.2w is not nothing.  It could be meaningful I guess, but
> the concern that I have are...
> 
> * How wildly is it gonna be deployed?  Most ultraportables don't have
>   ODDs to begin with and extra features in ATA land tend to be very
>   cumbersome to deploy.  It usually ends up like - half of them
>   reporting the supportq incorrectly, quarter of them malfunction when
>   enabled and so on.
> 

Per my understood, Intel and AMD's are said they support SATA ODD zero
power on newest notebook platform, and currently still have many
notebook ship with ODD. Intel didn't open their spec for support this
new function.

And, 
Yes, this patch didn't test on ultraportables, especially for if they
try to use USB ODD. For this patch We still need have a lot of test.

> * Why is software involved in this at all?  The device needs to
>   process the eject button to begin with.  I don't really see why ACPI
>   or operating system needs to be involved here at all.  What prevents
>   ODD from implementing it itself?  Is there any fundamental reason
>   why OS should be involved?
> 

I don't know how does Intel implement this function (by EC or ACPI?),
but AMD customized a ACPI object to provide this function. I will find a
chance to use this function on Windows platform, per hardward guys give
me some information, Windows can resume the ODD power from GUI, I will
try to verify it on Windows platform then reply to this mail.

>   The thing is that if it isn't essential, people aren't gonna
>   implement it anyway and we just end up with unused code.
> 
> Thanks.
> 

Yes, this is not a essential feature, just Intel and AMD all said they
support it and some ODM also want to implement. 


Thank's a lot!
Joey Lee

Re: [PATCH RFC v2] support SATA ODD zero power

[Joey Lee]

於 二，2011-03-22 於 07:06 +0000，joeyli(Joey Lee) 提到：
> Hi Tejun, 
> 
> 於 一，2011-03-21 於 18:27 +0100，Tejun Heo 提到：
> > Hello,
> > 
> > On Mon, Mar 21, 2011 at 12:07:21AM -0600, Joey Lee wrote:
> > > Sorry bother you for about support SATA ODD zero power in kernel.
> > > 
> > > Henry didn't reply me. 
> > > Could you please kindly give any comment for this topic?
> > > 
> > > Looks the SATA ODD zero power didn't have good power reserve number,
> > > does that mean support it in Linux kernel is not worthwhile?
> > 
> > Well, 0.1-0.2w is not nothing.  It could be meaningful I guess, but
> > the concern that I have are...
> > 
> > * How wildly is it gonna be deployed?  Most ultraportables don't have
> >   ODDs to begin with and extra features in ATA land tend to be very
> >   cumbersome to deploy.  It usually ends up like - half of them
> >   reporting the supportq incorrectly, quarter of them malfunction when
> >   enabled and so on.
> > 
> 
> Per my understood, Intel and AMD's are said they support SATA ODD zero
> power on newest notebook platform, and currently still have many
> notebook ship with ODD. Intel didn't open their spec for support this
> new function.
> 
> And, 
> Yes, this patch didn't test on ultraportables, especially for if they
> try to use USB ODD. For this patch We still need have a lot of test.
> 
> > * Why is software involved in this at all?  The device needs to
> >   process the eject button to begin with.  I don't really see why ACPI
> >   or operating system needs to be involved here at all.  What prevents
> >   ODD from implementing it itself?  Is there any fundamental reason
> >   why OS should be involved?
> > 
> 
> I don't know how does Intel implement this function (by EC or ACPI?),
> but AMD customized a ACPI object to provide this function. I will find a
> chance to use this function on Windows platform, per hardward guys give
> me some information, Windows can resume the ODD power from GUI, I will
> try to verify it on Windows platform then reply to this mail.
> 

More information about the SATA ODD zero power support on Windows
platform from BIOSer. Unfortunately, I still didn't see the REAL windows
system that support SATA ODD zero power.

Per discuss with BIOSer, they said: 
	- The ODD icon still show up when ODD power-off.
		+ Driver turn-off ODD power if there have no disk in tray.
	- End user can power-on ODD by click ODD icon on GUI.
		+ Driver evaluate ACPI method to power-on ODD.
	- Intel and AMD all use ACPI to implement interactive with Windows.

Until now, Intel still didn't open any spec for ODD, as I know, there
also didn't have many shipped notebook support it.


Thank's a lot!
Joey Lee

> >   The thing is that if it isn't essential, people aren't gonna
> >   implement it anyway and we just end up with unused code.
> > 
> > Thanks.
> > 
> 
> Yes, this is not a essential feature, just Intel and AMD all said they
> support it and some ODM also want to implement. 
> 
> 
> Thank's a lot!
> Joey Lee
>