On Fri, 7 Dec 2012, Sarah Sharp wrote:
> > > 2. It doesn't consider a connect status change to be a failed reset. If
> > > multiple warm resets needed to be issued, the connect status may have
> > > changed, so we need to ignore that and look at the port link state
> > > instead. hub_port_reset will now do that.
> >
> > In fact, we might as well do the same thing for non-SuperSpeed
> > connections too. The kernel probably is smart enough not to get
> > confused if the user manages to replace one device with another while a
> > reset is in progress.
>
> Ok, I'll look into that, although I'm really not fond of the idea of
> testing that much legacy code.
That change can be separate from the rest of your -stable cleanup
stuff.
> > > 3. It unconditionally sets udev->speed on all types of successful
> > > resets. The old recursive code would set the port speed when the second
> > > hub_port_reset returned.
> > >
> > > With the new code in hub_port_reset, the 'warm' variable may change on a
> > > transition from a hot reset to a warm reset. So hub_port_finish_reset
> > > could be called with 'warm' set to true when a connected USB device has
> > > been reset. Fix the code by unconditionally updating the device number,
> > > informing the host that the device has been reset, and setting the
> > > device state to default.
> >
> > Why was it necessary to skip these things before, when "warm" was set?
> > Was it merely an optimization? If not, do the same reasons still
> > apply?
>
> First, you need to understand that the old code did not handle connected
> devices needing a warm reset well. There were only two situations that
> the old code handled correctly: an empty port needing a warm reset, and a
> hot reset that migrated to a warm reset.
>
> When an empty port needed a warm reset, hub_port_reset was called with
> the warm variable set. The code in hub_port_finish_reset would skip
> telling the USB core and the xHC host that the device was reset, because
> otherwise that would result in a NULL pointer dereference.
>
> When a USB 3.0 device reset migrated to a warm reset, the recursive call
> made the call stack look like this:
>
> hub_port_reset(warm = false)
> hub_wait_port_reset(warm = false)
> hub_port_reset(warm = true)
> hub_wait_port_reset(warm = true)
> hub_port_finish_reset(warm = true)
> (return up the call stack to the first wait)
>
> hub_port_finish_reset(warm = false)
>
> The old code didn't want to notify the USB core or the xHC host of device reset
> twice, so it only did it in the second call to hub_port_finish_reset,
> when warm was set to false. This was necessary because
> before patch two ("USB: Ignore xHCI Reset Device status."), the USB core
> would pay attention to the xHC Reset Device command error status, and
> the second call would always fail.
>
> Now that we no longer have the recursive call, and warm can change from
> false to true in hub_port_reset, we need to have hub_port_finish_reset
> unconditionally notify the USB core and the xHC of the device reset.
>
> Does that make sense?
Yes, thank you. Can you stick a copy of this explanation into the
patch description?
Alan Stern
--
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
