On Mon, May 14, 2012 at 02:41:41PM -0400, Alan Stern wrote:
> On Mon, 14 May 2012, Felipe Balbi wrote:
>
> > > + case PCI_VENDOR_ID_PHILIPS:
> > > + /*
> > > + * Philips controllers set HCC_PGM_FRAMELISTLEN, but
> > > + * they don't implement schedule sizes shorter than 1024.
> > > + */
> > > + ehci->sched_size_bug = 1;
> > > }
> > >
> > > /* cache this readonly data; minimize chip reads */
> >
> > 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
don't understand why you say you still require runtime checks. The only
problem I see is that the way the *HCI PCI drivers were written, we
do all quirk checks too late and don't have access to the pci_device_id
pointer anymore. Still, all your switch jump table could be converted
into something like:
ehci->no_selective_suspend = data->no_selective_suspend;
ehci->has_fsl_port_bug = data->has_fsl_port_bug;
ehci->big_endian_desc = data->big_endian_desc;
ehci->big_endian_capbase = data->big_endian_capbase;
ehci->has_amcc_usb23 = data->has_amcc_usb23;
ehci->need_io_watchdog = data->need_io_watchdog;
ehci->broken_periodic = data->broken_periodic;
ehci->amd_pll_fix = data->amd_pll_fix;
ehci->fs_i_thresh = data->fs_i_thresh;
ehci->use_dummy_qh = data->use_dummy_qh;
ehci->has_synopsys_hc_bug = data->has_synopsys_hc_bug;
ehci->frame_index_bug = data->frame_index_bug;
#ifdef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
ehci->big_endian_mmio = data->big_endian_mmio;
#endif
Because all those field were properly initialized on pci_device_id. It
would look something like:
static const struct
ehci_pci_driver_data ehci_intel_27cc_driver_data __devinitconst = {
.fs_i_thresh = true,
.broken_periodic = true,
};
static DEFINE_PCI_DEVICE_TABLE(pci_ids) = {
PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x27cc)
.driver_data = (kernel_ulong_t) &ehci_intel_27cc_driver_data,
};
or something similar. The only thing I saw was the VIA VT6212 which uses
a different sleep time, but that register write can be done no matter if
it's VIA or not... just write the 10usec to the register on all cases.
It won't (shouldn't) do any harm.
--
balbi
Attachment:
signature.asc
Description: Digital signature
