ncr53c90: Numerous fixes

- Prevent recursive stepping when scsi_ctrl_changed is called in the middle of a state (most likely by HLE SCSI devices programmed to respond instantly)
- Add some calls to the state machine stepping handler for non-timeout conditions that may advance it, particularly non-DMA writes to the FIFO
- Always wait for FIFO to have at least one byte before sending anything
- Properly interpret configuration flag for Save Residual Byte 16-bit DMA mode
- Use little-endian byte order for 16-bit DMA handlers, but add alternate byte-swapping handlers for convenient use with big-endian systems
- Allow 16-bit DMA handlers to access just one byte in the FIFO rather than exit MAME with an exception
- Always honor timeout for selection arbitration (previously any event could short-circuit it)
- Allow side effects of read handlers to be disabled for debugging

* macquadra700.cpp: Correct SCSI chip type and bus configuration mode

src/devices/machine/ncr53c90.cpp

@@ -3,7 +3,7 @@
 
 /*
  * TODO
- *   - 16 bit dma order, alignment and last byte handling
+ *   - 16 bit dma alignment and last byte handling
  *   - clean up variable naming and protection
  */
 
@@ -23,6 +23,7 @@
 DEFINE_DEVICE_TYPE(NCR53C90, ncr53c90_device, "ncr53c90", "NCR 53C90 SCSI Controller")
 DEFINE_DEVICE_TYPE(NCR53C90A, ncr53c90a_device, "ncr53c90a", "NCR 53C90A Advanced SCSI Controller")
 DEFINE_DEVICE_TYPE(NCR53C94, ncr53c94_device, "ncr53c94", "NCR 53C94 Advanced SCSI Controller")
+DEFINE_DEVICE_TYPE(NCR53C96, ncr53c96_device, "ncr53c96", "NCR 53C96 Advanced SCSI Controller")
 DEFINE_DEVICE_TYPE(NCR53CF94, ncr53cf94_device, "ncr53cf94", "NCR 53CF94-2 Fast SCSI Controller") // TODO: differences not emulated
 DEFINE_DEVICE_TYPE(NCR53CF96, ncr53cf96_device, "ncr53cf96", "NCR 53CF96-2 Fast SCSI Controller") // TODO: differences not emulated
 
