Merge pull request #2164 from npwoods/coco_multipak_cleanup

[CoCo] Further cleanups in CoCo cartridge slot signal handling

src/devices/bus/coco/coco_fdc.h

@@ -51,7 +51,7 @@ protected:
 	// wrapper for setting the cart line
 	void cart_set_line(cococart_slot_device::line which, cococart_slot_device::line_value value)
 	{
-		m_owner->cart_set_line(which, value);
+		m_owner->set_line_value(which, value);
 	}
 	void cart_set_line(cococart_slot_device::line which, bool value)
 	{

src/devices/bus/coco/coco_multi.cpp

@@ -2,9 +2,9 @@
 // copyright-holders:Nathan Woods
 /***************************************************************************
 
-    coco_multi.c
+	coco_multi.cpp
 
-    Code for emulating CoCo's Multi-Pak Interface
+	Code for emulating CoCo's Multi-Pak Interface
 
      The Multi-Pak interface multiplexes all I/O lines from the Color
      Computer's expansion port to four identical ports. All I/O lines
@@ -18,7 +18,7 @@
      panel, four position switch. When adjusted the switch will direct the
      MPI to target these three I/O lines to the selected slot.
 
-     Second, the MPI will listen to writes to 0xff7f and respond accordingly:
+     Second, the MPI will listen to writes to $FF7F and respond accordingly:
 
      bit 0 --\___ Target *SCS to this slot number
      bit 1 --/
@@ -29,11 +29,11 @@
      bit 6 ------ Ignore
      bit 7 ------ Ignore
 
-     After writing to 0xff7f, the position of the physical switch has no
+     After writing to $FF7F, the position of the physical switch has no
      effect until reset.
 
-     Reading is supported on 0xff7f. It will reflect the position of the
+     Reading is supported on $FF7F. It will reflect the position of the
-     physical switch. Until data is written to 0xff7f, then it will only
+     physical switch. Until data is written to $FF7F, then it will only
      reflect what has been written until a reset.
 
      A common modification users of the OS-9 operating system made was to
@@ -46,8 +46,10 @@
      Because of sloppy address decoding the original MPI also responds to
      $FF9F. No software is known to take advantage of this. After the
      introduction of the CoCo 3, which uses $FF9F internally, Tandy provided
-     free upgrades to any MPI to fix this problem.
+     free upgrades to any MPI to fix this problem.  This behavior is not
+	 emulated (yet).
 
+	 Slots seem to be one-counted (i.e. - 1-4, not 0-3) by convention
 
 ***************************************************************************/
 
@@ -84,43 +86,27 @@ static SLOT_INTERFACE_START(coco_cart_slot4)
 	SLOT_INTERFACE("pak", COCO_PAK)
 SLOT_INTERFACE_END
 
-WRITE_LINE_MEMBER(coco_multipak_device::multi_cart_w)
-{
-	cococart_slot_device *cart = dynamic_cast<cococart_slot_device *>(owner());
-	cart->m_cart_callback(state);
-}
-
-WRITE_LINE_MEMBER(coco_multipak_device::multi_nmi_w)
-{
-	cococart_slot_device *cart = dynamic_cast<cococart_slot_device *>(owner());
-	cart->m_nmi_callback(state);
-}
-
-WRITE_LINE_MEMBER(coco_multipak_device::multi_halt_w)
-{
-	cococart_slot_device *cart = dynamic_cast<cococart_slot_device *>(owner());
-	cart->m_halt_callback(state);
-}
 
 static MACHINE_CONFIG_FRAGMENT(coco_multi)
 	MCFG_COCO_CARTRIDGE_ADD(SLOT1_TAG, coco_cart_slot1_3, nullptr)
-	MCFG_COCO_CARTRIDGE_CART_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_cart_w))
+	MCFG_COCO_CARTRIDGE_CART_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot1_cart_w))
-	MCFG_COCO_CARTRIDGE_NMI_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_nmi_w))
+	MCFG_COCO_CARTRIDGE_NMI_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot1_nmi_w))
-	MCFG_COCO_CARTRIDGE_HALT_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_halt_w))
+	MCFG_COCO_CARTRIDGE_HALT_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot1_halt_w))
 	MCFG_COCO_CARTRIDGE_ADD(SLOT2_TAG, coco_cart_slot1_3, nullptr)
