USB: musb_udc: Bugfix musb_peri_rx_ep
Subsystems (2)
PATCH 1/1 USB: musb_udc: Bugfix musb_peri_rx_ep
From: Pankaj Bharadiya <[email protected]>
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.
--- drivers/usb/musb/musb_udc.c | 97 +++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 45 deletions(-)
drivers/usb/musb/musb_udc.c +52 -45
@@ -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); } }
Signature · 1 line
1.7.9.5
PATCH 1/1 USB: musb_udc: Bugfix musb_peri_rx_ep
drivers/usb/musb/musb_udc.c static void musb_peri_ep0(void) · line 689
Quoted 9 lines · On Thu, 13 Sep 2012 10:26:31 -0700 Tom Rini <[email protected]> wrote:
drivers/usb/musb/musb_udc.c +9
-0+ }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
PATCH 1/1 USB: musb_udc: Bugfix musb_peri_rx_ep
Quoted 14 lines · Kim Phillips
drivers/usb/musb/musb_udc.c static void musb_peri_ep0(void) · line 689
Quoted 9 lines · On Thu, 13 Sep 2012 10:26:31 -0700 Tom Rini <[email protected]> wrote:
drivers/usb/musb/musb_udc.c +9
-0+ }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.
Quoted 1 line
strings are allowed to exceed the 80 column limit, for grep purposes.
Good point, just following existing style to the extreme in this case.
Quoted 2 lines
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.
Signature · 1 line
Tom
PATCH 1/1 USB: musb_udc: Bugfix musb_peri_rx_ep
In reply to Tom RiniDear Tom Rini,
Quoted 5 lines
From: Pankaj Bharadiya <[email protected]>
The endpoint rx count register value will be zero if it is read before receive packet ready bit (PERI_RXCSR:RXPKTRDY) is set.
Quoted 7 lines
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
PATCH 1/1 USB: musb_udc: Bugfix musb_peri_rx_ep
In reply to Tom RiniDear Tom Rini,
Quoted 54 lines
From: Pankaj Bharadiya <[email protected]>
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 <[email protected]>Signed-off-by Tom Rini <[email protected]>--- drivers/usb/musb/musb_udc.c | 97+++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 45 deletions(-)
drivers/usb/musb/musb_udc.c +0
-26@@ -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);
drivers/usb/musb/musb_udc.c static void musb_peri_ep0(void) · line 644
Quoted 10 lines
drivers/usb/musb/musb_udc.c +2
-8- -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.
Quoted 34 lines
drivers/usb/musb/musb_udc.c +33
-0+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,
drivers/usb/musb/musb_udc.c static void musb_peri_ep0(void) · line 688
Quoted 10 lines
drivers/usb/musb/musb_udc.c +10
-0+ 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.
drivers/usb/musb/musb_udc.c static void musb_peri_ep0(void) · line 692
Quoted 7 lines
drivers/usb/musb/musb_udc.c +2
-3+ } }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 /!\
Quoted 9 lines
drivers/usb/musb/musb_udc.c +3
-3+"with endpoint\n" , + __func__, ep); + } }else {if (debug_level >0 ) -serial_printf ("ERROR : %s %d problem with " -"endpoint\n" , - __PRETTY_FUNCTION__, ep);
drivers/usb/musb/musb_udc.c static void musb_peri_ep0(void) · line 703
Quoted 10 lines
drivers/usb/musb/musb_udc.c +2
-5+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