On Mon, 2014-05-12 at 15:14 -0400, Alan Stern wrote:
> On Mon, 12 May 2014, Dan Williams wrote:
>
> > Fixed. And dropped "wakeup" out of the patch subject.
> >
> > > There's a nasty bug here. I'll let you figure it out for yourself. :-)
> > > Hint: Hiding a variable by declaring another local variable with the
> > > same name in an inner block often leads to trouble.
> >
> > Added hub_disconnect_children() and a note in the changelog.
> >
> > 8<---------
> > Subject: usb: resume child device when port is powered on
> >
> > From: Dan Williams <[email protected]>
> >
> > Unconditionally wake up the child device when the power session is
> > recovered.
> >
> > This addresses the following scenarios:
> >
> > 1/ The device may need a reset on power-session loss, without this
> > change port power-on recovery exposes khubd to scenarios that
> > usb_port_resume() is set to handle. Prior to port power control the
> > only time a power session would be lost is during dpm_suspend of the
> > hub. In that scenario usb_port_resume() is guaranteed to be called
> > prior to khubd running for that port. With this change we wakeup the
> > child device as soon as possible (prior to khubd running again for this
> > port).
> >
> > Although khubd has facilities to wake a child device it will only do
> > so if the portstatus / portchange indicates a suspend state. In the
> > case of port power control we are not coming from a hub-port-suspend
> > state. This implementation simply uses pm_request_resume() to wake the
> > device and relies on the port_dev->status_lock to prevent any collisions
> > between khubd and usb_port_resume().
> >
> > 2/ This mechanism rate limits port power toggling. The minimum port
> > power on/off period is now gated by the child device suspend/resume
> > latency. Empirically this mitigates devices downgrading their connection
> > on perceived instability of the host connection. This ratelimiting is
> > really only relevant to port power control testing, but it is a nice
> > side effect of closing the above race. Namely, the race of khubd for
> > the given port running while a usb_port_resume() event is pending.
> >
> > 3/ Going forward we are finding that power-session recovery requires
> > warm-resets (http://marc.info/?t=138659232900003&r=1&w=2). This
> > mechanism allows for warm-resets to be requested at the same point in
> > the resume path for hub dpm_suspend power session losses, or port
> > rpm_suspend power session losses.
> >
> > 4/ If the device *was* disconnected the only time we'll know for sure is
> > after a failed resume, so it's necessary for usb_port_runtime_resume()
> > to expedite a usb_port_resume() to clean up the removed device. The
> > reasoning for this is "least surprise" for the user. Turning on a port
> > means that hotplug detection is again enabled for the port, it is
> > surprising that devices that were removed while the port was off are not
> > disconnected until they are attempted to be used. As a user "why would
> > I try to use a device I removed from the system?"
> >
> > 1, 2, and 4 are not a problem in the system dpm_resume() case because,
> > although the power-session is lost, khubd is frozen until after device
> > resume. For the rpm_resume() case pm_request_resume() is used to
> > request re-validation of the device, and if it happens to collide with a
> > khubd run we rely on the port_dev->status_lock to synchronize those
> > operations.
> >
> > Besides testing, the primary scenario where this mechanism is expected
> > to be triggered is when the user changes the port power policy
> > (control/pm_qos_no_poweroff, or power/control). Each time power is
> > enabled want to revalidate the child device, where the revalidation is
> > handled by usb_port_resume().
> >
> > Given that this arranges for port_dev->child to be de-referenced in
> > usb_port_runtime_resume() we need to make sure not to collide with
> > usb_disconnect() that frees the usb_device. To this end we hold the
> > port active with the "child_usage" reference across the disconnect
> > event. Subsequently, the need to access hub->child_usage_bits lead to
> > the creation of hub_disconnect_children() to remove any ambiguity of
> > which "hub" is being acted on in usb_disconnect() (prompted-by sharp
> > eyes from Alan).
> >
> > Cc: Rafael J. Wysocki <[email protected]>
> > Signed-off-by: Dan Williams <[email protected]>
>
> This version looks good.
>
> Acked-by: Alan Stern <[email protected]>
>
> Is that everything? Or did we skip a few patches in the middle? It's
> hard to keep track.
Sorry for the delay had some medically induced latency earlier this
week, feeling better. My ack tracker script currently says:
ACK [01/19] USB: mutual exclusion for resetting a hub and power-managing a port
ACK [02/19] usb: disable port power control if not supported in
wHubCharacteristics
ACK [03/19] usb: rename usb_port device objects
ACK [04/19] usb: cleanup setting udev->removable from port_dev->connect_type
ACK [05/19] usb: assign default peer ports for root hubs
ACK [06/19] usb: assign usb3 external hub port peers
ACK [07/19] usb: find internal hub tier mismatch via acpi
ACK [08/19] usb: sysfs link peer ports
[09/19] usb: make usb_port flags atomic, rename did_runtime_put to
child_usage
ACK [10/19] usb: block suspension of superspeed port while hispeed peer is
active
ACK [11/19] usb: don't clear FEAT_C_ENABLE on usb_port_runtime_resume failure
ACK [12/19] usb: usb3 ports do not support FEAT_C_ENABLE
[13/19] usb: refactor port handling in hub_events()
[14/19] usb: synchronize port poweroff and khubd
[15/19] usb: introduce port status lock
ACK [16/19] usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y
ACK [17/19] usb: resume child device when port is powered on
ack [18/19] usb: force warm reset to break link re-connect livelock
[19/19] usb: documentation for usb port power off mechanisms
...where that lower case "ack" means acked, but not by you.
--
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
