sata_nv hotplug in 2.6.18

sata_nv hotplug in 2.6.18

[Jim Paris]

Hi Tejun,

I set up a new machine with a NVIDIA SATA controller and 2.6.18.
Hotplug did not work correctly.  I discovered that it's the same
problem Philipp Wagner had a few months ago with 2.6.18-rc4:

  http://www.spinics.net/lists/linux-ide/msg04081.html

The patch you provided there solves the problem on my system as well.
I haven't seen this patch mentioned anywhere else, and I don't see 
it in libata-dev.git, so I just wanted to check on its status.

Relevant logs below.

Thanks,
-jim

######

Affected device:

00:07.0 IDE interface: nVidia Corporation CK804 Serial ATA Controller (rev f3) (prog-if 85 [Master SecO PriO])
        Subsystem: Tyan Computer Unknown device 2877
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
        Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
        Latency: 0 (750ns min, 250ns max)
        Interrupt: pin A routed to IRQ 58
        Region 0: I/O ports at 1440 [size=8]
        Region 1: I/O ports at 1434 [size=4]
        Region 2: I/O ports at 1438 [size=8]
        Region 3: I/O ports at 1430 [size=4]
        Region 4: I/O ports at 1410 [size=16]
        Region 5: Memory at d8002000 (32-bit, non-prefetchable) [size=4K]
        Capabilities: [44] Power Management version 2
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 PME-Enable- DSel=0 DScale=0 PME-

######

Without patch:

Removed disk:

[  298.452420] ata3: exception Emask 0x10 SAct 0x0 SErr 0x1910000 action 0x2 frozen
[  299.169503] ata3: soft resetting port
[  299.169509] ata3: SATA link down (SStatus 0 SControl 300)
[  299.169514] ata3: failed to recover some devices, retrying in 5 secs
[  304.171105] ata3: hard resetting port
[  304.894753] ata3: SATA link down (SStatus 0 SControl 300)
[  304.894758] ata3: failed to recover some devices, retrying in 5 secs
[  309.896354] ata3: hard resetting port
[  310.620003] ata3: SATA link down (SStatus 0 SControl 300)
[  310.620010] ata3.00: disabled
[  311.123763] ata3: EH complete
[  311.123774] ata3.00: detaching (SCSI 2:0:0:0)

Plugged disk back in:

[  352.429871] ata3: exception Emask 0x10 SAct 0x0 SErr 0x50000 action 0x2 frozen
[  360.148230] ata3: port is slow to respond, please be patient
[  383.153148] ata3: port failed to respond (30 secs)
[  383.153198] ata3: soft resetting port
[  390.309735] ata3: port is slow to respond, please be patient
[  413.318647] ata3: port failed to respond (30 secs)
[  413.318705] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  413.318712] ata3: EH complete

The disk never comes back.

######

With patch:

Removed disk:

[  132.414496] ata3: exception Emask 0x10 SAct 0x0 SErr 0x1810000 action 0x2 frozen
[  132.925157] ata3: hard resetting port
[  133.648811] ata3: SATA link down (SStatus 0 SControl 300)
[  133.648818] ata3: failed to recover some devices, retrying in 5 secs
[  139.162162] ata3: hard resetting port
[  139.885815] ata3: SATA link down (SStatus 0 SControl 300)
[  139.885821] ata3: failed to recover some devices, retrying in 5 secs
[  145.399165] ata3: hard resetting port
[  146.122816] ata3: SATA link down (SStatus 0 SControl 300)
[  146.122821] ata3.00: disabled
[  146.626577] ata3: EH pending after completion, repeating EH (cnt=4)
[  146.626590] ata3: EH complete
[  146.626597] ata3.00: detaching (SCSI 2:0:0:0)

Plugged disk back in:

[  174.511797] ata3: exception Emask 0x10 SAct 0x0 SErr 0x50000 action 0x2 frozen
[  175.024925] ata3: waiting for device to spin up (8 secs)
[  175.027704] ata3: hard resetting port
[  181.621773] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  181.622628] ata3.00: ATA-7, max UDMA/133, 625142448 sectors: LBA48 NCQ (depth 0/32)
[  181.623677] ata3.00: configured for UDMA/133
[  181.623681] ata3: EH pending after completion, repeating EH (cnt=4)
[  181.623691] ata3: EH complete
[  181.623810]   Vendor: ATA       Model: ST3320620AS       Rev: 3.AA
[  181.623821]   Type:   Direct-Access                      ANSI SCSI revision: 05
[  181.623910] SCSI device sdc: 625142448 512-byte hdwr sectors (320073 MB)
[  181.623923] sdc: Write Protect is off
[  181.623926] sdc: Mode Sense: 00 3a 00 00
[  181.623946] SCSI device sdc: drive cache: write back
[  181.624005] SCSI device sdc: 625142448 512-byte hdwr sectors (320073 MB)
[  181.624017] sdc: Write Protect is off
[  181.624020] sdc: Mode Sense: 00 3a 00 00
[  181.624039] SCSI device sdc: drive cache: write back
[  181.624044]  sdc: sdc1
[  181.636017] sd 2:0:0:0: Attached scsi disk sdc

