[ adding Greg to the cc as a ping and in case he was curious of the current review state of this patchset ] On Thu, May 15, 2014 at 12:41 PM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > 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 <dan.j.williams@xxxxxxxxx> >> > >> > 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 <rjw@xxxxxxxxxxxxx> >> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> >> >> This version looks good. >> >> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> >> >> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
