From linux-usb-devel-admin@lists.sourceforge.net Thu May  5 13:58:35 2005
From: Roman Kagan <rkagan@mail.ru>
To: David Brownell <david-b@pacbell.net>
Subject: USB: update urb documentation
Message-ID: <20050505205556.GB2352@katya>
Date: Fri, 6 May 2005 00:55:56 +0400

On Wed, May 04, 2005 at 01:37:30PM -0700, David Brownell wrote:
> On Wednesday 04 May 2005 12:19 pm, Roman Kagan wrote:
> > struct urb {
> > 	/* private, usb core and host controller only fields in the urb */
> > 	...
> > 	struct list_head urb_list;	/* list pointer to all active urbs */
> > 	...
> > };
> > 
> > Is it safe to use it for driver's purposes when the driver owns the urb,
> > that is, starting from the completion routine until the urb is submitted
> > with usb_submit_urb()? 
> 
> Right now, it should be.

Great!  FWIW I've briefly tested a modified version of usbatm using
the list head in struct urb instead of creating a wrapper struct, and I
haven't seen any failures yet.  So I tend to believe that your "should
be" actually means "is" :)

> > If it is, can it be guaranteed in future, e.g. 
> > by moving the list head into the public section of struct urb?
> 
> In fact I'm not sure why it ever got called "private" to usbcore/hcds.
> I thought the idea was that it should be like urb->status, reserved for
> whoever controls the URB.

OK then how about the following (essentially documentation) patch?


Signed-off-by: Roman Kagan <rkagan@mail.ru>
Acked-by: David Brownell <david-b@pacbell.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 include/linux/usb.h |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletion(-)

--- gregkh-2.6.orig/include/linux/usb.h	2005-05-06 09:23:02.000000000 -0700
+++ gregkh-2.6/include/linux/usb.h	2005-05-06 12:14:14.000000000 -0700
@@ -795,6 +795,10 @@
  * of the iso_frame_desc array, and the number of errors is reported in
  * error_count.  Completion callbacks for ISO transfers will normally
  * (re)submit URBs to ensure a constant transfer rate.
+ *
+ * Note that even fields marked "public" should not be touched by the driver
+ * when the urb is owned by the hcd, that is, since the call to
+ * usb_submit_urb() till the entry into the completion routine.
  */
 struct urb
 {
@@ -802,12 +806,12 @@
 	struct kref kref;		/* reference count of the URB */
 	spinlock_t lock;		/* lock for the URB */
 	void *hcpriv;			/* private data for host controller */
-	struct list_head urb_list;	/* list pointer to all active urbs */
 	int bandwidth;			/* bandwidth for INT/ISO request */
 	atomic_t use_count;		/* concurrent submissions counter */
 	u8 reject;			/* submissions will fail */
 
 	/* public, documented fields in the urb that can be used by drivers */
+	struct list_head urb_list;	/* list head for use by the urb owner */
 	struct usb_device *dev; 	/* (in) pointer to associated device */
 	unsigned int pipe;		/* (in) pipe information */
 	int status;			/* (return) non-ISO status */