Everything is fine.

[PATCH] libata: lengthen interval between SRST set and clear

[Tejun Heo]

20us isn't enough for some SATA controllers (sata_nv) ending up not
sending the second FIS27 to clear SRST.  This usually results in SRST
timeout causing excessive delays during hotplug.

This patch lengthens the delay from 20us to 1ms and also converts it
to msleep() instead of busy wait.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Hello, Jim Paris, Philipp Wagner.

Can you guys please give this patch a shot?  This patch is againt
libata-dev#upstream so if you're using 2.6.18, the patched file
(libata-core.c) wouldn't be there.  If that's the case, just edit
drivers/scsi/libata-core.c accordng to the following patch (replace
udelay(20)s in ata_bus_softreset() w/ msleep(1)).

Jeff, this function is called from both old EH and new EH's
ata_std_softreset().  I think this change is safe but curious about
your opinion.

Thanks.

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 753b015..007020e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2414,15 +2414,15 @@ static unsigned int ata_bus_softreset(st
 	/* software reset.  causes dev0 to be selected */
 	if (ap->flags & ATA_FLAG_MMIO) {
 		writeb(ap->ctl, (void __iomem *) ioaddr->ctl_addr);
-		udelay(20);	/* FIXME: flush */
+		msleep(1);	/* FIXME: flush */
 		writeb(ap->ctl | ATA_SRST, (void __iomem *) ioaddr->ctl_addr);
-		udelay(20);	/* FIXME: flush */
+		msleep(1);	/* FIXME: flush */
 		writeb(ap->ctl, (void __iomem *) ioaddr->ctl_addr);
 	} else {
 		outb(ap->ctl, ioaddr->ctl_addr);
-		udelay(10);
+		msleep(1);
 		outb(ap->ctl | ATA_SRST, ioaddr->ctl_addr);
-		udelay(10);
+		msleep(1);
 		outb(ap->ctl, ioaddr->ctl_addr);
 	}
 

Re: [PATCH] libata: lengthen interval between SRST set and clear

[Jim Paris]

Hi Tejun,

> 20us isn't enough for some SATA controllers (sata_nv) ending up not
> sending the second FIS27 to clear SRST.  This usually results in SRST
> timeout causing excessive delays during hotplug.
>
> This patch lengthens the delay from 20us to 1ms and also converts it
> to msleep() instead of busy wait.

Thanks for the patch.  Unfortunately it doesn't seem to help on 2.6.18:

Unplug:
[  190.559849] ata3: exception Emask 0x10 SAct 0x0 SErr 0x1810000 action 0x2 frozen
[  191.274576] ata3: soft resetting port
[  191.274580] ata3: SATA link down (SStatus 0 SControl 300)
[  191.274585] ata3: failed to recover some devices, retrying in 5 secs
[  196.276276] ata3: hard resetting port
[  196.999941] ata3: SATA link down (SStatus 0 SControl 300)
[  196.999947] ata3: failed to recover some devices, retrying in 5 secs
[  202.001639] ata3: hard resetting port
[  202.725303] ata3: SATA link down (SStatus 0 SControl 300)
[  202.725309] ata3.00: disabled
[  203.229080] ata3: EH complete
[  203.229091] ata3.00: detaching (SCSI 2:0:0:0)

Replug:
[  222.501022] ata3: exception Emask 0x10 SAct 0x0 SErr 0x150000 action 0x2 frozen
[  230.220666] ata3: port is slow to respond, please be patient
[  253.226040] ata3: port failed to respond (30 secs)
[  253.226090] ata3: soft resetting port
[  260.398768] ata3: port is slow to respond, please be patient
[  283.404138] ata3: port failed to respond (30 secs)
[  283.404195] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  283.404203] ata3: EH complete

and the disk never comes back.  This is just like my earlier log from
vanilla 2.6.18 (except SErr differs by 0x100000).

-jim

Re: [PATCH] libata: lengthen interval between SRST set and clear

[Tejun Heo]

On Thu, Sep 28, 2006 at 02:23:57AM -0400, Jim Paris wrote:
> Hi Tejun,
> 
> > 20us isn't enough for some SATA controllers (sata_nv) ending up not
> > sending the second FIS27 to clear SRST.  This usually results in SRST
> > timeout causing excessive delays during hotplug.
> >
> > This patch lengthens the delay from 20us to 1ms and also converts it
> > to msleep() instead of busy wait.
> 
> Thanks for the patch.  Unfortunately it doesn't seem to help on 2.6.18:

I see.

