On Tue, Jan 22, 2013 at 10:23:12PM +0800, Lan Tianyu wrote:
> On 2013/1/22 5:33, Greg KH wrote:
> >On Mon, Jan 21, 2013 at 10:18:06PM +0800, Lan Tianyu wrote:
> >>Some usb devices can't be resumed correctly after power off. This
> >>patch is to add usb_device_allow_power_off() and usb_device_prevent_power_off()
> >>for device's driver. Call pm_runtime_get_sync(portdev) to increase port's usage
> >>count and then port will not be suspended. The device will not be powered off.
> >>
> >>Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> >>Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> >>---
> >> drivers/usb/core/port.c | 28 ++++++++++++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >>
> >>diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> >>index 0c51d24..0334d91 100644
> >>--- a/drivers/usb/core/port.c
> >>+++ b/drivers/usb/core/port.c
> >>@@ -18,11 +18,39 @@
> >>
> >> #include <linux/slab.h>
> >> #include <linux/pm_qos.h>
> >>+#include <linux/module.h>
> >>
> >> #include "hub.h"
> >>
> >> static const struct attribute_group *port_dev_group[];
> >>
> >>+/**
> >>+ * usb_device_control_power_off - Allow or prohibit power off device.
> >>+ * @udev: target usb device
> >>+ * @allow: choice of allow or prohibit
> >>+ *
> >>+ * Call pm_runtime_get/put_sync(portdev) to allow or prohibit target
> >>+ * usb device to be powered off in the kernel. The operations of setting
> >>+ * true and false should be couple. The default status is allowed.
> >>+ */
> >>+int usb_device_control_power_off(struct usb_device *udev, bool allow)
> >>+{
> >>+ struct usb_port *port_dev;
> >>+
> >>+ if (!udev->parent)
> >>+ return -EINVAL;
> >>+
> >>+ port_dev = hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
> >>+
> >>+ if (allow)
> >>+ pm_runtime_put_sync(&port_dev->dev);
> >>+ else
> >>+ pm_runtime_get_sync(&port_dev->dev);
> >>+
> >>+ return 0;
> >>+}
> >>+EXPORT_SYMBOL_GPL(usb_device_control_power_off);
> >
> >Oh, I don't see any code calling this function, so why did you add it?
> >
> >Who needs it? Where is that code?
> This is provided for device driver. Some device may not be
> compatible with the power off mechanism and driver can use these
> function to forbid/allow it. But currently, we are not sure which
> kinds of device
> they are. So just provide new interfaces.
We don't add new interfaces to the kernel unless they have a user. If I
were to see this in the tree, I would expect it to be removed because of
that. So don't add it at all, only add it when it is needed.
thanks,
greg k-h
--
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
