Skip to contentSkip to conversation

ata_piix: verify SIDPR access before enabling it

PATCH In review Tejun Heo 2 participants 2 messages May 13, 2008 libata

lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 ata_piix: verify SIDPR access before enabling it

On certain configurations (certain macbooks), even though all the conditions for SIDPR access described in the datasheet are met, actually reading those registers just returns 0 and have no effect on write. Verify SIDPR is actually working before enabling it.

This is reported by Ryan Roth in bz#10512.

---
This patch is now in Linus' tree and it's probably a good idea to include it for 2.6.25-x.

Thanks.

drivers/ata/ata_piix.c | 25 +++++++++++++++++++++++++

1 file changed, 25 insertions(+)
drivers/ata/ata_piix.c +0 -0
index ea2c764..a9027b8 100644
@@ -1348,6 +1348,8 @@ static void __devinit piix_init_sidpr(struct ata_host *host)
{
	struct pci_dev *pdev = to_pci_dev(host->dev);
	struct piix_host_priv *hpriv = host->private_data;
+	struct ata_device *dev0 = &host->ports[0]->link.device[0];
+	u32 scontrol;
	int i;

	/* check for availability */
@@ -1366,6 +1368,29 @@ static void __devinit piix_init_sidpr(struct ata_host *host)
		return;

	hpriv->sidpr = pcim_iomap_table(pdev)[PIIX_SIDPR_BAR];

+
+ /* SCR access via SIDPR doesn't work on some configurations.
+ * Give it a test drive by inhibiting power save modes which
+ * we'll do anyway.
+ */
+ scontrol = piix_sidpr_read(dev0, SCR_CONTROL);
+
+ /* if IPM is already 3, SCR access is probably working. Don't
+ * un-inhibit power save modes as BIOS might have inhibited
+ * them for a reason.
+ */
+ if ((scontrol & 0xf00) != 0x300) {
+ scontrol |= 0x300;
+ piix_sidpr_write(dev0, SCR_CONTROL, scontrol);
+ scontrol = piix_sidpr_read(dev0, SCR_CONTROL);
+
+ if ((scontrol & 0xf00) != 0x300) {
+ dev_printk(KERN_INFO, host->dev, "SCR access via "
+ "SIDPR is available but doesn't work\n");
+ return;
+ }
+ }
+

	host->ports[0]->ops = &piix_sidpr_sata_ops;
	host->ports[1]->ops = &piix_sidpr_sata_ops;
}
#1 lore raw

patch ata_piix-verify-sidpr-access-before-enabling-it.patch added to 2.6.25-stable tree

This is a note to let you know that we have just queued up the patch titled

Subject: ata_piix: verify SIDPR access before enabling it

to the 2.6.25-stable tree. Its filename is

ata_piix-verify-sidpr-access-before-enabling-it.patch

A git repo of this tree can be found at
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

Quoted 1 line

From [email protected] Tue May 13 01:24:07 2008

From: Tejun Heo <[email protected]>
Date: Tue, 13 May 2008 17:23:38 +0900
Subject: ata_piix: verify SIDPR access before enabling it
To: [email protected], Jeff Garzik l <[email protected]>, IDE/ATA development list <[email protected]>
Message-ID: <[email protected]>

From: Tejun Heo <[email protected]>

commit cb6716c879ecf49e2af344926c6a476821812061 upstream

On certain configurations (certain macbooks), even though all the conditions for SIDPR access described in the datasheet are met, actually reading those registers just returns 0 and have no effect on write. Verify SIDPR is actually working before enabling it.

This is reported by Ryan Roth in bz#10512.

---
 drivers/ata/ata_piix.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
drivers/ata/ata_piix.c +25 -0
@@ -1531,6 +1531,8 @@ static void __devinit piix_init_sidpr(st
 {
 	struct pci_dev *pdev = to_pci_dev(host->dev);
 	struct piix_host_priv *hpriv = host->private_data;
+	struct ata_device *dev0 = &host->ports[0]->link.device[0];
+	u32 scontrol;
 	int i;
 
 	/* check for availability */
@@ -1549,6 +1551,29 @@ static void __devinit piix_init_sidpr(st
 		return;
 
 	hpriv->sidpr = pcim_iomap_table(pdev)[PIIX_SIDPR_BAR];
+
+	/* SCR access via SIDPR doesn't work on some configurations.
+	 * Give it a test drive by inhibiting power save modes which
+	 * we'll do anyway.
+	 */
+	scontrol = piix_sidpr_read(dev0, SCR_CONTROL);
+
+	/* if IPM is already 3, SCR access is probably working.  Don't
+	 * un-inhibit power save modes as BIOS might have inhibited
+	 * them for a reason.
+	 */
+	if ((scontrol & 0xf00) != 0x300) {
+		scontrol |= 0x300;
+		piix_sidpr_write(dev0, SCR_CONTROL, scontrol);
+		scontrol = piix_sidpr_read(dev0, SCR_CONTROL);
+
+		if ((scontrol & 0xf00) != 0x300) {
+			dev_printk(KERN_INFO, host->dev, "SCR access via "
+				   "SIDPR is available but doesn't work\n");
+			return;
+		}
+	}
+
 	host->ports[0]->ops = &piix_sidpr_sata_ops;
 	host->ports[1]->ops = &piix_sidpr_sata_ops;
 }

Patches currently in stable-queue which might be from [email protected] are

queue-2.6.25/ata_piix-verify-sidpr-access-before-enabling-it.patch