@@ -137,7 +138,7 @@ ncr53c90_device::ncr53c90_device(const machine_config &mconfig, device_type type
 	: nscsi_device(mconfig, type, tag, owner, clock)
 	, nscsi_slot_card_interface(mconfig, *this, DEVICE_SELF)
 	, tm(nullptr), config(0), status(0), istatus(0), clock_conv(0), sync_offset(0), sync_period(0), bus_id(0)
-	, select_timeout(0), seq(0), tcount(0), tcounter(0), tcounter_mask(0xffff), mode(0), fifo_pos(0), command_pos(0), state(0), xfr_phase(0), command_length(0), dma_dir(0), irq(false), drq(false), test_mode(false)
+	, select_timeout(0), seq(0), tcount(0), tcounter(0), tcounter_mask(0xffff), mode(0), fifo_pos(0), command_pos(0), state(0), xfr_phase(0), command_length(0), dma_dir(0), irq(false), drq(false), test_mode(false), stepping(0)
 	, m_irq_handler(*this)
 	, m_drq_handler(*this)
 {
@@ -174,6 +175,11 @@ ncr53c94_device::ncr53c94_device(const machine_config &mconfig, device_type type
 {
 }
 
+ncr53c96_device::ncr53c96_device(const machine_config &mconfig, const char *tag, device_t *owner, uint32_t clock)
+	: ncr53c94_device(mconfig, NCR53C96, tag, owner, clock)
+{
+}
+
 ncr53cf94_device::ncr53cf94_device(const machine_config &mconfig, const char *tag, device_t *owner, uint32_t clock)
 	: ncr53c94_device(mconfig, NCR53CF94, tag, owner, clock)
 {
@@ -233,6 +239,7 @@ void ncr53c90_device::device_reset()
 	seq = 0;
 	config &= 7;
 	status = 0;
+	command_length = 0;
 	istatus = 0;
 	irq = false;
 	m_irq_handler(irq);
@@ -269,7 +276,9 @@ void ncr53c90_device::scsi_ctrl_changed()
 		return;
 	}
 
-	step(false);
+	// disallow further recursion from here
+	if(!stepping)
+		step(false);
 }
 
 TIMER_CALLBACK_MEMBER(ncr53c90_device::update_tick)
@@ -283,9 +292,11 @@ void ncr53c90_device::step(bool timeout)
 	uint32_t data = scsi_bus->data_r();
 	uint8_t c     = command[0] & 0x7f;
 
-	LOGMASKED(LOG_STATE, "state=%d.%d %s\n",
+	LOGMASKED(LOG_STATE, "state=%d.%d %s @ %s\n",
 		state & STATE_MASK, (state & SUB_MASK) >> SUB_SHIFT,
-		timeout ? "timeout" : "change");
+		timeout ? "timeout" : "change", machine().time().to_string());
+
+	stepping++;
 
 	if(mode == MODE_I && !(ctrl & S_BSY)) {
 		state = IDLE;
@@ -465,6 +476,14 @@ void ncr53c90_device::step(bool timeout)
 		break;
 
 	case DISC_SEL_ARBITRATION_INIT:
+		if(!timeout)
+			break;
+
+		state = DISC_SEL_ARBITRATION;
+		step(false);
+		break;
+
+	case DISC_SEL_ARBITRATION:
 		// wait until a command is in the fifo
 		if (!fifo_pos) {
 			// this sequence isn't documented for initiator selection, but
@@ -476,20 +495,13 @@ void ncr53c90_device::step(bool timeout)
 			break;
 		}
 
-		command_length = fifo_pos + tcounter;
-		state = DISC_SEL_ARBITRATION;
-		step(false);
-		break;
-
-	case DISC_SEL_ARBITRATION:
 		if(c == CD_SELECT) {
 			state = DISC_SEL_WAIT_REQ;
 		} else
 			state = DISC_SEL_ATN_WAIT_REQ;
 
 		scsi_bus->ctrl_wait(scsi_refid, S_REQ, S_REQ);
-		if(ctrl & S_REQ)
+		step(false);
-			step(false);
 		break;
 
 	case DISC_SEL_ATN_WAIT_REQ:
@@ -506,12 +518,14 @@ void ncr53c90_device::step(bool timeout)
 		break;
 
 	case DISC_SEL_ATN_SEND_BYTE:
-		command_length--;
+		if(command_length)
+			command_length--;
 		if(c == CD_SELECT_ATN_STOP) {
 			seq = 2;
 			function_bus_complete();
 		} else {
 			state = DISC_SEL_WAIT_REQ;
+			step(false);
 		}
 		break;
 
@@ -519,7 +533,7 @@ void ncr53c90_device::step(bool timeout)
 		if(!(ctrl & S_REQ))
 			break;
 		if((ctrl & S_PHASE_MASK) != S_PHASE_COMMAND) {
-			if(!command_length)
+			if(dma_command ? (status & S_TC0) && !command_length : !fifo_pos)
 				seq = 4;
 			else
 				seq = 2;
@@ -527,6 +541,8 @@ void ncr53c90_device::step(bool timeout)
 			function_bus_complete();
 			break;
 		}
+		if(!fifo_pos)
+			break;
 		if(seq < 3)
 			seq = 3;
 		state = DISC_SEL_SEND_BYTE;
@@ -534,18 +550,22 @@ void ncr53c90_device::step(bool timeout)
 		break;
 
 	case DISC_SEL_SEND_BYTE:
-		if(command_length) {
+		if(!dma_command && !fifo_pos)
+			seq = 4;
+		else if(dma_command && (status & S_TC0) && command_length) {
 			command_length--;
 			if(!command_length)
 				seq = 4;
 		}
 
 		state = DISC_SEL_WAIT_REQ;
+		step(false);
 		break;
 
 	case INIT_CPT_RECV_BYTE_ACK:
 		state = INIT_CPT_RECV_WAIT_REQ;
 		scsi_bus->ctrl_w(scsi_refid, 0, S_ACK);
+		step(false);
 		break;
 
 	case INIT_CPT_RECV_WAIT_REQ:
@@ -641,6 +661,7 @@ void ncr53c90_device::step(bool timeout)
 	case INIT_XFR_RECV_BYTE_ACK:
 		state = INIT_XFR_WAIT_REQ;
 		scsi_bus->ctrl_w(scsi_refid, 0, S_ACK);
+		step(false);
 		break;
 
 	case INIT_XFR_RECV_BYTE_NACK:
@@ -657,7 +678,8 @@ void ncr53c90_device::step(bool timeout)
 
 	case INIT_XFR_BUS_COMPLETE:
 		// wait for dma transfer to complete and fifo to drain
-		if (dma_command && (!(status & S_TC0) || fifo_pos))
+		// (FIFO may still contain one residual byte if enabled for 16-bit DMA)
+		if (dma_command && drq)
 			break;
 		bus_complete();
 		break;
@@ -670,6 +692,8 @@ void ncr53c90_device::step(bool timeout)
 			command_pos = 0;
 			bus_complete();
 		} else {
+			if(!fifo_pos)
+				break;
 			state = INIT_XFR_SEND_PAD;
 			send_byte();
 		}
@@ -712,6 +736,9 @@ void ncr53c90_device::step(bool timeout)
 			state & STATE_MASK, (state & SUB_MASK) >> SUB_SHIFT);
 		exit(0);
 	}
+
+	assert(stepping > 0);
+	stepping--;
 }
 
 void ncr53c90_device::send_byte()
