Patchwork [v2,4/4] ata: add ata port runtime PM callbacks

login
register
mail settings
Submitter Lin Ming
Date Nov. 10, 2011, 6:22 a.m.
Message ID <1320906166-15459-5-git-send-email-ming.m.lin@intel.com>
Download mbox | patch
Permalink /patch/124790/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Lin Ming - Nov. 10, 2011, 6:22 a.m.
Add ata port runtime suspend/resume/idle callbacks.
Set ->eh_noresume to skip the runtime PM calls on scsi host
in the error handler to avoid dead lock.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/ata/libata-core.c      |   19 +++++++++++++++++++
 drivers/ata/libata-scsi.c      |    1 +
 drivers/ata/libata-transport.c |    3 +++
 3 files changed, 23 insertions(+), 0 deletions(-)
Alan Stern - Nov. 10, 2011, 3:40 p.m.
On Thu, 10 Nov 2011, Lin Ming wrote:

> Add ata port runtime suspend/resume/idle callbacks.
> Set ->eh_noresume to skip the runtime PM calls on scsi host
> in the error handler to avoid dead lock.

...

> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3377,6 +3377,7 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
>  		if (!shost)
>  			goto err_alloc;
>  
> +		shost->eh_noresume = 1;
>  		*(struct ata_port **)&shost->hostdata[0] = ap;
>  		ap->scsi_host = shost;

Are you really sure you want to keep this flag set throughout the 
lifetime of the port?

What happens if a command fails, then the port suspends, and then the 
error handler is invoked to take care of the failed command?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Nov. 10, 2011, 3:45 p.m.
On Thu, Nov 10, 2011 at 10:40:21AM -0500, Alan Stern wrote:
> Are you really sure you want to keep this flag set throughout the 
> lifetime of the port?
> 
> What happens if a command fails, then the port suspends, and then the 
> error handler is invoked to take care of the failed command?

Runtime PM doesn't happen if the device is in use in any way, so
there's no request flying in from upper layers.  While the port is
suspended, qc's which which requested EH are short-circuited.  AFAICS,
nothing is really wrong.

Thanks.

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index b77c40c..719d23c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -66,6 +66,7 @@ 
 #include <asm/byteorder.h>
 #include <linux/cdrom.h>
 #include <linux/ratelimit.h>
+#include <linux/pm_runtime.h>
 
 #include "libata.h"
 #include "libata-transport.h"
@@ -5302,9 +5303,27 @@  static int ata_port_resume(struct device *dev)
 	return rc;
 }
 
+static int ata_port_runtime_suspend(struct device *dev)
+{
+	struct ata_port *ap = to_ata_port(dev);
+	int rc;
+
+	rc = ata_port_request_pm(ap, PMSG_SUSPEND, 0, ATA_EHI_QUIET, 1);
+	return rc;
+}
+
+static int ata_port_runtime_idle(struct device *dev)
+{
+	return pm_runtime_suspend(dev);
+}
+
 static const struct dev_pm_ops ata_port_pm_ops = {
 	.suspend = ata_port_suspend,
 	.resume = ata_port_resume,
+
+	.runtime_suspend = ata_port_runtime_suspend,
+	.runtime_resume = ata_port_resume,
+	.runtime_idle = ata_port_runtime_idle,
 };
 
 /**
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 4c2a524..f7afdd5 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3377,6 +3377,7 @@  int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 		if (!shost)
 			goto err_alloc;
 
+		shost->eh_noresume = 1;
 		*(struct ata_port **)&shost->hostdata[0] = ap;
 		ap->scsi_host = shost;
 
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index a979bd76..9a7f0ea 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -291,6 +291,9 @@  int ata_tport_add(struct device *parent,
 		goto tport_err;
 	}
 
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
 	transport_add_device(dev);
 	transport_configure_device(dev);