On Wednesday, November 14, 2012 02:34:37 PM Lan Tianyu wrote:
> On 2012年11月14日 08:08, Rafael J. Wysocki wrote:
> > On Tuesday, November 13, 2012 04:00:02 PM Lan Tianyu wrote:
> >> This patch is to add runtime pm callback for usb port device.
> >> Set/clear PORT_POWER feature in the resume/suspend callbak.
> >> Add portnum for struct usb_port to record port number.
> >>
> >> Signed-off-by: Lan Tianyu <[email protected]>
> >
> > This one looks reasonable to me. From the PM side
> >
> > Acked-by: Rafael J. Wysocki <[email protected]>
> Hi Rafael and Alan:
> This patch has a collaboration problem with pm qos. Since pm core would
> pm_runtime_get_sync/put(port_dev) if pm qos flags was changed and port's
> suspend call_back() clear PORT_POWER feature without any check. This
> will cause PORT_POWER feather being cleared every time after pm qos
> flags being changed.
>
> I have an idea that add check in the port's runtime idle callback.
> Check NO_POWER_OFF flag firstly. If set return. Second, for port without
> device, suspend port directly and for port with device, increase child
> device's runtime usage without resume and do barrier to ensure all
> suspend process finish, at last check the child runtime status. If it
> was suspended, suspend port and if do nothing.
>
> static int usb_port_runtime_idle(struct device *dev)
> {
> struct usb_port *port_dev = to_usb_port(dev);
> int retval = 0;
>
> if (dev_pm_qos_flags(&port_dev->dev, PM_QOS_FLAG_NO_POWER_OFF)
> == PM_QOS_FLAGS_ALL)
> return 0;
>
> if (!port_dev->child) {
> retval = pm_runtime_suspend(&port_dev->dev);
> if (!retval)
> port_dev->power_on =false;
> }
> else {
This usually is written as "} else {" in the kernel code.
> pm_runtime_get_noresume(&port_dev->child->dev);
> pm_runtime_barrier(&port_dev->child->dev);
> if (port_dev->child->dev.power.runtime_status
> == RPM_SUSPENDED) {
> retval = pm_runtime_suspend(&port_dev->dev);
> if (!retval)
> port_dev->power_on =false;
> }
> pm_runtime_put_noidle(&port_dev->child->dev);
> }
Hmm. If child->dev is not suspended, then our usage_count should be
at least 1, so pm_runtime_suspend(&port_dev->dev) shouldn't actually
suspend us. Isn't that the case?
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
