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: <CAODwPW-HSY3RoBi9VEhHSJ98drTsdche-2=mKfAViXWaUa3X1g@mail.gmail.com>
Date:	Fri, 20 Jun 2014 09:58:19 -0700
From:	Julius Werner <jwerner@...omium.org>
To:	Andrew Bresticker <abrestic@...omium.org>
Cc:	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	linux-doc@...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>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.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>,
	Mathias Nyman <mathias.nyman@...el.com>,
	Grant Likely <grant.likely@...aro.org>,
	Alan Stern <stern@...land.harvard.edu>,
	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

> +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).

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.
--
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/