linux-kernel - Re: [PATCH v1 6/9] usb: xhci: Add NVIDIA Tegra XHCI host-controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL1qeaHT8Yz7kRY3Qm5i+bYCF4D5BT=BVZ6BMfQufyaQFkt0mw@mail.gmail.com>
Date:	Tue, 8 Jul 2014 14:52:10 -0700
From:	Andrew Bresticker <abrestic@...omium.org>
To:	Julius Werner <jwerner@...omium.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	Mathias Nyman <mathias.nyman@...el.com>,
	Alan Stern <stern@...land.harvard.edu>
Cc:	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	linux-doc@...r.kernel.org,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Randy Dunlap <rdunlap@...radead.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Thierry Reding <thierry.reding@...il.com>,
	Russell King <linux@....linux.org.uk>,
	Linus Walleij <linus.walleij@...aro.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Grant Likely <grant.likely@...aro.org>,
	Kishon Vijay Abraham I <kishon@...com>,
	Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH v1 6/9] usb: xhci: Add NVIDIA Tegra XHCI host-controller driver

On Fri, Jun 20, 2014 at 9:58 AM, Julius Werner <jwerner@...omium.org> wrote:
>> +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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/