Subject: Re: kern/25659
To: None <bouyer@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: netbsd-bugs
Date: 06/17/2005 21:26:02
The following reply was made to PR kern/25659; it has been noted by GNATS.

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: John Refling <refling@stanfordalumni.org>
Cc: smb@research.att.com, gnats-bugs@NetBSD.org,
	kern-bug-people@NetBSD.org, gnats-admin@NetBSD.org,
	netbsd-bugs@NetBSD.org
Subject: Re: kern/25659
Date: Fri, 17 Jun 2005 23:24:47 +0200

 --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--