[PATCH 04/12] libata: implement ata_all_ports list

[PATCH 04/12] libata: implement ata_all_ports list

[Tejun Heo]

Implement ata_all_ports list.  All active ata_ports are linked on this
list.  This will be used to dynamically modify libata-wide
configurations.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   13 +++++++++++++
 include/linux/libata.h     |    2 ++
 2 files changed, 15 insertions(+), 0 deletions(-)

a962488760934f639dd202793694339afd2db1cf
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 61db4f5..1477d32 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -70,6 +70,9 @@ static unsigned int ata_dev_init_params(
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
+static DEFINE_MUTEX(ata_all_ports_mutex);
+static LIST_HEAD(ata_all_ports);
+
 static unsigned int ata_unique_id = 1;
 static struct workqueue_struct *ata_wq;
 
@@ -5464,6 +5467,11 @@ int ata_device_add(const struct ata_prob
 
 		ap = host_set->ports[i];
 
+		/* this port is active now, add it to all_ports */
+		mutex_lock(&ata_all_ports_mutex);
+		list_add_tail(&ap->all_ports_entry, &ata_all_ports);
+		mutex_unlock(&ata_all_ports_mutex);
+
 		/* init sata_spd_limit to the current value */
 		if (sata_scr_read(ap, SCR_CONTROL, &scontrol) == 0) {
 			int spd = (scontrol >> 4) & 0xf;
@@ -5594,6 +5602,11 @@ void ata_port_detach(struct ata_port *ap
 	cancel_delayed_work(&ap->hotplug_task);
 	flush_workqueue(ata_aux_wq);
 
+	/* this port is dead now, remove from all_ports */
+	mutex_lock(&ata_all_ports_mutex);
+	list_del_init(&ap->all_ports_entry);
+	mutex_unlock(&ata_all_ports_mutex);
+
 	/* remove the associated SCSI host */
 	scsi_remove_host(ap->host);
 }
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 66c3100..c9ed035 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -555,6 +555,8 @@ struct ata_port {
 	pm_message_t		pm_mesg;
 	int			*pm_result;
 
+	struct list_head	all_ports_entry;
+
 	void			*private_data;
 
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
-- 
1.3.2

[PATCHSET] libata: implement runtime link powersave

[Tejun Heo]

Hello, all.

This is the first take of powersave patchset.  It implements runtime
link-level power management.  To avoid confusion with regular
device/controller power management (suspend/resume), it's callsed
powersave.  It primarily implements partial/slumber/phy-off power
states SATA standard specifies but if LLD has different way of
implementing link powersave, it's allowed to.  Both host-initiated and
device-initiated operations are supported.

This patchset contains the following patches.

#01-07	: misc fixes/preps for powersave
#08-10	: implements core powersave infrastructure
#11-12	: implement powersave feature for ahci and sata_sil24

Powersave can be configured at boot time or while running by writing
to kernel module parameter /sys/module/libata/parameters/powersave.
The following powersave modes are supported.

* none		: no powersave, link is powered up all the time
* HIPS		: host-initiated powersave
* DIPS		: device-initiated powersave
* static	: no powersave on occupied link, power off empty link
* HIPS/static	: HIPS on occupied link, power off empty link
* DIPS/static	: DIPS on occupied link, power off empty link

For more information, please read head messages of individual patches.

All supported modes are tested on ICH7R AHCI, ICH6M AHCI, SiI3124,
SiI3132.  Unfortunately SiI3112/4 family controllers don't support
HIPS and choke on DIPS and thus dropped from supported list.

Here are some results on my ICH6M which can only do static PS.  Each
entry is average of 10 battery discharge rate readings 10 secs apart.

 none       static     diff  pct
---------------------------------
 1140.1mA / 1222.7mA 82.6mA 6.8%
 1145.4mA / 1209.8mA 64.4mA 5.3%
 1148.6mA / 1209.9mA 61.3mA 5.0%
 1150.8mA / 1217.2mA 66.4mA 5.5%
 l153.0mA / 1220.5mA 67.5mA 5.5%

I'd really like to see how much power DIPS/HIPS can save.  If you have
a notebook with smarter ICH, please don't hesitate to benchmark.

This patchset is against.

  upstream (309bade002e9226781c2d7a015340d0089e399b5)
+ [1] fix-autopsy patch
+ [2] fix-eh_skip_recovery patch
+ [3] improve-EH-actions-and-EHI-handling patch
+ [4] PHY-reset-requires-writing-0x4-to-SControl

This patchset is also available in the following git tree.

 http://htj.dyndns.org/git/?p=libata-tj.git;a=shortlog;h=powersave
 git://htj.dyndns.org/libata-tj powersave

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/11966
[2] http://article.gmane.org/gmane.linux.ide/12001
[3] http://article.gmane.org/gmane.linux.ide/12002
[4] http://article.gmane.org/gmane.linux.ide/11870

[PATCH 02/12] libata: add ata_id_has_sata() and use it in ata_id_has_ncq()

[Tejun Heo]

If word 76 is 0 or 0xffff, all SATA IDENTIFY words are invalid.  Add
ata_id_has_sata() for this test and use it in ata_id_has_ncq().

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 include/linux/ata.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

bd18162e08a1508a802a188d6cec69ac3cd5bc8b
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 3671af8..a7798c3 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -286,7 +286,8 @@ #define ata_id_has_wcache(id)	((id)[82] 
 #define ata_id_has_pm(id)	((id)[82] & (1 << 3))
 #define ata_id_has_lba(id)	((id)[49] & (1 << 9))
 #define ata_id_has_dma(id)	((id)[49] & (1 << 8))
-#define ata_id_has_ncq(id)	((id)[76] & (1 << 8))
+#define ata_id_has_sata(id)	((id)[76] && (id)[76] != 0xffff)
+#define ata_id_has_ncq(id)	(ata_id_has_sata(id) && ((id)[76] & (1 << 8)))
 #define ata_id_queue_depth(id)	(((id)[75] & 0x1f) + 1)
 #define ata_id_removeable(id)	((id)[0] & (1 << 7))
 #define ata_id_has_dword_io(id)	((id)[50] & (1 << 0))
-- 
1.3.2

[PATCH 01/12] libata: add msec_to_jiffies()

[Tejun Heo]

Add msec_to_jiffies() to drivers/scsi/libata.h and use it in
ata_wait_register().

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    2 +-
 drivers/scsi/libata.h      |    5 +++++
 2 files changed, 6 insertions(+), 1 deletions(-)

017cbbd4749d4f40fca7764281dd8283c562ae4c
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index d8c3bed..61db4f5 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5904,7 +5904,7 @@ u32 ata_wait_register(void __iomem *reg,
 	 * preceding writes reach the controller before starting to
 	 * eat away the timeout.
 	 */
-	timeout = jiffies + (timeout_msec * HZ) / 1000;
+	timeout = jiffies + msec_to_jiffies(timeout_msec);
 
 	while ((tmp & mask) == val && time_before(jiffies, timeout)) {
 		msleep(interval_msec);
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index c325679..e2fa600 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -38,6 +38,11 @@ struct ata_scsi_args {
 	void			(*done)(struct scsi_cmnd *);
 };
 
+static inline unsigned long msec_to_jiffies(unsigned long ms)
+{
+	return (ms * HZ + 999) / 1000;
+}
+
 /* libata-core.c */
 extern struct workqueue_struct *ata_aux_wq;
 extern int atapi_enabled;
-- 
1.3.2

[PATCH 03/12] libata: add more SATA specific constants and macros to ata.h

[Tejun Heo]

Add more SATA specific constants and macros to ata.h.

* SATA SETFEATURES constants
* SControl field selectors and extracting macros
* link powersave feature test macros

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 include/linux/ata.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

cc62d2cee0b3edb867a7cfde35b8383c37900e65
diff --git a/include/linux/ata.h b/include/linux/ata.h
index a7798c3..089a2df 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -184,6 +184,12 @@ enum {
 	SETFEATURES_WC_ON	= 0x02, /* Enable write cache */
 	SETFEATURES_WC_OFF	= 0x82, /* Disable write cache */
 
+	SETFEATURES_SATA_ON	= 0x10, /* Enable SATA feature */
+	SETFEATURES_SATA_OFF	= 0x90, /* Disable SATA feature */
+
+	/* SATA feature nsect values */
+	SETFEATURES_SATA_DIPS	= 0x03,
+
 	/* ATAPI stuff */
 	ATAPI_PKT_DMA		= (1 << 0),
 	ATAPI_DMADIR		= (1 << 2),	/* ATAPI data dir:
@@ -204,6 +210,13 @@ enum {
 	SCR_ACTIVE		= 3,
 	SCR_NOTIFICATION	= 4,
 
+	/* SControl subfields, each field is 4 bit wide */
+	ATA_SCTL_DET		= 0, /* lsb */
+	ATA_SCTL_SPD		= 1,
+	ATA_SCTL_IPM		= 2,
+	ATA_SCTL_SPM		= 3,
+	ATA_SCTL_PMP		= 4,
+
 	/* SError bits */
 	SERR_DATA_RECOVERED	= (1 << 0), /* recovered data error */
 	SERR_COMM_RECOVERED	= (1 << 1), /* recovered comm failure */
@@ -289,6 +302,9 @@ #define ata_id_has_dma(id)	((id)[49] & (
 #define ata_id_has_sata(id)	((id)[76] && (id)[76] != 0xffff)
 #define ata_id_has_ncq(id)	(ata_id_has_sata(id) && ((id)[76] & (1 << 8)))
 #define ata_id_queue_depth(id)	(((id)[75] & 0x1f) + 1)
+#define ata_id_has_hips(id)	(ata_id_has_sata(id) && ((id)[76] & (1 << 9)))
+#define ata_id_has_dips(id)	(ata_id_has_sata(id) && ((id)[78] & (1 << 3)))
+#define ata_id_dips_enabled(id)	(ata_id_has_sata(id) && ((id)[79] & (1 << 3)))
 #define ata_id_removeable(id)	((id)[0] & (1 << 7))
 #define ata_id_has_dword_io(id)	((id)[50] & (1 << 0))
 #define ata_id_u32(id,n)	\
@@ -367,4 +383,9 @@ static inline int lba_48_ok(u64 block, u
 	return ((block + n_block - 1) < ((u64)1 << 48)) && (n_block <= 65536);
 }
 
+static inline u8 ata_scontrol_field(u32 scontrol, int sel)
+{
+	return (scontrol >> (sel * 4)) & 0xf;
+}
+
 #endif /* __LINUX_ATA_H__ */
-- 
1.3.2

[PATCH 05/12] libata: make counting functions global

[Tejun Heo]

Move ata_port_nr_{vacant|enabled}() to libata-core.c and make them
global.  This is in preparation for powersave support.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   20 ++++++++++++++++++++
 drivers/scsi/libata-eh.c   |   20 --------------------
 drivers/scsi/libata.h      |    2 ++
 3 files changed, 22 insertions(+), 20 deletions(-)

633b38c08535057d6ad9f91a563758b6641ed0be
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 1477d32..8e75899 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -100,6 +100,26 @@ MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
 
+int ata_port_nr_vacant(struct ata_port *ap)
+{
+	int i, cnt = 0;
+
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		if (ap->device[i].class == ATA_DEV_UNKNOWN)
+			cnt++;
+	return cnt;
+}
+
+int ata_port_nr_enabled(struct ata_port *ap)
+{
+	int i, cnt = 0;
+
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		if (ata_dev_enabled(&ap->device[i]))
+			cnt++;
+	return cnt;
+}
+
 /**
  *	ata_tf_to_fis - Convert ATA taskfile to SATA FIS structure
  *	@tf: Taskfile to convert
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 29f5934..7c5217a 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -1837,26 +1837,6 @@ static int ata_eh_resume(struct ata_port
 	return 0;
 }
 
-static int ata_port_nr_enabled(struct ata_port *ap)
-{
-	int i, cnt = 0;
-
-	for (i = 0; i < ATA_MAX_DEVICES; i++)
-		if (ata_dev_enabled(&ap->device[i]))
-			cnt++;
-	return cnt;
-}
-
-static int ata_port_nr_vacant(struct ata_port *ap)
-{
-	int i, cnt = 0;
-
-	for (i = 0; i < ATA_MAX_DEVICES; i++)
-		if (ap->device[i].class == ATA_DEV_UNKNOWN)
-			cnt++;
-	return cnt;
-}
-
 static int ata_eh_skip_recovery(struct ata_port *ap)
 {
 	struct ata_eh_context *ehc = &ap->eh_context;
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index e2fa600..d77b5ff 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -48,6 +48,8 @@ extern struct workqueue_struct *ata_aux_
 extern int atapi_enabled;
 extern int atapi_dmadir;
 extern int libata_fua;
+extern int ata_port_nr_vacant(struct ata_port *ap);
+extern int ata_port_nr_enabled(struct ata_port *ap);
 extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev);
 extern int ata_rwcmd_protocol(struct ata_queued_cmd *qc);
 extern void ata_dev_disable(struct ata_device *dev);
-- 
1.3.2

[PATCH 10/12] libata: implement standard powersave methods

[Tejun Heo]

Implement helpers to build SATA ->set_powersave() and HIPS timer, and
use them to implement sata_std_set_powersave() and
sata_std_hips_timer_fn() for standard SATA link powersave using
SControl register.

Depending on controller capability, the following modes are supported.

none:		no powersave
HIPS:		host-initiated partial/slumber (both or either one)
DIPS:		device-initiated partial/slumber (both or either one)
static:		link off by writing 0x4 to DET
HIPS/static:	HIPS + static
DIPS/static;	DIPS + static

Timeouts for HIPS can be modified using module parameters -
libata.partial_timeout and libata.slumber_timeout.  Setting timeout to
zero disables the powersave mode.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |  280 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h     |   10 ++
 2 files changed, 290 insertions(+), 0 deletions(-)

e19c560ab7e8fa9b20ed5ec3233fb75e0daceb63
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 1658cd1..9890387 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -71,6 +71,7 @@ static unsigned int ata_dev_set_xfermode
 static void ata_dev_xfermask(struct ata_device *dev);
 
 static int ata_param_set_powersave(const char *val, struct kernel_param *kp);
+static int ata_param_set_hips_timeout(const char *val, struct kernel_param *kp);
 
 static DEFINE_MUTEX(ata_all_ports_mutex);
 static LIST_HEAD(ata_all_ports);
@@ -102,6 +103,18 @@ module_param_call(powersave, ata_param_s
 MODULE_PARM_DESC(powersave, "Powersave mode (0=none, 1=HIPS, 2=DIPS, "
 		 "3=static, 4=HIPS/static, 5=DIPS/static)");
 
+static unsigned long libata_partial_timeout = 100;
+module_param_call(partial_timeout, ata_param_set_hips_timeout, param_get_ulong,
+		  &libata_partial_timeout, 0644);
+MODULE_PARM_DESC(hips_timeout, "Host-initiated partial powersave timeout "
+		 "(milliseconds, default 100, 0 to disable)");
+
+static unsigned long libata_slumber_timeout = 3000;
+module_param_call(slumber_timeout, ata_param_set_hips_timeout, param_get_ulong,
+		  &libata_slumber_timeout, 0644);
+MODULE_PARM_DESC(slumber_timeout, "Host-initiated slumber powersave timeout "
+		 "(milliseconds, default 3000, 0 to disable)");
+
 MODULE_AUTHOR("Jeff Garzik");
 MODULE_DESCRIPTION("Library module for ATA devices");
 MODULE_LICENSE("GPL");
@@ -2857,6 +2870,217 @@ void ata_std_postreset(struct ata_port *
 	DPRINTK("EXIT\n");
 }
 
+static void sata_std_update_sctl_spm(struct ata_port *ap, u8 sctl_spm,
+				     int may_push_sctl)
+{
+	if (may_push_sctl)
+		sata_update_scontrol_push(ap, ATA_SCTL_SPM, sctl_spm);
+	else
+		sata_update_scontrol(ap, ATA_SCTL_SPM, sctl_spm);
+}
+
+/**
+ *	sata_do_hips_timer_fn - helper to build SATA HIPS timer callback
+ *	@ap: target ATA port
+ *	@seq: current PS sequence
+ *	@sctl_ipm: current SControl IPM
+ *	@update_sctl_spm: update SControl SPM method
+ *
+ *	Implements standard SATA OS-driven host-initiated link
+ *	powersave.  @sctl_ipm is used to determine which powersave
+ *	modes are allowed and @update_sctl_spm is used to actually
+ *	transit powersave mode.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(ap->lock).
+ *
+ *	RETURNS:
+ *	Timeout in jiffies if next PS sequence is needed, 0 otherwise.
+ */
+unsigned long sata_do_hips_timer_fn(struct ata_port *ap, int seq, u8 sctl_ipm,
+				    ata_update_sctl_spm_fn_t update_sctl_spm)
+{
+	unsigned long next_timeout = 0;
+
+	switch (seq) {
+	case 0:
+		if (!(sctl_ipm & 0x1)) {
+			update_sctl_spm(ap, 0x1, 0);
+
+			if (!(sctl_ipm & 0x2))
+				next_timeout = ap->ps_2nd_timeout;
+		} else if (!(sctl_ipm & 0x2))
+			update_sctl_spm(ap, 0x2, 0);
+		break;
+
+	case 1:
+		/* Slumber timeout expired.  Cannot directly transit
+		 * to slumber from partial.  Transit to active first.
+		 */
+		update_sctl_spm(ap, 0x4, 0);
+
+		/* spec says 1ms max, be generous and give it 5 */
+		next_timeout = msec_to_jiffies(5);
+		break;
+
+	case 2:
+		update_sctl_spm(ap, 0x2, 0);
+		break;
+	}
+
+	return next_timeout;
+}
+
+/**
+ *	sata_std_hips_timer_fn - SATA standard powersave HIPS timer callback
+ *	@ap: target ATA port
+ *	@seq: current PS sequence
+ *
+ *	SATA standard powersave HIPS timer callback.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(ap->lock).
+ *
+ *	RETURNS:
+ *	Timeout in jiffies if next PS sequence is needed, 0 otherwise.
+ */
+unsigned long sata_std_hips_timer_fn(struct ata_port *ap, int seq)
+{
+	u8 sctl_ipm = ata_scontrol_field(ap->scontrol, ATA_SCTL_IPM);
+
+	return sata_do_hips_timer_fn(ap, seq, sctl_ipm,
+				     sata_std_update_sctl_spm);
+}
+
+/**
+ *	sata_do_set_powersave - helper to build ->set_powersave method
+ *	@ap: target ATA port
+ *	@ps_state: target powersave state
+ *	@sctl_ipm: SControl IPM value for HIPS/DIPS
+ *	@update_sctl_spm: update SControl SPM method (can be NULL)
+ *
+ *	This function helps building ->set_powersave method for
+ *	controllers with standard SCR registers.
+ *
+ *	LOCKING:
+ *	None (must be called from EH context).
+ */
+void sata_do_set_powersave(struct ata_port *ap, int ps_state, u8 sctl_ipm,
+			   ata_update_sctl_spm_fn_t update_sctl_spm)
+{
+	struct ata_eh_context *ehc = &ap->eh_context;
+	unsigned long flags;
+
+	if (ps_state == ATA_PS_NONE) {
+		/* powersave off */
+		if (ata_ps_dynamic(ap->ps_state)) {
+			/* Make link active.  If host cannot issue PS
+			 * state change, the link can be in PS when
+			 * recovery kicks in, which results in SRST
+			 * failure as link looks offline.  Force
+			 * hardreset in such cases.
+			 */
+			if (update_sctl_spm)
+				update_sctl_spm(ap, 0x4, 1);
+			else if (ehc->i.action & ATA_EH_RESET_MASK)
+				ehc->i.action |= ATA_EH_HARDRESET;
+		}
+		sata_update_scontrol_push(ap, ATA_SCTL_IPM, 0x3);
+		sata_update_scontrol(ap, ATA_SCTL_DET, 0x0);
+		return;
+	}
+
+	/* entering powersave mode */
+	if (ata_ps_static(ps_state) && !ata_port_nr_ready(ap)) {
+		/* no ready device, power down PHY */
+		sata_update_scontrol(ap, ATA_SCTL_DET, 0x4);
+		return;
+	}
+
+	switch (ata_ps_dynamic(ps_state)) {
+	case ATA_PS_NONE:
+		break;
+
+	case ATA_PS_HIPS:
+		sata_update_scontrol(ap, ATA_SCTL_IPM, sctl_ipm);
+
+		if (ap->ps_timer_fn && ap->ps_timeout) {
+			spin_lock_irqsave(ap->lock, flags);
+			ap->pflags |= ATA_PFLAG_PS_TIMER;
+			spin_unlock_irqrestore(ap->lock, flags);
+		}
+		break;
+
+	case ATA_PS_DIPS:
+		sata_update_scontrol(ap, ATA_SCTL_IPM, sctl_ipm);
+		break;
+	}
+}
+
+/**
+ *	sata_determine_hips_params - determine standard HIPS params
+ *	@ap: target ATA port
+ *	@sctl_ipm: I/O argument to constraint and return resulting SControl IPM
+ *
+ *	Determine standard HIPS parameters from two module parameters
+ *	- libata.partial_timeout and libata.slumber_timeout.
+ *	Determined timeout values are stored in ap->ps_[2nd_]timeout
+ *	and SControl IPM value in *@sctl_ipm.
+ *
+ *	LOCKING:
+ *	None.
+ */
+void sata_determine_hips_params(struct ata_port *ap, u8 *sctl_ipm)
+{
+	unsigned long partial_tout = msec_to_jiffies(libata_partial_timeout);
+	unsigned long slumber_tout = msec_to_jiffies(libata_slumber_timeout);
+
+	if (!(*sctl_ipm & 0x1) && partial_tout &&
+	    (!slumber_tout || partial_tout < slumber_tout)) {
+		ap->ps_timeout = partial_tout;
+
+		if (slumber_tout)
+			ap->ps_2nd_timeout = slumber_tout - partial_tout;
+		else
+			*sctl_ipm |= 0x2;
+	} else if (!(*sctl_ipm & 0x2) && slumber_tout) {
+		*sctl_ipm |= 0x1;
+		ap->ps_timeout = slumber_tout;
+	} else {
+		*sctl_ipm |= 0x3;
+		ap->ps_timeout = 0;
+
+		ata_port_printk(ap, KERN_WARNING, "failed to initialize "
+				"HIPS timer, illegal parameters\n");
+	}
+}
+
+/**
+ *	sata_std_set_powersave - standard SATA set_powersave method
+ *	@ap: target ATA port
+ *	@ps_state: target powersave state
+ *
+ *	Standard SATA set_powersave method.
+ *
+ *	LOCKING:
+ *	None (must be called from EH context).
+ */
+void sata_std_set_powersave(struct ata_port *ap, int ps_state)
+{
+	ata_update_sctl_spm_fn_t update_sctl_spm = NULL;
+	u8 sctl_ipm = 0;
+
+	if (ap->flags & ATA_FLAG_HIPS)
+		update_sctl_spm = sata_std_update_sctl_spm;
+
+	if (ata_ps_dynamic(ps_state) == ATA_PS_HIPS) {
+		ap->ps_timer_fn = sata_std_hips_timer_fn;
+		sata_determine_hips_params(ap, &sctl_ipm);
+	}
+
+	sata_do_set_powersave(ap, ps_state, sctl_ipm, update_sctl_spm);
+}
+
 /**
  *	ata_dev_same_device - Determine whether new ID matches configured device
  *	@dev: device to compare against
@@ -5411,6 +5635,57 @@ static int ata_param_set_powersave(const
 }
 
 /**
+ *	ata_param_set_hips_timeout - param_set method for HIPS timeouts
+ *	@val: input value from user
+ *	@kp: kernel_param pointing to libata.(partial|slumber)_timeout
+ *
+ *	This function is invoked when user writes to module parameter
+ *	node /sys/module/libata/parameters/(partial|slumber)_timeout
+ *	and responsible for changing powersave configuration
+ *	accordingly.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ */
+static int ata_param_set_hips_timeout(const char *val, struct kernel_param *kp)
+{
+	unsigned long *timeout = kp->arg;
+	struct kernel_param tkp;
+	struct ata_port *ap;
+	unsigned long new_val;
+	int rc;
+
+	tkp = *kp;
+	tkp.arg = &new_val;
+
+	rc = param_set_ulong(val, &tkp);
+	if (rc)
+		return rc;
+	if (new_val == *timeout)
+		return 0;
+
+	/* timeout updated */
+	*timeout = new_val;
+
+	/* tell EH to update PS configuration */
+	mutex_lock(&ata_all_ports_mutex);
+	list_for_each_entry(ap, &ata_all_ports, all_ports_entry) {
+		unsigned long flags;
+
+		spin_lock_irqsave(ap->lock, flags);
+		ap->eh_info.flags |= ATA_EHI_QUIET;
+		ata_port_schedule_eh(ap);
+		spin_unlock_irqrestore(ap->lock, flags);
+	}
+	mutex_unlock(&ata_all_ports_mutex);
+
+	return 0;
+}
+
+/**
  *	ata_host_remove - Unregister SCSI host structure with upper layers
  *	@ap: Port to unregister
  *	@do_unregister: 1 if we fully unregister, 0 to just stop the port
@@ -6224,6 +6499,11 @@ EXPORT_SYMBOL_GPL(ata_std_prereset);
 EXPORT_SYMBOL_GPL(ata_std_softreset);
 EXPORT_SYMBOL_GPL(sata_std_hardreset);
 EXPORT_SYMBOL_GPL(ata_std_postreset);
+EXPORT_SYMBOL_GPL(sata_do_hips_timer_fn);
+EXPORT_SYMBOL_GPL(sata_std_hips_timer_fn);
+EXPORT_SYMBOL_GPL(sata_do_set_powersave);
+EXPORT_SYMBOL_GPL(sata_determine_hips_params);
+EXPORT_SYMBOL_GPL(sata_std_set_powersave);
 EXPORT_SYMBOL_GPL(ata_dev_revalidate);
 EXPORT_SYMBOL_GPL(ata_dev_classify);
 EXPORT_SYMBOL_GPL(ata_dev_pair);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 0881814..b8bee5f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -337,6 +337,8 @@ typedef int (*ata_prereset_fn_t)(struct 
 typedef int (*ata_reset_fn_t)(struct ata_port *ap, unsigned int *classes);
 typedef void (*ata_postreset_fn_t)(struct ata_port *ap, unsigned int *classes);
 typedef unsigned long (*ata_ps_timer_fn_t)(struct ata_port *ap, int seq);
+typedef void (*ata_update_sctl_spm_fn_t)(struct ata_port *ap, u8 sctl_spm,
+					 int may_push_sctl);
 
 struct ata_ioports {
 	unsigned long		cmd_addr;
@@ -573,6 +575,7 @@ struct ata_port {
 	/* powersave timer, protected by ap->lock */
 	ata_ps_timer_fn_t	ps_timer_fn;	/* initialized by LLD */
 	unsigned long		ps_timeout;	/* ditto */
+	unsigned long		ps_2nd_timeout;	/* owned by LLD */
 
 	int			ps_seq;		/* managed by libata */
 	struct timer_list	ps_timer;	/* ditto */
@@ -699,6 +702,13 @@ extern int ata_std_prereset(struct ata_p
 extern int ata_std_softreset(struct ata_port *ap, unsigned int *classes);
 extern int sata_std_hardreset(struct ata_port *ap, unsigned int *class);
 extern void ata_std_postreset(struct ata_port *ap, unsigned int *classes);
+extern unsigned long sata_do_hips_timer_fn(struct ata_port *ap, int seq,
+			u8 sctl_ipm, ata_update_sctl_spm_fn_t update_sctl_spm);
+extern unsigned long sata_std_hips_timer_fn(struct ata_port *ap, int seq);
+extern void sata_do_set_powersave(struct ata_port *ap, int ps_state,
+			u8 sctl_ipm, ata_update_sctl_spm_fn_t update_sctl_spm);
+extern void sata_determine_hips_params(struct ata_port *ap, u8 *sctl_ipm);
+extern void sata_std_set_powersave(struct ata_port *ap, int ps_state);
 extern int ata_dev_revalidate(struct ata_device *dev, int post_reset);
 extern void ata_port_disable(struct ata_port *);
 extern void ata_std_ports(struct ata_ioports *ioaddr);
-- 
1.3.2

[PATCH 08/12] libata: implement interface power management infrastructure

[Tejun Heo]

Implement interface power management infrastructure.  To discern link
power management from device power management (suspend/resume), link
power management is called 'powersave' or 'PS' while device power
mangement is called 'power management' or 'PM'.

libata PS infrastructure is primarily designed to accomodate SATA link
powersave (link ACTIVE/PARTIAL/SLUMBER) but is not limited to it.
libata implements the following powersave modes.

* none		: no powersave, link is powered up all the time
* HIPS		: host-initiated powersave
* DIPS		: device-initiated powersave
* static	: no powersave on occupied link, power off empty link
* HIPS/static	: HIPS on occupied link, power off empty link
* DIPS/static	: DIPS on occupied link, power off empty link

HIPS/DIPS are called dynamic PS while static is static PS.  LLD can
indicate which dynamic PS modes it supports using ATA_FLAG_HIPS and
ATA_FLAG_DIPS.  Static mode support is mandatory but LLD is free to
implement it as noop.  In fact, if LLD doesn't implement any powersave
feature, libata will automatically handle static PS as noop.

PS mode is disengaged during EH recovery and reenabled on recovery
completion.  Device configuration for DIPS is done by libata EH and
LLD only has to configure the controller when instructed via
->set_powersave() callback.

libata guarantees that there is a reset before changing PS mode.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   78 ++++++++++++++++++++++++++++-
 drivers/scsi/libata-eh.c   |  119 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h     |   33 ++++++++++++
 3 files changed, 228 insertions(+), 2 deletions(-)

4a08bd8677a09fb31619435a5046c9cacd8b73d2
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index cc77bd5..b968b44 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -70,6 +70,8 @@ static unsigned int ata_dev_init_params(
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
+static int ata_param_set_powersave(const char *val, struct kernel_param *kp);
+
 static DEFINE_MUTEX(ata_all_ports_mutex);
 static LIST_HEAD(ata_all_ports);
 
@@ -94,6 +96,12 @@ static int ata_probe_timeout = ATA_TMOUT
 module_param(ata_probe_timeout, int, 0444);
 MODULE_PARM_DESC(ata_probe_timeout, "Set ATA probing timeout (seconds)");
 
+static int libata_powersave = 0;	/* protected by all_ports_mutex */
+module_param_call(powersave, ata_param_set_powersave, param_get_int,
+		  &libata_powersave, 0644);
+MODULE_PARM_DESC(powersave, "Powersave mode (0=none, 1=HIPS, 2=DIPS, "
+		 "3=static, 4=HIPS/static, 5=DIPS/static)");
+
 MODULE_AUTHOR("Jeff Garzik");
 MODULE_DESCRIPTION("Library module for ATA devices");
 MODULE_LICENSE("GPL");
@@ -5258,6 +5266,66 @@ void ata_host_stop (struct ata_host_set 
 		iounmap(host_set->mmio_base);
 }
 
+/**
+ *	ata_param_set_powersave - param_set method for libata.powersave
+ *	@val: input value from user
+ *	@kp: kernel_param pointing to libata.powersave
+ *
+ *	This function is invoked when user writes to module parameter
+ *	node /sys/module/libata/parameters/powersave and responsible
+ *	for changing powersave configuration accordingly.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ */
+static int ata_param_set_powersave(const char *val, struct kernel_param *kp)
+{
+	struct kernel_param tkp;
+	struct ata_port *ap;
+	int new_val, rc;
+
+	tkp = *kp;
+	tkp.arg = &new_val;
+
+	rc = param_set_int(val, &tkp);
+	if (rc)
+		return rc;
+	if (new_val == libata_powersave)
+		return 0;
+	if (new_val < 0 || new_val >= ATA_PS_NR_STATES)
+		return -EINVAL;
+
+	/* powersave state change requested */
+	mutex_lock(&ata_all_ports_mutex);
+
+	list_for_each_entry(ap, &ata_all_ports, all_ports_entry) {
+		unsigned long flags;
+
+		/* set target ps_state and schedule EH */
+		spin_lock_irqsave(ap->lock, flags);
+
+		ap->target_ps_state = new_val;
+
+		ap->eh_info.action |= ATA_EH_SOFTRESET;
+		ap->eh_info.flags |= ATA_EHI_QUIET;
+		ata_port_schedule_eh(ap);
+
+		spin_unlock_irqrestore(ap->lock, flags);
+	}
+
+	/* wait for EH */
+	list_for_each_entry(ap, &ata_all_ports, all_ports_entry)
+		ata_port_wait_eh(ap);
+
+	libata_powersave = new_val;
+
+	mutex_unlock(&ata_all_ports_mutex);
+
+	return 0;
+}
 
 /**
  *	ata_host_remove - Unregister SCSI host structure with upper layers
@@ -5536,9 +5604,13 @@ int ata_device_add(const struct ata_prob
 
 		ap = host_set->ports[i];
 
-		/* this port is active now, add it to all_ports */
+		/* This port is active now, add it to all_ports.
+		 * Initial powersave setting is also configured here
+		 * as it's protected by all_ports_mutex.
+		 */
 		mutex_lock(&ata_all_ports_mutex);
 		list_add_tail(&ap->all_ports_entry, &ata_all_ports);
+		ap->target_ps_state = libata_powersave;
 		mutex_unlock(&ata_all_ports_mutex);
 
 		/* init scontrol and sata_spd_limit to the current value */
@@ -5677,6 +5749,10 @@ void ata_port_detach(struct ata_port *ap
 	cancel_delayed_work(&ap->hotplug_task);
 	flush_workqueue(ata_aux_wq);
 
+	/* turn it off */
+	if (ap->ops->set_powersave)
+		ap->ops->set_powersave(ap, ATA_PS_STATIC);
+
 	/* this port is dead now, remove from all_ports */
 	mutex_lock(&ata_all_ports_mutex);
 	list_del_init(&ap->all_ports_entry);
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 7c5217a..d5126b7 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -1030,7 +1030,8 @@ static void ata_eh_analyze_serror(struct
 		err_mask |= AC_ERR_SYSTEM;
 		action |= ATA_EH_SOFTRESET;
 	}
-	if (serror & (SERR_PHYRDY_CHG | SERR_DEV_XCHG))
+	if ((serror & SERR_DEV_XCHG) ||
+	    (!ata_ps_dynamic(ap->ps_state) && (serror & SERR_PHYRDY_CHG)))
 		ata_ehi_hotplugged(&ehc->i);
 
 	ehc->i.err_mask |= err_mask;
@@ -1837,6 +1838,114 @@ static int ata_eh_resume(struct ata_port
 	return 0;
 }
 
+static unsigned int ata_dev_set_dips(struct ata_device *dev, int on)
+{
+	struct ata_taskfile tf;
+	unsigned int err_mask;
+
+	/* set up set-features taskfile */
+	DPRINTK("set features - SATA DIPS\n");
+
+	ata_tf_init(dev, &tf);
+	tf.command = ATA_CMD_SET_FEATURES;
+	if (on)
+		tf.feature = SETFEATURES_SATA_ON;
+	else
+		tf.feature = SETFEATURES_SATA_OFF;
+	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+	tf.protocol = ATA_PROT_NODATA;
+	tf.nsect = SETFEATURES_SATA_DIPS;
+
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
+
+	DPRINTK("EXIT, err_mask=%x\n", err_mask);
+	return err_mask;
+}
+
+static int ata_eh_set_powersave(struct ata_port *ap,
+				struct ata_device **r_failed_dev)
+{
+	static const char * const ps_strs[] = {
+		[ATA_PS_NONE]		= "none",
+		[ATA_PS_HIPS]		= "HIPS",
+		[ATA_PS_DIPS]		= "DIPS",
+		[ATA_PS_STATIC]		= "static",
+		[ATA_PS_HIPS_STATIC]	= "HIPS/static",
+		[ATA_PS_DIPS_STATIC]	= "DIPS/static",
+	};
+	void (*set_ps)(struct ata_port *, int) = ap->ops->set_powersave;
+	int ps_state, ps_static, ps_dynamic;
+	int dev_dips, dev_hips, i;
+
+	/* power up for recovery */
+	if (r_failed_dev == NULL) {
+		if (ap->ps_state && set_ps)
+			set_ps(ap, ATA_PS_NONE);
+		return 0;
+	}
+
+	/* determine possible ps_state */
+	ps_state = ap->target_ps_state;
+	ps_static = ata_ps_static(ps_state);
+	ps_dynamic = ata_ps_dynamic(ps_state);
+
+	dev_dips = 1;
+	dev_hips = 1;
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *dev = &ap->device[i];
+
+		if (ata_dev_ready(dev) && !ata_id_has_dips(dev->id))
+			dev_dips = 0;
+		if (ata_dev_ready(dev) && !ata_id_has_hips(dev->id))
+			dev_hips = 0;
+	}
+
+	if (ps_dynamic == ATA_PS_DIPS && (!(ap->flags & ATA_FLAG_DIPS) ||
+					  !dev_dips))
+		ps_dynamic--;
+
+	if (ps_dynamic == ATA_PS_HIPS && (!(ap->flags & ATA_FLAG_HIPS) ||
+					  !dev_hips))
+		ps_dynamic--;
+
+	ps_state = ps_static + ps_dynamic;
+
+	/* At this point, we're in ATA_PS_NONE state and DIPS setting
+	 * is unknown.  Execute the requested PS state transition.
+	 */
+	if (ps_state && set_ps)
+		set_ps(ap, ps_state);
+
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		struct ata_device *dev = &ap->device[i];
+		int is_dips = ps_dynamic == ATA_PS_DIPS;
+		unsigned int err_mask;
+
+		if (!ata_dev_ready(dev) || !ata_id_has_dips(dev->id))
+			continue;
+
+		if (!!ata_id_dips_enabled(dev->id) == is_dips)
+			continue;
+
+		err_mask = ata_dev_set_dips(dev, is_dips);
+		if (err_mask) {
+			if (ps_state && set_ps)
+				set_ps(ap, ATA_PS_NONE);
+			*r_failed_dev = dev;
+			return -EIO;
+		}
+	}
+
+	if (ap->ps_state != ps_state) {
+		ata_port_printk(ap, KERN_INFO,
+				"powersave mode changed, %s -> %s\n",
+				ps_strs[ap->ps_state], ps_strs[ps_state]);
+		ap->ps_state = ps_state;
+	}
+
+	return 0;
+}
+
 static int ata_eh_skip_recovery(struct ata_port *ap)
 {
 	struct ata_eh_context *ehc = &ap->eh_context;
@@ -1901,6 +2010,9 @@ static int ata_eh_recover(struct ata_por
 
 	DPRINTK("ENTER\n");
 
+	/* power up for recovery */
+	ata_eh_set_powersave(ap, NULL);
+
 	/* prep for recovery */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		dev = &ap->device[i];
@@ -1978,6 +2090,11 @@ static int ata_eh_recover(struct ata_por
 	if (rc)
 		goto dev_fail;
 
+	/* EH complete, configure powersave mode */
+	rc = ata_eh_set_powersave(ap, &dev);
+	if (rc)
+		goto dev_fail;
+
 	goto out;
 
  dev_fail:
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1656d5b..b3f56c5 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -162,6 +162,8 @@ enum {
 	ATA_FLAG_SKIP_D2H_BSY	= (1 << 12), /* can't wait for the first D2H
 					      * Register FIS clearing BSY */
 	ATA_FLAG_DEBUGMSG	= (1 << 13),
+	ATA_FLAG_HIPS		= (1 << 14), /* SATA host-initiated powersave */
+	ATA_FLAG_DIPS		= (1 << 15), /* SATA dev-initiated powersave */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
@@ -245,6 +247,15 @@ enum {
 	ATA_PORT_PRIMARY	= (1 << 0),
 	ATA_PORT_SECONDARY	= (1 << 1),
 
+	/* powersave constants */
+	ATA_PS_NONE		= 0, /* no powersave */
+	ATA_PS_HIPS		= 1, /* SATA host-initiated powersave */
+	ATA_PS_DIPS		= 2, /* SATA device-initiated powersave */
+	ATA_PS_STATIC		= 3, /* turn off unoccupied PHY */
+	ATA_PS_HIPS_STATIC	= 4, /* HIPS + STATIC */
+	ATA_PS_DIPS_STATIC	= 5, /* DIPS + STATIC */
+	ATA_PS_NR_STATES	= 6,
+
 	/* ering size */
 	ATA_ERING_SIZE		= 32,
 
@@ -554,6 +565,11 @@ struct ata_port {
 	struct list_head	eh_done_q;
 	wait_queue_head_t	eh_wait_q;
 
+	/* powersave (dynamic link power management) */
+	int			target_ps_state;
+	int			ps_state;
+
+	/* power management (host suspend and resume) */
 	pm_message_t		pm_mesg;
 	int			*pm_result;
 
@@ -613,6 +629,8 @@ struct ata_port_operations {
 	void (*scr_write) (struct ata_port *ap, unsigned int sc_reg,
 			   u32 val);
 
+	void (*set_powersave) (struct ata_port *ap, int ps_state);
+
 	int (*port_suspend) (struct ata_port *ap, pm_message_t mesg);
 	int (*port_resume) (struct ata_port *ap);
 
@@ -982,6 +1000,21 @@ static inline int ata_port_max_devices(c
 	return 1;
 }
 
+/*
+ * powersave helpers
+ */
+static inline int ata_ps_static(int ps_state)
+{
+	return ps_state >= ATA_PS_STATIC ? ATA_PS_STATIC : 0;
+}
+
+static inline int ata_ps_dynamic(int ps_state)
+{
+	if (ps_state >= ATA_PS_STATIC)
+		ps_state -= ATA_PS_STATIC;
+	return ps_state;
+}
+
 
 static inline u8 ata_chk_status(struct ata_port *ap)
 {
-- 
1.3.2

[PATCH 06/12] libata: add ata_port_nr_ready()

[Tejun Heo]

Add ready counting function - ata_port_nr_ready().  This is in
preparation for powersave support.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   10 ++++++++++
 drivers/scsi/libata.h      |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

efc7e0e82005a0135d9b658af156d6388c49367d
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 8e75899..3e35839 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -120,6 +120,16 @@ int ata_port_nr_enabled(struct ata_port 
 	return cnt;
 }
 
+int ata_port_nr_ready(struct ata_port *ap)
+{
+	int i, cnt = 0;
+
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		if (ata_dev_ready(&ap->device[i]))
+			cnt++;
+	return cnt;
+}
+
 /**
  *	ata_tf_to_fis - Convert ATA taskfile to SATA FIS structure
  *	@tf: Taskfile to convert
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index d77b5ff..ce44e06 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -50,6 +50,7 @@ extern int atapi_dmadir;
 extern int libata_fua;
 extern int ata_port_nr_vacant(struct ata_port *ap);
 extern int ata_port_nr_enabled(struct ata_port *ap);
+extern int ata_port_nr_ready(struct ata_port *ap);
 extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev);
 extern int ata_rwcmd_protocol(struct ata_queued_cmd *qc);
 extern void ata_dev_disable(struct ata_device *dev);
-- 
1.3.2

[PATCH 11/12] ahci: implement link powersave

[Tejun Heo]

Implement link powersave.  AHCI can initiate transition to powersave
mode but doesn't use standard SControl SPM field.  It uses ICC field
in PORT_CMD register.  As PHY RDY changed interrupt can trigger during
dynamic link powersave, it needs to be turned off while dynamic
powersave is active.  All other can be taken care of by standard
helpers.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/ahci.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)

14143d016b88395ffcec7be07ffa99d652a3353d
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 77e7202..b5fe19a 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -92,6 +92,8 @@ enum {
 	HOST_AHCI_EN		= (1 << 31), /* AHCI enabled */
 
 	/* HOST_CAP bits */
+	HOST_CAP_PARTIAL	= (1 << 13), /* Partial state support */
+	HOST_CAP_SLUMBER	= (1 << 14), /* Slumber state support */
 	HOST_CAP_CLO		= (1 << 24), /* Command List Override support */
 	HOST_CAP_NCQ		= (1 << 30), /* Native Command Queueing */
 	HOST_CAP_64		= (1 << 31), /* PCI DAC (64-bit DMA) support */
@@ -158,6 +160,7 @@ enum {
 	PORT_CMD_ICC_ACTIVE	= (0x1 << 28), /* Put i/f in active state */
 	PORT_CMD_ICC_PARTIAL	= (0x2 << 28), /* Put i/f in partial state */
 	PORT_CMD_ICC_SLUMBER	= (0x6 << 28), /* Put i/f in slumber state */
+	PORT_CMD_ICC_MASK	= (0xf << 28),
 
 	/* hpriv->flags bits */
 	AHCI_FLAG_MSI		= (1 << 0),
@@ -212,6 +215,7 @@ static void ahci_freeze(struct ata_port 
 static void ahci_thaw(struct ata_port *ap);
 static void ahci_error_handler(struct ata_port *ap);
 static void ahci_post_internal_cmd(struct ata_queued_cmd *qc);
+static void ahci_set_powersave(struct ata_port *ap, int ps_state);
 static void ahci_remove_one (struct pci_dev *pdev);
 
 static struct scsi_host_template ahci_sht = {
@@ -257,6 +261,8 @@ static const struct ata_port_operations 
 	.error_handler		= ahci_error_handler,
 	.post_internal_cmd	= ahci_post_internal_cmd,
 
+	.set_powersave		= ahci_set_powersave,
+
 	.port_start		= ahci_port_start,
 	.port_stop		= ahci_port_stop,
 };
@@ -913,6 +919,9 @@ static void ahci_host_intr(struct ata_po
 	status = readl(port_mmio + PORT_IRQ_STAT);
 	writel(status, port_mmio + PORT_IRQ_STAT);
 
+	if (ata_ps_dynamic(ap->ps_state))
+		status &= ~PORT_IRQ_PHYRDY;
+
 	if (unlikely(status & PORT_IRQ_ERROR)) {
 		ahci_error_intr(ap, status);
 		return;
@@ -1077,6 +1086,60 @@ static void ahci_post_internal_cmd(struc
 	}
 }
 
+static void ahci_update_sctl_spm(struct ata_port *ap, u8 sctl_spm,
+				 int may_push_sctl)
+{
+	static const u32 icc_map[] = {
+		[0x1]	= PORT_CMD_ICC_PARTIAL,
+		[0x2]	= PORT_CMD_ICC_SLUMBER,
+		[0x4]	= PORT_CMD_ICC_ACTIVE,
+	};
+	void __iomem *mmio = ap->host_set->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+	u32 tmp;
+
+	tmp = readl(port_mmio + PORT_CMD);
+
+	/* proceed only if idle */
+	if (!(tmp & PORT_CMD_ICC_MASK))
+		writel(tmp | icc_map[sctl_spm], port_mmio + PORT_CMD);
+}
+
+static unsigned long ahci_hips_timer_fn(struct ata_port *ap, int seq)
+{
+	u8 sctl_ipm = ata_scontrol_field(ap->scontrol, ATA_SCTL_IPM);
+
+	return sata_do_hips_timer_fn(ap, seq, sctl_ipm, ahci_update_sctl_spm);
+}
+
+static void ahci_set_powersave(struct ata_port *ap, int ps_state)
+{
+	void __iomem *mmio = ap->host_set->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+	struct ahci_host_priv *hpriv = ap->host_set->private_data;
+	u8 sctl_ipm;
+
+	/* determine IPM */
+	sctl_ipm = 0x3;
+	if (hpriv->cap & HOST_CAP_PARTIAL)
+		sctl_ipm &= ~0x1;
+	if (hpriv->cap & HOST_CAP_SLUMBER)
+		sctl_ipm &= ~0x2;
+
+	/* turn off SError.N IRQ if entering dynamic powersave mode */
+	if (ata_ps_dynamic(ps_state))
+		writel(DEF_PORT_IRQ & ~PORT_IRQ_PHYRDY,
+		       port_mmio + PORT_IRQ_MASK);
+
+	/* do standard SATA set_powersave */
+	if (ata_ps_dynamic(ps_state) == ATA_PS_HIPS) {
+		ap->ps_timer_fn = ahci_hips_timer_fn;
+		sata_determine_hips_params(ap, &sctl_ipm);
+	}
+
+	sata_do_set_powersave(ap, ps_state, sctl_ipm, ahci_update_sctl_spm);
+}
+
 static void ahci_setup_port(struct ata_ioports *port, unsigned long base,
 			    unsigned int port_idx)
 {
@@ -1398,6 +1461,9 @@ static int ahci_init_one (struct pci_dev
 	    (hpriv->cap & HOST_CAP_NCQ))
 		probe_ent->host_flags |= ATA_FLAG_NCQ;
 
+	if (hpriv->cap & (HOST_CAP_PARTIAL | HOST_CAP_SLUMBER))
+		probe_ent->host_flags |= ATA_FLAG_HIPS | ATA_FLAG_DIPS;
+
 	ahci_print_info(probe_ent);
 
 	/* FIXME: check ata_device_add return value */
-- 
1.3.2

[PATCH 07/12] libata: implement sata_update_scontrol()

[Tejun Heo]

SControl register hosts several control fields and update to one field
shouldn't alter others.  libata used to do read-modify-write to
achieve this.  However, this results in excessive SControl reading and
loss of SControl setting under certain circumstances (e.g. suspend /
resume cycles, hardware glitch).

This patch adds SControl cache, ap->scontrol, and two helper functions
- sata_update_scontrol_push() which only updates the cache and
sata_update_scontrol() which updates & commits the updated value to
SControl.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |  139 +++++++++++++++++++++++++++++---------------
 drivers/scsi/sata_mv.c     |    4 +
 include/linux/libata.h     |    4 +
 3 files changed, 99 insertions(+), 48 deletions(-)

ac471e3d83d7a3a7ae2b4cf976b348ce7a74948b
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 3e35839..cc77bd5 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1859,26 +1859,19 @@ int sata_down_spd_limit(struct ata_port 
 	return 0;
 }
 
-static int __sata_set_spd_needed(struct ata_port *ap, u32 *scontrol)
+static inline int sata_spd_val(struct ata_port *ap)
 {
-	u32 spd, limit;
-
 	if (ap->sata_spd_limit == UINT_MAX)
-		limit = 0;
+		return 0;
 	else
-		limit = fls(ap->sata_spd_limit);
-
-	spd = (*scontrol >> 4) & 0xf;
-	*scontrol = (*scontrol & ~0xf0) | ((limit & 0xf) << 4);
-
-	return spd != limit;
+		return fls(ap->sata_spd_limit);
 }
 
 /**
  *	sata_set_spd_needed - is SATA spd configuration needed
  *	@ap: Port in question
  *
- *	Test whether the spd limit in SControl matches
+ *	Test whether the spd limit in ap->scontrol matches
  *	@ap->sata_spd_limit.  This function is used to determine
  *	whether hardreset is necessary to apply SATA spd
  *	configuration.
@@ -1891,12 +1884,9 @@ static int __sata_set_spd_needed(struct 
  */
 int sata_set_spd_needed(struct ata_port *ap)
 {
-	u32 scontrol;
+	u32 spd = (ap->scontrol >> 4) & 0xf;
 
-	if (sata_scr_read(ap, SCR_CONTROL, &scontrol))
-		return 0;
-
-	return __sata_set_spd_needed(ap, &scontrol);
+	return spd != sata_spd_val(ap);
 }
 
 /**
@@ -1914,16 +1904,12 @@ int sata_set_spd_needed(struct ata_port 
  */
 int sata_set_spd(struct ata_port *ap)
 {
-	u32 scontrol;
 	int rc;
 
-	if ((rc = sata_scr_read(ap, SCR_CONTROL, &scontrol)))
-		return rc;
-
-	if (!__sata_set_spd_needed(ap, &scontrol))
+	if (!sata_set_spd_needed(ap))
 		return 0;
 
-	if ((rc = sata_scr_write(ap, SCR_CONTROL, scontrol)))
+	if ((rc = sata_update_scontrol(ap, ATA_SCTL_SPD, sata_spd_val(ap))))
 		return rc;
 
 	return 1;
@@ -2595,15 +2581,9 @@ int sata_phy_debounce(struct ata_port *a
  */
 int sata_phy_resume(struct ata_port *ap, const unsigned long *params)
 {
-	u32 scontrol;
 	int rc;
 
-	if ((rc = sata_scr_read(ap, SCR_CONTROL, &scontrol)))
-		return rc;
-
-	scontrol = (scontrol & 0x0f0) | 0x300;
-
-	if ((rc = sata_scr_write(ap, SCR_CONTROL, scontrol)))
+	if ((rc = sata_update_scontrol(ap, ATA_SCTL_DET, 0x0)))
 		return rc;
 
 	/* Some PHYs react badly if SStatus is pounded immediately
@@ -2765,7 +2745,6 @@ int sata_std_hardreset(struct ata_port *
 {
 	struct ata_eh_context *ehc = &ap->eh_context;
 	const unsigned long *timing = sata_ehc_deb_timing(ehc);
-	u32 scontrol;
 	int rc;
 
 	DPRINTK("ENTER\n");
@@ -2776,24 +2755,14 @@ int sata_std_hardreset(struct ata_port *
 		 * reconfiguration.  This works for at least ICH7 AHCI
 		 * and Sil3124.
 		 */
-		if ((rc = sata_scr_read(ap, SCR_CONTROL, &scontrol)))
-			return rc;
-
-		scontrol = (scontrol & 0x0f0) | 0x304;
-
-		if ((rc = sata_scr_write(ap, SCR_CONTROL, scontrol)))
+		if ((rc = sata_update_scontrol(ap, ATA_SCTL_DET, 0x4)))
 			return rc;
 
 		sata_set_spd(ap);
 	}
 
 	/* issue phy wake/reset */
-	if ((rc = sata_scr_read(ap, SCR_CONTROL, &scontrol)))
-		return rc;
-
-	scontrol = (scontrol & 0x0f0) | 0x301;
-
-	if ((rc = sata_scr_write_flush(ap, SCR_CONTROL, scontrol)))
+	if ((rc = sata_update_scontrol(ap, ATA_SCTL_DET, 0x1)))
 		return rc;
 
 	/* Couldn't find anything in SATA I/II specs, but AHCI-1.1
@@ -4975,6 +4944,77 @@ int sata_scr_write_flush(struct ata_port
 }
 
 /**
+ *	sata_update_scontrol_push - push SControl register update
+ *	@ap: ATA port to update SControl for
+ *	@sel: ATA_SCTL_* subfield selector
+ *	@val: value to be written
+ *
+ *	SControl hosts several control subfields which need to be
+ *	treated separately.  SControl value is cached on
+ *	initialization and this function updates only the requested
+ *	field of the cache.  This function only accumulates SControl
+ *	updates in the cache and does NOT write the updated value to
+ *	SControl.
+ *
+ *	SControl cache reduces SControl access and helps preserving
+ *	SControl when hardware forgets the configured value (e.g. over
+ *	suspend/resume).
+ *
+ *	LOCKING:
+ *	ap->scontrol is protected by ap->lock while EH is inactive and
+ *	owned by EH while it's active.  So, spin_lock_irqsave(host_set
+ *	lock) out of EH, and none during EH.
+ *
+ *	RETURNS:
+ *	0 on success, negative errno on failure.
+ */
+void sata_update_scontrol_push(struct ata_port *ap, int sel, u8 val)
+{
+	u32 scontrol = ap->scontrol;
+	int shift = sel * 4;
+
+	WARN_ON(val > 0xf);
+	val &= 0xf;
+
+	scontrol = (scontrol & ~(0xf << shift)) | (val << shift);
+
+	ap->scontrol = scontrol;
+}
+
+/**
+ *	sata_update_scontrol - update SControl register
+ *	@ap: ATA port to update SControl for
+ *	@sel: ATA_SCTL_* subfield selector
+ *	@val: value to be written
+ *
+ *	Update SControl cache using sata_update_scontrol_push() and
+ *	write the cached value to the SControl register.
+ *
+ *	LOCKING:
+ *	ap->scontrol is protected by ap->lock while EH is inactive and
+ *	owned by EH while it's active.  So, spin_lock_irqsave(host_set
+ *	lock) out of EH, and none during EH.
+ *
+ *	RETURNS:
+ *	0 on success, negative errno on failure.
+ */
+int sata_update_scontrol(struct ata_port *ap, int sel, u8 val)
+{
+	int rc;
+
+	/* push requested change */
+	sata_update_scontrol_push(ap, sel, val);
+
+	/* always use the flushing version */
+	rc = sata_scr_write_flush(ap, SCR_CONTROL, ap->scontrol);
+
+	/* SPM is oneshot field, don't cache it over write */
+	ap->scontrol &= ~(0xf << (ATA_SCTL_SPM * 4));
+
+	return 0;
+}
+
+/**
  *	ata_port_online - test whether the given port is online
  *	@ap: ATA port to test
  *
@@ -5492,7 +5532,6 @@ int ata_device_add(const struct ata_prob
 	DPRINTK("probe begin\n");
 	for (i = 0; i < count; i++) {
 		struct ata_port *ap;
-		u32 scontrol;
 		int rc;
 
 		ap = host_set->ports[i];
@@ -5502,9 +5541,15 @@ int ata_device_add(const struct ata_prob
 		list_add_tail(&ap->all_ports_entry, &ata_all_ports);
 		mutex_unlock(&ata_all_ports_mutex);
 
-		/* init sata_spd_limit to the current value */
-		if (sata_scr_read(ap, SCR_CONTROL, &scontrol) == 0) {
-			int spd = (scontrol >> 4) & 0xf;
+		/* init scontrol and sata_spd_limit to the current value */
+		ap->scontrol = 0x300; /* default value */
+		if (sata_scr_read(ap, SCR_CONTROL, &ap->scontrol) == 0) {
+			int spd;
+
+			/* zero PMP/SPM and no powersaving */
+			ap->scontrol = (ap->scontrol & 0xfff000ff) | 0x300;
+
+			spd = ata_scontrol_field(ap->scontrol, ATA_SCTL_SPD);
 			ap->hw_sata_spd_limit &= (1 << spd) - 1;
 		}
 		ap->sata_spd_limit = ap->hw_sata_spd_limit;
@@ -6036,6 +6081,8 @@ EXPORT_SYMBOL_GPL(sata_scr_valid);
 EXPORT_SYMBOL_GPL(sata_scr_read);
 EXPORT_SYMBOL_GPL(sata_scr_write);
 EXPORT_SYMBOL_GPL(sata_scr_write_flush);
+EXPORT_SYMBOL_GPL(sata_update_scontrol_push);
+EXPORT_SYMBOL_GPL(sata_update_scontrol);
 EXPORT_SYMBOL_GPL(ata_port_online);
 EXPORT_SYMBOL_GPL(ata_port_offline);
 EXPORT_SYMBOL_GPL(ata_host_set_suspend);
diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
index 1053c7c..52bf790 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -1952,10 +1952,10 @@ static void __mv_phy_reset(struct ata_po
 
 	/* Issue COMRESET via SControl */
 comreset_retry:
-	sata_scr_write_flush(ap, SCR_CONTROL, 0x301);
+	sata_update_scontrol(ap, ATA_SCTL_DET, 0x1);
 	__msleep(1, can_sleep);
 
-	sata_scr_write_flush(ap, SCR_CONTROL, 0x300);
+	sata_update_scontrol(ap, ATA_SCTL_DET, 0x0);
 	__msleep(20, can_sleep);
 
 	timeout = jiffies + msecs_to_jiffies(200);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index c9ed035..1656d5b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -521,6 +521,8 @@ struct ata_port {
 	unsigned int		mwdma_mask;
 	unsigned int		udma_mask;
 	unsigned int		cbl;	/* cable type; ATA_CBL_xxx */
+
+	u32			scontrol;	/* for update_scontrol */
 	unsigned int		hw_sata_spd_limit;
 	unsigned int		sata_spd_limit;	/* SATA PHY speed limit */
 
@@ -696,6 +698,8 @@ extern int sata_scr_valid(struct ata_por
 extern int sata_scr_read(struct ata_port *ap, int reg, u32 *val);
 extern int sata_scr_write(struct ata_port *ap, int reg, u32 val);
 extern int sata_scr_write_flush(struct ata_port *ap, int reg, u32 val);
+extern void sata_update_scontrol_push(struct ata_port *ap, int sel, u8 val);
+extern int sata_update_scontrol(struct ata_port *ap, int sel, u8 val);
 extern int ata_port_online(struct ata_port *ap);
 extern int ata_port_offline(struct ata_port *ap);
 extern int ata_scsi_device_resume(struct scsi_device *);
-- 
1.3.2

[PATCH 09/12] libata: implement powersave timer

[Tejun Heo]

Implement powersave timer.  It is primarily for OS-driven HIPS
implementation but can be used for any other PS purpose LLD sees fit.
During normal operation, PS timer is automatically started with
timeout ap->ps_timeout on port idle and stopped when the port becomes
busy.  The timer is also stopped while EH.

To minimize overhead and allow easy implementation of expected
operation model, ata_ps_timer_worker() is used as timer callback which
invokes LLD supplied ap->ps_timer_fn() if condition meets and also
helps implementing sequenced multi-step operation.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   90 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/libata-eh.c   |   19 +++++++++
 drivers/scsi/libata.h      |    1 
 include/linux/libata.h     |    8 ++++
 4 files changed, 116 insertions(+), 2 deletions(-)

aa69e4ca8379e6db85e133f8e25bba00961201c7
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index b968b44..1658cd1 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -4425,13 +4425,15 @@ void ata_qc_complete(struct ata_queued_c
 	 * taken care of.
 	 */
 	if (ap->ops->error_handler) {
+		int internal = ata_tag_internal(qc->tag);
+
 		WARN_ON(ap->pflags & ATA_PFLAG_FROZEN);
 
 		if (unlikely(qc->err_mask))
 			qc->flags |= ATA_QCFLAG_FAILED;
 
 		if (unlikely(qc->flags & ATA_QCFLAG_FAILED)) {
-			if (!ata_tag_internal(qc->tag)) {
+			if (!internal) {
 				/* always fill result TF for failed qc */
 				ap->ops->tf_read(ap, &qc->result_tf);
 				ata_qc_schedule_eh(qc);
@@ -4444,6 +4446,13 @@ void ata_qc_complete(struct ata_queued_c
 			ap->ops->tf_read(ap, &qc->result_tf);
 
 		__ata_qc_complete(qc);
+
+		/* update PS timer */
+		if ((ap->pflags & ATA_PFLAG_PS_TIMER) &&
+		    !ap->qc_active && !internal) {
+			ap->ps_seq = 0;
+			mod_timer(&ap->ps_timer, jiffies + ap->ps_timeout);
+		}
 	} else {
 		if (qc->flags & ATA_QCFLAG_EH_SCHEDULED)
 			return;
@@ -4551,6 +4560,9 @@ void ata_qc_issue(struct ata_queued_cmd 
 	 */
 	WARN_ON(ap->ops->error_handler && ata_tag_valid(ap->active_tag));
 
+	if (ap->pflags & ATA_PFLAG_PS_TIMER)
+		del_timer(&ap->ps_timer);
+
 	if (qc->tf.protocol == ATA_PROT_NCQ) {
 		WARN_ON(ap->sactive & (1 << qc->tag));
 		ap->sactive |= 1 << qc->tag;
@@ -5207,6 +5219,77 @@ void ata_host_set_resume(struct ata_host
 }
 
 /**
+ *	ata_kill_ps_timer - kill powersave timer
+ *	@ap: ATA port of interest
+ *
+ *	Kill powersave timer.  On return from this function, powersave
+ *	timer is guaranteed to be not pending && not running.
+ *
+ *	LOCKING:
+ *	None (must be called from EH context).
+ */
+void ata_kill_ps_timer(struct ata_port *ap)
+{
+	unsigned long flags;
+	int rc;
+
+	if (ap->pflags & ATA_PFLAG_PS_TIMER) {
+		do {
+			spin_lock_irqsave(ap->lock, flags);
+			rc = try_to_del_timer_sync(&ap->ps_timer);
+			ap->pflags &= ~ATA_PFLAG_PS_TIMER;
+			spin_unlock_irqrestore(ap->lock, flags);
+			cpu_relax();
+		} while (rc < 0);
+
+		ap->ps_timer_fn = NULL;
+		ap->ps_timeout = 0;
+	} else
+		BUG_ON(timer_pending(&ap->ps_timer));
+}
+
+/**
+ *	ata_ps_timer_worker - powersave timer worker
+ *	@arg: ATA port of interest
+ *
+ *	This function is called when ap->ps_timer expires.  If the
+ *	condition is right, it calls ap->ps_timer_fn() with the
+ *	current sequence number.
+ *
+ *	libata automatically arms and cancels PS timer on port idle
+ *	and command issue respectively.  libata also initializes
+ *	sequence number to zero when it arms PS timer because of port
+ *	idle.
+ *
+ *	ap->ps_timer_fn() can request requeue of PS timer by returning
+ *	non-zero value which will be used as timeout.  Each requeue
+ *	increments PS sequence number by one.
+ *
+ *	LOCKING:
+ *	None.
+ */
+static void ata_ps_timer_worker(unsigned long arg)
+{
+	struct ata_port *ap = (void *)arg;
+	unsigned long flags;
+
+	spin_lock_irqsave(ap->lock, flags);
+
+	if (!ap->qc_active && !timer_pending(&ap->ps_timer)) {
+		unsigned long next_timeout;
+
+		next_timeout = ap->ps_timer_fn(ap, ap->ps_seq);
+		if (next_timeout) {
+			ap->ps_seq++;
+			ap->ps_timer.expires = jiffies + next_timeout;
+			add_timer(&ap->ps_timer);
+		}
+	}
+
+	spin_unlock_irqrestore(ap->lock, flags);
+}
+
+/**
  *	ata_port_start - Set port up for dma.
  *	@ap: Port to initialize
  *
@@ -5439,6 +5522,7 @@ #endif
 	INIT_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan, ap);
 	INIT_LIST_HEAD(&ap->eh_done_q);
 	init_waitqueue_head(&ap->eh_wait_q);
+	setup_timer(&ap->ps_timer, ata_ps_timer_worker, (unsigned long)ap);
 
 	/* set cable type */
 	ap->cbl = ATA_CBL_NONE;
@@ -5749,7 +5833,9 @@ void ata_port_detach(struct ata_port *ap
 	cancel_delayed_work(&ap->hotplug_task);
 	flush_workqueue(ata_aux_wq);
 
-	/* turn it off */
+	/* kill PS timer and power off */
+	ata_kill_ps_timer(ap);
+
 	if (ap->ops->set_powersave)
 		ap->ops->set_powersave(ap, ATA_PS_STATIC);
 
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index d5126b7..b19eaae 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -261,6 +261,9 @@ void ata_scsi_error(struct Scsi_Host *ho
 	} else
 		spin_unlock_wait(ap->lock);
 
+	/* kill powersave timer before beginning EH */
+	ata_kill_ps_timer(ap);
+
  repeat:
 	/* invoke error handler */
 	if (ap->ops->error_handler) {
@@ -340,6 +343,14 @@ void ata_scsi_error(struct Scsi_Host *ho
 
 	ap->pflags &= ~(ATA_PFLAG_SCSI_HOTPLUG | ATA_PFLAG_RECOVERED);
 
+	/* start PS timer if requested */
+	if (ap->pflags & ATA_PFLAG_PS_TIMER) {
+		BUG_ON(!ap->ps_timer_fn || !ap->ps_timeout);
+		ap->ps_seq = 0;
+		ap->ps_timer.expires = jiffies + ap->ps_timeout;
+		add_timer(&ap->ps_timer);
+	}
+
 	/* tell wait_eh that we're done */
 	ap->pflags &= ~ATA_PFLAG_EH_IN_PROGRESS;
 	wake_up_all(&ap->eh_wait_q);
@@ -1876,11 +1887,13 @@ static int ata_eh_set_powersave(struct a
 	void (*set_ps)(struct ata_port *, int) = ap->ops->set_powersave;
 	int ps_state, ps_static, ps_dynamic;
 	int dev_dips, dev_hips, i;
+	unsigned long flags;
 
 	/* power up for recovery */
 	if (r_failed_dev == NULL) {
 		if (ap->ps_state && set_ps)
 			set_ps(ap, ATA_PS_NONE);
+		WARN_ON(ap->flags & ATA_PFLAG_PS_TIMER);
 		return 0;
 	}
 
@@ -1929,9 +1942,15 @@ static int ata_eh_set_powersave(struct a
 
 		err_mask = ata_dev_set_dips(dev, is_dips);
 		if (err_mask) {
+			spin_lock_irqsave(ap->lock, flags);
+			ap->pflags &= ~ATA_PFLAG_PS_TIMER;
+			spin_unlock_irqrestore(ap->lock, flags);
+
 			if (ps_state && set_ps)
 				set_ps(ap, ATA_PS_NONE);
+
 			*r_failed_dev = dev;
+
 			return -EIO;
 		}
 	}
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index ce44e06..9e44f56 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -74,6 +74,7 @@ extern void ata_dev_select(struct ata_po
                            unsigned int wait, unsigned int can_sleep);
 extern void swap_buf_le16(u16 *buf, unsigned int buf_words);
 extern int ata_flush_cache(struct ata_device *dev);
+extern void ata_kill_ps_timer(struct ata_port *ap);
 extern void ata_dev_init(struct ata_device *dev);
 extern int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg);
 extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index b3f56c5..0881814 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -185,6 +185,7 @@ enum {
 	ATA_PFLAG_FLUSH_PORT_TASK = (1 << 16), /* flush port task */
 	ATA_PFLAG_SUSPENDED	= (1 << 17), /* port is suspended (power) */
 	ATA_PFLAG_PM_PENDING	= (1 << 18), /* PM operation pending */
+	ATA_PFLAG_PS_TIMER	= (1 << 19), /* PS timer active */
 
 	/* struct ata_queued_cmd flags */
 	ATA_QCFLAG_ACTIVE	= (1 << 0), /* cmd not yet ack'd to scsi lyer */
@@ -335,6 +336,7 @@ typedef void (*ata_qc_cb_t) (struct ata_
 typedef int (*ata_prereset_fn_t)(struct ata_port *ap);
 typedef int (*ata_reset_fn_t)(struct ata_port *ap, unsigned int *classes);
 typedef void (*ata_postreset_fn_t)(struct ata_port *ap, unsigned int *classes);
+typedef unsigned long (*ata_ps_timer_fn_t)(struct ata_port *ap, int seq);
 
 struct ata_ioports {
 	unsigned long		cmd_addr;
@@ -568,6 +570,12 @@ struct ata_port {
 	/* powersave (dynamic link power management) */
 	int			target_ps_state;
 	int			ps_state;
+	/* powersave timer, protected by ap->lock */
+	ata_ps_timer_fn_t	ps_timer_fn;	/* initialized by LLD */
+	unsigned long		ps_timeout;	/* ditto */
+
+	int			ps_seq;		/* managed by libata */
+	struct timer_list	ps_timer;	/* ditto */
 
 	/* power management (host suspend and resume) */
 	pm_message_t		pm_mesg;
-- 
1.3.2

[PATCH 12/12] sata_sil24: implement link powersave

[Tejun Heo]

Implement link powersave.  sata_sil24 is mostly compliant with the
standard and standard methods can do everything other than plugging
PHY RDY changed IRQ.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/sata_sil24.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

600172dbafec31447eaee86ca91a3aa8370fbc47
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 2e0f4a4..0b5c3f1 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -230,7 +230,8 @@ enum {
 	/* host flags */
 	SIL24_COMMON_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-				  ATA_FLAG_NCQ | ATA_FLAG_SKIP_D2H_BSY,
+				  ATA_FLAG_NCQ | ATA_FLAG_SKIP_D2H_BSY |
+				  ATA_FLAG_HIPS | ATA_FLAG_DIPS,
 	SIL24_FLAG_PCIX_IRQ_WOC	= (1 << 24), /* IRQ loss errata on PCI-X */
 
 	IRQ_STAT_4PORTS		= 0xf,
@@ -335,6 +336,7 @@ static void sil24_freeze(struct ata_port
 static void sil24_thaw(struct ata_port *ap);
 static void sil24_error_handler(struct ata_port *ap);
 static void sil24_post_internal_cmd(struct ata_queued_cmd *qc);
+static void sil24_set_powersave(struct ata_port *ap, int ps_state);
 static int sil24_port_start(struct ata_port *ap);
 static void sil24_port_stop(struct ata_port *ap);
 static void sil24_host_stop(struct ata_host_set *host_set);
@@ -405,6 +407,8 @@ static const struct ata_port_operations 
 	.error_handler		= sil24_error_handler,
 	.post_internal_cmd	= sil24_post_internal_cmd,
 
+	.set_powersave		= sil24_set_powersave,
+
 	.port_start		= sil24_port_start,
 	.port_stop		= sil24_port_stop,
 	.host_stop		= sil24_host_stop,
@@ -764,6 +768,10 @@ static void sil24_error_intr(struct ata_
 	irq_stat = readl(port + PORT_IRQ_STAT);
 	writel(irq_stat, port + PORT_IRQ_STAT);
 
+	/* ignore PHYRDY_CHG during dynamic powersave */
+	if (ata_ps_dynamic(ap->ps_state))
+		irq_stat &= ~PORT_IRQ_PHYRDY_CHG;
+
 	/* first, analyze and record host port events */
 	ata_ehi_clear_desc(ehi);
 
@@ -928,6 +936,18 @@ static void sil24_post_internal_cmd(stru
 		sil24_init_port(ap);
 }
 
+static void sil24_set_powersave(struct ata_port *ap, int ps_state)
+{
+	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
+
+	/* turn off SError.N IRQ if entering dynamic powersave mode */
+	if (ata_ps_dynamic(ps_state))
+		writel(PORT_IRQ_PHYRDY_CHG, port + PORT_IRQ_ENABLE_CLR);
+
+	/* everything else can be done with standard SCR registers */
+	sata_std_set_powersave(ap, ps_state);
+}
+
 static inline void sil24_cblk_free(struct sil24_port_priv *pp, struct device *dev)
 {
 	const size_t cb_size = sizeof(*pp->cmd_block) * SIL24_MAX_CMDS;
-- 
1.3.2

Re: [PATCH 03/12] libata: add more SATA specific constants and macros to ata.h

[Jeff Garzik]

Tejun Heo wrote:
> Add more SATA specific constants and macros to ata.h.
> 
> * SATA SETFEATURES constants
> * SControl field selectors and extracting macros
> * link powersave feature test macros
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

ACK patches 1-3

Re: [PATCH 04/12] libata: implement ata_all_ports list

[Jeff Garzik]

Tejun Heo wrote:
> Implement ata_all_ports list.  All active ata_ports are linked on this
> list.  This will be used to dynamically modify libata-wide
> configurations.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/libata-core.c |   13 +++++++++++++
>  include/linux/libata.h     |    2 ++
>  2 files changed, 15 insertions(+), 0 deletions(-)
> 
> a962488760934f639dd202793694339afd2db1cf
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index 61db4f5..1477d32 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -70,6 +70,9 @@ static unsigned int ata_dev_init_params(
>  static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
>  static void ata_dev_xfermask(struct ata_device *dev);
>  
> +static DEFINE_MUTEX(ata_all_ports_mutex);
> +static LIST_HEAD(ata_all_ports);

NAK.  I'm pretty strongly against things that cross the controller 
boundary.  Any information or data structure shared across multiple 
controllers often winds up unnecessary limiting future implementations, 
and become locking/serialization nightmares.

Re: [PATCH 07/12] libata: implement sata_update_scontrol()

[Jeff Garzik]

ACK patches 5-7

Re: [PATCHSET] libata: implement runtime link powersave

[Jeff Garzik]

Tejun Heo wrote:
> Hello, all.
> 
> This is the first take of powersave patchset.  It implements runtime
> link-level power management.  To avoid confusion with regular
> device/controller power management (suspend/resume), it's callsed
> powersave.  It primarily implements partial/slumber/phy-off power
> states SATA standard specifies but if LLD has different way of
> implementing link powersave, it's allowed to.  Both host-initiated and
> device-initiated operations are supported.
> 
> This patchset contains the following patches.
> 
> #01-07	: misc fixes/preps for powersave
> #08-10	: implements core powersave infrastructure
> #11-12	: implement powersave feature for ahci and sata_sil24
> 
> Powersave can be configured at boot time or while running by writing
> to kernel module parameter /sys/module/libata/parameters/powersave.
> The following powersave modes are supported.
> 
> * none		: no powersave, link is powered up all the time
> * HIPS		: host-initiated powersave
> * DIPS		: device-initiated powersave
> * static	: no powersave on occupied link, power off empty link
> * HIPS/static	: HIPS on occupied link, power off empty link
> * DIPS/static	: DIPS on occupied link, power off empty link
> 
> For more information, please read head messages of individual patches.
> 
> All supported modes are tested on ICH7R AHCI, ICH6M AHCI, SiI3124,
> SiI3132.  Unfortunately SiI3112/4 family controllers don't support
> HIPS and choke on DIPS and thus dropped from supported list.

Any chance you tried ata_piix static PS, using PCS register?  :)

	Jeff

Re: [PATCH 08/12] libata: implement interface power management infrastructure

[Jeff Garzik]

Tejun Heo wrote:
> Implement interface power management infrastructure.  To discern link
> power management from device power management (suspend/resume), link
> power management is called 'powersave' or 'PS' while device power
> mangement is called 'power management' or 'PM'.
> 
> libata PS infrastructure is primarily designed to accomodate SATA link
> powersave (link ACTIVE/PARTIAL/SLUMBER) but is not limited to it.
> libata implements the following powersave modes.
> 
> * none		: no powersave, link is powered up all the time
> * HIPS		: host-initiated powersave
> * DIPS		: device-initiated powersave
> * static	: no powersave on occupied link, power off empty link
> * HIPS/static	: HIPS on occupied link, power off empty link
> * DIPS/static	: DIPS on occupied link, power off empty link
> 
> HIPS/DIPS are called dynamic PS while static is static PS.  LLD can
> indicate which dynamic PS modes it supports using ATA_FLAG_HIPS and
> ATA_FLAG_DIPS.  Static mode support is mandatory but LLD is free to
> implement it as noop.  In fact, if LLD doesn't implement any powersave
> feature, libata will automatically handle static PS as noop.
> 
> PS mode is disengaged during EH recovery and reenabled on recovery
> completion.  Device configuration for DIPS is done by libata EH and
> LLD only has to configure the controller when instructed via
> ->set_powersave() callback.
> 
> libata guarantees that there is a reset before changing PS mode.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

> +static int libata_powersave = 0;	/* protected by all_ports_mutex */
> +module_param_call(powersave, ata_param_set_powersave, param_get_int,
> +		  &libata_powersave, 0644);
> +MODULE_PARM_DESC(powersave, "Powersave mode (0=none, 1=HIPS, 2=DIPS, "
> +		 "3=static, 4=HIPS/static, 5=DIPS/static)");
> +

NAK.

Module parameters in libata are largely for turning off/on major 
features, during a transitional testing period.  They largely provide 
users with workaround solutions to "it won't boot" type problems.

I also think you have been seduced by the relative ease of adding this 
control to libata, as opposed to the preferred alternative:  digging 
through sysfs objects to find the right one to which attributes should 
be added.

As an aside, in addition to per-controller (or per-port) sysfs powersave 
controls, libata should [eventually] recognize the external "laptop 
mode" setting.  When laptop mode == ON, program powersave aggresively.

In general, though, we should peek to minimize power usage for the 
general case, where feasible.  People with huge server closets like 
Google need every ounce of power savings possible.

With regards to the patch content:  other than the control interface, 
the implementation looks OK.

Re: [PATCH 09/12] libata: implement powersave timer

[Jeff Garzik]

Tejun Heo wrote:
> Implement powersave timer.  It is primarily for OS-driven HIPS
> implementation but can be used for any other PS purpose LLD sees fit.
> During normal operation, PS timer is automatically started with
> timeout ap->ps_timeout on port idle and stopped when the port becomes
> busy.  The timer is also stopped while EH.
> 
> To minimize overhead and allow easy implementation of expected
> operation model, ata_ps_timer_worker() is used as timer callback which
> invokes LLD supplied ap->ps_timer_fn() if condition meets and also
> helps implementing sequenced multi-step operation.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

This makes me wonder what Jens thinks about having a device idle timer 
and callback at the block level?  At the very least, this feels like it 
should be implemented in the SCSI layer, or somewhere other than libata.

This is NOT a NAK, however.  The code looks OK, and if consensus is 
against doing this sort of thing in block or SCSI, then I'm OK with the 
current patch.

Re: [PATCH 10/12] libata: implement standard powersave methods

[Jeff Garzik]

Tejun Heo wrote:
> Implement helpers to build SATA ->set_powersave() and HIPS timer, and
> use them to implement sata_std_set_powersave() and
> sata_std_hips_timer_fn() for standard SATA link powersave using
> SControl register.
> 
> Depending on controller capability, the following modes are supported.
> 
> none:		no powersave
> HIPS:		host-initiated partial/slumber (both or either one)
> DIPS:		device-initiated partial/slumber (both or either one)
> static:		link off by writing 0x4 to DET
> HIPS/static:	HIPS + static
> DIPS/static;	DIPS + static
> 
> Timeouts for HIPS can be modified using module parameters -
> libata.partial_timeout and libata.slumber_timeout.  Setting timeout to
> zero disables the powersave mode.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/libata-core.c |  280 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/libata.h     |   10 ++
>  2 files changed, 290 insertions(+), 0 deletions(-)
> 
> e19c560ab7e8fa9b20ed5ec3233fb75e0daceb63
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index 1658cd1..9890387 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -71,6 +71,7 @@ static unsigned int ata_dev_set_xfermode
>  static void ata_dev_xfermask(struct ata_device *dev);
>  
>  static int ata_param_set_powersave(const char *val, struct kernel_param *kp);
> +static int ata_param_set_hips_timeout(const char *val, struct kernel_param *kp);
>  
>  static DEFINE_MUTEX(ata_all_ports_mutex);
>  static LIST_HEAD(ata_all_ports);
> @@ -102,6 +103,18 @@ module_param_call(powersave, ata_param_s
>  MODULE_PARM_DESC(powersave, "Powersave mode (0=none, 1=HIPS, 2=DIPS, "
>  		 "3=static, 4=HIPS/static, 5=DIPS/static)");
>  
> +static unsigned long libata_partial_timeout = 100;
> +module_param_call(partial_timeout, ata_param_set_hips_timeout, param_get_ulong,
> +		  &libata_partial_timeout, 0644);
> +MODULE_PARM_DESC(hips_timeout, "Host-initiated partial powersave timeout "
> +		 "(milliseconds, default 100, 0 to disable)");
> +
> +static unsigned long libata_slumber_timeout = 3000;
> +module_param_call(slumber_timeout, ata_param_set_hips_timeout, param_get_ulong,
> +		  &libata_slumber_timeout, 0644);
> +MODULE_PARM_DESC(slumber_timeout, "Host-initiated slumber powersave timeout "
> +		 "(milliseconds, default 3000, 0 to disable)");
> +

Should be a per-controller variable, not a global libata module parm.

So NAK on interface, ACK on implementation behind the interface.

	Jeff

Re: [PATCH 11/12] ahci: implement link powersave

[Jeff Garzik]

ACK patches 11-12

Re: [PATCH 09/12] libata: implement powersave timer

[Jens Axboe]

On Wed, Jul 19 2006, Jeff Garzik wrote:
> Tejun Heo wrote:
> >Implement powersave timer.  It is primarily for OS-driven HIPS
> >implementation but can be used for any other PS purpose LLD sees fit.
> >During normal operation, PS timer is automatically started with
> >timeout ap->ps_timeout on port idle and stopped when the port becomes
> >busy.  The timer is also stopped while EH.
> >
> >To minimize overhead and allow easy implementation of expected
> >operation model, ata_ps_timer_worker() is used as timer callback which
> >invokes LLD supplied ap->ps_timer_fn() if condition meets and also
> >helps implementing sequenced multi-step operation.
> >
> >Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> This makes me wonder what Jens thinks about having a device idle timer 
> and callback at the block level?  At the very least, this feels like it 
> should be implemented in the SCSI layer, or somewhere other than libata.

Since this will be used only when the device is idle, we can reuse the
unplug timer and get it pretty much for free. So I think that's a good
idea.

-- 
Jens Axboe

Re: [PATCH 09/12] libata: implement powersave timer

[Tejun Heo]

Jens Axboe wrote:
> On Wed, Jul 19 2006, Jeff Garzik wrote:
>> Tejun Heo wrote:
>>> Implement powersave timer.  It is primarily for OS-driven HIPS
>>> implementation but can be used for any other PS purpose LLD sees fit.
>>> During normal operation, PS timer is automatically started with
>>> timeout ap->ps_timeout on port idle and stopped when the port becomes
>>> busy.  The timer is also stopped while EH.
>>>
>>> To minimize overhead and allow easy implementation of expected
>>> operation model, ata_ps_timer_worker() is used as timer callback which
>>> invokes LLD supplied ap->ps_timer_fn() if condition meets and also
>>> helps implementing sequenced multi-step operation.
>>>
>>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>> This makes me wonder what Jens thinks about having a device idle timer 
>> and callback at the block level?  At the very least, this feels like it 
>> should be implemented in the SCSI layer, or somewhere other than libata.
> 
> Since this will be used only when the device is idle, we can reuse the
> unplug timer and get it pretty much for free. So I think that's a good
> idea.
> 

Hello, Jeff, Jens.

I agree that the unplug timer can be reused here but I'm not very sure 
about what the high level semantics would be.  This timer tracks link 
idleness and also used to drive link power down sequence.  e.g. For 
host-initiated partial/slumber powersave, the timer will be used like 
the following.

1. Timer for partial PS armed when the link becomes idle (100ms by 
default).  The 'link' abstraction doesn't exist at the block layer yet, 
so this one is already problematic.  I'm thinking of using link-wide 
idleness to implement powersave on ata_piix (ICH8 can access SCRs 
without enabling AHCI address space, hooray!) and possibly PMP.  As 
ICH8s will end up on notebooks, supporting ata_piix is important.

2. On timer expiration, the PS handler is invoked and transits the link 
to partial power state.  As the link also supports slumber mode, it 
re-arms the timer w/ 3s timeout.

3. On timer expiration, the PS handler wakes up the link because SATA 
link cannot transit directly from partial to slumber.  As waking up 
takes time (1ms max by spec), the timer is rearmed (5ms).

4. On timer expiration, the link is put to slumber mode.

In addition, whenever there is any event on the link, the timer is 
reset.  Timer activation and canceling are tightly integrated into 
libata core layer to reduce overhead.

This is very SATA specific and I don't think pushing this to upper layer 
is a good idea.  Link powersave is closely related to transport topology 
and way too low level to be at block layer, IMHO.

Thanks.

-- 
tejun

Re: [PATCHSET] libata: implement runtime link powersave

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Hello, all.
>>
>> This is the first take of powersave patchset.  It implements runtime
>> link-level power management.  To avoid confusion with regular
>> device/controller power management (suspend/resume), it's callsed
>> powersave.  It primarily implements partial/slumber/phy-off power
>> states SATA standard specifies but if LLD has different way of
>> implementing link powersave, it's allowed to.  Both host-initiated and
>> device-initiated operations are supported.
>>
>> This patchset contains the following patches.
>>
>> #01-07    : misc fixes/preps for powersave
>> #08-10    : implements core powersave infrastructure
>> #11-12    : implement powersave feature for ahci and sata_sil24
>>
>> Powersave can be configured at boot time or while running by writing
>> to kernel module parameter /sys/module/libata/parameters/powersave.
>> The following powersave modes are supported.
>>
>> * none        : no powersave, link is powered up all the time
>> * HIPS        : host-initiated powersave
>> * DIPS        : device-initiated powersave
>> * static    : no powersave on occupied link, power off empty link
>> * HIPS/static    : HIPS on occupied link, power off empty link
>> * DIPS/static    : DIPS on occupied link, power off empty link
>>
>> For more information, please read head messages of individual patches.
>>
>> All supported modes are tested on ICH7R AHCI, ICH6M AHCI, SiI3124,
>> SiI3132.  Unfortunately SiI3112/4 family controllers don't support
>> HIPS and choke on DIPS and thus dropped from supported list.
> 
> Any chance you tried ata_piix static PS, using PCS register?  :)

No, but I think both static and dynamic powersave can be implemented 
nicely on ICH8s as SCRs can be accessed on those controllers without 
mapping AHCI BAR.  Hmm.. yeah, STATIC powersave mode can be done with 
PCS although I'm pretty scared to mess with PCS at this point.  :(

-- 
tejun

Re: [PATCH 08/12] libata: implement interface power management infrastructure

[Tejun Heo]

Hi,

Jeff Garzik wrote:
> NAK.
> 
> Module parameters in libata are largely for turning off/on major 
> features, during a transitional testing period.  They largely provide 
> users with workaround solutions to "it won't boot" type problems.
> 
> I also think you have been seduced by the relative ease of adding this 
> control to libata, as opposed to the preferred alternative:  digging 
> through sysfs objects to find the right one to which attributes should 
> be added.
> 
> As an aside, in addition to per-controller (or per-port) sysfs powersave 
> controls, libata should [eventually] recognize the external "laptop 
> mode" setting.  When laptop mode == ON, program powersave aggresively.

* I agree that cross-port synchronization is not a particularly nice 
thing to implement, but it's a nice thing to have from user's POV. 
Also, it is necessary to implement any kind of global control like 
laptop_mode.  I think it's a necessary evil.

* I want to export per-port and eventually per-link powersave interface 
but the problem is that libata doesn't have any sysfs representation 
other than what's supplied by SCSI, which means empty links don't have 
anything to represent them in sysfs.  Unfortunately, sysfs is tied to 
driver implementation ATM, so we can't start building sysfs tree on 
as-needed basis.

So, until sysfs representation is detached from internal driver data 
structure or ATA moves out of SCSI, I don't really see how we can add 
those fine-grained knobs.  Also, although having fine knobs is a good 
thing, I think powersave with global control goes a long way by itself 
and global control is needed with or without fine knobs.

However, one thing I can think of is to implement (yet) bogus ATA bus 
and add global controls there.  e.g. /sys/bus/ata/powersave.  What do 
you think about this?

* For global control, overloading laptop_mode doesn't seem to be a good 
idea.  That's a VM-specific knob with very specific role.  I think we 
should add system-wide sysfs node - maybe /sys/power/powersave? - which 
advises the kernel about power consumption.  It can propagate powersave 
requests using notifier chain and both VM and libata can hook into it to 
control laptop_mode and libata powersaving respectively.

> In general, though, we should peek to minimize power usage for the 
> general case, where feasible.  People with huge server closets like 
> Google need every ounce of power savings possible.

I'm not too sure about enabling powersave by default, if that's what you 
mean.  Yeah, people with large server farms will definitely benefit from 
powersave.  That's why I'm thinking of implementing full powersave 
including both HIPS and DIPS on PMP.  But powersave also has the 
possibility of causing malfunctions on devices which used to work okay.

I've seen sil3112 family of controllers malfunction when powersave kicks 
in and won't be surprised to see other controllers and drives show 
erratic behavior on powersave.

Another problem is that, however minute, powersave does introduce extra 
delay when waking up and thus can negatively affect performance.  The 
performance drop should be negligible on most non-SSD devices but it's 
still there.

> With regards to the patch content:  other than the control interface, 
> the implementation looks OK.

Nice, thanks.

-- 
tejun

Re: [PATCH 09/12] libata: implement powersave timer

[Jens Axboe]

On Mon, Jul 24 2006, Tejun Heo wrote:
> Jens Axboe wrote:
> >On Wed, Jul 19 2006, Jeff Garzik wrote:
> >>Tejun Heo wrote:
> >>>Implement powersave timer.  It is primarily for OS-driven HIPS
> >>>implementation but can be used for any other PS purpose LLD sees fit.
> >>>During normal operation, PS timer is automatically started with
> >>>timeout ap->ps_timeout on port idle and stopped when the port becomes
> >>>busy.  The timer is also stopped while EH.
> >>>
> >>>To minimize overhead and allow easy implementation of expected
> >>>operation model, ata_ps_timer_worker() is used as timer callback which
> >>>invokes LLD supplied ap->ps_timer_fn() if condition meets and also
> >>>helps implementing sequenced multi-step operation.
> >>>
> >>>Signed-off-by: Tejun Heo <htejun@gmail.com>
> >>This makes me wonder what Jens thinks about having a device idle timer 
> >>and callback at the block level?  At the very least, this feels like it 
> >>should be implemented in the SCSI layer, or somewhere other than libata.
> >
> >Since this will be used only when the device is idle, we can reuse the
> >unplug timer and get it pretty much for free. So I think that's a good
> >idea.
> >
> 
> Hello, Jeff, Jens.
> 
> I agree that the unplug timer can be reused here but I'm not very sure 
> about what the high level semantics would be.  This timer tracks link 
> idleness and also used to drive link power down sequence.  e.g. For 
> host-initiated partial/slumber powersave, the timer will be used like 
> the following.
> 
> 1. Timer for partial PS armed when the link becomes idle (100ms by 
> default).  The 'link' abstraction doesn't exist at the block layer yet, 
> so this one is already problematic.  I'm thinking of using link-wide 
> idleness to implement powersave on ata_piix (ICH8 can access SCRs 
> without enabling AHCI address space, hooray!) and possibly PMP.  As 
> ICH8s will end up on notebooks, supporting ata_piix is important.
> 
> 2. On timer expiration, the PS handler is invoked and transits the link 
> to partial power state.  As the link also supports slumber mode, it 
> re-arms the timer w/ 3s timeout.
> 
> 3. On timer expiration, the PS handler wakes up the link because SATA 
> link cannot transit directly from partial to slumber.  As waking up 
> takes time (1ms max by spec), the timer is rearmed (5ms).
> 
> 4. On timer expiration, the link is put to slumber mode.
> 
> In addition, whenever there is any event on the link, the timer is 
> reset.  Timer activation and canceling are tightly integrated into 
> libata core layer to reduce overhead.
> 
> This is very SATA specific and I don't think pushing this to upper layer 
> is a good idea.  Link powersave is closely related to transport topology 
> and way too low level to be at block layer, IMHO.

As long as there's a 1:1 mapping between link and disk, the block layer
can easily be used. When that isn't the case, I think we should do the
cleaner thing and leave the link management to the layer that has such
knowledge.

-- 
Jens Axboe