Re: [RFC PATCH v2 3/6] usb: add runtime pm support for usb port device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



于 2012/11/14 17:49, Rafael J. Wysocki 写道:
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 <tianyu.lan@xxxxxxxxx>

This one looks reasonable to me.  From the PM side

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
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?
No, because the child device is not under port device and so even if
child->dev is not suspended, port device's usage still can be 0 and
power off the port.

Rafael



--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux