[PATCH 0/3] may_power_off for scsi device

[PATCH 0/3] may_power_off for scsi device

[Aaron Lu]

Add a new flag may_power_off for scsi device to let user control when
it is runtime suspended, can we remove its power.

This patchset makes the following patch sent previously deprecated:
[PATCH] scsi: sr: add may_power_off flag
http://marc.info/?l=linux-scsi&m=134614323832520&w=2

The patch "scsi: pm: add may_power_off flag" also appears in another
patchset intended for libata due to libata will use this flag to
decide if ACPI D3cold can be used. They two patches are identical.

Based on top of the previously sent v4 ZPODD patchset:
https://lkml.org/lkml/2012/7/27/71

Aaron Lu (3):
  scsi: pm: add may_power_off flag
  scsi: sr: use may_power_off to enable/disable ZPODD
  scsi: sd: add may_power_off sysfs entry

 drivers/scsi/sd.c          | 34 ++++++++++++++++++++++++++++
 drivers/scsi/sr.c          | 55 +++++++++++++++++++++++++++++++++++++++++-----
 include/scsi/scsi_device.h |  1 +
 3 files changed, 84 insertions(+), 6 deletions(-)

-- 
1.7.11.5

[PATCH 1/3] scsi: pm: add may_power_off flag

[Aaron Lu]

Add a new flag may_power_off for scsi device, it gives the user a chance
to control when the device is runtime suspended, can we remove its power
if possible.

I'm planning using this flag for sr and sd.

For sr, if user set 0 to may_power_off, we will disable runtime suspend
for ODD, since runtime suspend for ODD is wholly for zero power ODD, if
we can't power off the ODD, there is no meaning to do runtime PM on it.

For sd, if user set 0 to may_power_off, we will not power off the device
when it is runtime suspended.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 include/scsi/scsi_device.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 4bc4ac4..0bfc17f 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -157,6 +157,7 @@ struct scsi_device {
 	unsigned can_power_off:1; /* Device supports runtime power off */
 	unsigned wakeup_by_user:1;	/* User wakes up the ODD */
 	unsigned wce_default_on:1;	/* Cache is ON by default */
+	unsigned may_power_off:1;	/* power off is allowed by user */
 
 	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
 	struct list_head event_list;	/* asserted events */
-- 
1.7.11.5

[PATCH 2/3] scsi: sr: use may_power_off to enable/disable ZPODD

[Aaron Lu]

In case user's system is broken with ZPODD(either platform or device),
we use this flag to let user disable ZPODD.

A sysfs entry may_power_off is used as the interface to show/store
the scsi device's may_power_off flag.

If 0, we will disable ZPODD; If 1, we will re-enable ZPODD.

The sysfs entry for may_power_off only appears for those ZPODD
capable system(platform + device), and may_power_off implies
can_power_off.

By default, we set may_power_off to 1.

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

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index e6e5549..bc9df62 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -158,7 +158,7 @@ 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 (cd->device->can_power_off && scsi_autopm_get_device(cd->device))
+	if (scsi_autopm_get_device(cd->device))
 		goto out_pm;
 	goto out;
 
@@ -179,11 +179,50 @@ 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);
-	if (sdev->can_power_off)
-		scsi_autopm_put_device_autosuspend(sdev);
+	scsi_autopm_put_device_autosuspend(sdev);
 	mutex_unlock(&sr_ref_mutex);
 }
 
+static ssize_t
+may_power_off_show(struct device *dev, struct device_attribute *attr,
+		   char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	return snprintf(buf, 10, "%d\n", sdev->may_power_off);
+}
+
+static ssize_t
+may_power_off_store(struct device *dev, struct device_attribute *attr,
+		    const char *buf, size_t count)
+{
+	int value = -EINVAL;
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct scsi_cd *cd = dev_get_drvdata(dev);
+
+	if (buf[1] == '\0' || (buf[1] == '\n' && buf[2] == '\0')) {
+		if (buf[0] == '1')
+			value = 1;
+		else if (buf[0] == '0')
+			value = 0;
+	}
+
+	if (value >= 0) {
+		if (sdev->may_power_off != value) {
+			if (value == 0) {
+				if (!atomic_dec_and_test(&cd->suspend_count))
+					scsi_autopm_get_device(cd->device);
+			} else
+				atomic_set(&cd->suspend_count, 1);
+			sdev->may_power_off = value;
+		}
+		value = count;
+	}
+
+	return value;
+}
+DEVICE_ATTR(may_power_off, S_IRUGO | S_IWUSR,
+		may_power_off_show, may_power_off_store);
+
 static int sr_suspend(struct device *dev, pm_message_t msg)
 {
 	int poweroff;
@@ -342,7 +381,7 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 		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)) {
+	if (cd->device->may_power_off && cd->dbml && sr_unit_load_done(cd)) {
 		events = 0;
 		goto do_tur;
 	}
@@ -396,7 +435,7 @@ do_tur:
 		cd->tur_changed = true;
 	}
 
