HI Alan:
I just find Rafael's patch has resolved this issue. In this patch, enable runtime PM
right after executing subsystem/driver .resume_early() callbacks. When do resume(),
the device's runtime pm has been enabled. This patch now is already in the v3.8-rc4.
So this patchset will not cause problem with Rafael patch and I have tested. About the
port system suspend/resume() callback, Could I do this in the next step?
commit a8d3848dadc2fd41a9117cb08dc04fe6d7c551f9
Author: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Date: Sat Dec 22 23:59:01 2012 +0100
PM: Move disabling/enabling runtime PM to late suspend/early resume
Currently, the PM core disables runtime PM for all devices right
after executing subsystem/driver .suspend() callbacks for them
and re-enables it right before executing subsystem/driver .resume()
callbacks for them. This may lead to problems when there are
two devices such that the .suspend() callback executed for one of
them depends on runtime PM working for the other. In that case,
if runtime PM has already been disabled for the second device,
the first one's .suspend() won't work correctly (and analogously
for resume).
To make those issues go away, make the PM core disable runtime PM
for devices right before executing subsystem/driver .suspend_late()
callbacks for them and enable runtime PM for them right after
executing subsystem/driver .resume_early() callbacks for them. This
way the potential conflitcs between .suspend_late()/.resume_early()
and their runtime PM counterparts are still prevented from happening,
but the subtle ordering issues related to disabling/enabling runtime
PM for devices during system suspend/resume are much easier to avoid.
Best Regards
Tianyu Lan
-----Original Message-----
From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx]
Sent: Saturday, January 19, 2013 12:21 AM
To: Lan, Tianyu
Cc: rjw@xxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; sarah.a.sharp@xxxxxxxxxxxxxxx; oneukum@xxxxxxx; linux-usb@xxxxxxxxxxxxxxx
Subject: Re: [Resend PATCH V4 0/10] usb: usb port power off mechanism anc expose usb port connect type
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
