[PATCH 0/4] usbtty/musb: Fix file transfers

[PATCH 0/4] usbtty/musb: Fix file transfers

[Pali Rohár]

This is second usbtty patch series which is fixing stability of musb code
and allows usage of file transfers via Kermit protocol on Nokia N900.

Kermit file transfer via U-Boot loadb command is stable on Nokia N900 and
gives about 52kB/s transfer rate.

This patch series depends on previous patch series:
"Nokia RX-51: Fix USB TTY console and enable it"

Pali Roh?r (4):
  serial: usbtty: Send urb data in correct order
  usb: musb: Fix receiving of bigger buffers
  usb: musb: Fix handling interrupts for EP0 and SET ADDRESS commmand
  usb: musb: Ensure that we set musb dynamic FIFO buffer for every
    endpoint

 drivers/serial/usbtty.c     | 12 +++--------
 drivers/usb/musb/musb_udc.c | 42 ++++++++++++++++++-------------------
 2 files changed, 24 insertions(+), 30 deletions(-)

-- 
2.20.1

[PATCH 1/4] serial: usbtty: Send urb data in correct order

[Pali Rohár]

Function next_urb() selects the last urb data buffer from linked list to
which next data from usbtty puts should be appended.

But to check if TX data still exists it is needed to look at the first urb
data buffer from linked list. So check for endpoint->tx_urb (first from the
linked list) instead of current_urb (the last from the linked list).

Successful call to udc_endpoint_write() may invalidate active urb and
allocate new in queue which invalidate pointer returned by next_urb()
function.

So call next_urb() prior putting data into urb buffer and call it every
time after using udc_endpoint_write() function to prevent sending data from
usbtty puts in incorrect order.

This patch fixes issue that usbtty code does not transmit data when they
are waiting in the tx queue.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/serial/usbtty.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c
index 02f8edf200..4f4eb02de0 100644
--- a/drivers/serial/usbtty.c
+++ b/drivers/serial/usbtty.c
@@ -849,17 +849,9 @@ static int write_buffer (circbuf_t * buf)
 			&endpoint_instance[tx_endpoint];
 	struct urb *current_urb = NULL;
 
-	current_urb = next_urb (device_instance, endpoint);
-
-	if (!current_urb) {
-		TTYERR ("current_urb is NULL, buf->size %d\n",
-		buf->size);
-		return 0;
-	}
-
 	/* TX data still exists - send it now
 	 */
