[PATCHSET] libata: various fixes related to EH, take #4

[PATCHSET] libata: various fixes related to EH, take #4

[Tejun Heo]

Hello, Jeff.

< %nn denotes patch nn from the last iteration, #nn in this iteration >

This is the fourth take of new reset mechanism.  4 (%01-%04) out of 11
patches made into upstream in the last iteration [1].  The rest are
acked but haven't been applied because standard reset operations need
more changes before being used.

This patchset is composed of 9 patches.

#01-02	: modify std reset operations such that they act almost
	  identically with ->phy_reset register-wise.
#03-09	: These seven patches are %05-%09 with minor changes.

After #01, the only meaningful difference of the new reset mechanism
from ->phy_reset is that SError is cleared after SATA hardreset.  This
difference is removed by #02.  #02 is separated to make the change
optional.  If #02 gets into upstream, I'll have to submit another
patch to resurrect it later for EH.  Jeff, it's your call.

After #01 and #02, the following differences remain.

1. SStatus access for sata_print_link_status() in ata_std_postreset()
   is at different postion register-wise compared to ->phy_reset.

2. ->phy_reset doesn't proceed with softreset if ata_busy_sleep()
   fails after PHY is waken, but new reset mechanism proceeds anyway.
   Note that the original code is not very consistent regarding this.
   Some drivers (e.g. ata_piix), uses ata_bus_reset() instead of
   sata_phy_reset() and thus doesn't perform ata_busy_sleep() before
   SRST in the first place.

If a low level driver uses ata_std_probeinit() but doesn't implement
softreset, it will end up performing extra phy_resume and
ata_busy_sleep before the actual hardreset sequence compared to
->phy_reset.  This is noted int the comment above ata_std_probeinit().

#08 has been modified not to use ata_std_probeinit() because it only
implements hardreset, but #09 implements softreset and adds
ata_std_probeinit() back; thus, the end result is unchanged.

Thanks.

--
tejun

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

[PATCH 2/9] libata: kill SError clearing in sata_std_hardreset().

[Tejun Heo]

Don't clear SError in sata_std_hardreset().  This makes hardreset act
identically to ->phy_reset register-wise.

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

---

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

24bff807a5f2bed38aadb4b8aa25bc40791bf2aa
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 8424bd9..38e72c1 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -2018,8 +2018,6 @@ int ata_std_softreset(struct ata_port *a
  */
 int sata_std_hardreset(struct ata_port *ap, int verbose, unsigned int *class)
 {
-	u32 serror;
-
 	DPRINTK("ENTER\n");
 
 	/* Issue phy wake/reset */
@@ -2034,10 +2032,6 @@ int sata_std_hardreset(struct ata_port *
 	/* Bring phy back */
 	sata_phy_resume(ap);
 
-	/* Clear SError */
-	serror = scr_read(ap, SCR_ERROR);
-	scr_write(ap, SCR_ERROR, serror);
-
 	/* TODO: phy layer with polling, timeouts, etc. */
 	if (!sata_dev_present(ap)) {
 		*class = ATA_DEV_NONE;
-- 
1.1.5

[PATCH 6/9] ata_piix: convert pata to new reset mechanism

[Tejun Heo]

Convert ata_piix pata ->phy_reset to new reset mechanism.

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

---

 drivers/scsi/ata_piix.c |   47 +++++++++++++++++++++++++++++++----------------
 1 files changed, 31 insertions(+), 16 deletions(-)

aa17cffcebd940a844e0a8a5a99d2192699c9b4e
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 4933ba2..650c687 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -131,7 +131,7 @@ enum {
 static int piix_init_one (struct pci_dev *pdev,
 				    const struct pci_device_id *ent);
 
-static void piix_pata_phy_reset(struct ata_port *ap);
+static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void piix_sata_phy_reset(struct ata_port *ap);
 static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev);
 static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
@@ -208,7 +208,7 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.phy_reset		= piix_pata_phy_reset,
+	.probe_reset		= piix_pata_probe_reset,
 
 	.bmdma_setup		= ata_bmdma_setup,
 	.bmdma_start		= ata_bmdma_start,
@@ -259,8 +259,7 @@ static struct ata_port_info piix_port_in
 	/* ich5_pata */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST |
-				  PIIX_FLAG_CHECKINTR,
+		.host_flags	= ATA_FLAG_SLAVE_POSS | PIIX_FLAG_CHECKINTR,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 #if 0
 		.mwdma_mask	= 0x06, /* mwdma1-2 */
@@ -285,7 +284,7 @@ static struct ata_port_info piix_port_in
 	/* piix4_pata */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
+		.host_flags	= ATA_FLAG_SLAVE_POSS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 #if 0
 		.mwdma_mask	= 0x06, /* mwdma1-2 */
@@ -367,30 +366,46 @@ cbl40:
 }
 
 /**
- *	piix_pata_phy_reset - Probe specified port on PATA host controller
- *	@ap: Port to probe
+ *	piix_pata_postreset - Postreset stuff on PATA host controller
+ *	@ap: Target port
+ *	@classes: Classes of attached devices
  *
- *	Probe PATA phy.
+ *	Postreset processing including cable detection.
  *
  *	LOCKING:
  *	None (inherited from caller).
  */
+static void piix_pata_postreset(struct ata_port *ap, unsigned int *classes)
+{
+	piix_pata_cbl_detect(ap);
+	ata_std_postreset(ap, classes);
+}
 
-static void piix_pata_phy_reset(struct ata_port *ap)
+/**
+ *	piix_pata_probe_reset - Perform reset on PATA port and classify
+ *	@ap: Port to reset
+ *	@classes: Resulting classes of attached devices
+ *
+ *	Reset PATA phy and classify attached devices.
+ *
+ *	LOCKING:
+ *	None (inherited from caller).
+ */
+static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes)
 {
 	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
+	int i;
 
 	if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->hard_port_no])) {
-		ata_port_disable(ap);
+		for (i = 0; i < ATA_MAX_DEVICES; i++)
+			classes[i] = ATA_DEV_NONE;
 		printk(KERN_INFO "ata%u: port disabled. ignoring.\n", ap->id);
-		return;
+		return 0;
 	}
 
