[PATCH 1/2] libata: backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA

[PATCH 1/2] libata: backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA

[Tejun Heo]

Backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA.  These are
originally link flags (ATA_LFLAG_*) but link abstraction doesn't exist
on 2.6.23, so make it port flags.

This is for the following workaround for ASUS P5W DH Deluxe.

These new flags don't introduce any behavior change unless set and
nobody sets them yet.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
This and the next patch are a bit large for -stable but they don't
change anything for machines other than P5W DH and P5W DH users have
been suffering long enough, so I think it'll be nice to include these
patches in the next -stable release.  However, feel free to NACK if
you can see some danger in these patches.

Jeff, what do you think?

 drivers/ata/libata-eh.c |   32 ++++++++++++++++++++++++--------
 include/linux/libata.h  |    2 ++
 2 files changed, 26 insertions(+), 8 deletions(-)

Index: tree0/drivers/ata/libata-eh.c
===================================================================
--- tree0.orig/drivers/ata/libata-eh.c
+++ tree0/drivers/ata/libata-eh.c
@@ -1759,9 +1759,11 @@ static int ata_do_reset(struct ata_port 
 	return 0;
 }
 
-static int ata_eh_followup_srst_needed(int rc, int classify,
-				       const unsigned int *classes)
+static int ata_eh_followup_srst_needed(struct ata_port *ap, int rc,
+				int classify, const unsigned int *classes)
 {
+	if (ap->flags & ATA_FLAG_NO_SRST)
+		return 0;
 	if (rc == -EAGAIN)
 		return 1;
 	if (rc != 0)
@@ -1792,7 +1794,8 @@ static int ata_eh_reset(struct ata_port 
 	 */
 	action = ehc->i.action;
 	ehc->i.action &= ~ATA_EH_RESET_MASK;
-	if (softreset && (!hardreset || (!sata_set_spd_needed(ap) &&
+	if (softreset && (!hardreset || (!(ap->flags & ATA_FLAG_NO_SRST) &&
+					 !sata_set_spd_needed(ap) &&
 					 !(action & ATA_EH_HARDRESET))))
 		ehc->i.action |= ATA_EH_SOFTRESET;
 	else
@@ -1855,7 +1858,7 @@ static int ata_eh_reset(struct ata_port 
 	rc = ata_do_reset(ap, reset, classes, deadline);
 
 	if (reset == hardreset &&
-	    ata_eh_followup_srst_needed(rc, classify, classes)) {
+	    ata_eh_followup_srst_needed(ap, rc, classify, classes)) {
 		/* okay, let's do follow-up softreset */
 		reset = softreset;
 
@@ -1870,8 +1873,8 @@ static int ata_eh_reset(struct ata_port 
 		ata_eh_about_to_do(ap, NULL, ATA_EH_RESET_MASK);
 		rc = ata_do_reset(ap, reset, classes, deadline);
 
-		if (rc == 0 && classify &&
-		    classes[0] == ATA_DEV_UNKNOWN) {
+		if (rc == 0 && classify && classes[0] == ATA_DEV_UNKNOWN &&
+		    !(ap->flags & ATA_FLAG_ASSUME_ATA)) {
 			ata_port_printk(ap, KERN_ERR,
 					"classification failed\n");
 			rc = -EINVAL;
@@ -1879,6 +1882,10 @@ static int ata_eh_reset(struct ata_port 
 		}
 	}
 
+	/* if we skipped follow-up srst, clear rc */
+	if (rc == -EAGAIN)
+		rc = 0;
+
 	if (rc && try < ARRAY_SIZE(ata_eh_reset_timeouts)) {
 		unsigned long now = jiffies;
 
@@ -1906,8 +1913,17 @@ static int ata_eh_reset(struct ata_port 
 		/* After the reset, the device state is PIO 0 and the
 		 * controller state is undefined.  Record the mode.
 		 */
-		for (i = 0; i < ATA_MAX_DEVICES; i++)
-			ap->device[i].pio_mode = XFER_PIO_0;
+		for (i = 0; i < ata_port_max_devices(ap); i++) {
+			struct ata_device *dev = &ap->device[i];
+
+			dev->pio_mode = XFER_PIO_0;
+
+			if (ata_port_offline(ap))
+				continue;
+
+			if (ap->flags & ATA_FLAG_ASSUME_ATA)
+				classes[dev->devno] = ATA_DEV_ATA;
+		}
 
 		/* record current link speed */
 		if (sata_scr_read(ap, SCR_STATUS, &sstatus) == 0)
Index: tree0/include/linux/libata.h
===================================================================
--- tree0.orig/include/linux/libata.h
+++ tree0/include/linux/libata.h
@@ -177,6 +177,8 @@ enum {
 	ATA_FLAG_IGN_SIMPLEX	= (1 << 15), /* ignore SIMPLEX */
 	ATA_FLAG_NO_IORDY	= (1 << 16), /* controller lacks iordy */
 	ATA_FLAG_ACPI_SATA	= (1 << 17), /* need native SATA ACPI layout */
+	ATA_FLAG_NO_SRST	= (1 << 18),
+	ATA_FLAG_ASSUME_ATA	= (1 << 19),
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be

[PATCH 2/2 -stable] libata: backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA

[Tejun Heo]

P5W-DH Deluxe has ICH7R which doesn't have PMP support but SIMG 4726
hardwired to the second port of AHCI controller at PCI device 1f.2.
The 4726 doesn't work as PMP but as a storage processor which can do
hardware RAID on downstream ports.

When no device is attached to the downstream port of the 4726, pseudo
ATA device for configuration appears.  Unfortunately, ATA emulation on
the device is very lousy and causes long hang during boot.

This patch implements workaround for the board.  If the mainboard is
P5W-DH Deluxe (matched using DMI), only hardreset is used on the
second port of AHCI controller @ 1f.2 and the hardreset doesn't depend
on receiving the first FIS and just proceed to IDENTIFY.

This workaround fixes bugzilla #8923.

  http://bugzilla.kernel.org/show_bug.cgi?id=8923

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/ahci.c |  143 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

Index: tree0/drivers/ata/ahci.c
===================================================================
--- tree0.orig/drivers/ata/ahci.c
+++ tree0/drivers/ata/ahci.c
@@ -41,6 +41,7 @@
 #include <linux/interrupt.h>
 #include <linux/dma-mapping.h>
 #include <linux/device.h>
+#include <linux/dmi.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_cmnd.h>
 #include <linux/libata.h>
@@ -231,6 +232,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_vt8251_error_handler(struct ata_port *ap);
+static void ahci_p5wdh_error_handler(struct ata_port *ap);
 static void ahci_post_internal_cmd(struct ata_queued_cmd *qc);
 static int ahci_port_resume(struct ata_port *ap);
 static unsigned int ahci_fill_sg(struct ata_queued_cmd *qc, void *cmd_tbl);
@@ -329,6 +331,40 @@ static const struct ata_port_operations 
 	.port_stop		= ahci_port_stop,
 };
 
+static const struct ata_port_operations ahci_p5wdh_ops = {
+	.port_disable		= ata_port_disable,
+
+	.check_status		= ahci_check_status,
+	.check_altstatus	= ahci_check_status,
+	.dev_select		= ata_noop_dev_select,
+
+	.tf_read		= ahci_tf_read,
+
+	.qc_prep		= ahci_qc_prep,
+	.qc_issue		= ahci_qc_issue,
+
+	.irq_clear		= ahci_irq_clear,
+	.irq_on			= ata_dummy_irq_on,
+	.irq_ack		= ata_dummy_irq_ack,
+
+	.scr_read		= ahci_scr_read,
+	.scr_write		= ahci_scr_write,
+
+	.freeze			= ahci_freeze,
+	.thaw			= ahci_thaw,
+
+	.error_handler		= ahci_p5wdh_error_handler,
+	.post_internal_cmd	= ahci_post_internal_cmd,
+
+#ifdef CONFIG_PM
+	.port_suspend		= ahci_port_suspend,
+	.port_resume		= ahci_port_resume,
+#endif
+
+	.port_start		= ahci_port_start,
+	.port_stop		= ahci_port_stop,
+};
+
 static const struct ata_port_info ahci_port_info[] = {
 	/* board_ahci */
 	{
@@ -1176,6 +1212,52 @@ static int ahci_vt8251_hardreset(struct 
 	return rc ?: -EAGAIN;
 }
 
+static int ahci_p5wdh_hardreset(struct ata_port *ap, unsigned int *class,
+				unsigned long deadline)
+{
+	struct ahci_port_priv *pp = ap->private_data;
+	u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
+	struct ata_taskfile tf;
+	int rc;
+
+	ahci_stop_engine(ap);
+
+	/* clear D2H reception area to properly wait for D2H FIS */
+	ata_tf_init(ap->device, &tf);
+	tf.command = 0x80;
+	ata_tf_to_fis(&tf, 0, 0, d2h_fis);
+
+	rc = sata_port_hardreset(ap, sata_ehc_deb_timing(&ap->eh_context),
+				 deadline);
+
+	ahci_start_engine(ap);
+
+	if (rc || ata_port_offline(ap))
+		return rc;
+
+	/* spec mandates ">= 2ms" before checking status */
+	msleep(150);
+
+	/* The pseudo configuration device on SIMG4726 attached to
+	 * ASUS P5W-DH Deluxe doesn't send signature FIS after
+	 * hardreset if no device is attached to the first downstream
+	 * port && the pseudo device locks up on SRST w/ PMP==0.  To
+	 * work around this, wait for !BSY only briefly.  If BSY isn't
+	 * cleared, perform CLO and proceed to IDENTIFY (achieved by
+	 * ATA_LFLAG_NO_SRST and ATA_LFLAG_ASSUME_ATA).
+	 *
+	 * Wait for two seconds.  Devices attached to downstream port
+	 * which can't process the following IDENTIFY after this will
+	 * have to be reset again.  For most cases, this should
+	 * suffice while making probing snappish enough.
+	 */
+	rc = ata_wait_ready(ap, jiffies + 2 * HZ);
+	if (rc)
+		ahci_kick_engine(ap, 0);
+
+	return 0;
+}
+
 static void ahci_postreset(struct ata_port *ap, unsigned int *class)
 {
 	void __iomem *port_mmio = ahci_port_base(ap);
@@ -1556,6 +1638,19 @@ static void ahci_vt8251_error_handler(st
 		  ahci_postreset);
 }
 
+static void ahci_p5wdh_error_handler(struct ata_port *ap)
+{
+	if (!(ap->pflags & ATA_PFLAG_FROZEN)) {
+		/* restart engine */
+		ahci_stop_engine(ap);
+		ahci_start_engine(ap);
+	}
+
+	/* perform recovery */
+	ata_do_eh(ap, ata_std_prereset, ahci_softreset, ahci_p5wdh_hardreset,
+		  ahci_postreset);
+}
+
 static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
@@ -1802,6 +1897,51 @@ static void ahci_print_info(struct ata_h
 		);
 }
 
+/* On ASUS P5W DH Deluxe, the second port of PCI device 00:1f.2 is
+ * hardwired to on-board SIMG 4726.  The chipset is ICH8 and doesn't
+ * support PMP and the 4726 either directly exports the device
+ * attached to the first downstream port or acts as a hardware storage
+ * controller and emulate a single ATA device (can be RAID 0/1 or some
+ * other configuration).
+ *
+ * When there's no device attached to the first downstream port of the
+ * 4726, "Config Disk" appears, which is a pseudo ATA device to
+ * configure the 4726.  However, ATA emulation of the device is very
+ * lame.  It doesn't send signature D2H Reg FIS after the initial
+ * hardreset, pukes on SRST w/ PMP==0 and has bunch of other issues.
+ *
+ * The following function works around the problem by always using
+ * hardreset on the port and not depending on receiving signature FIS
+ * afterward.  If signature FIS isn't received soon, ATA class is
+ * assumed without follow-up softreset.
+ */
+static void ahci_p5wdh_workaround(struct ata_host *host)
+{
+	static struct dmi_system_id sysids[] = {
+		{
+			.ident = "P5W DH Deluxe",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR,
+					  "ASUSTEK COMPUTER INC"),
+				DMI_MATCH(DMI_PRODUCT_NAME, "P5W DH Deluxe"),
+			},
+		},
+		{ }
+	};
+	struct pci_dev *pdev = to_pci_dev(host->dev);
+
+	if (pdev->bus->number == 0 && pdev->devfn == PCI_DEVFN(0x1f, 2) &&
+	    dmi_check_system(sysids)) {
+		struct ata_port *ap = host->ports[1];
+
+		dev_printk(KERN_INFO, &pdev->dev, "enabling ASUS P5W DH "
+			   "Deluxe on-board SIMG4726 workaround\n");
+
+		ap->ops = &ahci_p5wdh_ops;
+		ap->flags |= ATA_FLAG_NO_SRST | ATA_FLAG_ASSUME_ATA;
+	}
+}
+
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version;
@@ -1863,6 +2003,9 @@ static int ahci_init_one(struct pci_dev 
 			ap->ops = &ata_dummy_port_ops;
 	}
 
+	/* apply workaround for ASUS P5W DH Deluxe mainboard */
+	ahci_p5wdh_workaround(host);
+
 	/* initialize adapter */
 	rc = ahci_configure_dma_masks(pdev, hpriv->cap & HOST_CAP_64);
 	if (rc)

Re: [PATCH 1/2] libata: backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA

[Jeff Garzik]

Tejun Heo wrote:
> Backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA.  These are
> originally link flags (ATA_LFLAG_*) but link abstraction doesn't exist
> on 2.6.23, so make it port flags.
> 
> This is for the following workaround for ASUS P5W DH Deluxe.
> 
> These new flags don't introduce any behavior change unless set and
> nobody sets them yet.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
> This and the next patch are a bit large for -stable but they don't
> change anything for machines other than P5W DH and P5W DH users have
> been suffering long enough, so I think it'll be nice to include these
> patches in the next -stable release.  However, feel free to NACK if
> you can see some danger in these patches.
> 
> Jeff, what do you think?
> 
>  drivers/ata/libata-eh.c |   32 ++++++++++++++++++++++++--------
>  include/linux/libata.h  |    2 ++
>  2 files changed, 26 insertions(+), 8 deletions(-)

ACK from me, though I wonder if we shouldn't wait and get feedback once 
this hits upstream (my next push to Andrew and Linus), before applying 
to stable.  No special reason, just being conservative...

	Jeff

Re: [PATCH 1/2] libata: backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA.  These are
>> originally link flags (ATA_LFLAG_*) but link abstraction doesn't exist
>> on 2.6.23, so make it port flags.
>>
>> This is for the following workaround for ASUS P5W DH Deluxe.
>>
>> These new flags don't introduce any behavior change unless set and
>> nobody sets them yet.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>> ---
>> This and the next patch are a bit large for -stable but they don't
>> change anything for machines other than P5W DH and P5W DH users have
>> been suffering long enough, so I think it'll be nice to include these
>> patches in the next -stable release.  However, feel free to NACK if
>> you can see some danger in these patches.
>>
>> Jeff, what do you think?
>>
>>  drivers/ata/libata-eh.c |   32 ++++++++++++++++++++++++--------
>>  include/linux/libata.h  |    2 ++
>>  2 files changed, 26 insertions(+), 8 deletions(-)
> 
> ACK from me, though I wonder if we shouldn't wait and get feedback once 
> this hits upstream (my next push to Andrew and Linus), before applying 
> to stable.  No special reason, just being conservative...

Agreed.  Let's give the upstream changes a few weeks before updating 
-stable.  I'll ping this thread after a few weeks.

Thanks.

-- 
tejun

Re: [stable] [PATCH 1/2] libata: backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA

[Greg KH]

On Thu, Oct 25, 2007 at 04:36:01PM +0900, Tejun Heo wrote:
> Jeff Garzik wrote:
> > Tejun Heo wrote:
> >> Backport ATA_FLAG_NO_SRST and ATA_FLAG_ASSUME_ATA.  These are
> >> originally link flags (ATA_LFLAG_*) but link abstraction doesn't exist
> >> on 2.6.23, so make it port flags.
> >>
> >> This is for the following workaround for ASUS P5W DH Deluxe.
> >>
> >> These new flags don't introduce any behavior change unless set and
> >> nobody sets them yet.
> >>
> >> Signed-off-by: Tejun Heo <htejun@gmail.com>
> >> ---
> >> This and the next patch are a bit large for -stable but they don't
> >> change anything for machines other than P5W DH and P5W DH users have
> >> been suffering long enough, so I think it'll be nice to include these
> >> patches in the next -stable release.  However, feel free to NACK if
> >> you can see some danger in these patches.
> >>
> >> Jeff, what do you think?
> >>
> >>  drivers/ata/libata-eh.c |   32 ++++++++++++++++++++++++--------
> >>  include/linux/libata.h  |    2 ++
> >>  2 files changed, 26 insertions(+), 8 deletions(-)
> > 
> > ACK from me, though I wonder if we shouldn't wait and get feedback once 
> > this hits upstream (my next push to Andrew and Linus), before applying 
> > to stable.  No special reason, just being conservative...
> 
> Agreed.  Let's give the upstream changes a few weeks before updating 
> -stable.  I'll ping this thread after a few weeks.

Ok, thanks, I'll hold off until you respond.

greg k-h