SAM3/4: Some minor design improvements to the SAM3/4 serial driver

3 files changed, 51 insertions, 53 deletions

diff --git a/nuttx/arch/arm/src/sam34/chip/sam3u_uart.h b/nuttx/arch/arm/src/sam34/chip/sam3u_uart.h
index bf2dbff2d..86f48639e 100644
--- a/nuttx/arch/arm/src/sam34/chip/sam3u_uart.h
+++ b/nuttx/arch/arm/src/sam34/chip/sam3u_uart.h
@@ -319,6 +319,12 @@
 
 #define UART_INT_MANE                (1 << 24) /* Bit 24: Manchester Error Interrupt (USART only) */
 
+#if defined(CONFIG_ARCH_CHIP_SAM4S)
+#  define UART_INT_ALLINTS           0x010f3fff
+#else
+#  define UART_INT_ALLINTS           0x01083fff
+#endif
+
 /* UART Receiver Holding Register */
 
 #if 0
diff --git a/nuttx/arch/arm/src/sam34/sam_serial.c b/nuttx/arch/arm/src/sam34/sam_serial.c
index 4d28227c6..13867d65b 100644
--- a/nuttx/arch/arm/src/sam34/sam_serial.c
+++ b/nuttx/arch/arm/src/sam34/sam_serial.c
@@ -332,7 +332,6 @@ struct up_dev_s
 {
   uint32_t usartbase; /* Base address of USART registers */
   uint32_t baud;      /* Configured baud */
-  uint32_t imr;       /* Saved interrupt mask bits value */
   uint32_t sr;        /* Saved status bits */
   uint8_t  irq;       /* IRQ associated with this USART */
   uint8_t  parity;    /* 0=none, 1=odd, 2=even */
@@ -608,36 +607,14 @@ static inline void up_serialout(struct up_dev_s *priv, int offset, uint32_t valu
 }
 
 /****************************************************************************
- * Name: up_enableint
- ****************************************************************************/
-
-static inline void up_enableint(struct up_dev_s *priv)
-{
-  up_serialout(priv, SAM_UART_IER_OFFSET, priv->imr);
-}
-
-/****************************************************************************
- * Name: up_disableint
- ****************************************************************************/
-
-static inline void up_disableint(struct up_dev_s *priv)
-{
-  up_serialout(priv, SAM_UART_IDR_OFFSET, ~priv->imr);
-}
-
-/****************************************************************************
  * Name: up_restoreusartint
  ****************************************************************************/
 
-static void up_restoreusartint(struct up_dev_s *priv, uint32_t imr)
+static inline void up_restoreusartint(struct up_dev_s *priv, uint32_t imr)
 {
-  /* Save the interrupt mask */
-
-  priv->imr = imr;
+  /* Restore the previous interrupt state */
 
-  /* And re-enable interrrupts previoulsy disabled by up_disableallints */
-
-  up_enableint(priv);
+  up_serialout(priv, SAM_UART_IMR_OFFSET, imr);
 }
 
 /****************************************************************************
@@ -646,17 +623,22 @@ static void up_restoreusartint(struct up_dev_s *priv, uint32_t imr)
 
 static void up_disableallints(struct up_dev_s *priv, uint32_t *imr)
 {
+  irqstate_t flags;
+
+  /* The following must be atomic */
+
+  flags = irqsave();
   if (imr)
     {
       /* Return the current interrupt mask */
 
-      *imr = priv->imr;
+      *imr = up_serialin(priv, SAM_UART_IMR_OFFSET);
     }
 
   /* Disable all interrupts */
 
-  priv->imr = 0;
-  up_disableint(priv);
+  up_serialout(priv, SAM_UART_IDR_OFFSET, UART_INT_ALLINTS);
+  irqrestore(flags);
 }
 
 /****************************************************************************
@@ -855,6 +837,7 @@ static int up_interrupt(int irq, void *context)
   struct uart_dev_s *dev = NULL;
   struct up_dev_s   *priv;
   uint32_t           pending;
+  uint32_t           imr;
   int                passes;
   bool               handled;
 
@@ -917,8 +900,9 @@ static int up_interrupt(int irq, void *context)
 
       /* Get the UART/USART status (we are only interested in the unmasked interrupts). */
 
-      priv->sr = up_serialin(priv, SAM_UART_SR_OFFSET); /* Save for error reporting */
-      pending  = priv->sr & priv->imr;                  /* Mask out disabled interrupt sources */
+      priv->sr = up_serialin(priv, SAM_UART_SR_OFFSET);  /* Save for error reporting */
+      imr      = up_serialin(priv, SAM_UART_IMR_OFFSET); /* Interrupt mask */
+      pending  = priv->sr & imr;                        /* Mask out disabled interrupt sources */
 
       /* Handle an incoming, receive byte.  RXRDY: At least one complete character
        * has been received and US_RHR has not yet been read.
@@ -1033,14 +1017,12 @@ static void up_rxint(struct uart_dev_s *dev, bool enable)
        */
 
 #ifndef CONFIG_SUPPRESS_SERIAL_INTS
