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

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

[Juha Kuikka]

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.

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

[Mike Frysinger]

On Monday 11 January 2010 15:00:40 Juha Kuikka wrote:
> 2. 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100111/f787f2ae/attachment.pgp 

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

[Tom]

Juha Kuikka wrote:
> 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
> +

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