From c64e2da46dea622b35350b8cd9fe38da4b265d1b Mon Sep 17 00:00:00 2001 From: AJR Date: Thu, 23 Feb 2023 22:49:48 -0500 Subject: [PATCH] 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 | 141 ++++++++++++++++++++----------- src/devices/machine/ncr53c90.h | 11 +++ src/mame/apple/macpdm.cpp | 2 +- src/mame/apple/macquadra700.cpp | 34 ++------ src/mame/mips/mips.cpp | 4 +- 5 files changed, 115 insertions(+), 77 deletions(-) diff --git a/src/devices/machine/ncr53c90.cpp b/src/devices/machine/ncr53c90.cpp index 64c74968ed9..3f635fa3b9b 100644 --- a/src/devices/machine/ncr53c90.cpp +++ b/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); - istatus = 0; - seq = 0; - } - check_irq(); - if(res) - command_pop_and_chain(); + if (irq) + { + status &= ~(S_GROSS_ERROR | S_PARITY | S_TCC); + istatus = 0; + seq = 0; + } + 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()); - if (irq) + //LOG("status_r %02x (%s)\n", res, machine().describe_context()); + 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]; - fifo_pos -= 2; - memmove(fifo, fifo + 2, fifo_pos); - - // update drq - if ((sync_offset != 0) || ((scsi_bus->ctrl_r() & S_PHASE_MASK) != S_PHASE_DATA_IN)) + u16 const data = fifo[0] | (fifo[1] << 8); + if (!machine().side_effects_disabled()) { - decrement_tcounter(2); - } - check_drq(); + fifo_pos -= 2; + memmove(fifo, fifo + 2, fifo_pos); - 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) - fatalerror("ncr53c94_device::dma16_w fifo_pos %d\n", fifo_pos); + if (fifo_pos > 14 || tcounter == 1) + { + 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 diff --git a/src/devices/machine/ncr53c90.h b/src/devices/machine/ncr53c90.h index 1d908b8eb09..3ceb3905265 100644 --- a/src/devices/machine/ncr53c90.h +++ b/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) diff --git a/src/mame/apple/macpdm.cpp b/src/mame/apple/macpdm.cpp index 0169f215b85..c1b079a083a 100644 --- a/src/mame/apple/macpdm.cpp +++ b/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); diff --git a/src/mame/apple/macquadra700.cpp b/src/mame/apple/macquadra700.cpp index 7bc6a1ab864..e0e674990ba 100644 --- a/src/mame/apple/macquadra700.cpp +++ b/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 m_floppy; required_device m_rtc; required_device m_scsibus1; - required_device m_ncr1; + required_device m_ncr1; required_device m_sonic; required_device m_screen; required_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); - } - else // pseudo-DMA: read from the FIFO - { - return m_ncr1->dma_r(); - } - - // never executed - return 0; + return m_ncr1->read(offset>>4); } void macquadra_state::mac_5396_w(offs_t offset, uint8_t data) { - if (offset < 0x100) - { - m_ncr1->write(offset>>4, data); - } - else // pseudo-DMA: write to the FIFO - { - m_ncr1->dma_w(data); - } + m_ncr1->write(offset>>4, 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(*device); + ncr53c96_device &adapter = downcast(*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)); }); diff --git a/src/mame/mips/mips.cpp b/src/mame/mips/mips.cpp index 7e5c6908c8a..b12757eddbb 100644 --- a/src/mame/mips/mips.cpp +++ b/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_w<0>().set("scsi:7:ncr53c94", FUNC(ncr53c94_device::dma16_w)); + 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_swap_w)); // scsi bus and devices NSCSI_BUS(config, m_scsibus);