machine/wd_fdc.cpp: lower drq at the end of track write

[mgarlanger]

When testing formatting disks with the new MMS controller in #12413, the formatting would fail after writing the first track. The code was checking the status byte, and expecting S1 Data Request to be 0. By adding this drop_drq() at the end of the command, it allowed the formatting to complete successfully.

[Robbbert]

This change affects dozens of other systems. Have you done testing of them?

[mgarlanger]

This change affects dozens of other systems. Have you done testing of them?

In addition to the MMS controller on the H89 with MMS's version of CP/M, I tested with the Z37 controller on the H89 along with Heath's version of CP/M. I can setup some other systems to test with.

[cuavas]

Given the amount of scrutiny this device has come under already, I’d be very reluctant to merge this without a much better test plan than this. You’ve only tested with one system. You still seem to be thinking of things in terms of the path of least resistance to making the Heath/Zenith systems “usable”. As I’ve said plenty of times, this isn’t good enough when you’re working on common devices, particularly devices used by numerous systems that abuse corner cases (very much the case when it comes to floppy copy protection schemes).

We’ve already had too many regressions from myopic changes to the common floppy controller devices intended to make one thing work without considering the bigger picture. Ideally, I’d like to see empirical evidence that actual chips work like this, either captured using a logic analyser or from analysis of die photographs.

[mgarlanger]

So far I've been having problems tracking down the boot/utility disks I need to test formatting on these other systems. I don't have a logic analyzer but I have a friend that may be able to test the chip behavior on a real system. I'll update you if we get any results.

This really wasn't the path of least resistance. I truly think this is an error in the implementation of the WD FDC and felt it should be fixed there. The FDC chip is well designed, and it makes no sense why they would keep the DRQ line high once INTRQ is triggered to signify the end of the command. Why would they request the next data byte, when they are only going to toss it. Seeing how much of the TRACK_DONE code hasn't been touch in at least 12 years, it seemed more likely this corner case was not considered when the code was written, than this being an intentional design decision to leave DRQ set.

The alternative fix I found, was to switch the order of checks in the if checks in mms_intr_cntrl::get_instruction to first check for drq, but that seemed like a kludge to me. Even though that approach would have been trivial to get merged, I felt it would be better to try to get fix into the common code and face the expected pushback, than to just work around what I think is a bug in the implementation.

[durgadas311]

I ran a test on a (all real hardware) Kaypro 2X (1793 FDC). The test starts a WRITE TRACK command and sends data until BUSY goes off, then prints the status register contents from the input that showed BUSY off. In all runs, single or double density, the DRQ bit was off when BUSY turned off.

[rb6502]

I'm going to get a lot louder about asking people who are submitting computers to have at least a minimal software list to test them in the future because of situations like this. (And to add usage information to wiki.mamedev.org). That would also make life easier for Bob Zed to show something interesting in his videos.

Thanks for the on-hardware test, @durgadas311! That's the behavior I would expect - other FDC and SCSI controllers with DMA drop DRQ when asserting IRQ at the end of a transfer as well.

[rb6502]

In addition to the on-hardware test that was reported, I was able to format disks successfully in MAME with the change on the Vista A800 DMA 8" floppy controller card for the Apple II (which uses a WD1797). This change only affects formatting disks, not normal read/write operations, so it's pretty safe to begin with, but here we go.