ohci-/ehci-platform: Change compatible string to ?hci-platform

ohci-/ehci-platform: Change compatible string to ?hci-platform

[Hans de Goede]

Hi Greg,

Can you please add these 2 patches to usb-next, to unbreak usb on various
ARM platforms?

These 2 patches can either be squashed into the first 2 patches of my previous
set or added as is to preserve history, either way is fine with me.

The 2nd patch also fixes one of the Kconfig issues and I've a better plan
for the 2nd Kconfig issue too, so please consider this a self-nack for my
drivers/usb/host/Kconfig patches.

Here is a copy of the echi-platform commit-msg to explain the rationale of
these changes, as well as how the breakage happened in the first place:

The initial versions of the devicetree enablement patches for ehci-platform
used "ehci-platform" as compatible string. However this was disliked by various
reviewers because the platform bus is a Linux invention and devicetree is
supposed to be OS agnostic. After much discussion I gave up, added a:
"depends on !PPC_OF" to Kconfig to avoid a known conflict with PPC-OF platforms
and went with the generic usb-ehci as requested.

In retro-spect I should have stuck to my guns, because the dts files for many
existing boards already claim to be compatible with "usb-ehci", ie they have:

	compatible = "ti,ehci-omap", "usb-ehci";

In theory this should not be a problem since the "ti,ehci-omap" entry takes
presedence, but in practice using a conflicting compatible string is an issue,
because it makes which driver gets used depend on driver registration order.

This patch changes the compatible string claimed by ehci-platform (back) to
"ehci-platform", avoiding the driver registration / module loading ordering
problems, and removes the "depends on !PPC_OF" workaround.

Note that there already is a precedent for using ?hci-platform, in the form
of xhci-platform.c using "xhci-platfrom" as compatible string.

Thanks & Regards,

Hans

[PATCH 1/2] ohci-platform: Change compatible string from usb-ohci to ohci-platform

[Hans de Goede]

The initial versions of the devicetree enablement patches for ohci-platform
used "ohci-platform" as compatible string. However this was disliked by various
reviewers because the platform bus is a Linux invention and devicetree is
supposed to be OS agnostic. After much discussion I gave up and went with
the generic usb-ohci as requested.

In retro-spect I should have stuck to my guns, because the dts files for many
existing boards already claim to be compatible with "usb-ohci", ie they have:

	compatible = "ti,ohci-omap3", "usb-ohci";

In theory this should not be a problem since the "ti,ohci-omap3" entry takes
presedence, but in practice using a conflicting compatible string is an issue,
because it makes which driver gets used depent on driver registration order.

This patch changes the compatible string claimed by ohci-platform (back) to
"ohci-platform", avoiding the driver registration / module loading ordering
problems. Note that there already is a precedent for using ?hci-platform, in
the form of xhci-platform.c using "xhci-platfrom" as compatible string.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/usb/usb-ohci.txt | 4 ++--
 drivers/usb/host/ohci-platform.c                   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
index 6933b0c..a8e576a 100644
--- a/Documentation/devicetree/bindings/usb/usb-ohci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
@@ -1,7 +1,7 @@
 USB OHCI controllers
 
 Required properties:
-- compatible : "usb-ohci"
+- compatible : "ohci-platform"
 - reg : ohci controller register range (address and length)
 - interrupts : ohci controller interrupt
 
