Skip to contentSkip to conversation

ata_piix: add workaround for Samsung DB-P70

PATCH Applied Tejun Heo 3 participants 5 messages Mar 3, 2009 – Mar 13, 2009 libata

lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 ata_piix: add workaround for Samsung DB-P70

Samsung DB-P70 somehow botched the first ICH9 SATA port. The board doesn't expose the first port but somehow SStatus reports link online while failing SRST protocol leading to repeated probe failures and thus long boot delay.

Because the BIOS doesn't carry any identifying DMI information, the port can't be blacklisted safely. Fortunately, the controller does have subsystem vendor and ID set. It's unclear whether the subsystem IDs are used only for the board but it can be safely worked around by disabling SIDPR access and just using SRST works around the problem. Even when the workaround is triggered on an unaffected board the only side effect will be missing SCR access.

---
 drivers/ata/ata_piix.c |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
drivers/ata/ata_piix.c +37 -0
index 54961c0..ef8b30d 100644
@@ -1289,6 +1289,39 @@ static const int *__devinit piix_init_sata_map(struct pci_dev *pdev,
 	return map;
 }
 
+static bool piix_no_sidpr(struct ata_host *host)
+{
+	struct pci_dev *pdev = to_pci_dev(host->dev);
+
+	/*
+	 * Samsung DB-P70 only has three ATA ports exposed and
+	 * curiously the unconnected first port reports link online
+	 * while not responding to SRST protocol causing excessive
+	 * detection delay.
+	 *
+	 * Unfortunately, the system doesn't carry enough DMI
+	 * information to identify the machine but does have subsystem
+	 * vendor and device set.  As it's unclear whether the
+	 * subsystem vendor/device is used only for this specific
+	 * board, the port can't be disabled solely with the
+	 * information; however, turning off SIDPR access works around
+	 * the problem.  Turn it off.
+	 *
+	 * This problem is reported in bnc#441240.
+	 *
+	 * https://bugzilla.novell.com/show_bug.cgi?id=441420
+	 */
+	if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x2920 &&
+	    pdev->subsystem_vendor == PCI_VENDOR_ID_SAMSUNG &&
+	    pdev->subsystem_device == 0xb049) {
+		dev_printk(KERN_WARNING, host->dev,
+			   "Samsung DB-P70 detected, disabling SIDPR\n");
+		return true;
+	}
+
+	return false;
+}
+
 static int __devinit piix_init_sidpr(struct ata_host *host)
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);
@@ -1302,6 +1335,10 @@ static int __devinit piix_init_sidpr(struct ata_host *host)
 		if (hpriv->map[i] == IDE)
 			return 0;
 
+	/* is it blacklisted? */
+	if (piix_no_sidpr(host))
+		return 0;
+
 	if (!(host->ports[0]->flags & PIIX_FLAG_SIDPR))
 		return 0;
 
Signature · 1 line
tejun
#1 lore raw
Quoted 5 lines · Tejun Heo

Samsung DB-P70 somehow botched the first ICH9 SATA port. The board doesn't expose the first port but somehow SStatus reports link online while failing SRST protocol leading to repeated probe failures and thus long boot delay.

Quoted 7 lines

Because the BIOS doesn't carry any identifying DMI information, the port can't be blacklisted safely. Fortunately, the controller does have subsystem vendor and ID set. It's unclear whether the subsystem IDs are used only for the board but it can be safely worked around by disabling SIDPR access and just using SRST works around the problem. Even when the workaround is triggered on an unaffected board the only side effect will be missing SCR access.

..

A suggestion, then. As a follow-up patch, would it be possible to drop the workaround kludge, and restore the missing SCR access, __after the first time that a drive is detected on that port__?

This way, there won't be any funny business for cases where the same ID might occur on different (non-botched) hardware ?

Probably overkill, though. :)

Cheers

author
#2 lore raw

Hello, Mark.

Quoted 3 lines · Mark Lord

A suggestion, then. As a follow-up patch, would it be possible to drop the workaround kludge, and restore the missing SCR access, __after the first time that a drive is detected on that port__?

