From bfa500234d9041dd52b872b8d53fcf324f48eb3b Mon Sep 17 00:00:00 2001 From: slmnemo Date: Thu, 21 Jul 2022 20:35:46 -0700 Subject: [PATCH] Fixed UART bug related to parity and MSR/LSR --- pipelined/src/uncore/uartPC16550D.sv | 50 ++++++++++++++++------------ 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/pipelined/src/uncore/uartPC16550D.sv b/pipelined/src/uncore/uartPC16550D.sv index 524a63454..89bdc837d 100644 --- a/pipelined/src/uncore/uartPC16550D.sv +++ b/pipelined/src/uncore/uartPC16550D.sv @@ -165,6 +165,7 @@ module uartPC16550D( SCR <= #1 8'b0; // not strictly necessary to reset end else begin if (~MEMWb) begin + /* verilator lint_off CASEINCOMPLETE */ case (A) /* -----\/----- EXCLUDED -----\/----- 3'b000: if (DLAB) DLL <= #1 Din; // else TXHR <= #1 Din; // TX handled in TX register/FIFO section @@ -177,34 +178,40 @@ module uartPC16550D( // freq /baud / 16 = div //3'b000: if (DLAB) DLL <= #1 8'd38; //else TXHR <= #1 Din; // TX handled in TX register/FIFO section //3'b000: if (DLAB) DLL <= #1 8'd11; //else TXHR <= #1 Din; // TX handled in - 3'b000: if (DLAB) DLL <= #1 8'd8; //else TXHR <= #1 Din; // TX handled in + 3'b000: if (DLAB) DLL <= #1 8'd8; //else TXHR <= #1 Din; // TX handled in 3'b001: if (DLAB) DLM <= #1 8'b0; else IER <= #1 Din[3:0]; - 3'b010: FCR <= #1 {Din[7:6], 2'b0, Din[3], 2'b0, Din[0]}; // Write only FIFO Control Register; 4:5 reserved and 2:1 self-clearing 3'b011: LCR <= #1 Din; 3'b100: MCR <= #1 Din[4:0]; - 3'b101: LSR[6:1] <= #1 Din[6:1]; // recommended only for test, see 8.6.3 - 3'b110: MSR <= #1 Din[3:0]; 3'b111: SCR <= #1 Din; endcase + /* verilator lint_on CASEINCOMPLETE */ end - + // Line Status Register (8.6.3) - // Ben 6/9/21 I don't like how this is a register. A lot of the individual bits have clocked components, so this just adds unecessary delay. - LSR[0] <= #1 rxdataready; // Data ready - LSR[1] <= #1 (LSR[1] | RXBR[10]) & ~squashRXerrIP;; // overrun error - LSR[2] <= #1 (LSR[2] | RXBR[9]) & ~squashRXerrIP; // parity error - LSR[3] <= #1 (LSR[3] | RXBR[8]) & ~squashRXerrIP; // framing error - LSR[4] <= #1 (LSR[4] | rxbreak) & ~squashRXerrIP; // break indicator - LSR[5] <= #1 THRE; // THRE - LSR[6] <= #1 ~txsrfull & THRE; // TEMT - if (rxfifohaserr) LSR[7] <= #1 1; // any bits in FIFO have error + // Ben 6/9/21 I don't like how this is a register. A lot of the individual bits have clocked components, so this just adds unecessary delay. + if (~MEMWb & (A == 3'b101)) + LSR[6:1] <= #1 Din[6:1]; // recommended only for test, see 8.6.3 + else begin + LSR[0] <= #1 rxdataready; // Data ready + LSR[1] <= #1 (LSR[1] | RXBR[10]) & ~squashRXerrIP;; // overrun error + LSR[2] <= #1 (LSR[2] | RXBR[9]) & ~squashRXerrIP; // parity error + LSR[3] <= #1 (LSR[3] | RXBR[8]) & ~squashRXerrIP; // framing error + LSR[4] <= #1 (LSR[4] | rxbreak) & ~squashRXerrIP; // break indicator + LSR[5] <= #1 THRE; // THRE + LSR[6] <= #1 ~txsrfull & THRE; // TEMT + if (rxfifohaserr) LSR[7] <= #1 1; // any bits in FIFO have error + end // Modem Status Register (8.6.8) - MSR[0] <= #1 MSR[0] | CTSb2 ^ CTSbsync; // Delta Clear to Send - MSR[1] <= #1 MSR[1] | DSRb2 ^ DSRbsync; // Delta Data Set Ready - MSR[2] <= #1 MSR[2] | (~RIb2 & RIbsync); // Trailing Edge of Ring Indicator - MSR[3] <= #1 MSR[3] | DCDb2 ^ DCDbsync; // Delta Data Carrier Detect + if (~MEMWb & (A == 3'b110)) + MSR <= #1 Din[3:0]; + else begin + MSR[0] <= #1 MSR[0] | CTSb2 ^ CTSbsync; // Delta Clear to Send + MSR[1] <= #1 MSR[1] | DSRb2 ^ DSRbsync; // Delta Data Set Ready + MSR[2] <= #1 MSR[2] | (~RIb2 & RIbsync); // Trailing Edge of Ring Indicator + MSR[3] <= #1 MSR[3] | DCDb2 ^ DCDbsync; // Delta Data Carrier Detect + end end always_comb if (~MEMRb) @@ -215,7 +222,8 @@ module uartPC16550D( 3'b011: Dout = LCR; 3'b100: Dout = {3'b000, MCR}; 3'b101: Dout = LSR; - 3'b110: Dout = {~CTSbsync, ~DSRbsync, ~RIbsync, ~DCDbsync, MSR[3:0]}; + // 3'b110: Dout = {~CTSbsync, ~DSRbsync, ~RIbsync, ~DCDbsync, MSR[3:0]}; + 3'b110: Dout = {~DCDbsync, ~RIbsync, ~DSRbsync, ~CTSbsync, MSR[3:0]}; 3'b111: Dout = SCR; endcase else Dout = 8'b0; @@ -304,7 +312,7 @@ module uartPC16550D( // ERROR CONDITIONS assign rxparity = ^rxdata; - assign rxparityerr = rxparity ^ rxparitybit ^ ~evenparitysel; // Check even/odd parity (*** check if LCR needs to be inverted) + assign rxparityerr = (rxparity ^ rxparitybit ^ ~evenparitysel) & LCR[3]; // Check even/odd parity (*** check if LCR needs to be inverted) assign rxoverrunerr = fifoenabled ? (rxfifoentries == 15) : rxdataready; // overrun if FIFO or receive buffer register full assign rxframingerr = ~rxstopbit; // framing error if no stop bit assign rxbreak = rxframingerr & (rxdata9 == 9'b0); // break when 0 for start + data + parity + stop time @@ -405,7 +413,7 @@ module uartPC16550D( txstate <= #1 UART_IDLE; end - assign txbitsexpected = 4'd1 + (4'd5 + {2'b00, LCR[1:0]}) + {3'b000, LCR[3]} + 4'd1 + {3'b000, LCR[2]} - 4'd1; // start bit + data bits + (parity bit) + stop bit(s) + assign txbitsexpected = 4'd1 + (4'd5 + {2'b00, LCR[1:0]}) + {3'b000, LCR[3]} + 4'd1 + {3'b000, LCR[2]} - 4'd1; // start bit + data bits + (parity bit) + stop bit(s) - 1 // *** explain; is this necessary? if (`QEMU) assign txnextbit = txbaudpulse & (txoversampledcnt[1:0] == 2'b00); // implies txstate = UART_ACTIVE else assign txnextbit = txbaudpulse & (txoversampledcnt == 4'b0000); // implies txstate = UART_ACTIVE