Subject: Re: kern/29600: cbb(4) hangs on boot
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: FUKAUMI Naoki <fun@naobsd.org>
List: netbsd-bugs
Date: 03/12/2005 08:14:01
The following reply was made to PR kern/29600; it has been noted by GNATS.

From: FUKAUMI Naoki <fun@naobsd.org>
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/29600: cbb(4) hangs on boot
Date: Sat, 12 Mar 2005 17:13:48 +0900

 At Sat, 12 Mar 2005 16:55:46 +0900,
 FUKAUMI Naoki wrote:
 > > >  		microtime(&before);
 > > >  		s = splbio();
 > > > -		while (pwrcycle == sc->sc_pwrcycle)
 > > > -			tsleep(&sc->sc_pwrcycle, PWAIT, "pccpwr", 0);
 > > > +		/* XXX: Set timeout to 200ms because power cycle event will be
 > > > +		   never happened when attaching 16-bit card. */
 > > > +		error = tsleep(&sc->sc_pwrcycle, PWAIT, "pccpwr", hz/5);
 > > >  		splx(s);
 > > 
 > > is there any reason not to check sc_pwrcycle?
 > 
 > We don't have any reason to wait value is changed in while() loop.
 > 
 > tsleep() will be awakened if power cycle event occured. There is no
 > reason to check sc_pwrcycle value.
 
 Current code do
   sleep forever by while()
   sleep forever by tsleep()
 in pccbb_power(),
 
 and do
   break forever sleep by sc->sc_pwrcycle++
   break forever sleep by wake()
 in pccbbintr().
 
 I think these are redundant.
 
 > We should not wait (forever!) because wake() is never occured in some
 > situation. (e.g. Ricoh controller & 16-bit card)
 > 
 > It doesn't need to do "sc->sc_pwrcycle++;" in pccbbintr(), too.
 > I forgot to remove this line in my patch, sorry.
 > 
 > Maybe, we can use sc or another variable/function pointer instead of
 > &sc->sc_pwrcycle as tsleep() identifier. And we can remove sc_pwrcycle
 > in struct pccbb_softc.
 
 Regards,
 
 --
 FUKAUMI Naoki