-	piix_pata_cbl_detect(ap);
-
-	ata_port_probe(ap);
-
-	ata_bus_reset(ap);
+	return ata_drive_probe_reset(ap, ata_std_probeinit,
+				     ata_std_softreset, NULL,
+				     piix_pata_postreset, classes);
 }
 
 /**
-- 
1.1.5

[PATCH 4/9] sata_sil24: convert to new reset mechanism

[Tejun Heo]

Convert sata_sil24 ->phy_reset to new reset mechanism.

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

---

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

e616f994715634d1feb81f6e0ac24e34896a4bb1
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 962396b..ccac05a 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -249,7 +249,7 @@ static u8 sil24_check_status(struct ata_
 static u32 sil24_scr_read(struct ata_port *ap, unsigned sc_reg);
 static void sil24_scr_write(struct ata_port *ap, unsigned sc_reg, u32 val);
 static void sil24_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
-static void sil24_phy_reset(struct ata_port *ap);
+static int sil24_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void sil24_qc_prep(struct ata_queued_cmd *qc);
 static unsigned int sil24_qc_issue(struct ata_queued_cmd *qc);
 static void sil24_irq_clear(struct ata_port *ap);
@@ -306,7 +306,7 @@ static const struct ata_port_operations 
 
 	.tf_read		= sil24_tf_read,
 
-	.phy_reset		= sil24_phy_reset,
+	.probe_reset		= sil24_probe_reset,
 
 	.qc_prep		= sil24_qc_prep,
 	.qc_issue		= sil24_qc_issue,
@@ -336,8 +336,8 @@ static struct ata_port_info sil24_port_i
 	{
 		.sht		= &sil24_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  ATA_FLAG_PIO_DMA | SIL24_NPORTS2FLAG(4),
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
+				  SIL24_NPORTS2FLAG(4),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -347,8 +347,8 @@ static struct ata_port_info sil24_port_i
 	{
 		.sht		= &sil24_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  ATA_FLAG_PIO_DMA | SIL24_NPORTS2FLAG(2),
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
+				  SIL24_NPORTS2FLAG(2),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -358,8 +358,8 @@ static struct ata_port_info sil24_port_i
 	{
 		.sht		= &sil24_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  ATA_FLAG_PIO_DMA | SIL24_NPORTS2FLAG(1),
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
+				  SIL24_NPORTS2FLAG(1),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -428,7 +428,8 @@ static void sil24_tf_read(struct ata_por
 	*tf = pp->tf;
 }
 
-static int sil24_issue_SRST(struct ata_port *ap)
+static int sil24_softreset(struct ata_port *ap, int verbose,
+			   unsigned int *class)
 {
 	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
 	struct sil24_port_priv *pp = ap->private_data;
@@ -437,6 +438,8 @@ static int sil24_issue_SRST(struct ata_p
 	u32 irq_enable, irq_stat;
 	int cnt;
 
+	DPRINTK("ENTER\n");
+
 	/* temporarily turn off IRQs during SRST */
 	irq_enable = readl(port + PORT_IRQ_ENABLE_SET);
 	writel(irq_enable, port + PORT_IRQ_ENABLE_CLR);
@@ -466,30 +469,27 @@ static int sil24_issue_SRST(struct ata_p
 	/* restore IRQs */
 	writel(irq_enable, port + PORT_IRQ_ENABLE_SET);
 
-	if (!(irq_stat & PORT_IRQ_COMPLETE))
-		return -1;
+	if (sata_dev_present(ap)) {
+		if (!(irq_stat & PORT_IRQ_COMPLETE)) {
+			DPRINTK("EXIT, srst failed\n");
+			return -EIO;
+		}
 
-	/* update TF */
-	sil24_update_tf(ap);
+		sil24_update_tf(ap);
+		*class = ata_dev_classify(&pp->tf);
+	}
+	if (*class == ATA_DEV_UNKNOWN)
+		*class = ATA_DEV_NONE;
+
+	DPRINTK("EXIT, class=%u\n", *class);
 	return 0;
 }
 
