Fixed bug 3276379.

git-svn-id: svn://svn.code.sf.net/p/chibios/svn/trunk@2872 35acf78f-673a-0410-8e92-d51de3d6d3f4

os/hal/src/serial_usb.c

@@ -59,44 +59,44 @@ static cdc_linecoding_t linecoding = {
 
 static size_t writes(void *ip, const uint8_t *bp, size_t n) {
 
-  return chOQWriteTimeout(&((SerialDriver *)ip)->oqueue, bp,
+  return chOQWriteTimeout(&((SerialUSBDriver *)ip)->oqueue, bp,
                           n, TIME_INFINITE);
 }
 
 static size_t reads(void *ip, uint8_t *bp, size_t n) {
 
-  return chIQReadTimeout(&((SerialDriver *)ip)->iqueue, bp,
+  return chIQReadTimeout(&((SerialUSBDriver *)ip)->iqueue, bp,
                          n, TIME_INFINITE);
 }
 
 static bool_t putwouldblock(void *ip) {
 
-  return chOQIsFullI(&((SerialDriver *)ip)->oqueue);
+  return chOQIsFullI(&((SerialUSBDriver *)ip)->oqueue);
 }
 
 static bool_t getwouldblock(void *ip) {
 
-  return chIQIsEmptyI(&((SerialDriver *)ip)->iqueue);
+  return chIQIsEmptyI(&((SerialUSBDriver *)ip)->iqueue);
 }
 
 static msg_t putt(void *ip, uint8_t b, systime_t timeout) {
 
-  return chOQPutTimeout(&((SerialDriver *)ip)->oqueue, b, timeout);
+  return chOQPutTimeout(&((SerialUSBDriver *)ip)->oqueue, b, timeout);
 }
 
 static msg_t gett(void *ip, systime_t timeout) {
 
-  return chIQGetTimeout(&((SerialDriver *)ip)->iqueue, timeout);
+  return chIQGetTimeout(&((SerialUSBDriver *)ip)->iqueue, timeout);
 }
 
 static size_t writet(void *ip, const uint8_t *bp, size_t n, systime_t time) {
 
-  return chOQWriteTimeout(&((SerialDriver *)ip)->oqueue, bp, n, time);
+  return chOQWriteTimeout(&((SerialUSBDriver *)ip)->oqueue, bp, n, time);
 }
 
 static size_t readt(void *ip, uint8_t *bp, size_t n, systime_t time) {
 
-  return chIQReadTimeout(&((SerialDriver *)ip)->iqueue, bp, n, time);
+  return chIQReadTimeout(&((SerialUSBDriver *)ip)->iqueue, bp, n, time);
 }
 
 static ioflags_t getflags(void *ip) {
@@ -124,7 +124,7 @@ static void inotify(GenericQueue *qp) {
                        sdup->iqueue.q_buffer, SERIAL_USB_BUFFERS_SIZE);
     if (n != USB_ENDPOINT_BUSY) {
       sdup->iqueue.q_rdptr = sdup->iqueue.q_buffer;
-      chSemSetCounterI(&sdup->iqueue.q_sem, n);
+      chSemAddCounterI(&sdup->iqueue.q_sem, n);
       chIOAddFlagsI(sdup, IO_INPUT_AVAILABLE);
     }
   }
@@ -135,15 +135,16 @@ static void inotify(GenericQueue *qp) {
  */
 static void onotify(GenericQueue *qp) {
   SerialUSBDriver *sdup = (SerialUSBDriver *)qp->q_rdptr;
-  size_t n;
+  size_t w, n;
 
   /* If there is any data in the output queue then it is sent within a
      single packet and the queue is emptied.*/
-  n = usbWritePacketI(sdup->config->usbp, DATA_REQUEST_EP,
+  n = chOQGetFullI(&sdup->oqueue);
-                      sdup->oqueue.q_buffer, chOQGetFullI(&sdup->oqueue));
+  w = usbWritePacketI(sdup->config->usbp, DATA_REQUEST_EP,
-  if (n != USB_ENDPOINT_BUSY) {
+                      sdup->oqueue.q_buffer, n);
+  if (w != USB_ENDPOINT_BUSY) {
     sdup->oqueue.q_wrptr = sdup->oqueue.q_buffer;
-    chSemSetCounterI(&sdup->oqueue.q_sem, SERIAL_USB_BUFFERS_SIZE);
+    chSemAddCounterI(&sdup->oqueue.q_sem, n);
     chIOAddFlagsI(sdup, IO_OUTPUT_EMPTY);
   }
 }
@@ -276,17 +277,17 @@ bool_t sduRequestsHook(USBDriver *usbp) {
  */
 void sduDataTransmitted(USBDriver *usbp, usbep_t ep) {
   SerialUSBDriver *sdup = usbp->param;
-  size_t n;
+  size_t n, w;
 
   chSysLockFromIsr();
   /* If there is any data in the output queue then it is sent within a
      single packet and the queue is emptied.*/
   n = chOQGetFullI(&sdup->oqueue);
   if (n > 0) {
-    n = usbWritePacketI(usbp, ep, sdup->oqueue.q_buffer, n);
+    w = usbWritePacketI(usbp, ep, sdup->oqueue.q_buffer, n);
-    if (n != USB_ENDPOINT_BUSY) {
+    if (w != USB_ENDPOINT_BUSY) {
       sdup->oqueue.q_wrptr = sdup->oqueue.q_buffer;
-      chSemSetCounterI(&sdup->oqueue.q_sem, SERIAL_USB_BUFFERS_SIZE);
+      chSemAddCounterI(&sdup->oqueue.q_sem, n);
       chIOAddFlagsI(sdup, IO_OUTPUT_EMPTY);
     }
   }
@@ -314,7 +315,7 @@ void sduDataReceived(USBDriver *usbp, usbep_t ep) {
                        SERIAL_USB_BUFFERS_SIZE);
     if (n != USB_ENDPOINT_BUSY) {
       sdup->iqueue.q_rdptr = sdup->iqueue.q_buffer;
-      chSemSetCounterI(&sdup->iqueue.q_sem, n);
+      chSemAddCounterI(&sdup->iqueue.q_sem, n);
       chIOAddFlagsI(sdup, IO_INPUT_AVAILABLE);
     }
   }

os/kernel/include/chsem.h

@@ -52,7 +52,7 @@ extern "C" {
   msg_t chSemWaitTimeoutS(Semaphore *sp, systime_t time);
   void chSemSignal(Semaphore *sp);
   void chSemSignalI(Semaphore *sp);
-  void chSemSetCounterI(Semaphore *sp, cnt_t n);
+  void chSemAddCounterI(Semaphore *sp, cnt_t n);
 #if CH_USE_SEMSW
   msg_t chSemSignalWait(Semaphore *sps, Semaphore *spw);
 #endif

os/kernel/src/chsem.c

@@ -313,35 +313,32 @@ void chSemSignalI(Semaphore *sp) {
 }
 
 /**
- * @brief   Sets the semaphore counter to the specified value.
+ * @brief   Adds the specified value to the semaphore counter.
- * @post    After invoking this function all the threads waiting on the
- *          semaphore, if any, are released and the semaphore counter is set
- *          to the specified, non negative, value.
  * @post    This function does not reschedule so a call to a rescheduling
  *          function must be performed before unlocking the kernel. Note that
  *          interrupt handlers always reschedule on exit so an explicit
  *          reschedule must not be performed in ISRs.
  *
  * @param[in] sp        pointer to a @p Semaphore structure
- * @param[in] n         the new value of the semaphore counter. The value must
+ * @param[in] n         value to be added to the semaphore counter. The value
- *                      be non-negative.
+ *                      must be positive.
  *
  * @iclass
  */
-void chSemSetCounterI(Semaphore *sp, cnt_t n) {
+void chSemAddCounterI(Semaphore *sp, cnt_t n) {
-  cnt_t cnt;
 
-  chDbgCheck((sp != NULL) && (n >= 0), "chSemSetCounterI");
+  chDbgCheck((sp != NULL) && (n > 0), "chSemAddCounterI");
 
   chDbgAssert(((sp->s_cnt >= 0) && isempty(&sp->s_queue)) ||
               ((sp->s_cnt < 0) && notempty(&sp->s_queue)),
-              "chSemSetCounterI(), #1",
+              "chSemAddCounterI(), #1",
               "inconsistent semaphore");
 
-  cnt = sp->s_cnt;
+  while (n > 0) {
-  sp->s_cnt = n;
+    if (++sp->s_cnt <= 0)
-  while (++cnt <= 0)
+      chSchReadyI(fifo_remove(&sp->s_queue))->p_u.rdymsg = RDY_OK;
-    chSchReadyI(lifo_remove(&sp->s_queue))->p_u.rdymsg = RDY_OK;
+    n--;
+  }
 }
 
 #if CH_USE_SEMSW

readme.txt

@@ -71,9 +71,15 @@
 *****************************************************************************
 
 *** 2.3.2 ***
+- FIX: Fixed spurious characters generated by Serial over USB driver (bug
+  3276379).
 - OPT: STM32 PWM driver implementation simplified.
 - CHANGE: Now pwmChangePeriod() does not implicitly disable the active
   PWM channels.
+- CHANGE: Renamed the chSemSetCounterI() function to chSemAddCounterI() and
+  changed its behavior.
+- CHANGE: Modified the STM32 USB-CDC test demo to spawn a shell over the USB
+  serial connection.
 
 *** 2.3.1 ***
 - FIX: Fixed insufficient idle thread stack in Cortex-M0-GCC port (bug 3226671)

test/testsem.c

@@ -104,9 +104,9 @@ static void sem1_execute(void) {
   test_assert_sequence(1, "ABCDE");
 #endif
   threads[0] = chThdCreateStatic(wa[0], WA_SIZE, chThdGetPriority()+5, thread1, "A");
-  chSemSetCounterI(&sem1, 2);
+  chSemAddCounterI(&sem1, 2);
   test_wait_threads();
-  test_assert(2, chSemGetCounterI(&sem1) == 2, "invalid counter");
+  test_assert(2, chSemGetCounterI(&sem1) == 1, "invalid counter");
 }
 
 ROMCONST struct testcase testsem1 = {

testhal/STM32/USB_CDC/Makefile

@@ -72,7 +72,7 @@ CSRC = $(PORTSRC) \
        $(HALSRC) \
        $(PLATFORMSRC) \
        $(BOARDSRC) \
-       $(CHIBIOS)/os/various/evtimer.c \
+       $(CHIBIOS)/os/various/shell.c \
        $(CHIBIOS)/os/various/syscalls.c \
        main.c
 

testhal/STM32/USB_CDC/halconf.h

@@ -94,7 +94,7 @@
  * @brief   Enables the SERIAL subsystem.
  */
 #if !defined(HAL_USE_SERIAL) || defined(__DOXYGEN__)
-#define HAL_USE_SERIAL              TRUE
+#define HAL_USE_SERIAL              FALSE
 #endif
 
 /**

testhal/STM32/USB_CDC/main.c

@@ -18,11 +18,15 @@
     along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+#include <stdio.h>
+#include <string.h>
+
 #include "ch.h"
 #include "hal.h"
 #include "test.h"
 
 #include "usb_cdc.h"
+#include "shell.h"
 
 /*===========================================================================*/
 /* USB related stuff.                                                        */
@@ -305,6 +309,97 @@ static const SerialUSBConfig serusbcfg = {
   }
 };
 
+/*===========================================================================*/
+/* Command line related.                                                     */
+/*===========================================================================*/
+
+#define SHELL_WA_SIZE   THD_WA_SIZE(2048)
+#define TEST_WA_SIZE    THD_WA_SIZE(256)
+
+static void cmd_mem(BaseChannel *chp, int argc, char *argv[]) {
+  size_t n, size;
+  char buf[52];
+
+  (void)argv;
+  if (argc > 0) {
+    shellPrintLine(chp, "Usage: mem");
+    return;
+  }
+  n = chHeapStatus(NULL, &size);
+  siprintf(buf, "core free memory : %u bytes", chCoreStatus());
+  shellPrintLine(chp, buf);
+  siprintf(buf, "heap fragments   : %u", n);
+  shellPrintLine(chp, buf);
+  siprintf(buf, "heap free total  : %u bytes", size);
+  shellPrintLine(chp, buf);
+}
+
+static void cmd_threads(BaseChannel *chp, int argc, char *argv[]) {
+  static const char *states[] = {
+    "READY",
+    "CURRENT",
+    "SUSPENDED",
+    "WTSEM",
+    "WTMTX",
+    "WTCOND",
+    "SLEEPING",
+    "WTEXIT",
+    "WTOREVT",
+    "WTANDEVT",
+    "SNDMSGQ",
+    "SNDMSG",
+    "WTMSG",
+    "FINAL"
+  };
+  Thread *tp;
+  char buf[60];
+
+  (void)argv;
+  if (argc > 0) {
+    shellPrintLine(chp, "Usage: threads");
+    return;
+  }
+  shellPrintLine(chp, "    addr    stack prio refs     state time");
+  tp = chRegFirstThread();
+  do {
+    siprintf(buf, "%8lx %8lx %4u %4i %9s %u",
+             (uint32_t)tp, (uint32_t)tp->p_ctx.r13,
+             (unsigned int)tp->p_prio, tp->p_refs - 1,
+             states[tp->p_state], (unsigned int)tp->p_time);
+    shellPrintLine(chp, buf);
+    tp = chRegNextThread(tp);
+  } while (tp != NULL);
+}
+
+static void cmd_test(BaseChannel *chp, int argc, char *argv[]) {
+  Thread *tp;
+
+  (void)argv;
+  if (argc > 0) {
+    shellPrintLine(chp, "Usage: test");
+    return;
+  }
+  tp = chThdCreateFromHeap(NULL, TEST_WA_SIZE, chThdGetPriority(),
+                           TestThread, chp);
+  if (tp == NULL) {
+    shellPrintLine(chp, "out of memory");
+    return;
+  }
+  chThdWait(tp);
+}
+
+static const ShellCommand commands[] = {
+  {"mem", cmd_mem},
+  {"threads", cmd_threads},
+  {"test", cmd_test},
+  {NULL, NULL}
+};
+
+static const ShellConfig shell_cfg1 = {
+  (BaseChannel *)&SDU1,
+  commands
+};
+
 /*===========================================================================*/
 /* Generic code.                                                             */
 /*===========================================================================*/
@@ -324,31 +419,11 @@ static msg_t Thread1(void *arg) {
   }
 }
 
-/*
- * USB CDC loopback thread.
- */
-static WORKING_AREA(waThread2, 256);
-static msg_t Thread2(void *arg) {
-  SerialUSBDriver *sdup = arg;
-  EventListener el;
-
-  chEvtRegisterMask(chIOGetEventSource(&SDU1), &el, 1);
-  while (TRUE) {
-    chEvtWaitAny(ALL_EVENTS);
-    if (chOQIsEmptyI(&SDU1.oqueue)) {
-      uint8_t buffer[0x40];
-      size_t n = chIQReadTimeout(&sdup->iqueue, buffer,
-                                 sizeof(buffer), TIME_IMMEDIATE);
-      if (n > 0)
-        chOQWriteTimeout(&sdup->oqueue, buffer, n, TIME_IMMEDIATE);
-    }
-  }
-}
-
 /*
  * Application entry point.
  */
 int main(void) {
+  Thread *shelltp = NULL;
 
   /*
    * System initializations.
@@ -368,27 +443,26 @@ int main(void) {
   palClearPad(GPIOC, GPIOC_USB_DISC);
 
   /*
-   * Activates the serial driver 2 using the driver default configuration.
+   * Shell manager initialization.
    */
-  sdStart(&SD2, NULL);
+  shellInit();
 
   /*
    * Creates the blinker thread.
    */
   chThdCreateStatic(waThread1, sizeof(waThread1), NORMALPRIO, Thread1, NULL);
 
-  /*
-   * Creates the USB CDC loopback thread.
-   */
-  chThdCreateStatic(waThread2, sizeof(waThread2), NORMALPRIO, Thread2, &SDU1);
-
   /*
    * Normal main() thread activity, in this demo it does nothing except
    * sleeping in a loop and check the button state.
    */
   while (TRUE) {
-    if (palReadPad(IOPORT1, GPIOA_BUTTON))
+    if (!shelltp && (SDU1.config->usbp->state == USB_ACTIVE))
-      TestThread(&SD2);
+      shelltp = shellCreate(&shell_cfg1, SHELL_WA_SIZE, NORMALPRIO);
+    else if (chThdTerminated(shelltp)) {
+      chThdRelease(shelltp);    /* Recovers memory of the previous shell.   */
+      shelltp = NULL;           /* Triggers spawning of a new shell.        */
+    }
     chThdSleepMilliseconds(1000);
   }
 }