SAMA5 ADC: Seems functional in all modes including DMA

ChangeLog

@@ -5900,4 +5900,8 @@
 	  is called as part of a failed pthread startup before the flags
 	  field in the TCB has been initialized, then a crash occurs.
 	  Pointed out by David Sidrane (2013-10-27)
+	* arch/arm/src/sama5/sam_adc.c:  ADC now works in all implemented
+	  modes:  single channel or multiple channel with sequencer support.
+	  software trigger or timer trigger;  ADC channel interrupts or
+	  DMA (2013-10-28).
 

arch/arm/src/sama5/Kconfig

@@ -1697,12 +1697,12 @@ config SAMA5_ADC_DMASAMPLES
 		The DMA logic uses ping-pong buffers, so the total buffering
 		requirement will be
 
-			2 Buffers * Number_of_ADC_Channels * SAMA5_ADC_DMASAMPLES * sizeof(uint32_t)
+			2 Buffers * Number_of_ADC_Channels * SAMA5_ADC_DMASAMPLES * sizeof(uint16_t)
 
 		So, for example, if you had 8 ADC channels and 8 triggers per DMA
 		transfer, then the total DMA buffering requirment would be:
 
-			2 * 8 * 8 * 4 = 512 bytes.
+			2 * 8 * 8 * 2 = 256 bytes.
 
 config SAMA5_ADC_AUTOCALIB
 	bool "ADC auto-calibration"

arch/arm/src/sama5/sam_adc.c

@@ -418,8 +418,8 @@ struct sam_adc_s
   /* DMA sample data buffer */
 
 #ifdef CONFIG_SAMA5_ADC_DMA
-  uint32_t evenbuf[SAMA5_ADC_SAMPLES];
+  uint16_t evenbuf[SAMA5_ADC_SAMPLES];
-  uint32_t oddbuf[SAMA5_ADC_SAMPLES];
+  uint16_t oddbuf[SAMA5_ADC_SAMPLES];
 #endif
 #endif /* SAMA5_ADC_HAVE_CHANNELS */
 
@@ -451,6 +451,7 @@ static void sam_adc_dmadone(void *arg);
 static void sam_adc_dmacallback(DMA_HANDLE handle, void *arg, int result);
 static int  sam_adc_dmasetup(struct sam_adc_s *priv, FAR uint8_t *buffer,
                              size_t buflen);
+static void sam_adc_dmastart(struct sam_adc_s *priv);
 #endif
 
 /* ADC interrupt handling */