Justing, the solution doesn't seem to work.  Have you guys verified
that the controller doesn't send the second H2D regsiter FIS clearing
SRST w/ 20us delay?  If you guys did, the patch needs to make into
tree whether it helps this particular case or not.  If not, I think
it's best to leave it alone.

Jim, can you please test the following patch?  It only adds
ATA_FLAG_HRST_TO_RESUME.

diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 8cd730f..647e7b8 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -257,7 +257,8 @@ static struct ata_port_info nv_port_info
 	/* generic */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
@@ -266,7 +267,8 @@ static struct ata_port_info nv_port_info
 	/* nforce2/3 */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
@@ -275,7 +277,8 @@ static struct ata_port_info nv_port_info
 	/* ck804 */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,

Re: [PATCH] libata: lengthen interval between SRST set and clear

[Jim Paris]

Tejun Heo wrote:

> Jim, can you please test the following patch?  It only adds
> ATA_FLAG_HRST_TO_RESUME.

Yes, just ATA_FLAG_HRST_TO_RESUME also works fine:

Unplug:
[  220.354520] ata3: exception Emask 0x10 SAct 0x0 SErr 0x1810000 action 0x2 frozen
[  220.354591] ata3: hard resetting port
[  221.075828] ata3: SATA link down (SStatus 0 SControl 300)
[  221.075833] ata3: failed to recover some devices, retrying in 5 secs
[  226.080467] ata3: hard resetting port
[  226.804116] ata3: SATA link down (SStatus 0 SControl 300)
[  226.804121] ata3: failed to recover some devices, retrying in 5 secs
[  231.805716] ata3: hard resetting port
[  232.529366] ata3: SATA link down (SStatus 0 SControl 300)
[  232.529373] ata3.00: disabled
[  233.033123] ata3: EH pending after completion, repeating EH (cnt=4)
[  233.033133] ata3: EH complete
[  233.033141] ata3.00: detaching (SCSI 2:0:0:0)

Replug:
[  247.048727] ata3: exception Emask 0x10 SAct 0x0 SErr 0x150000 action 0x2 frozen
[  247.048798] ata3: hard resetting port
[  253.867127] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  253.867930] ata3.00: ATA-7, max UDMA/133, 625142448 sectors: LBA48 NCQ (depth 0/32)
[  253.868982] ata3.00: configured for UDMA/133
[  253.868986] ata3: EH pending after completion, repeating EH (cnt=4)
[  253.868994] ata3: EH complete
[  253.869342]   Vendor: ATA       Model: ST3320620AS       Rev: 3.AA
[  253.869351]   Type:   Direct-Access                      ANSI SCSI revision: 05
[  253.869507] SCSI device sdc: 625142448 512-byte hdwr sectors (320073 MB)
[  253.869737] sdc: Write Protect is off
[  253.869740] sdc: Mode Sense: 00 3a 00 00
[  253.869931] SCSI device sdc: drive cache: write back
[  253.870082] SCSI device sdc: 625142448 512-byte hdwr sectors (320073 MB)
[  253.870189] sdc: Write Protect is off
[  253.870192] sdc: Mode Sense: 00 3a 00 00
[  253.870372] SCSI device sdc: drive cache: write back
[  253.870378]  sdc: sdc1
[  253.887285] sd 2:0:0:0: Attached scsi disk sdc

Thanks,
-jim

[PATCH] sata_nv: SRST sometimes fails after hotplug, use HRST_TO_RESUME

[Tejun Heo]

NV controllers sometimes fail to perform softreset after hotplug.
Make it use hardreset to resume link.

Signed-off-by: Tejun Heo <htejun@gmail.com>

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 8cd730f..647e7b8 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -257,7 +257,8 @@ static struct ata_port_info nv_port_info
 	/* generic */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
@@ -266,7 +267,8 @@ static struct ata_port_info nv_port_info
 	/* nforce2/3 */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
@@ -275,7 +277,8 @@ static struct ata_port_info nv_port_info
 	/* ck804 */
 	{
 		.sht		= &nv_sht,
-		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
+		.flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_HRST_TO_RESUME,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,

Re: [PATCH] sata_nv: SRST sometimes fails after hotplug, use HRST_TO_RESUME

[Jeff Garzik]

Tejun Heo wrote:
> NV controllers sometimes fail to perform softreset after hotplug.
> Make it use hardreset to resume link.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

applied

Re: [PATCH] libata: lengthen interval between SRST set and clear

[justin]

Dear Tejun,

> Justing, the solution doesn't seem to work.  Have you guys verified
> that the controller doesn't send the second H2D regsiter FIS clearing
> SRST w/ 20us delay?  If you guys did, the patch needs to make into
> tree whether it helps this particular case or not.  If not, I think
> it's best to leave it alone.

Sure , we did it.

the hotplug problem happened on some sata enclosure
but It sounds like for particular case.

Thanks.