From owner-cvs-all Thu Jan 29 07:00:46 1998 Return-Path: Received: (from majordom@localhost) by hub.freebsd.org (8.8.8/8.8.8) id HAA19048 for cvs-all-outgoing; Thu, 29 Jan 1998 07:00:46 -0800 (PST) (envelope-from owner-cvs-all@FreeBSD.ORG) Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.19]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id HAA18983; Thu, 29 Jan 1998 07:00:28 -0800 (PST) (envelope-from bde@godzilla.zeta.org.au) Received: (from bde@localhost) by godzilla.zeta.org.au (8.8.7/8.8.7) id BAA00445; Fri, 30 Jan 1998 01:55:58 +1100 Date: Fri, 30 Jan 1998 01:55:58 +1100 From: Bruce Evans Message-Id: <199801291455.BAA00445@godzilla.zeta.org.au> To: bde@zeta.org.au, mike@smith.net.au Subject: Re: cvs commit: src/sys/i386/isa atapi.c Cc: cvs-committers@FreeBSD.ORG, msmith@FreeBSD.ORG Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-To-Unsubscribe: mail to majordomo@FreeBSD.org "unsubscribe cvs-all" >> - The PC98 code waits for DRQ to be deasserted between the above 2 outb's >> this may be to hide the bug related to stale DRQs in atapi_wait() (if >> BSY and DRQ are both stale (and wrong), then atapi_wait() returns >> early). > >If this is the case, then the PC98 define is misplaced. I am not happy >with the amount of PC98 crap blasted through various drivers; it tends >to chop the flow up *extremely* badly. It's better than duplicating all the drivers. The ones that are too different are already duplicated. >> > /* Check that device is present. */ >> > if (inb (port + AR_STATUS) == 0xff) { >> >> - Bogus. The device had better be there since we have written a lot to >> its registers. atapi_wait() has just succeeded, so its status was >> not 0xff when it was read before the command was issued (ARS_BSY was >> clear). The value just read is invalid since we didn't wait 400 nsec. > >Not necessarily bogus, because if you assume the first drive selection >may potentially fail and the second may be the first honoured, then we >have not yet checked for a device responding. We should check for a >response to selection before issuing a command, however writing a >command to a drive that's not present is relatively harmless. Bogus. In fact, more bogus than I first thought :-). It is possible for the status to be 0xff although atapi_wait() has just "succeeded", because of bugs in atapi_wait() and non-checking of the error bit returned by atapi_wait(). Not checking the error bit is probably correct - see a comment in wdwait(), but since we don't check it we only know that atapi_wait() didn't time out. Assuming that the drive select fails, then the failure mode may be: outb(/* drive select stuff */...); atapi_wait(...); /* Neglect the necessary delay of 400 nsec. */ garbage_status = inb(...); /* garbage_status happens to have ARS_BSY clear. */ return (garbage); /* The inb() normally provides a delay of 400 nsec, so we * can now get a non-garbage status. We don't trust atapi_wait() * and want to supply a delay as a side effect to confuse ourself * about how this driver works, so check the status again. */ if (inb(...) == 0xff) abort(); The magic 0xff here isn't good. Checking ARS_BSY might work better. However, if the master drive is normal IDE, then the status for a nonexistent slave is supposed to be 0, not 0xff. You could detect this by checking DRDY. OTOH, DRQ won't be falsely detected if the status is 0. I still don't see how the status can be 0xff without the false DRQ being detected. If all bits are 1, then the ARS_CHECK bit is set, so the failure of the ATAPIC_IDENTIFY command is detected. >> >## Zip is going off the bus at this point, but not gone. >> >## Read returns 0xa1. >> >> I see, you think 0xa1 is an echo of the value just read. > >I don't actually know *what* the 0xa1 value is. I do know that a UTSL. 0xa1 is ATAPIC_IDENTFY, which was just written to the command register. This is probably not a coincidence. >subsequent call to atapi_wait(, DRQ) returns as though DRQ had been >validly asserted, but that after the return there is nothing actually >selected. This is consistent with the status being 0xff for a nonexistent drive (or non-atapi drive?) except on the first inb() in atapi_wait() which may see bus noise. >Yes. A complete reimplementation of atapi_probe() could bring it >within spec but would lose the implicit (and unrecoverable) bug/feature >support that the current version has. It seems to have only bugfeature support for its own software bugs. >> The bogus inb() probably helps because it delays for >= 400 nsec. What >> does the next inb() return? > >Which next one? ie. a subsequent read from the status port? Yes. >> Did it actually read as 0xff? This is possible, since BSY is not checked >> for. > >Obviously it did. Why else would I have bothered with the change? > >> >> Perhaps the problem is earlier. According to an old draft version of >> >> the ATA spec, the timing for issueing a polled-mode input command is: >> >> >0.0 Select the target >> >0.5 Wait XXX> > (some register) to indicate drive is selected. >> >> I think it's actually the polling of DRQ without checking BSY (step 9 >> or 10), which is most likely to cause problems if the drive is smart >> enough to set DRQ 700 nsec in advance of becoming ready, to give drivers >> advance notice. > >I think you are still under the mistaken assumption that we are trying >to talk to the Zip here. The whole point of the change was to deal >with the case where we aren't. Oops, yes, I thought your changes had the side effect of getting the Zip probed. Bruce