-	if(endpoint->sent < current_urb->actual_length){
+	if(endpoint->sent < endpoint->tx_urb->actual_length){
 		if(udc_endpoint_write (endpoint)){
 			/* Write pre-empted by RX */
 			return -1;
@@ -878,6 +870,8 @@ static int write_buffer (circbuf_t * buf)
 		 */
 		while (buf->size > 0) {
 
+			current_urb = next_urb (device_instance, endpoint);
+
 			dest = (char*)current_urb->buffer +
 				current_urb->actual_length;
 
-- 
2.20.1

[PATCH 2/4] usb: musb: Fix receiving of bigger buffers

[Pali Rohár]

If musb_peri_rx_ep() was called to processed received HW buffer but U-Boot
cannot read it yet (e.g. because U-Boot SW buffer is full) then interrupt
was marked as processed but HW buffer stayed unprocessed.

U-Boot tried to process this buffer again when it receive interrupt again,
but it can receive it only when sender (host) send a new data. As sender
(host) is not going to send a new data until U-Boot process current data
this issue caused a deadlock in case sender (host) is emitting data faster
than U-Boot can process it.

Reading musb intrrx register automatically clears this register and mark
interrupt as processed. So to prevent marking interrupt in U-Boot as
processed and a new variable pending_intrrx which would contain unprocessed
bits of intrrx register.

And as a second step, every time when musb_peri_rx_ep() is called and there
are waiting data to be processed (signaled by MUSB_RXCSR_RXPKTRDY) either
acknowledge sender (via musb_peri_rx_ack()) that whole HW buffer was
processed or set corresponding bit in pending_intrrx that HW buffer was not
fully processed yet and next iteration is required after U-Boot allocate
space for reading HW buffer.

This patch fixes receiving large usb buffers, e.g. file transfer via Kermit
protocol implemented by 'loadb' U-Boot command over usbtty serial console.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/usb/musb/musb_udc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c
index 28719cc3f6..7c74422623 100644
--- a/drivers/usb/musb/musb_udc.c
+++ b/drivers/usb/musb/musb_udc.c
@@ -104,6 +104,8 @@ struct usb_endpoint_instance *ep0_endpoint;
 static struct usb_device_instance *udc_device;
 static int enabled;
 
+u16 pending_intrrx;
+
 #ifdef MUSB_DEBUG
 static void musb_db_regs(void)
 {
@@ -664,7 +666,10 @@ static void musb_peri_rx_ep(unsigned int ep)
 				/* The common musb fifo reader */
 				read_fifo(ep, length, data);
 
-				musb_peri_rx_ack(ep);
+				if (length == peri_rxcount)
+					musb_peri_rx_ack(ep);
+				else
+					pending_intrrx |= (1 << ep);
 
 				/*
 				 * urb's actual_length is updated in
@@ -677,18 +682,24 @@ static void musb_peri_rx_ep(unsigned int ep)
 					serial_printf("ERROR : %s %d no space "
 						      "in rcv buffer\n",
 						      __PRETTY_FUNCTION__, ep);
+
+				pending_intrrx |= (1 << ep);
 			}
 		} else {
 			if (debug_level > 0)
 				serial_printf("ERROR : %s %d problem with "
 					      "endpoint\n",
 					      __PRETTY_FUNCTION__, ep);
+
+			pending_intrrx |= (1 << ep);
 		}
 
 	} else {
 		if (debug_level > 0)
 			serial_printf("ERROR : %s %d with nothing to do\n",
 				      __PRETTY_FUNCTION__, ep);
+
+		musb_peri_rx_ack(ep);
 	}
 }
 
@@ -770,6 +781,9 @@ void udc_irq(void)
 			intrrx = readw(&musbr->intrrx);
 			intrtx = readw(&musbr->intrtx);
 
+			intrrx |= pending_intrrx;
+			pending_intrrx = 0;
+
 			if (intrrx)
 				musb_peri_rx(intrrx);
 
-- 
2.20.1

[PATCH 3/4] usb: musb: Fix handling interrupts for EP0 and SET ADDRESS commmand

[Pali Rohár]

Interrupt for EP0 is indicated in intrtx register via first bit. It is set
for both RX and TX. First bit in intrrx register is reserved and not set.

So remove calling musb_peri_ep0() function at every iteration of udc_irq()
and musb_peri_rx() and call it only from musb_peri_tx() when correct
interrupt bit in initrtx it set.

Address from SET ADDRESS command must be set to faddr register only after
acknowledging SERV_RXPKTRDY followed by received EP0 interrupt. So prior
calling musb_peri_ep0_set_address() check for EP0 interrupt instead of
(incorrect) MUSB_INTR_SOF interrupt.

This patch fixes issue that host (computer) cannot register U-Boot USB
device and failing with errors:

    usb 1-1: new full-speed USB device number 86 using xhci_hcd
    usb 1-1: Device not responding to setup address.
    usb 1-1: Device not responding to setup address.
    usb 1-1: device not accepting address 86, error -71

U-Boot was writing address to faddr register too early and did not wait for
correct interrupt after which should update address.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/usb/musb/musb_udc.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c
index 7c74422623..50d8bc319c 100644
--- a/drivers/usb/musb/musb_udc.c
+++ b/drivers/usb/musb/musb_udc.c
@@ -707,9 +707,7 @@ static void musb_peri_rx(u16 intr)
 {
 	unsigned int ep;
 
-	/* Check for EP0 */
-	if (0x01 & intr)
-		musb_peri_ep0();
+	/* First bit is reserved and does not indicate interrupt for EP0 */
 
 	for (ep = 1; ep < 16; ep++) {
 		if ((1 << ep) & intr)
@@ -721,9 +719,9 @@ static void musb_peri_tx(u16 intr)
 {
 	unsigned int ep;
 
-	/* Check for EP0 */
+	/* Check for EP0: first bit indicates interrupt for both RX and TX */
 	if (0x01 & intr)
-		musb_peri_ep0_tx();
+		musb_peri_ep0();
 
 	for (ep = 1; ep < 16; ep++) {
 		if ((1 << ep) & intr)
@@ -750,8 +748,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();
@@ -790,7 +786,7 @@ void udc_irq(void)
 			if (intrtx)
 				musb_peri_tx(intrtx);
 		} else {
-			if (MUSB_INTR_SOF & intrusb) {
+			if (readw(&musbr->intrtx) & 0x1) {
 				u8 faddr;
 				faddr = readb(&musbr->faddr);
 				/*
-- 
2.20.1

[PATCH 4/4] usb: musb: Ensure that we set musb dynamic FIFO buffer for every endpoint

[Pali Rohár]

If we do not set FIFO buffer address and size for some endpoint which is in
use then default address 0x0 would be used which is in conflict with FIFO
buffer for endpoint 0 which is at fixed address 0x0. Sharing address space
between more endpoint cause data loss and unexpected errors.

This patch is fixing transmission of characters over usbtty serial console
and allow using of usbtty for debugging purposes on Nokia N900.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/usb/musb/musb_udc.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c
index 50d8bc319c..ea1284850e 100644
--- a/drivers/usb/musb/musb_udc.c
+++ b/drivers/usb/musb/musb_udc.c
@@ -875,18 +875,8 @@ void udc_setup_ep(struct usb_device_instance *device, unsigned int id,
 		ep0_endpoint->endpoint_address = 0xff;
 		ep0_urb = usbd_alloc_urb(device, endpoint);
 	} else if (MAX_ENDPOINT >= id) {
-		int ep_addr;
-
-		/* Check the direction */
-		ep_addr = endpoint->endpoint_address;
-		if (USB_DIR_IN == (ep_addr & USB_ENDPOINT_DIR_MASK)) {
-			/* IN */
-			epinfo[(id * 2) + 1].epsize = endpoint->tx_packetSize;
-		} else {
-			/* OUT */
-			epinfo[id * 2].epsize = endpoint->rcv_packetSize;
-		}
-
+		epinfo[(id * 2) + 0].epsize = endpoint->rcv_packetSize;
+		epinfo[(id * 2) + 1].epsize = endpoint->tx_packetSize;
 		musb_configure_ep(&epinfo[0], ARRAY_SIZE(epinfo));
 	} else {
 		if (debug_level > 0)
-- 
2.20.1

[PATCH 3/4] usb: musb: Fix handling interrupts for EP0 and SET ADDRESS commmand

[Lukasz Majewski]

Hi Pali,

> Interrupt for EP0 is indicated in intrtx register via first bit. It
> is set for both RX and TX. First bit in intrrx register is reserved
> and not set.
> 
> So remove calling musb_peri_ep0() function at every iteration of
> udc_irq() and musb_peri_rx() and call it only from musb_peri_tx()
> when correct interrupt bit in initrtx it set.
> 
> Address from SET ADDRESS command must be set to faddr register only
> after acknowledging SERV_RXPKTRDY followed by received EP0 interrupt.
> So prior calling musb_peri_ep0_set_address() check for EP0 interrupt
> instead of (incorrect) MUSB_INTR_SOF interrupt.
> 
> This patch fixes issue that host (computer) cannot register U-Boot USB
> device and failing with errors:
> 
>     usb 1-1: new full-speed USB device number 86 using xhci_hcd
>     usb 1-1: Device not responding to setup address.
>     usb 1-1: Device not responding to setup address.
>     usb 1-1: device not accepting address 86, error -71
> 
> U-Boot was writing address to faddr register too early and did not
> wait for correct interrupt after which should update address.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
> ---
>  drivers/usb/musb/musb_udc.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c
> index 7c74422623..50d8bc319c 100644
> --- a/drivers/usb/musb/musb_udc.c
> +++ b/drivers/usb/musb/musb_udc.c
> @@ -707,9 +707,7 @@ static void musb_peri_rx(u16 intr)
>  {
>  	unsigned int ep;
>  
> -	/* Check for EP0 */
> -	if (0x01 & intr)
> -		musb_peri_ep0();
> +	/* First bit is reserved and does not indicate interrupt for
> EP0 */ 
>  	for (ep = 1; ep < 16; ep++) {
>  		if ((1 << ep) & intr)
> @@ -721,9 +719,9 @@ static void musb_peri_tx(u16 intr)
>  {
>  	unsigned int ep;
>  
> -	/* Check for EP0 */
> +	/* Check for EP0: first bit indicates interrupt for both RX
> and TX */ if (0x01 & intr)
> -		musb_peri_ep0_tx();
> +		musb_peri_ep0();
>  
>  	for (ep = 1; ep < 16; ep++) {
>  		if ((1 << ep) & intr)
> @@ -750,8 +748,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();
> @@ -790,7 +786,7 @@ void udc_irq(void)
>  			if (intrtx)
>  				musb_peri_tx(intrtx);
>  		} else {
> -			if (MUSB_INTR_SOF & intrusb) {
> +			if (readw(&musbr->intrtx) & 0x1) {
>  				u8 faddr;
>  				faddr = readb(&musbr->faddr);
>  				/*

Applying this patch causes error. Could you rebase your work on newest
master (after onging PR is accepted).

Thanks in advance and sorry for delay.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210123/7bae5f10/attachment.sig>

[PATCH 3/4] usb: musb: Fix handling interrupts for EP0 and SET ADDRESS commmand

[Pali Rohár]

On Saturday 23 January 2021 16:16:11 Lukasz Majewski wrote:
> Hi Pali,
> 
> > Interrupt for EP0 is indicated in intrtx register via first bit. It
> > is set for both RX and TX. First bit in intrrx register is reserved
> > and not set.
> > 
> > So remove calling musb_peri_ep0() function at every iteration of
> > udc_irq() and musb_peri_rx() and call it only from musb_peri_tx()
> > when correct interrupt bit in initrtx it set.
> > 
> > Address from SET ADDRESS command must be set to faddr register only
> > after acknowledging SERV_RXPKTRDY followed by received EP0 interrupt.
> > So prior calling musb_peri_ep0_set_address() check for EP0 interrupt
> > instead of (incorrect) MUSB_INTR_SOF interrupt.
> > 
> > This patch fixes issue that host (computer) cannot register U-Boot USB
> > device and failing with errors:
> > 
> >     usb 1-1: new full-speed USB device number 86 using xhci_hcd
> >     usb 1-1: Device not responding to setup address.
> >     usb 1-1: Device not responding to setup address.
> >     usb 1-1: device not accepting address 86, error -71
> > 
> > U-Boot was writing address to faddr register too early and did not
> > wait for correct interrupt after which should update address.
> > 
> > Signed-off-by: Pali Roh?r <pali@kernel.org>
> > ---
> >  drivers/usb/musb/musb_udc.c | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/usb/musb/musb_udc.c b/drivers/usb/musb/musb_udc.c
> > index 7c74422623..50d8bc319c 100644
> > --- a/drivers/usb/musb/musb_udc.c
> > +++ b/drivers/usb/musb/musb_udc.c
> > @@ -707,9 +707,7 @@ static void musb_peri_rx(u16 intr)
> >  {
> >  	unsigned int ep;
> >  
> > -	/* Check for EP0 */
> > -	if (0x01 & intr)
> > -		musb_peri_ep0();
> > +	/* First bit is reserved and does not indicate interrupt for
> > EP0 */ 
> >  	for (ep = 1; ep < 16; ep++) {
> >  		if ((1 << ep) & intr)
> > @@ -721,9 +719,9 @@ static void musb_peri_tx(u16 intr)
> >  {
> >  	unsigned int ep;
> >  
> > -	/* Check for EP0 */
> > +	/* Check for EP0: first bit indicates interrupt for both RX
> > and TX */ if (0x01 & intr)
> > -		musb_peri_ep0_tx();
> > +		musb_peri_ep0();
> >  
> >  	for (ep = 1; ep < 16; ep++) {
> >  		if ((1 << ep) & intr)
> > @@ -750,8 +748,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();
> > @@ -790,7 +786,7 @@ void udc_irq(void)
> >  			if (intrtx)
> >  				musb_peri_tx(intrtx);
> >  		} else {
> > -			if (MUSB_INTR_SOF & intrusb) {
> > +			if (readw(&musbr->intrtx) & 0x1) {
> >  				u8 faddr;
> >  				faddr = readb(&musbr->faddr);
> >  				/*
> 
> Applying this patch causes error. Could you rebase your work on newest
> master (after onging PR is accepted).

Hello! This patch series is rebased on top of the another patch series
"Nokia RX-51: Fix USB TTY console and enable it" as it depends on it.
So I cannot rebase it on top of master as it would not work.

Nokia RX-51: Fix USB TTY console and enable it:
<20201129164618.5829-1-pali@kernel.org>
https://lists.denx.de/pipermail/u-boot/2020-November/433728.html

> Thanks in advance and sorry for delay.
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de

[PATCH 3/4] usb: musb: Fix handling interrupts for EP0 and SET ADDRESS commmand

[Lukasz Majewski]

Hi Pali,

> On Saturday 23 January 2021 16:16:11 Lukasz Majewski wrote:
> > Hi Pali,
> >   
> > > Interrupt for EP0 is indicated in intrtx register via first bit.
> > > It is set for both RX and TX. First bit in intrrx register is
> > > reserved and not set.
> > > 
> > > So remove calling musb_peri_ep0() function at every iteration of
> > > udc_irq() and musb_peri_rx() and call it only from musb_peri_tx()
> > > when correct interrupt bit in initrtx it set.
> > > 
> > > Address from SET ADDRESS command must be set to faddr register
> > > only after acknowledging SERV_RXPKTRDY followed by received EP0
> > > interrupt. So prior calling musb_peri_ep0_set_address() check for
> > > EP0 interrupt instead of (incorrect) MUSB_INTR_SOF interrupt.
> > > 
> > > This patch fixes issue that host (computer) cannot register
> > > U-Boot USB device and failing with errors:
> > > 
> > >     usb 1-1: new full-speed USB device number 86 using xhci_hcd
> > >     usb 1-1: Device not responding to setup address.
> > >     usb 1-1: Device not responding to setup address.
> > >     usb 1-1: device not accepting address 86, error -71
> > > 
> > > U-Boot was writing address to faddr register too early and did not
> > > wait for correct interrupt after which should update address.
> > > 
> > > Signed-off-by: Pali Roh?r <pali@kernel.org>
> > > ---
> > >  drivers/usb/musb/musb_udc.c | 12 ++++--------
> > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/usb/musb/musb_udc.c
> > > b/drivers/usb/musb/musb_udc.c index 7c74422623..50d8bc319c 100644
> > > --- a/drivers/usb/musb/musb_udc.c
> > > +++ b/drivers/usb/musb/musb_udc.c
> > > @@ -707,9 +707,7 @@ static void musb_peri_rx(u16 intr)
> > >  {
> > >  	unsigned int ep;
> > >  
> > > -	/* Check for EP0 */
> > > -	if (0x01 & intr)
> > > -		musb_peri_ep0();
> > > +	/* First bit is reserved and does not indicate interrupt
> > > for EP0 */ 
> > >  	for (ep = 1; ep < 16; ep++) {
> > >  		if ((1 << ep) & intr)
> > > @@ -721,9 +719,9 @@ static void musb_peri_tx(u16 intr)
> > >  {
> > >  	unsigned int ep;
> > >  
> > > -	/* Check for EP0 */
> > > +	/* Check for EP0: first bit indicates interrupt for both
> > > RX and TX */ if (0x01 & intr)
> > > -		musb_peri_ep0_tx();
> > > +		musb_peri_ep0();
> > >  
> > >  	for (ep = 1; ep < 16; ep++) {
> > >  		if ((1 << ep) & intr)
> > > @@ -750,8 +748,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();
> > > @@ -790,7 +786,7 @@ void udc_irq(void)
> > >  			if (intrtx)
> > >  				musb_peri_tx(intrtx);
> > >  		} else {
> > > -			if (MUSB_INTR_SOF & intrusb) {
> > > +			if (readw(&musbr->intrtx) & 0x1) {
> > >  				u8 faddr;
> > >  				faddr = readb(&musbr->faddr);
> > >  				/*  
> > 
> > Applying this patch causes error. Could you rebase your work on
> > newest master (after onging PR is accepted).  
> 
> Hello! This patch series is rebased on top of the another patch series
> "Nokia RX-51: Fix USB TTY console and enable it" as it depends on it.

I took this patch series as they were queued in patchwork.

> So I cannot rebase it on top of master as it would not work.
> 
> Nokia RX-51: Fix USB TTY console and enable it:
> <20201129164618.5829-1-pali@kernel.org>
> https://lists.denx.de/pipermail/u-boot/2020-November/433728.html

Those were delegated to Marek, and are not yet in his tree
(-master/-next).

Ok. I will wait for them to be pulled.

> 
> > Thanks in advance and sorry for delay.
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma at denx.de  
> 
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210123/9eec2fc9/attachment.sig>

[PATCH 3/4] usb: musb: Fix handling interrupts for EP0 and SET ADDRESS commmand

[Pali Rohár]

On Saturday 23 January 2021 22:23:10 Lukasz Majewski wrote:
> Hi Pali,
> 
> > On Saturday 23 January 2021 16:16:11 Lukasz Majewski wrote:
> > > Hi Pali,
> > >   
> > > > Interrupt for EP0 is indicated in intrtx register via first bit.
> > > > It is set for both RX and TX. First bit in intrrx register is
> > > > reserved and not set.
> > > > 
> > > > So remove calling musb_peri_ep0() function at every iteration of
> > > > udc_irq() and musb_peri_rx() and call it only from musb_peri_tx()
> > > > when correct interrupt bit in initrtx it set.
> > > > 
> > > > Address from SET ADDRESS command must be set to faddr register
> > > > only after acknowledging SERV_RXPKTRDY followed by received EP0
> > > > interrupt. So prior calling musb_peri_ep0_set_address() check for
> > > > EP0 interrupt instead of (incorrect) MUSB_INTR_SOF interrupt.
> > > > 
> > > > This patch fixes issue that host (computer) cannot register
> > > > U-Boot USB device and failing with errors:
> > > > 
> > > >     usb 1-1: new full-speed USB device number 86 using xhci_hcd
> > > >     usb 1-1: Device not responding to setup address.
> > > >     usb 1-1: Device not responding to setup address.
> > > >     usb 1-1: device not accepting address 86, error -71
> > > > 
> > > > U-Boot was writing address to faddr register too early and did not
> > > > wait for correct interrupt after which should update address.
> > > > 
> > > > Signed-off-by: Pali Roh?r <pali@kernel.org>
> > > > ---
> > > >  drivers/usb/musb/musb_udc.c | 12 ++++--------
> > > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/usb/musb/musb_udc.c
> > > > b/drivers/usb/musb/musb_udc.c index 7c74422623..50d8bc319c 100644
> > > > --- a/drivers/usb/musb/musb_udc.c
> > > > +++ b/drivers/usb/musb/musb_udc.c
> > > > @@ -707,9 +707,7 @@ static void musb_peri_rx(u16 intr)
> > > >  {
> > > >  	unsigned int ep;
> > > >  
> > > > -	/* Check for EP0 */
> > > > -	if (0x01 & intr)
> > > > -		musb_peri_ep0();
> > > > +	/* First bit is reserved and does not indicate interrupt
> > > > for EP0 */ 
> > > >  	for (ep = 1; ep < 16; ep++) {
> > > >  		if ((1 << ep) & intr)
> > > > @@ -721,9 +719,9 @@ static void musb_peri_tx(u16 intr)
> > > >  {
> > > >  	unsigned int ep;
> > > >  
> > > > -	/* Check for EP0 */
> > > > +	/* Check for EP0: first bit indicates interrupt for both
> > > > RX and TX */ if (0x01 & intr)
> > > > -		musb_peri_ep0_tx();
> > > > +		musb_peri_ep0();
> > > >  
> > > >  	for (ep = 1; ep < 16; ep++) {
> > > >  		if ((1 << ep) & intr)
> > > > @@ -750,8 +748,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();
> > > > @@ -790,7 +786,7 @@ void udc_irq(void)
> > > >  			if (intrtx)
> > > >  				musb_peri_tx(intrtx);
> > > >  		} else {
> > > > -			if (MUSB_INTR_SOF & intrusb) {
> > > > +			if (readw(&musbr->intrtx) & 0x1) {
> > > >  				u8 faddr;
> > > >  				faddr = readb(&musbr->faddr);
> > > >  				/*  
> > > 
> > > Applying this patch causes error. Could you rebase your work on
> > > newest master (after onging PR is accepted).  
> > 
> > Hello! This patch series is rebased on top of the another patch series
> > "Nokia RX-51: Fix USB TTY console and enable it" as it depends on it.
> 
> I took this patch series as they were queued in patchwork.

I have wrote this information also into the cover letter of this patch
series that it depends on another set.

> > So I cannot rebase it on top of master as it would not work.
> > 
> > Nokia RX-51: Fix USB TTY console and enable it:
> > <20201129164618.5829-1-pali@kernel.org>
> > https://lists.denx.de/pipermail/u-boot/2020-November/433728.html
> 
> Those were delegated to Marek, and are not yet in his tree
> (-master/-next).
> 
> Ok. I will wait for them to be pulled.
> 
> > 
> > > Thanks in advance and sorry for delay.
> > > 
> > > 
> > > Best regards,
> > > 
> > > Lukasz Majewski
> > > 
> > > --
> > > 
> > > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > > lukma at denx.de  
> > 
> > 
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de