1581: PHP without PLP resulting in problems

Hello,

some of you might know the code problem of the 1581 described here:

http://forum.6502.org/viewtopic.php?f=4&t=5714

In short: In the function wdstatus, there is a PHP, but in one code
path, the value is never pulled back from the stack, resulting in an
error by the following RTS.

The code is as follows:


getwdstat

        jsr  wdunbusy   ; wait for unbusy first

;

wdstatus

        php
        WDTEST
        lda  WDSTAT     ; get status
        lsr  a
        lsr  a
        lsr  a
        bcs  1$

        and  #bit3+bit1+bit0
        tax
        plp             ; restore carry
        lda  wdtrans,x
        .byte skip2
1$      lda  #9
        sta  controller_stat
        lda  controller_stat
        rts

wdtrans .byte 0,5,2,0,0,0,0,0,8


You see, if the "bcs 1$" is taken, then the PLP is skipped.


What does this function do?

It takes the WD177x status code and maps it to the error codes that
the GCR drives usually use.

First, it checks bit 2 of the status register (with the BCS). If it is
set (that is, "data lost" because the data register is read or written
too slow), it results in the error code 9 ("data block too long").

Otherwise, it tests bits 3, 4 and 6 and maps it as follows:

Bit          Index   Job-Error-Code
3 - 4 - 6
0   0   0      0     0 - here: o.k.
1   0   0      1     5 - Data CRC is wrong
0   1   0      2     2 - Block header not foud
1   1   0      3     0 - here: o.k.
0   0   1      8     8 - Diskette write protected

The meanings of the bits of the status register of the WD177x are:

Bit 2: Lost data/byte
Bit 3: CRC error
Bit 4: Record not found
Bit 5: Record type / spin-up: Either the motor runs (type 1 command), or the data block is deleted (type 2 commands)
Bit 6: Write protect

The zero between index 4 and 8 skip the masked out values for bit 6.


In a GCR drive, the job error code of "1" is used for "o.k". In the
1581, is used the "0" because it eases the test for an error (BNE/BEQ).

I do not know the WD177x enough, and I did not find it in the data
sheet: Does it guarantee that there are no two errors are once?

For example, can bit 3 and 4 be set simultaneously? In this case, the
routine above will return a "o.k." instead of an error!

Or bit 6 and either 3 or 4? In this case, the routine above will read
some "garbage" that is behind the table above...



Nevertheless, I wanted to ask about the PHP/PLP.

First of all, why doesn't the 1581 crash in most cases? Having bit 2 set
means that the CPU was too slow to keep up with the WD177x. It might be
that the code is always fast enough, and this problem never happens.
However, I did not check each and every location.


The comment for the PLP tells us that it wants to restore the carry.
This makes sense, because N and Z are overwritten afterwards, and there
is no CLI or SEI in this routine.

But: Why would it do this? I have looked into the calls to wdstatus (do
not exist!) and to getwdstat in the sources. I did not find any place
where the carry does matter!

Am I missing something? Is it ok to replace the PHP and the PLP with
NOPs? This will change the timing, though.

Otherwise, I thought about moving the PLP just before the RTS, so it
cannot be skipped.


What do you think?

Regards,
Spiro

-- 
Spiro R. Trikaliotis
https://spiro.trikaliotis.net/