@@ -609,7 +610,8 @@ static bool sam_adc_checkreg(struct sam_adc_s *priv, bool wr,
 static void sam_adc_dmadone(void *arg)
 {
   struct sam_adc_s *priv = (struct sam_adc_s *)arg;
-  uint32_t *buffer;
+  uint16_t *buffer;
+  uint16_t *next;
   uint16_t sample;
   int chan;
   int i;
@@ -617,20 +619,58 @@ static void sam_adc_dmadone(void *arg)
   avdbg("ready=%d enabled=%d\n", priv->enabled, priv->ready);
   ASSERT(priv != NULL && !priv->ready);
 
-  /* If the DMA is disabled, just ignore the data */
+  /* If the DMA transfer is not enabled, just ignore the data (and do not start
+   * the next DMA transfer).
+   */
 
-  if (!priv->enabled)
+  if (priv->enabled)
     {
-      /* Select the completed DMA buffer */
+      /* Toggle to the next buffer.
+       *
+       *   buffer - The buffer on which the DMA has just completed
+       *   next   - The buffer in which to start the next DMA
+       */
+
+      if (priv->odd)
+        {
+          buffer    = priv->oddbuf;
+          next      = priv->evenbuf;
+          priv->odd = false;
+        }
+      else
+        {
+          buffer    = priv->evenbuf;
+          next      = priv->oddbuf;
+          priv->odd = true;
+        }
+
+      /* Restart the DMA conversion as quickly as possible using the next
+       * buffer.
+       *
+       * REVISIT: In the original design, toggling the ping-pong buffers and
+       * restarting the DMA was done in the interrupt handler so that the
+       * next buffer could be filling while the current buffer is being
+       * processed here on the worker thread.  But, unfortunately,
+       * sam_adcm_dmasetup() cannot be called from an interrupt handler.
+       *
+       * A consequence of this is that there is a small window from the time
+       * that the last set of samples was taken, the worker thread runs, and
+       * the follow logic restarts the DMA in which samples could be lost!
+       *
+       * Without the interrupt level DMA restart logic, there is not really
+       * any good reason to support the ping-poing buffers at all.
+       */
+
+      sam_adc_dmasetup(priv, (FAR uint8_t *)next,
+                       SAMA5_ADC_SAMPLES * sizeof(uint16_t));
 
-      buffer = priv->odd ? priv->evenbuf : priv->oddbuf;
 
       /* Invalidate the DMA buffer so that we are guaranteed to reload the
        * newly DMAed data from RAM.
        */
 
       cp15_invalidate_dcache((uintptr_t)buffer,
-                             (uintptr_t)buffer + SAMA5_ADC_SAMPLES * sizeof(uint32_t));
+                             (uintptr_t)buffer + SAMA5_ADC_SAMPLES * sizeof(uint16_t));
 
       /* Process each sample */
 
@@ -639,7 +679,7 @@ static void sam_adc_dmadone(void *arg)
           /* Get the sample and the channel number */
 
           chan   = (int)((*buffer & ADC_LCDR_CHANB_MASK) >> ADC_LCDR_CHANB_SHIFT);
-          sample = (uint16_t)((*buffer & ADC_LCDR_DATA_MASK) >> ADC_LCDR_DATA_SHIFT);
+          sample = ((*buffer & ADC_LCDR_DATA_MASK) >> ADC_LCDR_DATA_SHIFT);
 
           /* And give the sample data to the ADC upper half */
 
@@ -653,6 +693,26 @@ static void sam_adc_dmadone(void *arg)
 }
 #endif
 
+/****************************************************************************
+ * Name: sam_adc_dmastart
+ *
+ * Description:
+ *   Initiate DMA sampling.
+ *
+ ****************************************************************************/
+
+static void sam_adc_dmastart(struct sam_adc_s *priv)
+{
+  /* Make sure that the worker is available and that DMA is not disabled */
+
+  if (priv->ready && priv->enabled)
+    {
+      priv->odd = false;  /* Start with the even buffer */
+      sam_adc_dmasetup(priv, (FAR uint8_t *)priv->evenbuf,
+                       SAMA5_ADC_SAMPLES * sizeof(uint16_t));
+    }
+}
+
 /****************************************************************************
  * Name: sam_adc_dmacallback
  *
@@ -670,22 +730,28 @@ static void sam_adc_dmacallback(DMA_HANDLE handle, void *arg, int result)
   int ret;
 
   allvdbg("ready=%d enabled=%d\n", priv->enabled, priv->ready);
+  DEBUGASSERT(priv->ready);
 
-  /* Check of the bottom half is keeping up with us */
+  /* Check of the bottom half is keeping up with us.
+   *
+   * ready == false:  Would mean that the worker thready has not ran since
+   *   the the last DMA callback.
+   * enabled == false: Means that the upper half has asked us nicely to stop
+   *   transferring DMA data.
+   */
 
   if (priv->ready && priv->enabled)
     {
-      /* Toggle to the next buffer.  Note that the toggle only occurs if
+      /* Verify that the worker is available */
-       * the bottom half is ready to accept more data.  Otherwise, we
-       * will get a data overrun and just re-use the last buffer.
-       */
-
-      priv->odd   = !priv->odd;
-      priv->ready = false;
-
-      /* Transfer processing to the bottom half */
 
       DEBUGASSERT(priv->work.worker == NULL);
+
+      /* Mark the work as busy and schedule the DMA done processing to
+       * occur on the worker thread.
+       */
+
+      priv->ready = false;
+
       ret = work_queue(HPWORK, &priv->work, sam_adc_dmadone, priv, 0);
       if (ret != 0)
         {
@@ -693,11 +759,20 @@ static void sam_adc_dmacallback(DMA_HANDLE handle, void *arg, int result)
         }
     }
 
-  /* Restart the DMA conversion using the next buffer */
+  /* REVISIT: There used to be logic here to toggle the ping-pong buffers and
-
+   * to restart the DMA conversion.  This would allow refilling one buffer
-  sam_adc_dmasetup(priv->dma,
+   * while the worker processes the other buffer that was just filled. But,
-                   priv->odd ? (void *)priv->oddbuf : (void *)priv->evenbuf,
+   * unfortunately, sam_adcm_dmasetup() and dma_rxsetup cannot be called
-                   SAMA5_ADC_SAMPLES * sizeof(uint32_t));
+   * from an interrupt handler.
+   *
+   * A consequence of this is that there is a small window from the time
+   * that the last set of samples was taken, the worker thread runs, and the
+   * logic on the worker thread restarts the DMA.  Samples trigger during
+   * this window will be be lost!
+   *
+   * Without this logic, there is not really any strong reason to support
+   * the ping-poing buffers at all.
+   */
 }
 #endif
 
