SAMA5: Some improvements to the HSCMI card removal/insertion logic

6 files changed, 156 insertions, 65 deletions

diff --git a/nuttx/arch/arm/src/sam34/sam_hsmci.c b/nuttx/arch/arm/src/sam34/sam_hsmci.c
index b9c4e883a..42ae70d8f 100644
--- a/nuttx/arch/arm/src/sam34/sam_hsmci.c
+++ b/nuttx/arch/arm/src/sam34/sam_hsmci.c
@@ -2438,8 +2438,8 @@ static void sam_callback(void *arg)
         {
           /* Yes.. queue it */
 
-           fvdbg("Queuing callback to %p(%p)\n", priv->callback, priv->cbarg);
-          (void)work_queue(HPWORK, &priv->cbwork, (worker_t)priv->callback, priv->cbarg, 0);
+           fllvdbg("Queuing callback to %p(%p)\n", priv->callback, priv->cbarg);
+          (void)work_queue(LPWORK, &priv->cbwork, (worker_t)priv->callback, priv->cbarg, 0);
         }
       else
         {
@@ -2551,7 +2551,7 @@ void sdio_mediachange(FAR struct sdio_dev_s *dev, bool cardinslot)
       priv->cdstatus &= ~SDIO_STATUS_PRESENT;
     }
 
-  fvdbg("cdstatus OLD: %02x NEW: %02x\n", cdstatus, priv->cdstatus);
+  fllvdbg("cdstatus OLD: %02x NEW: %02x\n", cdstatus, priv->cdstatus);
 
   /* Perform any requested callback if the status has changed */
 
diff --git a/nuttx/arch/arm/src/sama5/sam_hsmci.c b/nuttx/arch/arm/src/sama5/sam_hsmci.c
index 3776f53c7..ae7350146 100644
--- a/nuttx/arch/arm/src/sama5/sam_hsmci.c
+++ b/nuttx/arch/arm/src/sama5/sam_hsmci.c
@@ -2704,8 +2704,8 @@ static void sam_callback(void *arg)
         {
           /* Yes.. queue it */
 
-           fvdbg("Queuing callback to %p(%p)\n", priv->callback, priv->cbarg);
-          (void)work_queue(HPWORK, &priv->cbwork, (worker_t)priv->callback, priv->cbarg, 0);
+           fllvdbg("Queuing callback to %p(%p)\n", priv->callback, priv->cbarg);
+          (void)work_queue(LPWORK, &priv->cbwork, (worker_t)priv->callback, priv->cbarg, 0);
         }
       else
         {
@@ -2931,7 +2931,7 @@ void sdio_mediachange(FAR struct sdio_dev_s *dev, bool cardinslot)
       priv->cdstatus &= ~SDIO_STATUS_PRESENT;
     }
 
-  fvdbg("cdstatus OLD: %02x NEW: %02x\n", cdstatus, priv->cdstatus);
+  fllvdbg("cdstatus OLD: %02x NEW: %02x\n", cdstatus, priv->cdstatus);
 
   /* Perform any requested callback if the status has changed */
 
diff --git a/nuttx/configs/sama5d3x-ek/README.txt b/nuttx/configs/sama5d3x-ek/README.txt
index 0bb288cca..abf0d29eb 100644
--- a/nuttx/configs/sama5d3x-ek/README.txt
+++ b/nuttx/configs/sama5d3x-ek/README.txt
@@ -1045,6 +1045,45 @@ Configurations
        Application Configuration -> NSH Library
          CONFIG_NSH_ARCHINIT=y                 : NSH board-initialization
 
+       Using the SD card:
+
+       1) If you try mounting an SD card with nothing in the slot, the
+          mount will fail:
+
+            nsh> mount -t vfat /dev/mmcsd1 /mnt/sd1
+            nsh: mount: mount failed: 19
+
+          NSH can be configured to provide errors as strings instead of
+          numbers.  But in this case, only the error number is reported.
+          The  error numbers can be found in nuttx/include/errno.h:
+
+            #define ENODEV              19
+            #define ENODEV_STR          "No such device"
+
+          So the mount command is saying that there is no device or, more
+          correctly, that there is no card in the SD card slot.
+
+        2) Inserted the SD card.  Then the mount should succeed.
+
+            nsh> mount -t vfat /dev/mmcsd1 /mnt/sd1
+            nsh> ls /mnt/sd1
+            /mnt/sd1:
+             atest.txt
+            nsh> cat /mnt/sd1/atest.txt
+            This is a test
+
+        3) Before removing the card, you must umount the file system.  This
+           is equivalent to "ejecting" or "safely removing" the card on
+           Windows:  It flushes any cached data to the card and makes the SD
+           card unavailable to the applications.
+
+             nsh> mount -t vfat /dev/mmcsd1 /mnt/sd1
+
+          It is now safe to remove the card.  NuttX provides into callbacks
+          that can be used by an application to automatically unmount the
+          volume when it is removed.  But those callbacks are not used in
+          this configuration.
+
     STATUS:
       2013-7-19:  This configuration (as do the others) run at 396MHz.
         The SAMA5D3 can run at 536MHz.  I still need to figure out the
@@ -1063,7 +1102,7 @@ Configurations
 
       2013-7-31:  Using delay loop calibration from the hello configuration.
         That configuration runs out of internal SRAM and, as a result, this
-        configuration needs to be recalibrated.
+        configuration should be recalibrated.
 
       2013-8-3:  SDRAM configuration and RAM test usage have been verified
         and are functional.  I note some issues; occassionally, SDRAM is
@@ -1080,6 +1119,18 @@ Configurations
 
       2013-8-10: Basic HSCMI1 functionality (with DMA) has been verified.
         Most testing is needed to assure that this is a stable solution.
+      2013-8-11: HSMCI0 is more finicky.  Usually there is no card
+        communcation and I get timeouts.  But if I remove and re-insert the
+        card it few times, sometimes communication is successfully and the
+        card behaves normally.  I suspected an electro-mechanical issue but
+        but now think there is more to the problem than that.
+      2013-8-11: I see another problem doing card insertion and card removal
+        testing.  When there is a lot of debug output, the system locks up.
+        I have traced to this the debug output itself.  The debug output
+        from the device driver interferes with normal serial port operation
+        and prevents NSH from receiving data.  There is no issue when the
+        debug output is suppressed and card insertial and removal works as
+        expected (at least on the HSMCI1 microSD slot).
 
   ostest:
     This configuration directory, performs a simple OS test using
diff --git a/nuttx/configs/sama5d3x-ek/src/sam_hsmci.c b/nuttx/configs/sama5d3x-ek/src/sam_hsmci.c
index d8d37bc76..62ea0fa7f 100644
--- a/nuttx/configs/sama5d3x-ek/src/sam_hsmci.c
+++ b/nuttx/configs/sama5d3x-ek/src/sam_hsmci.c
@@ -93,46 +93,45 @@
 /****************************************************************************
  * Private Types
  ****************************************************************************/
-/* This structure holds information unique to one HSMCI peripheral */
+/* This structure holds static information unique to one HSMCI peripheral */
 
-struct sam_hsmci_info_s
+struct sam_hsmci_state_s
 {
+  struct sdio_dev_s *hsmci;   /* R/W device handle */
   pio_pinset_t pincfg;        /* Card detect PIO pin configuratin */
   uint8_t irq;                /* Interrupt number (same as pid) */
+  uint8_t slotno;             /* Slot number */
+  bool cd;                    /* TRUE: card is inserted */
   xcpt_t handler;             /* Interrupt handler */
-  struct sdio_dev_s **hsmci;  /* R/W device handle */
 };
 
 /****************************************************************************
  * Private Data
  ****************************************************************************/
 
-/* Retained HSMCI driver handles for use by interrupt handlers */
-
-#ifdef CONFIG_SAMA5_HSMCI0
-static struct sdio_dev_s *g_hsmci0;
-#endif
-#ifdef CONFIG_SAMA5_HSMCI1
-static struct sdio_dev_s *g_hsmci1;
-#endif
-
-/* HSCMI device characteristics */
+/* HSCMI device state */
 
 #ifdef CONFIG_SAMA5_HSMCI0
 static int sam_hsmci0_cardetect(int irq, void *regs);
 
-static const struct sam_hsmci_info_s g_hsmci0_info =
+static struct sam_hsmci_state_s g_hsmci0 =
 {
-  PIO_MCI0_CD, IRQ_MCI0_CD, sam_hsmci0_cardetect, &g_hsmci0
+  .pincfg  = PIO_MCI0_CD,
+  .irq     = IRQ_MCI0_CD,
+  .slotno  = 0,
+  .handler = sam_hsmci0_cardetect,
 };
 #endif
 
 #ifdef CONFIG_SAMA5_HSMCI1
 static int sam_hsmci1_cardetect(int irq, void *regs);
 
-static const struct sam_hsmci_info_s g_hsmci1_info =
+static struct sam_hsmci_state_s g_hsmci1 =
 {
-  PIO_MCI1_CD, IRQ_MCI1_CD, sam_hsmci1_cardetect, &g_hsmci1
+  .pincfg  = PIO_MCI1_CD,
+  .irq     = IRQ_MCI1_CD,
+  .slotno  = 1,
+  .handler = sam_hsmci1_cardetect,
 };
 #endif
 
