Hello,

on Tue, 9 Aug 1994 03:09:24 +0200 Chris Torek wrote 
concerning "Re: port-sparc/403: Sparc panics when a bad disk block is 
encountered " something like this:

> Actually, your fix is insufficient.  

Hmmm, yes, it's not completly correct. I've tried it with a second disk and
of course espicmd() may not start a new transfer, when hba_busy was still
set. I'm including a new patch, that fixes the problem correctly.

> espicmd() can clobber an ongoing
> command if another drive is doing a transfer.

I'm not sure, if I understand what you mean. I've tried the new patch with
two hard disks (and one controller, I don't have a second one). It works
just fine. (What I did was to keep one disk busy while I'm reading the bad
sector on the other disk.) How can espicmd() clobber an ongoing transfer?
The controller is still flagged busy and the first transfer is already over.

> The entire SCSI design needs to be re-done, but we (BSDI) are holding
> off until we change the rest of the I/O system to match.

I agree, that the SCSI design should be redone (and someone of the NetBSD
core team is working on that). But until then, I think the old code should 
be fixed as much as possible (especially if it's going to be released ;)).

Ok, here is the new fix (I guess I don't need to send another bug report,
do I?):

*** esp.c.orig	Mon Aug  8 19:29:17 1994
--- esp.c	Fri Aug 12 17:16:26 1994
***************
*** 1036,1041 ****
--- 1036,1042 ----
  	register volatile struct dmareg *dma = sc->sc_dma;
  	register int r, s, wait;
  	register struct sq *sq;
+ 	register char old_busy;
  
  	/*
  	 * Wait for any ongoing operation to complete.
***************
*** 1045,1050 ****
--- 1046,1052 ----
  		sc->sc_iwant = 1;
  		tsleep((caddr_t)&sc->sc_iwant, PRIBIO, "espicmd", 0);
  	}
+ 	old_busy = sc->sc_hba.hba_busy;
  	sc->sc_hba.hba_busy = 1;
  	splx(s);
  
***************
*** 1110,1123 ****
  done:
  	sc->sc_state = S_IDLE;
  	s = splbio();
! 	if (sc->sc_iwant) {
! 		sc->sc_iwant = 0;
! 		wakeup((caddr_t)&sc->sc_iwant);
! 	} else if ((sq = sc->sc_hba.hba_head) != NULL) {
! 		sc->sc_hba.hba_head = sq->sq_forw;
! 		(*sq->sq_dgo)(sq->sq_dev, &sc->sc_cdbspace);
! 	} else
! 		sc->sc_hba.hba_busy = 0;
  	splx(s);
  	return (r);
  }
--- 1112,1127 ----
  done:
  	sc->sc_state = S_IDLE;
  	s = splbio();
! 	if (!old_busy) {
! 		if (sc->sc_iwant) {
! 			sc->sc_iwant = 0;
! 			wakeup((caddr_t)&sc->sc_iwant);
! 		} else if ((sq = sc->sc_hba.hba_head) != NULL) {
! 			sc->sc_hba.hba_head = sq->sq_forw;
! 			(*sq->sq_dgo)(sq->sq_dev, &sc->sc_cdbspace);
! 		} else
! 			sc->sc_hba.hba_busy = 0;
! 	}
  	splx(s);
  	return (r);
  }


Bye, Rolf

P.S: Why don't you use esprel() here and at the end of espintr() to 
     do the command/busy handling ? This would save some duplicate code.

------------------------------------------------------------------------------