@@ -1015,17 +1090,18 @@ static int sam_adc_setup(struct adc_dev_s *dev)
   sam_adc_autocalibrate(priv);
 
 #ifdef CONFIG_SAMA5_ADC_DMA
-  /* Configure for DMA transfer */
+  /* Initiate DMA transfers */
 
-  priv->odd     = false;
+  priv->ready   = true;   /* Worker is avaiable */
-  priv->ready   = true;
+  priv->enabled = true;   /* Transfers are enabled */
-  priv->enabled = false;
+
+  sam_adc_dmastart(priv);
 
-  sam_adc_dmasetup(priv, (void *)priv->evenbuf, SAMA5_ADC_SAMPLES);
 #else
   /* Enable end-of-conversion interrupts for all enabled channels. */
 
   sam_adc_putreg(priv, SAM_ADC_IER, SAMA5_CHAN_ENABLE);
+
 #endif
 
   /* Configure trigger mode and start conversion */
@@ -1048,11 +1124,12 @@ static void sam_adc_shutdown(struct adc_dev_s *dev)
 
   avdbg("Shutdown\n");
 
-  /* Disable ADC interrupts, both at the level of the ADC device and at the
+  /* Reset the ADC peripheral */
-   * level of the AIC.
+
-   */
+  sam_adc_reset(dev);
+
+  /* Disable ADC interrupts at the level of the AIC */
 
-  sam_adc_putreg(priv, SAM_ADC_IDR, ADC_INT_ALL);
   up_disable_irq(SAM_IRQ_ADC);
 
   /* Then detach the ADC interrupt handler. */
@@ -1075,10 +1152,21 @@ static void sam_adc_rxint(struct adc_dev_s *dev, bool enable)
   avdbg("enable=%d\n", enable);
 
 #ifdef CONFIG_SAMA5_ADC_DMA
-  /* We don't stop the DMA when RX is disabled, we just stop the data transfer */
+  /* Ignore redundant requests */
+
+  if (priv->enabled != enable)
+    {
+      /* Set a flag.  If disabling, the DMA sequence will terminate at the
+       * completion of the next DMA.
+       */
 
       priv->enabled = enable;
 
+      /* If enabling, then we need to restart the DMA transfer */
+
+      sam_adc_dmastart(priv);
+    }
+
 #else
   /* Are we enabling or disabling? */
 

configs/sama5d3x-ek/README.txt

@@ -794,6 +794,13 @@ SAMA5 ADC Support
 
     System Type -> ADC Configuration
       CONFIG_SAMA5_ADC_DMA=y           : Enable ADC DMA transfers
+      CONFIG_SAMA5_ADC_DMASAMPLES=2    : Collect two sets of samples per DMA
+
+    Drivers -> Analog device (ADC/DAC) support
+      CONFIG_ADC_FIFOSIZE=16           : Driver may need a large ring buffer
+
+    Application Configuration -> Examples -> ADC eample
+      CONFIG_EXAMPLES_ADC_GROUPSIZE=16 : Larger buffers in the test
 
 SAMA5D3x-EK Configuration Options
 =================================