-      priv->imr |= UART_INT_RXRDY;
-      up_enableint(priv);
+      up_serialout(priv, SAM_UART_IER_OFFSET, UART_INT_RXRDY);
 #endif
     }
   else
     {
-      priv->imr &= ~UART_INT_RXRDY;
-      up_disableint(priv);
+      up_serialout(priv, SAM_UART_IDR_OFFSET, UART_INT_RXRDY);
     }
 }
 
@@ -1093,22 +1075,22 @@ static void up_txint(struct uart_dev_s *dev, bool enable)
        */
 
 #ifndef CONFIG_SUPPRESS_SERIAL_INTS
-      priv->imr |= UART_INT_TXRDY;
-      up_enableint(priv);
+      up_serialout(priv, SAM_UART_IER_OFFSET, UART_INT_TXRDY);
 
+#  if 0 /* Seems to be unnecessary */
       /* Fake a TX interrupt here by just calling uart_xmitchars() with
        * interrupts disabled (note this may recurse).
        */
 
       uart_xmitchars(dev);
+#  endif
 #endif
     }
   else
     {
       /* Disable the TX interrupt */
 
-      priv->imr &= ~UART_INT_TXRDY;
-      up_disableint(priv);
+      up_serialout(priv, SAM_UART_IDR_OFFSET, UART_INT_TXRDY);
     }
 
   irqrestore(flags);
diff --git a/nuttx/configs/sam3u-ek/README.txt b/nuttx/configs/sam3u-ek/README.txt
index 0d348aa6a..fa89991bd 100644
--- a/nuttx/configs/sam3u-ek/README.txt
+++ b/nuttx/configs/sam3u-ek/README.txt
@@ -38,22 +38,29 @@ GNU Toolchain Options
 
   All testing has been conducted using the NuttX buildroot toolchain.  However,
   the make system is setup to default to use the devkitARM toolchain.  To use
-  the CodeSourcery, devkitARM or Raisonance GNU toolchain, you simply need to
-  add one of the following configuration options to your .config (or defconfig)
-  file:
-
-    CONFIG_SAM34_CODESOURCERYW=y  : CodeSourcery under Windows
-    CONFIG_SAM34_CODESOURCERYL=y  : CodeSourcery under Linux
-    CONFIG_SAM34_DEVKITARM=y      : devkitARM under Windows
-    CONFIG_SAM34_BUILDROOT=y      : NuttX buildroot under Linux or Cygwin (default)
-
-  If you are not using CONFIG_SAM34_BUILDROOT, then you may also have to modify
+  the CodeSourcery, devkitARM, Atollic, or AtmelStudio GNU toolchain, you simply
+  need to add one of the following configuration options to your .config (or
+  defconfig) file:
+
+    CONFIG_ARMV7M_TOOLCHAIN_CODESOURCERYW=y  : CodeSourcery under Windows
+    CONFIG_ARMV7M_TOOLCHAIN_CODESOURCERYL=y  : CodeSourcery under Linux
+    CONFIG_ARMV7M_TOOLCHAIN_ATOLLIC=y        : Atollic toolchain for Windos
+    CONFIG_ARMV7M_TOOLCHAIN_DEVKITARM=y      : devkitARM under Windows
+    CONFIG_ARMV7M_TOOLCHAIN_BUILDROOT=y      : NuttX buildroot under Linux or Cygwin (default)
+    CONFIG_ARMV7M_TOOLCHAIN_GNU_EABIL=y      : Generic GCC ARM EABI toolchain for Linux
+    CONFIG_ARMV7M_TOOLCHAIN_GNU_EABIW=y      : Generic GCC ARM EABI toolchain for Windows
+
+  If you are not using CONFIG_ARMV7M_TOOLCHAIN_BUILDROOT, then you may also have to modify
   the PATH in the setenv.h file if your make cannot find the tools.
 
-  NOTE: the CodeSourcery (for Windows), devkitARM, and Raisonance toolchains are
-  Windows native toolchains.  The CodeSourcey (for Linux) and NuttX buildroot
-  toolchains are Cygwin and/or Linux native toolchains. There are several limitations
-  to using a Windows based toolchain in a Cygwin environment.  The three biggest are:
+  NOTE about Windows native toolchains
+  ------------------------------------
+
+  The CodeSourcery (for Windows), Atollic, and devkitARM toolchains are
+  Windows native toolchains.  The CodeSourcery (for Linux), NuttX buildroot,
+  and, perhaps, the generic GCC toolchains are Cygwin and/or Linux native
+  toolchains. There are several limitations to using a Windows based
+  toolchain in a Cygwin environment.  The three biggest are:
 
   1. The Windows toolchain cannot follow Cygwin paths.  Path conversions are
      performed automatically in the Cygwin makefiles using the 'cygpath' utility
@@ -462,6 +469,9 @@ Configurations
      use to set or PATH variable.  The path in the that file may not,
      however, be correct for your installation.
 
+     See also the "NOTE about Windows native toolchains" in the section call
+     "GNU Toolchain Options" above.
+
 Configuration sub-directories
 -----------------------------