On Wed, 6 Nov 2013, Alistair Popple wrote:
> Currently the ppc-of driver uses the compatibility string
> "usb-ehci". This means platforms that use device-tree and implement an
> EHCI compatible interface have to either use the ppc-of driver or add
> a compatible line to the ehci-platform driver. It would be more
> appropriate for the platform driver to be compatible with "usb-ehci"
> as non-powerpc platforms are also beginning to utilise device-tree.
>
> This patch merges the device tree property parsing from ehci-ppc-of
> into the platform driver and adds a "usb-ehci" compatibility
> string. The existing ehci-ppc-of driver is renamed to ehci-440epx as
> it contains platform specific work arounds for the 440EPX SoC.
>
> Signed-off-by: Alistair Popple <alist...@popple.id.au>
> ---
>
> So I could submit something like this that essentially merges the ppc-of
> driver into the platform driver instead of adding the "ibm,akebono-ehci"
> compatible line to the platform driver.
>
> However I'm still fairly new to device-tree so I'm not sure what (if any) the
> broader impact would be. A quick grep for "usb-ehci" turned up a couple of
> ARM
> device tree's using it however they all provided their own drivers and don't
> select CONFIG_USB_EHCI_HCD_PLATFORM so I'm guess they shouldn't be impacted.
>
> I have attempted to fix up any PowerPC device trees/configs, although I
> wasn't
> sure if "usb-ehci" should remain in sequoia.dts or not given that it needs to
> use the 440EPX specific driver.
>
> Also this hasn't been tested (beyond compilation) yet.
> diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-
> platform.c
> index f6b790c..027f368 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -77,6 +77,7 @@ static int ehci_platform_probe(struct platform_device *dev)
> struct usb_hcd *hcd;
> struct resource *res_mem;
> struct usb_ehci_pdata *pdata;
> + struct device_node *dn = dev->dev.of_node;
> int irq;
> int err = -ENOMEM;
>
> @@ -96,6 +97,18 @@ static int ehci_platform_probe(struct platform_device *dev)
>
> pdata = dev_get_platdata(&dev->dev);
>
> + /* Initialise platform data from device tree if available. */
> + if (!dn) {
Shouldn't this be "if (dn)"?
> + if (of_get_property(dn, "big-endian", NULL)) {
> + pdata->big_endian_mmio = 1;
> + pdata->big_endian_desc = 1;
> + }
> + if (of_get_property(dn, "big-endian-regs", NULL))
> + pdata->big_endian_mmio = 1;
> + if (of_get_property(dn, "big-endian-desc", NULL))
> + pdata->big_endian_desc = 1;
> + }
> +
This isn't good if there is more than one EHCI controller using
ehci-platform. To accomodate such cases, it would be necessary to
allocate a separate copy of ehci_platform_defaults for each controller.
Alan Stern
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