drivers/analog/Kconfig

@@ -11,37 +11,50 @@ config ADC
 		not only Analog-to-Digital Converters (ADC) but also amplifiers and
 		analog multiplexers.
 
+if ADC
+
+config ADC_FIFOSIZE
+	int "ADC buffer size"
+	default 8
+	---help---
+		This variable defines the size of the ADC ring buffer that is used
+		to queue received ADC data until they can be retrieved by the
+		application by reading from the ADC character device.  NOTE:  Since
+		this is a ring buffer, the actual number of bytes that can be
+		retained in buffer is (ADC_FIFOSIZE - 1).
+
 config ADC_ADS125X
 	bool "TI ADS1255/ADS1256 support"
 	default n
-	depends on ADC
 	select SPI
 
+if ADC_ADS125X
+
 config ADS1255_FREQUENCY
 	int "ADS1255/ADS1256 SPI frequency"
 	default 1000000
-	depends on ADC_ADS125X
+
+endif # ADC_ADS125X
 
 config ADC_PGA11X
 	bool "TI PGA112/3/6/7 support"
 	default n
-	depends on ADC
 	select SPI
 	---help---
 		Enables support for the  PGA112, PGA113, PGA116, PGA117 Zerø-Drift
 		PROGRAMMABLE GAIN AMPLIFIER with MUX
 
+if ADC_PGA11X
+
 config PGA11X_SPIFREQUENCY
 	int "TI PGA112/3/6/7 SPI frequency"
 	default 1000000
-	depends on ADC_PGA11X
 	---help---
 		PGA11x SPI frequency.
 
 config PGA11X_SPIMODE
 	int "TI PGA112/3/6/7 SPI mode"
 	default 0
-	depends on ADC_PGA11X
 	---help---
 		PGA11x SPI mode.  The specification says that the device operates in Mode 0 or
 		Mode 3.  But sometimes you need to tinker with this to get things to work
@@ -50,28 +63,33 @@ config PGA11X_SPIMODE
 config PGA11X_DAISYCHAIN
 	bool "TI PGA112/3/6/7 daisy chain mode"
 	default n
-	depends on ADC_PGA11X
 	---help---
 		Enable support to use two PGA116/7's in Daisy Chain configuration.
 
 config PGA11X_MULTIPLE
 	bool "Multiple TI PGA112/3/6/7 support"
 	default n
-	depends on ADC_PGA11X && !PGA11X_DAISYCHAIN
+	depends on !PGA11X_DAISYCHAIN
 	---help---
 		Can be defined to support multiple PGA11X devices on board with separate
 		chip selects (not daisy chained).  Each device will require a customized
 		SPI interface to distinguish them when SPI_SELECT is called with
 		devid=SPIDEV_MUX.
 
+endif # if ADC_PGA11X
+endif # ADC
+
 config DAC
 	bool "Digital-to-Analog Conversion"
 	default n
 	---help---
 		Select to enable support for Digital-to-Analog Converters (DACs).
 
+if DAC
+
 config DAC_AD5410
 	bool "AD5410 support"
 	default n
-	depends on DAC
 	select SPI
+
+endif # DAC

drivers/analog/adc.c

@@ -158,6 +158,7 @@ static int adc_open(FAR struct file *filep)
 
       sem_post(&dev->ad_closesem);
     }
+
   return ret;
 }
 
@@ -207,6 +208,7 @@ static int adc_close(FAR struct file *filep)
           sem_post(&dev->ad_closesem);
         }
     }
+
   return ret;
 }
 

include/nuttx/analog/adc.h

@@ -61,7 +61,7 @@
  * Pre-processor Definitions
  ************************************************************************************/
 
-/* Default configuration settings that may be overridden in the board configuration.
+/* Default configuration settings that may be overridden in the NuttX configuration
  * file.  The configured size is limited to 255 to fit into a uint8_t.
  */