altos/cc1111: Hack on USB driver to make Windows happy

The Windows modem driver is quite chatty at startup time, getting and
setting the comm parameters each time the device is opened. Sometimes,
when setting the parameters, the cc1111 would STALL EP0.

Most of the time, Windows would happily pass this as an error back to
AltosUI which would then re-try the open (and succeed, most of the
time).

Sometimes, Windows would stall for 30 seconds before passing the error
back. This made the whole UI freeze, and I suspect most people assumed
our app had died.

A bit of analysis with the beagle USB sniffer and I discovered the
STALL settings, but there wasn't any correlation between the data on
the wire and when the STALL would be generated.

So, I found a couple of other cc1111 USB stacks on the net and just
looked to see how our driver differed. There wasn't anything clearly
related, but there were a list of small differences:

 1) Other drivers didn't bother waiting for the hardware to
    ack the USBADDR setting; doing it this way means we can set
    the address *before* acking the setup packet. It'll get
    set eventually, at which point the device will start responding to
    packets again.

    Easy to fix, and saves a bit of code space too.

 2) The other drivers set the STALL bit for setup packets which aren't
    understood. This shouldn't have any effect on 'good' systems as
    those shouldn't ever be generating bogus setup packets anyways.

    The driver already handled the STALL state in the interrupt
    handler, the only requirement was to figure out when to explicitly
    set the STALL bit.

    That required moving the state updating code from the start of the
    ep0 setup handling to the end, after the setup packet had been
    examined and data queued in or out as appropriate.

 3) Our driver explicitly queued an IN packet for any setup request
    that wasn't waiting for an OUT pack. This appears to tie in with
    the USBADDR change above as before I made that change, this change
    caused the driver to fail to respond to most setup packets.

    This was simple once the above change was made, just move the
    generation of the IN packet inside the code that switched to the
    IN state.

Signed-off-by: Keith Packard <keithp@keithp.com>

diff --git a/src/cc1111/ao_usb.c b/src/cc1111/ao_usb.c
index 8bd2efdf73706e2c9060e02d389191fa2112f04b..a655d1be903589374bae5c5d1af63244ac873697 100644 (file)
--- a/src/cc1111/ao_usb.c
+++ b/src/cc1111/ao_usb.c
@@ -152,19 +152,17 @@ ao_usb_ep0_fill(void)
                *ao_usb_ep0_out_data++ = USBFIFO[0];
 }
 
-void
+static void
 ao_usb_ep0_queue_byte(uint8_t a)
 {
        ao_usb_ep0_in_buf[ao_usb_ep0_in_len++] = a;
 }
 
-void
+static void
 ao_usb_set_address(uint8_t address)
 {
        ao_usb_running = 1;
-       USBADDR = address | 0x80;
-       while (USBADDR & 0x80)
-               ;
+       USBADDR = address;
 }
 
 static void
@@ -191,24 +189,6 @@ ao_usb_ep0_setup(void)
        if (ao_usb_ep0_out_len != 0)
                return;
 
-       /* Figure out how to ACK the setup packet */
-       if (ao_usb_setup.dir_type_recip & AO_USB_DIR_IN) {
-               if (ao_usb_setup.length)
-                       ao_usb_ep0_state = AO_USB_EP0_DATA_IN;
-               else
-                       ao_usb_ep0_state = AO_USB_EP0_IDLE;
-       } else {
-               if (ao_usb_setup.length)
-                       ao_usb_ep0_state = AO_USB_EP0_DATA_OUT;
-               else
-                       ao_usb_ep0_state = AO_USB_EP0_IDLE;
-       }
-       USBINDEX = 0;
-       if (ao_usb_ep0_state == AO_USB_EP0_IDLE)
-               USBCS0 = USBCS0_CLR_OUTPKT_RDY | USBCS0_DATA_END;
-       else
-               USBCS0 = USBCS0_CLR_OUTPKT_RDY;
-
        ao_usb_ep0_in_data = ao_usb_ep0_in_buf;
        ao_usb_ep0_in_len = 0;
        switch(ao_usb_setup.dir_type_recip & AO_USB_SETUP_TYPE_MASK) {
@@ -274,10 +254,39 @@ ao_usb_ep0_setup(void)
                }
                break;
        }
