Minor updates for PIC32 USB device driver bugs

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@4497 42af7a65-404d-4744-a932-0658087f49c3

8 files changed, 85 insertions, 18 deletions

diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog
index 48efd5a46..229326e99 100644
--- a/nuttx/ChangeLog
+++ b/nuttx/ChangeLog
@@ -2578,4 +2578,4 @@
 	* arch/mips/src/pic32mx/picm32mx-usbdev.c:  Several stall-related fixes so that
 	  the USB device driver can used the the mass storage class (which does a LOT
 	  of stalling as part of its normal protocol).  The PIC32 USB Mass Storage
-	  device now seems functional (but has not been tested exhaustively).
+	  device is, however, still non-functional when debug is OFF.
diff --git a/nuttx/TODO b/nuttx/TODO
index d20309ae1..0baac17ab 100644
--- a/nuttx/TODO
+++ b/nuttx/TODO
@@ -1,4 +1,4 @@
-NuttX TODO List (Last updated February 21, 2012)
+NuttX TODO List (Last updated March 19, 2012)
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 This file summarizes known NuttX bugs, limitations, inconsistencies with 
@@ -35,7 +35,7 @@ nuttx/
   (3)  AVR (arch/avr)
   (0)  Intel x86 (arch/x86)
   (4)  8051 / MCS51 (arch/8051/)
-  (0)  MIPS/PIC32 (arch/mips)
+  (1)  MIPS/PIC32 (arch/mips)
   (1)  Hitachi/Renesas SH-1 (arch/sh/src/sh1)
   (4)  Renesas M16C/26 (arch/sh/src/m16c)
  (10)  z80/z8/ez80 (arch/z80/)
@@ -1253,8 +1253,37 @@ o 8051 / MCS51 (arch/8051/)
   Status:      Open
   Priority:    Low -- only because there as so many other issues with 8051
 
-o MIPS (arch/mips)
-  ^^^^^^^^^^^^^^^^
+o MIPS/PIC32(arch/mips)
+  ^^^^^^^^^^^^^^^^^^^^^
+
+  Title:       PIC32 USB DRIVER DOES NOT WORK WITH MASS STORAGE CLASS
+  Description: The PIC32 USB driver either crashes or hangs when used with
+               the mass storage class when trying to write files to the target
+               storage device.  This usually works with debug on, but does not
+               work with debug OFF (implying some race condition?)
+
+               Here are some details of what I see in debugging:
+
+               1. The USB MSC device completes processing of a read request
+                  and returns the read request to the driver.
+               2. Before the MSC device can even begin the wait for the next
+                  driver, many packets come in at interrupt level.  The MSC
+                  device goes to sleep (on pthread_cond_wait) with all of the
+                  read buffers ready (16 in my test case).
+               3. The pthread_cond_wait() does not wake up.  This implies
+                  a problem with pthread_con_wait(?).  But in other cases,
+                  the MSC device does wake up, but then immediately crashes
+                  because its stack is bad.
+               4. If I force the pthread_cond_wait to wake up (by using
+                  pthread_cond_timedwait instead), then the thread wakes
+                  up and crashes with a bad stack.
+
+               So far, I have no clue why this is failing.
+  Status:      Open
+  Priority:    High
+
+
+               
 
 o Hitachi/Renesas SH-1 (arch/sh/src/sh1)
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/nuttx/arch/mips/src/pic32mx/pic32mx-usbdev.c b/nuttx/arch/mips/src/pic32mx/pic32mx-usbdev.c
index 9998ca442..79a5bfaa8 100644
--- a/nuttx/arch/mips/src/pic32mx/pic32mx-usbdev.c
+++ b/nuttx/arch/mips/src/pic32mx/pic32mx-usbdev.c
@@ -1013,7 +1013,7 @@ static void pic32mx_rqrestart(int argc, uint32_t arg1, ...)
           privep->stalled   = false;
           privep->txnullpkt = false;
 
-          /* Check the request at the head of the endpoint's active request queue */
+          /* Check the request at the head of the endpoint's pending request queue */
 
           privreq = pic32mx_rqhead(&privep->pend);
           if (privreq)