@@ -720,9 +747,9 @@ void ncr53c90_device::send_byte()
 		fatalerror("ncr53c90_device::send_byte - !fifo_pos\n");
 
 	state = (state & STATE_MASK) | (SEND_WAIT_SETTLE << SUB_SHIFT);
-	if((state & STATE_MASK) != INIT_XFR_SEND_PAD &&
+	if((state & STATE_MASK) != INIT_XFR_SEND_PAD /*&&
 		((state & STATE_MASK) != DISC_SEL_SEND_BYTE ||
-		command_length))
+		!(status & S_TC0) || command_length)*/)
 		scsi_bus->data_w(scsi_refid, fifo_pop());
 	else
 		scsi_bus->data_w(scsi_refid, 0);
@@ -847,6 +874,7 @@ void ncr53c90_device::fifo_w(uint8_t data)
 		fifo[fifo_pos++] = data;
 
 	check_drq();
+	step(false);
 }
 
 uint8_t ncr53c90_device::command_r()
@@ -1055,7 +1083,7 @@ uint8_t ncr53c90_device::status_r()
 {
 	uint32_t ctrl = scsi_bus->ctrl_r();
 	uint8_t res = status | (ctrl & S_MSG ? 4 : 0) | (ctrl & S_CTL ? 2 : 0) | (ctrl & S_INP ? 1 : 0);
-	LOG("status_r %02x (%s)\n", res, machine().describe_context());
+	//LOG("status_r %02x (%s)\n", res, machine().describe_context());
 
 	return res;
 }
@@ -1070,17 +1098,20 @@ uint8_t ncr53c90_device::istatus_r()
 {
 	uint8_t res = istatus;
 
-	if (irq)
+	if (!machine().side_effects_disabled())
 	{
-		status &= ~(S_GROSS_ERROR | S_PARITY | S_TCC);
+		if (irq)
-		istatus = 0;
+		{
-		seq = 0;
+			status &= ~(S_GROSS_ERROR | S_PARITY | S_TCC);
-	}
+			istatus = 0;
-	check_irq();
+			seq = 0;
-	if(res)
+		}
-		command_pop_and_chain();
+		check_irq();
+		if(res)
+			command_pop_and_chain();
 
-	LOG("istatus_r %02x (%s)\n", res, machine().describe_context());
+		LOG("istatus_r %02x (%s)\n", res, machine().describe_context());
+	}
 	return res;
 }
 
