make usb code work with optimizations (ie. -O2)

[lpcware]

Content originally posted in LPCWare by domen on Tue May 11 08:54:55 MST 2010
Basically it's two changes.
First one fails because UsbAddPtr gets inlined, and pD is not reloaded when pD->length is checked on next loop iteration (not sure if this is actually a compiler bug, or a valid "optimization").

Second one is deffinitely a code bug, volatile should be used, so code isn't optimized away. And even then... who makes delays with C LOOPS?!

Index: usbcore.c
===================================================================
--- usbcore.c    (revision 353)
+++ usbcore.c    (working copy)
@@ -394,32 +394,6 @@
 
 
 /*
- * Add a number of bytes to a pointer's address
- * Harder than you might think. Some compilers say:
- * Expected an lvalue  -- Assignment expects its first operand to be
- * an lvalue.  Please note that a cast removes the lvaluedness of an
- * expression.
- *
- * vpptr = void pointer to pointer
- * n = number of bytes to add to pointer
- * Call looks like: AddPtr((void **)&myPointer, 8);
- */
-
-__inline void UsbAddPtr(void **vpptr, uint32_t n)
-{
-    /* Declare a pointer to a pointer to a byte. Only a byte pointer
-     * can be incremented by a number of bytes. Other pointers will
-     * increment by a multiple of what they point to.
-     */
-    uint8_t **bpptr;
-
-    /* Convert our void pointer to a pointer to a byte pointer to a pointer */
-    bpptr = (uint8_t **)vpptr;
-
-    /* Add 'n' bytes to our pointer value */
-    (*bpptr) += n;
-}
-/*
  *  Set Configuration USB Request
  *    Parameters:      None (global SetupPacket)
  *    Return Value:    TRUE - Success, FALSE - Error
@@ -462,7 +436,10 @@
                   USB_DeviceStatus &= ~USB_GETSTATUS_SELF_POWERED;
                 }
               } else {
-                UsbAddPtr((void **)&pD, ((USB_CONFIGURATION_DESCRIPTOR *)pD)->wTotalLength);
+                /* increment pointer */
+                uint8_t *tmp = (void*)pD;
+                tmp += ((USB_CONFIGURATION_DESCRIPTOR *)pD)->wTotalLength;
+                pD = (void*)tmp;
                 continue;
               }
               break;
@@ -480,7 +457,10 @@
               }
               break;
           }
-          UsbAddPtr((void **)&pD, pD->bLength);
+          /* increment pointer */
+          uint8_t *tmp = (void*)pD;
+          tmp += pD->bLength;
+          pD = (void*)tmp;
         }
       }
       else {
@@ -554,7 +534,10 @@
         switch (pD->bDescriptorType) {
           case USB_CONFIGURATION_DESCRIPTOR_TYPE:
             if (((USB_CONFIGURATION_DESCRIPTOR *)pD)->bConfigurationValue != USB_Configuration) {
-              UsbAddPtr((void **)&pD, ((USB_CONFIGURATION_DESCRIPTOR *)pD)->wTotalLength);
+              /* increment pointer */
+              uint8_t *tmp = (void*)pD;
+              tmp += ((USB_CONFIGURATION_DESCRIPTOR *)pD)->wTotalLength;
+              pD = (void*)tmp;
               continue;
             }
             break;
@@ -588,7 +571,10 @@
             }
            break;
         }
-        UsbAddPtr((void **)&pD, pD->bLength);
+        /* increment pointer */
+        uint8_t *tmp = (void*)pD;
+        tmp += pD->bLength;
+        pD = (void*)tmp;
       }
       break;
     default:
