Patchwork link resets with SSD on AHCI

login
register
mail settings
Submitter Tejun Heo
Date May 5, 2010, 10:15 a.m.
Message ID <4BE1452A.6040607@kernel.org>
Download mbox | patch
Permalink /patch/51683/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Tejun Heo - May 5, 2010, 10:15 a.m.
Hello,

On 04/29/2010 11:59 PM, Olof Johansson wrote:
> I did notice that ALPM is enabled at boot, and doesn't seem to be
> re-enabled after the error reset. Based on this, I experimented with
> disabling it (just returning -EINVAL in ahci_enable_alpm). That did make
> the problem not happen after a significant test run (overnight vs 4.5
> minutes above).

It could be that libata's ALPM enable sequence isn't liked by the
controller.  libata first resets the link disabling all powersave
transitions, then turn on ALPM then allows powersave transitions.
It's possible that the controller or device somehow gets upset by this
(ie. the device is told to go to powersave mode only to find out that
the host side isn't allowing it).

Does the attached patch make any difference?  Can you please post the
kernel boot log with the patch applied?

Thanks.
Olof Johansson - May 12, 2010, 6:49 p.m.
On Wed, May 05, 2010 at 12:15:06PM +0200, Tejun Heo wrote:
> Hello,
> 
> On 04/29/2010 11:59 PM, Olof Johansson wrote:
> > I did notice that ALPM is enabled at boot, and doesn't seem to be
> > re-enabled after the error reset. Based on this, I experimented with
> > disabling it (just returning -EINVAL in ahci_enable_alpm). That did make
> > the problem not happen after a significant test run (overnight vs 4.5
> > minutes above).
> 
> It could be that libata's ALPM enable sequence isn't liked by the
> controller.  libata first resets the link disabling all powersave
> transitions, then turn on ALPM then allows powersave transitions.
> It's possible that the controller or device somehow gets upset by this
> (ie. the device is told to go to powersave mode only to find out that
> the host side isn't allowing it).
> 
> Does the attached patch make any difference?  Can you please post the
> kernel boot log with the patch applied?

The patch didn't make a difference for me, but I got sidetracked looking
at some other things and didn't get a chance to collect data just yet;
it's coming.

I also got second thoughts about what actually makes it happen; and
I am now of the suspicion that it does indeed take an invocation of
the laptop-mode tools setting power settings for battery operation
(i.e. taking the link off of max_performance) for the problem to
show. Earlier I had been of the impression that it happened even with
it being left untouched, that does not seem to be the case.

As for ALPM overall: I have also noticed that the link doesn't actually
seem to be put in low-power mode with these SSD devices, since I see
now power consumption difference between having it on or off.


-Olof

--
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/libata-core.c b/drivers/ata/libata-core.c
index 49cffb6..696be5f 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3810,7 +3810,7 @@  int sata_link_resume(struct ata_link *link, const unsigned long *params,
 	 * cleared.
 	 */
 	do {
-		scontrol = (scontrol & 0x0f0) | 0x300;
+		scontrol = (scontrol & 0x0f0)/* | 0x300*/;
 		if ((rc = sata_scr_write(link, SCR_CONTROL, scontrol)))
 			return rc;
 		/*
@@ -3823,9 +3823,9 @@  int sata_link_resume(struct ata_link *link, const unsigned long *params,
 		/* is SControl restored correctly? */
 		if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
 			return rc;
-	} while ((scontrol & 0xf0f) != 0x300 && --tries);
+	} while ((scontrol & 0xf0f) != /*0x300*/0 && --tries);
 
-	if ((scontrol & 0xf0f) != 0x300) {
+	if ((scontrol & 0xf0f) != /*0x300*/0) {
 		ata_link_printk(link, KERN_ERR,
 				"failed to resume link (SControl %X)\n",
 				scontrol);