-static void sil24_phy_reset(struct ata_port *ap)
+static int sil24_probe_reset(struct ata_port *ap, unsigned int *classes)
 {
-	struct sil24_port_priv *pp = ap->private_data;
-
-	__sata_phy_reset(ap);
-	if (ap->flags & ATA_FLAG_PORT_DISABLED)
-		return;
-
-	if (sil24_issue_SRST(ap) < 0) {
-		printk(KERN_ERR DRV_NAME
-		       " ata%u: SRST failed, disabling port\n", ap->id);
-		ap->ops->port_disable(ap);
-		return;
-	}
-
-	ap->device->class = ata_dev_classify(&pp->tf);
+	return ata_drive_probe_reset(ap, ata_std_probeinit,
+				     sil24_softreset, NULL,
+				     ata_std_postreset, classes);
 }
 
 static inline void sil24_fill_sg(struct ata_queued_cmd *qc,
-- 
1.1.5

[PATCH 7/9] ata_piix: convert sata to new reset mechanism

[Tejun Heo]

Convert ata_piix sata ->phy_reset to new reset mechanism.

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

---

 drivers/scsi/ata_piix.c |   39 +++++++++++++++++++--------------------
 1 files changed, 19 insertions(+), 20 deletions(-)

9314defd1d310a822bf491b60cddbf9d652ab6e0
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 650c687..0da8076 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -132,7 +132,7 @@ static int piix_init_one (struct pci_dev
 				    const struct pci_device_id *ent);
 
 static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes);
-static void piix_sata_phy_reset(struct ata_port *ap);
+static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev);
 static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
 
@@ -236,7 +236,7 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.phy_reset		= piix_sata_phy_reset,
+	.probe_reset		= piix_sata_probe_reset,
 
 	.bmdma_setup		= ata_bmdma_setup,
 	.bmdma_start		= ata_bmdma_start,
@@ -273,8 +273,8 @@ static struct ata_port_info piix_port_in
 	/* ich5_sata */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
-				  PIIX_FLAG_COMBINED | PIIX_FLAG_CHECKINTR,
+		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED |
+				  PIIX_FLAG_CHECKINTR,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f,	/* udma0-6 */
@@ -298,8 +298,7 @@ static struct ata_port_info piix_port_in
 	/* ich6_sata */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
-				  PIIX_FLAG_COMBINED_ICH6 |
+		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 |
 				  PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -310,8 +309,7 @@ static struct ata_port_info piix_port_in
 	/* ich6_sata_ahci */
 	{
 		.sht		= &piix_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
-				  PIIX_FLAG_COMBINED_ICH6 |
+		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 |
 				  PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS |
 				  PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
@@ -456,28 +454,29 @@ static int piix_sata_probe (struct ata_p
 }
 
 /**
- *	piix_sata_phy_reset - Probe specified port on SATA host controller
- *	@ap: Port to probe
+ *	piix_sata_probe_reset - Perform reset on SATA port and classify
+ *	@ap: Port to reset
+ *	@classes: Resulting classes of attached devices
  *
- *	Probe SATA phy.
+ *	Reset SATA phy and classify attached devices.
  *
  *	LOCKING:
  *	None (inherited from caller).
  */
-
-static void piix_sata_phy_reset(struct ata_port *ap)
+static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes)
 {
+	int i;
+
 	if (!piix_sata_probe(ap)) {
-		ata_port_disable(ap);
+		for (i = 0; i < ATA_MAX_DEVICES; i++)
+			classes[i] = ATA_DEV_NONE;
 		printk(KERN_INFO "ata%u: SATA port has no device.\n", ap->id);
-		return;
+		return 0;
 	}
 
-	ap->cbl = ATA_CBL_SATA;
-
-	ata_port_probe(ap);
-
-	ata_bus_reset(ap);
+	return ata_drive_probe_reset(ap, ata_std_probeinit,
+				     ata_std_softreset, NULL,
+				     ata_std_postreset, classes);
 }
 
 /**
-- 
1.1.5

[PATCH 1/9] libata: make new reset act identical to ->phy_reset register-wise

[Tejun Heo]

This patch makes std component operations act identical to ->phy_reset
register-wise except for SError clearing on sata_std_hardreset.

Note that if a driver only implements/uses hardreset, it should not
use ata_std_probeinit() to avoid extra sata_phy_resume() and
ata_busy_sleep() compared to ->phy_reset.

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

---

 drivers/scsi/libata-core.c |   51 +++++++++++++++++++++++++++++---------------
 1 files changed, 34 insertions(+), 17 deletions(-)

132ad18278df23894fe8b0768ef0be8087ce790d
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index b938c7a..8424bd9 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1924,11 +1924,20 @@ static int sata_phy_resume(struct ata_po
  *
  *	@ap is about to be probed.  Initialize it.  This function is
  *	to be used as standard callback for ata_drive_probe_reset().
+ *
+ *	NOTE!!! Do not use this function as probeinit if a low level
+ *	driver implements only hardreset.  Just pass NULL as probeinit
+ *	in that case.  Using this function is probably okay but doing
+ *	so makes reset sequence different from the original
+ *	->phy_reset implementation and Jeff nervous.  :-P
  */
 extern void ata_std_probeinit(struct ata_port *ap)
 {
-	if (ap->flags & ATA_FLAG_SATA && ap->ops->scr_read)
+	if (ap->flags & ATA_FLAG_SATA && ap->ops->scr_read) {
 		sata_phy_resume(ap);
+		if (sata_dev_present(ap))
+			ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT);
+	}
 }
 
 /**
@@ -1954,20 +1963,17 @@ int ata_std_softreset(struct ata_port *a
 
 	DPRINTK("ENTER\n");
 
+	if (ap->ops->scr_read && !sata_dev_present(ap)) {
+		classes[0] = ATA_DEV_NONE;
+		goto out;
+	}
+
 	/* determine if device 0/1 are present */
 	if (ata_devchk(ap, 0))
 		devmask |= (1 << 0);
 	if (slave_possible && ata_devchk(ap, 1))
 		devmask |= (1 << 1);
 