@@ -1646,6 +1646,7 @@ static int pic32mx_rdrequest(struct pic32mx_usbdev_s *priv,
   if (ret == OK)
     {
       privreq = pic32mx_remfirst(&privep->pend);
+      DEBUGASSERT(privreq != NULL);
       pic32mx_addlast(&privep->active, privreq);
     }
   return ret;
diff --git a/nuttx/configs/pic32-starterkit/README.txt b/nuttx/configs/pic32-starterkit/README.txt
index fb693f54e..db3886343 100644
--- a/nuttx/configs/pic32-starterkit/README.txt
+++ b/nuttx/configs/pic32-starterkit/README.txt
@@ -1171,4 +1171,30 @@ Where <subdir> is one of the following:
     little testing I have done with it, it appears functional.  But the
     logic has not been stressed and there could still be lurking issues.
  
-
+    Update. The following was added to the top-level TODO list:
+
+    Title:       PIC32 USB DRIVER DOES NOT WORK WITH MASS STORAGE CLASS
+    Description: The PIC32 USB driver either crashes or hangs when used with
+                 the mass storage class when trying to write files to the target
+                 storage device.  This usually works with debug on, but does not
+                 work with debug OFF (implying some race condition?)
+
+                 Here are some details of what I see in debugging:
+
+                 1. The USB MSC device completes processing of a read request
+                    and returns the read request to the driver.
+                 2. Before the MSC device can even begin the wait for the next
+                    driver, many packets come in at interrupt level.  The MSC
+                    device goes to sleep (on pthread_cond_wait) with all of the
+                    read buffers ready (16 in my test case).
+                 3. The pthread_cond_wait() does not wake up.  This implies
+                    a problem with pthread_con_wait(?).  But in other cases,
+                    the MSC device does wake up, but then immediately crashes
+                    because its stack is bad.
+                 4. If I force the pthread_cond_wait to wake up (by using
+                    pthread_cond_timedwait instead), then the thread wakes
+                    up and crashes with a bad stack.
+
+                 So far, I have no clue why this is failing.
+    Status:      Open
+    Priority:    High
diff --git a/nuttx/drivers/usbdev/usbmsc_scsi.c b/nuttx/drivers/usbdev/usbmsc_scsi.c
index fa443497f..ccc967618 100644
--- a/nuttx/drivers/usbdev/usbmsc_scsi.c
+++ b/nuttx/drivers/usbdev/usbmsc_scsi.c
@@ -1626,11 +1626,6 @@ static int usbmsc_idlestate(FAR struct usbmsc_dev_s *priv)
       req->priv     = privreq;
       req->callback = usbmsc_rdcomplete;
 
-      if (EP_SUBMIT(priv->epbulkout, req) != OK)
-        {
-          usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_IDLERDSUBMIT), (uint16_t)-ret);
-        }
-
       /* Change to the CMDPARSE state and return success */
 
       usbtrace(TRACE_CLASSSTATE(USBMSC_CLASSSTATE_IDLECMDPARSE), priv->cdb[0]);
@@ -1638,6 +1633,13 @@ static int usbmsc_idlestate(FAR struct usbmsc_dev_s *priv)
       ret = OK;
     }
 
+  /* In any event, return the request to be refilled */
+
+  if (EP_SUBMIT(priv->epbulkout, req) != OK)
+    {
+      usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_IDLERDSUBMIT), (uint16_t)-ret);
+    }
+
   return ret;
 }
 
diff --git a/nuttx/include/nuttx/usb/usbdev_trace.h b/nuttx/include/nuttx/usb/usbdev_trace.h
index 051b4ffa6..ae8e13c3a 100644
--- a/nuttx/include/nuttx/usb/usbdev_trace.h
+++ b/nuttx/include/nuttx/usb/usbdev_trace.h
@@ -1,8 +1,8 @@
 /****************************************************************************
  * include/nuttx/usb/usbdev_trace.h
  *
- *   Copyright (C) 2008, 2009-2010 Gregory Nutt. All rights reserved.
- *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
+ *   Copyright (C) 2008, 2009-2010, 2012 Gregory Nutt. All rights reserved.
+ *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
diff --git a/nuttx/sched/pthread_condsignal.c b/nuttx/sched/pthread_condsignal.c
index a10dda6be..8f96532df 100644
--- a/nuttx/sched/pthread_condsignal.c
+++ b/nuttx/sched/pthread_condsignal.c
@@ -1,8 +1,8 @@
 /****************************************************************************
  * sched/pthread_condsignal.c
  *
- *   Copyright (C) 2007-2009 Gregory Nutt. All rights reserved.
- *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
+ *   Copyright (C) 2007-2009, 2012 Gregory Nutt. All rights reserved.
+ *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -112,6 +112,15 @@ int pthread_cond_signal(FAR pthread_cond_t *cond)
 
       else
         {
+          /* One of my objectives in this design was to make pthread_cond_signal
+           * usable from interrupt handlers.  However, from interrupt handlers,
+           * you cannot take the associated mutex before signaling the condition.
+           * As a result, I think that there could be a race condition with
+           * the following logic which assumes that the if sval < 0 then the 
+           * thread is waiting.  Without the mutex, there is no atomic, protected
+           * operation that will guarantee this to be so.
+           */
+
           sdbg("sval=%d\n", sval);
           if (sval < 0)
             {
diff --git a/nuttx/sched/pthread_condwait.c b/nuttx/sched/pthread_condwait.c
index 38e3ddeb9..986349137 100644
--- a/nuttx/sched/pthread_condwait.c
+++ b/nuttx/sched/pthread_condwait.c
@@ -1,8 +1,8 @@
 /****************************************************************************
  * sched/pthread_condwait.c
  *
- *   Copyright (C) 2007-2009 Gregory Nutt. All rights reserved.
- *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
+ *   Copyright (C) 2007-2009, 2012 Gregory Nutt. All rights reserved.
+ *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions