[PATCH rtems] arm/xilinx: Fix zynq-uart interrupt receive
Kinsey Moore
kinsey.moore at oarcorp.com
Wed Aug 18 17:41:29 UTC 2021
This is functional on the ZynqMP board I currently have setup for
testing and on ZynqMP QEMU except for the data corruption/loss caused by
the removal of the post-baud-set null write.
Unrelated to this patch, I just realized that zynq_uart_set_attributes
needs a call to zynq_uart_reset_tx_flush before adjusting any registers
to avoid data loss there.
One nit below.
Kinsey
On 8/17/2021 03:39, chrisj at rtems.org wrote:
> From: Chris Johns <chrisj at rtems.org>
>
> - Trigger on a single character entering the RX FIFO
>
> - Disable the RX timeout
>
> - Send up to a FIFO full of data
> ---
> bsps/include/dev/serial/zynq-uart.h | 1 +
> bsps/shared/dev/serial/zynq-uart-polled.c | 18 ++-----
> bsps/shared/dev/serial/zynq-uart.c | 66 +++++++++++------------
> 3 files changed, 36 insertions(+), 49 deletions(-)
>
> diff --git a/bsps/include/dev/serial/zynq-uart.h b/bsps/include/dev/serial/zynq-uart.h
> index 220d9b7717..b21e16f6de 100644
> --- a/bsps/include/dev/serial/zynq-uart.h
> +++ b/bsps/include/dev/serial/zynq-uart.h
> @@ -52,6 +52,7 @@ extern "C" {
> typedef struct {
> rtems_termios_device_context base;
> volatile struct zynq_uart *regs;
> + int tx_queued;
> bool transmitting;
> rtems_vector_number irq;
> } zynq_uart_context;
> diff --git a/bsps/shared/dev/serial/zynq-uart-polled.c b/bsps/shared/dev/serial/zynq-uart-polled.c
> index 95c51dea11..201d9070d3 100644
> --- a/bsps/shared/dev/serial/zynq-uart-polled.c
> +++ b/bsps/shared/dev/serial/zynq-uart-polled.c
> @@ -126,30 +126,18 @@ void zynq_uart_initialize(rtems_termios_device_context *base)
>
> zynq_cal_baud_rate(ZYNQ_UART_DEFAULT_BAUD, &brgr, &bauddiv, regs->mode);
>
> - regs->control &= ~(ZYNQ_UART_CONTROL_RXEN | ZYNQ_UART_CONTROL_TXEN);
> - regs->control = ZYNQ_UART_CONTROL_RXDIS
> - | ZYNQ_UART_CONTROL_TXDIS;
> + regs->control = ZYNQ_UART_CONTROL_RXDIS | ZYNQ_UART_CONTROL_TXDIS;
> regs->mode = ZYNQ_UART_MODE_CHMODE(ZYNQ_UART_MODE_CHMODE_NORMAL)
> | ZYNQ_UART_MODE_PAR(ZYNQ_UART_MODE_PAR_NONE)
> | ZYNQ_UART_MODE_CHRL(ZYNQ_UART_MODE_CHRL_8);
> regs->baud_rate_gen = ZYNQ_UART_BAUD_RATE_GEN_CD(brgr);
> regs->baud_rate_div = ZYNQ_UART_BAUD_RATE_DIV_BDIV(bauddiv);
> /* A Tx/Rx logic reset must be issued after baud rate manipulation */
> - regs->control = ZYNQ_UART_CONTROL_RXDIS
> - | ZYNQ_UART_CONTROL_TXDIS
> - | ZYNQ_UART_CONTROL_RXRES
> - | ZYNQ_UART_CONTROL_TXRES;
> + regs->control = ZYNQ_UART_CONTROL_RXRES | ZYNQ_UART_CONTROL_TXRES;
> regs->rx_fifo_trg_lvl = ZYNQ_UART_RX_FIFO_TRG_LVL_RTRIG(0);
> regs->rx_timeout = ZYNQ_UART_RX_TIMEOUT_RTO(0);
> regs->control = ZYNQ_UART_CONTROL_RXEN
> - | ZYNQ_UART_CONTROL_TXEN
> - | ZYNQ_UART_CONTROL_RSTTO;
> -
> - /*
> - * Some ZynqMP UARTs have a hardware bug that causes TX/RX logic restarts to
> - * require a kick after baud rate registers are initialized.
> - */
> - zynq_uart_write_polled(base, 0);
> + | ZYNQ_UART_CONTROL_TXEN;
> }
>
> int zynq_uart_read_polled(rtems_termios_device_context *base)
> diff --git a/bsps/shared/dev/serial/zynq-uart.c b/bsps/shared/dev/serial/zynq-uart.c
> index 8503e31d49..5906a015d9 100644
> --- a/bsps/shared/dev/serial/zynq-uart.c
> +++ b/bsps/shared/dev/serial/zynq-uart.c
> @@ -37,24 +37,25 @@ static void zynq_uart_interrupt(void *arg)
> rtems_termios_tty *tty = arg;
> zynq_uart_context *ctx = rtems_termios_get_device_context(tty);
> volatile zynq_uart *regs = ctx->regs;
> - uint32_t channel_sts;
>
> - if ((regs->irq_sts & (ZYNQ_UART_TIMEOUT | ZYNQ_UART_RTRIG)) != 0) {
> - regs->irq_sts = ZYNQ_UART_TIMEOUT | ZYNQ_UART_RTRIG;
> -
> - do {
> - char c = (char) ZYNQ_UART_TX_RX_FIFO_FIFO_GET(regs->tx_rx_fifo);
> -
> - rtems_termios_enqueue_raw_characters(tty, &c, 1);
> -
> - channel_sts = regs->channel_sts;
> - } while ((channel_sts & ZYNQ_UART_CHANNEL_STS_REMPTY) == 0);
> - } else {
> - channel_sts = regs->channel_sts;
> + if ((regs->irq_sts & ZYNQ_UART_RTRIG) != 0) {
> + char buf[32];
> + int c = 0;
> + regs->irq_sts = ZYNQ_UART_RTRIG;
> + while (c < sizeof(buf) &&
> + (regs->channel_sts & ZYNQ_UART_CHANNEL_STS_REMPTY) == 0) {
> + buf[c++] = (char) ZYNQ_UART_TX_RX_FIFO_FIFO_GET(regs->tx_rx_fifo);
> + }
> + rtems_termios_enqueue_raw_characters(tty, buf, c);
> }
>
> - if (ctx->transmitting && (channel_sts & ZYNQ_UART_CHANNEL_STS_TEMPTY) != 0) {
> - rtems_termios_dequeue_characters(tty, 1);
> + if (ctx->transmitting &&
> + (regs->channel_sts & ZYNQ_UART_CHANNEL_STS_TEMPTY) != 0) {
> + int sent = ctx->tx_queued;
> + regs->irq_dis = ZYNQ_UART_TEMPTY;
> + ctx->transmitting = false;
> + ctx->tx_queued = 0;
> + rtems_termios_dequeue_characters(tty, sent);
> }
> }
> #endif
> @@ -76,11 +77,10 @@ static bool zynq_uart_first_open(
> zynq_uart_initialize(base);
>
> #ifdef ZYNQ_CONSOLE_USE_INTERRUPTS
> - regs->rx_timeout = 32;
> - regs->rx_fifo_trg_lvl = ZYNQ_UART_FIFO_DEPTH / 2;
> + regs->rx_fifo_trg_lvl = 1;
> regs->irq_dis = 0xffffffff;
> regs->irq_sts = 0xffffffff;
> - regs->irq_en = ZYNQ_UART_RTRIG | ZYNQ_UART_TIMEOUT;
> + regs->irq_en = ZYNQ_UART_RTRIG;
> sc = rtems_interrupt_handler_install(
> ctx->irq,
> "UART",
> @@ -119,18 +119,22 @@ static void zynq_uart_write_support(
> zynq_uart_context *ctx = (zynq_uart_context *) base;
> volatile zynq_uart *regs = ctx->regs;
>
> + regs->irq_dis = ZYNQ_UART_TEMPTY;
> +
> if (len > 0) {
> - ctx->transmitting = true;
> + const char *p = &buf[0];
> regs->irq_sts = ZYNQ_UART_TEMPTY;
> + while (ctx->tx_queued < 32 && len > 0) {
Is 32 the size of the TX FIFO? It would be nice to see that as a
sizeof() or a #define for better context.
> + regs->tx_rx_fifo = ZYNQ_UART_TX_RX_FIFO_FIFO(*p);
> + ++p;
> + ++ctx->tx_queued;
> + --len;
> + }
> + ctx->transmitting = true;
> regs->irq_en = ZYNQ_UART_TEMPTY;
> - regs->tx_rx_fifo = ZYNQ_UART_TX_RX_FIFO_FIFO(buf[0]);
> - } else {
> - ctx->transmitting = false;
> - regs->irq_dis = ZYNQ_UART_TEMPTY;
> }
> #else
> ssize_t i;
> -
> for (i = 0; i < len; ++i) {
> zynq_uart_write_polled(base, buf[i]);
> }
> @@ -208,22 +212,16 @@ static bool zynq_uart_set_attributes(
> mode |= ZYNQ_UART_MODE_NBSTOP(ZYNQ_UART_MODE_NBSTOP_STOP_1);
> }
>
> - regs->control &= ~(ZYNQ_UART_CONTROL_RXEN | ZYNQ_UART_CONTROL_TXEN);
> + regs->control = ZYNQ_UART_CONTROL_RXDIS | ZYNQ_UART_CONTROL_TXDIS;
> regs->mode = mode;
> /* Ignore baud rate of B0. There are no modem control lines to de-assert */
> if (baud > 0) {
> regs->baud_rate_gen = ZYNQ_UART_BAUD_RATE_GEN_CD(brgr);
> regs->baud_rate_div = ZYNQ_UART_BAUD_RATE_DIV_BDIV(bauddiv);
> - regs->control |= ZYNQ_UART_CONTROL_RXRES
> - | ZYNQ_UART_CONTROL_TXRES;
> }
> - regs->control |= ZYNQ_UART_CONTROL_RXEN | ZYNQ_UART_CONTROL_TXEN;
> -
> - /*
> - * Some ZynqMP UARTs have a hardware bug that causes TX/RX logic restarts to
> - * require a kick after baud rate registers are initialized.
> - */
> - zynq_uart_write_polled(context, 0);
> + regs->control = ZYNQ_UART_CONTROL_RXRES | ZYNQ_UART_CONTROL_TXRES;
> + regs->irq_sts = 0xffffffff;
> + regs->control = ZYNQ_UART_CONTROL_RXEN | ZYNQ_UART_CONTROL_TXEN;
>
> return true;
> }
More information about the devel
mailing list