Fixed bug 3276379.

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

8 files changed, 146 insertions, 68 deletions

diff --git a/os/hal/src/serial_usb.c b/os/hal/src/serial_usb.c
index b6ad10c13..bc2c39426 100644
--- a/os/hal/src/serial_usb.c
+++ b/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,

-                      sdup->oqueue.q_buffer, chOQGetFullI(&sdup->oqueue));

-  if (n != USB_ENDPOINT_BUSY) {

+  n = chOQGetFullI(&sdup->oqueue);

+  w = usbWritePacketI(sdup->config->usbp, DATA_REQUEST_EP,

+                      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);

-    if (n != USB_ENDPOINT_BUSY) {

+    w = usbWritePacketI(usbp, ep, 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);

     }

   }

@@ -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);

     }

   }

diff --git a/os/kernel/include/chsem.h b/os/kernel/include/chsem.h
index f33b294fa..04e079466 100644
--- a/os/kernel/include/chsem.h
+++ b/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

diff --git a/os/kernel/src/chsem.c b/os/kernel/src/chsem.c
index 3e0fcd0e8..c22a568ea 100644
--- a/os/kernel/src/chsem.c
+++ b/os/kernel/src/chsem.c
@@ -313,35 +313,32 @@ void chSemSignalI(Semaphore *sp) {
 }

 

 /**

- * @brief   Sets the semaphore counter to the specified value.

- * @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.

+ * @brief   Adds the specified value to the semaphore counter.

  * @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

- *                      be non-negative.

+ * @param[in] n         value to be added to the semaphore counter. The value

+ *                      must be positive.

  *

  * @iclass

  */

-void chSemSetCounterI(Semaphore *sp, cnt_t n) {

-  cnt_t cnt;

+void chSemAddCounterI(Semaphore *sp, cnt_t n) {

 

-  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;

-  sp->s_cnt = n;

-  while (++cnt <= 0)

-    chSchReadyI(lifo_remove(&sp->s_queue))->p_u.rdymsg = RDY_OK;

+  while (n > 0) {

+    if (++sp->s_cnt <= 0)

+      chSchReadyI(fifo_remove(&sp->s_queue))->p_u.rdymsg = RDY_OK;

+    n--;

+  }

 }

 

 #if CH_USE_SEMSW

diff --git a/readme.txt b/readme.txt
index 2c5bc3bcc..3a464c3b1 100644
--- a/readme.txt
+++ b/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)

diff --git a/test/testsem.c b/test/testsem.c
index 009a5910f..6a6a622ef 100644
--- a/test/testsem.c
+++ b/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 = {

diff --git a/testhal/STM32/USB_CDC/Makefile b/testhal/STM32/USB_CDC/Makefile
index 9ae1a088e..16b23cc48 100644
--- a/testhal/STM32/USB_CDC/Makefile
+++ b/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
 

diff --git a/testhal/STM32/USB_CDC/halconf.h b/testhal/STM32/USB_CDC/halconf.h
index e401453e0..91ea7a5f2 100644
--- a/testhal/STM32/USB_CDC/halconf.h
+++ b/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

 

 /**

diff --git a/testhal/STM32/USB_CDC/main.c b/testhal/STM32/USB_CDC/main.c
index 23962c496..6d8994c8d 100644
--- a/testhal/STM32/USB_CDC/main.c
+++ b/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.                                                        */

@@ -306,6 +310,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.                                                             */

 /*===========================================================================*/

 

@@ -325,30 +420,10 @@ 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,9 +443,9 @@ 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.

@@ -378,17 +453,16 @@ int main(void) {
   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))

-      TestThread(&SD2);

+    if (!shelltp && (SDU1.config->usbp->state == USB_ACTIVE))

+      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);

   }

 }