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);
  }

_______________________________________________
rtems-devel mailing list
rtems-devel@rtems.org
http://www.rtems.org/mailman/listinfo/rtems-devel

Reply via email to