Slot Floppies Patch

  • The one that started it all. Get ready to slot floppies! Perfect for the Rhodie-at-heart. 3D lettered, PVC rubber. 3'x2' Shipped First Class with tracking.
  • R/weekendgunnit: No linking to other subs, or from other subs to here, np or otherwise, or you will be banned. All link posts must contain some form.
ID Category [?] Severity [?] Reproducibility Date Submitted Last Update
06640 Core Major Always Aug 1, 2017, 21:38 Aug 4, 2017, 06:43
Tester MrGW View Status Public Platform MAME (Official Binary)
Assigned To Duke Resolution Fixed OS Windows 10 (32-bit)
Status [?]ResolvedDrivercoco3.cpp
Version 0.188 Fixed in Version 0.189 Build Normal
Summary 06640: coco3, coco3h: TRS-80 Color Computer FDC data corruption issue
Description David Ladd (davidwladd@gmail.com) has discovered a serious data corruption issue when using the FDC with the COCOx drivers in MAME. He originally submitted MAME bug #6639 as part of an earlier issue he saw when running OS9 and accessing /d0 and /d1 drives (under certain circumstances).
http://mametesters.org/view.php?id=6639
It looks like this latest discovery is what was causing the earlier issue.
In any case, MAME 0.162 was the last version known to work correctly with the FDC. Something changed in MAME 0.163 to 'break' the FDC and it's been that way ever since.
Steps To Reproduce Here's a simple test that will demonstrate the issue:
Startup CoCo 2 or CoCo3 emulation
Create a new disk image in drive 0
Create a new disk image in drive 1
DSKINI 0
DSKINI 1
Enter a quick basic program like this and save it to disk:
10 PRINT 'DIR 0'
20 DIR 0
30 PRINT 'DIR 1'
40 DIR 1
50 GOTO 10
SAVE'TEST1.BAS'
Then all you have to do to see the problem is to:
DIR 0:DIR 1
You will see TEST1.BAS show up twice. Which it should not.
The issue is also reproducible using the methods detailed in the videos below:

Additional Information Credit goes to David Ladd for finding this issue (twice). It was never resolved in the original MAME bug report:
http://mametesters.org/view.php?id=6639
Flags
Regression Version
Affected Sets / Systems coco3, coco3h
Attached Files
test_disks.zip (799 bytes) Aug 1, 2017, 23:53 Uploaded by Tafoid
Test Disk Images - disk1 has program, disk2 is empty - both are formatted
coco.diff (683 bytes) Aug 2, 2017, 07:50 Uploaded by Osso
[Show Content]
related to06639ResolvedDuke coco3, coco3h: crash of emulated system accessing multiple floppy drives
Slot floppies patcher

A derogatory term for an insurgent fighter who trended to be black. Originated with Rhodesian military personnel in Rhodesia (currently Zimbabwe) during the Rhodesian Bush War. Floppy disk sales have, well, flopped but there are still masses of PCs and old embedded PC-based systems out there with floppy disk slots and drives. Now this near-dead space can be made usable again, with a 32GB FLOPPYFlash drive from Solid State Disks Ltd. It's a drop-in replacement for a floppy disk drive and takes.

Notes

Slot Floppies Patch 3

