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 <[email protected]> > >>Signed-off-by: Lan Tianyu <[email protected]> > >>--- > >> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