-       if (ao_usb_ep0_state != AO_USB_EP0_DATA_OUT) {
+
+       /* Figure out how to ACK the setup packet and the
+        * next state
+        */
+       USBINDEX = 0;
+       if (ao_usb_ep0_in_len) {
+
+               /* Sending data back to the host
+                */
+               ao_usb_ep0_state = AO_USB_EP0_DATA_IN;
+               USBCS0 = USBCS0_CLR_OUTPKT_RDY;
                if (ao_usb_setup.length < ao_usb_ep0_in_len)
                        ao_usb_ep0_in_len = ao_usb_setup.length;
                ao_usb_ep0_flush();
+       } else if (ao_usb_ep0_out_len) {
+               
+               /* Receiving data from the host
+                */
+               ao_usb_ep0_state = AO_USB_EP0_DATA_OUT;
+               USBCS0 = USBCS0_CLR_OUTPKT_RDY;
+       } else if (ao_usb_setup.length) {
+
+               /* Uh-oh, the host expected to send or receive data
+                * and we don't know what to do.
+                */
+               ao_usb_ep0_state = AO_USB_EP0_STALL;
+               USBCS0 = USBCS0_CLR_OUTPKT_RDY | USBCS0_SEND_STALL;
+       } else {
+
+               /* Simple setup packet with no data
+                */
+               ao_usb_ep0_state = AO_USB_EP0_IDLE;
+               USBCS0 = USBCS0_CLR_OUTPKT_RDY | USBCS0_DATA_END;
        }
 }
 
@@ -299,12 +308,12 @@ ao_usb_ep0(void)
                USBINDEX = 0;
                cs0 = USBCS0;
                if (cs0 & USBCS0_SETUP_END) {
-                       ao_usb_ep0_state = AO_USB_EP0_IDLE;
                        USBCS0 = USBCS0_CLR_SETUP_END;
+                       ao_usb_ep0_state = AO_USB_EP0_IDLE;
                }
                if (cs0 & USBCS0_SENT_STALL) {
+                       USBCS0 = 0;
                        ao_usb_ep0_state = AO_USB_EP0_IDLE;
-                       USBCS0 &= ~USBCS0_SENT_STALL;
                }
                if (ao_usb_ep0_state == AO_USB_EP0_DATA_IN &&
                    (cs0 & USBCS0_INPKT_RDY) == 0)
@@ -318,12 +327,11 @@ ao_usb_ep0(void)
                                break;
                        case AO_USB_EP0_DATA_OUT:
                                ao_usb_ep0_fill();
-                               if (ao_usb_ep0_out_len == 0)
-                                       ao_usb_ep0_state = AO_USB_EP0_IDLE;
                                USBINDEX = 0;
-                               if (ao_usb_ep0_state == AO_USB_EP0_IDLE)
+                               if (ao_usb_ep0_out_len == 0) {
+                                       ao_usb_ep0_state = AO_USB_EP0_IDLE;
                                        USBCS0 = USBCS0_CLR_OUTPKT_RDY | USBCS0_DATA_END;
-                               else
+                               } else
                                        USBCS0 = USBCS0_CLR_OUTPKT_RDY;
                                break;
                        }

diff --git a/src/core/ao_usb.h b/src/core/ao_usb.h
index 4476ee6be99f4ffd76db8f6d651ff2171616d989..6bc77608ef331a8782b6be051c948deab9cdcc52 100644 (file)
--- a/src/core/ao_usb.h
+++ b/src/core/ao_usb.h
@@ -114,6 +114,7 @@ extern __code __at (0x00aa) uint8_t ao_usb_descriptors [];
 #define AO_USB_EP0_IDLE                0
 #define AO_USB_EP0_DATA_IN     1
 #define AO_USB_EP0_DATA_OUT    2
+#define AO_USB_EP0_STALL       3
 
 #define LE_WORD(x)    ((x)&0xFF),((uint8_t) (((uint16_t) (x))>>8))