Re: git: cda31e734925 - main - xhci(4): Ensure the so-called data toggle gets properly reset.

From: Nathan Whitehorn <nwhitehorn_at_freebsd.org>
Date: Mon, 02 May 2022 22:25:10 UTC
After this change, I cannot use devices attached to the USB 3 hub in my 
Dell monitor anymore. The hub appears properly, but any device plugged 
into it just leaves this in dmesg:
ugen0.3: <Unknown > at usbus0 (disconnected)

Note that there is no corresponding connected message or anything else.

This is with an AMD XHCI controller:
xhci0@pci0:2:0:0:       class=0x0c0330 rev=0x02 hdr=0x00 vendor=0x1022 
device=0x43bb subvendor=0x1b21 subdevice=0x1142
     vendor     = 'Advanced Micro Devices, Inc. [AMD]'
     device     = '300 Series Chipset USB 3.1 xHCI Controller'

-Nathan

On 4/21/22 11:02, Hans Petter Selasky wrote:
> The branch main has been updated by hselasky:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=cda31e734925346328fd2369585ab3f6767ec225
>
> commit cda31e734925346328fd2369585ab3f6767ec225
> Author:     Hans Petter Selasky <hselasky@FreeBSD.org>
> AuthorDate: 2022-04-21 14:59:09 +0000
> Commit:     Hans Petter Selasky <hselasky@FreeBSD.org>
> CommitDate: 2022-04-21 15:01:13 +0000
>
>      xhci(4): Ensure the so-called data toggle gets properly reset.
>      
>      Use the drop and enable endpoint context commands to force a reset of
>      the data toggle for USB 2.0 and USB 3.0 after:
>       - clear endpoint halt command (when the driver wishes).
>       - set config command (when the kernel or user-space wants).
>       - set alternate setting command (only affected endpoints).
>      
>      Some XHCI HW implementations may not allow the endpoint reset command when
>      the endpoint context is not in the halted state.
>      
>      Reported by:            Juniper and Gary Jennejohn
>      MFC after:              1 week
>      Sponsored by:           NVIDIA Networking
> ---
>   sys/dev/usb/controller/xhci.c | 27 +++++++++++++++++++++++----
>   1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/sys/dev/usb/controller/xhci.c b/sys/dev/usb/controller/xhci.c
> index f95996b7ab32..72d1ff5e0ae5 100644
> --- a/sys/dev/usb/controller/xhci.c
> +++ b/sys/dev/usb/controller/xhci.c
> @@ -3772,6 +3772,7 @@ xhci_configure_reset_endpoint(struct usb_xfer *xfer)
>   	uint32_t mask;
>   	uint8_t index;
>   	uint8_t epno;
> +	uint8_t drop;
>   
>   	pepext = xhci_get_endpoint_ext(xfer->xroot->udev,
>   	    xfer->endpoint->edesc);
> @@ -3813,15 +3814,19 @@ xhci_configure_reset_endpoint(struct usb_xfer *xfer)
>   	 */
>   	switch (xhci_get_endpoint_state(udev, epno)) {
>   	case XHCI_EPCTX_0_EPSTATE_DISABLED:
> -                break;
> +		drop = 0;
> +		break;
>   	case XHCI_EPCTX_0_EPSTATE_STOPPED:
> +		drop = 1;
>   		break;
>   	case XHCI_EPCTX_0_EPSTATE_HALTED:
>   		err = xhci_cmd_reset_ep(sc, 0, epno, index);
> -		if (err != 0)
> +		drop = (err != 0);
> +		if (drop)
>   			DPRINTF("Could not reset endpoint %u\n", epno);
>   		break;
>   	default:
> +		drop = 1;
>   		err = xhci_cmd_stop_ep(sc, 0, epno, index);
>   		if (err != 0)
>   			DPRINTF("Could not stop endpoint %u\n", epno);
> @@ -3842,11 +3847,25 @@ xhci_configure_reset_endpoint(struct usb_xfer *xfer)
>   	 */
>   
>   	mask = (1U << epno);
> -	xhci_configure_mask(udev, mask | 1U, 0);
> +
> +	if (epno != 1 && drop != 0) {
> +		/* drop endpoint context to reset data toggle value, if any. */
> +	  	xhci_configure_mask(udev, mask, 1);
> +		err = xhci_cmd_configure_ep(sc, buf_inp.physaddr, 0, index);
> +		if (err != 0) {
> +			DPRINTF("Could not drop "
> +			    "endpoint %u at slot %u.\n", epno, index);
> +		} else {
> +			sc->sc_hw.devs[index].ep_configured &= ~mask;
> +		}
> +	}
> +
> +	xhci_configure_mask(udev, mask, 0);
>   
>   	if (!(sc->sc_hw.devs[index].ep_configured & mask)) {
> -		sc->sc_hw.devs[index].ep_configured |= mask;
>   		err = xhci_cmd_configure_ep(sc, buf_inp.physaddr, 0, index);
> +		if (err == 0)
> +			sc->sc_hw.devs[index].ep_configured |= mask;
>   	} else {
>   		err = xhci_cmd_evaluate_ctx(sc, buf_inp.physaddr, index);
>   	}
>