On Thu, Dec 02, 2010 at 03:18:25PM -0500, Alan Stern wrote:
> On Thu, 2 Dec 2010, Sarah Sharp wrote:
>
> > > Have the core check for HCD_NO_RESOURCES. If that flag is set, don't
> > > allocate or free the bandwidth mutex. In your probe routine, copy the
> > > mutex pointer from one hcd to the other.
> > >
> > > I admit, this amounts to a bunch of somewhat ad-hoc changes just to
> > > avoid changing the hcd-is-a-bus assumption. But since that would end
> > > up being such a big change, this seems justified.
> >
> > When the PCI probe function sees HCD_NO_RESOURCES, should it also not
> > allocate things like hcd bounce buffers?
>
> It doesn't allocate bounce buffers currently, so I don't see why it
> should start doing so when HCD_NO_RESOURCES is set. It _does_ create a
> few DMA pools, but that's not a terribly big deal.
>
> > Should those be shared across
> > the two roothubs?
>
> That's up to you.
I really only made the usb_hcds share fields that I understood how they
were used. If you don't think it's a big deal for the usb_hcds to have
mostly independently allocated resources, I'm not inclined to make them
share things like DMA pools.
> > If the answer is yes, I'd rather have the code to
> > make those decisions in the USB core, rather than the xHCI driver. It
> > seems like it would be a time-bomb just waiting for someone to add some
> > sort of new resource to the HCD structure and not update the xHCI
> > driver. I'd really rather have the changes be visible in the USB core.
>
> I suppose instead of HCD_NO_RESOURCES you could add a "primary_hcd"
> pointer to the usb_hcd structure. Normally it would be NULL, but for
> secondary hcd's (where you don't want resources allocated) you set it
> to point to the associated primary hcd. Then whenever this pointer is
> set, instead of allocating a new set of DMA pools the core could just
> copy the pool pointers from the primary hcd.
I already added a "shared_hcd" pointer in the usb_hcd structure. That's needed
because the xHCI driver needs to let the USB core know when both usb_hcds die,
or to figure out which bus to kick khubd for when a port status change comes
in. I already have a "main_hcd" field in the xhci_hcd structure that points to
the USB 3.0 usb_hcd. I need that because I can't use containerof to get the
usb_hcd from the xhci_hcd, since the hcd->priv is now a pointer, not a
statically allocated structure. I could mirror that field into a "primary_hcd"
field in usb_hcd, but it seems redundant.
> The advantage over what you posted is that this doesn't make
> assumptions about dev_get_drvdata(hcd->self.controller); the core
> shouldn't have anything to do with this -- it's private to the driver.
> (Some drivers might not want to store a pointer to the hcd there.)
Since the USB core set that data in the PCI probe function, I thought it
was private to the USB core. But if other non-PCI drivers can set it to
something else, then yes, the usb_hcd_is_main_hcd() function shouldn't
check dev_get_drvdata(hcd->self.controller). In that case, it should
either check the primary_hcd field or the bcdUSB field you suggested.
I'm leaning towards the bcdUSB field, so I don't have the redundant
primary_hcd field. Then usb_hcd_is_main_hcd() would look something like
this:
inline int usb_hcd_is_main_hcd(struct usb_hcd *hcd)
{
if ((hcd->driver->flags & HCD_MASK) == HCD_USB3 &&
hcd->bcdUSB == HCD_USB2)
return 0;
return 1;
}
EXPORT_SYMBOL_GPL(usb_hcd_is_main_hcd);
bcdUSB could be originally set to xhci->driver->flags & HCD_MASK, and
then could be over-written in the usb_hcd_pci_probe() function after the
second usb_hcd function is created.
> It also doesn't make assumptions about whether the primary is USB-2 and
> the secondary is USB-3 or vice versa.
The USB core really has to know both whether the usb_hcd is the primary
usb_hcd, and whether the usb_hcd is the USB2 or USB3 roothub.
Of the 6 calls to usb_hcd_is_main_hcd() in the core, 3 are used to
determine the type of roothub (for figuring out which device descriptors
or configuration descriptors to return for the roothub, and setting the
roothub speed in hcd.c). Three callers use usb_hcd_is_main_hcd() to
decide whether to enable/free IRQs.
> Alternatively, we could bite the bullet and really separate buses from
> hcds. The difficulty of doing this will only increase as time passes.
I'm not willing to go down that path right now. I have other xHCI
features I need to work on. :)
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
