Skip to contentSkip to conversation

new Power Management for libata, take 2

PATCH In review Tejun Heo 3 participants 29 messages Jun 24, 2006 – Jun 27, 2006

Subsystems (2)
SCSI libata
lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 new Power Management for libata, take 2

Hello, all.

This is the second take of new-power-management patchset. Changes from the last take[1] are...

* Per-dev PM is now supported. Now generic device PM layer drives whole PM sequence and libata port-wide PM does much less.

* Suspend is now sequential - ie. harddrives hanging off a controller are spinned down one after another. Spinning down is pretty fast and this doesn't make much difference.

* Resume is still done asynchronously but it also uses generic PM to drive it.

* sata_sil and sata_sil24 are fixed to perform PCI resume before reinitializing the controller.

This patchset contains 15 patches.

#01-07 : preparation patches, misc updates & fixes #08-11 : new core PM implementation
#12-15 : convert LLDs to use new PM

As before, ata_piix, sata_sil and sata_sil24 are converted. Both mem and disk sleeps are tested and verifed to work on the following configurations.

* On P5LD2 + latest BIOS
- onboard ICH7R in ata_piix mode
- sil3112 PCI card
- sil3114 PCI card
- sil3124 PCI-x card on a PCI slot
- sil3132 PCI-e card

* On Fujitsu lifebook P7120 (+ PATA enabled + patch for PCI ID)
- onboard ICH6M in ata_piix mode
- sil3112 PCMCIA card

As in the last take, most of PM operations are performed by EH and fully synchronized with all other operations. Putting to sleep and waking up should be safe under any circumstances as far as libata is concerned.

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/11336

author
#1 lore raw

PATCH 1/15 libata: move ata_eh_clear_action() upward

Move ata_eh_clear_action() upward.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-eh.c |   50 +++++++++++++++++++++++-----------------------
 1 files changed, 25 insertions(+), 25 deletions(-)

5437c8195e49efeb2eed0aea79f5749e6c4a9521

drivers/scsi/libata-eh.c +25 -25
index 8233859..126be36 100644
@@ -93,6 +93,31 @@ static int ata_ering_map(struct ata_erin
 	return rc;
 }
 
+static void ata_eh_clear_action(struct ata_device *dev,
+				struct ata_eh_info *ehi, unsigned int action)
+{
+	int i;
+
+	if (!dev) {
+		ehi->action &= ~action;
+		for (i = 0; i < ATA_MAX_DEVICES; i++)
+			ehi->dev_action[i] &= ~action;
+	} else {
+		/* doesn't make sense for port-wide EH actions */
+		WARN_ON(!(action & ATA_EH_PERDEV_MASK));
+
+		/* break ehi->action into ehi->dev_action */
+		if (ehi->action & action) {
+			for (i = 0; i < ATA_MAX_DEVICES; i++)
+				ehi->dev_action[i] |= ehi->action & action;
+			ehi->action &= ~action;
+		}
+
+		/* turn off the specified per-dev action */
+		ehi->dev_action[dev->devno] &= ~action;
+	}
+}
+
 /**
  *	ata_scsi_timed_out - SCSI layer time out callback
  *	@cmd: timed out SCSI command

@@ -705,31 +730,6 @@ static void ata_eh_detach_dev(struct ata
 	spin_unlock_irqrestore(ap->lock, flags);
 }
 
-static void ata_eh_clear_action(struct ata_device *dev,
-				struct ata_eh_info *ehi, unsigned int action)
-{
-	int i;
-
-	if (!dev) {
-		ehi->action &= ~action;
-		for (i = 0; i < ATA_MAX_DEVICES; i++)
-			ehi->dev_action[i] &= ~action;
-	} else {
-		/* doesn't make sense for port-wide EH actions */
-		WARN_ON(!(action & ATA_EH_PERDEV_MASK));
-
-		/* break ehi->action into ehi->dev_action */
-		if (ehi->action & action) {
-			for (i = 0; i < ATA_MAX_DEVICES; i++)
-				ehi->dev_action[i] |= ehi->action & action;
-			ehi->action &= ~action;
-		}
-
-		/* turn off the specified per-dev action */
-		ehi->dev_action[dev->devno] &= ~action;
-	}
-}
-
 /**
  *	ata_eh_about_to_do - about to perform eh_action
  *	@ap: target ATA port
Signature · 1 line
1.3.2
author
#3 lore raw

PATCH 2/15 libata: implement and use ata_deh_dev_action()

In reply to Tejun Heo

Implement and use ata_eh_dev_action() which returns EH action mask for a device.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-eh.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

0b207dfe1d875f76363c79dbd0b4111f2cbd53f8

drivers/scsi/libata-eh.c +8 -1
index 126be36..c7e6298 100644
@@ -93,6 +93,13 @@ static int ata_ering_map(struct ata_erin
 	return rc;
 }
 
+static unsigned int ata_eh_dev_action(struct ata_device *dev)
+{
+	struct ata_eh_context *ehc = &dev->ap->eh_context;
+
+	return ehc->i.action | ehc->i.dev_action[dev->devno];
+}
+
 static void ata_eh_clear_action(struct ata_device *dev,
 				struct ata_eh_info *ehi, unsigned int action)
 {

@@ -1592,7 +1599,7 @@ static int ata_eh_revalidate_and_attach(
 		unsigned int action;
 
 		dev = &ap->device[i];
-		action = ehc->i.action | ehc->i.dev_action[dev->devno];
+		action = ata_eh_dev_action(dev);
 
 		if (action & ATA_EH_REVALIDATE && ata_dev_enabled(dev)) {
 			if (ata_port_offline(ap)) {
Signature · 1 line
1.3.2
author
#4 lore raw

PATCH 3/15 libata: clear EH action on device detach

In reply to Tejun Heo

Clear related EH action on device detach such that new device doesn't receive EH actions scheduled for the old one.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-eh.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

578e73a7401b8942425bc669ba0f4dcc1b6c8a45

drivers/scsi/libata-eh.c +4 -0
index c7e6298..bf5a72a 100644
@@ -734,6 +734,10 @@ static void ata_eh_detach_dev(struct ata
 		ap->flags |= ATA_FLAG_SCSI_HOTPLUG;
 	}
 
+	/* clear per-dev EH actions */
+	ata_eh_clear_action(dev, &ap->eh_info, ATA_EH_PERDEV_MASK);
+	ata_eh_clear_action(dev, &ap->eh_context.i, ATA_EH_PERDEV_MASK);
+
 	spin_unlock_irqrestore(ap->lock, flags);
 }
Signature · 1 line
1.3.2
author
#5 lore raw

PATCH 4/15 libata: move ata_do_simple_cmd() below ata_exec_internal()

In reply to Tejun Heo

Move ata_do_simple_cmd() below ata_exec_internal() for consistency.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   46 ++++++++++++++++++++++----------------------
 1 files changed, 23 insertions(+), 23 deletions(-)

6797d58704ea97b5c90e3e222a7c052dd65ccf8d

drivers/scsi/libata-core.c +23 -23
index 6c66877..bbd6665 100644
@@ -1131,6 +1131,29 @@ unsigned ata_exec_internal(struct ata_de
 	return err_mask;
 }
 