-	/* devchk reports device presence without actual device on
-	 * most SATA controllers.  Check SStatus and turn devmask off
-	 * if link is offline.  Note that we should continue resetting
-	 * even when it seems like there's no device.
-	 */
-	if (ap->ops->scr_read && !sata_dev_present(ap))
-		devmask = 0;
-
 	/* select device 0 again */
 	ap->ops->dev_select(ap, 0);
 
@@ -1989,6 +1995,7 @@ int ata_std_softreset(struct ata_port *a
 	if (slave_possible && err != 0x81)
 		classes[1] = ata_dev_try_classify(ap, 1, &err);
 
+ out:
 	DPRINTK("EXIT, classes[0]=%u [1]=%u\n", classes[0], classes[1]);
 	return 0;
 }
@@ -2047,6 +2054,8 @@ int sata_std_hardreset(struct ata_port *
 		return -EIO;
 	}
 
+	ap->ops->dev_select(ap, 0);	/* probably unnecessary */
+
 	*class = ata_dev_try_classify(ap, 0, NULL);
 
 	DPRINTK("EXIT, class=%u\n", *class);
@@ -2081,11 +2090,9 @@ void ata_std_postreset(struct ata_port *
 	if (ap->cbl == ATA_CBL_SATA)
 		sata_print_link_status(ap);
 
-	/* bail out if no device is present */
-	if (classes[0] == ATA_DEV_NONE && classes[1] == ATA_DEV_NONE) {
-		DPRINTK("EXIT, no device\n");
-		return;
-	}
+	/* re-enable interrupts */
+	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
+		ata_irq_on(ap);
 
 	/* is double-select really necessary? */
 	if (classes[0] != ATA_DEV_NONE)
@@ -2093,9 +2100,19 @@ void ata_std_postreset(struct ata_port *
 	if (classes[1] != ATA_DEV_NONE)
 		ap->ops->dev_select(ap, 0);
 
-	/* re-enable interrupts & set up device control */
-	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
-		ata_irq_on(ap);
+	/* bail out if no device is present */
+	if (classes[0] == ATA_DEV_NONE && classes[1] == ATA_DEV_NONE) {
+		DPRINTK("EXIT, no device\n");
+		return;
+	}
+
+	/* set up device control */
+	if (ap->ioaddr.ctl_addr) {
+		if (ap->flags & ATA_FLAG_MMIO)
+			writeb(ap->ctl, (void __iomem *) ap->ioaddr.ctl_addr);
+		else
+			outb(ap->ctl, ap->ioaddr.ctl_addr);
+	}
 
 	DPRINTK("EXIT\n");
 }
-- 
1.1.5

[PATCH 3/9] sata_sil: convert to new reset mechanism

[Tejun Heo]

Convert sata_sil to use new reset mechanism.  sata_sil is fairly
generic and can directly use std routine.

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

---

 drivers/scsi/sata_sil.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

0ae93718b02a7763abfacd0a900b51ba27c6212d
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index f40f25e..bd28877 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -158,7 +158,7 @@ static const struct ata_port_operations 
 	.check_status		= ata_check_status,
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
-	.phy_reset		= sata_phy_reset,
+	.probe_reset		= ata_std_probe_reset,
 	.post_set_mode		= sil_post_set_mode,
 	.bmdma_setup            = ata_bmdma_setup,
 	.bmdma_start            = ata_bmdma_start,
@@ -181,7 +181,7 @@ static const struct ata_port_info sil_po
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO,
+				  ATA_FLAG_MMIO,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -190,8 +190,7 @@ static const struct ata_port_info sil_po
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  SIL_FLAG_MOD15WRITE,
+				  ATA_FLAG_MMIO | SIL_FLAG_MOD15WRITE,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -200,7 +199,7 @@ static const struct ata_port_info sil_po
 	{
 		.sht		= &sil_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO,
+				  ATA_FLAG_MMIO,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
-- 
1.1.5

[PATCH 5/9] sata_sil24: add hardreset

[Tejun Heo]

Now that libata is smart enough to handle both soft and hard resets,
add hardreset method.  Note that sil24 hardreset doesn't supply
signature; still, the new reset mechanism can make good use of it.

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

---

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

1e7c4c83847442f2e881e04e6a3b8f257dcc241e
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index ccac05a..228a7fa 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -485,10 +485,19 @@ static int sil24_softreset(struct ata_po
 	return 0;
 }
 
+static int sil24_hardreset(struct ata_port *ap, int verbose,
+			   unsigned int *class)
+{
+	unsigned int dummy_class;
+
+	/* sil24 doesn't report device signature after hard reset */
+	return sata_std_hardreset(ap, verbose, &dummy_class);
+}
+
 static int sil24_probe_reset(struct ata_port *ap, unsigned int *classes)
 {
 	return ata_drive_probe_reset(ap, ata_std_probeinit,
-				     sil24_softreset, NULL,
+				     sil24_softreset, sil24_hardreset,
 				     ata_std_postreset, classes);
 }
 
-- 
1.1.5

[PATCH 9/9] ahci: add softreset

[Tejun Heo]

Now that libata is smart enought to handle both soft and hard resets,
add softreset method.

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

---

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

264f1acba3f46c66575e135e521d83000c7ccf01
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 9fb97ff..2789274 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -515,6 +515,138 @@ static void ahci_fill_cmd_slot(struct at
 	pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
 }
 
+static int ahci_wait_for_bit(void __iomem *reg, u32 mask, u32 val,
+			     unsigned long interval_msec,
+			     unsigned long timeout_msec)
+{
+	unsigned long timeout;
+	u32 tmp;
+
+	timeout = jiffies + (timeout_msec * HZ) / 1000;
+	do {
+		tmp = readl(reg);
+		if ((tmp & mask) != val)
+			return 0;
+		msleep(interval_msec);
+	} while (time_before(jiffies, timeout));
+
+	return -1;
+}
+
+static int ahci_softreset(struct ata_port *ap, int verbose, unsigned int *class)
+{
+	struct ahci_host_priv *hpriv = ap->host_set->private_data;
+	struct ahci_port_priv *pp = ap->private_data;
+	void __iomem *mmio = ap->host_set->mmio_base;
+	void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+	const u32 cmd_fis_len = 5; /* five dwords */
+	const char *reason = NULL;
+	struct ata_taskfile tf;
+	u8 *fis;
+	int rc;
+
+	DPRINTK("ENTER\n");
+
+	/* prepare for SRST (AHCI-1.1 10.4.1) */
+	rc = ahci_stop_engine(ap);
+	if (rc) {
+		reason = "failed to stop engine";
+		goto fail_restart;
+	}
+
+	/* check BUSY/DRQ, perform Command List Override if necessary */
+	ahci_tf_read(ap, &tf);
+	if (tf.command & (ATA_BUSY | ATA_DRQ)) {
+		u32 tmp;
+
+		if (!(hpriv->cap & HOST_CAP_CLO)) {
+			rc = -EIO;
+			reason = "port busy but no CLO";
+			goto fail_restart;
+		}
+
+		tmp = readl(port_mmio + PORT_CMD);
+		tmp |= PORT_CMD_CLO;
+		writel(tmp, port_mmio + PORT_CMD);
+		readl(port_mmio + PORT_CMD); /* flush */
+
+		if (ahci_wait_for_bit(port_mmio + PORT_CMD,
+				      PORT_CMD_CLO, PORT_CMD_CLO, 1, 500)) {
+			rc = -EIO;
+			reason = "CLO failed";
+			goto fail_restart;
+		}
+	}
+
+	/* restart engine */
+	ahci_start_engine(ap);
+
+	ata_tf_init(ap, &tf, 0);
+	fis = pp->cmd_tbl;
+
+	/* issue the first D2H Register FIS */
+	ahci_fill_cmd_slot(ap, cmd_fis_len | AHCI_CMD_RESET | AHCI_CMD_CLR_BUSY);
+
+	tf.ctl |= ATA_SRST;
+	ata_tf_to_fis(&tf, fis, 0);
+	fis[1] &= ~(1 << 7);	/* turn off Command FIS bit */
+
+	writel(1, port_mmio + PORT_CMD_ISSUE);
+	readl(port_mmio + PORT_CMD_ISSUE);	/* flush */
+
+	if (ahci_wait_for_bit(port_mmio + PORT_CMD_ISSUE, 0x1, 0x1, 1, 500)) {
+		rc = -EIO;
+		reason = "1st FIS failed";
+		goto fail;
+	}
+
+	/* spec says at least 5us, but be generous and sleep for 1ms */
+	msleep(1);
+
+	/* issue the second D2H Register FIS */
+	ahci_fill_cmd_slot(ap, cmd_fis_len);
+
+	tf.ctl &= ~ATA_SRST;
+	ata_tf_to_fis(&tf, fis, 0);
+	fis[1] &= ~(1 << 7);	/* turn off Command FIS bit */
+
+	writel(1, port_mmio + PORT_CMD_ISSUE);
+	readl(port_mmio + PORT_CMD_ISSUE);	/* flush */
+
+	/* spec mandates ">= 2ms" before checking status.
+	 * We wait 150ms, because that was the magic delay used for
+	 * ATAPI devices in Hale Landis's ATADRVR, for the period of time
+	 * between when the ATA command register is written, and then
+	 * status is checked.  Because waiting for "a while" before
+	 * checking status is fine, post SRST, we perform this magic
+	 * delay here as well.
+	 */
+	msleep(150);
+
+	*class = ATA_DEV_NONE;
+	if (sata_dev_present(ap)) {
+		if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
+			rc = -EIO;
+			reason = "device not ready";
+			goto fail;
+		}
+		*class = ahci_dev_classify(ap);
+	}
+
+	DPRINTK("EXIT, class=%u\n", *class);
+	return 0;
+
+ fail_restart:
+	ahci_start_engine(ap);
+ fail:
+	if (verbose)
+		printk(KERN_ERR "ata%u: softreset failed (%s)\n",
+		       ap->id, reason);
+	else
+		DPRINTK("EXIT, rc=%d reason=\"%s\"\n", rc, reason);
+	return rc;
+}
+
 static int ahci_hardreset(struct ata_port *ap, int verbose, unsigned int *class)
 {
 	int rc;
@@ -555,7 +687,8 @@ static void ahci_postreset(struct ata_po
 
 static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes)
 {
-	return ata_drive_probe_reset(ap, NULL, NULL, ahci_hardreset,
+	return ata_drive_probe_reset(ap, ata_std_probeinit,
+				     ahci_softreset, ahci_hardreset,
 				     ahci_postreset, classes);
 }
 
-- 
1.1.5

[PATCH 8/9] ahci: convert to new reset mechanism

[Tejun Heo]

Convert ahci ->phy_reset to new reset mechanism.

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

---

 drivers/scsi/ahci.c |   46 +++++++++++++++++++++++++++++-----------------
 1 files changed, 29 insertions(+), 17 deletions(-)

f91eac3e9ceb5fdd5e52a3b6c4e2d53d616b3cc4
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 98ce6bb..9fb97ff 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -190,7 +190,7 @@ static void ahci_scr_write (struct ata_p
 static int ahci_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
 static irqreturn_t ahci_interrupt (int irq, void *dev_instance, struct pt_regs *regs);
-static void ahci_phy_reset(struct ata_port *ap);
+static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void ahci_irq_clear(struct ata_port *ap);
 static void ahci_eng_timeout(struct ata_port *ap);
 static int ahci_port_start(struct ata_port *ap);
@@ -230,7 +230,7 @@ static const struct ata_port_operations 
 
 	.tf_read		= ahci_tf_read,
 
-	.phy_reset		= ahci_phy_reset,
+	.probe_reset		= ahci_probe_reset,
 
 	.qc_prep		= ahci_qc_prep,
 	.qc_issue		= ahci_qc_issue,
@@ -252,8 +252,7 @@ static const struct ata_port_info ahci_p
 	{
 		.sht		= &ahci_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SATA_RESET | ATA_FLAG_MMIO |
-				  ATA_FLAG_PIO_DMA,
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,
@@ -516,28 +515,35 @@ static void ahci_fill_cmd_slot(struct at
 	pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
 }
 
-static void ahci_phy_reset(struct ata_port *ap)
+static int ahci_hardreset(struct ata_port *ap, int verbose, unsigned int *class)
 {
-	void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
-	struct ata_device *dev = &ap->device[0];
-	u32 new_tmp, tmp;
+	int rc;
+
+	DPRINTK("ENTER\n");
 
 	ahci_stop_engine(ap);
-	__sata_phy_reset(ap);
+	rc = sata_std_hardreset(ap, verbose, class);
 	ahci_start_engine(ap);
 
-	if (ap->flags & ATA_FLAG_PORT_DISABLED)
-		return;
+	if (rc == 0)
+		*class = ahci_dev_classify(ap);
+	if (*class == ATA_DEV_UNKNOWN)
+		*class = ATA_DEV_NONE;
 
-	dev->class = ahci_dev_classify(ap);
-	if (!ata_dev_present(dev)) {
-		ata_port_disable(ap);
-		return;
-	}
+	DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);
+	return rc;
+}
+
+static void ahci_postreset(struct ata_port *ap, unsigned int *class)
+{
+	void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
+	u32 new_tmp, tmp;
+
+	ata_std_postreset(ap, class);
 
 	/* Make sure port's ATAPI bit is set appropriately */
 	new_tmp = tmp = readl(port_mmio + PORT_CMD);
-	if (dev->class == ATA_DEV_ATAPI)
+	if (*class == ATA_DEV_ATAPI)
 		new_tmp |= PORT_CMD_ATAPI;
 	else
 		new_tmp &= ~PORT_CMD_ATAPI;
@@ -547,6 +553,12 @@ static void ahci_phy_reset(struct ata_po
 	}
 }
 
+static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes)
+{
+	return ata_drive_probe_reset(ap, NULL, NULL, ahci_hardreset,
+				     ahci_postreset, classes);
+}
+
 static u8 ahci_check_status(struct ata_port *ap)
 {
 	void __iomem *mmio = (void __iomem *) ap->ioaddr.cmd_addr;
-- 
1.1.5

Re: Sorry, wrong PATCHSET name, it should be [PATCHSET] libata: various fixes related to EH, take #4

[Tejun Heo]

Tejun Heo wrote:
> Hello, Jeff.
> 
> < %nn denotes patch nn from the last iteration, #nn in this iteration >
> 
> This is the fourth take of new reset mechanism.  4 (%01-%04) out of 11
> patches made into upstream in the last iteration [1].  The rest are
> acked but haven't been applied because standard reset operations need
> more changes before being used.
> 
> This patchset is composed of 9 patches.
> 
> #01-02	: modify std reset operations such that they act almost
> 	  identically with ->phy_reset register-wise.
> #03-09	: These seven patches are %05-%09 with minor changes.
> 
> After #01, the only meaningful difference of the new reset mechanism
> from ->phy_reset is that SError is cleared after SATA hardreset.  This
> difference is removed by #02.  #02 is separated to make the change
> optional.  If #02 gets into upstream, I'll have to submit another
> patch to resurrect it later for EH.  Jeff, it's your call.
> 
> After #01 and #02, the following differences remain.
> 
> 1. SStatus access for sata_print_link_status() in ata_std_postreset()
>    is at different postion register-wise compared to ->phy_reset.
> 
> 2. ->phy_reset doesn't proceed with softreset if ata_busy_sleep()
>    fails after PHY is waken, but new reset mechanism proceeds anyway.
>    Note that the original code is not very consistent regarding this.
>    Some drivers (e.g. ata_piix), uses ata_bus_reset() instead of
>    sata_phy_reset() and thus doesn't perform ata_busy_sleep() before
>    SRST in the first place.
> 
> If a low level driver uses ata_std_probeinit() but doesn't implement
> softreset, it will end up performing extra phy_resume and
> ata_busy_sleep before the actual hardreset sequence compared to
> ->phy_reset.  This is noted int the comment above ata_std_probeinit().
> 
> #08 has been modified not to use ata_std_probeinit() because it only
> implements hardreset, but #09 implements softreset and adds
> ata_std_probeinit() back; thus, the end result is unchanged.
> 
> Thanks.
> 
> --
> tejun
> 
> [1] http://article.gmane.org/gmane.linux.ide/7804
> 


-- 
tejun

Re: Gee, wrong again. libata: [PATCHSET] libata: new reset mechanism, take#4

[Tejun Heo]

And I'm not even smoking something good.  Sorry people.

--
tejun

Re: [PATCH 1/9] libata: make new reset act identical to ->phy_reset register-wise

[Jeff Garzik]

Tejun Heo wrote:
> This patch makes std component operations act identical to ->phy_reset
> register-wise except for SError clearing on sata_std_hardreset.
> 
> Note that if a driver only implements/uses hardreset, it should not
> use ata_std_probeinit() to avoid extra sata_phy_resume() and
> ata_busy_sleep() compared to ->phy_reset.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

applied 1-5 to 'upstream'

Re: [PATCH 6/9] ata_piix: convert pata to new reset mechanism

[Jeff Garzik]

Tejun Heo wrote:
> Convert ata_piix pata ->phy_reset to new reset mechanism.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/ata_piix.c |   47 +++++++++++++++++++++++++++++++----------------
>  1 files changed, 31 insertions(+), 16 deletions(-)
> 
> aa17cffcebd940a844e0a8a5a99d2192699c9b4e
> diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
> index 4933ba2..650c687 100644
> --- a/drivers/scsi/ata_piix.c
> +++ b/drivers/scsi/ata_piix.c
> @@ -131,7 +131,7 @@ enum {
>  static int piix_init_one (struct pci_dev *pdev,
>  				    const struct pci_device_id *ent);
>  
> -static void piix_pata_phy_reset(struct ata_port *ap);
> +static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes);
>  static void piix_sata_phy_reset(struct ata_port *ap);
>  static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev);
>  static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
> @@ -208,7 +208,7 @@ static const struct ata_port_operations 
>  	.exec_command		= ata_exec_command,
>  	.dev_select		= ata_std_dev_select,
>  
> -	.phy_reset		= piix_pata_phy_reset,
> +	.probe_reset		= piix_pata_probe_reset,
>  
>  	.bmdma_setup		= ata_bmdma_setup,
>  	.bmdma_start		= ata_bmdma_start,
> @@ -259,8 +259,7 @@ static struct ata_port_info piix_port_in
>  	/* ich5_pata */
>  	{
>  		.sht		= &piix_sht,
> -		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST |
> -				  PIIX_FLAG_CHECKINTR,
> +		.host_flags	= ATA_FLAG_SLAVE_POSS | PIIX_FLAG_CHECKINTR,
>  		.pio_mask	= 0x1f,	/* pio0-4 */
>  #if 0
>  		.mwdma_mask	= 0x06, /* mwdma1-2 */
> @@ -285,7 +284,7 @@ static struct ata_port_info piix_port_in
>  	/* piix4_pata */
>  	{
>  		.sht		= &piix_sht,
> -		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
> +		.host_flags	= ATA_FLAG_SLAVE_POSS,
>  		.pio_mask	= 0x1f,	/* pio0-4 */
>  #if 0
>  		.mwdma_mask	= 0x06, /* mwdma1-2 */
> @@ -367,30 +366,46 @@ cbl40:
>  }
>  
>  /**
> - *	piix_pata_phy_reset - Probe specified port on PATA host controller
> - *	@ap: Port to probe
> + *	piix_pata_postreset - Postreset stuff on PATA host controller
> + *	@ap: Target port
> + *	@classes: Classes of attached devices
>   *
> - *	Probe PATA phy.
> + *	Postreset processing including cable detection.
>   *
>   *	LOCKING:
>   *	None (inherited from caller).
>   */
> +static void piix_pata_postreset(struct ata_port *ap, unsigned int *classes)
> +{
> +	piix_pata_cbl_detect(ap);
> +	ata_std_postreset(ap, classes);
> +}
>  
> -static void piix_pata_phy_reset(struct ata_port *ap)
> +/**
> + *	piix_pata_probe_reset - Perform reset on PATA port and classify
> + *	@ap: Port to reset
> + *	@classes: Resulting classes of attached devices
> + *
> + *	Reset PATA phy and classify attached devices.
> + *
> + *	LOCKING:
> + *	None (inherited from caller).
> + */
> +static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes)
>  {
>  	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
> +	int i;
>  
>  	if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->hard_port_no])) {
> -		ata_port_disable(ap);
> +		for (i = 0; i < ATA_MAX_DEVICES; i++)
> +			classes[i] = ATA_DEV_NONE;

ACK except for the above loop.  The above is really a regression, since 
its hand-coding something in a driver that libata should really be 
responsible for.

Whether you can ata_port_disable() or create a new function, the driver 
should be making a function call not hand-coding the loop.

Think about it conceptually:  the driver is really requesting that 
libata mark all devices on this port non-existent.  If you ever need to 
change the definition of non-existent (as this patch does, in fact), 
then its much easier to modify a function in libata once, rather than 
touch each driver as necessary.

	Jeff

Re: [PATCH 7/9] ata_piix: convert sata to new reset mechanism

[Jeff Garzik]

Tejun Heo wrote:
> Convert ata_piix sata ->phy_reset to new reset mechanism.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/ata_piix.c |   39 +++++++++++++++++++--------------------
>  1 files changed, 19 insertions(+), 20 deletions(-)
> 
> 9314defd1d310a822bf491b60cddbf9d652ab6e0
> diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
> index 650c687..0da8076 100644
> --- a/drivers/scsi/ata_piix.c
> +++ b/drivers/scsi/ata_piix.c
> @@ -132,7 +132,7 @@ static int piix_init_one (struct pci_dev
>  				    const struct pci_device_id *ent);
>  
>  static int piix_pata_probe_reset(struct ata_port *ap, unsigned int *classes);
> -static void piix_sata_phy_reset(struct ata_port *ap);
> +static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes);
>  static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev);
>  static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
>  
> @@ -236,7 +236,7 @@ static const struct ata_port_operations 
>  	.exec_command		= ata_exec_command,
>  	.dev_select		= ata_std_dev_select,
>  
> -	.phy_reset		= piix_sata_phy_reset,
> +	.probe_reset		= piix_sata_probe_reset,
>  
>  	.bmdma_setup		= ata_bmdma_setup,
>  	.bmdma_start		= ata_bmdma_start,
> @@ -273,8 +273,8 @@ static struct ata_port_info piix_port_in
>  	/* ich5_sata */
>  	{
>  		.sht		= &piix_sht,
> -		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
> -				  PIIX_FLAG_COMBINED | PIIX_FLAG_CHECKINTR,
> +		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED |
> +				  PIIX_FLAG_CHECKINTR,
>  		.pio_mask	= 0x1f,	/* pio0-4 */
>  		.mwdma_mask	= 0x07, /* mwdma0-2 */
>  		.udma_mask	= 0x7f,	/* udma0-6 */
> @@ -298,8 +298,7 @@ static struct ata_port_info piix_port_in
>  	/* ich6_sata */
>  	{
>  		.sht		= &piix_sht,
> -		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
> -				  PIIX_FLAG_COMBINED_ICH6 |
> +		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 |
>  				  PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS,
>  		.pio_mask	= 0x1f,	/* pio0-4 */
>  		.mwdma_mask	= 0x07, /* mwdma0-2 */
> @@ -310,8 +309,7 @@ static struct ata_port_info piix_port_in
>  	/* ich6_sata_ahci */
>  	{
>  		.sht		= &piix_sht,
> -		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST |
> -				  PIIX_FLAG_COMBINED_ICH6 |
> +		.host_flags	= ATA_FLAG_SATA | PIIX_FLAG_COMBINED_ICH6 |
>  				  PIIX_FLAG_CHECKINTR | ATA_FLAG_SLAVE_POSS |
>  				  PIIX_FLAG_AHCI,
>  		.pio_mask	= 0x1f,	/* pio0-4 */
> @@ -456,28 +454,29 @@ static int piix_sata_probe (struct ata_p
>  }
>  
>  /**
> - *	piix_sata_phy_reset - Probe specified port on SATA host controller
> - *	@ap: Port to probe
> + *	piix_sata_probe_reset - Perform reset on SATA port and classify
> + *	@ap: Port to reset
> + *	@classes: Resulting classes of attached devices
>   *
> - *	Probe SATA phy.
> + *	Reset SATA phy and classify attached devices.
>   *
>   *	LOCKING:
>   *	None (inherited from caller).
>   */
> -
> -static void piix_sata_phy_reset(struct ata_port *ap)
> +static int piix_sata_probe_reset(struct ata_port *ap, unsigned int *classes)
>  {
> +	int i;
> +
>  	if (!piix_sata_probe(ap)) {
> -		ata_port_disable(ap);
> +		for (i = 0; i < ATA_MAX_DEVICES; i++)
> +			classes[i] = ATA_DEV_NONE;

Same comment as previous patch.

	Jeff