On Thu, May 8, 2014 at 8:44 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Wed, 7 May 2014, Dan Williams wrote: > >> Unconditionally wake up the child device when the power session is >> recovered. > > ... > >> 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() and >> runtime-pm-synchronization is used to guarantee the same sequence of >> events. When pm_request_resume() is invoked on the child usb_device >> port device is in the RPM_RESUMING state. khubd in turn performs a >> pm_runtime_barrier() on the port device to flush the port recovery, and >> holds the port active while it resumes the child with another >> pm_runtime_barrier(). This guarantees that the portstatus khubd >> evaluates via port_event() is always on an active port and a usb_device >> that has recovered from power loss. > > I don't understand this last part. Why do we need to guarantee the > child device has recovered from power loss? Why not proceed the same > way we do now when the child is suspended? Two reasons I believe: 1/ The child may be gone, and usb_port_resume() will mark it for disconnect 2/ Currently port_event() knows how to handle suspended devices (USB_PORT_STAT_C_SUSPEND), but in the case of power loss recovery the status and change bits are different. I figure why special case port_event()? Just make it so it handles all the same cases that are presented when the port does not lose power. > If you take that stuff out, it seems that there won't be any need to > use wakeup_bits or usb_kick_khubd() for this purpose. See 1/ I think we want to handle disconnects right away, hence the khubd kick. >> Given that this patch de-references port_dev->child we need to make sure >> not to collide with usb_disconnect(). > > I don't like the way this was done. We shouldn't disable runtime PM > unless there's no choice. In this case, the disable would interfere > with the immediately preceding pm_runtime_put(). > > You could change the test_and_clear_bit()/pm_runtime_put() calls to > !test_and_set_bit()/pm_runtime_get_sync. Then replace the > runtime-enable with clear_bit/pm_runtime_put. Ok, I'll fix this up. -- 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
