Hi,

Mathias Nyman <[email protected]> writes:
> A control transfer that stopped at the status stage incorrectly
> warned about a "unexpected TRB Type 4", and did not set the
> transferred actual_length for the URB.
>
> The transferred actual_length should be set the same way for
> COMP_STOPPED control transfers as in the generic cases.
>
> generic case if we get an event at:
>
> TRB_SETUP stage:
>   length = 0;
>
> TRB_DATA/TRB_NORMAL state:
>   length = requested - remaining;
>
> TRB_STATUS stage:
>   length = requested
>
> The URB actual_length for control transfers doesn't care about sent
> bytes of the SETUP stage, or remaining bytes of the  STATUS stage.
>
> Cc: <[email protected]>
> Signed-off-by: Mathias Nyman <[email protected]>
> ---
>  drivers/usb/host/xhci-ring.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index d9936c7..584b6fe 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1975,25 +1975,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, 
> struct xhci_td *td,
>               *status = 0;
>               break;
>       case COMP_STOPPED_SHORT_PACKET:
> -             if (trb_type == TRB_DATA || trb_type == TRB_NORMAL)
> +             if (trb_type == TRB_DATA || trb_type == TRB_NORMAL) {
> +                     td->urb_length_set = true;
>                       td->urb->actual_length = remaining;
> -             else
> +             } else {
>                       xhci_warn(xhci, "WARN: Stopped Short Packet on ctrl 
> setup or status TRB\n");
> +             }
>               goto finish_td;
>       case COMP_STOPPED:
> -             switch (trb_type) {
> -             case TRB_SETUP:
> -                     td->urb->actual_length = 0;
> -                     goto finish_td;
> -             case TRB_DATA:
> -             case TRB_NORMAL:
> -                     td->urb->actual_length = requested - remaining;
> -                     goto finish_td;
> -             default:
> -                     xhci_warn(xhci, "WARN: unexpected TRB Type %d\n",
> -                               trb_type);
> -                     goto finish_td;
> -             }
> +             break;

instead of this giant patch, why didn't you just add case TRB_STATUS to
the switch statement above? It would've been a single line that would
solve your problem.

Not to mention that we loose the xhci_warn() for unexpected TRB types
which might help finding valid issues (such as $subject).

Note that now we do nothing on COMP_STOPPED, this means that we're
relying on some generic case that, arguably, could be removed
altogether. $subject is just re-adding part of obfuscation we had and
took so much work to clean up.

How have you tested $subject, btw?

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to