| Message ID | 20201226182851.18493-4-pali@kernel.org |
|---|---|
| State | Not Applicable |
| Delegated to: | Lukasz Majewski |
| Headers | show |
| Series | usbtty/musb: Fix file transfers | expand |
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@denx.de
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@denx.de
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@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@denx.de
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@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@denx.de
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); /*
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(-)