Bletch
Aug 1, 2017, 22:11
From the MAME 0.163 release notes:
- update all drivers to use the new wd floppy controller implementation and remove the legacy one.
I'm guessing that this is more than a coincidence.
Tafoid
Aug 1, 2017, 23:52
Added disk1 and disk2 images to assist in testing.
I've noticed that it doesn't always file the first time, but usually fails the 'DIR 0:DIR 1' test the 2nd time
star2root
Aug 2, 2017, 02:03
I think it might have to with these changes…
85 src/emu/bus/coco/coco_fdc.c
@@ -291,16 +298,17 @@ void coco_fdc_device::dskreg_w(UINT8 data)
else if (data & 0x40)
drive = 3;
- legacy_floppy_image_device *floppy[4];
+ floppy_image_device *floppy[4];
- floppy[0] = subdevice<legacy_floppy_image_device>(FLOPPY_0);
- floppy[1] = subdevice<legacy_floppy_image_device>(FLOPPY_1);
- floppy[2] = subdevice<legacy_floppy_image_device>(FLOPPY_2);
- floppy[3] = subdevice<legacy_floppy_image_device>(FLOPPY_3);
+ floppy[0] = subdevice<floppy_connector>(WD_TAG ':0')->get_device();
+ floppy[1] = subdevice<floppy_connector>(WD_TAG ':1')->get_device();
+ floppy[2] = subdevice<floppy_connector>(WD_TAG ':2')->get_device();
+ floppy[3] = subdevice<floppy_connector>(WD_TAG ':3')->get_device();
for (int i = 0; i < 4; i++)
{
- floppy[i]->floppy_mon_w(i drive ? CLEAR_LINE : ASSERT_LINE);
+ if (floppy[i])
+ floppy[i]->mon_w(i drive ? CLEAR_LINE : ASSERT_LINE);
}
head = ((data & 0x40) && (drive != 3)) ? 1 : 0;
@@ -309,8 +317,11 @@ void coco_fdc_device::dskreg_w(UINT8 data)
update_lines();
- m_wd17xx->set_drive(drive);
- m_wd17xx->set_side(head);
+ m_wd17xx->set_floppy(floppy[drive]);
+
+ if (floppy[drive])
+ floppy[drive]->ss_w(head);
+
m_wd17xx->dden_w(!BIT(m_dskreg, 5));
}
@@ -379,7 +390,7 @@ WRITE8_MEMBER(coco_fdc_device::write)
dskreg_w(data);
break;
case 8:
- m_wd17xx->command_w(space, 0, data);
+ m_wd17xx->cmd_w(space, 0, data);
break;
case 9:
m_wd17xx->track_w(space, 0, data);
@@ -388,6 +399,7 @@ WRITE8_MEMBER(coco_fdc_device::write)
m_wd17xx->sector_w(space, 0, data);
break;
case 11:
+ //printf('data w %02xn', data);
m_wd17xx->data_w(space, 0, data);
break;
};
@@ -413,10 +425,12 @@ WRITE8_MEMBER(coco_fdc_device::write)
//**************************************************************************
static MACHINE_CONFIG_FRAGMENT(dragon_fdc)
- MCFG_DEVICE_ADD(WD2797_TAG, WD2797, 0)
- MCFG_WD17XX_DEFAULT_DRIVE4_TAGS
+ MCFG_WD2797_ADD(WD2797_TAG, XTAL_1MHz)
- MCFG_LEGACY_FLOPPY_4_DRIVES_ADD(coco_floppy_interface)
+ MCFG_FLOPPY_DRIVE_ADD(WD2797_TAG ':0', coco_fdc_floppies, 'qd', coco_fdc_device::floppy_formats)
+ MCFG_FLOPPY_DRIVE_ADD(WD2797_TAG ':1', coco_fdc_floppies, 'qd', coco_fdc_device::floppy_formats)
+ MCFG_FLOPPY_DRIVE_ADD(WD2797_TAG ':2', coco_fdc_floppies, ', coco_fdc_device::floppy_formats)
+ MCFG_FLOPPY_DRIVE_ADD(WD2797_TAG ':3', coco_fdc_floppies, ', coco_fdc_device::floppy_formats)
MACHINE_CONFIG_END
@@ -511,10 +525,20 @@ void dragon_fdc_device::dskreg_w(UINT8 data)
data);
}
- if (data & 0x04)
- m_wd2797->set_drive(data & 0x03);
+ floppy_image_device *floppy = NULL;
+
+ switch (data & 0x03)
+ {
+ case 0: floppy = subdevice<floppy_connector>(WD2797_TAG ':0')->get_device(); break;
+ case 1: floppy = subdevice<floppy_connector>(WD2797_TAG ':1')->get_device(); break;
+ case 2: floppy = subdevice<floppy_connector>(WD2797_TAG ':2')->get_device(); break;
+ case 3: floppy = subdevice<floppy_connector>(WD2797_TAG ':3')->get_device(); break;
+ }
+
+ m_wd2797->set_floppy(floppy);
m_wd2797->dden_w(BIT(data, 3));
+
m_dskreg = data;
}
@@ -556,12 +580,7 @@ WRITE8_MEMBER(dragon_fdc_device::write)
switch(offset & 0xEF)
{
case 0:
- m_wd2797->command_w(space, 0, data);
-
- /* disk head is encoded in the command byte */
- /* Only for type 3 & 4 commands */
- if (data & 0x80)
- m_wd2797->set_side((data & 0x02) ? 1 : 0);
+ m_wd2797->cmd_w(space, 0, data);
break;
case 1:
m_wd2797->track_w(space, 0, data);
Osso
Aug 2, 2017, 07:52
Attached a small patch which seems to work for this case, but may actually cause horrible breakage elsewhere since I know nothing about the coco hardware and haven't tested anything else.
None the less, it might be useful as a hint of what's going wrong.
Haze
Aug 2, 2017, 13:33
the original code is definitely suspicious, it looks like it will turn off the motor for every drive other than the one you're trying to access.
so in that sense, I'd say your fix looks sane even if I know nothing about the system
lostwizard
Aug 2, 2017, 19:14
In case it helps, the correct drive motor behaviour on the Coco FDC is:
If the motor enable bit (bit 3 of FF40) is enabled, *all* drive motors should start and *stay* on until the motor enable bit is cleared. That is, the drive select lines have no effect on whether a drive is spinning or not. All are or none are.
Edit: Also, if the drive motors are turned off (bit 3 of FF40 clear), the drive selects still operate and 'enable' the specified drive(s). The drive light actually comes on with the motors turned off on the real hardware in that case.
However, I suspect another issue in addition to the drive motor behaviour since the drive not being spun up properly should result in a read failure (CRC fail leading to I/O error or obviously garbled data) rather than the described behaviour.
No.14058
Tester
Osso,
I just tested the patch you supplied. It doesn't fully address the issue.
You can also do a DIR 1:DIR 0 and then the directory on drive 0 doesn't show up correctly.
Also you can have up to 4 single side floppy drives or 3 double sided floppy drives connected to a real CoCo.
No.14059
Tester
Ciaran Anscomb (from the Xroar project) provided this comment and patch to try:
'They've come up with a patch, but that fails in a different way.
Suggest the attached: update motor state on *all* drives, not just
selected.'
diff --git a/src/devices/bus/coco/coco_fdc.cpp b/src/devices/bus/coco/coco_fdc.cpp
index f56bc047c5..cf9bd3038c 100644
--- a/src/devices/bus/coco/coco_fdc.cpp
+++ b/src/devices/bus/coco/coco_fdc.cpp
@@ -277,7 +277,7 @@ void coco_fdc_device_base::dskreg_w(uint8_t data)
{
floppy_image_device *floppy = m_floppies[i]->get_device();
if (floppy)
- floppy->mon_w(((i drive) && (data & 0x08)) ? CLEAR_LINE : ASSERT_LINE);
+ floppy->mon_w((data & 0x08) ? CLEAR_LINE : ASSERT_LINE);
}
head = ((data & 0x40) && (drive != 3)) ? 1 : 0;
No.14060
Developer
Thanks lostwizard, just committed a patch according to your explanation. It seems to fully address the issue here. Would be good if someone could try 06639 now.
No.14063
Tester
Just so everyone knows that Ciaran Anscomb patch seems to have fixed the Disk Basic & OS-9/NitrOS-9 problems I ran into and was doing random reading & writing to both drives for a few minutes and did not notice any corruptions at this time. Before if accessing both floppy drives at the same time would crash OS-9 before within a few seconds.
The DIFF code Ciaran and supplied to us is as follows:
diff --git a/src/devices/bus/coco/coco_fdc.cpp b/src/devices/bus/coco/coco_fdc.cpp
index f56bc047c5..cf9bd3038c 100644
--- a/src/devices/bus/coco/coco_fdc.cpp
+++ b/src/devices/bus/coco/coco_fdc.cpp
@@ -277,7 +277,7 @@ void coco_fdc_device_base::dskreg_w(uint8_t data)
{
floppy_image_device *floppy = m_floppies[i]->get_device();
if (floppy)
- floppy->mon_w(((i drive) && (data & 0x08)) ? CLEAR_LINE : ASSERT_LINE);
+ floppy->mon_w((data & 0x08) ? CLEAR_LINE : ASSERT_LINE);
}
head = ((data & 0x40) && (drive != 3)) ? 1 : 0;
No.14064
Tester
edited on: Aug 3, 2017, 02:16
While we are in this file, I would like suggest a patch that addresses another minor issue. Currently the CoCo's available floppy setting are either none, or quad density. Most CoCo's had either SSDD or DSDD 5 1/4 drives. This patch below set the default number of drives to 3, and adds the ability to select SSDD, DSDD, or QD (quad density) drives as an option. It also includes the patch above to fix the data corruption issue. Ileft the default at QD since due to another existing bug I have not been able to fix yet, you can't change the drive setting until you select 'reset' from the slot devices menu and QD drives do work with almost all existing disk images.
--- src/devices/bus/coco/coco_fdc.cpp 2017-08-02 01:36:40.000000000 -0400
+++ src/devices/bus/coco/coco_fdc.cpp 2017-08-02 22:13:09.000000000 -0400
@@ -118,6 +118,8 @@
FLOPPY_FORMATS_END
static SLOT_INTERFACE_START( coco_fdc_floppies )
+ SLOT_INTERFACE('ssdd', FLOPPY_525_SSDD)
+ SLOT_INTERFACE('dsdd', FLOPPY_525_DD)
SLOT_INTERFACE('qd', FLOPPY_525_QD)
SLOT_INTERFACE_END
@@ -130,7 +132,7 @@
MCFG_FLOPPY_DRIVE_SOUND(true)
MCFG_FLOPPY_DRIVE_ADD(WD_TAG ':1', coco_fdc_floppies, 'qd', coco_fdc_device_base::floppy_formats)
MCFG_FLOPPY_DRIVE_SOUND(true)
- MCFG_FLOPPY_DRIVE_ADD(WD_TAG ':2', coco_fdc_floppies, nullptr, coco_fdc_device_base::floppy_formats)
+ MCFG_FLOPPY_DRIVE_ADD(WD_TAG ':2', coco_fdc_floppies, 'qd', coco_fdc_device_base::floppy_formats)
MCFG_FLOPPY_DRIVE_SOUND(true)
MCFG_FLOPPY_DRIVE_ADD(WD_TAG ':3', coco_fdc_floppies, nullptr, coco_fdc_device_base::floppy_formats)
MCFG_FLOPPY_DRIVE_SOUND(true)
@@ -273,25 +275,32 @@
else if (data & 0x40)
drive = 3;
+ // Enable/disable the motor line for all drives since it is common
+ // to all drives.
for (int i = 0; i < 4; i++)
{
floppy_image_device *floppy = m_floppies[i]->get_device();
if (floppy)
- floppy->mon_w(((i drive) && (data & 0x08)) ? CLEAR_LINE : ASSERT_LINE);
+ floppy->mon_w((data & 0x08) ? CLEAR_LINE : ASSERT_LINE);
}
+ // Determine the value of the head (side) select line.
head = ((data & 0x40) && (drive != 3)) ? 1 : 0;
+ // Update the floppy controller register
set_dskreg(data);
update_lines();
+ // Select the floppy drive that is active.
floppy_image_device *selected_floppy = m_floppies[drive]->get_device();
m_wd17xx->set_floppy(selected_floppy);
+ // Apply the side select value.
if (selected_floppy)
selected_floppy->ss_w(head);
+ // Set the density
m_wd17xx->dden_w(!BIT(dskreg(), 5));
}
drencorxeen
Aug 4, 2017, 06:43
Duke,
So after hours of testing it does seem that the patch was made for coco_fdc.cpp has at least resolved this issue at this time.