[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