[PATCH v3 0/7] ZPODD patches for scsi tree

[PATCH v3 0/7] ZPODD patches for scsi tree

[Aaron Lu]

v3:
Rebase on top of scsi-misc tree;
Add the sr related patches previously in Jeff's libata tree;
Re-organize the sr patches.
A problem for now: for patch
scsi: sr: support zero power ODD(ZPODD)
I can't set a flag in libata-acpi.c since a related function is
missing in scsi-misc tree. Will fix this when 3.6-rc1 released.

v2:
Bug fix for v1;
Use scsi_autopm_* in sr driver instead of pm_runtime_*;

v1:
Here are some patches to make ZPODD easier to use for end users and
a fix for using ZPODD with system suspend.

Aaron Lu (7):
  scsi: sr: check support for device busy class events
  scsi: pm: use autosuspend if device supports it
  scsi: sr: support zero power ODD(ZPODD)
  scsi: sr: block events when runtime suspended
  scsi: pm: use runtime resume callback if available
  scsi: sr: balance sr disk events block depth
  block: genhd: add an interface to set disk's poll interval

 block/genhd.c              |  25 +++++--
 drivers/scsi/scsi_pm.c     |  23 ++++--
 drivers/scsi/sr.c          | 176 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/sr.h          |   3 +
 include/linux/cdrom.h      |  43 +++++++++++
 include/linux/genhd.h      |   1 +
 include/scsi/scsi_device.h |   2 +
 7 files changed, 260 insertions(+), 13 deletions(-)

-- 
1.7.11.3

[PATCH v3 1/7] scsi: sr: check support for device busy class events

[Aaron Lu]

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 drivers/scsi/sr.c     | 23 +++++++++++++++++++++++
 drivers/scsi/sr.h     |  1 +
 include/linux/cdrom.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..abfefab 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -101,6 +101,7 @@ static DEFINE_MUTEX(sr_ref_mutex);
 static int sr_open(struct cdrom_device_info *, int);
 static void sr_release(struct cdrom_device_info *);
 
+static void check_dbml(struct scsi_cd *);
 static void get_sectorsize(struct scsi_cd *);
 static void get_capabilities(struct scsi_cd *);
 
@@ -728,6 +729,28 @@ fail:
 	return error;
 }
 
