Re: [PATCH] usb: xhci: reset endpoint on USB stall

2022-02-16 Thread Marek Vasut

On 2/16/22 08:53, Bin Meng wrote:

On Mon, Sep 27, 2021 at 8:43 PM Stefan Agner  wrote:


There are devices which cause a USB stall when trying to read strings.
Specifically Arduino Mega R3 stalls when trying to read the product
string.

The stall currently remains unhandled, and subsequent retries submit new
transfers on a stopped endpoint which ultimately cause a crash in
abort_td():
WARN halted endpoint, queueing URB anyway.
XHCI control transfer timed out, aborting...
Unexpected XHCI event TRB, skipping... (3affe040  1300 02008401)
BUG at drivers/usb/host/xhci-ring.c:505/abort_td()!
BUG!
resetting ...

Linux seems to be able to recover from the stall by issuing a
TRB_RESET_EP command.

Introduce reset_ep() which issues a TRB_RESET_EP followed by setting the
transfer ring dequeue pointer via TRB_SET_DEQ. This allows to properly
recover from a USB stall error and continue communicating with the USB
device.

Signed-off-by: Stefan Agner 


Applied, thanks.


Re: [PATCH] usb: xhci: reset endpoint on USB stall

2022-02-15 Thread Bin Meng
On Mon, Sep 27, 2021 at 8:43 PM Stefan Agner  wrote:
>
> There are devices which cause a USB stall when trying to read strings.
> Specifically Arduino Mega R3 stalls when trying to read the product
> string.
>
> The stall currently remains unhandled, and subsequent retries submit new
> transfers on a stopped endpoint which ultimately cause a crash in
> abort_td():
> WARN halted endpoint, queueing URB anyway.
> XHCI control transfer timed out, aborting...
> Unexpected XHCI event TRB, skipping... (3affe040  1300 02008401)
> BUG at drivers/usb/host/xhci-ring.c:505/abort_td()!
> BUG!
> resetting ...
>
> Linux seems to be able to recover from the stall by issuing a
> TRB_RESET_EP command.
>
> Introduce reset_ep() which issues a TRB_RESET_EP followed by setting the
> transfer ring dequeue pointer via TRB_SET_DEQ. This allows to properly
> recover from a USB stall error and continue communicating with the USB
> device.
>
> Signed-off-by: Stefan Agner 
> ---
> This has been reported on the ML by Mike Hoogstraten a while ago:
> https://www.mail-archive.com/u-boot@lists.denx.de/msg390604.html
>
> FWIW, for the Arduino Mega R3 I also opened an issue:
> https://github.com/arduino/ArduinoCore-avr/issues/431
>
> --
> Stefan
>
>  drivers/usb/host/xhci-ring.c | 31 +++
>  1 file changed, 31 insertions(+)
>

Sorry for the delay!

Reviewed-by: Bin Meng 


Re: [PATCH] usb: xhci: reset endpoint on USB stall

2022-01-24 Thread Stefan Agner
Hi Bin,

