SAMD20: Some progress debugging SPI. Currently hangs

4 files changed, 102 insertions, 68 deletions

diff --git a/nuttx/arch/arm/src/samd/Kconfig b/nuttx/arch/arm/src/samd/Kconfig
index 281f51aec..c7e82921c 100644
--- a/nuttx/arch/arm/src/samd/Kconfig
+++ b/nuttx/arch/arm/src/samd/Kconfig
@@ -371,8 +371,8 @@ config SAMD_HAVE_SPI
 
 if SAMD_HAVE_SPI
 
-config SAMA5_SPI_REGDEBUG
-	bool "SPI egister-Level Debug"
+config SAMD_SPI_REGDEBUG
+	bool "SPI register-Level Debug"
 	default n
 	depends on DEBUG_SPI
 	---help---
diff --git a/nuttx/arch/arm/src/samd/chip/sam_spi.h b/nuttx/arch/arm/src/samd/chip/sam_spi.h
index f64be7c88..d38b19bd0 100644
--- a/nuttx/arch/arm/src/samd/chip/sam_spi.h
+++ b/nuttx/arch/arm/src/samd/chip/sam_spi.h
@@ -161,9 +161,6 @@
 #define SPI_CTRLA_FORM_MASK        (7 << SPI_CTRLA_FORM_SHIFT)
 #  define SPI_CTRLA_FORM_SPI       (0 << SPI_CTRLA_FORM_SHIFT) /* SPI frame (no address) */
 #  define SPI_CTRLA_FORM_ADDR      (2 << SPI_CTRLA_FORM_SHIFT) /* SPI frame (w/address) */
-#define SPI_CTRLA_CMODE            (1 << 28)  /* Bit 28: Communication mode */
-#  define SPI_CTRLA_ASYNCH         (0)
-#  define SPI_CTRLA_SYNCH          SPI_CTRLA_CMODE
 #define SPI_CTRLA_CPHA             (1 << 28)  /* Bit 28: Clock phase */
 #define SPI_CTRLA_CPOL             (1 << 29)  /* Bit 29: Clock polarity */
 #define SPI_CTRLA_DORD             (1 << 30)  /* Bit 30: Data order */
diff --git a/nuttx/arch/arm/src/samd/sam_sercom.c b/nuttx/arch/arm/src/samd/sam_sercom.c
index 81f0f3aec..608cca25a 100644
--- a/nuttx/arch/arm/src/samd/sam_sercom.c
+++ b/nuttx/arch/arm/src/samd/sam_sercom.c
@@ -79,7 +79,7 @@
  *   Configure the SERCOM core source clock.
  *
  *   Two generic clocks are used by the SERCOM: GCLK_SERCOMx_CORE and
- *   GCLK_SERCOMx_SLOW.  The core clock (GCLK_SERCOMx_CORE) is required to
+ *   GCLK_SERCOM_SLOW.  The core clock (GCLK_SERCOMx_CORE) is required to
  *   clock the SERCOM while operating as a master, while the slow clock
  *   (GCLK_SERCOM_SLOW) is only required for certain functions.  SERCOM
  *   modules must share the same slow GCLK channel ID.
@@ -136,7 +136,7 @@ void sercom_coreclk_configure(int sercom, int gclkgen, bool wrlock)
  *   Configure the SERCOM slow source clock.
  *
  *   Two generic clocks are used by the SERCOM: GCLK_SERCOMx_CORE and
- *   GCLK_SERCOMx_SLOW.  The core clock (GCLK_SERCOMx_CORE) is required to
+ *   GCLK_SERCOM_SLOW.  The core clock (GCLK_SERCOMx_CORE) is required to
  *   clock the SERCOM while operating as a master, while the slow clock
  *   (GCLK_SERCOM_SLOW) is only required for certain functions.  SERCOM
  *   modules must share the same slow GCLK channel ID.
@@ -145,38 +145,50 @@ void sercom_coreclk_configure(int sercom, int gclkgen, bool wrlock)
 
 void sercom_slowclk_configure(int gclkgen)
 {
+  static bool configured = false;
   uint16_t regval;
 
-  /* Set up the SERCOM_GCLK_ID_SLOW clock */
+  /* Since GCLK_SERCOM_SLOW is shard amongst all SERCOM modules, it should
+   * only be configured one time.
+   */
 
-  regval = (SERCOM_GCLK_ID_SLOW << GCLK_CLKCTRL_ID_SHIFT);
+  if (!configured)
+    {
+      /* Set up the SERCOM_GCLK_ID_SLOW clock */
 
-  /* Select and disable the SERCOM_GCLK_ID_SLOW generic clock */
+      regval = (SERCOM_GCLK_ID_SLOW << GCLK_CLKCTRL_ID_SHIFT);
 
-  putreg16(regval, SAM_GCLK_CLKCTRL);
+      /* Select and disable the SERCOM_GCLK_ID_SLOW generic clock */
 
-  /* Wait for clock to become disabled */
+      putreg16(regval, SAM_GCLK_CLKCTRL);
 
-  while ((getreg16(SAM_GCLK_CLKCTRL) & GCLK_CLKCTRL_CLKEN) != 0);
+      /* Wait for clock to become disabled */
 
-  /* Select the SERCOM_GCLK_ID_SLOW clock source generator */
+      while ((getreg16(SAM_GCLK_CLKCTRL) & GCLK_CLKCTRL_CLKEN) != 0);
 
-  regval |= (uint16_t)gclkgen << GCLK_CLKCTRL_GEN_SHIFT;
+      /* Select the SERCOM_GCLK_ID_SLOW clock source generator */
 
-  /* Write the new configuration */
+      regval |= (uint16_t)gclkgen << GCLK_CLKCTRL_GEN_SHIFT;
 
-  putreg16(regval, SAM_GCLK_CLKCTRL);
+      /* Write the new configuration */
 
-  /* Enable the GCLK_SERCOM_SLOW generic clock and lock further
-   * writes to this GCLK.  When this bit is written, it will lock
-   * further writes to the generic clock pointed by the CLKCTRL.ID. The
-   * generic clock generator pointed by CLKCTRL.GEN and the GENDIV.DIV
-   * will also be locked.
-   *
-   * We lock the SERCOM slow clock because it is common to all SERCOM modules
-   * and, once set, should not be changed again.
-   */
+      putreg16(regval, SAM_GCLK_CLKCTRL);
 
-  regval |= (GCLK_CLKCTRL_WRTLOCK | GCLK_CLKCTRL_CLKEN);
-  putreg16(regval, SAM_GCLK_CLKCTRL);
+      /* Enable the GCLK_SERCOM_SLOW generic clock and lock further
+       * writes to this GCLK.  When this bit is written, it will lock
+       * further writes to the generic clock pointed by the CLKCTRL.ID. The
+       * generic clock generator pointed by CLKCTRL.GEN and the GENDIV.DIV
+       * will also be locked.
+       *
+       * We lock the SERCOM slow clock because it is common to all SERCOM modules
+       * and, once set, should not be changed again.
+       */
+
+      regval |= (/* GCLK_CLKCTRL_WRTLOCK | */ GCLK_CLKCTRL_CLKEN);
+      putreg16(regval, SAM_GCLK_CLKCTRL);
+
+      /* Now we are configured */
+
+      configured = true;
+    }
 }
diff --git a/nuttx/arch/arm/src/samd/sam_spi.c b/nuttx/arch/arm/src/samd/sam_spi.c
index 18dc33bb5..46ea0f467 100644
--- a/nuttx/arch/arm/src/samd/sam_spi.c
+++ b/nuttx/arch/arm/src/samd/sam_spi.c
@@ -145,9 +145,9 @@ struct sam_spidev_s
   /* Debug stuff */
 
 #ifdef CONFIG_SAMD_SPI_REGDEBUG
-   bool     wrlast;            /* Last was a write */
-   uint32_t addresslast;       /* Last address */
-   uint32_t valuelast;         /* Last value */
+   bool     wr;                /* Last was a write */
+   uint32_t regaddr;           /* Last address */
+   uint32_t regval;            /* Last value */
    int      ntimes;            /* Number of times */
 #endif
 };
