[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080814155649.GA6046@elf.ucw.cz>
Date: Thu, 14 Aug 2008 17:56:49 +0200
From: Pavel Machek <pavel@...e.cz>
To: Alan Stern <stern@...land.harvard.edu>
Cc: kernel list <linux-kernel@...r.kernel.org>,
Linux-pm mailing list <linux-pm@...ts.osdl.org>,
James.Bottomley@...senPartnership.com, teheo@...ell.com,
oneukum@...e.de
Subject: Re: Power management for SCSI
Hi!
> > > Add support for autosuspend/autoresume. Lowlevel driver can use it to
> > > spin the disk down and power down its SATA link, to turn off the USB
> > > interface, etc.
> > >
> > > Spinning down the disk is useful - saves ~0.5W here. Powering down
> > > SATA controller is even better -- should save ~1W.
> > >
> > > Now, I guess the patch will need to be split to small pieces for
> > > merge... I tried to rearrange it so that the documentation and hooks
> > > go before stuff that needs the hooks, and before Kconfig enabler. If
> > > it looks reasonably good, I'll split it into smaller pieces.
> >
> > James had a number of objections to my original patch; you can read
> > them here:
> >
> > https://lists.linux-foundation.org/pipermail/linux-pm/2008-March/016849.html
> >
> > I haven't had time yet to work on an improved version.
>
> Ok, I see, "its done at the wrong level" sounds pretty serious.
>
> First the general comments/questions:
...
> #2. As you say in the comment, the thing we're trying to power down is
> #the link. In most SCSI implementations, the link has a rather complex
> #relationship to the target, what we want to do in
> #periodic_autosuspend_scan() is run over the devices on each link, and
> #if
> #they're not busy suspend the link? What's probably needed is a set of
> #adjunct helpers for the transport classes to do this.
>
> So the host suspend/resume stuff should go into struct
> scsi_transport_template?
Is this step in the right direction? Moved autosuspend from
scsi_host_template to scsi_transport_template...
Pavel
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index e4864d9..2b8cf09 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -320,14 +320,14 @@ static struct device_attribute *ahci_sde
};
struct pci_dev *my_pdev;
-int autosuspend_enabled = 0; /* HERE */
+int autosuspend_enabled = 1; /* HERE */
struct sleep_disabled_reason ahci_active = {
"ahci"
};
/* The host and its devices are all idle so we can autosuspend */
-static int autosuspend(struct Scsi_Host *host)
+int ahci_autosuspend(struct Scsi_Host *host)
{
if (my_pdev && autosuspend_enabled) {
printk("ahci: should autosuspend\n");
@@ -340,7 +340,7 @@ static int autosuspend(struct Scsi_Host
}
/* The host needs to be autoresumed */
-static int autoresume(struct Scsi_Host *host)
+int ahci_autoresume(struct Scsi_Host *host)
{
if (my_pdev && autosuspend_enabled) {
printk("ahci: should autoresume\n");
@@ -360,8 +360,8 @@ static struct scsi_host_template ahci_sh
.sg_tablesize = AHCI_MAX_SG,
.dma_boundary = AHCI_DMA_BOUNDARY,
.shost_attrs = ahci_shost_attrs,
- .autosuspend = autosuspend,
- .autoresume = autoresume,
+// .autosuspend = autosuspend,
+// .autoresume = autoresume,
.sdev_attrs = ahci_sdev_attrs,
};
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b9d3ba4..d3526a0 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -103,6 +103,9 @@ static const u8 def_control_mpage[CONTRO
0, 30 /* extended self test time, see 05-359r1 */
};
+int ahci_autosuspend(struct Scsi_Host *host);
+int ahci_autoresume(struct Scsi_Host *host);
+
/*
* libata transport template. libata doesn't do real transport stuff.
* It just needs the eh_timed_out hook.
@@ -111,6 +114,8 @@ static struct scsi_transport_template at
.eh_strategy_handler = ata_scsi_error,
.eh_timed_out = ata_scsi_timed_out,
.user_scan = ata_scsi_user_scan,
+ .autosuspend = ahci_autosuspend,
+ .autoresume = ahci_autoresume,
};
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 5ef69c4..d2de371 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -10,6 +10,7 @@ #define DEBUG
#include <scsi/scsi.h>
#include <scsi/scsi_device.h>
#include <scsi/scsi_host.h>
+#include <scsi/scsi_transport.h>
#include <linux/delay.h>
@@ -50,8 +51,8 @@ void scsi_autosuspend_host(struct Scsi_H
if (shost->pm_usage_cnt <= 0 && !shost->is_suspended &&
shost->shost_state == SHOST_RUNNING) {
WARN_ON(shost->host_busy);
- if (!shost->hostt->autosuspend ||
- shost->hostt->autosuspend(shost) == 0) {
+ if (!shost->transportt->autosuspend ||
+ shost->transportt->autosuspend(shost) == 0) {
shost->is_suspended = 1;
shost_dbg(shost, "suspended\n");
}
@@ -82,10 +83,10 @@ int scsi_autoresume_host(struct Scsi_Hos
mutex_lock(&shost->pm_mutex);
++shost->pm_usage_cnt;
if (shost->is_suspended) {
- if (shost->hostt->autoresume &&
+ if (shost->transportt->autoresume &&
(shost->shost_state == SHOST_RUNNING ||
shost->shost_state == SHOST_RECOVERY))
- status = shost->hostt->autoresume(shost);
+ status = shost->transportt->autoresume(shost);
if (status == 0) {
shost->is_suspended = 0;
shost_dbg(shost, "resumed\n");
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index ef64fd8..c96f11f 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -499,10 +499,6 @@ struct scsi_host_template usb_stor_host_
.eh_device_reset_handler = device_reset,
.eh_bus_reset_handler = bus_reset,
- /* dynamic power management */
- .autosuspend = autosuspend,
- .autoresume = autoresume,
-
/* queue commands only, only one command per LUN */
.can_queue = 1,
.cmd_per_lun = 1,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index b60445f..0f30451 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -176,21 +176,6 @@ #endif
int (* eh_bus_reset_handler)(struct scsi_cmnd *);
int (* eh_host_reset_handler)(struct scsi_cmnd *);
- /*
- * Power management routines. These are optional; you should
- * implement them if you want your LLD to perform dynamic Power
- * Management. The autosuspend method will be called whenever
- * all the devices below a host have been suspended (are in an
- * idle state), at which time the host adapter can safely be
- * autosuspended. The autoresume method will be called whenever
- * a suspended host must be resumed for one of its devices to
- * carry out a command. Both routines are always called in a
- * process context with interrupts enabled.
- *
- * Status: OPTIONAL
- */
- int (* autosuspend)(struct Scsi_Host *);
- int (* autoresume)(struct Scsi_Host *);
/*
* Before the mid layer attempts to scan for a new device where none
diff --git a/include/scsi/scsi_transport.h b/include/scsi/scsi_transport.h
index 490bd13..15c7886 100644
--- a/include/scsi/scsi_transport.h
+++ b/include/scsi/scsi_transport.h
@@ -77,6 +77,22 @@ struct scsi_transport_template {
* request for target drivers.
*/
int (* tsk_mgmt_response)(struct Scsi_Host *, u64, u64, int);
+
+ /*
+ * Power management routines. These are optional; you should
+ * implement them if you want your LLD to perform dynamic Power
+ * Management. The autosuspend method will be called whenever
+ * all the devices below a host have been suspended (are in an
+ * idle state), at which time the host adapter can safely be
+ * autosuspended. The autoresume method will be called whenever
+ * a suspended host must be resumed for one of its devices to
+ * carry out a command. Both routines are always called in a
+ * process context with interrupts enabled.
+ *
+ * Status: OPTIONAL
+ */
+ int (* autosuspend)(struct Scsi_Host *);
+ int (* autoresume)(struct Scsi_Host *);
};
#define transport_class_to_shost(tc) \
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists