usb_cdcacm: various fixups.

Mark internal state static. Properly reset the RX/TX state on USB
reset. Choose better names for countTx and newBytes.

Move the exposed configuration back into the .c; this information is
not beneficial to hooks.

Signed-off-by: Marti Bolivar <mbolivar@leaflabs.com>

2 files changed, 60 insertions, 64 deletions

diff --git a/libmaple/include/libmaple/usb_cdcacm.h b/libmaple/include/libmaple/usb_cdcacm.h
index 2cb3ad4..5ceecc3 100644
--- a/libmaple/include/libmaple/usb_cdcacm.h
+++ b/libmaple/include/libmaple/usb_cdcacm.h
@@ -42,31 +42,6 @@ extern "C" {
 #endif
 
 /*
- * Exposed configuration
- */
-
-#define USB_CDCACM_CTRL_EPNUM           0x00
-#define USB_CDCACM_CTRL_RX_ADDR         0x40
-#define USB_CDCACM_CTRL_TX_ADDR         0x80
-#define USB_CDCACM_CTRL_EPSIZE          0x40
-
-#define USB_CDCACM_TX_ENDP              1
-#define USB_CDCACM_TX_EPNUM             0x01
-#define USB_CDCACM_TX_ADDR              0xC0
-#define USB_CDCACM_TX_EPSIZE            0x40
-
-#define USB_CDCACM_MANAGEMENT_ENDP      2
-#define USB_CDCACM_MANAGEMENT_EPNUM     0x02
-#define USB_CDCACM_MANAGEMENT_ADDR      0x100
-#define USB_CDCACM_MANAGEMENT_EPSIZE    0x40
-
-#define USB_CDCACM_RX_ENDP              3
-#define USB_CDCACM_RX_EPNUM             0x03
-#define USB_CDCACM_RX_ADDR              0x110
-#define USB_CDCACM_RX_EPSIZE            0x40
-#define USB_CDCACM_RX_BUFLEN            (USB_CDCACM_RX_EPSIZE*3)
-
-/*
  * CDC ACM Requests
  */
 
diff --git a/libmaple/usb/stm32f1/usb_cdcacm.c b/libmaple/usb/stm32f1/usb_cdcacm.c
index e664797..07cf9c4 100644
--- a/libmaple/usb/stm32f1/usb_cdcacm.c
+++ b/libmaple/usb/stm32f1/usb_cdcacm.c
@@ -83,6 +83,27 @@ static void usbSetConfiguration(void);
 static void usbSetDeviceAddress(void);
 
 /*
+ * Endpoint configuration
+ */
+
+#define USB_CDCACM_CTRL_ENDP            0
+#define USB_CDCACM_CTRL_RX_ADDR         0x40
+#define USB_CDCACM_CTRL_TX_ADDR         0x80
+#define USB_CDCACM_CTRL_EPSIZE          0x40
+
+#define USB_CDCACM_TX_ENDP              1
+#define USB_CDCACM_TX_ADDR              0xC0
+#define USB_CDCACM_TX_EPSIZE            0x40
+
+#define USB_CDCACM_MANAGEMENT_ENDP      2
+#define USB_CDCACM_MANAGEMENT_ADDR      0x100
+#define USB_CDCACM_MANAGEMENT_EPSIZE    0x40
+
+#define USB_CDCACM_RX_ENDP              3
+#define USB_CDCACM_RX_ADDR              0x110
+#define USB_CDCACM_RX_EPSIZE            0x40
+
+/*
  * Descriptors
  */
 
@@ -90,7 +111,7 @@ static void usbSetDeviceAddress(void);
 #define USB_DEVICE_SUBCLASS_CDC           0x00
 #define LEAFLABS_ID_VENDOR                0x1EAF
 #define MAPLE_ID_PRODUCT                  0x0004
-const USB_Descriptor_Device usbVcomDescriptor_Device = {
+static const USB_Descriptor_Device usbVcomDescriptor_Device = {
     .bLength            = sizeof(USB_Descriptor_Device),
     .bDescriptorType    = USB_DESCRIPTOR_TYPE_DEVICE,
     .bcdUSB             = 0x0200,
@@ -121,7 +142,7 @@ typedef struct {
 } __packed USB_Descriptor_Config;
 
 #define MAX_POWER (100 >> 1)
-const USB_Descriptor_Config usbVcomDescriptor_Config = {
+static const USB_Descriptor_Config usbVcomDescriptor_Config = {
     .Config_Header = {
         .bLength              = sizeof(USB_Descriptor_Config_Header),
         .bDescriptorType      = USB_DESCRIPTOR_TYPE_CONFIGURATION,
@@ -178,7 +199,7 @@ const USB_Descriptor_Config usbVcomDescriptor_Config = {
         .bLength          = sizeof(USB_Descriptor_Endpoint),
         .bDescriptorType  = USB_DESCRIPTOR_TYPE_ENDPOINT,
         .bEndpointAddress = (USB_DESCRIPTOR_ENDPOINT_IN |
-                             USB_CDCACM_MANAGEMENT_EPNUM),
+                             USB_CDCACM_MANAGEMENT_ENDP),
         .bmAttributes     = EP_TYPE_INTERRUPT,
         .wMaxPacketSize   = USB_CDCACM_MANAGEMENT_EPSIZE,
         .bInterval        = 0xFF,
@@ -200,7 +221,7 @@ const USB_Descriptor_Config usbVcomDescriptor_Config = {
         .bLength          = sizeof(USB_Descriptor_Endpoint),
         .bDescriptorType  = USB_DESCRIPTOR_TYPE_ENDPOINT,
         .bEndpointAddress = (USB_DESCRIPTOR_ENDPOINT_OUT |
-                             USB_CDCACM_RX_EPNUM),
+                             USB_CDCACM_RX_ENDP),
         .bmAttributes     = EP_TYPE_BULK,
         .wMaxPacketSize   = USB_CDCACM_RX_EPSIZE,
         .bInterval        = 0x00,
@@ -209,7 +230,7 @@ const USB_Descriptor_Config usbVcomDescriptor_Config = {
     .DataInEndpoint = {
         .bLength          = sizeof(USB_Descriptor_Endpoint),
         .bDescriptorType  = USB_DESCRIPTOR_TYPE_ENDPOINT,
-        .bEndpointAddress = (USB_DESCRIPTOR_ENDPOINT_IN | USB_CDCACM_TX_EPNUM),
+        .bEndpointAddress = (USB_DESCRIPTOR_ENDPOINT_IN | USB_CDCACM_TX_ENDP),
         .bmAttributes     = EP_TYPE_BULK,
         .wMaxPacketSize   = USB_CDCACM_TX_EPSIZE,
         .bInterval        = 0x00,
@@ -233,38 +254,38 @@ const USB_Descriptor_Config usbVcomDescriptor_Config = {
   which is 0x0409 for US English
 */
 
-const uint8 usbVcomDescriptor_LangID[USB_DESCRIPTOR_STRING_LEN(1)] = {
+static const uint8 usbVcomDescriptor_LangID[USB_DESCRIPTOR_STRING_LEN(1)] = {
     USB_DESCRIPTOR_STRING_LEN(1),
     USB_DESCRIPTOR_TYPE_STRING,
     0x09,
     0x04,
 };
 
-const uint8 usbVcomDescriptor_iManufacturer[USB_DESCRIPTOR_STRING_LEN(8)] = {
+static const uint8 usbVcomDescriptor_iManufacturer[USB_DESCRIPTOR_STRING_LEN(8)] = {
     USB_DESCRIPTOR_STRING_LEN(8),
     USB_DESCRIPTOR_TYPE_STRING,
     'L', 0, 'e', 0, 'a', 0, 'f', 0,
     'L', 0, 'a', 0, 'b', 0, 's', 0,
 };
 
-const uint8 usbVcomDescriptor_iProduct[USB_DESCRIPTOR_STRING_LEN(8)] = {
+static const uint8 usbVcomDescriptor_iProduct[USB_DESCRIPTOR_STRING_LEN(8)] = {
     USB_DESCRIPTOR_STRING_LEN(8),
     USB_DESCRIPTOR_TYPE_STRING,
     'M', 0, 'a', 0, 'p', 0, 'l', 0,
     'e', 0, ' ', 0, ' ', 0, ' ', 0
 };
 
-ONE_DESCRIPTOR Device_Descriptor = {
+static ONE_DESCRIPTOR Device_Descriptor = {
     (uint8*)&usbVcomDescriptor_Device,
     sizeof(USB_Descriptor_Device)
 };
 
-ONE_DESCRIPTOR Config_Descriptor = {
+static ONE_DESCRIPTOR Config_Descriptor = {
     (uint8*)&usbVcomDescriptor_Config,
     sizeof(USB_Descriptor_Config)
 };
 
-ONE_DESCRIPTOR String_Descriptor[3] = {
+static ONE_DESCRIPTOR String_Descriptor[3] = {
     {(uint8*)&usbVcomDescriptor_LangID,       USB_DESCRIPTOR_STRING_LEN(1)},
     {(uint8*)&usbVcomDescriptor_iManufacturer,USB_DESCRIPTOR_STRING_LEN(8)},
     {(uint8*)&usbVcomDescriptor_iProduct,     USB_DESCRIPTOR_STRING_LEN(8)}
@@ -277,13 +298,13 @@ ONE_DESCRIPTOR String_Descriptor[3] = {
 /* I/O state */
 
 /* Received data */
-static volatile uint8 vcomBufferRx[USB_CDCACM_RX_BUFLEN];
+static volatile uint8 vcomBufferRx[USB_CDCACM_RX_EPSIZE];
 /* Read index into vcomBufferRx */
 static volatile uint32 rx_offset = 0;
 /* Number of bytes left to transmit */
-static volatile uint32 countTx = 0;
+static volatile uint32 n_unsent_bytes = 0;
 /* Number of unread bytes */
-static volatile uint32 newBytes = 0;
+static volatile uint32 n_unread_bytes = 0;
 
 /* Other state (line coding, DTR/RTS) */
 
@@ -426,11 +447,10 @@ void usb_cdcacm_putc(char ch) {
 /* This function is non-blocking.
  *
  * It copies data from a usercode buffer into the USB peripheral TX
- * buffer and return the number placed in that buffer.
- */
+ * buffer, and returns the number of bytes copied. */
 uint32 usb_cdcacm_tx(const uint8* buf, uint32 len) {
-    /* Last transmission hasn't finished, abort */
-    if (countTx) {
+    /* Last transmission hasn't finished, so abort. */
+    if (n_unsent_bytes) {
         return 0;
     }
 
@@ -439,24 +459,23 @@ uint32 usb_cdcacm_tx(const uint8* buf, uint32 len) {
         len = USB_CDCACM_TX_EPSIZE;
     }
 
-    /* Queue bytes for sending */
+    /* Queue bytes for sending. */
     if (len) {
         usb_copy_to_pma(buf, len, USB_CDCACM_TX_ADDR);
         usb_set_ep_tx_count(USB_CDCACM_TX_ENDP, len);
-        countTx = len;
+        n_unsent_bytes = len;
         usb_set_ep_tx_stat(USB_CDCACM_TX_ENDP, USB_EP_STAT_TX_VALID);
     }
 
     return len;
 }
 
-/* returns the number of available bytes are in the recv FIFO */
 uint32 usb_cdcacm_data_available(void) {
-    return newBytes;
+    return n_unread_bytes;
 }
 
 uint16 usb_cdcacm_get_pending() {
-    return countTx;
+    return n_unsent_bytes;
 }
 
 /* Nonblocking byte receive.
@@ -468,12 +487,12 @@ uint32 usb_cdcacm_rx(uint8* buf, uint32 len) {
     uint32 n_copied = usb_cdcacm_peek(buf, len);
 
     /* Mark bytes as read. */
-    newBytes -= n_copied;
+    n_unread_bytes -= n_copied;
     rx_offset += n_copied;
 
     /* If all bytes have been read, re-enable the RX endpoint, which
      * was set to NAK when the current batch of bytes was received. */
-    if (newBytes == 0) {
+    if (n_unread_bytes == 0) {
         usb_set_ep_rx_count(USB_CDCACM_RX_ENDP, USB_CDCACM_RX_EPSIZE);
         usb_set_ep_rx_stat(USB_CDCACM_RX_ENDP, USB_EP_STAT_RX_VALID);
         rx_offset = 0;
@@ -488,8 +507,8 @@ uint32 usb_cdcacm_rx(uint8* buf, uint32 len) {
 uint32 usb_cdcacm_peek(uint8* buf, uint32 len) {
     int i;
 
-    if (len > newBytes) {
-        len = newBytes;
+    if (len > n_unread_bytes) {
+        len = n_unread_bytes;
     }
 
     for (i = 0; i < len; i++) {
@@ -512,20 +531,22 @@ uint8 usb_cdcacm_get_rts() {
  */
 
 static void vcomDataTxCb(void) {
-    /* assumes tx transactions are atomic 64 bytes (nearly certain they are) */
-    /* TODO determine if TX txns. are actually "atomic 64 bytes",
-     * whatever that means. */
-    countTx = 0;
+    /* The following assumes that all of the bytes we copied during
+     * the last call to usb_cdcacm_tx were sent during the IN
+     * transaction (this seems to be the case). */
+    /* TODO find out why this is broken:
+     * n_unsent_bytes = usb_get_ep_tx_count(USB_CDCACM_TX_ENDP); */
+    n_unsent_bytes = 0;
 }
 
 static void vcomDataRxCb(void) {
-    /* This following is safe since sizeof(vcomBufferRx) exceeds the
-     * largest possible USB_CDCACM_RX_EPSIZE, and we set to NAK after
-     * each data packet. Only when all bytes have been read is the RX
-     * endpoint set back to VALID. */
-    newBytes = usb_get_ep_rx_count(USB_CDCACM_RX_ENDP);
+    /* This following is safe since we set the RX endpoint to NAK
+     * after each data packet received, and only set it to VALID when
+     * all bytes have been read. */
+    n_unread_bytes = usb_get_ep_rx_count(USB_CDCACM_RX_ENDP);
     usb_set_ep_rx_stat(USB_CDCACM_RX_ENDP, USB_EP_STAT_RX_NAK);
-    usb_copy_from_pma((uint8*)vcomBufferRx, newBytes, USB_CDCACM_RX_ADDR);
+    usb_copy_from_pma((uint8*)vcomBufferRx, n_unread_bytes,
+                      USB_CDCACM_RX_ADDR);
 
     if (rx_hook) {
         rx_hook(USB_CDCACM_HOOK_RX, 0);
@@ -558,7 +579,6 @@ static void usbInit(void) {
     USBLIB->state = USB_UNCONNECTED;
 }
 
-/* choose addresses to give endpoints the max 64 byte buffers */
 #define BTABLE_ADDRESS        0x00
 static void usbReset(void) {
     pInformation->Current_Configuration = 0;
@@ -604,7 +624,8 @@ static void usbReset(void) {
     SetDeviceAddress(0);
 
     /* Reset the RX/TX state */
-    countTx = 0;
+    n_unread_bytes = 0;
+    n_unsent_bytes = 0;
     rx_offset = 0;
 }