On Fri, 18 Jan 2013, Lan Tianyu wrote:
> > By the way, have you checked whether the auto-power-off mechanism works
> > correctly when you do a system suspend?
> >
> Thanks for reminder. I test this today and find an issue. If usb device
> was powered off during runtime, pm_runtime_get_sync() in the
> usb_port_resume() will return -EACCES when do system resume. This is
> because port's runtime pm was disabled during system suspend.
Actually it's worse than that. The PM layer assumes that devices are
brought back to full power during system resume. But that's not true
for your usb_port devices, because they don't have a resume callback.
In addition, we need to enable async PM for the usb_ports, just like
everything else in the USB stack. This means you have to call
device_enable_async_suspend before the port is registered.
> Following
> are some log. The device's runtime pm will be enabled after system
> suspend. The port device is advance to be inserted in the dpm_list than
> usb device(port device is created before creating usb device). So the
> resume sequence is that resume usb device firstly and then usb port. In
> the result, pm_runtime_get_sync() doesn't work.
When async is enabled, there won't be any defined ordering. We will
have to enforce the proper ordering by hand. This means
usb_resume_device will have to call device_pm_wait_for_dev for the port
device (it already does this for the companion root hub). In fact, the
code to do this waiting probably should be moved to usb_resume, because
it applies only to system resume, not to runtime resume.
> [ 70.448028] power LNXPOWER:03: driver resume
> [ 70.448029] usb usb4: runtime enable
> [ 70.448031] usb 3-1: can't resume usb port, status -13
> [ 70.448032] usb usb4: type resume
> [ 70.448039] dpm_run_callback(): usb_dev_resume+0x0/0x15 returns -13
> [ 70.448040] usb usb4: usb resume
> [ 70.448041] usb 3-2: can't resume usb port, status -13
> [ 70.448043] PM: Device 3-1 failed to resume async: error -13
> [ 70.448047] dpm_run_callback(): usb_dev_resume+0x0/0x15 returns -13
> [ 70.448048] PM: Device 3-2 failed to resume async: error -13
> [ 70.448056] hub 4-0:1.0: hub_resume
> [ 70.448088] acpi device:49: runtime enable
>
> I found one solution of adding pm_runtime_enable/disable() in the
> usb_port_resume(). This is simple sovlution.
>
> if (port_dev->did_runtime_put) {
> if (!PMSG_IS_AUTO(msg)) {
> pm_runtime_enable(&port_dev->dev);
> status = pm_runtime_get_sync(&port_dev->dev);
> pm_runtime_disable(&port_dev->dev);
> } else
> status = pm_runtime_get_sync(&port_dev->dev);
>
> port_dev->did_runtime_put = false;
> if (status < 0) {
> dev_dbg(&udev->dev, "can't resume usb port, status %d\n",
> status);
> return status;
> }
> }
>
> This can work but I am not sure that it is safe to do system resume() in
> the system resume().
>
> Another proposal is to export usb_port_runtime_resume() or do a wrapper.
> Call it in the usb_port_resume() with pm_runtime_get_noresume() and
> pm_runtime_set_active(). This also work.
>
> if (port_dev->did_runtime_put) {
> if (!PMSG_IS_AUTO(msg)) {
> status = usb_port_runtime_resume(&port_dev->dev);
> pm_runtime_get_noresume(&port_dev->dev);
> pm_runtime_set_active(&udev->dev);
> } else
> status = pm_runtime_get_sync(&port_dev->dev);
>
> port_dev->did_runtime_put = false;
> if (status < 0) {
> dev_dbg(&udev->dev, "can't resume usb port, status %d\n",
> status);
> return status;
> }
> }
No, neither of these. The right approach is to define a new
usb_port_system_resume routine. It should call usb_port_runtime_resume
directly and set the runtime PM status back to ACTIVE, similar to what
usb_resume() does. In the dev_pm_ops structure, both the resume and
the restore callbacks should point to this new routine.
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
