Date: Fri, 30 Jan 1998 01:55:58 +1100 From: Bruce Evans <bde@zeta.org.au> To: bde@zeta.org.au, mike@smith.net.au Cc: cvs-committers@FreeBSD.ORG, msmith@FreeBSD.ORG Subject: Re: cvs commit: src/sys/i386/isa atapi.c Message-ID: <199801291455.BAA00445@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
>> - 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<units) for drive to respond to selection, or poll
>> > (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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199801291455.BAA00445>