[PATCH 1/3] bsp/leon3: Fix interrupt-driven console driver

Daniel Hellstrom daniel at gaisler.com
Fri Nov 29 13:35:54 UTC 2013


Hello,

I will try to go through and comment the patches next week. I have glanced them through and I have some comments.

Thanks,
Daniel


On 11/26/2013 09:19 AM, Sebastian Huber wrote:
> ---
>   c/src/lib/libbsp/sparc/leon3/console/console.c |  170 ++++++++++++++----------
>   1 files changed, 98 insertions(+), 72 deletions(-)
>
> diff --git a/c/src/lib/libbsp/sparc/leon3/console/console.c b/c/src/lib/libbsp/sparc/leon3/console/console.c
> index 1b1882d..0b30369 100644
> --- a/c/src/lib/libbsp/sparc/leon3/console/console.c
> +++ b/c/src/lib/libbsp/sparc/leon3/console/console.c
> @@ -85,7 +85,7 @@ static int uarts = 0;
>   #if CONSOLE_USE_INTERRUPTS
>   
>   /* Handle UART interrupts */
> -void console_isr(void *arg)
> +static void leon3_console_isr(void *arg)
>   {
>     struct apbuart_priv *uart = arg;
>     unsigned int status;
> @@ -100,13 +100,10 @@ void console_isr(void *arg)
>       rtems_termios_enqueue_raw_characters(uart->cookie, &data, 1);
>     }
>   
> -  if (status & LEON_REG_UART_STATUS_THE) {
> -    /* Sent the one char, we disable TX interrupts */
> -    uart->regs->ctrl &= ~LEON_REG_UART_CTRL_TI;
> -
> -    /* Tell close that we sent everything */
> -    uart->sending = 0;
> -
> +  if (
> +    (status & LEON_REG_UART_STATUS_THE)
> +      && (uart->regs->ctrl & LEON_REG_UART_CTRL_TI) != 0
> +  ) {
>       /* write_interrupt will get called from this function */
>       rtems_termios_dequeue_characters(uart->cookie, 1);
>     }
> @@ -116,27 +113,34 @@ void console_isr(void *arg)
>    *  Console Termios Write-Buffer Support Entry Point
>    *
>    */
> -int console_write_interrupt(int minor, const char *buf, int len)
> +static int leon3_console_write_support(int minor, const char *buf, size_t len)
>   {
> -  if (len > 0) {
> -    struct apbuart_priv *uart;
> -
> -    if (minor == 0)
> -      uart = &apbuarts[syscon_uart_index];
> -    else
> -      uart = &apbuarts[minor - 1];
> +  struct apbuart_priv *uart;
> +  int sending;
>   
> -    /* Remember what position in buffer */
> +  if (minor == 0)
> +    uart = &apbuarts[syscon_uart_index];
> +  else
> +    uart = &apbuarts[minor - 1];
>   
> -    /* Enable TX interrupt */
> +  if (len > 0) {
> +    /* Enable TX interrupt (interrupt is edge-triggered) */
>       uart->regs->ctrl |= LEON_REG_UART_CTRL_TI;
>   
>       /* start UART TX, this will result in an interrupt when done */
>       uart->regs->data = *buf;
>   
> -    uart->sending = 1;
> +    sending = 1;
> +  } else {
> +    /* No more to send, disable TX interrupts */
> +    uart->regs->ctrl &= ~LEON_REG_UART_CTRL_TI;
> +
> +    /* Tell close that we sent everything */
> +    sending = 0;
>     }
>   
> +  uart->sending = sending;
> +
>     return 0;
>   }
>   
> @@ -212,6 +216,11 @@ int console_set_attributes(int minor, const struct termios *t)
>     else
>       uart = &apbuarts[minor - 1];
>   
> +  /*
> +   * FIXME: This read-modify-write sequence is broken since interrupts may
> +   * interfere.
> +   */
> +
>     /* Read out current value */
>     ctrl = uart->regs->ctrl;
>   
> @@ -351,49 +360,19 @@ rtems_device_driver console_initialize(
>     return RTEMS_SUCCESSFUL;
>   }
>   
> -rtems_device_driver console_open(
> -  rtems_device_major_number major,
> -  rtems_device_minor_number minor,
> -  void                    * arg
> +#if CONSOLE_USE_INTERRUPTS
> +static struct rtems_termios_tty *leon3_console_get_tty(
> +  rtems_libio_open_close_args_t *args
>   )
>   {
> -  rtems_status_code sc;
> -  struct apbuart_priv *uart;
> -#if CONSOLE_USE_INTERRUPTS
> -  rtems_libio_open_close_args_t *priv = arg;
> -
> -  /* Interrupt mode routines */
> -  static const rtems_termios_callbacks Callbacks = {
> -    NULL,                        /* firstOpen */
> -    NULL,                        /* lastClose */
> -    NULL,                        /* pollRead */
> -    console_write_interrupt,     /* write */
> -    console_set_attributes,      /* setAttributes */
> -    NULL,                        /* stopRemoteTx */
> -    NULL,                        /* startRemoteTx */
> -    1                            /* outputUsesInterrupts */
> -  };
> -#else
> -  /* Polling mode routines */
> -  static const rtems_termios_callbacks Callbacks = {
> -    NULL,                        /* firstOpen */
> -    NULL,                        /* lastClose */
> -    console_pollRead,            /* pollRead */
> -    console_write_polled,        /* write */
> -    console_set_attributes,      /* setAttributes */
> -    NULL,                        /* stopRemoteTx */
> -    NULL,                        /* startRemoteTx */
> -    0                            /* outputUsesInterrupts */
> -  };
> +  return args->iop->data1;
> +}
>   #endif
>   
> -  assert(minor <= uarts);
> -  if (minor > uarts || minor == (syscon_uart_index + 1))
> -    return RTEMS_INVALID_NUMBER;
> -
> -  sc = rtems_termios_open(major, minor, arg, &Callbacks);
> -  if (sc != RTEMS_SUCCESSFUL)
> -    return sc;
> +static int leon3_console_first_open(int major, int minor, void *arg)
> +{
> +  struct apbuart_priv *uart;
> +  rtems_status_code sc;
>   
>     if (minor == 0)
>       uart = &apbuarts[syscon_uart_index];
> @@ -401,17 +380,15 @@ rtems_device_driver console_open(
>       uart = &apbuarts[minor - 1];
>   
>   #if CONSOLE_USE_INTERRUPTS
> -  if (priv && priv->iop)
> -    uart->cookie = priv->iop->data1;
> -  else
> -    uart->cookie = NULL;
> +  uart->cookie = leon3_console_get_tty(arg);
>   
>     /* Register Interrupt handler */
>     sc = rtems_interrupt_handler_install(uart->irq, "console",
> -                                       RTEMS_INTERRUPT_SHARED, console_isr,
> +                                       RTEMS_INTERRUPT_SHARED,
> +                                       leon3_console_isr,
>                                          uart);
>     if (sc != RTEMS_SUCCESSFUL)
> -    return sc;
> +    return -1;
>   
>     uart->sending = 0;
>     /* Enable Receiver and transmitter and Turn on RX interrupts */
> @@ -423,17 +400,15 @@ rtems_device_driver console_open(
>   #endif
>     uart->regs->status = 0;
>   
> -  return RTEMS_SUCCESSFUL;
> +  return 0;
>   }
>   
> -rtems_device_driver console_close(
> -  rtems_device_major_number major,
> -  rtems_device_minor_number minor,
> -  void                    * arg
> -)
> -{
>   #if CONSOLE_USE_INTERRUPTS
> +static int leon3_console_last_close(int major, int minor, void *arg)
> +{
> +  struct rtems_termios_tty *tty = leon3_console_get_tty(arg);
>     struct apbuart_priv *uart;
> +  rtems_interrupt_level level;
>   
>     if (minor == 0)
>       uart = &apbuarts[syscon_uart_index];
> @@ -441,7 +416,9 @@ rtems_device_driver console_close(
>       uart = &apbuarts[minor - 1];
>   
>     /* Turn off RX interrupts */
> +  rtems_termios_interrupt_lock_acquire(tty, level);
>     uart->regs->ctrl &= ~(LEON_REG_UART_CTRL_RI);
> +  rtems_termios_interrupt_lock_release(tty, level);
>   
>     /**** Flush device ****/
>     while (uart->sending) {
> @@ -449,8 +426,57 @@ rtems_device_driver console_close(
>     }
>   
>     /* uninstall ISR */
> -  rtems_interrupt_handler_remove(uart->irq, console_isr, uart);
> +  rtems_interrupt_handler_remove(uart->irq, leon3_console_isr, uart);
> +
> +  return 0;
> +}
>   #endif
> +
> +rtems_device_driver console_open(
> +  rtems_device_major_number major,
> +  rtems_device_minor_number minor,
> +  void                    * arg
> +)
> +{
> +#if CONSOLE_USE_INTERRUPTS
> +  /* Interrupt mode routines */
> +  static const rtems_termios_callbacks Callbacks = {
> +    leon3_console_first_open,    /* firstOpen */
> +    leon3_console_last_close,    /* lastClose */
> +    NULL,                        /* pollRead */
> +    leon3_console_write_support, /* write */
> +    console_set_attributes,      /* setAttributes */
> +    NULL,                        /* stopRemoteTx */
> +    NULL,                        /* startRemoteTx */
> +    1                            /* outputUsesInterrupts */
> +  };
> +#else
> +  /* Polling mode routines */
> +  static const rtems_termios_callbacks Callbacks = {
> +    leon3_console_first_open,    /* firstOpen */
> +    NULL,                        /* lastClose */
> +    console_pollRead,            /* pollRead */
> +    console_write_polled,        /* write */
> +    console_set_attributes,      /* setAttributes */
> +    NULL,                        /* stopRemoteTx */
> +    NULL,                        /* startRemoteTx */
> +    0                            /* outputUsesInterrupts */
> +  };
> +#endif
> +
> +  assert(minor <= uarts);
> +  if (minor > uarts || minor == (syscon_uart_index + 1))
> +    return RTEMS_INVALID_NUMBER;
> +
> +  return rtems_termios_open(major, minor, arg, &Callbacks);
> +}
> +
> +rtems_device_driver console_close(
> +  rtems_device_major_number major,
> +  rtems_device_minor_number minor,
> +  void                    * arg
> +)
> +{
>     return rtems_termios_close(arg);
>   }
>   




More information about the devel mailing list