Re: [patch 3/7] usb: add usb_dev_reset_delayed

On Monday 06 August 2007, Alan Stern wrote:
> On Fri, 3 Aug 2007, Inaky Perez-Gonzalez wrote:
> 
> > +static
> > +void usb_dev_reset_delayed_task(struct work_struct *ws)
...
> > +	if (usb_dev == NULL) {
> > +		WARN_ON(1);
> > +		goto release;
> > +	}
> 
> This can never happen; there's no reason to test for it.

Ouch, remnant from old implementation -- wiped.

> > +error_pending:
> > +	usb_put_dev(usb_dev);
> 
> You forgot to call module_put().
 
yikes!

Fixed patch inlined

Thanks Alan!

usb: add usb_dev_reset_delayed

Introduce a new utility function to be able to reset a device from
interrupt context using a workqueue. If a reset is already pending, it
won't schedule another one.

Signed-off-by: Inaky Perez-Gonzalez <[email protected]>


---
 drivers/usb/core/hub.c |   92 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb.h    |    2 +
 2 files changed, 94 insertions(+)

Index: linux.hg/include/linux/usb.h
===================================================================
--- linux.hg.orig/include/linux/usb.h	2007-07-31 12:18:58.000000000 -0700
+++ linux.hg/include/linux/usb.h	2007-07-31 12:18:59.000000000 -0700
@@ -386,6 +386,7 @@
 	unsigned authorized:1;		/* Policy has determined we can use it */
 	unsigned authenticated:1;	/* Crypto authentication passed */
 	unsigned wusb:1;		/* Device is Wireless USB */
+	unsigned delayed_reset:1;	/* Delayed reset in progress */
 	int string_langid;		/* language ID for strings */
 
 	/* static strings from the device */
@@ -445,6 +446,7 @@
 extern int usb_reset_device(struct usb_device *dev);
 extern int usb_reset_composite_device(struct usb_device *dev,
 		struct usb_interface *iface);
+extern void usb_dev_reset_delayed(struct usb_device *usb_dev);
 
 extern struct usb_device *usb_find_device(u16 vendor_id, u16 product_id);
 
Index: linux.hg/drivers/usb/core/hub.c
===================================================================
--- linux.hg.orig/drivers/usb/core/hub.c	2007-07-31 12:18:58.000000000 -0700
+++ linux.hg/drivers/usb/core/hub.c	2007-07-31 12:18:59.000000000 -0700
@@ -3175,3 +3175,92 @@
 	return ret;
 }
 EXPORT_SYMBOL(usb_reset_composite_device);
+
+
+/* Support for delayed resets */
+struct usb_dev_reset_ctx {
+	struct work_struct ws;
+	struct usb_device  *usb_dev;
+};
+
+
+/*
+ * We need to lock the device before resetting, but we may already have the
+ * lock. So we try to lock the device, if it fails (returns < 0) then we
+ * cannot proceed. If it returned 1 then we acquired the lock here and need
+ * to release the lock. If it returned 0 then we already have the lock and
+ * we leave it to the piece that acquired the lock to release it.
+ */
+static
+void usb_dev_reset_delayed_task(struct work_struct *ws)
+{
+	struct usb_dev_reset_ctx *reset_ctx =
+		container_of(ws, struct usb_dev_reset_ctx, ws);
+	struct usb_device *usb_dev = reset_ctx->usb_dev;
+	struct device *dev = &usb_dev->dev;
+	int had_to_lock;
+	int result = 0;
+
+	had_to_lock = usb_lock_device_for_reset(usb_dev, NULL);
+	if (had_to_lock < 0) {
+		if (had_to_lock != -ENODEV)	/* ignore dissapearance */
+			dev_err(dev, "Cannot lock device for reset: %d\n",
+				had_to_lock);
+	} else {
+		result = usb_reset_device(usb_dev);
+		if (result < 0 && result != -ENODEV)
+			dev_err(dev, "Unable to reset device: %d\n", result);
+		if (had_to_lock)
+			usb_unlock_device(usb_dev);
+	}
+	usb_dev->delayed_reset = 0;
+	usb_put_dev(usb_dev);
+release:
+	module_put(THIS_MODULE);
+	kfree(reset_ctx);
+}
+
+
+/**
+ * Schedule a delayed USB device reset
+ *
+ * @usb_dev: USB device that needs to be reset. We assume you have a
+ *           valid reference on it (so it won't dissapear) until we
+ *           take another one that we'll own.
+ *
+ * Allocates a context structure containing a workqueue struct with
+ * all the pertinent info; gets a reference to @usb_dev and schedules
+ * the call that will be executed later on.
+ *
+ * NOTE: for use in atomic contexts
+ */
+void usb_dev_reset_delayed(struct usb_device *usb_dev)
+{
+	struct usb_dev_reset_ctx *reset_ctx;
+	struct device *dev = &usb_dev->dev;
+	reset_ctx = kmalloc(sizeof(*reset_ctx), GFP_ATOMIC);
+	if (reset_ctx == NULL) {
+		if (printk_ratelimit())
+			dev_err(dev, "USB: cannot allocate memory for "
+				"delayed device reset\n");
+		return;
+	}
+	if (try_module_get(THIS_MODULE) == 0)
+		goto error_module_get;
+	usb_get_dev(usb_dev);
+	if (usb_dev->delayed_reset)
+		goto error_pending;
+	usb_dev->delayed_reset = 1;
+	reset_ctx->usb_dev = usb_dev;
+	INIT_WORK(&reset_ctx->ws, usb_dev_reset_delayed_task);
+	schedule_work(&reset_ctx->ws);
+	return;
+
+error_pending:
+	usb_put_dev(usb_dev);
+	module_put(THIS_MODULE);
+error_module_get:
+	kfree(reset_ctx);
+	return;
+}
+EXPORT_SYMBOL_GPL(usb_dev_reset_delayed);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel