On 2013年01月16日 23:45, Alan Stern wrote:
> On Tue, 15 Jan 2013, Lan Tianyu wrote:
>
>> Hi Greg&Alan:
>> Do you have some more comments about this patchset? Thanks.
>
> I don't have any more comments at this point. It looks okay to me.
>
> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>
> 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. 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.
[ 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;
}
}
> Alan Stern
>
--
Best regards
Tianyu Lan
--
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