@@ -141,47 +140,86 @@ static const struct sam_hsmci_info_s g_hsmci1_info =
  ****************************************************************************/
 
 /****************************************************************************
- * Name: sam_hsmci0_cardetect and sam_hsmci1_cardetect
+ * Name: sam_cardinserted_internal
+ *
+ * Description:
+ *   Check if a card is inserted into the selected HSMCI slot
+ *
+ ****************************************************************************/
+
+bool sam_cardinserted_internal(struct sam_hsmci_state_s *state)
+{
+  bool inserted;
+
+  /* Get the state of the PIO pin */
+
+  inserted = sam_pioread(state->pincfg);
+  fllvdbg("Slot %d inserted: %s\n", state->slotno, inserted ? "NO" : "YES");
+  return !inserted;
+}
+
+/****************************************************************************
+ * Name: sam_hsmci_cardetect, sam_hsmci0_cardetect, and sam_hsmci1_cardetect
  *
  * Description:
  *   Card detect interrupt handlers
  *
  ****************************************************************************/
 
+static int sam_hsmci_cardetect(struct sam_hsmci_state_s *state)
+{
+  /* Get the current card insertion state */
+
+  bool cd = sam_cardinserted_internal(state);
+
+  /* Has the card detect state changed? */
+
+  if (cd != state->cd)
+    {
+      /* Yes... remember that new state and inform the HSMCI driver */
+
+      state->cd = cd;
+
+      /* Report the new state to the SDIO driver */
+
+      sdio_mediachange(state->hsmci, cd);
+    }
+
+   return OK;
+}
+
 #ifdef CONFIG_SAMA5_HSMCI0
 static int sam_hsmci0_cardetect(int irq, void *regs)
 {
-   sdio_mediachange(g_hsmci0, sam_cardinserted(0));
-   return OK;
+  return sam_hsmci_cardetect(&g_hsmci0);
 }
 #endif
 
 #ifdef CONFIG_SAMA5_HSMCI1
 static int sam_hsmci1_cardetect(int irq, void *regs)
 {
-   sdio_mediachange(g_hsmci1, sam_cardinserted(1));
-   return OK;
+  return sam_hsmci_cardetect(&g_hsmci1);
 }
 #endif
 
 /****************************************************************************
- * Name: sam_hsmci_info
+ * Name: sam_hsmci_state
  *
  * Description:
  *   Initialize HSMCI PIOs.
  *
  ****************************************************************************/
 
-static const struct sam_hsmci_info_s *sam_hsmci_info(int slotno)
+static struct sam_hsmci_state_s *sam_hsmci_state(int slotno)
 {
-   const struct sam_hsmci_info_s *info = NULL;
+  struct sam_hsmci_state_s *state = NULL;
 
 #ifdef CONFIG_SAMA5_HSMCI0
 #ifdef CONFIG_SAMA5_HSMCI1
   if (slotno == 0)
 #endif
     {
-      info = &g_hsmci0_info;
+      state = &g_hsmci0;
     }
 #ifdef CONFIG_SAMA5_HSMCI1
   else
@@ -190,11 +228,11 @@ static const struct sam_hsmci_info_s *sam_hsmci_info(int slotno)
 
 #ifdef CONFIG_SAMA5_HSMCI1
     {
-      info = &g_hsmci1_info;
+      state = &g_hsmci1;
     }
 #endif
 
-  return info;
+  return state;
 }
 
 /****************************************************************************
@@ -211,27 +249,27 @@ static const struct sam_hsmci_info_s *sam_hsmci_info(int slotno)
 
 int sam_hsmci_initialize(int slotno, int minor)
 {
-  const struct sam_hsmci_info_s *info;
+  struct sam_hsmci_state_s *state;
   int ret;
 
-  /* Get the HSMI description */
+  /* Get the static HSMI description */
 
-  info = sam_hsmci_info(slotno);
-  if (!info)
+  state = sam_hsmci_state(slotno);
+  if (!state)
     {
-      fdbg("No info for slotno %d\n", slotno);
+      fdbg("No state for slotno %d\n", slotno);
       return -EINVAL;
     }
 
   /* Initialize card-detect and write-protect PIOs */
 
-   sam_configpio(info->pincfg);
+  sam_configpio(state->pincfg);
 
   /* Mount the SDIO-based MMC/SD block driver */
   /* First, get an instance of the SDIO interface */
 
-  *info->hsmci = sdio_initialize(slotno);
-  if (!*info->hsmci)
+  state->hsmci = sdio_initialize(slotno);
+  if (!state->hsmci)
     {
       fdbg("Failed to initialize SDIO slot %d\n",  slotno);
       return -ENODEV;
@@ -239,7 +277,7 @@ int sam_hsmci_initialize(int slotno, int minor)
 
   /* Now bind the SDIO interface to the MMC/SD driver */
 
-  ret = mmcsd_slotinitialize(minor, *info->hsmci);
+  ret = mmcsd_slotinitialize(minor, state->hsmci);
   if (ret != OK)
     {
       fdbg("Failed to bind SDIO to the MMC/SD driver: %d\n", ret);
@@ -248,14 +286,17 @@ int sam_hsmci_initialize(int slotno, int minor)
 
   /* Configure card detect interrupts */
 
-  sam_pioirq(info->pincfg);
-  (void)irq_attach(info->irq, info->handler);
-  sam_pioirqenable(info->irq);
+  sam_pioirq(state->pincfg);
+  (void)irq_attach(state->irq, state->handler);
 
   /* Then inform the HSMCI driver if there is or is not a card in the slot. */
 
-   sdio_mediachange(*info->hsmci, sam_cardinserted(slotno));
+  state->cd = sam_cardinserted_internal(state);
+  sdio_mediachange(state->hsmci, state->cd);
+
+  /* Enable card detect interrupts */
 
+  sam_pioirqenable(state->irq);
   return OK;
 }
 
@@ -269,23 +310,20 @@ int sam_hsmci_initialize(int slotno, int minor)
 
 bool sam_cardinserted(int slotno)
 {
-  const struct sam_hsmci_info_s *info;
-  bool inserted;
+  struct sam_hsmci_state_s *state;
 
   /* Get the HSMI description */
 
-  info = sam_hsmci_info(slotno);
-  if (!info)
+  state = sam_hsmci_state(slotno);
+  if (!state)
     {
-      fdbg("No info for slotno %d\n", slotno);
+      fdbg("No state for slotno %d\n", slotno);
       return false;
     }
 
-  /* Get the state of the PIO pin */
+  /* Return the state of the PIO pin */
 
-  inserted = sam_pioread(info->pincfg);
-  fvdbg("Slot %d inserted: %s\n", slotno, inserted ? "NO" : "YES");
-  return !inserted;
+  return sam_cardinserted_internal(state);
 }
 
 /****************************************************************************
diff --git a/nuttx/drivers/mmcsd/mmcsd_sdio.c b/nuttx/drivers/mmcsd/mmcsd_sdio.c
index 6c9c01660..0ce842ae2 100644
--- a/nuttx/drivers/mmcsd/mmcsd_sdio.c
+++ b/nuttx/drivers/mmcsd/mmcsd_sdio.c
@@ -2231,6 +2231,7 @@ static void mmcsd_mediachange(FAR void *arg)
 
       SDIO_CALLBACKENABLE(priv->dev, SDIOMEDIA_INSERTED);
     }
+
   mmcsd_givesem(priv);
 }
 
diff --git a/nuttx/drivers/serial/serial.c b/nuttx/drivers/serial/serial.c
index 690ae2efd..d452b6b4b 100644
--- a/nuttx/drivers/serial/serial.c
+++ b/nuttx/drivers/serial/serial.c
@@ -289,7 +289,8 @@ static int uart_putxmitchar(FAR uart_dev_t *dev, int ch, bool oktoblock)
  * Name: uart_irqwrite
  ************************************************************************************/
 
-static inline ssize_t uart_irqwrite(FAR uart_dev_t *dev, FAR const char *buffer, size_t buflen)
+static inline ssize_t uart_irqwrite(FAR uart_dev_t *dev, FAR const char *buffer,
+                                    size_t buflen)
 {
   ssize_t ret = buflen;
 
@@ -321,9 +322,9 @@ static inline ssize_t uart_irqwrite(FAR uart_dev_t *dev, FAR const char *buffer,
 static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer,
                           size_t buflen)
 {
-  FAR struct inode *inode  = filep->f_inode;
-  FAR uart_dev_t   *dev    = inode->i_private;
-  ssize_t           nread  = buflen;
+  FAR struct inode *inode    = filep->f_inode;
+  FAR uart_dev_t   *dev      = inode->i_private;
+  ssize_t           nwritten = buflen;
   bool              oktoblock;
   int               ret;
   char              ch;
@@ -473,13 +474,13 @@ static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer,
            * interrupted transfer.
            */
 
-          if (buflen < nread)
+          if (buflen < nwritten)
             {
               /* Some data was transferred.  Return the number of bytes that
                * were successfully transferred.
                */
 
-              nread -= buflen;
+              nwritten -= buflen;
             }
           else
             {
@@ -487,7 +488,7 @@ static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer,
                * The VFS layer will set the errno value appropriately).
                */
  
-              nread = ret;
+              nwritten = ret;
             }
 
           break;
@@ -500,7 +501,7 @@ static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer,
     }
 
   uart_givesem(&dev->xmit.sem);
-  return nread;
+  return nwritten;
 }
 
 /************************************************************************************