On Mon, Apr 28, 2014 at 1:29 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 19 Mar 2014, Dan Williams wrote:
[..]
>> --- a/drivers/usb/core/port.c
>> +++ b/drivers/usb/core/port.c
>> @@ -104,6 +104,8 @@ static int usb_port_runtime_resume(struct device *dev)
>> if (retval < 0)
>> dev_dbg(&port_dev->dev, "can't get reconnection after setting port power on, status %d\n",
>> retval);
>> +
>> + usb_wakeup_notification(hdev, port1);
>> retval = 0;
>> }
>
> I think this can be simplified a lot. At first glance, almost no
> changes to hub.c will be necessary if instead you insert:
>
> if (port_dev->did_runtime_put) {
> port_dev->did_runtime_put = false;
> pm_runtime_get_noresume(&port_dev->dev);
> pm_request_resume(&port_dev->child->dev);
> }
>
> Then in usb_port_resume(), simply interchange these two lines:
>
> status = pm_runtime_get_sync(&port_dev->dev);
> port_dev->did_runtime_put = false;
>
So I spent a day debugging this approach as it de-stabilized my tests,
resume failures and disconnects, relative to the
usb_wakeup_notification approach. I got it stabilized again, but I
have effectively open coded the usb_wakeup_notification(). To me
"abusing" usb_wakeup_notification() vs open coding it is a distinction
without a difference.
pm_request_resume() arranges for the child device wakeup to run
asynchronously with respect to khubd outside of usb_lock_device().
In comparison, waking up the device in port_event() means:
1/ the hub is powered (as we are within usb_autopm_get_interface() for the hub)
2/ portstatus has already been read for the port_event()
3/ we can take usb_lock_device().
I can leave it open coded if you like, but I'd just as soon re-use the
existing wakeup notification infrastructure. Maybe a comment to
clarify the distinction between remote and port pm runtime induced
resume of the device?
--
Dan
--
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
