[PATCH v2 0/7] Fix for ZPODD

[PATCH v2 0/7] Fix for ZPODD

[Aaron Lu]

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: fix for sr suspend and resume
  scsi: pm: use autosuspend if device supports it
  scsi: sr: use scsi layer autopm in sr driver
  scsi: sr: runtime pm when ODD is open/closed
  scsi: sr: block events when runtime suspended
  scsi: pm: use runtime resume callback if available
  block: genhd: add an interface to set disk's poll interval

 block/genhd.c          | 25 ++++++++++++++++------
 drivers/scsi/scsi_pm.c | 22 +++++++++++++------
 drivers/scsi/sr.c      | 57 ++++++++++++++++++++++++++++++++------------------
 include/linux/genhd.h  |  1 +
 4 files changed, 73 insertions(+), 32 deletions(-)

-- 
1.7.11.3

[PATCH v2 1/7] scsi: sr: fix for sr suspend and resume

[Aaron Lu]

In sr_suspend, we do not need to do anything if it is not a runtime pm
request, so just return by checking the PM_IS_AUTO macro.
And in sr_resume, only reset the suspend_count back to 1 if the ODD is
waken up by the user, or the usage count of the scsi device will not
balance.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
V2:
Use PMSG_IS_AUTO as suggested by Alan Stern.

 drivers/scsi/sr.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 3da0879..225848c 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -178,8 +178,8 @@ static int sr_suspend(struct device *dev, pm_message_t msg)
 	struct scsi_sense_hdr sshdr;
 	struct scsi_cd *cd = dev_get_drvdata(dev);
 
-	/* no action for system suspend */
-	if (msg.event == PM_EVENT_SUSPEND)
+	/* no action for system pm operations */
+	if (!PMSG_IS_AUTO(msg))
 		return 0;
 
 	/* do another TUR to see if the ODD is still ready to be powered off */
@@ -217,9 +217,9 @@ static int sr_resume(struct device *dev)
 		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);
+		atomic_set(&cd->suspend_count, 1);
+	}
 
 	return 0;
 }
-- 
1.7.11.3

[PATCH v2 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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index d4201de..a002fbf 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -196,7 +196,12 @@ 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

[PATCH v2 3/7] scsi: sr: use scsi layer autopm in sr driver

[Aaron Lu]

Use scsi_autopm_* whenever possible instead of the pm_runtime_* in sr
driver as suggested by Alan Stern.

The reason pm_runtime_get_noresume in sr_suspend is used is that we
can't do sync resume call in suspend callback, or rpm_resume will wait
forever for the suspend request to finish.

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
V2:
Use scsi_autopm_* instead of pm_runtime_* in sr driver as suggested
by Alan Stern.

 drivers/scsi/sr.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 225848c..e54945a 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -377,10 +377,8 @@ do_tur:
 		 * 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)) {
-			pm_runtime_mark_last_busy(&cd->device->sdev_gendev);
-			pm_runtime_put_autosuspend(&cd->device->sdev_gendev);
-		}
+		if (poweroff && atomic_add_unless(&cd->suspend_count, -1, 0))
+			scsi_autopm_put_device(cd->device);
 	}
 
 	if (cd->ignore_get_event)
@@ -655,11 +653,8 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	int ret;
 
 	/* Make sure the ODD is not suspended */
-	ret = pm_runtime_get_sync(&sdev->sdev_gendev);
-	if (ret < 0) {
-		pm_runtime_put_noidle(&sdev->sdev_gendev);
+	if (scsi_autopm_get_device(sdev))
 		return -EACCES;
-	}
 
 	mutex_lock(&sr_mutex);
 
@@ -692,8 +687,7 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 
 out:
 	mutex_unlock(&sr_mutex);
-	pm_runtime_mark_last_busy(&cd->device->sdev_gendev);
-	pm_runtime_put_autosuspend(&cd->device->sdev_gendev);
+	scsi_autopm_put_device(sdev);
 	return ret;
 }
 
@@ -1121,7 +1115,7 @@ static int sr_remove(struct device *dev)
 
 	/* disable runtime pm and possibly resume the device */
 	if (!atomic_dec_and_test(&cd->suspend_count))
-		pm_runtime_get_sync(dev);
+		scsi_autopm_get_device(cd->device);
 
 	blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
 	del_gendisk(cd->disk);
-- 
1.7.11.3

[PATCH v2 4/7] scsi: sr: runtime pm when ODD is open/closed

[Aaron Lu]

The ODD can either be runtime resumed by the user or by a software
request. And for the latter part, we only support runtime resume the ODD
when the eject request is received. We did this in sr's block ioctl
function, this looks ugly.

Change this by runtime resuming the ODD in its open function and runtime
suspending it in its release function.

The downside of this approach is that in old distros with old udisk
daemon, the ODD will be polled by udisk daemon so open/close will
constantly be called, which will cause the ODD frequently resume from
suspend state, breaking the effect of power saving. User with such a
distro is advised to issue a udisk command to inhibit polling of the
disk like this:
$ udisks --inhibit-polling /dev/sr0
And since newer kernel has in kernel polling, there is no problem
regarding ODD's event report.

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

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index e54945a..2aa50c3 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -152,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;
@@ -169,6 +173,7 @@ 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);
 }
 
@@ -652,10 +657,6 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	void __user *argp = (void __user *)arg;
 	int ret;
 
-	/* Make sure the ODD is not suspended */
-	if (scsi_autopm_get_device(sdev))
-		return -EACCES;
-
 	mutex_lock(&sr_mutex);
 
 	/*
@@ -687,7 +688,6 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 
 out:
 	mutex_unlock(&sr_mutex);
-	scsi_autopm_put_device(sdev);
 	return ret;
 }
 
-- 
1.7.11.3

[PATCH v2 5/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 2aa50c3..d6c0574 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;
 }
 
@@ -226,6 +228,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 v2 6/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 instead of the resume
callback.

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

Signed-off-by: Aaron Lu <aaron.lu@amd.com>
---
V2:
Check if the resume callback is NULL before calling it as pointed by
Sergei Shtylyov.

 drivers/scsi/scsi_pm.c | 15 ++++++++++-----
 drivers/scsi/sr.c      | 21 +++++++++++++++++++++
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index a002fbf..717d2c1 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;
@@ -84,7 +89,7 @@ static int scsi_bus_resume_common(struct device *dev)
 		 * Resume it on behalf of child.
 		 */
 		pm_runtime_get_sync(dev->parent);
-		err = scsi_dev_type_resume(dev);
+		err = scsi_dev_type_resume(dev, false);
 		pm_runtime_put_sync(dev->parent);
 	}
 
@@ -159,7 +164,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 */
 
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index d6c0574..466ff8e 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

[PATCH v2 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 466ff8e..d0c3084 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -863,6 +863,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