On Tue, Mar 08, 2011 at 10:52:16AM -0500, Alan Stern wrote:
> On Mon, 7 Mar 2011, Sarah Sharp wrote:
>
> > Introduce the notion of a PCI device that may be associated with more than
> > one USB host controller driver (struct usb_hcd). This patch is the start
> > of the work to separate the xHCI host controller into two roothubs: a USB
> > 3.0 roothub with SuperSpeed-only ports, and a USB 2.0 roothub with
> > HS/FS/LS ports.
> >
> > One usb_hcd structure is designated to be the "primary HCD", and a pointer
> > is added to the usb_hcd structure to keep track of that. A new function
> > call, usb_hcd_is_primary_hcd() is added to check whether the USB hcd is
> > marked as the primary HCD (or if it is not part of a roothub pair). To
> > allow the USB core and xHCI driver to access either roothub in a pair, a
> > "shared_hcd" pointer is added to the usb_hcd structure.
> >
> > Add a new function, usb_create_shared_hcd(), that does roothub allocation
> > for paired roothubs. It will act just like usb_create_hcd() did if the
> > primary_hcd pointer argument is NULL. If it is passed a non-NULL
> > primary_hcd pointer, it sets usb_hcd->shared_hcd and usb_hcd->primary_hcd
> > fields. It will also skip the bandwidth_mutex allocation, and set the
> > secondary hcd's bandwidth_mutex pointer to the primary HCD's mutex.
> >
> > IRQs are only allocated once for the primary roothub.
> >
> > Introduce a new usb_hcd driver flag that indicates the host controller
> > driver wants to create two roothubs. If the HCD_SHARED flag is set, then
> > the USB core PCI probe methods will allocate a second roothub, and make
> > sure that second roothub gets freed during rmmod and in initialization
> > error paths.
>
> ...
>
> > +/*
> > + * Roothubs that share one PCI device must also share the bandwidth mutex.
> > + * Don't deallocate the bandwidth_mutex until the last shared usb_hcd is
> > + * deallocated.
> > + *
> > + * The first shared roothub that is released will set shared_hcd to NULL. The
> > + * second shared roothub that is released (or a USB host controller driver that
> > + * doesn't set up shared roothubs) will deallocate the bandwidth mutex.
> > + */
> > static void hcd_release (struct kref *kref)
> > {
> > struct usb_hcd *hcd = container_of (kref, struct usb_hcd, kref);
> >
> > - kfree(hcd->bandwidth_mutex);
> > + if (hcd->shared_hcd)
> > + hcd->shared_hcd->shared_hcd = NULL;
> > + else
> > + kfree(hcd->bandwidth_mutex);
> > kfree(hcd);
> > }
>
> This is okay as it stands, but I wonder if it can be simplified. The
> order in which the hcd structures are released doesn't matter, because
> the bandwidth mutex won't be accessed after the root hubs are
> unregistered. Hence the mutex can be deallocated when the primary hcd
> is removed (since the primary hcd structure always has to be removed
> last).
>
> Or am I missing something?
No, you're not missing something, and I think your suggestion simplifies
things a lot.
The hcd->primary_hcd is set when usb_create_shared_hcd() is called, but
looking through the code, it's never cleared when the primary_hcd is
freed. It would be useful to use that fact when testing for the
primary_hcd in hcd_release(), but I wonder if it will lead to bad
pointer dereferences later...
Sarah Sharp
--
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
