I have discovered that the UART handling code in libbase, used by the
Milkymist BIOS, encounters a bug when the UART module responds too quickly
to a write. I doubt this bug is encountered on Milkymist, but it does occur
in my port of Milkymist to an Altera devkit. This is because, in my port I
have a UART module that can respond to a TX write within two sysclk cycles
due to an internal FIFO.

The bug is in uart_isr on line 65:

https://github.com/milkymist/milkymist/blob/530ae74293cf9f74621ce323af159ac887469056/software/libbase/uart.c#L65

I'll quote the necessary code here for convenience:
...



        if(stat & UART_STAT_TX_EVT) {



                if(tx_produce != tx_consume) {


                        CSR_UART_RXTX = tx_buf[tx_consume];   //////


                        tx_consume = (tx_consume + 1) & UART_RINGBUFFER_MASK_TX;



                } else



                        tx_cts = 1;



        }




        CSR_UART_STAT = stat;   //////

        irq_ack(IRQ_UART);

}


I've highlighted the offending lines. uart_isr, when applicable, will reset
CSR_UART_STAT *after *it has written new data to the UART module. If the
UART module is able to finish that request before uart_isr writes to
CSR_UART_STAT, then uart_isr will end up clearing the wrong event. This
prevents uart_isr from ever being called again, and stalls all outgoing
data.

Again, I doubt this occurs on Milkymist, but with my custom UART module it
does happen.

The fix is simple, and is available on my repo:

https://github.com/progranism/Open-Source-System-on-Chip-Experiment/blob/847a5750e7897b2d6572c7064c0df97f5677ccc5/software/libbase/uart.c#L51

I merely moved 'CSR_UART_STAT = stat;' to the top of the function.

Is this worthy of patching into the Milkymist repo? I could fork and submit
a patch/pull request if that helps.


~William
_______________________________________________
http://lists.milkymist.org/listinfo.cgi/devel-milkymist.org
IRC: #milkymist@Freenode

Reply via email to