On Tue, Aug 25, 2020 at 02:53:56PM +0300, Mathias Nyman wrote:
> On 25.8.2020 11.00, Martin Thierer wrote:
> >> Can you try the code below? It should force dropping and adding the endpoints
> >> for the intrface(s) of that configuration, and xhci should reset the toggles.
> >>
> >> Completely untested, it compiles :)
> >
> > Sorry, no, that doesn't work:
> >
> > xhci_hcd 0000:00:14.0: Trying to add endpoint 0x83 without dropping it.
> > BUG: kernel NULL pointer dereference, address: 0000000000000010
>
> Ah, I see.
> Endpoints weren't dropped on host side as pointer to the endpoints were cleaned up before this.
> And the code to recover from a failed call got messed up as we removed some stuff it depends on.
>
> Here's a second version.
> I'm again not able to test this at all from my current location, so it might fail because
> of some silly mistake, but it compiles..
>
> This version keeps endpoint pointers intact until endpoints are dropped from hcd side,
> it also removes the recover path (might need to fix one later)
>
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 6197938dcc2d..e90e8781f872 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1537,9 +1537,21 @@ int usb_reset_configuration(struct usb_device *dev)
> * calls during probe() are fine
> */
>
> + /*
> + * TEST2 flush and disable endpoints but leave the pointers intact until
> + * usb_hcd_alloc_bandwidth() has dropped them from host controller side
> + */
> for (i = 1; i < 16; ++i) {
> - usb_disable_endpoint(dev, i, true);
> - usb_disable_endpoint(dev, i + USB_DIR_IN, true);
> + if (dev->ep_out[i]) {
> + dev->ep_out[i]->enabled = 0;
> + usb_hcd_flush_endpoint(dev, dev->ep_out[i]);
> + usb_hcd_disable_endpoint(dev, dev->ep_out[i]);
> + }
> + if (dev->ep_in[i]) {
> + dev->ep_in[i]->enabled = 0;
> + usb_hcd_flush_endpoint(dev, dev->ep_in[i]);
> + usb_hcd_disable_endpoint(dev, dev->ep_in[i]);
> + }
> }
There's got to be a better way to do this, something that doesn't
involve so much code duplication. For instance, maybe we could make
this routine and usb_set_configuration() both call a new
__usb_set_config(), with an extra flag telling the routine whether to
change the interface devices and bindings.
Alan Stern