@@ -16,7 +16,7 @@ Optional properties:
 Example:
 
 	ohci0: usb at 01c14400 {
-		compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
+		compatible = "allwinner,sun4i-a10-ohci", "ohci-platform";
 		reg = <0x01c14400 0x100>;
 		interrupts = <64>;
 		clocks = <&usb_clk 6>, <&ahb_gates 2>;
diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index e2c28fd..59f3551 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -319,7 +319,7 @@ static int ohci_platform_resume(struct device *dev)
 #endif /* CONFIG_PM */
 
 static const struct of_device_id ohci_platform_ids[] = {
-	{ .compatible = "usb-ohci", },
+	{ .compatible = "ohci-platform", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ohci_platform_ids);
-- 
1.8.5.3

[PATCH 1/2] ohci-platform: Change compatible string from usb-ohci to ohci-platform

[Kumar Gala]

On Feb 11, 2014, at 8:10 AM, Hans De Goede <hdegoede@redhat.com> wrote:

> The initial versions of the devicetree enablement patches for ohci-platform
> used "ohci-platform" as compatible string. However this was disliked by various
> reviewers because the platform bus is a Linux invention and devicetree is
> supposed to be OS agnostic. After much discussion I gave up and went with
> the generic usb-ohci as requested.
> 
> In retro-spect I should have stuck to my guns, because the dts files for many
> existing boards already claim to be compatible with "usb-ohci", ie they have:
> 
> 	compatible = "ti,ohci-omap3", "usb-ohci";
> 
> In theory this should not be a problem since the "ti,ohci-omap3" entry takes
> presedence, but in practice using a conflicting compatible string is an issue,
> because it makes which driver gets used depent on driver registration order.
> 
> This patch changes the compatible string claimed by ohci-platform (back) to
> "ohci-platform", avoiding the driver registration / module loading ordering
> problems. Note that there already is a precedent for using ?hci-platform, in
> the form of xhci-platform.c using "xhci-platfrom" as compatible string.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Documentation/devicetree/bindings/usb/usb-ohci.txt | 4 ++--
> drivers/usb/host/ohci-platform.c                   | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
> index 6933b0c..a8e576a 100644
> --- a/Documentation/devicetree/bindings/usb/usb-ohci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
> @@ -1,7 +1,7 @@
> USB OHCI controllers
> 
> Required properties:
> -- compatible : "usb-ohci?

Why not leave ?usb-ohci? and deprecate it?

> +- compatible : "ohci-platform"
> - reg : ohci controller register range (address and length)
> - interrupts : ohci controller interrupt
> 
> @@ -16,7 +16,7 @@ Optional properties:
> Example:
> 
> 	ohci0: usb at 01c14400 {
> -		compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
> +		compatible = "allwinner,sun4i-a10-ohci", "ohci-platform";
> 		reg = <0x01c14400 0x100>;
> 		interrupts = <64>;
> 		clocks = <&usb_clk 6>, <&ahb_gates 2>;
> diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
> index e2c28fd..59f3551 100644
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -319,7 +319,7 @@ static int ohci_platform_resume(struct device *dev)
> #endif /* CONFIG_PM */
> 
> static const struct of_device_id ohci_platform_ids[] = {
> -	{ .compatible = "usb-ohci", },
> +	{ .compatible = "ohci-platform", },
> 	{ }
> };
> MODULE_DEVICE_TABLE(of, ohci_platform_ids);
> -- 
> 1.8.5.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 1/2] ohci-platform: Change compatible string from usb-ohci to ohci-platform

[Hans de Goede]

Hi,

On 02/11/2014 04:06 PM, Kumar Gala wrote:
> 
> On Feb 11, 2014, at 8:10 AM, Hans De Goede <hdegoede@redhat.com> wrote:
> 
>> The initial versions of the devicetree enablement patches for ohci-platform
>> used "ohci-platform" as compatible string. However this was disliked by various
>> reviewers because the platform bus is a Linux invention and devicetree is
>> supposed to be OS agnostic. After much discussion I gave up and went with
>> the generic usb-ohci as requested.
>>
>> In retro-spect I should have stuck to my guns, because the dts files for many
>> existing boards already claim to be compatible with "usb-ohci", ie they have:
>>
>> 	compatible = "ti,ohci-omap3", "usb-ohci";
>>
>> In theory this should not be a problem since the "ti,ohci-omap3" entry takes
>> presedence, but in practice using a conflicting compatible string is an issue,
>> because it makes which driver gets used depent on driver registration order.
>>
>> This patch changes the compatible string claimed by ohci-platform (back) to
>> "ohci-platform", avoiding the driver registration / module loading ordering
>> problems. Note that there already is a precedent for using ?hci-platform, in
>> the form of xhci-platform.c using "xhci-platfrom" as compatible string.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Documentation/devicetree/bindings/usb/usb-ohci.txt | 4 ++--
>> drivers/usb/host/ohci-platform.c                   | 2 +-
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
>> index 6933b0c..a8e576a 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-ohci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
>> @@ -1,7 +1,7 @@
>> USB OHCI controllers
>>
>> Required properties:
>> -- compatible : "usb-ohci?
> 
> Why not leave ?usb-ohci? and deprecate it?

As it was introduced just a couple of days ago in linux-next, and has never seen
the light in any released kernel.

Regards,

Hans

[PATCH 1/2] ohci-platform: Change compatible string from usb-ohci to ohci-platform

[Kumar Gala]

On Feb 11, 2014, at 9:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 02/11/2014 04:06 PM, Kumar Gala wrote:
>> 
>> On Feb 11, 2014, at 8:10 AM, Hans De Goede <hdegoede@redhat.com> wrote:
>> 
>>> The initial versions of the devicetree enablement patches for ohci-platform
>>> used "ohci-platform" as compatible string. However this was disliked by various
>>> reviewers because the platform bus is a Linux invention and devicetree is
>>> supposed to be OS agnostic. After much discussion I gave up and went with
>>> the generic usb-ohci as requested.
>>> 
>>> In retro-spect I should have stuck to my guns, because the dts files for many
>>> existing boards already claim to be compatible with "usb-ohci", ie they have:
>>> 
>>> 	compatible = "ti,ohci-omap3", "usb-ohci";
>>> 
>>> In theory this should not be a problem since the "ti,ohci-omap3" entry takes
>>> presedence, but in practice using a conflicting compatible string is an issue,
>>> because it makes which driver gets used depent on driver registration order.
>>> 
>>> This patch changes the compatible string claimed by ohci-platform (back) to
>>> "ohci-platform", avoiding the driver registration / module loading ordering
>>> problems. Note that there already is a precedent for using ?hci-platform, in
>>> the form of xhci-platform.c using "xhci-platfrom" as compatible string.
>>> 
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Documentation/devicetree/bindings/usb/usb-ohci.txt | 4 ++--
>>> drivers/usb/host/ohci-platform.c                   | 2 +-
>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
>>> index 6933b0c..a8e576a 100644
>>> --- a/Documentation/devicetree/bindings/usb/usb-ohci.txt
>>> +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
>>> @@ -1,7 +1,7 @@
>>> USB OHCI controllers
>>> 
>>> Required properties:
>>> -- compatible : "usb-ohci?
>> 
>> Why not leave ?usb-ohci? and deprecate it?
> 
> As it was introduced just a couple of days ago in linux-next, and has never seen
> the light in any released kernel.

ah, never mind :)

- k
-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 2/2] ehci-platform: Change compatible string from usb-ehci to ehci-platform

