Patchwork Intermittent SATA link down SStatus 0

login
register
mail settings
Submitter Tejun Heo
Date July 15, 2010, 9:29 a.m.
Message ID <4C3ED4E2.9080806@kernel.org>
Download mbox | patch
Permalink /patch/58969/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Tejun Heo - July 15, 2010, 9:29 a.m.
On 07/15/2010 11:20 AM, Tejun Heo wrote:
> Hmm... it looks like SStatus and SControl are swapped here.  Maybe
> there's a race condition in SIDPR code.  I'll look into it a bit more.

So, something like this.  Can you please apply the patch and see
whether the problem goes away?

Thanks.
Paul Check - July 15, 2010, 5:06 p.m.
Ok, I've applied the patch and had a successful boot on the first try. Of
course the problem appears only about 40% of the time, so I'll re-boot
several times later and see if I can get 10 in a row.  I'm busy now
(unsuccessful => raid re-build, no time for that now)...will report back
later today.  P

> On 07/15/2010 11:20 AM, Tejun Heo wrote:
>> Hmm... it looks like SStatus and SControl are swapped here.  Maybe
>> there's a race condition in SIDPR code.  I'll look into it a bit more.
>
> So, something like this.  Can you please apply the patch and see
> whether the problem goes away?


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 7409f98..3971bc0 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -158,6 +158,7 @@  struct piix_map_db {
 struct piix_host_priv {
 	const int *map;
 	u32 saved_iocfg;
+	spinlock_t sidpr_lock;	/* FIXME: remove once locking in EH is fixed */
 	void __iomem *sidpr;
 };
 
@@ -951,12 +952,15 @@  static int piix_sidpr_scr_read(struct ata_link *link,
 			       unsigned int reg, u32 *val)
 {
 	struct piix_host_priv *hpriv = link->ap->host->private_data;
+	unsigned long flags;
 
 	if (reg >= ARRAY_SIZE(piix_sidx_map))
 		return -EINVAL;
 
+	spin_lock_irqsave(&hpriv->sidpr_lock, flags);
 	piix_sidpr_sel(link, reg);
 	*val = ioread32(hpriv->sidpr + PIIX_SIDPR_DATA);
+	spin_unlock_irqrestore(&hpriv->sidpr_lock, flags);
 	return 0;
 }
 
@@ -964,12 +968,15 @@  static int piix_sidpr_scr_write(struct ata_link *link,
 				unsigned int reg, u32 val)
 {
 	struct piix_host_priv *hpriv = link->ap->host->private_data;
+	unsigned long flags;
 
 	if (reg >= ARRAY_SIZE(piix_sidx_map))
 		return -EINVAL;
 
+	spin_lock_irqsave(&hpriv->sidpr_lock, flags);
 	piix_sidpr_sel(link, reg);
 	iowrite32(val, hpriv->sidpr + PIIX_SIDPR_DATA);
+	spin_unlock_irqrestore(&hpriv->sidpr_lock, flags);
 	return 0;
 }
 
@@ -1566,6 +1573,7 @@  static int __devinit piix_init_one(struct pci_dev *pdev,
 	hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
 	if (!hpriv)
 		return -ENOMEM;
+	spin_lock_init(&hpriv->sidpr_lock);
 
 	/* Save IOCFG, this will be used for cable detection, quirk
 	 * detection and restoration on detach.  This is necessary