Re: SATA: link online but device misclassified

[Tejun Heo <htejun@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

Marc Bowes wrote:
> Ok. Just keep me informed :)

Please test the attached patch and report the resulting boot log.  Oh
and please turn on CONFIG_PRINTK_TIME.

Thanks.

-- 
tejun

[-- Attachment #2: gigabyte-unreliable-online.patch --]
[-- Type: text/x-patch, Size: 3975 bytes --]

 drivers/ata/ahci.c      |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/ata/libata-eh.c |   15 +++++++++++----
 include/linux/libata.h  |    1 +
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 2141a31..58cf5f7 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -2677,6 +2677,48 @@ static bool ahci_broken_suspend(struct pci_dev *pdev)
 	return !ver || strcmp(ver, dmi->driver_data) < 0;
 }
 
+static bool ahci_broken_online(struct pci_dev *pdev)
+{
+#define ENCODE_BUSDEVFN(bus, slot, func)			\
+	(void *)(unsigned long)(((bus) << 8) | PCI_DEVFN((slot), (func)))
+	static const struct dmi_system_id sysids[] = {
+		/*
+		 * There are several gigabyte boards which use
+		 * SIMG5723s configured as hardware RAID.  Certain
+		 * 5723 firmware revisions shipped there keep the link
+		 * online but fail to answer properly to SRST or
+		 * IDENTIFY when no device is attached downstream
+		 * causing libata to retry quite a few times leading
+		 * to excessive detection delay.
+		 *
+		 * As these firmwares respond to the second reset try
+		 * with invalid device signature, considering unknown
+		 * sig as offline works around the problem acceptably.
+		 */
+		{
+			.ident = "EP45-DQ6",
+			.matches = {
+				DMI_MATCH(DMI_BOARD_VENDOR,
+					  "Gigabyte Technology Co., Ltd."),
+				DMI_MATCH(DMI_BOARD_NAME,
+					  "EP45-DQ6"),
+			},
+			.driver_data = ENCODE_BUSDEVFN(0x0a, 0x00, 0),
+		},
+		{ }	/* terminate list */
+	};
+#undef ENCODE_BUSDEVFN
+	const struct dmi_system_id *dmi = dmi_first_match(sysids);
+	unsigned int val;
+
+	if (!dmi)
+		return false;
+
+	val = (unsigned long)dmi->driver_data;
+
+	return pdev->bus->number == (val >> 8) && pdev->devfn == (val & 0xff);
+}
+
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version;
@@ -2788,6 +2830,12 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 			   "BIOS update required for suspend/resume\n");
 	}
 
+	if (ahci_broken_online(pdev)) {
+		pi.link_flags |= ATA_LFLAG_UNK_IS_OFFLINE;
+		dev_info(&pdev->dev,
+			 "online status unreliable, applying workaround\n");
+	}
+
 	/* CAP.NP sometimes indicate the index of the last enabled
 	 * port, at other times, that of the last possible port, so
 	 * determining the maximum port number requires looking at
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 94919ad..cfc98c3 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2595,16 +2595,23 @@ int ata_eh_reset(struct ata_link *link, int classify,
 	}
 
 	if (classify && nr_unknown) {
-		if (try < max_tries) {
+		bool unk_is_offline = link->flags & ATA_LFLAG_UNK_IS_OFFLINE;
+
+		if (!unk_is_offline && try < max_tries) {
 			ata_link_printk(link, KERN_WARNING, "link online but "
 				       "device misclassified, retrying\n");
 			failed_link = link;
 			rc = -EAGAIN;
 			goto fail;
 		}
-		ata_link_printk(link, KERN_WARNING,
-			       "link online but device misclassified, "
-			       "device detection might fail\n");
+		if (unk_is_offline)
+			ata_link_printk(link, KERN_INFO,
+					"link online but device misclassified, "
+					"treating as offline\n");
+		else
+			ata_link_printk(link, KERN_WARNING,
+					"link online but device misclassified, "
+					"device detection might fail\n");
 	}
 
 	/* reset successful, schedule revalidation */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3d501db..934e805 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -172,6 +172,7 @@ enum {
 	ATA_LFLAG_NO_RETRY	= (1 << 5), /* don't retry this link */
 	ATA_LFLAG_DISABLED	= (1 << 6), /* link is disabled */
 	ATA_LFLAG_SW_ACTIVITY	= (1 << 7), /* keep activity stats */
+	ATA_LFLAG_UNK_IS_OFFLINE = (1 << 8), /* treat unknown sig as offline */
 
 	/* struct ata_port flags */
 	ATA_FLAG_SLAVE_POSS	= (1 << 0), /* host supports slave dev */