-	if (cd->device->can_power_off && !cd->media_present) {
+	if (cd->device->may_power_off && !cd->media_present) {
 		if (cd->cdi.mask & CDC_CLOSE_TRAY)
 			poweroff = 1;
 		else
@@ -850,6 +889,8 @@ static int sr_probe(struct device *dev)
 		pm_runtime_set_autosuspend_delay(dev, 180 * 1000);
 		pm_runtime_use_autosuspend(dev);
 		atomic_set(&cd->suspend_count, 1);
+		sdev->may_power_off = 1;
+		device_create_file(dev, &dev_attr_may_power_off);
 	}
 
 	disk->driverfs_dev = &sdev->sdev_gendev;
@@ -1138,10 +1179,12 @@ static int sr_remove(struct device *dev)
 	struct scsi_cd *cd = dev_get_drvdata(dev);
 
 	/* disable runtime pm and possibly resume the device */
-	if (cd->device->can_power_off &&
+	if (cd->device->may_power_off &&
 			!atomic_dec_and_test(&cd->suspend_count))
 		scsi_autopm_get_device(cd->device);
 
+	device_remove_file(dev, &dev_attr_may_power_off);
+
 	blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
 	del_gendisk(cd->disk);
 
-- 
1.7.11.5

[PATCH 3/3] scsi: sd: add may_power_off sysfs entry

[Aaron Lu]

If the platform is able to power off the scsi device, add a sysfs entry
may_power_off to let user control the may_power_off flag of the device.

This flag will control when the scsi device is runtime suspended, can we
remove power from it(e.g. let it enter D3 cold ACPI device state).

This flag is used in libata-acpi.c to decide which ACPI D-State it will
enter when runtime suspended.

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

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 4df73e5..db25569 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2584,6 +2584,38 @@ static int sd_format_disk_name(char *prefix, int index, char *buf, int buflen)
 	return 0;
 }
 
+static ssize_t
+may_power_off_show(struct device *dev, struct device_attribute *attr,
+		   char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	return snprintf(buf, 10, "%d\n", sdev->may_power_off);
+}
+
+static ssize_t
+may_power_off_store(struct device *dev, struct device_attribute *attr,
+		    const char *buf, size_t count)
+{
+	int value = -EINVAL;
+	struct scsi_device *sdev = to_scsi_device(dev);
+
+	if (buf[1] == '\0' || (buf[1] == '\n' && buf[2] == '\0')) {
+		if (buf[0] == '1')
+			value = 1;
+		else if (buf[0] == '0')
+			value = 0;
+	}
+
+	if (value >= 0) {
+		sdev->may_power_off = value;
+		value = count;
+	}
+
+	return value;
+}
+DEVICE_ATTR(may_power_off, S_IRUGO | S_IWUSR,
+		may_power_off_show, may_power_off_store);
+
 /*
  * The asynchronous part of sd_probe
  */
@@ -2638,6 +2670,8 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 
 	sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
 		  sdp->removable ? "removable " : "");
+	if (sdp->can_power_off)
+		device_create_file(&sdp->sdev_gendev, &dev_attr_may_power_off);
 	scsi_autopm_put_device(sdp);
 	put_device(&sdkp->dev);
 }
-- 
1.7.11.5

Re: [PATCH 1/3] scsi: pm: add may_power_off flag

[Alan Stern]

On Thu, 30 Aug 2012, Aaron Lu wrote:

> Add a new flag may_power_off for scsi device, it gives the user a chance
> to control when the device is runtime suspended, can we remove its power
> if possible.
> 
> I'm planning using this flag for sr and sd.
> 
> For sr, if user set 0 to may_power_off, we will disable runtime suspend
> for ODD, since runtime suspend for ODD is wholly for zero power ODD, if
> we can't power off the ODD, there is no meaning to do runtime PM on it.

This doesn't seem like a good idea.  Even though you may not be able to
reduce the drive's power usage, you can still allow the runtime_suspend
routine to succeed.  This will tell the kernel to consider the drive as
being in a suspended (and therefore unusable) state, allowing the
kernel to do a runtime suspend of the SCSI host that the drive is
attached to.

Alan Stern

Re: [PATCH 1/3] scsi: pm: add may_power_off flag

[Aaron Lu]

On Thu, Aug 30, 2012 at 10:24:39AM -0400, Alan Stern wrote:
> On Thu, 30 Aug 2012, Aaron Lu wrote:
> 
> > Add a new flag may_power_off for scsi device, it gives the user a chance
> > to control when the device is runtime suspended, can we remove its power
> > if possible.
> > 
> > I'm planning using this flag for sr and sd.
> > 
> > For sr, if user set 0 to may_power_off, we will disable runtime suspend
> > for ODD, since runtime suspend for ODD is wholly for zero power ODD, if
> > we can't power off the ODD, there is no meaning to do runtime PM on it.
> 
> This doesn't seem like a good idea.  Even though you may not be able to
> reduce the drive's power usage, you can still allow the runtime_suspend
> routine to succeed.  This will tell the kernel to consider the drive as
> being in a suspended (and therefore unusable) state, allowing the
> kernel to do a runtime suspend of the SCSI host that the drive is
> attached to.

Thanks Alan, I agree that this doesn't quite follow the meaning of
may_power_off.

I tried to seperate runtime pm with runtime power off in v5 patch of
ZPODD, please take a look to see if I did it the right way.

Thanks,
Aaron