[PATCH v9 2/4] ehci-platform: Add support for clks and phy passed through devicetree

On 02/11/2014 12:13 PM, Hans de Goede wrote:

Hi,

On 02/11/2014 11:00 AM, Roger Quadros wrote:

quoted

On 02/11/2014 11:31 AM, Hans de Goede wrote:

quoted

Hi,

On 02/11/2014 10:12 AM, Roger Quadros wrote:

quoted

Hi Hans,

On 02/07/2014 05:36 PM, Hans de Goede wrote:

quoted

Currently ehci-platform is only used in combination with devicetree when used
with some Via socs. By extending it to (optionally) get clks and a phy from
devicetree, and enabling / disabling those on power_on / off, it can be used
more generically. Specifically after this commit it can be used for the
ehci controller on Allwinner sunxi SoCs.

Since ehci-platform is intended to handle any generic enough non pci ehci
device, add a "usb-ehci" compatibility string.

There already is a usb-ehci device-tree bindings document, update this
with clks and phy bindings info.

Although actually quite generic so far the via,vt8500 compatibilty string
had its own bindings document. Somehow we even ended up with 2 of them. Since
these provide no extra information over the generic usb-ehci documentation,
this patch removes them.

The ehci-ppc-of.c driver also claims the usb-ehci compatibility string,
even though it mostly is ibm,usb-ehci-440epx specific. ehci-platform.c is
not needed on ppc platforms, so add a !PPC_OF dependency to it to avoid
2 drivers claiming the same compatibility string getting build on ppc.

This breaks all OMAP platforms on linux-next for the exact same reason. see [1].

./arch/arm/boot/dts/omap4.dtsi:				compatible = "ti,ehci-omap", "usb-ehci";
./arch/arm/boot/dts/omap3.dtsi:				compatible = "ti,ehci-omap", "usb-ehci";
./arch/arm/boot/dts/omap5.dtsi:				compatible = "ti,ehci-omap", "usb-ehci";

That should not be the case, the driver core should try to find a driver matching
the compatibility string from left to right, or in other words from most specific
to least specific. This is part of the whole devicetree design.

So as long as the driver claiming "ti,ehci-omap" is available at probe time that
one should get used and things should work fine. Now if ehci-platform is built-in
and ehci-omap is a module, then I guess one could see the described breakage.

If the driver is built-in and things are not working, then we will need to do some
debugging as to why the left to right matching is not working as expected.

Both ehci_platform and ehci_omap were built-in and still the ehci_platform driver got
probe preference. So it looks like the left to right compatible list priority probing
feature doesn't work.

Oops, I guess nothing relies on it sofar. Well we could go and debug and fix this but...

I think we have a misunderstanding. The compatible list priority probing is only for the
platform identification compatible. I don't think it is meant to work with driver core yet.

http://lxr.free-electrons.com/source/Documentation/devicetree/usage-model.txt#L116

quoted

quoted

I must admit I'm not sure what happens if both are a module, the kernel direct
module load will likely fail due to lack of a rootfs at that point, and then
the module will later get loaded by udev I assume, at which point there are no
loading ordering guarantees.

The easiest solution to ensure that "ti,ehci-omap" is available at probe time
(if enabled) seems to be to change USB_EHCI_HCD_OMAP to a boolean.

That is a limitation I don't like to have for USB_EHCI_HCD_OMAP.

I completely understand, thinking more about this I'm simply going to change the
compatibility string for ohci- and ehci-platform to be "ohci-platform" resp.
"ehci-platform". I know there are some people who don't like the -platform
suffix, but though luck, as the above clearly shows using the generic "usb-ohci" /
"usb-ehci" they were advocating for leads to a ton of issues, and we already
have a precedent for ?hci-platform in the form of "xhci-platform".

+1

cheers,
-roger