On Thu, 8 May 2014, Dan Williams wrote: > On Thu, May 8, 2014 at 11:09 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, 8 May 2014, Dan Williams wrote: > > > >> On Thu, May 8, 2014 at 9:09 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > >> > On Thu, 8 May 2014, Dan Williams wrote: > >> > > >> >> > 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. > >> > > >> > How much special casing would really be needed? > >> > >> Don't know. Is the forced wakeup really that onerous, vs the risk of > >> touch established code? I've broken the port_event() path enough > >> times through this exercise that I'd want a driving reason beyond "why > >> not?". > > > > As far as I can see, it looks like no special casing is needed. Now, I > > haven't gone through all the changes introduced by the patch series to > > make sure, but it seems that all the various port status tests in > > port_event() will see everything appearing normal, because that's how > > hub_port_debounce_be_connected() -- or whatever its current name is -- > > will leave the port. > > > > Thus, nothing will happen up until the place in > > hub_port_connect_change() headed by the "Try to resuscitate an existing > > device" comment. At that point we will resume the child device. That > > is, assuming khubd thinks it has any reason for for calling > > hub_port_connect_change(), which it probably won't. > > > > So at first sight, it appears that nothing would need to be changed. I > > suggest you try it and see. > > > > To be clear what I am trying to remove is the udev runtime_barrier > prior to the port_event, right? We still agree that > pm_request_resume() needs to be there in the port_dev resume path? Yes, pm_request_resume() is necessary. But setting wakeup_bits and calling usb_kick_khubd() aren't, which means the code you added to hub_events() with the comment "Revalidate the device if it was requested by usb_port_runtime_resume" can be left out. 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