+/*
+ * Execute a 'simple' command, that only consists of the opcode 'cmd' itself,
+ * without filling any other registers
+ */
+static int ata_do_simple_cmd(struct ata_device *dev, u8 cmd)
+{
+	struct ata_taskfile tf;
+	int err;
+
+	ata_tf_init(dev, &tf);
+
+	tf.command = cmd;
+	tf.flags |= ATA_TFLAG_DEVICE;
+	tf.protocol = ATA_PROT_NODATA;
+
+	err = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
+	if (err)
+		ata_dev_printk(dev, KERN_ERR, "%s: ata command failed: %d\n",
+			       __FUNCTION__, err);
+
+	return err;
+}
+
 /**
  *	ata_pio_need_iordy	-	check if iordy needed
  *	@adev: ATA device

@@ -4946,29 +4969,6 @@ int ata_port_offline(struct ata_port *ap
 	return 0;
 }
 
-/*
- * Execute a 'simple' command, that only consists of the opcode 'cmd' itself,
- * without filling any other registers
- */
-static int ata_do_simple_cmd(struct ata_device *dev, u8 cmd)
-{
-	struct ata_taskfile tf;
-	int err;
-
-	ata_tf_init(dev, &tf);
-
-	tf.command = cmd;
-	tf.flags |= ATA_TFLAG_DEVICE;
-	tf.protocol = ATA_PROT_NODATA;
-
-	err = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
-	if (err)
-		ata_dev_printk(dev, KERN_ERR, "%s: ata command failed: %d\n",
-			       __FUNCTION__, err);
-
-	return err;
-}
-
 static int ata_flush_cache(struct ata_device *dev)
 {
 	u8 cmd;
Signature · 1 line
1.3.2
author
#6 lore raw

PATCH 5/15 libata: update ata_do_simple_cmd()

In reply to Tejun Heo

* the function has always returned AC_ERR_* masks not -errno but its return type was int. Make return type unsigned int.

* don't print error message automatically. it's the caller's responsibility.

* add header comment

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   57 +++++++++++++++++++++++++++++++++-----------
 1 files changed, 43 insertions(+), 14 deletions(-)

f078c744c2e4ab68aaaaca2183cdfc6a17d91a44

drivers/scsi/libata-core.c +43 -14
index bbd6665..4a44e75 100644
@@ -1131,14 +1131,23 @@ unsigned ata_exec_internal(struct ata_de
 	return err_mask;
 }
 
-/*
- * Execute a 'simple' command, that only consists of the opcode 'cmd' itself,
- * without filling any other registers
+/**
+ *	ata_do_simple_cmd - execute simple internal command
+ *	@dev: Device to which the command is sent
+ *	@cmd: Opcode to execute
+ *
+ *	Execute a 'simple' command, that only consists of the opcode
+ *	'cmd' itself, without filling any other registers
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ *
+ *	RETURNS:
+ *	Zero on success, AC_ERR_* mask on failure
  */
-static int ata_do_simple_cmd(struct ata_device *dev, u8 cmd)
+static unsigned int ata_do_simple_cmd(struct ata_device *dev, u8 cmd)
 {
 	struct ata_taskfile tf;
-	int err;
 
 	ata_tf_init(dev, &tf);
 

@@ -1146,12 +1155,7 @@ static int ata_do_simple_cmd(struct ata_
 	tf.flags |= ATA_TFLAG_DEVICE;
 	tf.protocol = ATA_PROT_NODATA;
 
-	err = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
-	if (err)
-		ata_dev_printk(dev, KERN_ERR, "%s: ata command failed: %d\n",
-			       __FUNCTION__, err);
-
-	return err;
+	return ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
 }
 
 /**

@@ -4971,6 +4975,7 @@ int ata_port_offline(struct ata_port *ap
 
 static int ata_flush_cache(struct ata_device *dev)
 {
+	unsigned int err_mask;
 	u8 cmd;
 
 	if (!ata_try_flush_cache(dev))

@@ -4981,17 +4986,41 @@ static int ata_flush_cache(struct ata_de
 	else
 		cmd = ATA_CMD_FLUSH;
 
-	return ata_do_simple_cmd(dev, cmd);
+	err_mask = ata_do_simple_cmd(dev, cmd);
+	if (err_mask) {
+		ata_dev_printk(dev, KERN_ERR, "failed to flush cache\n");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 static int ata_standby_drive(struct ata_device *dev)
 {
-	return ata_do_simple_cmd(dev, ATA_CMD_STANDBYNOW1);
+	unsigned int err_mask;
+
+	err_mask = ata_do_simple_cmd(dev, ATA_CMD_STANDBYNOW1);
+	if (err_mask) {
+		ata_dev_printk(dev, KERN_ERR, "failed to standby drive "
+			       "(err_mask=0x%x)\n", err_mask);
+		return -EIO;
+	}
+
+	return 0;
 }
 
 static int ata_start_drive(struct ata_device *dev)
 {
-	return ata_do_simple_cmd(dev, ATA_CMD_IDLEIMMEDIATE);
+	unsigned int err_mask;
+
+	err_mask = ata_do_simple_cmd(dev, ATA_CMD_IDLEIMMEDIATE);
+	if (err_mask) {
+		ata_dev_printk(dev, KERN_ERR, "failed to start drive "
+			       "(err_mask=0x%x)\n", err_mask);
+		return -EIO;
+	}
+
+	return 0;
 }
 
 /**
Signature · 1 line
1.3.2
author
#7 lore raw

PATCH 6/15 libata: make two functions global

In reply to Tejun Heo

Make ata_do_simple_cmd() and ata_flush_cache() global. These will be used from libata-eh.c.

Signed-off-by Tejun Heo <[email protected]>

---

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

e2e9ba22088d9511fb376e5e397f0515537ac5ab

drivers/scsi/libata-core.c +2 -2
index 4a44e75..60e80e3 100644
@@ -1145,7 +1145,7 @@ unsigned ata_exec_internal(struct ata_de
  *	RETURNS:
  *	Zero on success, AC_ERR_* mask on failure
  */
-static unsigned int ata_do_simple_cmd(struct ata_device *dev, u8 cmd)
+unsigned int ata_do_simple_cmd(struct ata_device *dev, u8 cmd)
 {
 	struct ata_taskfile tf;
 

@@ -4973,7 +4973,7 @@ int ata_port_offline(struct ata_port *ap
 	return 0;
 }
 
-static int ata_flush_cache(struct ata_device *dev)
+int ata_flush_cache(struct ata_device *dev)
 {
 	unsigned int err_mask;
 	u8 cmd;
drivers/scsi/libata.h +2 -0
index bdd4888..16ce23a 100644
@@ -50,6 +50,7 @@ extern void ata_port_flush_task(struct a
 extern unsigned ata_exec_internal(struct ata_device *dev,
 				  struct ata_taskfile *tf, const u8 *cdb,
 				  int dma_dir, void *buf, unsigned int buflen);
+extern unsigned int ata_do_simple_cmd(struct ata_device *dev, u8 cmd);
 extern int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
 			   int post_reset, u16 *id);
 extern int ata_dev_configure(struct ata_device *dev, int print_info);

@@ -64,6 +65,7 @@ extern int ata_check_atapi_dma(struct at
 extern void ata_dev_select(struct ata_port *ap, unsigned int device,
                            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_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);
Signature · 1 line
1.3.2
author
#8 lore raw

PATCH 7/15 libata: implement ata_port_max_devices()

In reply to Tejun Heo

Implement ata_port_max_devices(). This function returns the number of possible devices on a port. This will be used by new PM implementation.

Signed-off-by Tejun Heo <[email protected]>

---

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

af0dd1d6c6651ab9e8e3a9d8e7264daf57eff032

include/linux/libata.h +14 -0
index 20b1cf5..3afd009 100644
@@ -887,6 +887,9 @@ static inline unsigned int ata_tag_inter
 	return tag == ATA_MAX_QUEUE - 1;
 }
 
+/*
+ * device helpers
+ */
 static inline unsigned int ata_class_enabled(unsigned int class)
 {
 	return class == ATA_DEV_ATA || class == ATA_DEV_ATAPI;

@@ -917,6 +920,17 @@ static inline unsigned int ata_dev_absen
 	return ata_class_absent(dev->class);
 }
 
+/*
+ * port helpers
+ */
+static inline int ata_port_max_devices(const struct ata_port *ap)
+{
+	if (ap->flags & ATA_FLAG_SLAVE_POSS)
+		return 2;
+	return 1;
+}
+
+
 static inline u8 ata_chk_status(struct ata_port *ap)
 {
 	return ap->ops->check_status(ap);
Signature · 1 line
1.3.2
author
#9 lore raw

PATCH 8/15 libata: implement PM EH actions

In reply to Tejun Heo

Implement two PM per-dev EH actions - ATA_EH_SUSPEND and ATA_EH_RESUME. These two actions put the target device into suspended mode and resumes from it respectively.

Once a device is put to suspended mode, no EH operations other than RESUME is allowed on the device. The device will stay suspended till it gets resumed and thus reset and revalidated. To implement this, a new device state helper - ata_dev_ready() - is implemented and used over various EH action implementations to operate only on attached & running devices.

Also, if all possible devices on a port are suspended, reset is skipped too. This prevents spurious events including hotplug events from disrupting suspended devices.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |    5 +
 drivers/scsi/libata-eh.c   |  192 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h     |   11 ++-
 3 files changed, 203 insertions(+), 5 deletions(-)

48af6e94fe14a236934c3e3127ab2a5276ef54ee

drivers/scsi/libata-core.c +3 -2
index 60e80e3..ab6ae1f 100644
@@ -2131,7 +2131,7 @@ int ata_set_mode(struct ata_port *ap, st
 		 * return error code and failing device on failure.
 		 */
 		for (i = 0; i < ATA_MAX_DEVICES; i++) {
-			if (ata_dev_enabled(&ap->device[i])) {
+			if (ata_dev_ready(&ap->device[i])) {
 				ap->ops->set_mode(ap);
 				break;
 			}

@@ -2197,7 +2197,8 @@ int ata_set_mode(struct ata_port *ap, st
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		dev = &ap->device[i];
 
-		if (!ata_dev_enabled(dev))
+		/* don't udpate suspended devices' xfer mode */
+		if (!ata_dev_ready(dev))
 			continue;
 
 		rc = ata_dev_set_mode(dev);
drivers/scsi/libata-eh.c +190 -2

235 lines not on this page · raw message

include/linux/libata.h +10 -1
index 3afd009..6b582da 100644
@@ -130,6 +130,7 @@ enum {
 	ATA_DFLAG_CFG_MASK	= (1 << 8) - 1,
 
 	ATA_DFLAG_PIO		= (1 << 8), /* device currently in PIO mode */
+	ATA_DFLAG_SUSPENDED	= (1 << 9), /* device suspended */
 	ATA_DFLAG_INIT_MASK	= (1 << 16) - 1,
 
 	ATA_DFLAG_DETACH	= (1 << 16),

@@ -247,9 +248,12 @@ enum {
 	ATA_EH_REVALIDATE	= (1 << 0),
 	ATA_EH_SOFTRESET	= (1 << 1),
 	ATA_EH_HARDRESET	= (1 << 2),
+	ATA_EH_SUSPEND		= (1 << 3),
+	ATA_EH_RESUME		= (1 << 4),
 
 	ATA_EH_RESET_MASK	= ATA_EH_SOFTRESET | ATA_EH_HARDRESET,
-	ATA_EH_PERDEV_MASK	= ATA_EH_REVALIDATE,
+	ATA_EH_PERDEV_MASK	= ATA_EH_REVALIDATE |
+				  ATA_EH_SUSPEND | ATA_EH_RESUME,
 
 	/* ata_eh_info->flags */
 	ATA_EHI_HOTPLUGGED	= (1 << 0),  /* could have been hotplugged */

@@ -920,6 +924,11 @@ static inline unsigned int ata_dev_absen
 	return ata_class_absent(dev->class);
 }
 
+static inline unsigned int ata_dev_ready(const struct ata_device *dev)
+{
+	return ata_dev_enabled(dev) && !(dev->flags & ATA_DFLAG_SUSPENDED);
+}
+
 /*
  * port helpers
  */
Signature · 1 line
1.3.2
author
#10 lore raw

PATCH 9/15 libata: reimplement per-dev PM

In reply to Tejun Heo

Reimplement per-dev PM. The original implementation directly put the device into suspended mode and didn't synchronize w/ EH operations including hotplug. This patch reimplements ata_scsi_device_suspend() and ata_scsi_device_resume() such that they request EH to perform the respective operations. Both functions synchronize with hotplug such that it doesn't affect already detached devices.

Suspend wait for completion but resume just issues request and returns. This allows parallel wake up of devices and thus speeds up system resume.

Due to sdev detach synchronization, it's not feasible to separate out EH requesting from sdev handling; thus, ata_device_suspend/resume() are removed and everything is implemented in respective libata-scsi functions.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   84 ---------------------------------
 drivers/scsi/libata-scsi.c |  112 ++++++++++++++++++++++++++++++++++++++++++--
 include/linux/libata.h     |   11 ++--
 3 files changed, 111 insertions(+), 96 deletions(-)

e917c5f9fe9ec0a8dc22425a84246f68f436a785

drivers/scsi/libata-core.c +0 -84
index ab6ae1f..5e9108e 100644
@@ -4996,88 +4996,6 @@ int ata_flush_cache(struct ata_device *d
 	return 0;
 }
 
-static int ata_standby_drive(struct ata_device *dev)
-{
-	unsigned int err_mask;
-
-	err_mask = ata_do_simple_cmd(dev, ATA_CMD_STANDBYNOW1);
-	if (err_mask) {
-		ata_dev_printk(dev, KERN_ERR, "failed to standby drive "
-			       "(err_mask=0x%x)\n", err_mask);
-		return -EIO;
-	}
-
-	return 0;
-}
-
-static int ata_start_drive(struct ata_device *dev)
-{
-	unsigned int err_mask;
-
-	err_mask = ata_do_simple_cmd(dev, ATA_CMD_IDLEIMMEDIATE);
-	if (err_mask) {
-		ata_dev_printk(dev, KERN_ERR, "failed to start drive "
-			       "(err_mask=0x%x)\n", err_mask);
-		return -EIO;
-	}
-
-	return 0;
-}
-
-/**
- *	ata_device_resume - wakeup a previously suspended devices
- *	@dev: the device to resume
- *
- *	Kick the drive back into action, by sending it an idle immediate
- *	command and making sure its transfer mode matches between drive
- *	and host.
- *
- */
-int ata_device_resume(struct ata_device *dev)
-{
-	struct ata_port *ap = dev->ap;
-
-	if (ap->flags & ATA_FLAG_SUSPENDED) {
-		struct ata_device *failed_dev;
-
-		ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT);
-		ata_busy_wait(ap, ATA_BUSY | ATA_DRQ, 200000);
-
-		ap->flags &= ~ATA_FLAG_SUSPENDED;
-		while (ata_set_mode(ap, &failed_dev))
-			ata_dev_disable(failed_dev);
-	}
-	if (!ata_dev_enabled(dev))
-		return 0;
-	if (dev->class == ATA_DEV_ATA)
-		ata_start_drive(dev);
-
-	return 0;
-}
-
-/**
- *	ata_device_suspend - prepare a device for suspend
- *	@dev: the device to suspend
- *	@state: target power management state
- *
- *	Flush the cache on the drive, if appropriate, then issue a
- *	standbynow command.
- */
-int ata_device_suspend(struct ata_device *dev, pm_message_t state)
-{
-	struct ata_port *ap = dev->ap;
-
-	if (!ata_dev_enabled(dev))
-		return 0;
-	if (dev->class == ATA_DEV_ATA)
-		ata_flush_cache(dev);
-
-	if (state.event != PM_EVENT_FREEZE)
-		ata_standby_drive(dev);
-	ap->flags |= ATA_FLAG_SUSPENDED;
-	return 0;
-}
-
 /**
  *	ata_port_start - Set port up for dma.
  *	@ap: Port to initialize

@@ -5930,8 +5848,6 @@ EXPORT_SYMBOL_GPL(ata_pci_default_filter
 EXPORT_SYMBOL_GPL(ata_pci_clear_simplex);
 #endif /* CONFIG_PCI */
 
-EXPORT_SYMBOL_GPL(ata_device_suspend);
-EXPORT_SYMBOL_GPL(ata_device_resume);
 EXPORT_SYMBOL_GPL(ata_scsi_device_suspend);
 EXPORT_SYMBOL_GPL(ata_scsi_device_resume);
drivers/scsi/libata-scsi.c +106 -6

126 lines not on this page · raw message

include/linux/libata.h +5 -6
index 6b582da..0bd3abe 100644
@@ -248,12 +248,13 @@ enum {
 	ATA_EH_REVALIDATE	= (1 << 0),
 	ATA_EH_SOFTRESET	= (1 << 1),
 	ATA_EH_HARDRESET	= (1 << 2),
-	ATA_EH_SUSPEND		= (1 << 3),
-	ATA_EH_RESUME		= (1 << 4),
+	ATA_EH_SUSPEND		= (1 << 4),
+	ATA_EH_RESUME		= (1 << 5),
+	ATA_EH_PM_FREEZE	= (1 << 6), /* freeze, not actual suspend */
 
 	ATA_EH_RESET_MASK	= ATA_EH_SOFTRESET | ATA_EH_HARDRESET,
-	ATA_EH_PERDEV_MASK	= ATA_EH_REVALIDATE |
-				  ATA_EH_SUSPEND | ATA_EH_RESUME,
+	ATA_EH_PERDEV_MASK	= ATA_EH_REVALIDATE | ATA_EH_SUSPEND |
+				  ATA_EH_RESUME | ATA_EH_PM_FREEZE,
 
 	/* ata_eh_info->flags */
 	ATA_EHI_HOTPLUGGED	= (1 << 0),  /* could have been hotplugged */

@@ -667,8 +668,6 @@ extern int ata_port_online(struct ata_po
 extern int ata_port_offline(struct ata_port *ap);
 extern int ata_scsi_device_resume(struct scsi_device *);
 extern int ata_scsi_device_suspend(struct scsi_device *, pm_message_t state);
-extern int ata_device_resume(struct ata_device *);
-extern int ata_device_suspend(struct ata_device *, pm_message_t state);
 extern int ata_ratelimit(void);
 extern unsigned int ata_busy_sleep(struct ata_port *ap,
 				   unsigned long timeout_pat,
Signature · 1 line
1.3.2
author
#11 lore raw

PATCH 10/15 libata: move ata_flush_cache() from libata-core.c to libata-eh.c

In reply to Tejun Heo

With recent PM reimplementation, there's no user left in libata-core.c. Move the function to libata-eh.c and make it static.

Signed-off-by Tejun Heo <[email protected]>

---

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

31f414d15d2994866853e48ee4d9c94d7dd21a59

drivers/scsi/libata-core.c +0 -22
index 5e9108e..1aa8a03 100644
@@ -4974,28 +4974,6 @@ int ata_port_offline(struct ata_port *ap
 	return 0;
 }
 
-int ata_flush_cache(struct ata_device *dev)
-{
-	unsigned int err_mask;
-	u8 cmd;
-
-	if (!ata_try_flush_cache(dev))
-		return 0;
-
-	if (ata_id_has_flush_ext(dev->id))
-		cmd = ATA_CMD_FLUSH_EXT;
-	else
-		cmd = ATA_CMD_FLUSH;
-
-	err_mask = ata_do_simple_cmd(dev, cmd);
-	if (err_mask) {
-		ata_dev_printk(dev, KERN_ERR, "failed to flush cache\n");
-		return -EIO;
-	}
-
-	return 0;
-}
-
 /**
  *	ata_port_start - Set port up for dma.
  *	@ap: Port to initialize
drivers/scsi/libata-eh.c +22 -0
index f05334a..1c2b48c 100644
@@ -1653,6 +1653,28 @@ static int ata_eh_revalidate_and_attach(
 	return rc;
 }
 
+static int ata_flush_cache(struct ata_device *dev)
+{
+	unsigned int err_mask;
+	u8 cmd;
+
+	if (!ata_try_flush_cache(dev))
+		return 0;
+
+	if (ata_id_has_flush_ext(dev->id))
+		cmd = ATA_CMD_FLUSH_EXT;
+	else
+		cmd = ATA_CMD_FLUSH;
+
+	err_mask = ata_do_simple_cmd(dev, cmd);
+	if (err_mask) {
+		ata_dev_printk(dev, KERN_ERR, "failed to flush cache\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
 /**
  *	ata_eh_suspend - handle suspend EH action
  *	@ap: target host port
drivers/scsi/libata.h +0 -1
index 16ce23a..539b0b5 100644
@@ -65,7 +65,6 @@ extern int ata_check_atapi_dma(struct at
 extern void ata_dev_select(struct ata_port *ap, unsigned int device,
                            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_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);
Signature · 1 line
1.3.2
author
#12 lore raw

PATCH 11/15 libata: reimplement controller-wide PM

In reply to Tejun Heo

Reimplement controller-wide PM. ata_host_set_suspend/resume() are defined to suspend and resume a host_set. While suspended, EHs for all ports in the host_set are pegged using ATA_FLAG_SUSPENDED and frozen.

Because SCSI device hotplug is done asynchronously against the rest of libata EH and the same mutex is used when adding new device, suspend cannot wait for hotplug to complete. So, if SCSI device hotplug is in progress, suspend fails with -EBUSY.

In most cases, host_set resume is followed by device resume. As each resume operation requires a reset, a single host_set-wide resume operation may result in multiple resets. To avoid this, resume waits upto 1 second giving PM to request resume for devices.

There is no room left for additional port flag, but several bits are schedueled to be removed. This patch adds ATA_FLAG_RESUME as bit 31 which is reserved for LLD flags. No in-kernel LLD uses bit 31 now, so this works for the time being and we can change it once some of the obsolete bits are removed.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |  138 ++++++++++++++++++++++++++++++++++++++++++--
 drivers/scsi/libata-eh.c   |   52 ++++++++++++++++-
 include/linux/libata.h     |   10 +++
 3 files changed, 192 insertions(+), 8 deletions(-)

5d5811d2d817853e94a5b7be2f7554c5109b5047

drivers/scsi/libata-core.c +133 -5
index 1aa8a03..91f34ac 100644
@@ -4975,6 +4975,95 @@ int ata_port_offline(struct ata_port *ap
 }
 
 /**
+ *	ata_host_set_suspend - suspend host_set
+ *	@host_set: host_set to suspend
+ *	@mesg: PM message
+ *
+ *	Suspend @host_set.  Actual operation is performed by EH.  This
+ *	function requests EH to perform PM operations and waits for EH
+ *	to finish.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, -errno on failure.
+ */
+int ata_host_set_suspend(struct ata_host_set *host_set, pm_message_t mesg)
+{
+	unsigned long flags;
+	int i, j;
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		/* set SUSPENDED and make sure EH sees it */
+		spin_lock_irqsave(&ap->host_set->lock, flags);
+		ap->flags |= ATA_FLAG_SUSPENDED;
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+		ata_port_wait_eh(ap);
+
+		/* EH is quiescent now.  Fail if we have any ready
+		 * device.  This happens if hotplug occurs between
+		 * completion of device suspension and here.
+		 */
+		for (j = 0; j < ATA_MAX_DEVICES; j++) {
+			struct ata_device *dev = &ap->device[j];
+
+			if (ata_dev_ready(dev)) {
+				ata_port_printk(ap, KERN_INFO,
+						"suspend failed, device %d "
+						"still active\n", dev->devno);
+				goto fail;
+			}
+		}
+
+		/* freeze, won't be thawed until resume */
+		spin_lock_irqsave(&ap->host_set->lock, flags);
+		ata_port_freeze(ap);
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+	}
+
+	host_set->dev->power.power_state = mesg;
+	return 0;
+
+ fail:
+	ata_host_set_resume(host_set);
+	return -EBUSY;
+}
+
+/**
+ *	ata_host_set_resume - resume host_set
+ *	@host_set: host_set to resume
+ *
+ *	Resume @host_set.  Actual operation is performed by EH.  This
+ *	function requests EH to perform PM operations and returns.
+ *	Note that all resume operations are performed parallely.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ */
+void ata_host_set_resume(struct ata_host_set *host_set)
+{
+	unsigned long flags;
+	int i;
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		/* request resume & kick EH in the ass */
+		spin_lock_irqsave(&ap->host_set->lock, flags);
+		ap->flags |= ATA_FLAG_RESUME;
+		ata_port_schedule_eh(ap);
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+	}
+
+	host_set->dev->power.power_state = PMSG_ON;
+	return;
+}
+
+/**
  *	ata_port_start - Set port up for dma.
  *	@ap: Port to initialize
  *

@@ -5614,20 +5703,55 @@ int pci_test_config_bits(struct pci_dev 
 	return (tmp == bits->val) ? 1 : 0;
 }
 
-int ata_pci_device_suspend(struct pci_dev *pdev, pm_message_t state)
+void ata_pci_device_do_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	pci_save_state(pdev);
-	pci_disable_device(pdev);
-	pci_set_power_state(pdev, PCI_D3hot);
-	return 0;
+
+	if (state.event == PM_EVENT_SUSPEND) {
+		pci_disable_device(pdev);
+		pci_set_power_state(pdev, PCI_D3hot);
+	}
 }
 
-int ata_pci_device_resume(struct pci_dev *pdev)
+void ata_pci_device_do_resume(struct pci_dev *pdev)
 {
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
 	pci_enable_device(pdev);
 	pci_set_master(pdev);
+}
+
+int ata_pci_device_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	int rc = 0;
+
+	rc = ata_host_set_suspend(host_set, state);
+	if (rc)
+		return rc;
+
+	if (host_set->next) {
+		rc = ata_host_set_suspend(host_set->next, state);
+		if (rc) {
+			ata_host_set_resume(host_set);
+			return rc;
+		}
+	}
+
+	ata_pci_device_do_suspend(pdev, state);
+
+	return 0;
+}
+
+int ata_pci_device_resume(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+
+	ata_pci_device_do_resume(pdev);
+	ata_host_set_resume(host_set);
+	if (host_set->next)
+		ata_host_set_resume(host_set->next);
+
 	return 0;
 }
 #endif /* CONFIG_PCI */

@@ -5806,6 +5930,8 @@ EXPORT_SYMBOL_GPL(sata_scr_write);
 EXPORT_SYMBOL_GPL(sata_scr_write_flush);
 EXPORT_SYMBOL_GPL(ata_port_online);
 EXPORT_SYMBOL_GPL(ata_port_offline);
+EXPORT_SYMBOL_GPL(ata_host_set_suspend);
+EXPORT_SYMBOL_GPL(ata_host_set_resume);
 EXPORT_SYMBOL_GPL(ata_id_string);
 EXPORT_SYMBOL_GPL(ata_id_c_string);
 EXPORT_SYMBOL_GPL(ata_scsi_simulate);

@@ -5820,6 +5946,8 @@ EXPORT_SYMBOL_GPL(ata_pci_host_stop);
 EXPORT_SYMBOL_GPL(ata_pci_init_native_mode);
 EXPORT_SYMBOL_GPL(ata_pci_init_one);
 EXPORT_SYMBOL_GPL(ata_pci_remove_one);
+EXPORT_SYMBOL_GPL(ata_pci_device_do_suspend);
+EXPORT_SYMBOL_GPL(ata_pci_device_do_resume);
 EXPORT_SYMBOL_GPL(ata_pci_device_suspend);
 EXPORT_SYMBOL_GPL(ata_pci_device_resume);
 EXPORT_SYMBOL_GPL(ata_pci_default_filter);
drivers/scsi/libata-eh.c +49 -3

72 lines not on this page · raw message

include/linux/libata.h +10 -0
index 0bd3abe..16c8fcc 100644
@@ -175,6 +175,11 @@ enum {
 	ATA_FLAG_DISABLED	= (1 << 22), /* port is disabled, ignore it */
 	ATA_FLAG_SUSPENDED	= (1 << 23), /* port is suspended (power) */
 
+	/* XXX - Bit 31 is reserved for LLDD but no LLDD is using it
+	 * ATM.  Steal it for the time being.  The following flag must
+	 * be updated once above obsolete flags are removed. */
+	ATA_FLAG_RESUME		= (1 << 31), /* resume */
+
 	/* bits 24:31 of ap->flags are reserved for LLDD specific flags */
 
 	/* struct ata_queued_cmd flags */

@@ -648,6 +653,8 @@ #ifdef CONFIG_PCI
 extern int ata_pci_init_one (struct pci_dev *pdev, struct ata_port_info **port_info,
 			     unsigned int n_ports);
 extern void ata_pci_remove_one (struct pci_dev *pdev);
+extern void ata_pci_device_do_suspend(struct pci_dev *pdev, pm_message_t state);
+extern void ata_pci_device_do_resume(struct pci_dev *pdev);
 extern int ata_pci_device_suspend(struct pci_dev *pdev, pm_message_t state);
 extern int ata_pci_device_resume(struct pci_dev *pdev);
 extern int ata_pci_clear_simplex(struct pci_dev *pdev);

@@ -668,6 +675,9 @@ extern int ata_port_online(struct ata_po
 extern int ata_port_offline(struct ata_port *ap);
 extern int ata_scsi_device_resume(struct scsi_device *);
 extern int ata_scsi_device_suspend(struct scsi_device *, pm_message_t state);
+extern int ata_host_set_suspend(struct ata_host_set *host_set,
+				pm_message_t mesg);
+extern void ata_host_set_resume(struct ata_host_set *host_set);
 extern int ata_ratelimit(void);
 extern unsigned int ata_busy_sleep(struct ata_port *ap,
 				   unsigned long timeout_pat,
Signature · 1 line
1.3.2
#13 lore raw

Re: [PATCH 11/15] libata: reimplement controller-wide PM

Quoted 50 lines · Tejun Heo

/**
+ * ata_host_set_suspend - suspend host_set
+ * @host_set: host_set to suspend
+ * @mesg: PM message
+ *
+ * Suspend @host_set. Actual operation is performed by EH. This
+ * function requests EH to perform PM operations and waits for EH
+ * to finish.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */

drivers/scsi/libata-core.c +34 -0
+int ata_host_set_suspend(struct ata_host_set *host_set, pm_message_t mesg)
+{
+	unsigned long flags;
+	int i, j;
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		/* set SUSPENDED and make sure EH sees it */
+		spin_lock_irqsave(&ap->host_set->lock, flags);
+		ap->flags |= ATA_FLAG_SUSPENDED;
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+		ata_port_wait_eh(ap);
+
+		/* EH is quiescent now.  Fail if we have any ready
+		 * device.  This happens if hotplug occurs between
+		 * completion of device suspension and here.
+		 */
+		for (j = 0; j < ATA_MAX_DEVICES; j++) {
+			struct ata_device *dev = &ap->device[j];
+
+			if (ata_dev_ready(dev)) {
+				ata_port_printk(ap, KERN_INFO,
+						"suspend failed, device %d "
+						"still active\n", dev->devno);
+				goto fail;
+			}
+		}
+
+		/* freeze, won't be thawed until resume */
+		spin_lock_irqsave(&ap->host_set->lock, flags);
+		ata_port_freeze(ap);
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);

drivers/scsi/libata-core.c int ata_port_offline(struct ata_port *ap · line 5035

Quoted 10 lines
drivers/scsi/libata-core.c +10 -0
+	}
+
+	host_set->dev->power.power_state = mesg;
+	return 0;
+
+ fail:
+	ata_host_set_resume(host_set);
+	return -EBUSY;
+}
+

ata_port_schedule_eh(ap); need to be invoked in order to schedule EH, right? But ata_port_schedule_eh(ap) is not called in ata_host_set_suspend().

Thanks,
Forrest

author
#14 lore raw

Re: [PATCH 11/15] libata: reimplement controller-wide PM

zhao, forrest wrote:
ort_schedule_eh(ap); need to be invoked in order to schedule EH,

Quoted 1 line

right?

Right.

Quoted 1 line

But ata_port_schedule_eh(ap) is not called in ata_host_set_suspend().

Because ata_host_set_suspend() doesn't need to invoke EH. It just needs to make sure that EH sees ATA_FLAG_SUSPENDED which is achieved by flushing EH after setting SUSPENDED.

Signature · 1 line
tejun
author
#15 lore raw

PATCH 12/15 sata_sil: separate out sil_init_controller()

In reply to Tejun Heo

Separate out controller initialization from sil_init_one() into sil_init_controller(). This will be used by resume.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/sata_sil.c |   86 ++++++++++++++++++++++++++---------------------
 1 files changed, 48 insertions(+), 38 deletions(-)

a8e0d30d07481375e455c83a4f55c05ccc6ffc2e

drivers/scsi/sata_sil.c +48 -38
index bc9f918..5c78fe8 100644
@@ -544,6 +544,52 @@ static void sil_dev_config(struct ata_po
 	}
 }
 
+static void sil_init_controller(struct pci_dev *pdev,
+				int n_ports, unsigned long host_flags,
+				void __iomem *mmio_base)
+{
+	u8 cls;
+	u32 tmp;
+	int i;
+
+	/* Initialize FIFO PCI bus arbitration */
+	cls = sil_get_device_cache_line(pdev);
+	if (cls) {
+		cls >>= 3;
+		cls++;  /* cls = (line_size/8)+1 */
+		for (i = 0; i < n_ports; i++)
+			writew(cls << 8 | cls,
+			       mmio_base + sil_port[i].fifo_cfg);
+	} else
+		dev_printk(KERN_WARNING, &pdev->dev,
+			   "cache line size not set.  Driver may not function\n");
+
+	/* Apply R_ERR on DMA activate FIS errata workaround */
+	if (host_flags & SIL_FLAG_RERR_ON_DMA_ACT) {
+		int cnt;
+
+		for (i = 0, cnt = 0; i < n_ports; i++) {
+			tmp = readl(mmio_base + sil_port[i].sfis_cfg);
+			if ((tmp & 0x3) != 0x01)
+				continue;
+			if (!cnt)
+				dev_printk(KERN_INFO, &pdev->dev,
+					   "Applying R_ERR on DMA activate "
+					   "FIS errata fix\n");
+			writel(tmp & ~0x3, mmio_base + sil_port[i].sfis_cfg);
+			cnt++;
+		}
+	}
+
+	if (n_ports == 4) {
+		/* flip the magic "make 4 ports work" bit */
+		tmp = readl(mmio_base + sil_port[2].bmdma);
+		if ((tmp & SIL_INTR_STEERING) == 0)
+			writel(tmp | SIL_INTR_STEERING,
+			       mmio_base + sil_port[2].bmdma);
+	}
+}
+
 static int sil_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version;

@@ -553,8 +599,6 @@ static int sil_init_one (struct pci_dev 
 	int rc;
 	unsigned int i;
 	int pci_dev_busy = 0;
-	u32 tmp;
-	u8 cls;
 
 	if (!printed_version++)
 		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");

@@ -613,42 +657,8 @@ static int sil_init_one (struct pci_dev 
 		ata_std_ports(&probe_ent->port[i]);
 	}
 
-	/* Initialize FIFO PCI bus arbitration */
-	cls = sil_get_device_cache_line(pdev);
-	if (cls) {
-		cls >>= 3;
-		cls++;  /* cls = (line_size/8)+1 */
-		for (i = 0; i < probe_ent->n_ports; i++)
-			writew(cls << 8 | cls,
-			       mmio_base + sil_port[i].fifo_cfg);
-	} else
-		dev_printk(KERN_WARNING, &pdev->dev,
-			   "cache line size not set.  Driver may not function\n");
-
-	/* Apply R_ERR on DMA activate FIS errata workaround */
-	if (probe_ent->host_flags & SIL_FLAG_RERR_ON_DMA_ACT) {
-		int cnt;
-
-		for (i = 0, cnt = 0; i < probe_ent->n_ports; i++) {
-			tmp = readl(mmio_base + sil_port[i].sfis_cfg);
-			if ((tmp & 0x3) != 0x01)
-				continue;
-			if (!cnt)
-				dev_printk(KERN_INFO, &pdev->dev,
-					   "Applying R_ERR on DMA activate "
-					   "FIS errata fix\n");
-			writel(tmp & ~0x3, mmio_base + sil_port[i].sfis_cfg);
-			cnt++;
-		}
-	}
-
-	if (ent->driver_data == sil_3114) {
-		/* flip the magic "make 4 ports work" bit */
-		tmp = readl(mmio_base + sil_port[2].bmdma);
-		if ((tmp & SIL_INTR_STEERING) == 0)
-			writel(tmp | SIL_INTR_STEERING,
-			       mmio_base + sil_port[2].bmdma);
-	}
+	sil_init_controller(pdev, probe_ent->n_ports, probe_ent->host_flags,
+			    mmio_base);
 
 	pci_set_master(pdev);
Signature · 1 line
1.3.2
author
#16 lore raw

PATCH 13/15 sata_sil: add suspend/sleep support

In reply to Tejun Heo

Add suspend/sleep support.

Signed-off-by Tejun Heo <[email protected]>

---

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

f42d5b06e40d39a08132daaca396be63ed1e5fde

drivers/scsi/sata_sil.c +17 -0
index 5c78fe8..d20e3c0 100644
@@ -107,6 +107,7 @@ enum {
 };
 
 static int sil_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
+static int sil_pci_device_resume(struct pci_dev *pdev);
 static void sil_dev_config(struct ata_port *ap, struct ata_device *dev);
 static u32 sil_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void sil_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);

@@ -158,6 +159,8 @@ static struct pci_driver sil_pci_driver 
 	.id_table		= sil_pci_tbl,
 	.probe			= sil_init_one,
 	.remove			= ata_pci_remove_one,
+	.suspend		= ata_pci_device_suspend,
+	.resume			= sil_pci_device_resume,
 };
 
 static struct scsi_host_template sil_sht = {

@@ -176,6 +179,8 @@ static struct scsi_host_template sil_sht
 	.slave_configure	= ata_scsi_slave_config,
 	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
+	.suspend		= ata_scsi_device_suspend,
+	.resume			= ata_scsi_device_resume,
 };
 
 static const struct ata_port_operations sil_ops = {

@@ -678,6 +683,18 @@ err_out:
 	return rc;
 }
 
+static int sil_pci_device_resume(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+
+	ata_pci_device_do_resume(pdev);
+	sil_init_controller(pdev, host_set->n_ports, host_set->ports[0]->flags,
+			    host_set->mmio_base);
+	ata_host_set_resume(host_set);
+
+	return 0;
+}
+
 static int __init sil_init(void)
 {
 	return pci_module_init(&sil_pci_driver);
Signature · 1 line
1.3.2
author
#17 lore raw

PATCH 14/15 sata_sil24: separate out sil24_init_controller()

In reply to Tejun Heo

Separate out controller initialization from sil24_init_one() into sil24_init_controller(). This will be used by resume.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/sata_sil24.c |  107 ++++++++++++++++++++++++++-------------------
 1 files changed, 62 insertions(+), 45 deletions(-)

6f284e7faf0222cd0187f5518ac18d8a53019530

drivers/scsi/sata_sil24.c +62 -45
index c8b477c..7726e6e 100644
@@ -988,6 +988,64 @@ static void sil24_host_stop(struct ata_h
 	kfree(hpriv);
 }
 
+static void sil24_init_controller(struct pci_dev *pdev, int n_ports,
+				  unsigned long host_flags,
+				  void __iomem *host_base,
+				  void __iomem *port_base)
+{
+	u32 tmp;
+	int i;
+
+	/* GPIO off */
+	writel(0, host_base + HOST_FLASH_CMD);
+
+	/* clear global reset & mask interrupts during initialization */
+	writel(0, host_base + HOST_CTRL);
+
+	/* init ports */
+	for (i = 0; i < n_ports; i++) {
+		void __iomem *port = port_base + i * PORT_REGS_SIZE;
+
+		/* Initial PHY setting */
+		writel(0x20c, port + PORT_PHY_CFG);
+
+		/* Clear port RST */
+		tmp = readl(port + PORT_CTRL_STAT);
+		if (tmp & PORT_CS_PORT_RST) {
+			writel(PORT_CS_PORT_RST, port + PORT_CTRL_CLR);
+			tmp = ata_wait_register(port + PORT_CTRL_STAT,
+						PORT_CS_PORT_RST,
+						PORT_CS_PORT_RST, 10, 100);
+			if (tmp & PORT_CS_PORT_RST)
+				dev_printk(KERN_ERR, &pdev->dev,
+				           "failed to clear port RST\n");
+		}
+
+		/* Configure IRQ WoC */
+		if (host_flags & SIL24_FLAG_PCIX_IRQ_WOC)
+			writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_STAT);
+		else
+			writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_CLR);
+
+		/* Zero error counters. */
+		writel(0x8000, port + PORT_DECODE_ERR_THRESH);
+		writel(0x8000, port + PORT_CRC_ERR_THRESH);
+		writel(0x8000, port + PORT_HSHK_ERR_THRESH);
+		writel(0x0000, port + PORT_DECODE_ERR_CNT);
+		writel(0x0000, port + PORT_CRC_ERR_CNT);
+		writel(0x0000, port + PORT_HSHK_ERR_CNT);
+
+		/* Always use 64bit activation */
+		writel(PORT_CS_32BIT_ACTV, port + PORT_CTRL_CLR);
+
+		/* Clear port multiplier enable and resume bits */
+		writel(PORT_CS_PM_EN | PORT_CS_RESUME, port + PORT_CTRL_CLR);
+	}
+
+	/* Turn on interrupts */
+	writel(IRQ_STAT_4PORTS, host_base + HOST_CTRL);
+}
+
 static int sil24_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version = 0;

@@ -1076,9 +1134,6 @@ static int sil24_init_one(struct pci_dev
 		}
 	}
 
-	/* GPIO off */
-	writel(0, host_base + HOST_FLASH_CMD);
-
 	/* Apply workaround for completion IRQ loss on PCI-X errata */
 	if (probe_ent->host_flags & SIL24_FLAG_PCIX_IRQ_WOC) {
 		tmp = readl(host_base + HOST_CTRL);

@@ -1090,56 +1145,18 @@ static int sil24_init_one(struct pci_dev
 			probe_ent->host_flags &= ~SIL24_FLAG_PCIX_IRQ_WOC;
 	}
 
-	/* clear global reset & mask interrupts during initialization */
-	writel(0, host_base + HOST_CTRL);
-
 	for (i = 0; i < probe_ent->n_ports; i++) {
-		void __iomem *port = port_base + i * PORT_REGS_SIZE;
-		unsigned long portu = (unsigned long)port;
+		unsigned long portu =
+			(unsigned long)port_base + i * PORT_REGS_SIZE;
 
 		probe_ent->port[i].cmd_addr = portu;
 		probe_ent->port[i].scr_addr = portu + PORT_SCONTROL;
 
 		ata_std_ports(&probe_ent->port[i]);
-
-		/* Initial PHY setting */
-		writel(0x20c, port + PORT_PHY_CFG);
-
-		/* Clear port RST */
-		tmp = readl(port + PORT_CTRL_STAT);
-		if (tmp & PORT_CS_PORT_RST) {
-			writel(PORT_CS_PORT_RST, port + PORT_CTRL_CLR);
-			tmp = ata_wait_register(port + PORT_CTRL_STAT,
-						PORT_CS_PORT_RST,
-						PORT_CS_PORT_RST, 10, 100);
-			if (tmp & PORT_CS_PORT_RST)
-				dev_printk(KERN_ERR, &pdev->dev,
-				           "failed to clear port RST\n");
-		}
-
-		/* Configure IRQ WoC */
-		if (probe_ent->host_flags & SIL24_FLAG_PCIX_IRQ_WOC)
-			writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_STAT);
-		else
-			writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_CLR);
-
-		/* Zero error counters. */
-		writel(0x8000, port + PORT_DECODE_ERR_THRESH);
-		writel(0x8000, port + PORT_CRC_ERR_THRESH);
-		writel(0x8000, port + PORT_HSHK_ERR_THRESH);
-		writel(0x0000, port + PORT_DECODE_ERR_CNT);
-		writel(0x0000, port + PORT_CRC_ERR_CNT);
-		writel(0x0000, port + PORT_HSHK_ERR_CNT);
-
-		/* Always use 64bit activation */
-		writel(PORT_CS_32BIT_ACTV, port + PORT_CTRL_CLR);
-
-		/* Clear port multiplier enable and resume bits */
-		writel(PORT_CS_PM_EN | PORT_CS_RESUME, port + PORT_CTRL_CLR);
 	}
 
-	/* Turn on interrupts */
-	writel(IRQ_STAT_4PORTS, host_base + HOST_CTRL);
+	sil24_init_controller(pdev, probe_ent->n_ports, probe_ent->host_flags,
+			      host_base, port_base);
 
 	pci_set_master(pdev);
Signature · 1 line
1.3.2
author
#18 lore raw

PATCH 15/15 sata_sil24: add suspend/sleep support

In reply to Tejun Heo

Add suspend/sleep support.

Signed-off-by Tejun Heo <[email protected]>

---

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

19356e72ec5b323cda9af3b8183400f096f78ba4

drivers/scsi/sata_sil24.c +25 -0
index 7726e6e..49f992f 100644
@@ -92,6 +92,7 @@ enum {
 	HOST_CTRL_STOP		= (1 << 18), /* latched PCI STOP */
 	HOST_CTRL_DEVSEL	= (1 << 19), /* latched PCI DEVSEL */
 	HOST_CTRL_REQ64		= (1 << 20), /* latched PCI REQ64 */
+	HOST_CTRL_GLOBAL_RST	= (1 << 31), /* global reset */
 
 	/*
 	 * Port registers

@@ -338,6 +339,7 @@ static int sil24_port_start(struct ata_p
 static void sil24_port_stop(struct ata_port *ap);
 static void sil24_host_stop(struct ata_host_set *host_set);
 static int sil24_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+static int sil24_pci_device_resume(struct pci_dev *pdev);
 
 static const struct pci_device_id sil24_pci_tbl[] = {
 	{ 0x1095, 0x3124, PCI_ANY_ID, PCI_ANY_ID, 0, 0, BID_SIL3124 },

@@ -353,6 +355,8 @@ static struct pci_driver sil24_pci_drive
 	.id_table		= sil24_pci_tbl,
 	.probe			= sil24_init_one,
 	.remove			= ata_pci_remove_one, /* safe? */
+	.suspend		= ata_pci_device_suspend,
+	.resume			= sil24_pci_device_resume,
 };
 
 static struct scsi_host_template sil24_sht = {

@@ -372,6 +376,8 @@ static struct scsi_host_template sil24_s
 	.slave_configure	= ata_scsi_slave_config,
 	.slave_destroy		= ata_scsi_slave_destroy,
 	.bios_param		= ata_std_bios_param,
+	.suspend		= ata_scsi_device_suspend,
+	.resume			= ata_scsi_device_resume,
 };
 
 static const struct ata_port_operations sil24_ops = {

@@ -1179,6 +1185,25 @@ static int sil24_init_one(struct pci_dev
 	return rc;
 }
 
+static int sil24_pci_device_resume(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	struct sil24_host_priv *hpriv = host_set->private_data;
+
+	ata_pci_device_do_resume(pdev);
+
+	if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND)
+		writel(HOST_CTRL_GLOBAL_RST, hpriv->host_base + HOST_CTRL);
+
+	sil24_init_controller(pdev, host_set->n_ports,
+			      host_set->ports[0]->flags,
+			      hpriv->host_base, hpriv->port_base);
+
+	ata_host_set_resume(host_set);
+
+	return 0;
+}
+
 static int __init sil24_init(void)
 {
 	return pci_module_init(&sil24_pci_driver);
Signature · 1 line
1.3.2
#20 lore raw
In reply to Tejun Heo
Quoted 8 lines · Tejun Heo

Hello, all.

This is the second take of new-power-management patchset. Changes from the last take[1] are...

* Per-dev PM is now supported. Now generic device PM layer drives whole PM sequence and libata port-wide PM does much less.

Hello, Tejun

It seems that port-level PM callbacks are removed in this patch set :( So if the author of other types of SATA controllers need to do port- level PM operations, they need to first add the callbacks by themselves, right?
Does it make sense to add port-wide PM callbacks in this patch set?

Thanks,
Forrest

author
#21 lore raw
Quoted 10 lines · forrest
Quoted 8 lines · Tejun Heo

Hello, all.

This is the second take of new-power-management patchset. Changes from the last take[1] are...

* Per-dev PM is now supported. Now generic device PM layer drives whole PM sequence and libata port-wide PM does much less.

Quoted 6 lines

Hello, Tejun

It seems that port-level PM callbacks are removed in this patch set :( So if the author of other types of SATA controllers need to do port- level PM operations, they need to first add the callbacks by themselves, right?

Right.

Quoted 1 line

Does it make sense to add port-wide PM callbacks in this patch set?

Yes, it does. I removed those because none of the currently converted drivers used it. Does AHCI need them? If so, I'll add them. Are you gonna do ACPI stuff w/ those callbacks?

Signature · 1 line
tejun
#22 lore raw
Quoted 6 lines · Tejun Heo
Quoted 1 line

Does it make sense to add port-wide PM callbacks in this patch set?

Yes, it does. I removed those because none of the currently converted drivers used it. Does AHCI need them? If so, I'll add them. Are you gonna do ACPI stuff w/ those callbacks?

Yeap, both AHCI suspend/resume patch and ACPI-SATA patch need these port-wide callbacks.
It's appreciated if you could add them in your PM patch :)

Thanks,
Forrest

author
#23 lore raw
Quoted 9 lines · forrest
Quoted 3 lines · Tejun Heo
Quoted 1 line

Does it make sense to add port-wide PM callbacks in this patch set?

Yes, it does. I removed those because none of the currently converted drivers used it. Does AHCI need them? If so, I'll add them. Are you gonna do ACPI stuff w/ those callbacks?

Yeap, both AHCI suspend/resume patch and ACPI-SATA patch need these port-wide callbacks.
It's appreciated if you could add them in your PM patch :)

Will do after this whole thing get reviewed. I don't know much about ACPI but don't we need to do it regardless of LLDs? If so, I think it might be better to hook those directly into ata_eh_suspend/resume(). Other than ACPI, what do you want to do in per-port callbacks for AHCI?

Signature · 1 line
tejun
#24 lore raw
Quoted 1 line · Tejun Heo

I don't know much about ACPI but don't we need to do it regardless of LLDs?

Yes. Though generally ACPI only ever has knowledge of the drives directly attached to the motherboard.

Jeff

author
#25 lore raw
Quoted 4 lines · Jeff Garzik
Quoted 1 line · Tejun Heo

I don't know much about ACPI but don't we need to do it regardless of LLDs?

Yes. Though generally ACPI only ever has knowledge of the drives directly attached to the motherboard.

I've tried to read ACPI spec through several times but, man, it was boring. Also, I've just printed out EFI spec and it is massive. Are there any better/more humanly readings regarding these topics? I don't mind reading specs but these two are monsters.

Signature · 1 line
tejun
#26 lore raw
Quoted 5 lines · Tejun Heo
Quoted 4 lines · Jeff Garzik
Quoted 2 lines · Tejun Heo

I don't know much about ACPI but don't we need to do it regardless of LLDs?

Quoted 8 lines
Quoted 2 lines

Yes. Though generally ACPI only ever has knowledge of the drives directly attached to the motherboard.

I've tried to read ACPI spec through several times but, man, it was boring. Also, I've just printed out EFI spec and it is massive. Are there any better/more humanly readings regarding these topics? I don't mind reading specs but these two are monsters.

For SATA-ACPI, you only need to read section 9.9 of ACPI spec 3.0. It's a short section. For basic concepts of ACPI, I don't know which document is more humanly reading. I asked my colleagues when I was confused about basic ACPI concepts.

Thanks,
Forrest

#27 lore raw
In reply to Tejun Heo
Quoted 3 lines · Tejun Heo

Will do after this whole thing get reviewed. I don't know much about ACPI but don't we need to do it regardless of LLDs? If so, I think it might be better to hook those directly into ata_eh_suspend/resume().

in ata_eh_suspend/resume(), all operations are device-level. We need a port-level function inside ata_eh_suspend/resume(), and inside this port-level function both generic port-level operations and LLD specific port-level callbacks are called.

Thanks,
Forrest

author
#28 lore raw
Quoted 7 lines · forrest
Quoted 1 line · Tejun Heo

Will do after this whole thing get reviewed. I don't know much about ACPI but don't we need to do it regardless of LLDs? If so, I think it might be better to hook those directly into ata_eh_suspend/resume().

in ata_eh_suspend/resume(), all operations are device-level. We need a port-level function inside ata_eh_suspend/resume(), and inside this port-level function both generic port-level operations and LLD specific port-level callbacks are called.

Then, please create a separate hook. It can be just side by side w/ ops->suspend/resume() calls but I think they need to be called separately from LLD-specific PM ops.

Signature · 1 line
tejun