Subject: Re: kern/25659
To: John Refling <refling@stanfordalumni.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: netbsd-bugs
Date: 06/17/2005 23:24:47
--6TrnltStXW4iwmi0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Jun 17, 2005 at 01:00:03AM -0700, John Refling wrote:
> 
> Alright, I have experimented with some of the patches
> in the log for this PR, and have found the following:
> 
> The first submission which was to patch ver 1.172 did
> not patch cleanly (I'm running 2.0, version 1.172.2.7)
> but with a tiny bit of work (the "int bcnt"; was inserted
> a few lines off), compiled, and solved the problem attaching
> the disk drive to wd1.
> 
> The second submission which was to patch 1.172.2.7
> also did not apply cleanly, (the "delay(1000000);" and
> the following line were swapped), which when changed,
> compiled, and also solved the disk problem.
> 
> The third submission which was to patch 1.172.2.7, which
> gave compiler warnings about uninitialized variables (st0,
> st1 on line 495), and after making it compile (st1 = 0,
> etc.), it did NOT solve the problem.
> 
> The forth submission was for 3.0 which I don't have
> checked out, so can't test it, yet.
> 
> I looked at the 2nd patch, and it seemed to be just
> many delays(), so I was curious to see if there was one
> delay that made all the difference, and in my case, there
> was!
> 
> The only statement that mattered was the "delay(1000000);"
> 
> I'm testing this with three old disk drives:
> 
> IBM DADA-24860 4860 MB (1998)  needs delay(200000) or greater
> Toshiba MK-2428 500 MB (1994?) needs delay(450000) or greater
> Apple DVAA-2810 733 MB (1995)  needs delay(380000) or greater

Ha this is interesting !

> 
> The above shows the range of delays needed from my
> tests.  I suppose a very old drive might need a larger
> delay.  I can test that sometime, when I find one.
> 
> Is this really an issue with the wdc.c controller, or
> is it possibly slow pcmcia hardware/driver between the
> wdc controller and the system?

I suspect hardware here. The PCMCIA socket is powered up just before
calling wdcattach(). The power up function waits for the PCMCIA chip to
assert that it is ready, but the pcmcia chip probably doesn't wait
for the drive to be ready.
If so, it should be enouth to add the delay in the wdc_pcmcia.c code.
From your tests, half a second should be enouth. Can you test the attached
patches (one for 2.0, one for current) ?

> 
> Also, what's going to happen when an old cd-rom is
> attached?  Aren't they slower than disk drives?

I don't think so. It's probably not a spin-up problem, it's the electronic
that is not ready when we start probing. Once the electronic is ready,
the probe code can wait for the device to spin up if needed.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

--6TrnltStXW4iwmi0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff20

Index: wdc_pcmcia.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pcmcia/wdc_pcmcia.c,v
retrieving revision 1.67
diff -u -r1.67 wdc_pcmcia.c
--- wdc_pcmcia.c	3 Jan 2004 22:56:53 -0000	1.67
+++ wdc_pcmcia.c	17 Jun 2005 21:18:19 -0000
@@ -41,8 +41,10 @@
 
 #include <sys/param.h>
 #include <sys/device.h>
+#include <sys/kernel.h>
 #include <sys/malloc.h>
 #include <sys/systm.h>
+#include <sys/proc.h>
 
 #include <machine/bus.h>
 #include <machine/intr.h>
@@ -379,6 +381,13 @@
 	sc->sc_wdcdev.sc_atapi_adapter._generic.adapt_enable =
 	    wdc_pcmcia_enable;
 
+        /*
+	 * Some devices needs some more delay after power up to stabilize
+	 * and probe properly, so give them half a second.
+	 * See PR 25659 for details.
+	 */
+	tsleep(wdc_pcmcia_attach, PWAIT, "wdcattach", hz / 2);
+
 	sc->sc_flags |= WDC_PCMCIA_ATTACH;
 	wdcattach(&sc->wdc_channel);
 

--6TrnltStXW4iwmi0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diffcurrent

Index: wdc_pcmcia.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pcmcia/wdc_pcmcia.c,v
retrieving revision 1.99
diff -u -r1.99 wdc_pcmcia.c
--- wdc_pcmcia.c	11 Mar 2005 16:17:57 -0000	1.99
+++ wdc_pcmcia.c	17 Jun 2005 21:18:04 -0000
@@ -41,8 +41,10 @@
 
 #include <sys/param.h>
 #include <sys/device.h>
+#include <sys/kernel.h>
 #include <sys/malloc.h>
 #include <sys/systm.h>
+#include <sys/proc.h>
 
 #include <machine/bus.h>
 #include <machine/intr.h>
@@ -300,6 +302,13 @@
 	    wdc_pcmcia_enable;
 	sc->sc_wdcdev.sc_atac.atac_atapi_adapter._generic.adapt_refcnt = 1;
 
+	/*
+	 * Some devices needs some more delay after power up to stabilize
+	 * and probe properly, so give them half a second.
+	 * See PR 25659 for details.
+	 */
+	tsleep(wdc_pcmcia_attach, PWAIT, "wdcattach", hz / 2);
+
 	wdcattach(&sc->ata_channel);
 	ata_delref(&sc->ata_channel);
 	sc->sc_state = WDC_PCMCIA_ATTACHED;

--6TrnltStXW4iwmi0--