On Wed, Dec 05, 2012 at 12:14:07PM -0500, Alan Stern wrote:
> On Tue, 4 Dec 2012, Sarah Sharp wrote:
>
> > An empty port can transition to either Inactive or Compliance Mode if a
> > newly connected USB 3.0 device fails to link train. In that case, we
> > issue a warm reset. Some devices, such as John's Roseweil eusb3
> > enclosure, slip back into Compliance Mode after the warm reset.
> >
> > The current warm reset code does not check for device connect status on
> > warm reset completion, and it incorrectly reports the warm reset
> > succeeded. This causes the USB core to attempt to send a Set Address
> > control transfer to a port in Compliance Mode, which will always fail.
> >
> > Make hub_port_wait_reset check the current connect status and link state
> > after the warm reset completes. Return a failure status if the device
> > is disconnected or the link state is Compliance Mode or SS.Inactive.
> >
> > Make hub_events disable the port if warm reset fails. This will disable
> > the port, and then bring it back into the RxDetect state. Make the USB
> > core ignore the connect change until the device reconnects.
>
> Is this really the right thing to do? It means that drivers will
> continue to submit URBs.
This is just a bandaid patch until the patch that handles connected
device reset failures. That patch will do the right thing by calling
usb_reset_device and properly disconnecting drivers on failure.
Basically the only reason this patch is broken out is to break the
patchset into readable chucks.
Or do you mean we'll have issues with drivers continuing to submit URBs
once the last patch is applied?
> > Note that this patch does NOT handle connected devices slipping into the
> > Inactive state very well. This is a concern, because devices can go
> > into the Inactive state on U1/U2 exit failure. However, the fix for
> > that case is too large for stable, so it will be submitted in a separate
> > patch.
> >
> > This patch should be backported to kernels as old as 3.2, contain the
> > commit ID 75d7cf72ab9fa01dc70877aa5c68e8ef477229dc "usbcore: refine warm
> > reset logic"
> >
> > Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> > Reported-by: John Covici <covici@xxxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > ---
> > drivers/usb/core/hub.c | 18 +++++++++++++++---
> > 1 files changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index b7b055f..85977de 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -2601,8 +2601,15 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1,
> > return 0;
> > }
> > } else {
> > - if (portchange & USB_PORT_STAT_C_BH_RESET)
> > - return 0;
> > + if (!(portchange & USB_PORT_STAT_C_BH_RESET))
> > + goto delay;
>
> Does this bit get set as soon as the reset is finished, or not until
> later? (The spec isn't clear about this, or I'm not looking in the
> right place.)
>
> If it gets set as soon as the reset is finished then this test isn't
> needed, because you already added a test for USB_PORT_STAT_RESET.
The USB_PORT_STAT_C_BH_RESET bit gets set along with the reset change
bit if the reset was a warm reset. So yes, you're correct this code
could just go away, and that change probably should go into the patch to
ignore port status until the reset finishes.
Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
