From 8a19805a6b079450b5afd5fa2334cede8495ae4a Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 17 May 2013 03:21:08 -0700 Subject: [PATCH] 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 --- src/cc1111/ao_usb.c | 68 +++++++++++++++++++++++++-------------------- src/core/ao_usb.h | 1 + 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/src/cc1111/ao_usb.c b/src/cc1111/ao_usb.c index 8bd2efdf..a655d1be 100644 --- 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 4476ee6b..6bc77608 100644 --- 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)) -- 2.30.2