[Hans de Goede]

The initial versions of the devicetree enablement patches for ehci-platform
used "ehci-platform" as compatible string. However this was disliked by various
reviewers because the platform bus is a Linux invention and devicetree is
supposed to be OS agnostic. After much discussion I gave up, added a:
"depends on !PPC_OF" to Kconfig to avoid a known conflict with PPC-OF platforms
and went with the generic usb-ehci as requested.

In retro-spect I should have stuck to my guns, because the dts files for many
existing boards already claim to be compatible with "usb-ehci", ie they have:

	compatible = "ti,ehci-omap", "usb-ehci";

In theory this should not be a problem since the "ti,ehci-omap" entry takes
presedence, but in practice using a conflicting compatible string is an issue,
because it makes which driver gets used depent on driver registration order.

This patch changes the compatible string claimed by ehci-platform (back) to
"ehci-platform", avoiding the driver registration / module loading ordering
problems, and removes the "depends on !PPC_OF" workaround.

Note that there already is a precedent for using ?hci-platform, in the form
of xhci-platform.c using "xhci-platfrom" as compatible string.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/usb/usb-ehci.txt | 4 ++--
 drivers/usb/host/Kconfig                           | 1 -
 drivers/usb/host/ehci-platform.c                   | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt
index 2c1aeeb..46f428a 100644
--- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
@@ -1,7 +1,7 @@
 USB EHCI controllers
 
 Required properties:
-  - compatible : should be "usb-ehci".
+  - compatible : should be "ehci-platform".
   - reg : should contain at least address and length of the standard EHCI
     register set for the device. Optional platform-dependent registers
     (debug-port or other) can be also specified here, but only after
@@ -27,7 +27,7 @@ Example (Sequoia 440EPx):
 
 Example (Allwinner sun4i A10 SoC):
    ehci0: usb at 01c14000 {
-	   compatible = "allwinner,sun4i-a10-ehci", "usb-ehci";
+	   compatible = "allwinner,sun4i-a10-ehci", "ehci-platform";
 	   reg = <0x01c14000 0x100>;
 	   interrupts = <39>;
 	   clocks = <&ahb_gates 1>;
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index e28cbe0..a9707da 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -255,7 +255,6 @@ config USB_EHCI_ATH79
 
 config USB_EHCI_HCD_PLATFORM
 	tristate "Generic EHCI driver for a platform device"
-	depends on !PPC_OF
 	default n
 	---help---
 	  Adds an EHCI host driver for a generic platform device, which
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 8fde649..4f4d78f 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -333,7 +333,7 @@ static int ehci_platform_resume(struct device *dev)
 static const struct of_device_id vt8500_ehci_ids[] = {
 	{ .compatible = "via,vt8500-ehci", },
 	{ .compatible = "wm,prizm-ehci", },
-	{ .compatible = "usb-ehci", },
+	{ .compatible = "ehci-platform", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, vt8500_ehci_ids);
-- 
1.8.5.3

[PATCH 2/2] ehci-platform: Change compatible string from usb-ehci to ehci-platform

[Roger Quadros]

Hi Hans,

On 02/11/2014 04:10 PM, Hans de Goede wrote:
> The initial versions of the devicetree enablement patches for ehci-platform
> used "ehci-platform" as compatible string. However this was disliked by various
> reviewers because the platform bus is a Linux invention and devicetree is
> supposed to be OS agnostic. After much discussion I gave up, added a:
> "depends on !PPC_OF" to Kconfig to avoid a known conflict with PPC-OF platforms
> and went with the generic usb-ehci as requested.
> 
> In retro-spect I should have stuck to my guns, because the dts files for many
> existing boards already claim to be compatible with "usb-ehci", ie they have:
> 
> 	compatible = "ti,ehci-omap", "usb-ehci";
> 
> In theory this should not be a problem since the "ti,ehci-omap" entry takes
> presedence, but in practice using a conflicting compatible string is an issue,
> because it makes which driver gets used depent on driver registration order.
> 
> This patch changes the compatible string claimed by ehci-platform (back) to
> "ehci-platform", avoiding the driver registration / module loading ordering
> problems, and removes the "depends on !PPC_OF" workaround.
> 
> Note that there already is a precedent for using ?hci-platform, in the form
> of xhci-platform.c using "xhci-platfrom" as compatible string.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  Documentation/devicetree/bindings/usb/usb-ehci.txt | 4 ++--
>  drivers/usb/host/Kconfig                           | 1 -
>  drivers/usb/host/ehci-platform.c                   | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> index 2c1aeeb..46f428a 100644
> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
> @@ -1,7 +1,7 @@
>  USB EHCI controllers
>  
>  Required properties:
> -  - compatible : should be "usb-ehci".
> +  - compatible : should be "ehci-platform".

Won't this break DT binding info for power PC?

I'm even OK with removing "usb-ehci" and "usb-ohci" compatibles from all OMAP dts files
since they aren't really compatible with the original PPC driver.

cheers,
-roger

>    - reg : should contain at least address and length of the standard EHCI
>      register set for the device. Optional platform-dependent registers
>      (debug-port or other) can be also specified here, but only after
> @@ -27,7 +27,7 @@ Example (Sequoia 440EPx):
>  
>  Example (Allwinner sun4i A10 SoC):
>     ehci0: usb at 01c14000 {
> -	   compatible = "allwinner,sun4i-a10-ehci", "usb-ehci";
> +	   compatible = "allwinner,sun4i-a10-ehci", "ehci-platform";
>  	   reg = <0x01c14000 0x100>;
>  	   interrupts = <39>;
>  	   clocks = <&ahb_gates 1>;
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index e28cbe0..a9707da 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -255,7 +255,6 @@ config USB_EHCI_ATH79
>  
>  config USB_EHCI_HCD_PLATFORM
>  	tristate "Generic EHCI driver for a platform device"
> -	depends on !PPC_OF
>  	default n
>  	---help---
>  	  Adds an EHCI host driver for a generic platform device, which
> diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
> index 8fde649..4f4d78f 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -333,7 +333,7 @@ static int ehci_platform_resume(struct device *dev)
>  static const struct of_device_id vt8500_ehci_ids[] = {
>  	{ .compatible = "via,vt8500-ehci", },
>  	{ .compatible = "wm,prizm-ehci", },
> -	{ .compatible = "usb-ehci", },
> +	{ .compatible = "ehci-platform", },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, vt8500_ehci_ids);
> 

[PATCH 2/2] ehci-platform: Change compatible string from usb-ehci to ehci-platform

[Hans de Goede]

Hi,

On 02/11/2014 04:00 PM, Roger Quadros wrote:
> Hi Hans,
> 
> On 02/11/2014 04:10 PM, Hans de Goede wrote:
>> The initial versions of the devicetree enablement patches for ehci-platform
>> used "ehci-platform" as compatible string. However this was disliked by various
>> reviewers because the platform bus is a Linux invention and devicetree is
>> supposed to be OS agnostic. After much discussion I gave up, added a:
>> "depends on !PPC_OF" to Kconfig to avoid a known conflict with PPC-OF platforms
>> and went with the generic usb-ehci as requested.
>>
>> In retro-spect I should have stuck to my guns, because the dts files for many
>> existing boards already claim to be compatible with "usb-ehci", ie they have:
>>
>> 	compatible = "ti,ehci-omap", "usb-ehci";
>>
>> In theory this should not be a problem since the "ti,ehci-omap" entry takes
>> presedence, but in practice using a conflicting compatible string is an issue,
>> because it makes which driver gets used depent on driver registration order.
>>
>> This patch changes the compatible string claimed by ehci-platform (back) to
>> "ehci-platform", avoiding the driver registration / module loading ordering
>> problems, and removes the "depends on !PPC_OF" workaround.
>>
>> Note that there already is a precedent for using ?hci-platform, in the form
>> of xhci-platform.c using "xhci-platfrom" as compatible string.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  Documentation/devicetree/bindings/usb/usb-ehci.txt | 4 ++--
>>  drivers/usb/host/Kconfig                           | 1 -
>>  drivers/usb/host/ehci-platform.c                   | 2 +-
>>  3 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-ehci.txt b/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> index 2c1aeeb..46f428a 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-ehci.txt
>> @@ -1,7 +1,7 @@
>>  USB EHCI controllers
>>  
>>  Required properties:
>> -  - compatible : should be "usb-ehci".
>> +  - compatible : should be "ehci-platform".
> 
> Won't this break DT binding info for power PC?

The powerpc bindings have never been really properly documented, ie
they rely on both usb-ehci and ibm,usb-ehci-440epx strings being there,
which was never documented. Given the issues surrounding using usb-ehci
as a compatible string I think completely removing it from the bindings
docs is best.

> I'm even OK with removing "usb-ehci" and "usb-ohci" compatibles from all OMAP dts files
> since they aren't really compatible with the original PPC driver.

I don't think that is necessary, as your grep has shown there are a lot
of dts files using compatible = "foo", "usb-?hci"; and some may even have
the dts in firmware, so we should simply make sure not to break such dts.

Regards,

Hans

[PATCH 2/2] ehci-platform: Change compatible string from usb-ehci to ehci-platform

[Maxime Ripard]

On Tue, Feb 11, 2014 at 04:26:16PM +0100, Hans de Goede wrote:
> > I'm even OK with removing "usb-ehci" and "usb-ohci" compatibles
> > from all OMAP dts files since they aren't really compatible with
> > the original PPC driver.
> 
> I don't think that is necessary, as your grep has shown there are a
> lot of dts files using compatible = "foo", "usb-?hci"; and some may
> even have the dts in firmware, so we should simply make sure not to
> break such dts.

For these devices, we can't do much, but at least for the DT that are
in Linux, relying on the fact that there is no driver having a
compatible of "usb-ehci" to work looks very fragile. I'd be in favour
of removing it from the OMAP DTs, and every affected DTs.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140212/bac7bc59/attachment.sig>

ohci-/ehci-platform: Change compatible string to ?hci-platform

[Alan Stern]

On Tue, 11 Feb 2014, Hans de Goede wrote:

> Hi Greg,
> 
> Can you please add these 2 patches to usb-next, to unbreak usb on various
> ARM platforms?
> 
> These 2 patches can either be squashed into the first 2 patches of my previous
> set or added as is to preserve history, either way is fine with me.
> 
> The 2nd patch also fixes one of the Kconfig issues and I've a better plan
> for the 2nd Kconfig issue too, so please consider this a self-nack for my
> drivers/usb/host/Kconfig patches.
> 
> Here is a copy of the echi-platform commit-msg to explain the rationale of
> these changes, as well as how the breakage happened in the first place:
> 
> The initial versions of the devicetree enablement patches for ehci-platform
> used "ehci-platform" as compatible string. However this was disliked by various
> reviewers because the platform bus is a Linux invention and devicetree is
> supposed to be OS agnostic. After much discussion I gave up, added a:
> "depends on !PPC_OF" to Kconfig to avoid a known conflict with PPC-OF platforms
> and went with the generic usb-ehci as requested.
> 
> In retro-spect I should have stuck to my guns, because the dts files for many
> existing boards already claim to be compatible with "usb-ehci", ie they have:
> 
> 	compatible = "ti,ehci-omap", "usb-ehci";
> 
> In theory this should not be a problem since the "ti,ehci-omap" entry takes
> presedence, but in practice using a conflicting compatible string is an issue,
> because it makes which driver gets used depend on driver registration order.
> 
> This patch changes the compatible string claimed by ehci-platform (back) to
> "ehci-platform", avoiding the driver registration / module loading ordering
> problems, and removes the "depends on !PPC_OF" workaround.
> 
> Note that there already is a precedent for using ?hci-platform, in the form
> of xhci-platform.c using "xhci-platfrom" as compatible string.

I still think that "ehci-generic" would be better than "ehci-platform"  
(and the same for ohci).  The reason is that "-platform" is essentially
meaningless, whereas "-generic" strongly suggests that the hardware
matches the specification with no special features or requirements.  
Since the compatibility string -- like everything else in DT -- is
supposed to be a description of the hardware, this seems only logical.

It might even be a good idea to change the "xhci-platform" string to 
match, it that doesn't cause too much trouble.

Alan Stern

ohci-/ehci-platform: Change compatible string to ?hci-platform

[Arnd Bergmann]

On Tuesday 11 February 2014 10:27:12 Alan Stern wrote:
> 
> It might even be a good idea to change the "xhci-platform" string to 
> match, it that doesn't cause too much trouble.

The original xhci binding was contributed by Al Cooper, but I don't
see any dts files using it. I agree that xhci-generic is a better
name than xhci-platform, and I think we should make that the recommended
string. If Al or someone thinks the xhci-generic string might already
be used in production devices, we should however allow both names
in the binding and in the driver.

	Arnd

ohci-/ehci-platform: Change compatible string to ?hci-platform

[Hans de Goede]

Hi,

On 02/11/2014 04:43 PM, Arnd Bergmann wrote:
> On Tuesday 11 February 2014 10:27:12 Alan Stern wrote:
>>
>> It might even be a good idea to change the "xhci-platform" string to 
>> match, it that doesn't cause too much trouble.
> 
> The original xhci binding was contributed by Al Cooper, but I don't
> see any dts files using it. I agree that xhci-generic is a better
> name than xhci-platform, and I think we should make that the recommended
> string. If Al or someone thinks the xhci-generic string might already
> be used in production devices, we should however allow both names
> in the binding and in the driver.

I can live with generic as pre/post fix. Looking at what seems to
be the common trend I believe it should be a pre-fix and nost a post-fix
though.

The common way to build a compatible string seems to be:
vendor,soc_model-function. We have a few deviations from this in the
usb bindings, but ie the interrupt-controller bindings use this
structure for the compatible strings everywhere.

So since generic has no vendor, we fill just the soc_model-function
part leading to:
generic-ohci
generic-ehci

And possible also:
generic-uhci
generic-xhci

So I'm going to respin my 2 fixup patches to move from ?hci-platform
to generic-?hci.

Regards,

Hans

ohci-/ehci-platform: Change compatible string to ?hci-platform

[Florian Fainelli]

Le mardi 11 f?vrier 2014, 16:43:37 Arnd Bergmann a ?crit :
> On Tuesday 11 February 2014 10:27:12 Alan Stern wrote:
> > It might even be a good idea to change the "xhci-platform" string to
> > match, it that doesn't cause too much trouble.
> 
> The original xhci binding was contributed by Al Cooper, but I don't
> see any dts files using it. I agree that xhci-generic is a better
> name than xhci-platform, and I think we should make that the recommended
> string. If Al or someone thinks the xhci-generic string might already
> be used in production devices, we should however allow both names
> in the binding and in the driver.

The platform on which "xhci-platform" is currently used is Broadcom STB for 
which Marc submitted the base support recently. The BCM7445 reference DTS will 
probably be updated at some point to reflect that.
-- 
Florian

ohci-/ehci-platform: Change compatible string to ?hci-platform

[Hans de Goede]

Hi Florian,

On 02/11/2014 06:01 PM, Florian Fainelli wrote:
> Le mardi 11 f?vrier 2014, 16:43:37 Arnd Bergmann a ?crit :
>> On Tuesday 11 February 2014 10:27:12 Alan Stern wrote:
>>> It might even be a good idea to change the "xhci-platform" string to
>>> match, it that doesn't cause too much trouble.
>>
>> The original xhci binding was contributed by Al Cooper, but I don't
>> see any dts files using it. I agree that xhci-generic is a better
>> name than xhci-platform, and I think we should make that the recommended
>> string. If Al or someone thinks the xhci-generic string might already
>> be used in production devices, we should however allow both names
>> in the binding and in the driver.
> 
> The platform on which "xhci-platform" is currently used is Broadcom STB for 
> which Marc submitted the base support recently. The BCM7445 reference DTS will 
> probably be updated at some point to reflect that.

Note I've just send a patch with the xhci-platform -> generic-xhci change
Arnd requested. I've left xhci-platform in the list of compatible strings
for now (marked as deprecated in the docs), so there should be no issues
regardless of what the dts version getting merged ends up using.

Would be good if we could ensure it uses generic-xhci though, and then
drop the xhci-platform compatible all-together in a follow-up patch.

Regards,

Hans