On 2022-01-05 02:21, Bin Meng wrote:
> Hi Stefan,
> 
> On Wed, Jan 5, 2022 at 3:48 AM Stefan Agner  wrote:
>>
>> Bin Meng,
>>
>> On 2021-09-27 17:14, Marek Vasut wrote:
>> > On 9/27/21 2:42 PM, Stefan Agner wrote:
>> >> There are devices which cause a USB stall when trying to read strings.
>> >> Specifically Arduino Mega R3 stalls when trying to read the product
>> >> string.
>> >>
>> >> The stall currently remains unhandled, and subsequent retries submit new
>> >> transfers on a stopped endpoint which ultimately cause a crash in
>> >> abort_td():
>> >> WARN halted endpoint, queueing URB anyway.
>> >> XHCI control transfer timed out, aborting...
>> >> Unexpected XHCI event TRB, skipping... (3affe040  1300 
>> >> 02008401)
>> >> BUG at drivers/usb/host/xhci-ring.c:505/abort_td()!
>> >> BUG!
>> >> resetting ...
>> >>
>> >> Linux seems to be able to recover from the stall by issuing a
>> >> TRB_RESET_EP command.
>> >>
>> >> Introduce reset_ep() which issues a TRB_RESET_EP followed by setting the
>> >> transfer ring dequeue pointer via TRB_SET_DEQ. This allows to properly
>> >> recover from a USB stall error and continue communicating with the USB
>> >> device.
>> >>
>> >> Signed-off-by: Stefan Agner 
>> >
>> > I hope to get AB/RB from Bin here, then it can go into this release I 
>> > think.
>>
>> Any chance you could have a look at this to get it into this release :)
>>
> 
> Sorry I missed this. I suspect it's too late for 2022.01 to include
> such big changes :(

I understand. Now that 2022.01 is out, could you have a look at this?

--
Stefan


Re: [PATCH] usb: xhci: reset endpoint on USB stall

2022-01-04 Thread Bin Meng
Hi Stefan,

On Wed, Jan 5, 2022 at 3:48 AM Stefan Agner  wrote:
>
> Bin Meng,
>
> On 2021-09-27 17:14, Marek Vasut wrote:
> > On 9/27/21 2:42 PM, Stefan Agner wrote:
> >> There are devices which cause a USB stall when trying to read strings.
> >> Specifically Arduino Mega R3 stalls when trying to read the product
> >> string.
> >>
> >> The stall currently remains unhandled, and subsequent retries submit new
> >> transfers on a stopped endpoint which ultimately cause a crash in
> >> abort_td():
> >> WARN halted endpoint, queueing URB anyway.
> >> XHCI control transfer timed out, aborting...
> >> Unexpected XHCI event TRB, skipping... (3affe040  1300 
> >> 02008401)
> >> BUG at drivers/usb/host/xhci-ring.c:505/abort_td()!
> >> BUG!
> >> resetting ...
> >>
> >> Linux seems to be able to recover from the stall by issuing a
> >> TRB_RESET_EP command.
> >>
> >> Introduce reset_ep() which issues a TRB_RESET_EP followed by setting the
> >> transfer ring dequeue pointer via TRB_SET_DEQ. This allows to properly
> >> recover from a USB stall error and continue communicating with the USB
> >> device.
> >>
> >> Signed-off-by: Stefan Agner 
> >
> > I hope to get AB/RB from Bin here, then it can go into this release I think.
>
> Any chance you could have a look at this to get it into this release :)
>

Sorry I missed this. I suspect it's too late for 2022.01 to include
such big changes :(

Regards,
Bin


Re: [PATCH] usb: xhci: reset endpoint on USB stall

2022-01-04 Thread Stefan Agner
Bin Meng,

On 2021-09-27 17:14, Marek Vasut wrote:
> On 9/27/21 2:42 PM, Stefan Agner wrote:
>> There are devices which cause a USB stall when trying to read strings.
>> Specifically Arduino Mega R3 stalls when trying to read the product
>> string.
>>
>> The stall currently remains unhandled, and subsequent retries submit new
>> transfers on a stopped endpoint which ultimately cause a crash in
>> abort_td():
>> WARN halted endpoint, queueing URB anyway.
>> XHCI control transfer timed out, aborting...
>> Unexpected XHCI event TRB, skipping... (3affe040  1300 02008401)
>> BUG at drivers/usb/host/xhci-ring.c:505/abort_td()!
>> BUG!
>> resetting ...
>>
>> Linux seems to be able to recover from the stall by issuing a
>> TRB_RESET_EP command.
>>
>> Introduce reset_ep() which issues a TRB_RESET_EP followed by setting the
>> transfer ring dequeue pointer via TRB_SET_DEQ. This allows to properly
>> recover from a USB stall error and continue communicating with the USB
>> device.
>>
>> Signed-off-by: Stefan Agner 
> 
> I hope to get AB/RB from Bin here, then it can go into this release I think.

Any chance you could have a look at this to get it into this release :)

--
Stefan


Re: [PATCH] usb: xhci: reset endpoint on USB stall

2021-09-27 Thread Marek Vasut

On 9/27/21 2:42 PM, Stefan Agner wrote:

There are devices which cause a USB stall when trying to read strings.
Specifically Arduino Mega R3 stalls when trying to read the product
string.