-	MCFG_COCO_CARTRIDGE_CART_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_cart_w))
+	MCFG_COCO_CARTRIDGE_CART_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot2_cart_w))
-	MCFG_COCO_CARTRIDGE_NMI_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_nmi_w))
+	MCFG_COCO_CARTRIDGE_NMI_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot2_nmi_w))
-	MCFG_COCO_CARTRIDGE_HALT_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_halt_w))
+	MCFG_COCO_CARTRIDGE_HALT_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot2_halt_w))
 	MCFG_COCO_CARTRIDGE_ADD(SLOT3_TAG, coco_cart_slot1_3, nullptr)
-	MCFG_COCO_CARTRIDGE_CART_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_cart_w))
+	MCFG_COCO_CARTRIDGE_CART_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot3_cart_w))
-	MCFG_COCO_CARTRIDGE_NMI_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_nmi_w))
+	MCFG_COCO_CARTRIDGE_NMI_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot3_nmi_w))
-	MCFG_COCO_CARTRIDGE_HALT_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_halt_w))
+	MCFG_COCO_CARTRIDGE_HALT_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot3_halt_w))
 	MCFG_COCO_CARTRIDGE_ADD(SLOT4_TAG, coco_cart_slot4, "fdcv11")
-	MCFG_COCO_CARTRIDGE_CART_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_cart_w))
+	MCFG_COCO_CARTRIDGE_CART_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot4_cart_w))
-	MCFG_COCO_CARTRIDGE_NMI_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_nmi_w))
+	MCFG_COCO_CARTRIDGE_NMI_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot4_nmi_w))
-	MCFG_COCO_CARTRIDGE_HALT_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_halt_w))
+	MCFG_COCO_CARTRIDGE_HALT_CB(DEVWRITELINE(DEVICE_SELF, coco_multipak_device, multi_slot4_halt_w))
 MACHINE_CONFIG_END
 
+
 //**************************************************************************
 //  GLOBAL VARIABLES
 //**************************************************************************
@@ -139,12 +125,11 @@ const device_type COCO_MULTIPAK = device_creator<coco_multipak_device>;
 
 coco_multipak_device::coco_multipak_device(const machine_config &mconfig, const char *tag, device_t *owner, uint32_t clock)
 		: device_t(mconfig, COCO_MULTIPAK, "CoCo Multi-Pak Interface", tag, owner, clock, "coco_multipak", __FILE__),
-		device_cococart_interface( mconfig, *this ), m_owner(nullptr), m_select(0)
+		device_cococart_interface( mconfig, *this ), m_select(0)
-	{
+{
 }
 
 
-
 //-------------------------------------------------
 //  device_start - device-specific startup
 //-------------------------------------------------
@@ -156,7 +141,6 @@ void coco_multipak_device::device_start()
 	m_slots[1] = dynamic_cast<cococart_slot_device *>(subdevice(SLOT2_TAG));
 	m_slots[2] = dynamic_cast<cococart_slot_device *>(subdevice(SLOT3_TAG));
 	m_slots[3] = dynamic_cast<cococart_slot_device *>(subdevice(SLOT4_TAG));
-	m_owner = dynamic_cast<cococart_slot_device *>(owner());
 
 	// install $FF7F handler
 	write8_delegate wh = write8_delegate(FUNC(coco_multipak_device::ff7f_write), this);
@@ -170,7 +154,6 @@ void coco_multipak_device::device_start()
 }
 
 
-
 //-------------------------------------------------
 //  device_reset - device-specific reset
 //-------------------------------------------------
@@ -181,7 +164,6 @@ void coco_multipak_device::device_reset()
 }
 
 
-
 //-------------------------------------------------
 //  machine_config_additions - device-specific
 //  machine configurations
@@ -193,51 +175,96 @@ machine_config_constructor coco_multipak_device::device_mconfig_additions() cons
 }
 
 
+//**************************************************************************
+//  INTERNAL ACCESSORS
+//**************************************************************************
 
-//-------------------------------------------------
+cococart_slot_device &coco_multipak_device::owning_slot()
-//  get_cart_base
-//-------------------------------------------------
-
-uint8_t* coco_multipak_device::get_cart_base()
 {
-	return active_cts_slot()->get_cart_base();
+	return *dynamic_cast<cococart_slot_device *>(owner());
 }
 
 
-
 //-------------------------------------------------
-//  read
+//  active_scs_slot_number
 //-------------------------------------------------
 
-READ8_MEMBER(coco_multipak_device::read)
+int coco_multipak_device::active_scs_slot_number() const
 {
-	return active_scs_slot()->read(space,offset);
+	return ((m_select >> 0) & 0x03) + 1;
 }
 
 
-
 //-------------------------------------------------
-//  write
+//  active_cts_slot_number
 //-------------------------------------------------
 
-WRITE8_MEMBER(coco_multipak_device::write)
+int coco_multipak_device::active_cts_slot_number() const
 {
-	active_scs_slot()->write(space,offset,data);
+	return ((m_select >> 4) & 0x03) + 1;
 }
 
 
-
 //-------------------------------------------------
-//  set_sound_enable
+//  slot
 //-------------------------------------------------
 
-void coco_multipak_device::set_sound_enable(bool sound_enable)
+cococart_slot_device &coco_multipak_device::slot(int slot_number)
 {
-	for (cococart_slot_device *slot : m_slots)
+	assert(slot_number >= 1 && slot_number <= 4);
-		slot->cart_set_line(cococart_slot_device::line::SOUND_ENABLE, sound_enable ? cococart_slot_device::line_value::ASSERT : cococart_slot_device::line_value::CLEAR);
+	return *m_slots[slot_number - 1];
 }
 
 
+//-------------------------------------------------
+//  active_scs_slot
+//-------------------------------------------------
+
+cococart_slot_device &coco_multipak_device::active_scs_slot()
+{
+	int slot_number = active_scs_slot_number();
+	return slot(slot_number);
+}
+
+
+//-------------------------------------------------
+//  active_cts_slot
+//-------------------------------------------------
+
+cococart_slot_device &coco_multipak_device::active_cts_slot()
+{
+	int slot_number = active_cts_slot_number();
+	return slot(slot_number);
+}
+
+
+//**************************************************************************
+//  METHODS
+//**************************************************************************
+
+//-------------------------------------------------
+//  set_select
+//-------------------------------------------------
+
+void coco_multipak_device::set_select(uint8_t new_select)
+{
+	// identify old value for CART, in case this needs to change
+	cococart_slot_device::line_value old_cart = active_cts_slot().get_line_value(cococart_slot_device::line::CART);
+
+	// change value
+	uint8_t xorval = m_select ^ new_select;
+	m_select = new_select;
+
+	// did the cartridge base change?
+	if (xorval & 0x03)
+		cart_base_changed();
+
+	// did the CART line change?
+	cococart_slot_device::line_value new_cart = active_cts_slot().get_line_value(cococart_slot_device::line::CART);
+	if (new_cart != old_cart)
+		update_line(active_cts_slot_number(), cococart_slot_device::line::CART);
+}
+
 
 //-------------------------------------------------
 //  ff7f_write
@@ -249,37 +276,94 @@ WRITE8_MEMBER(coco_multipak_device::ff7f_write)
 }
 
 
-
 //-------------------------------------------------
-//  set_select
+//  update_line
 //-------------------------------------------------
 
-void coco_multipak_device::set_select(uint8_t new_select)
+void coco_multipak_device::update_line(int slot_number, cococart_slot_device::line line)
 {
-	uint8_t xorval = m_select ^ new_select;
+	bool propagate;
-	m_select = new_select;
+
-	if (xorval & 0x30)
+	// one of our child devices set a line; we may need to propagate it upwards
-		cart_base_changed();
+	switch (line)
+	{
+	case cococart_slot_device::line::CART:
+		// only propagate if this is coming from the slot specified
+		propagate = slot_number == active_cts_slot_number();
+		break;
+
+	case cococart_slot_device::line::NMI:
+	case cococart_slot_device::line::HALT:
+		// always propagate these
+		propagate = true;
+		break;
+
+	default:
+		// do nothing
+		propagate = false;
+		break;
+	}
+
+	if (propagate)
+		owning_slot().set_line_value(line, slot(slot_number).get_line_value(line));
 }
 
 
-
 //-------------------------------------------------
-//  active_scs_slot
+//  set_sound_enable
 //-------------------------------------------------
 
-cococart_slot_device *coco_multipak_device::active_scs_slot(void)
+void coco_multipak_device::set_sound_enable(bool sound_enable)
 {
-	return m_slots[(m_select >> 0) & 0x03];
+	// the SOUND_ENABLE (SNDEN) line is different; it is controlled by the CPU
+	for (cococart_slot_device *slot : m_slots)
+		slot->set_line_value(cococart_slot_device::line::SOUND_ENABLE, sound_enable ? cococart_slot_device::line_value::ASSERT : cococart_slot_device::line_value::CLEAR);
 }
 
 
