[U-Boot] [RFC] MUSB-UDC issues and fixes

Hi,
I ran into some issues with the MUSB-UDC support while adding a new Davinci DM850/OMAP-L138 board:
1. Move endpoint count into SoC specific musb-header file. This implementation only has 4 instead of 15 OMAP2/3 has. 2. On DM850 the MUSB interrupts are read from a wrapper, not from MUSB block . Add optional interface function to read-and-clear interrupts. 3. Problem setting FADDR - in udc_irq() the musb_peri_ep0_set_address() gets called on the same run as the ep0 SET_ADDRESS request is handled. This causes the FADDR to change before the status stage interrupt is received and USB session fails. Fixed by removing extra call to musb_peri_ep0() from udc_irq(). It should only be called when EP0 interrupt is present. 4. udc_setup_ep() has a problem when id != 0, epinfo[] is indexed too far, this results in wrong EPs configured.
This is not a final patch, just asking for comments.
I am investigating another issue where rx endpoint stops receiving data when CDC ACM console is used. This happens under heavy inbound data conditions, namely using "loads".
------------------------- diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c index fc43cf4..b6f9bff 100644 --- a/drivers/usb/musb/musb_udc.c +++ b/drivers/usb/musb/musb_udc.c @@ -65,8 +65,6 @@ /* #define MUSB_DEBUG */ #include "musb_debug.h"
-#define MAX_ENDPOINT 15 - #define GET_ENDPOINT(dev,ep) \ (((struct usb_device_instance *)(dev))->bus->endpoint_array + ep)
@@ -157,6 +155,17 @@ static void musb_db_regs(void) #define musb_db_regs() #endif /* DEBUG_MUSB */
+#ifndef CONFIG_MUSB_SOC_IRQ +static void musb_get_interrupts(u8 *usb, u16 *rx, u16 *tx) +{ + *usb = readb(&musbr->intrusb); + *rx = readw(&musbr->intrrx); + *tx = readw(&musbr->intrtx); +} +#else +extern void musb_get_interrupts(u8 *usb, u16 *rx, u16 *tx); +#endif + static void musb_peri_softconnect(void) { u8 power, devctl; @@ -169,9 +178,7 @@ static void musb_peri_softconnect(void) writeb(power, &musbr->power);
/* Read intr to clear */ - intrusb = readb(&musbr->intrusb); - intrrx = readw(&musbr->intrrx); - intrtx = readw(&musbr->intrtx); + musb_get_interrupts(&intrusb, &intrrx, &intrtx);
udelay(1000 * 1000); /* 1 sec */
@@ -713,7 +720,7 @@ static void musb_peri_tx(u16 intr) { /* Check for EP0 */ if (0x01 & intr) - musb_peri_ep0_tx(); + musb_peri_ep0();
/* * Use this in the future when handling epN tx @@ -733,8 +740,9 @@ void udc_irq(void) /* This is a high freq called function */ if (enabled) { u8 intrusb; + u16 intrrx, intrtx;
- intrusb = readb(&musbr->intrusb); + musb_get_interrupts(&intrusb, &intrrx, &intrtx);
/* * See drivers/usb/gadget/mpc8xx_udc.c for @@ -747,8 +755,6 @@ void udc_irq(void) musb_peri_resume(); }
- musb_peri_ep0(); - if (MUSB_INTR_RESET & intrusb) { usbd_device_event_irq(udc_device, DEVICE_RESET, 0); musb_peri_reset(); @@ -773,10 +779,6 @@ void udc_irq(void) }
if (ep0_state != SET_ADDRESS) { - u16 intrrx, intrtx; - - intrrx = readw(&musbr->intrrx); - intrtx = readw(&musbr->intrtx);
if (intrrx) musb_peri_rx(intrrx); @@ -874,7 +876,7 @@ void udc_setup_ep(struct usb_device_instance *device, unsigned int id, ep0_urb = usbd_alloc_urb(device, endpoint); } else if (MAX_ENDPOINT >= id) { int ep_addr; - + id--; /* Check the direction */ ep_addr = endpoint->endpoint_address; if (USB_DIR_IN == (ep_addr & USB_ENDPOINT_DIR_MASK)) {
-------------------------
- Juha
-- Madness takes it's toll. Please have exact change.

On Monday 11 January 2010 15:00:40 Juha Kuikka wrote:
- On DM850 the MUSB interrupts are read from a wrapper, not from MUSB
block . Add optional interface function to read-and-clear interrupts.
+#ifndef CONFIG_MUSB_SOC_IRQ +static void musb_get_interrupts(u8 *usb, u16 *rx, u16 *tx) +{
- *usb = readb(&musbr->intrusb);
- *rx = readw(&musbr->intrrx);
- *tx = readw(&musbr->intrtx);
+} +#else +extern void musb_get_interrupts(u8 *usb, u16 *rx, u16 *tx); +#endif
this looks like a static MUSB implementation rather than a config option a user/board would control, right ? so perhaps we should stick with the "internal" symbol naming rather than CONFIG_MUSB_xxx ... e.g. the way we're handling dynamic fifos and/or multipoint. -mike

Juha Kuikka wrote:
Hi,
I ran into some issues with the MUSB-UDC support while adding a new Davinci DM850/OMAP-L138 board:
- Move endpoint count into SoC specific musb-header file. This
implementation only has 4 instead of 15 OMAP2/3 has. 2. On DM850 the MUSB interrupts are read from a wrapper, not from MUSB block . Add optional interface function to read-and-clear interrupts. 3. Problem setting FADDR - in udc_irq() the musb_peri_ep0_set_address() gets called on the same run as the ep0 SET_ADDRESS request is handled. This causes the FADDR to change before the status stage interrupt is received and USB session fails. Fixed by removing extra call to musb_peri_ep0() from udc_irq(). It should only be called when EP0 interrupt is present. 4. udc_setup_ep() has a problem when id != 0, epinfo[] is indexed too far, this results in wrong EPs configured.
This is not a final patch, just asking for comments.
I am investigating another issue where rx endpoint stops receiving data when CDC ACM console is used. This happens under heavy inbound data conditions, namely using "loads".
diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c index fc43cf4..b6f9bff 100644 --- a/drivers/usb/musb/musb_udc.c +++ b/drivers/usb/musb/musb_udc.c @@ -65,8 +65,6 @@ /* #define MUSB_DEBUG */ #include "musb_debug.h"
-#define MAX_ENDPOINT 15
#define GET_ENDPOINT(dev,ep) \ (((struct usb_device_instance *)(dev))->bus->endpoint_array + ep)
@@ -157,6 +155,17 @@ static void musb_db_regs(void) #define musb_db_regs() #endif /* DEBUG_MUSB */
+#ifndef CONFIG_MUSB_SOC_IRQ +static void musb_get_interrupts(u8 *usb, u16 *rx, u16 *tx) +{
- *usb = readb(&musbr->intrusb);
- *rx = readw(&musbr->intrrx);
- *tx = readw(&musbr->intrtx);
+} +#else +extern void musb_get_interrupts(u8 *usb, u16 *rx, u16 *tx); +#endif
The reading of INT registers clears them. If all you want in the usb INT's, you will wipe out the rx/tx to get them.
Tom
participants (3)
-
Juha Kuikka
-
Mike Frysinger
-
Tom