[PATCH v1 6/9] usb: xhci: Add NVIDIA Tegra XHCI host-controller driver
From: Andrew Bresticker <hidden>
Date: 2014-07-08 21:52:15
Also in:
linux-devicetree, linux-tegra, lkml
On Fri, Jun 20, 2014 at 9:58 AM, Julius Werner [off-list ref] wrote:
quoted
+static const struct hc_driver tegra_xhci_hc_driver = { + .description = "tegra-xhci-hcd", + .product_desc = "Tegra xHCI Host Controller", + .hcd_priv_size = sizeof(struct xhci_hcd *), + + /* + * generic hardware linkage + */ + .irq = xhci_irq, + .flags = HCD_MEMORY | HCD_USB3 | HCD_SHARED, + + /* + * basic lifecycle operations + */ + .reset = tegra_xhci_setup, + .start = xhci_run, + .stop = xhci_stop, + .shutdown = xhci_shutdown, + + /* + * managing i/o requests and associated device resources + */ + .urb_enqueue = xhci_urb_enqueue, + .urb_dequeue = xhci_urb_dequeue, + .alloc_dev = xhci_alloc_dev, + .free_dev = xhci_free_dev, + .alloc_streams = xhci_alloc_streams, + .free_streams = xhci_free_streams, + .add_endpoint = xhci_add_endpoint, + .drop_endpoint = xhci_drop_endpoint, + .endpoint_reset = xhci_endpoint_reset, + .check_bandwidth = xhci_check_bandwidth, + .reset_bandwidth = xhci_reset_bandwidth, + .address_device = xhci_address_device, + .enable_device = xhci_enable_device, + .update_hub_device = xhci_update_hub_device, + .reset_device = xhci_discover_or_reset_device, + + /* + * scheduling support + */ + .get_frame_number = xhci_get_frame, + + /* Root hub support */ + .hub_control = xhci_hub_control, + .hub_status_data = xhci_hub_status_data, + .bus_suspend = xhci_bus_suspend, + .bus_resume = xhci_bus_resume, +};I know I missed the first round of discussion where this was suggested, but I don't think it's a good idea to pull the whole hc_driver structure out into every platform implementation. It will lead to duplication, then to future additions only being applied to some of the implementations and everything getting out of sync. This is already a problem with the PCI/plat split (e.g. the LPM functions were only added to xhci-pci even though they should apply to both). Also, if I'm not mistaken this code would fail to compile as a module (you are referencing lots of symbols that are internal to the xhci-hcd module).
You're right Julius, this won't build as a module without a few EXPORT_SYMBOLs.
I think at the very least you should add a function "xhci_default_driver(struct hc_driver *driver)" to xhci-plat.c (or even better to xhci.c and use it for PCI as well) that initializes all function pointers to the default (internal) symbols, and can then be overridden afterwards.
Currently all XHCI host drivers (PCI, platform, MVEBU) will be built into the xhci-hcd module. I could append the Tegra driver to that module or introduce a xhci_init_driver() like EHCI does as Julius suggests. USB folks, do you have a preference?