On 06/09/2024 7.44, fps wrote:
> On September 5, 2024 4:32:58 PM GMT+02:00, Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> wrote:
>> From: Niklas Neronin <niklas.neronin@xxxxxxxxxxxxxxx>
>>
>> Introduce an initial check for an empty list prior to entering the while
>> loop. Which enables, the implementation of distinct warnings to
>> differentiate between scenarios where the list is initially empty and
>> when it has been emptied during processing skipped isoc TDs.
>>
>> These adjustments not only simplifies the large while loop, but also
>> facilitates future enhancements to the handle_tx_event() function.
>>
>> Signed-off-by: Niklas Neronin <niklas.neronin@xxxxxxxxxxxxxxx>
>> Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
>> ---
>> drivers/usb/host/xhci-ring.c | 51 +++++++++++++++++-------------------
>> 1 file changed, 24 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index d37eeee74960..a4383735b16c 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -2761,35 +2761,25 @@ static int handle_tx_event(struct xhci_hcd *xhci,
>> return 0;
>> }
>>
>> - do {
>> - /* This TRB should be in the TD at the head of this ring's
>> - * TD list.
>> + if (list_empty(&ep_ring->td_list)) {
>> + /*
>> + * Don't print wanings if ring is empty due to a stopped endpoint generating an
>
> "wanings" should be "warnings", no?
Thanks, yes it should be the latter.
It will fix it in a handle_tx_event() cleanup patch.
Thanks,
Niklas
>
>
>> + * extra completion event if the device was suspended. Or, a event for the last TRB
>> + * of a short TD we already got a short event for. The short TD is already removed
>> + * from the TD list.
>> */
>> - if (list_empty(&ep_ring->td_list)) {
>> - /*
>> - * Don't print wanings if it's due to a stopped endpoint
>> - * generating an extra completion event if the device
>> - * was suspended. Or, a event for the last TRB of a
>> - * short TD we already got a short event for.
>> - * The short TD is already removed from the TD list.
>> - */
>> -
>> - if (!(trb_comp_code == COMP_STOPPED ||
>> - trb_comp_code == COMP_STOPPED_LENGTH_INVALID ||
>> - ep_ring->last_td_was_short)) {
>> - xhci_warn(xhci, "WARN Event TRB for slot %u ep %d with no TDs queued?\n",
>> - slot_id, ep_index);
>> - }
>> - if (ep->skip) {
>> - ep->skip = false;
>> - xhci_dbg(xhci, "td_list is empty while skip flag set. Clear skip flag for slot %u ep %u.\n",
>> - slot_id, ep_index);
>> - }
>> -
>> - td = NULL;
>> - goto check_endpoint_halted;
>> + if (trb_comp_code != COMP_STOPPED &&
>> + trb_comp_code != COMP_STOPPED_LENGTH_INVALID &&
>> + !ep_ring->last_td_was_short) {
>> + xhci_warn(xhci, "Event TRB for slot %u ep %u with no TDs queued\n",
>> + slot_id, ep_index);
>> }
>>
>> + ep->skip = false;
>> + goto check_endpoint_halted;
>> + }
>> +
>> + do {
>> td = list_first_entry(&ep_ring->td_list, struct xhci_td,
>> td_list);
>>
>> @@ -2800,7 +2790,14 @@ static int handle_tx_event(struct xhci_hcd *xhci,
>>
>> if (ep->skip && usb_endpoint_xfer_isoc(&td->urb->ep->desc)) {
>> skip_isoc_td(xhci, td, ep, status);
>> - continue;
>> + if (!list_empty(&ep_ring->td_list))
>> + continue;
>> +
>> + xhci_dbg(xhci, "All TDs skipped for slot %u ep %u. Clear skip flag.\n",
>> + slot_id, ep_index);
>> + ep->skip = false;
>> + td = NULL;
>> + goto check_endpoint_halted;
>> }
>>
>> /*
>
> Kind regards,
> FPS
