Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm/rm380z.cpp: Correct floppy disk handling so double sided disks work and also add support for 8 inch disks #12144

Merged
merged 2 commits into from
Mar 16, 2024

Conversation

RobinSergeant
Copy link
Contributor

Double sided disks would not load before because the floppy drives were wrongly configured as single sided. The wrong control port bit was also being used to select the side. After fixing this I can load more software and access the second side of double sided disks (logical drives C: and D:).

I've also added support for 8 inch disks used by the /F firmware variants such as "COS 3.4D/F". The "/F" refers to FDS (Full Disc System - 8") as opposed to MDS (Mini Disc System - 5.25"), which is designated by "/M".

FDS and MDS actually use different ports, so I've also updated the IO port map to include both sets. The firmware guide shows both as being reserved for FDC use. NB the control port address is not fully decoded and can actually be written to using 4 addresses which is why I've expanded this.

I have tested various double sided MDS disks, but have not been able to test FDS as there don't seem to be any 8" disk dumps available. However, it does now show the "Disk not ready" message when trying to boot without a disk (previously nothing happened when trying to boot with COS 3.4D).

Comment on lines 189 to 192
map(0xc4, 0xc4).w(FUNC(rm380z_state::disk_0_control));
map(0xc5, 0xff).rw(FUNC(rm380z_state::rm380z_porthi_r), FUNC(rm380z_state::rm380z_porthi_w));
map(0xc4, 0xc7).w(FUNC(rm380z_state::disk_0_control));
map(0xe0, 0xe3).rw(m_fdc, FUNC(fd1771_device::read), FUNC(fd1771_device::write));
map(0xe4, 0xe7).w(FUNC(rm380z_state::disk_0_control));
map(0xe8, 0xff).rw(FUNC(rm380z_state::rm380z_porthi_r), FUNC(rm380z_state::rm380z_porthi_w));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it work if you do this?

	map(0xc4, 0xc7).mirror(0x20).w(FUNC(rm380z_state::disk_0_control));
	map(0xe0, 0xe3).rw(m_fdc, FUNC(fd1771_device::read), FUNC(fd1771_device::write));
	map(0xe8, 0xff).rw(FUNC(rm380z_state::rm380z_porthi_r), FUNC(rm380z_state::rm380z_porthi_w));

That way it’s clearer that it’s ignoring A5 for that handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that works. Thank you.

Comment on lines 271 to 279
FLOPPY_CONNECTOR(config, "wd1771:0", rm380z_floppies, "sssd", floppy_image_device::default_mfm_floppy_formats);
FLOPPY_CONNECTOR(config, "wd1771:1", rm380z_floppies, "sssd", floppy_image_device::default_mfm_floppy_formats);
FLOPPY_CONNECTOR(config, "wd1771:0", rm380z_mds_floppies, "mds", floppy_image_device::default_mfm_floppy_formats);
FLOPPY_CONNECTOR(config, "wd1771:1", rm380z_mds_floppies, "mds", floppy_image_device::default_mfm_floppy_formats);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use m_floppy0 and m_floppy1 in place of the literal tags here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines 303 to 309
void rm380z_state_cos34::configure_fds(machine_config &config)
{
rm380z_state_cos34::configure(config);

FLOPPY_CONNECTOR(config.replace(), "wd1771:0", rm380z_fds_floppies, "fds", floppy_image_device::default_mfm_floppy_formats);
FLOPPY_CONNECTOR(config.replace(), "wd1771:1", rm380z_fds_floppies, "fds", floppy_image_device::default_mfm_floppy_formats);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general you should avoid replacing devices unnecessarily and just

Can you just reconfigure the devices here? If you make the other change to not use literal tags, something like:

	m_floppy0->option_reset();
	rm380z_fds_floppies(*m_floppy0);
	m_floppy0->set_default_option("fds");
	m_floppy1->option_reset();
	rm380z_fds_floppies(*m_floppy1);
	m_floppy1->set_default_option("fds");

Alternatively, you could have both drives in one set of options, and make the slots fixed to prevent the user from selecting the wrong type:

static void rm380z_floppies(device_slot_interface &device)
{
	device.option_add("mds", FLOPPY_525_SD);
	device.option_add("fds", FLOPPY_8_DSSD);
}

	FLOPPY_CONNECTOR(config, m_floppy0, rm380z_floppies, "mds", floppy_image_device::default_mfm_floppy_formats).set_fixed(true);
	FLOPPY_CONNECTOR(config, m_floppy1, rm380z_floppies, "mds", floppy_image_device::default_mfm_floppy_formats).set_fixed(true);

void rm380z_state_cos34::configure_fds(machine_config &config)
{
	rm380z_state_cos34::configure(config);

	m_floppy0->set_default_option("fds");
	m_floppy1->set_default_option("fds");
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I've gone with your second alternative as that seems cleaner.

@RobinSergeant
Copy link
Contributor Author

Thanks for reviewing @cuavas. I've now addressed your comments.

@cuavas cuavas merged commit 4e8ab32 into mamedev:master Mar 16, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants