[CoCo] Further cleanups in CoCo cartridge slot signal handling

This is an attempt to make how the various signals on the CoCo cartridge slot (CART, NMI, HALT) a bit more standardized and less ad hoc, especially with the practice of Program Paks tying the CART line to Q.  This should fix some outstanding bugs in how these signals were passed around when using the CoCo Multi-Pak interface.

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;
@@ -55,11 +67,18 @@ private:
 	// 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);
 }