[U-Boot] [PATCH] USB: musb_udc: Bugfix musb_peri_rx_ep
From: Pankaj Bharadiya pankaj.bharadiya@ti.com
The endpoint rx count register value will be zero if it is read before receive packet ready bit (PERI_RXCSR:RXPKTRDY) is set.
Check for the receive packet ready bit (PERI_RXCSR:RXPKTRDY) before reading endpoint rx count register. Proceed with rx count read and FIFO read only if RXPKTRDY bit is set.
As this makes the function fit less-well within 80 columns, use __func__ in some debug statements rather than __PRETTY_FUNCTION__ as they are identical for C programs.
Signed-off-by: Pankaj Bharadiya pankaj.bharadiya@ti.com Signed-off-by: Tom Rini trini@ti.com --- drivers/usb/musb/musb_udc.c | 97 +++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c index 09cdec3..7180de8 100644 --- a/drivers/usb/musb/musb_udc.c +++ b/drivers/usb/musb/musb_udc.c @@ -640,58 +640,65 @@ static void musb_peri_ep0(void)
static void musb_peri_rx_ep(unsigned int ep) { - u16 peri_rxcount = readw(&musbr->ep[ep].epN.rxcount); - - if (peri_rxcount) { - struct usb_endpoint_instance *endpoint; - u32 length; - u8 *data; - - endpoint = GET_ENDPOINT(udc_device, ep); - if (endpoint && endpoint->rcv_urb) { - struct urb *urb = endpoint->rcv_urb; - unsigned int remaining_space = urb->buffer_length - - urb->actual_length; - - if (remaining_space) { - int urb_bad = 0; /* urb is good */ - - if (peri_rxcount > remaining_space) - length = remaining_space; - else - length = peri_rxcount; - - data = (u8 *) urb->buffer_data; - data += urb->actual_length; - - /* The common musb fifo reader */ - read_fifo(ep, length, data); - - musb_peri_rx_ack(ep); - - /* - * urb's actual_length is updated in - * usbd_rcv_complete - */ - usbd_rcv_complete(endpoint, length, urb_bad); + u8 peri_rxcsr = readw(&musbr->ep[ep].epN.rxcsr); + if ((peri_rxcsr & MUSB_RXCSR_RXPKTRDY)) { + u16 peri_rxcount = readw(&musbr->ep[ep].epN.rxcount); + if (peri_rxcount) { + struct usb_endpoint_instance *endpoint; + u32 length; + u8 *data;
+ endpoint = GET_ENDPOINT(udc_device, ep); + if (endpoint && endpoint->rcv_urb) { + struct urb *urb = endpoint->rcv_urb; + unsigned int remaining_space = + urb->buffer_length - + urb->actual_length; + + if (remaining_space) { + int urb_bad = 0; /* urb is good */ + + if (peri_rxcount > remaining_space) + length = remaining_space; + else + length = peri_rxcount; + + data = (u8 *) urb->buffer_data; + data += urb->actual_length; + + /* The common musb fifo reader */ + read_fifo(ep, length, data); + + musb_peri_rx_ack(ep); + + /* + * urb's actual_length is updated in + * usbd_rcv_complete + */ + usbd_rcv_complete(endpoint, length, + urb_bad); + + } else { + if (debug_level > 0) + serial_printf("ERROR : %s %d" + " no space " + "in rcv " + "buffer\n", + __func__, + ep); + } } else { if (debug_level > 0) - serial_printf("ERROR : %s %d no space " - "in rcv buffer\n", - __PRETTY_FUNCTION__, ep); + serial_printf("ERROR : %s %d problem " + "with endpoint\n", + __func__, ep); + } } else { if (debug_level > 0) - serial_printf("ERROR : %s %d problem with " - "endpoint\n", - __PRETTY_FUNCTION__, ep); + serial_printf("ERROR : %s %d with nothing " + "to do\n", __func__, ep); } - - } else { - if (debug_level > 0) - serial_printf("ERROR : %s %d with nothing to do\n", - __PRETTY_FUNCTION__, ep); } }
On Thu, 13 Sep 2012 10:26:31 -0700 Tom Rini trini@ti.com wrote:
} else {if (debug_level > 0)serial_printf("ERROR : %s %d"" no space ""in rcv ""buffer\n",__func__,ep);}
that's pretty bad.
strings are allowed to exceed the 80 column limit, for grep purposes.
also, if the conditional logic is reversed, one can regain some of that horizontal space, i.e.:
if (!(peri_rxcsr & MUSB_RXCSR_RXPKTRDY)) { if (debug_level > 0) serial_printf("ERROR : %s %d with nothing to do\n", __PRETTY_FUNCTION__, ep); return; } peri_rxcount = readw(&musbr->ep[ep].epN.rxcount); if (!peri_rxcount) if (debug_level > 0) { serial_printf("ERROR : %s %d problem with endpoint\n", __PRETTY_FUNCTION__, ep); serial_printf("ERROR : %s %d with nothing to do\n", __func__, ep); return; }
...and so on.
Kim
On Thu, Sep 13, 2012 at 02:01:43PM -0500, Kim Phillips wrote:
On Thu, 13 Sep 2012 10:26:31 -0700 Tom Rini trini@ti.com wrote:
} else {if (debug_level > 0)serial_printf("ERROR : %s %d"" no space ""in rcv ""buffer\n",__func__,ep);}that's pretty bad.
Yes, it is.
strings are allowed to exceed the 80 column limit, for grep purposes.
Good point, just following existing style to the extreme in this case.
also, if the conditional logic is reversed, one can regain some of that horizontal space, i.e.:
An execellent point. I'll post v2 which will debug print and return if the bit is not set.
Dear Tom Rini,
From: Pankaj Bharadiya pankaj.bharadiya@ti.com
The endpoint rx count register value will be zero if it is read before receive packet ready bit (PERI_RXCSR:RXPKTRDY) is set.
Check for the receive packet ready bit (PERI_RXCSR:RXPKTRDY) before reading endpoint rx count register. Proceed with rx count read and FIFO read only if RXPKTRDY bit is set.
As this makes the function fit less-well within 80 columns, use __func__ in some debug statements rather than __PRETTY_FUNCTION__ as they are identical for C programs.
I'd say this is TI stuff, so let's push it through TI tree (both the musb patches please).
Best regards, Marek Vasut
Dear Tom Rini,
From: Pankaj Bharadiya pankaj.bharadiya@ti.com
The endpoint rx count register value will be zero if it is read before receive packet ready bit (PERI_RXCSR:RXPKTRDY) is set.
Check for the receive packet ready bit (PERI_RXCSR:RXPKTRDY) before reading endpoint rx count register. Proceed with rx count read and FIFO read only if RXPKTRDY bit is set.
As this makes the function fit less-well within 80 columns, use __func__ in some debug statements rather than __PRETTY_FUNCTION__ as they are identical for C programs.
Signed-off-by: Pankaj Bharadiya pankaj.bharadiya@ti.com Signed-off-by: Tom Rini trini@ti.com
drivers/usb/musb/musb_udc.c | 97 +++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c index 09cdec3..7180de8 100644 --- a/drivers/usb/musb/musb_udc.c +++ b/drivers/usb/musb/musb_udc.c @@ -640,58 +640,65 @@ static void musb_peri_ep0(void)
static void musb_peri_rx_ep(unsigned int ep) {
- u16 peri_rxcount = readw(&musbr->ep[ep].epN.rxcount);
- if (peri_rxcount) {
struct usb_endpoint_instance *endpoint;u32 length;u8 *data;endpoint = GET_ENDPOINT(udc_device, ep);if (endpoint && endpoint->rcv_urb) {struct urb *urb = endpoint->rcv_urb;unsigned int remaining_space = urb->buffer_length -urb->actual_length;if (remaining_space) {int urb_bad = 0; /* urb is good */if (peri_rxcount > remaining_space)length = remaining_space;elselength = peri_rxcount;data = (u8 *) urb->buffer_data;data += urb->actual_length;/* The common musb fifo reader */read_fifo(ep, length, data);musb_peri_rx_ack(ep);/** urb's actual_length is updated in* usbd_rcv_complete*/usbd_rcv_complete(endpoint, length, urb_bad);
- u8 peri_rxcsr = readw(&musbr->ep[ep].epN.rxcsr);
- if ((peri_rxcsr & MUSB_RXCSR_RXPKTRDY)) {
if (!cond) return;
This will cut down one indent below.
u16 peri_rxcount = readw(&musbr->ep[ep].epN.rxcount);if (peri_rxcount) {struct usb_endpoint_instance *endpoint;u32 length;u8 *data;endpoint = GET_ENDPOINT(udc_device, ep);if (endpoint && endpoint->rcv_urb) {struct urb *urb = endpoint->rcv_urb;unsigned int remaining_space =urb->buffer_length -urb->actual_length;if (remaining_space) {int urb_bad = 0; /* urb is good */if (peri_rxcount > remaining_space)length = remaining_space;elselength = peri_rxcount;data = (u8 *) urb->buffer_data;data += urb->actual_length;/* The common musb fifo reader */read_fifo(ep, length, data);musb_peri_rx_ack(ep);/** urb's actual_length is updated in* usbd_rcv_complete*/usbd_rcv_complete(endpoint, length,urb_bad);} else {if (debug_level > 0)serial_printf("ERROR : %s %d"" no space ""in rcv ""buffer\n",__func__,ep);
So ... if (error) print error and die.
if (!remaining_space && debug_level) print and die.
{ rest of the function, as above }
One more indent level down.
Apply common sense to the other else {} and it's work out.
} } else { if (debug_level > 0)
serial_printf("ERROR : %s %d no space ""in rcv buffer\n",__PRETTY_FUNCTION__, ep);
serial_printf("ERROR : %s %d problem "
use printf(), serial_printf is flat out forbidden and this is a NACK /!\
"with endpoint\n",__func__, ep);}
serial_printf("ERROR : %s %d problem with ""endpoint\n",__PRETTY_FUNCTION__, ep);
serial_printf("ERROR : %s %d with nothing ""to do\n", __func__, ep);
- } else {
if (debug_level > 0)serial_printf("ERROR : %s %d with nothing to do\n",__PRETTY_FUNCTION__, ep);}
Best regards, Marek Vasut
participants (3)
-
Kim Phillips -
Marek Vasut -
Tom Rini