@@ -165,10 +165,8 @@ static bool     spi_checkreg(struct sam_spidev_s *priv, bool wr,
 # define        spi_checkreg(priv,wr,regval,regaddr) (false)
 #endif
 
-#if 0 /* Not used */
 static uint8_t  spi_getreg8(struct sam_spidev_s *priv,
                   unsigned int offset);
-#endif
 static void     spi_putreg8(struct sam_spidev_s *priv, uint8_t regval,
                   unsigned int offset);
 static uint16_t spi_getreg16(struct sam_spidev_s *priv,
@@ -231,6 +229,7 @@ static void     spi_recvblock(struct spi_dev_s *dev, void *buffer,
 
 /* Initialization */
 
+static void     spi_wait_synchronization(struct sam_spidev_s *priv);
 static void     spi_pad_configure(struct sam_spidev_s *priv);
 
 /****************************************************************************
@@ -577,9 +576,9 @@ static struct sam_spidev_s g_spi5dev =
 static bool spi_checkreg(struct sam_spidev_s *priv, bool wr, uint32_t regval,
                          uint32_t regaddr)
 {
-  if (wr      == priv->wrlast &&     /* Same kind of access? */
-      regval  == priv->valuelast &&  /* Same value? */
-      regaddr == priv->addresslast)  /* Same address? */
+  if (wr      == priv->wr &&      /* Same kind of access? */
+      regval  == priv->regval &&  /* Same value? */
+      regaddr == priv->regaddr)   /* Same address? */
     {
       /* Yes, then just keep a count of the number of times we did this. */
 
@@ -599,10 +598,10 @@ static bool spi_checkreg(struct sam_spidev_s *priv, bool wr, uint32_t regval,
 
       /* Save information about the new access */
 
-      priv->wrlast      = wr;
-      priv->valuelast   = regval;
-      priv->addresslast = regaddr;
-      priv->ntimes      = 0;
+      priv->wr      = wr;
+      priv->regval  = regval;
+      priv->regaddr = regaddr;
+      priv->ntimes  = 0;
     }
 
   /* Return true if this is the first time that we have done this operation */
@@ -619,11 +618,10 @@ static bool spi_checkreg(struct sam_spidev_s *priv, bool wr, uint32_t regval,
  *
  ****************************************************************************/
 
-#if 0 /* Not used */
 static uint8_t spi_getreg8(struct sam_spidev_s *priv, unsigned int offset)
 {
   uintptr_t regaddr = priv->base + offset;
-  uint8_t regval = getreg8(regaddr);
+  uint8_t regval    = getreg8(regaddr);
 
 #ifdef CONFIG_SAMD_SPI_REGDEBUG
   if (spi_checkreg(priv, false, (uint32_t)regval, regaddr))
@@ -634,7 +632,6 @@ static uint8_t spi_getreg8(struct sam_spidev_s *priv, unsigned int offset)
 
   return regval;
 }
-#endif
 
 /****************************************************************************
  * Name: spi_putreg8
@@ -670,7 +667,7 @@ static void spi_putreg8(struct sam_spidev_s *priv, uint8_t regval,
 static uint16_t spi_getreg16(struct sam_spidev_s *priv, unsigned int offset)
 {
   uintptr_t regaddr = priv->base + offset;
-  uint16_t regval = getreg16(regaddr);
+  uint16_t regval   = getreg16(regaddr);
 
 #ifdef CONFIG_SAMD_SPI_REGDEBUG
   if (spi_checkreg(priv, false, (uint32_t)regval, regaddr))
@@ -770,16 +767,16 @@ static void spi_putreg32(struct sam_spidev_s *priv, uint32_t regval,
 static void spi_dumpregs(struct sam_spidev_s *priv, const char *msg)
 {
   spivdbg("%s:\n", msg);
-  spivdbg("   CTRLA:%08x CTRLB:%08x DBGCTRL:%08x\n",
+  spivdbg("   CTRLA:%08x CTRLB:%08x DBGCTRL:%02x\n",
           getreg32(priv->base + SAM_SPI_CTRLA_OFFSET),
           getreg32(priv->base + SAM_SPI_CTRLB_OFFSET),
-          getreg32(priv->base + SAM_SPI_DBGCTRL_OFFSET));
-  spivdbg("    BAUD:%08x INTEN:%08x INTFLAG:%08x\n",
-          getreg32(priv->base + SAM_SPI_BAUD_OFFSET),
-          getreg32(priv->base + SAM_SPI_INTENCLR_OFFSET),
-          getreg32(priv->base + SAM_SPI_INTFLAG_OFFSET));
-  spivdbg("  STATUS:%08x  ADDR:%08x\n",
-          getreg32(priv->base + SAM_SPI_STATUS_OFFSET),
+          getreg8(priv->base + SAM_SPI_DBGCTRL_OFFSET));
+  spivdbg("    BAUD:%02x       INTEN:%02x       INTFLAG:%02x\n",
+          getreg8(priv->base + SAM_SPI_BAUD_OFFSET),
+          getreg8(priv->base + SAM_SPI_INTENCLR_OFFSET),
+          getreg8(priv->base + SAM_SPI_INTFLAG_OFFSET));
+  spivdbg("  STATUS:%04x      ADDR:%08x\n",
+          getreg16(priv->base + SAM_SPI_STATUS_OFFSET),
           getreg32(priv->base + SAM_SPI_ADDR_OFFSET));
 }
 #endif
@@ -807,8 +804,8 @@ static int spi_interrupt(struct sam_spidev_s *dev)
   * unmasked interrupts).
   */
 
-  intflag = sam_serialin8(priv, SAM_SPI_INTFLAG_OFFSET);
-  inten   = sam_serialin8(priv, SAM_SPI_INTENCLR_OFFSET);
+  intflag = sam_getreg8(priv, SAM_SPI_INTFLAG_OFFSET);
+  inten   = sam_getreg8(priv, SAM_SPI_INTENCLR_OFFSET);
   pending  = intflag & inten;
 
   /* Handle an incoming, receive byte.  The RXC flag is set when there is
@@ -962,6 +959,7 @@ static uint32_t spi_setfrequency(struct spi_dev_s *dev, uint32_t frequency)
   uint32_t maxfreq;
   uint32_t actual;
   uint32_t baud;
+  uint32_t ctrla;
 
   spivdbg("sercom=%d frequency=%d\n", priv->sercom, frequency);
 
@@ -1009,7 +1007,33 @@ static uint32_t spi_setfrequency(struct spi_dev_s *dev, uint32_t frequency)
       baud = 255;
     }
 
-  spi_putreg8(priv, (uint8_t)baud, SAM_SPI_BAUD_OFFSET);
+  /* Momentarily disable SPI while we apply the new BAUD setting (if it was
+   * previously enabled)
+   */
+
+  ctrla = spi_getreg32(priv, SAM_SPI_CTRLA_OFFSET);
+  if ((ctrla & SPI_CTRLA_ENABLE) != 0)
+    {
+      /* Disable SPI.. waiting for synchronization */
+
+      spi_putreg32(priv, ctrla & ~SPI_CTRLA_ENABLE, SAM_SPI_CTRLA_OFFSET);
+      spi_wait_synchronization(priv);
+
+      /* Set the new BAUD value */
+
+      spi_putreg8(priv, (uint8_t)baud, SAM_SPI_BAUD_OFFSET);
+
+      /* Re-enable SPI.. waiting for synchronization */
+
+      spi_putreg32(priv, ctrla, SAM_SPI_CTRLA_OFFSET);
+      spi_wait_synchronization(priv);
+    }
+  else
+    {
+      /* Set the new BAUD when the SPI is already disabled */
+
+      spi_putreg8(priv, (uint8_t)baud, SAM_SPI_BAUD_OFFSET);
+    }
 
   /* Calculate the new actual frequency */
 
@@ -1189,6 +1213,9 @@ static uint16_t spi_send(struct spi_dev_s *dev, uint16_t wd)
  * Returned Value:
  *   None
  *
+ * Assumptions/Limitations:
+ *   Data must be 16-bit aligned in 9-bit data transfer mode.
+ *
  ****************************************************************************/
 
 static void spi_exchange(struct spi_dev_s *dev, const void *txbuffer,
@@ -1270,15 +1297,15 @@ static void spi_exchange(struct spi_dev_s *dev, const void *txbuffer,
        * transferred to the serializer.
        */
 
-      while ((spi_getreg32(priv, SAM_SPI_INTFLAG_OFFSET) & SPI_INT_DRE) == 0);
+      while ((spi_getreg8(priv, SAM_SPI_INTFLAG_OFFSET) & SPI_INT_DRE) == 0);
 
       /* Write the data to transmitted to the DATA Register (TDR) */
 
-      spi_putreg32(priv, data, SAM_SPI_DATA_OFFSET);
+      spi_putreg16(priv, data, SAM_SPI_DATA_OFFSET);
 
       /* Wait for the read data to be available in the DATA register. */
 
-      while ((spi_getreg32(priv, SAM_SPI_INTFLAG_OFFSET) & SPI_INT_RXC) == 0);
+      while ((spi_getreg8(priv, SAM_SPI_INTFLAG_OFFSET) & SPI_INT_RXC) == 0);
 
       /* Check for data overflow.  The BUFOVF bit provides the status of the
        * next DATA to be read.  On buffer overflow, the corresponding DATA
@@ -1377,7 +1404,7 @@ static void spi_recvblock(struct spi_dev_s *dev, void *buffer, size_t nwords)
 
 static void spi_wait_synchronization(struct sam_spidev_s *priv)
 {
-  while ((getreg16(priv->base + SAM_SPI_STATUS_OFFSET) & SPI_STATUS_SYNCBUSY) != 0);
+  while ((spi_getreg16(priv, SAM_SPI_STATUS_OFFSET) & SPI_STATUS_SYNCBUSY) != 0);
 }
 
 /****************************************************************************
@@ -1506,11 +1533,11 @@ struct spi_dev_s *up_spiinitialize(int port)
   sercom_coreclk_configure(priv->sercom, priv->gclkgen, false);
   sercom_slowclk_configure(BOARD_SERCOM_SLOW_GCLKGEN);
 
-  /* Set the SERCOM in SPI master mode */
+  /* Set the SERCOM in SPI master mode (no address) */
 
   regval  = spi_getreg32(priv, SAM_SPI_CTRLA_OFFSET);
   regval &= ~SPI_CTRLA_MODE_MASK;
-  regval |= SPI_CTRLA_MODE_MASTER;
+  regval |= (SPI_CTRLA_MODE_MASTER | SPI_CTRLA_FORM_SPI);
   spi_putreg32(priv, regval, SAM_SPI_CTRLA_OFFSET);
 
   /* Configure pads */
@@ -1528,24 +1555,22 @@ struct spi_dev_s *up_spiinitialize(int port)
    */
 
   regval = (SPI_CTRLA_MSBFIRST | priv->muxconfig);
-  spi_putreg8(priv, regval, SAM_SPI_CTRLA_OFFSET);
+  spi_putreg32(priv, regval, SAM_SPI_CTRLA_OFFSET);
 
   /* Enable the receiver.  Note that 8-bit data width is assumed initially */
 
   regval = (SPI_CTRLB_RXEN | SPI_CTRLB_CHSIZE_8BITS);
-  spi_putreg8(priv, regval, SAM_SPI_CTRLB_OFFSET);
+  spi_putreg32(priv, regval, SAM_SPI_CTRLB_OFFSET);
+  spi_wait_synchronization(priv);
 
   priv->nbits = 8;
 
-  /* Wait until the synchronization is complete */
-
-  spi_wait_synchronization(priv);
-
   /* Enable SPI */
 
-  regval = spi_getreg32(priv, SAM_SPI_CTRLA_OFFSET);
+  regval  = spi_getreg32(priv, SAM_SPI_CTRLA_OFFSET);
   regval |= SPI_CTRLA_ENABLE;
   spi_putreg32(priv, regval, SAM_SPI_CTRLA_OFFSET);
+  spi_wait_synchronization(priv);
 
   /* Disable all interrupts at the SPI source and clear all pending
    * status that we can.