Hmmm... yeah. Maybe but it would either involve changing ap->ops on the fly (which is a big no no) or wrap SCR access such that it returns -EOPNOTSUPP if the kludge is in effect.

Quoted 4 lines

This way, there won't be any funny business for cases where the same ID might occur on different (non-botched) hardware ?

Probably overkill, though. :)

Missing SCR access doesn't impact direct usage too much, so yeah I think it should be fine as it is until there's some evidence that the subsystem IDs are in far wider use.

Thanks.

Signature · 1 line
tejun
#3 lore raw
Quoted 11 lines · Tejun Heo

Hello, Mark.

Quoted 3 lines · Mark Lord

A suggestion, then. As a follow-up patch, would it be possible to drop the workaround kludge, and restore the missing SCR access, __after the first time that a drive is detected on that port__?

Hmmm... yeah. Maybe but it would either involve changing ap->ops on the fly (which is a big no no) or wrap SCR access such that it returns -EOPNOTSUPP if the kludge is in effect.

Quoted 8 lines
Quoted 4 lines

This way, there won't be any funny business for cases where the same ID might occur on different (non-botched) hardware ?

Probably overkill, though. :)

Missing SCR access doesn't impact direct usage too much, so yeah I think it should be fine as it is until there's some evidence that the subsystem IDs are in far wider use.

..

Perfectly sensible!

Applied
#4 lore raw
In reply to Tejun Heo
Quoted 65 lines · Tejun Heo

Samsung DB-P70 somehow botched the first ICH9 SATA port. The board doesn't expose the first port but somehow SStatus reports link online while failing SRST protocol leading to repeated probe failures and thus long boot delay.

Because the BIOS doesn't carry any identifying DMI information, the port can't be blacklisted safely. Fortunately, the controller does have subsystem vendor and ID set. It's unclear whether the subsystem IDs are used only for the board but it can be safely worked around by disabling SIDPR access and just using SRST works around the problem. Even when the workaround is triggered on an unaffected board the only side effect will be missing SCR access.

---
 drivers/ata/ata_piix.c |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
drivers/ata/ata_piix.c +33 -0
index 54961c0..ef8b30d 100644
@@ -1289,6 +1289,39 @@ static const int *__devinit piix_init_sata_map(struct pci_dev *pdev,
 	return map;
 }
 
+static bool piix_no_sidpr(struct ata_host *host)
+{
+	struct pci_dev *pdev = to_pci_dev(host->dev);
+
+	/*
+	 * Samsung DB-P70 only has three ATA ports exposed and
+	 * curiously the unconnected first port reports link online
+	 * while not responding to SRST protocol causing excessive
+	 * detection delay.
+	 *
+	 * Unfortunately, the system doesn't carry enough DMI
+	 * information to identify the machine but does have subsystem
+	 * vendor and device set.  As it's unclear whether the
+	 * subsystem vendor/device is used only for this specific
+	 * board, the port can't be disabled solely with the
+	 * information; however, turning off SIDPR access works around
+	 * the problem.  Turn it off.
+	 *
+	 * This problem is reported in bnc#441240.
+	 *
+	 * https://bugzilla.novell.com/show_bug.cgi?id=441420
+	 */
+	if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x2920 &&
+	    pdev->subsystem_vendor == PCI_VENDOR_ID_SAMSUNG &&
+	    pdev->subsystem_device == 0xb049) {
+		dev_printk(KERN_WARNING, host->dev,
+			   "Samsung DB-P70 detected, disabling SIDPR\n");
+		return true;
+	}
+
+	return false;
+}
+
 static int __devinit piix_init_sidpr(struct ata_host *host)
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);

drivers/ata/ata_piix.c static int __devinit piix_init_sidpr(struct ata_host *host) · line 1340

Quoted 6 lines
drivers/ata/ata_piix.c +3 -0
@@ -1302,6 +1335,10 @@ static int __devinit piix_init_sidpr(struct ata_host *host)
 		if (hpriv->map[i] == IDE)
 			return 0;
 
+	/* is it blacklisted? */
+	if (piix_no_sidpr(host))
+		return 0;

applied