-
 //-------------------------------------------------
-//  active_cts_slot
+//  get_cart_base
 //-------------------------------------------------
 
-cococart_slot_device *coco_multipak_device::active_cts_slot(void)
+uint8_t* coco_multipak_device::get_cart_base()
 {
-	return m_slots[(m_select >> 4) & 0x03];
+	return active_cts_slot().get_cart_base();
 }
+
+
+//-------------------------------------------------
+//  read
+//-------------------------------------------------
+
+READ8_MEMBER(coco_multipak_device::read)
+{
+	return active_scs_slot().read(space, offset);
+}
+
+
+//-------------------------------------------------
+//  write
+//-------------------------------------------------
+
+WRITE8_MEMBER(coco_multipak_device::write)
+{
+	active_scs_slot().write(space, offset, data);
+}
+
+
+//-------------------------------------------------
+//  multiX_slotX_[cart|nmi|halt] trampolines
+//-------------------------------------------------
+
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot1_cart_w)	{ update_line(1, cococart_slot_device::line::CART); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot1_nmi_w)	{ update_line(1, cococart_slot_device::line::NMI); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot1_halt_w)	{ update_line(1, cococart_slot_device::line::HALT); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot2_cart_w)	{ update_line(2, cococart_slot_device::line::CART); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot2_nmi_w)	{ update_line(2, cococart_slot_device::line::NMI); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot2_halt_w)	{ update_line(2, cococart_slot_device::line::HALT); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot3_cart_w)	{ update_line(3, cococart_slot_device::line::CART); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot3_nmi_w)	{ update_line(3, cococart_slot_device::line::NMI); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot3_halt_w)	{ update_line(3, cococart_slot_device::line::HALT); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot4_cart_w)	{ update_line(4, cococart_slot_device::line::CART); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot4_nmi_w)	{ update_line(4, cococart_slot_device::line::NMI); }
+WRITE_LINE_MEMBER(coco_multipak_device::multi_slot4_halt_w)	{ update_line(4, cococart_slot_device::line::HALT); }

src/devices/bus/coco/coco_multi.h

@@ -36,9 +36,21 @@ public:
 
 	virtual uint8_t* get_cart_base() override;
 
-	DECLARE_WRITE_LINE_MEMBER(multi_cart_w);
+	// these are only public so they can be in a MACHINE_CONFIG_FRAGMENT
-	DECLARE_WRITE_LINE_MEMBER(multi_nmi_w);
+	// declaration; don't think about them as publically accessable
-	DECLARE_WRITE_LINE_MEMBER(multi_halt_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot1_cart_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot1_nmi_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot1_halt_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot2_cart_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot2_nmi_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot2_halt_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot3_cart_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot3_nmi_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot3_halt_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot4_cart_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot4_nmi_w);
+	DECLARE_WRITE_LINE_MEMBER(multi_slot4_halt_w);
+
 protected:
 	// device-level overrides
 	virtual void device_start() override;
@@ -49,17 +61,23 @@ protected:
 
 private:
 	// device references
-	cococart_slot_device *m_owner;
 	std::array<cococart_slot_device *, 4> m_slots;
 
 	// internal state
 	uint8_t m_select;
 
+	// internal accessors
+	cococart_slot_device &owning_slot();
+	int active_scs_slot_number() const;
+	int active_cts_slot_number() const;
+	cococart_slot_device &slot(int slot_number);
+	cococart_slot_device &active_scs_slot();
+	cococart_slot_device &active_cts_slot();
+
 	// methods
-	DECLARE_WRITE8_MEMBER(ff7f_write);
-	cococart_slot_device *active_scs_slot(void);
-	cococart_slot_device *active_cts_slot(void);
 	void set_select(uint8_t new_select);
+	DECLARE_WRITE8_MEMBER(ff7f_write);
+	void update_line(int slot_number, cococart_slot_device::line line);
 };
 
 

src/devices/bus/coco/coco_pak.cpp

@@ -117,7 +117,7 @@ void coco_pak_device::device_reset()
 			: cococart_slot_device::line_value::CLEAR;
 
 		// normal CoCo PAKs tie their CART line to Q - the system clock