The stall currently remains unhandled, and subsequent retries submit new
transfers on a stopped endpoint which ultimately cause a crash in
abort_td():
WARN halted endpoint, queueing URB anyway.
XHCI control transfer timed out, aborting...
Unexpected XHCI event TRB, skipping... (3affe040  1300 02008401)
BUG at drivers/usb/host/xhci-ring.c:505/abort_td()!
BUG!
resetting ...

Linux seems to be able to recover from the stall by issuing a
TRB_RESET_EP command.

Introduce reset_ep() which issues a TRB_RESET_EP followed by setting the
transfer ring dequeue pointer via TRB_SET_DEQ. This allows to properly
recover from a USB stall error and continue communicating with the USB
device.

Signed-off-by: Stefan Agner 


I hope to get AB/RB from Bin here, then it can go into this release I think.


[PATCH] usb: xhci: reset endpoint on USB stall

2021-09-27 Thread Stefan Agner
There are devices which cause a USB stall when trying to read strings.
Specifically Arduino Mega R3 stalls when trying to read the product
string.

The stall currently remains unhandled, and subsequent retries submit new
transfers on a stopped endpoint which ultimately cause a crash in
abort_td():
WARN halted endpoint, queueing URB anyway.
XHCI control transfer timed out, aborting...
Unexpected XHCI event TRB, skipping... (3affe040  1300 02008401)
BUG at drivers/usb/host/xhci-ring.c:505/abort_td()!
BUG!
resetting ...

Linux seems to be able to recover from the stall by issuing a
TRB_RESET_EP command.

Introduce reset_ep() which issues a TRB_RESET_EP followed by setting the
transfer ring dequeue pointer via TRB_SET_DEQ. This allows to properly
recover from a USB stall error and continue communicating with the USB
device.

Signed-off-by: Stefan Agner 
---
This has been reported on the ML by Mike Hoogstraten a while ago:
https://www.mail-archive.com/u-boot@lists.denx.de/msg390604.html

FWIW, for the Arduino Mega R3 I also opened an issue:
https://github.com/arduino/ArduinoCore-avr/issues/431

--
Stefan

 drivers/usb/host/xhci-ring.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 35bd5cd29e..1a9dff3b6c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -481,6 +481,33 @@ union xhci_trb *xhci_wait_for_event(struct xhci_ctrl 
*ctrl, trb_type expected)
BUG();
 }
 
+/*
+ * Send reset endpoint command for given endpoint. This recovers from a
+ * halted endpoint (e.g. due to a stall error).
+ */
+static void reset_ep(struct usb_device *udev, int ep_index)
+{
+   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
+   struct xhci_ring *ring =  ctrl->devs[udev->slot_id]->eps[ep_index].ring;
+   union xhci_trb *event;
+   u32 field;
+
+   printf("Resetting EP %d...\n", ep_index);
+   xhci_queue_command(ctrl, NULL, udev->slot_id, ep_index, TRB_RESET_EP);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   field = le32_to_cpu(event->trans_event.flags);
+   BUG_ON(TRB_TO_SLOT_ID(field) != udev->slot_id);
+   xhci_acknowledge_event(ctrl);
+
+   xhci_queue_command(ctrl, (void *)((uintptr_t)ring->enqueue |
+   ring->cycle_state), udev->slot_id, ep_index, TRB_SET_DEQ);
+   event = xhci_wait_for_event(ctrl, TRB_COMPLETION);
+   BUG_ON(TRB_TO_SLOT_ID(le32_to_cpu(event->event_cmd.flags))
+   != udev->slot_id || GET_COMP_CODE(le32_to_cpu(
+   event->event_cmd.status)) != COMP_SUCCESS);
+   xhci_acknowledge_event(ctrl);
+}
+
 /*
  * Stops transfer processing for an endpoint and throws away all unprocessed
  * TRBs by setting the xHC's dequeue pointer to our enqueue pointer. The next
@@ -928,6 +955,10 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
record_transfer_result(udev, event, length);
xhci_acknowledge_event(ctrl);
+   if (udev->status == USB_ST_STALLED) {
+   reset_ep(udev, ep_index);
+   return -EPIPE;
+   }
 
/* Invalidate buffer to make it available to usb-core */
if (length > 0)
-- 
2.33.0