On Wed, 16 May 2012, Felipe Balbi wrote:
> On Tue, May 15, 2012 at 10:59:36AM -0400, Alan Stern wrote:
> > On Tue, 15 May 2012, Felipe Balbi wrote:
> >
> > > > > Unrelated to $SUBJECT, but all those quirks could be passed through
> > > > > driver_data from pci ID table. That will remove the need for the switch
> > > > > statement. My 2 cents
> > > >
> > > > Many of them could, it's true. I don't know that it would be much of
> > > > an improvement, though. The code would still have to test for most of
> > > > the quirk flags because they require runtime checks (the one added by
> > > > this patch is an exception).
> > > >
> > > > Also, which do you think would occupy less memory space: an array of
> > > > pci_device_id structures or a switch statement jump table?
> > >
> > > The array gets freed after init with proper section annotations. I also
> >
> > Then what happens if you unbind ehci-hcd from a controller and rebind
> > it later?
>
> if you use "proper" section annotations that won't be a problem.
> __devinitconst won't free that table in such a case.
In other words, with "proper" section annotations the array doesn't get
freed after init. :-)
> > > don't understand why you say you still require runtime checks. The only
> >
> > Let's look through the code. The first NVIDIA handler does this:
> >
> > if (pci_set_consistent_dma_mask(pdev,
> > DMA_BIT_MASK(31)) < 0)
> > ehci_warn(ehci, "can't enable NVidia "
> > "workaround for >2GB RAM\n");
>
> you could set a flag for this, then check on probe()
>
> if (data->needs_nvidia_dma_workaround)
> pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(31));
>
> > One of the clauses in the Intel and TDI handlers does this:
> >
> > hcd->has_tt = 1;
> > tdi_reset(ehci);
>
> if (data->intel_tdi_handler) {
> hcd->has_tt = true;
> tdi_reset(ehci);
> }
Sure, that would work. You are essentially replacing a bunch of
"switch" cases with a bunch of "if" statements. Not a big change IMO.
On the other hand, all the other cases which really do nothing but set
a flag could be replaced entirely with driver_data.
> Not the way it's done currently, no. The thing is that we have
> driver_data for a reason and that's not to pass the driver itself, IMHO.
You are correct that the PCI drivers currently don't use driver_data
the way they should. It's one of the first things that will have to be
changed when we merge the PCI and other platform drivers.
In fact, it should be a fairly simple change. I'll try to get around
to it in the next week or so.
Alan Stern
--
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