-		m_owner->cart_set_line(cococart_slot_device::line::CART, cart_line);
+		m_owner->set_line_value(cococart_slot_device::line::CART, cart_line);
 	}
 }
 

src/devices/bus/coco/coco_t4426.cpp

@@ -208,7 +208,7 @@ void coco_t4426_device::device_reset()
 {
 	LOG("%s()\n", FUNCNAME );
 	auto cart_line = cococart_slot_device::line_value::Q;
-	m_owner->cart_set_line(cococart_slot_device::line::CART, cart_line);
+	m_owner->set_line_value(cococart_slot_device::line::CART, cart_line);
 }
 
 /*-------------------------------------------------

src/devices/bus/coco/cococart.cpp

@@ -239,8 +239,8 @@ void cococart_slot_device::twiddle_line_if_q(coco_cartridge_line &line)
 
 
 //-------------------------------------------------
-//  coco_cartridge_twiddle_q_lines - hack to
+//  twiddle_q_lines - hack to support twiddling the
-//  support twiddling the Q line
+//	Q line
 //-------------------------------------------------
 
 void cococart_slot_device::twiddle_q_lines()
@@ -252,34 +252,63 @@ void cococart_slot_device::twiddle_q_lines()
 
 
 //-------------------------------------------------
-//  coco_cartridge_set_line
+//  set_line_value
 //-------------------------------------------------
 
-void cococart_slot_device::cart_set_line(cococart_slot_device::line which, cococart_slot_device::line_value value)
+void cococart_slot_device::set_line_value(cococart_slot_device::line which, cococart_slot_device::line_value value)
 {
 	switch (which)
 	{
-		case line::CART:
+	case cococart_slot_device::line::CART:
-			set_line_timer(m_cart_line, value);
+		set_line_timer(m_cart_line, value);
-			break;
+		break;
 
-		case line::NMI:
+	case cococart_slot_device::line::NMI:
-			set_line_timer(m_nmi_line, value);
+		set_line_timer(m_nmi_line, value);
-			break;
+		break;
 
-		case line::HALT:
+	case cococart_slot_device::line::HALT:
-			set_line_timer(m_halt_line, value);
+		set_line_timer(m_halt_line, value);
-			break;
+		break;
 
-		case line::SOUND_ENABLE:
+	case cococart_slot_device::line::SOUND_ENABLE:
-			if (m_cart)
+		if (m_cart)
-				m_cart->set_sound_enable(value != cococart_slot_device::line_value::CLEAR);
+			m_cart->set_sound_enable(value != cococart_slot_device::line_value::CLEAR);
-			break;
+		break;
 	}
 }
 
 
 
+//-------------------------------------------------
+//  get_line_value
+//-------------------------------------------------
+
+cococart_slot_device::line_value cococart_slot_device::get_line_value(cococart_slot_device::line which) const
+{
+	line_value result;
+	switch (which)
+	{
+	case cococart_slot_device::line::CART:
+		result = m_cart_line.value;
+		break;
+
+	case cococart_slot_device::line::NMI:
+		result = m_nmi_line.value;
+		break;
+
+	case cococart_slot_device::line::HALT:
+		result = m_halt_line.value;
+		break;
+
+	default:
+		result = line_value::CLEAR;
+		break;
+	}
+	return result;
+}
+
+
 //-------------------------------------------------
 //  get_cart_base
 //-------------------------------------------------

src/devices/bus/coco/cococart.h

@@ -90,8 +90,9 @@ public:
 	DECLARE_READ8_MEMBER(read);
 	DECLARE_WRITE8_MEMBER(write);
 
-	// sets a cartridge line
+	// manipulation of cartridge lines
-	void cart_set_line(line line, line_value value);
+	void set_line_value(line line, line_value value);
+	line_value get_line_value(line line) const;
 
 	// hack to support twiddling the Q line
 	void twiddle_q_lines();

src/mame/machine/coco.cpp

@@ -695,7 +695,7 @@ void coco_state::update_sound(void)
 	m_cassette->change_state(cas_sound, CASSETTE_MASK_SPEAKER);
 
 	/* determine the cartridge sound status */
-	m_cococart->cart_set_line(
+	m_cococart->set_line_value(
 		cococart_slot_device::line::SOUND_ENABLE,
 		bCartSoundEnable ? cococart_slot_device::line_value::ASSERT : cococart_slot_device::line_value::CLEAR);
 }