Index: usbhw.c
===================================================================
--- usbhw.c    (revision 353)
+++ usbhw.c    (working copy)
@@ -90,7 +90,7 @@
  */
 
 void delay (uint32_t length ) {
-  uint32_t i;
+  volatile uint32_t i;
 
   for ( i = 0; i < length; i++ );
   return;

[lpcware]

Content originally posted in LPCWare by domen on Wed May 12 04:29:15 MST 2010
And this is what I needed to get usbcdc working:

If not volatile, the while (buffer_empty..) loop optimizes to a nice while (1)

Index: usbcore.c
===================================================================
--- usbcore.c    (revision 356)
+++ usbcore.c    (working copy)
@@ -61,7 +61,7 @@
 
 uint16_t  USB_DeviceStatus;
 uint8_t  USB_DeviceAddress;
-uint8_t  USB_Configuration;
+volatile uint8_t  USB_Configuration;
 uint32_t USB_EndPointMask;
 uint32_t USB_EndPointHalt;
 uint32_t USB_EndPointStall;                         /* EP must stay stalled */
@@ -374,7 +374,7 @@
 
   switch (SetupPacket.bmRequestType.BM.Recipient) {
     case REQUEST_TO_DEVICE:
-      EP0Data.pData = &USB_Configuration;
+      EP0Data.pData = (uint8_t *)&USB_Configuration;
       break;
     default:
       return (FALSE);
Index: usbcore.h
===================================================================
--- usbcore.h    (revision 356)
+++ usbcore.h    (working copy)
@@ -29,7 +29,7 @@
 /* USB Core Global Variables */
 extern uint16_t USB_DeviceStatus;
 extern uint8_t  USB_DeviceAddress;
-extern uint8_t  USB_Configuration;
+extern volatile uint8_t  USB_Configuration;
 extern uint32_t USB_EndPointMask;
 extern uint32_t USB_EndPointHalt;
 extern uint32_t USB_EndPointStall;
Index: serial.c
===================================================================
--- serial.c    (revision 356)
+++ serial.c    (working copy)
@@ -35,8 +35,8 @@
 // buffer type
 typedef struct __SER_BUF_T {
   unsigned char data[SER_BUF_SIZE];
-  unsigned int wrIdx;
-  unsigned int rdIdx;
+  volatile unsigned int wrIdx;
+  volatile unsigned int rdIdx;
 } SER_BUF_T;
 
 unsigned long          ser_txRestart;                  // NZ if TX restart is required
Index: cdcuser.c
===================================================================
--- cdcuser.c    (revision 356)
+++ cdcuser.c    (working copy)
@@ -56,8 +56,8 @@
 // CDC output buffer
 typedef struct __CDC_BUF_T {
   unsigned char data[CDC_BUF_SIZE];
-  unsigned int wrIdx;
-  unsigned int rdIdx;
+  volatile unsigned int wrIdx;
+  volatile unsigned int rdIdx;
 } CDC_BUF_T;
 
 CDC_BUF_T  CDC_OutBuf;                                 // buffer for all CDC Out data

[lpcware]

Content originally posted in LPCWare by domen on Tue May 11 15:18:53 MST 2010
Yeah, lets complicate already too complicated piece of code :P

I like my version better. Don't care what gets included.

[lpcware]

Content originally posted in LPCWare by NXP_USA on Tue May 11 13:51:15 MST 2010

---

Quote: domen
Basically it's two changes.
First one fails because UsbAddPtr gets inlined, and pD is not reloaded when pD->length is checked on next loop iteration (not sure if this is actually a compiler bug, or a valid "optimization").

---

Try this change- it should take care of the optimization problems.

__inline void UsbAddPtr(void **vpptr, const uint32_t n)
{
/* Declare a pointer to a pointer to a byte. Only a byte pointer
 * can be incremented by a number of bytes. Other pointers will
 * increment by a multiple of what they point to.
 */
volatile uint8_t **bpptr;

/* Convert our void pointer to a pointer to a byte pointer to a pointer */
bpptr = (volatile uint8_t **)vpptr;

/* Add 'n' bytes to our pointer value */
(*bpptr) += n;
}