@@ -1158,6 +1189,9 @@ void ncr53c90_device::dma_w(uint8_t val)
 
 uint8_t ncr53c90_device::dma_r()
 {
+	if (machine().side_effects_disabled())
+		return fifo[0];
+
 	uint8_t r = fifo_pop();
 
 	if ((sync_offset != 0) || ((scsi_bus->ctrl_r() & S_PHASE_MASK) != S_PHASE_DATA_IN))
@@ -1209,8 +1243,11 @@ void ncr53c90_device::decrement_tcounter(int count)
 	else
 		tcounter = 0;
 
-	if (tcounter == 0)
+	if (tcounter == 0 && !(status & S_TC0))
+	{
 		status |= S_TC0;
+		command_length = fifo_pos;
+	}
 
 	check_drq();
 }
@@ -1254,8 +1291,8 @@ uint8_t ncr53c90a_device::status_r()
 {
 	uint32_t ctrl = scsi_bus->ctrl_r();
 	uint8_t res = (irq ? S_INTERRUPT : 0) | status | (ctrl & S_MSG ? 4 : 0) | (ctrl & S_CTL ? 2 : 0) | (ctrl & S_INP ? 1 : 0);
-	LOG("status_r %02x (%s)\n", res, machine().describe_context());
+	//LOG("status_r %02x (%s)\n", res, machine().describe_context());
-	if (irq)
+	if (irq && !machine().side_effects_disabled())
 		status &= ~(S_GROSS_ERROR | S_PARITY | S_TCC);
 	return res;
 }
@@ -1295,21 +1332,24 @@ u16 ncr53c94_device::dma16_r()
 {
 	// check fifo underflow
 	if (fifo_pos < 2)
-		fatalerror("ncr53c94_device::dma16_r fifo_pos %d\n", fifo_pos);
+		return dma_r() | 0xff00;
 
 	// pop two bytes from fifo
-	u16 const data = (fifo[0] << 8) | fifo[1];
+	u16 const data = fifo[0] | (fifo[1] << 8);
-	fifo_pos -= 2;
+	if (!machine().side_effects_disabled())
-	memmove(fifo, fifo + 2, fifo_pos);
-
-	// update drq
-	if ((sync_offset != 0) || ((scsi_bus->ctrl_r() & S_PHASE_MASK) != S_PHASE_DATA_IN))
 	{
-		decrement_tcounter(2);
+		fifo_pos -= 2;
-	}
+		memmove(fifo, fifo + 2, fifo_pos);
-	check_drq();
 
-	step(false);
+		// update drq
+		if ((sync_offset != 0) || ((scsi_bus->ctrl_r() & S_PHASE_MASK) != S_PHASE_DATA_IN))
+		{
+			decrement_tcounter(2);
+		}
+		check_drq();
+
+		step(false);
+	}
 
 	return data;
 }
@@ -1317,12 +1357,15 @@ u16 ncr53c94_device::dma16_r()
 void ncr53c94_device::dma16_w(u16 data)
 {
 	// check fifo overflow
-	if (fifo_pos > 14)
+	if (fifo_pos > 14 || tcounter == 1)
-		fatalerror("ncr53c94_device::dma16_w fifo_pos %d\n", fifo_pos);
+	{
+		dma_w(data & 0x00ff);
+		return;
+	}
 
 	// push two bytes into fifo
-	fifo[fifo_pos++] = data >> 8;
 	fifo[fifo_pos++] = data;
+	fifo[fifo_pos++] = data >> 8;
 
 	// update drq
 	decrement_tcounter(2);
@@ -1342,11 +1385,11 @@ void ncr53c94_device::check_drq()
 			drq_state = false;
 			break;
 
-		case DMA_IN: // device to memory
+		case DMA_IN: // device to memory (optionally save last remaining byte for processor)
 			if (sync_offset == 0)
-				drq_state = (fifo_pos > 1);
+				drq_state = fifo_pos > (BIT(config3, 2) ? 1 : 0);
 			else
-				drq_state = !(status & S_TC0) && fifo_pos > 1;
+				drq_state = !(status & S_TC0) && fifo_pos > (BIT(config3, 2) ? 1 : 0);
 			break;
 
 		case DMA_OUT: // memory to device

src/devices/machine/ncr53c90.h

@@ -206,6 +206,7 @@ protected:
 	bool irq, drq;
 	bool dma_command;
 	bool test_mode;
+	int stepping;
 
 	void dma_set(int dir);
 	virtual void check_drq();
@@ -316,6 +317,9 @@ public:
 	u16 dma16_r();
 	void dma16_w(u16 data);
 
+	u16 dma16_swap_r() { return swapendian_int16(dma16_r()); }
+	void dma16_swap_w(u16 data) { return dma16_w(swapendian_int16(data)); }
+
 protected:
 	ncr53c94_device(const machine_config &mconfig, device_type type, const char *tag, device_t *owner, uint32_t clock);
 
@@ -339,6 +343,12 @@ private:
 	busmd_t m_busmd;
 };
 
+class ncr53c96_device : public ncr53c94_device
+{
+public:
+	ncr53c96_device(const machine_config &mconfig, const char *tag, device_t *owner, uint32_t clock);
+};
+
 class ncr53cf94_device : public ncr53c94_device
 {
 public:
@@ -354,6 +364,7 @@ public:
 DECLARE_DEVICE_TYPE(NCR53C90, ncr53c90_device)
 DECLARE_DEVICE_TYPE(NCR53C90A, ncr53c90a_device)
 DECLARE_DEVICE_TYPE(NCR53C94, ncr53c94_device)
+DECLARE_DEVICE_TYPE(NCR53C96, ncr53c96_device)
 DECLARE_DEVICE_TYPE(NCR53CF94, ncr53cf94_device)
 DECLARE_DEVICE_TYPE(NCR53CF96, ncr53cf96_device)
 

src/mame/apple/macpdm.cpp

@@ -1026,7 +1026,7 @@ void macpdm_state::pdm_map(address_map &map)
 	// 50f08000 = ethernet ID PROM
 	// 50f0a000 = MACE ethernet controller
 	map(0x50f10000, 0x50f10000).rw(FUNC(macpdm_state::scsi_r), FUNC(macpdm_state::scsi_w)).select(0xf0);
-	map(0x50f10100, 0x50f10101).rw(m_ncr53c94, FUNC(ncr53c94_device::dma16_r), FUNC(ncr53c94_device::dma16_w));
+	map(0x50f10100, 0x50f10101).rw(m_ncr53c94, FUNC(ncr53c94_device::dma16_swap_r), FUNC(ncr53c94_device::dma16_swap_w));
 	map(0x50f14000, 0x50f1401f).rw(m_awacs, FUNC(awacs_device::read), FUNC(awacs_device::write));
 	map(0x50f16000, 0x50f16000).rw(FUNC(macpdm_state::fdc_r), FUNC(macpdm_state::fdc_w)).select(0x1e00);
 

src/mame/apple/macquadra700.cpp

@@ -60,7 +60,7 @@ public:
 		m_floppy(*this, "fdc:%d", 0U),
 		m_rtc(*this,"rtc"),
 		m_scsibus1(*this, "scsi1"),
-		m_ncr1(*this, "scsi1:7:ncr5394"),
+		m_ncr1(*this, "scsi1:7:ncr53c96"),
 		m_sonic(*this, "sonic"),
 		m_screen(*this, "screen"),
 		m_palette(*this, "palette"),
@@ -87,7 +87,7 @@ private:
 	required_device_array<floppy_connector, 2> m_floppy;
 	required_device<rtc3430042_device> m_rtc;
 	required_device<nscsi_bus_device> m_scsibus1;
-	required_device<ncr53cf94_device> m_ncr1;
+	required_device<ncr53c96_device> m_ncr1;
 	required_device<dp83932c_device> m_sonic;
 	required_device<screen_device> m_screen;
 	required_device<palette_device> m_palette;
@@ -768,29 +768,12 @@ TIMER_CALLBACK_MEMBER(macquadra_state::mac_6015_tick)
 
 uint8_t macquadra_state::mac_5396_r(offs_t offset)
 {
-	if (offset < 0x100)
+	return m_ncr1->read(offset>>4);
-	{
-		return m_ncr1->read(offset>>4);
-	}
-	else    // pseudo-DMA: read from the FIFO
-	{
-		return m_ncr1->dma_r();
-	}
-
-	// never executed
-	return 0;
 }
 
 void macquadra_state::mac_5396_w(offs_t offset, uint8_t data)
 {
-	if (offset < 0x100)
+	m_ncr1->write(offset>>4, data);
-	{
-		m_ncr1->write(offset>>4, data);
-	}
-	else    // pseudo-DMA: write to the FIFO
-	{
-		m_ncr1->dma_w(data);
-	}
 }
 
 /***************************************************************************
@@ -805,7 +788,8 @@ void macquadra_state::quadra700_map(address_map &map)
 // 50008000 = Ethernet MAC ID PROM
 // 5000a000 = Sonic (DP83932) ethernet
 // 5000f000 = SCSI cf96, 5000f402 = SCSI #2 cf96
-	map(0x5000f000, 0x5000f401).rw(FUNC(macquadra_state::mac_5396_r), FUNC(macquadra_state::mac_5396_w)).mirror(0x00fc0000);
+	map(0x5000f000, 0x5000f0ff).rw(FUNC(macquadra_state::mac_5396_r), FUNC(macquadra_state::mac_5396_w)).mirror(0x00fc0000);
+	map(0x5000f100, 0x5000f101).rw(m_ncr1, FUNC(ncr53c94_device::dma16_swap_r), FUNC(ncr53c94_device::dma16_swap_w)).mirror(0x00fc0000);
 	map(0x5000c000, 0x5000dfff).rw(FUNC(macquadra_state::mac_scc_r), FUNC(macquadra_state::mac_scc_2_w)).mirror(0x00fc0000);
 	map(0x50014000, 0x50015fff).rw(m_easc, FUNC(asc_device::read), FUNC(asc_device::write)).mirror(0x00fc0000);
 	map(0x5001e000, 0x5001ffff).rw(FUNC(macquadra_state::swim_r), FUNC(macquadra_state::swim_w)).mirror(0x00fc0000);
@@ -950,12 +934,12 @@ void macquadra_state::macqd700(machine_config &config)
 	// HACK: Max clock for 5394/96 is 25 MHz, but we underrun the FIFO at that speed.
 	// Likely due to inaccurate 68040 and/or NSCSI bus timings; DAFB documentation is clear that there is
 	// no "magic latch" like the 5380 machines use.
-	NSCSI_CONNECTOR(config, "scsi1:7").option_set("ncr5394", NCR53CF94).clock(50_MHz_XTAL).machine_config(
+	NSCSI_CONNECTOR(config, "scsi1:7").option_set("ncr53c96", NCR53C96).clock(50_MHz_XTAL).machine_config(
 		[this] (device_t *device)
 		{
-			ncr53cf94_device &adapter = downcast<ncr53cf94_device &>(*device);
+			ncr53c96_device &adapter = downcast<ncr53c96_device &>(*device);
 
-			adapter.set_busmd(ncr53cf94_device::BUSMD_0);
+			adapter.set_busmd(ncr53c96_device::BUSMD_1);
 			adapter.irq_handler_cb().set(*this, FUNC(macquadra_state::irq_539x_1_w));
 			adapter.drq_handler_cb().set(*this, FUNC(macquadra_state::drq_539x_1_w));
 		});

src/mame/mips/mips.cpp

@@ -1031,8 +1031,8 @@ void rx3230_state::rx3230(machine_config &config)
 	m_rambo->parity_out().set_inputline(m_cpu, INPUT_LINE_IRQ5);
 	//m_rambo->buzzer_out().set(m_buzzer, FUNC(speaker_sound_device::level_w));
 	m_rambo->set_ram(m_ram);
-	m_rambo->dma_r<0>().set("scsi:7:ncr53c94", FUNC(ncr53c94_device::dma16_r));
+	m_rambo->dma_r<0>().set("scsi:7:ncr53c94", FUNC(ncr53c94_device::dma16_swap_r));
-	m_rambo->dma_w<0>().set("scsi:7:ncr53c94", FUNC(ncr53c94_device::dma16_w));
+	m_rambo->dma_w<0>().set("scsi:7:ncr53c94", FUNC(ncr53c94_device::dma16_swap_w));
 
 	// scsi bus and devices
 	NSCSI_BUS(config, m_scsibus);