+static void check_dbml(struct scsi_cd *cd)
+{
+	struct packet_command cgc;
+	unsigned char buffer[16];
+	struct rm_feature_desc *rfd;
+
+	init_cdrom_command(&cgc, buffer, sizeof(buffer), CGC_DATA_READ);
+	cgc.cmd[0] = GPCMD_GET_CONFIGURATION;
+	cgc.cmd[3] = CDF_RM;
+	cgc.cmd[8] = sizeof(buffer);
+	cgc.quiet = 1;
+
+	if (cd->cdi.ops->generic_packet(&cd->cdi, &cgc))
+		return;
+
+	rfd = (struct rm_feature_desc *)&buffer[sizeof(struct feature_header)];
+	if (be16_to_cpu(rfd->feature_code) != CDF_RM)
+		return;
+
+	if (rfd->dbml)
+		cd->dbml = 1;
+}
 
 static void get_sectorsize(struct scsi_cd *cd)
 {
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 37c8f6b..7cc40ad 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -41,6 +41,7 @@ typedef struct scsi_cd {
 	unsigned readcd_known:1;	/* drive supports READ_CD (0xbe) */
 	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
 	unsigned media_present:1;	/* media is present */
+	unsigned dbml:1;	/* generates device busy class events */
 
 	/* GET_EVENT spurious event handling, blk layer guarantees exclusion */
 	int tur_mismatch;		/* nr of get_event TUR mismatches */
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index dfd7f18..962be39 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -727,6 +727,7 @@ struct request_sense {
 /*
  * feature profile
  */
+#define CDF_RM		0x0003	/* "Removable Medium" */
 #define CDF_RWRT	0x0020	/* "Random Writable" */
 #define CDF_HWDM	0x0024	/* "Hardware Defect Management" */
 #define CDF_MRW 	0x0028
@@ -739,6 +740,48 @@ struct request_sense {
 #define CDM_MRW_BGFORMAT_ACTIVE		2
 #define CDM_MRW_BGFORMAT_COMPLETE	3
 
+/* Removable medium feature descriptor */
+struct rm_feature_desc {
+	__be16 feature_code;
+#if defined(__BIG_ENDIAN_BITFIELD)
+	__u8 reserved1		: 2;
+	__u8 feature_version	: 4;
+	__u8 persistent		: 1;
+	__u8 curr		: 1;
+#elif defined(__LITTLE_ENDIAN_BITFIELD)
+	__u8 curr		: 1;
+	__u8 persistent		: 1;
+	__u8 feature_version	: 4;
+	__u8 reserved1		: 2;
+#endif
+	__u8 add_len;
+#if defined(__BIG_ENDIAN_BITFIELD)
+	__u8 mech_type		: 3;
+	__u8 load		: 1;
+	__u8 eject		: 1;
+	__u8 pvnt_jmpr		: 1;
+	__u8 dbml		: 1;
+	__u8 lock		: 1;
+#elif defined(__LITTLE_ENDIAN_BITFIELD)
+	__u8 lock		: 1;
+	__u8 dbml		: 1;
+	__u8 pvnt_jmpr		: 1;
+	__u8 eject		: 1;
+	__u8 load		: 1;
+	__u8 mech_type		: 3;
+#endif
+	__u8 reserved2;
+	__u8 reserved3;
+	__u8 reserved4;
+};
+
+struct device_busy_event_desc {
+	__u8 device_busy_event	: 4;
+	__u8 reserved1		: 4;
+	__u8 device_busy_status;
+	__u8 time;
+};
+
 /*
  * mrw address spaces
  */
-- 
1.7.11.3

[PATCH v3 2/7] scsi: pm: use autosuspend if device supports it

[Aaron Lu]

If the device is using autosuspend, when scsi_autopm_put_device is
called for it, use autosuspend runtime pm calls instead of the sync
call.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 drivers/scsi/scsi_pm.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index dc0ad85..7c93723 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -197,7 +197,13 @@ EXPORT_SYMBOL_GPL(scsi_autopm_get_device);
 
 void scsi_autopm_put_device(struct scsi_device *sdev)
 {
-	pm_runtime_put_sync(&sdev->sdev_gendev);
+	if (sdev->sdev_gendev.power.use_autosuspend) {
+		pm_runtime_mark_last_busy(&sdev->sdev_gendev);
+		pm_runtime_put_autosuspend(&sdev->sdev_gendev);
+	} else {
+		pm_runtime_put_sync(&sdev->sdev_gendev);
+	}
+
 }
 EXPORT_SYMBOL_GPL(scsi_autopm_put_device);
 
-- 
1.7.11.3

Re: [PATCH v3 2/7] scsi: pm: use autosuspend if device supports it

[Oliver Neukum]

On Thursday 26 July 2012 18:05:24 Aaron Lu wrote:
> If the device is using autosuspend, when scsi_autopm_put_device is
> called for it, use autosuspend runtime pm calls instead of the sync
> call.

What is the purpose of this approach?
You need a very good reason to have an API do two different things
based on this.

	Regards
		Oliver

Re: [PATCH v3 2/7] scsi: pm: use autosuspend if device supports it

[Aaron Lu]

On Thu, Jul 26, 2012 at 12:44:24PM +0200, Oliver Neukum wrote:
> On Thursday 26 July 2012 18:05:24 Aaron Lu wrote:
> > If the device is using autosuspend, when scsi_autopm_put_device is
> > called for it, use autosuspend runtime pm calls instead of the sync
> > call.
> 
> What is the purpose of this approach?

The purpose is to let scsi layer driver(sd, sr, etc.) use the same pm
api(scsi_autopm_put_device) to put the device to runtime suspended
state.
When the device is ready to be suspended, if it does not make use of
autosuspend, call pm_runtime_put_sync for it; if it makes use of
autosuspend, call the autosuspend runtime pm apis for it.

> You need a very good reason to have an API do two different things
> based on this.

If you see the above reason not good, I'll prepare an updated version
to create a new api to cover the autosuspend case, something like:
void scsi_autopm_put_device_autosuspend(struct scsi_device *sdev)
{
	pm_runtime_mark_last_busy(&sdev->sdev_gendev);
	pm_runtime_put_autosuspend(&sdev->sdev_gendev);
}
Does this look right?

Thanks,
Aaron

Re: [PATCH v3 2/7] scsi: pm: use autosuspend if device supports it

[Oliver Neukum]

On Thursday 26 July 2012 20:43:32 Aaron Lu wrote:
> > What is the purpose of this approach?
> 
> The purpose is to let scsi layer driver(sd, sr, etc.) use the same pm
> api(scsi_autopm_put_device) to put the device to runtime suspended
> state.
> When the device is ready to be suspended, if it does not make use of
> autosuspend, call pm_runtime_put_sync for it; if it makes use of
> autosuspend, call the autosuspend runtime pm apis for it.
> 
> > You need a very good reason to have an API do two different things
> > based on this.
> 
> If you see the above reason not good, I'll prepare an updated version
> to create a new api to cover the autosuspend case, something like:
> void scsi_autopm_put_device_autosuspend(struct scsi_device *sdev)
> {
>         pm_runtime_mark_last_busy(&sdev->sdev_gendev);
>         pm_runtime_put_autosuspend(&sdev->sdev_gendev);
> }
> Does this look right?

Much better :-)

	Regards
		Oliver

[PATCH v3 3/7] scsi: sr: support zero power ODD(ZPODD)

[Aaron Lu]

The ODD will be placed into suspend state when:
1 For tray type ODD, no media inside and door closed;
2 For slot type ODD, no media inside;
And together with ACPI, when we suspend the ODD, we will omit the power
altogether to reduce power consumption(done in libata-acpi.c).

The ODD can be resumed either by user or by software.

For user to resume the suspended ODD:
1 For tray type ODD, press the eject button;
2 For slot type ODD, insert a disc;
Once such events happened, an ACPI notification will be sent and in our
handler, we will power up the ODD and set its status back to
active(again in libata-acpi.c).

For software to resume the suspended ODD, we did this in ODD's
open/release function: we scsi_autopm_get/put_device in scsi_cd_get/put.

On old distros, the udisk daemon will poll the ODD and thus ODD will be
open/closed every 2 seconds. To make use of ZPODD, udisks' poll has to
be inhibited:
$ udisks --inhibit-polling /dev/sr0

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 drivers/scsi/sr.c          | 130 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/sr.h          |   2 +
 include/scsi/scsi_device.h |   2 +
 3 files changed, 133 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index abfefab..2573a42 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -45,6 +45,7 @@
 #include <linux/blkdev.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 #include <asm/uaccess.h>
 
 #include <scsi/scsi.h>
@@ -79,6 +80,8 @@ static DEFINE_MUTEX(sr_mutex);
 static int sr_probe(struct device *);
 static int sr_remove(struct device *);
 static int sr_done(struct scsi_cmnd *);
+static int sr_suspend(struct device *dev, pm_message_t msg);
+static int sr_resume(struct device *dev);
 
 static struct scsi_driver sr_template = {
 	.owner			= THIS_MODULE,
@@ -86,6 +89,8 @@ static struct scsi_driver sr_template = {
 		.name   	= "sr",
 		.probe		= sr_probe,
 		.remove		= sr_remove,
+		.suspend	= sr_suspend,
+		.resume		= sr_resume,
 	},
 	.done			= sr_done,
 };
@@ -147,8 +152,12 @@ static inline struct scsi_cd *scsi_cd_get(struct gendisk *disk)
 	kref_get(&cd->kref);
 	if (scsi_device_get(cd->device))
 		goto out_put;
+	if (scsi_autopm_get_device(cd->device))
+		goto out_pm;
 	goto out;
 
+ out_pm:
+	scsi_device_put(cd->device);
  out_put:
 	kref_put(&cd->kref, sr_kref_release);
 	cd = NULL;
@@ -164,9 +173,61 @@ static void scsi_cd_put(struct scsi_cd *cd)
 	mutex_lock(&sr_ref_mutex);
 	kref_put(&cd->kref, sr_kref_release);
 	scsi_device_put(sdev);
+	scsi_autopm_put_device(sdev);
 	mutex_unlock(&sr_ref_mutex);
 }
 
+static int sr_suspend(struct device *dev, pm_message_t msg)
+{
+	int poweroff;
+	struct scsi_sense_hdr sshdr;
+	struct scsi_cd *cd = dev_get_drvdata(dev);
+
+	/* no action for system pm */
+	if (!PMSG_IS_AUTO(msg))
+		return 0;
+
+	/* do another TUR to see if the ODD is still ready to be powered off */
+	scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
+
+	if (cd->cdi.mask & CDC_CLOSE_TRAY)
+		/* no media for caddy/slot type ODD */
+		poweroff = scsi_sense_valid(&sshdr) && sshdr.asc == 0x3a;
+	else
+		/* no media and door closed for tray type ODD */
+		poweroff = scsi_sense_valid(&sshdr) && sshdr.asc == 0x3a &&
+			sshdr.ascq == 0x01;
+
+	if (!poweroff) {
+		pm_runtime_get_noresume(dev);
+		atomic_set(&cd->suspend_count, 1);
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+static int sr_resume(struct device *dev)
+{
+	struct scsi_cd *cd;
+	struct scsi_sense_hdr sshdr;
+
+	cd = dev_get_drvdata(dev);
+
+	/* get the disk ready */
+	scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
+
+	/* if user wakes up the ODD, eject the tray */
+	if (cd->device->wakeup_by_user) {
+		cd->device->wakeup_by_user = 0;
+		if (!(cd->cdi.mask & CDC_CLOSE_TRAY))
+			sr_tray_move(&cd->cdi, 1);
+		atomic_set(&cd->suspend_count, 1);
+	}
+
+	return 0;
+}
+
 static unsigned int sr_get_events(struct scsi_device *sdev)
 {
 	u8 buf[8];
@@ -201,6 +262,38 @@ static unsigned int sr_get_events(struct scsi_device *sdev)
 	return 0;
 }
 
+static int sr_unit_load_done(struct scsi_cd *cd)
+{
+	u8 buf[8];
+	u8 cmd[] = { GET_EVENT_STATUS_NOTIFICATION,
+		1,                 /* polled */
+		0, 0,              /* reserved */
+		1 << 6,            /* notification class: Device Busy */
+		0, 0,              /* reserved */
+		0, sizeof(buf),    /* allocation length */
+		0,                 /* control */
+	};
+	struct event_header *eh = (void *)buf;
+	struct device_busy_event_desc *desc = (void *)(buf + 4);
+	struct scsi_sense_hdr sshdr;
+	int result;
+
+	result = scsi_execute_req(cd->device, cmd, DMA_FROM_DEVICE, buf,
+			sizeof(buf), &sshdr, SR_TIMEOUT, MAX_RETRIES, NULL);
+
+	if (result || be16_to_cpu(eh->data_len) < sizeof(*desc))
+		return 0;
+
+	if (eh->nea || eh->notification_class != 0x6)
+		return 0;
+
+	if (desc->device_busy_event == 2 && desc->device_busy_status == 0)
+		return 1;
+	else
+		return 0;
+}
+
+
 /*
  * This function checks to see if the media has been changed or eject
  * button has been pressed.  It is possible that we have already
@@ -215,12 +308,21 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 	bool last_present;
 	struct scsi_sense_hdr sshdr;
 	unsigned int events;
-	int ret;
+	int ret, poweroff;
 
 	/* no changer support */
 	if (CDSL_CURRENT != slot)
 		return 0;
 
+	if (pm_runtime_suspended(&cd->device->sdev_gendev))
+		return 0;
+
+	/* if the logical unit just finished loading/unloading, do a TUR */
+	if (cd->device->can_power_off && cd->dbml && sr_unit_load_done(cd)) {
+		events = 0;
+		goto do_tur;
+	}
+
 	events = sr_get_events(cd->device);
 	cd->get_event_changed |= events & DISK_EVENT_MEDIA_CHANGE;
 
@@ -270,6 +372,20 @@ do_tur:
 		cd->tur_changed = true;
 	}
 
+	if (cd->device->can_power_off && !cd->media_present) {
+		if (cd->cdi.mask & CDC_CLOSE_TRAY)
+			poweroff = 1;
+		else
+			poweroff = sshdr.ascq == 0x01;
+		/*
+		 * This function might be called concurrently by a kernel
+		 * thread (in-kernel polling) and old versions of udisks,
+		 * to avoid put the device twice, an atomic operation is used.
+		 */
+		if (poweroff && atomic_add_unless(&cd->suspend_count, -1, 0))
+			scsi_autopm_put_device(cd->device);
+	}
+
 	if (cd->ignore_get_event)
 		return events;
 
@@ -703,6 +819,14 @@ static int sr_probe(struct device *dev)
 	get_capabilities(cd);
 	blk_queue_prep_rq(sdev->request_queue, sr_prep_fn);
 	sr_vendor_init(cd);
+	/* zero power support */
+	if (sdev->can_power_off) {
+		check_dbml(cd);
+		/* default delay time is 3 minutes */
+		pm_runtime_set_autosuspend_delay(dev, 180 * 1000);
+		pm_runtime_use_autosuspend(dev);
+		atomic_set(&cd->suspend_count, 1);
+	}
 
 	disk->driverfs_dev = &sdev->sdev_gendev;
 	set_capacity(disk, cd->capacity);
@@ -988,6 +1112,10 @@ static int sr_remove(struct device *dev)
 {
 	struct scsi_cd *cd = dev_get_drvdata(dev);
 
+	/* disable runtime pm and possibly resume the device */
+	if (!atomic_dec_and_test(&cd->suspend_count))
+		scsi_autopm_get_device(cd->device);
+
 	blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
 	del_gendisk(cd->disk);
 
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 7cc40ad..649fe76 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -49,6 +49,8 @@ typedef struct scsi_cd {
 	bool get_event_changed:1;	/* changed according to GET_EVENT */
 	bool ignore_get_event:1;	/* GET_EVENT is unreliable, use TUR */
 
+	atomic_t suspend_count;
+
 	struct cdrom_device_info cdi;
 	/* We hold gendisk and scsi_device references on probe and use
 	 * the refs on this kref to decide when to release them */
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 7539f52..9d11981 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -155,6 +155,8 @@ struct scsi_device {
 	unsigned try_rc_10_first:1;	/* Try READ_CAPACACITY_10 first */
 	unsigned is_visible:1;	/* is the device visible in sysfs */
 	unsigned wce_default_on:1;	/* Cache is ON by default */
+	unsigned can_power_off:1;	/* Device supports runtime power off */
+	unsigned wakeup_by_user:1;	/* user wakes up the ODD */
 
 	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
 	struct list_head event_list;	/* asserted events */
-- 
1.7.11.3

[PATCH v3 4/7] scsi: sr: block events when runtime suspended

[Aaron Lu]

When the ODD is runtime suspended, there is no need to poll it for
events, so block events poll for it and unblock when resumed.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 block/genhd.c     | 2 ++
 drivers/scsi/sr.c | 7 ++++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 9cf5583..bdb3682 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1458,6 +1458,7 @@ void disk_block_events(struct gendisk *disk)
 
 	mutex_unlock(&ev->block_mutex);
 }
+EXPORT_SYMBOL(disk_block_events);
 
 static void __disk_unblock_events(struct gendisk *disk, bool check_now)
 {
@@ -1502,6 +1503,7 @@ void disk_unblock_events(struct gendisk *disk)
 	if (disk->ev)
 		__disk_unblock_events(disk, false);
 }
+EXPORT_SYMBOL(disk_unblock_events);
 
 /**
  * disk_flush_events - schedule immediate event checking and flushing
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 2573a42..fd1c2f6 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -204,6 +204,8 @@ static int sr_suspend(struct device *dev, pm_message_t msg)
 		return -EBUSY;
 	}
 
+	disk_block_events(cd->disk);
+
 	return 0;
 }
 
@@ -225,6 +227,8 @@ static int sr_resume(struct device *dev)
 		atomic_set(&cd->suspend_count, 1);
 	}
 
+	disk_unblock_events(cd->disk);
+
 	return 0;
 }
 
@@ -314,9 +318,6 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 	if (CDSL_CURRENT != slot)
 		return 0;
 
-	if (pm_runtime_suspended(&cd->device->sdev_gendev))
-		return 0;
-
 	/* if the logical unit just finished loading/unloading, do a TUR */
 	if (cd->device->can_power_off && cd->dbml && sr_unit_load_done(cd)) {
 		events = 0;
-- 
1.7.11.3

[PATCH v3 5/7] scsi: pm: use runtime resume callback if available

[Aaron Lu]

When runtime resume a scsi device, if the device's driver has
implemented runtime resume callback, use that.

sr driver needs this to do different things for system resume and
runtime resume.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 drivers/scsi/scsi_pm.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 7c93723..65d723d 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -34,14 +34,19 @@ static int scsi_dev_type_suspend(struct device *dev, pm_message_t msg)
 	return err;
 }
 
-static int scsi_dev_type_resume(struct device *dev)
+static int scsi_dev_type_resume(struct device *dev, bool runtime)
 {
 	struct device_driver *drv;
 	int err = 0;
+	int (*resume)(struct device *);
 
 	drv = dev->driver;
-	if (drv && drv->resume)
-		err = drv->resume(dev);
+	if (runtime && drv && drv->pm && drv->pm->runtime_resume)
+		resume = drv->pm->runtime_resume;
+	else
+		resume = drv ? drv->resume : NULL;
+	if (resume)
+		err = resume(dev);
 	scsi_device_resume(to_scsi_device(dev));
 	dev_dbg(dev, "scsi resume: %d\n", err);
 	return err;
@@ -85,7 +90,7 @@ static int scsi_bus_resume_common(struct device *dev)
 	pm_runtime_get_sync(dev->parent);
 
 	if (scsi_is_sdev_device(dev))
-		err = scsi_dev_type_resume(dev);
+		err = scsi_dev_type_resume(dev, false);
 	if (err == 0) {
 		pm_runtime_disable(dev);
 		pm_runtime_set_active(dev);
@@ -160,7 +165,7 @@ static int scsi_runtime_resume(struct device *dev)
 
 	dev_dbg(dev, "scsi_runtime_resume\n");
 	if (scsi_is_sdev_device(dev))
-		err = scsi_dev_type_resume(dev);
+		err = scsi_dev_type_resume(dev, true);
 
 	/* Insert hooks here for targets, hosts, and transport classes */
 
-- 
1.7.11.3

[PATCH v3 6/7] scsi: sr: balance sr disk events block depth

[Aaron Lu]

When the ODD is resumed, disk_unblock_events should be called when:
1 The ODD is runtime resumed;
2 System is resuming from S3 and the ODD is runtime suspended before S3;
But not when the system is resuming from S3 and the ODD is runtime
active before S3.

So seperate the resume calls, one for system resume and one for runtime
resume to do different things accordingly.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 drivers/scsi/sr.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index fd1c2f6..b8c2f9d 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -82,6 +82,11 @@ static int sr_remove(struct device *);
 static int sr_done(struct scsi_cmnd *);
 static int sr_suspend(struct device *dev, pm_message_t msg);
 static int sr_resume(struct device *dev);
+static int sr_runtime_resume(struct device *dev);
+
+static struct dev_pm_ops sr_pm_ops = {
+	.runtime_resume = sr_runtime_resume,
+};
 
 static struct scsi_driver sr_template = {
 	.owner			= THIS_MODULE,
@@ -91,6 +96,7 @@ static struct scsi_driver sr_template = {
 		.remove		= sr_remove,
 		.suspend	= sr_suspend,
 		.resume		= sr_resume,
+		.pm		= &sr_pm_ops,
 	},
 	.done			= sr_done,
 };
@@ -211,6 +217,21 @@ static int sr_suspend(struct device *dev, pm_message_t msg)
 
 static int sr_resume(struct device *dev)
 {
+	struct scsi_cd *cd = dev_get_drvdata(dev);
+
+	/*
+	 * If ODD is runtime suspended before system pm, unblock disk
+	 * events now since on system resume, we will fully resume it
+	 * and set its rumtime status to active.
+	 */
+	if (pm_runtime_suspended(dev))
+		disk_unblock_events(cd->disk);
+
+	return 0;
+}
+
+static int sr_runtime_resume(struct device *dev)
+{
 	struct scsi_cd *cd;
 	struct scsi_sense_hdr sshdr;
 
-- 
1.7.11.3

Re: [PATCH v3 6/7] scsi: sr: balance sr disk events block depth

[Sergei Shtylyov]

Hello.

On 26-07-2012 14:05, Aaron Lu wrote:

> When the ODD is resumed, disk_unblock_events should be called when:
> 1 The ODD is runtime resumed;
> 2 System is resuming from S3 and the ODD is runtime suspended before S3;
> But not when the system is resuming from S3 and the ODD is runtime
> active before S3.

> So seperate the resume calls, one for system resume and one for runtime
> resume to do different things accordingly.

> Signed-off-by: Aaron Lu <aaron.lu@amd.com>
> ---
>   drivers/scsi/sr.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)

> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index fd1c2f6..b8c2f9d 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
[...]
> @@ -211,6 +217,21 @@ static int sr_suspend(struct device *dev, pm_message_t msg)
>
>   static int sr_resume(struct device *dev)
>   {
> +	struct scsi_cd *cd = dev_get_drvdata(dev);
> +
> +	/*
> +	 * If ODD is runtime suspended before system pm, unblock disk
> +	 * events now since on system resume, we will fully resume it

    Comma not needed.

> +	 * and set its rumtime status to active.

    s/rumtime/runtime/. Nice typo. %-)

WBR, Sergei

Re: [PATCH v3 6/7] scsi: sr: balance sr disk events block depth

[Aaron Lu]

On Thu, Jul 26, 2012 at 02:54:01PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 26-07-2012 14:05, Aaron Lu wrote:
> 
> >When the ODD is resumed, disk_unblock_events should be called when:
> >1 The ODD is runtime resumed;
> >2 System is resuming from S3 and the ODD is runtime suspended before S3;
> >But not when the system is resuming from S3 and the ODD is runtime
> >active before S3.
> 
> >So seperate the resume calls, one for system resume and one for runtime
> >resume to do different things accordingly.
> 
> >Signed-off-by: Aaron Lu <aaron.lu@amd.com>
> >---
> >  drivers/scsi/sr.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> 
> >diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> >index fd1c2f6..b8c2f9d 100644
> >--- a/drivers/scsi/sr.c
> >+++ b/drivers/scsi/sr.c
> [...]
> >@@ -211,6 +217,21 @@ static int sr_suspend(struct device *dev, pm_message_t msg)
> >
> >  static int sr_resume(struct device *dev)
> >  {
> >+	struct scsi_cd *cd = dev_get_drvdata(dev);
> >+
> >+	/*
> >+	 * If ODD is runtime suspended before system pm, unblock disk
> >+	 * events now since on system resume, we will fully resume it
> 
>    Comma not needed.

Thanks.

> 
> >+	 * and set its rumtime status to active.
> 
>    s/rumtime/runtime/. Nice typo. %-)

Thanks, and I'll use a spell checker next time to avoid such mistakes.

-Aaron

[PATCH v3 7/7] block: genhd: add an interface to set disk's poll interval

[Aaron Lu]

Set the ODD's in kernel poll interval to 2s for the user in case the
user is using an old distro on which udev will not set the system wide
block parameter events_dfl_poll_msecs.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
 block/genhd.c         | 23 +++++++++++++++++------
 drivers/scsi/sr.c     |  1 +
 include/linux/genhd.h |  1 +
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index bdb3682..de9b9d9 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1619,6 +1619,19 @@ static void disk_events_workfn(struct work_struct *work)
 		kobject_uevent_env(&disk_to_dev(disk)->kobj, KOBJ_CHANGE, envp);
 }
 
+int disk_events_set_poll_msecs(struct gendisk *disk, long intv)
+{
+	if (intv < 0 && intv != -1)
+		return -EINVAL;
+
+	disk_block_events(disk);
+	disk->ev->poll_msecs = intv;
+	__disk_unblock_events(disk, true);
+
+	return 0;
+}
+EXPORT_SYMBOL(disk_events_set_poll_msecs);
+
 /*
  * A disk events enabled device has the following sysfs nodes under
  * its /sys/block/X/ directory.
@@ -1675,16 +1688,14 @@ static ssize_t disk_events_poll_msecs_store(struct device *dev,
 {
 	struct gendisk *disk = dev_to_disk(dev);
 	long intv;
+	int ret;
 
 	if (!count || !sscanf(buf, "%ld", &intv))
 		return -EINVAL;
 
-	if (intv < 0 && intv != -1)
-		return -EINVAL;
-
-	disk_block_events(disk);
-	disk->ev->poll_msecs = intv;
-	__disk_unblock_events(disk, true);
+	ret = disk_events_set_poll_msecs(disk, intv);
+	if (ret)
+		return ret;
 
 	return count;
 }
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index b8c2f9d..d639bcc 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -862,6 +862,7 @@ static int sr_probe(struct device *dev)
 	dev_set_drvdata(dev, cd);
 	disk->flags |= GENHD_FL_REMOVABLE;
 	add_disk(disk);
+	disk_events_set_poll_msecs(disk, 2000);
 
 	sdev_printk(KERN_DEBUG, sdev,
 		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 017a7fb..7414fb5 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -418,6 +418,7 @@ extern void disk_block_events(struct gendisk *disk);
 extern void disk_unblock_events(struct gendisk *disk);
 extern void disk_flush_events(struct gendisk *disk, unsigned int mask);
 extern unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask);
+extern int disk_events_set_poll_msecs(struct gendisk *disk, long intv);
 
 /* drivers/char/random.c */
 extern void add_disk_randomness(struct gendisk *disk);
-- 
1.7.11.3

Re: [PATCH v3 0/7] ZPODD patches for scsi tree

[Jeff Garzik]

On 07/26/2012 06:05 AM, Aaron Lu wrote:
> v3:
> Rebase on top of scsi-misc tree;
> Add the sr related patches previously in Jeff's libata tree;
> Re-organize the sr patches.
> A problem for now: for patch
> scsi: sr: support zero power ODD(ZPODD)
> I can't set a flag in libata-acpi.c since a related function is
> missing in scsi-misc tree. Will fix this when 3.6-rc1 released.

What does this mean?  Would you be more specific?

	Jeff

Re: [PATCH v3 0/7] ZPODD patches for scsi tree

[Aaron Lu]

On Thu, Jul 26, 2012 at 09:43:37AM -0400, Jeff Garzik wrote:
> On 07/26/2012 06:05 AM, Aaron Lu wrote:
> >v3:
> >Rebase on top of scsi-misc tree;
> >Add the sr related patches previously in Jeff's libata tree;
> >Re-organize the sr patches.
> >A problem for now: for patch
> >scsi: sr: support zero power ODD(ZPODD)
> >I can't set a flag in libata-acpi.c since a related function is
> >missing in scsi-misc tree. Will fix this when 3.6-rc1 released.
> 
> What does this mean?  Would you be more specific?

The patch "libata-acpi: add ata port runtime D3Cold support" by Lin Ming
introduced a function ata_acpi_wake_dev in libata-acpi.c, and only lives
in libata-next tree but not scsi-misc tree.

In patch "scsi: sr: support zero power ODD(ZPODD)" I need to set the
wakeup_by_user flag of the scsi device in ata_acpi_wake_dev like this:

--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -985,8 +985,10 @@ static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
 	struct ata_device *ata_dev = context;
 
 	if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev &&
-			pm_runtime_suspended(&ata_dev->sdev->sdev_gendev))
+			pm_runtime_suspended(&ata_dev->sdev->sdev_gendev)) {
+		ata_dev->sdev->wakeup_by_user = 1;
 		scsi_autopm_get_device(ata_dev->sdev);
+	}
 }
 
But since there is no such function in scsi-misc tree, I can't set the
flag. Any suggestions?

Another minor issue is, I need to use the can_power_off and
wakeup_by_user flag of the scsi_device structure in sr patches, but
they are all introduced in patches in libata-next tree, so I have to
re-define them in this patch set. Will cause conflict if James send
these sr patches to Linus. Any way to avoid this?

Thanks,
Aaron

Re: [PATCH v3 0/7] ZPODD patches for scsi tree

[Jeff Garzik]

On 07/26/2012 10:41 AM, Aaron Lu wrote:
> On Thu, Jul 26, 2012 at 09:43:37AM -0400, Jeff Garzik wrote:
>> On 07/26/2012 06:05 AM, Aaron Lu wrote:
>>> I can't set a flag in libata-acpi.c since a related function is
>>> missing in scsi-misc tree. Will fix this when 3.6-rc1 released.

>> What does this mean?  Would you be more specific?

> The patch "libata-acpi: add ata port runtime D3Cold support" by Lin Ming
> introduced a function ata_acpi_wake_dev in libata-acpi.c, and only lives
> in libata-next tree but not scsi-misc tree.
[...]
> Another minor issue is, I need to use the can_power_off and
> wakeup_by_user flag of the scsi_device structure in sr patches, but
> they are all introduced in patches in libata-next tree, so I have to
> re-define them in this patch set. Will cause conflict if James send
> these sr patches to Linus. Any way to avoid this?


Linus said he just merged the libata patches, so they should appear in 
the torvalds/linux.git as soon as he pushes out (in the next 12 hours, 
I'm guessing).

Up to James how he wants to coordinate after that...  he might pull in 
Linus's tree into scsi-misc or another solution.

	Jeff