diff mbox

[v3,5/9] ARM: dts: imx25.dtsi: Fix USB support.

Message ID 1394618518-13803-5-git-send-email-denis@eukrea.com (mailing list archive)
State New, archived
Headers show

Commit Message

Denis Carikli March 12, 2014, 10:01 a.m. UTC 
From: Fabio Estevam <fabio.estevam@freescale.com>

This patch was adapted from the thread named
"USB Host support for mx25" on linux-usb@vger.kernel.org

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
Changelog v2->v3:
- rebased on top of the "usb: chipidea: Use standard usb-phy property." patch.
- Fixed the usbphy nodes index and added and added a reg property.

Changelog v1->v2:
- The usbphy nodes were made to look like the ones in imx53.dtsi
- The patch was rebased on top of the clock fixes commits.
---
 arch/arm/boot/dts/imx25.dtsi |   29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

Comments

Fabio Estevam March 12, 2014, 11:08 a.m. UTC | #1 
Hi Denis,

On Wed, Mar 12, 2014 at 7:01 AM, Denis Carikli <denis@eukrea.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> This patch was adapted from the thread named
> "USB Host support for mx25" on linux-usb@vger.kernel.org
>

As you add me in the From field, you also need to add:

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> above your
Signed-off-by line.

> Signed-off-by: Denis Carikli <denis@eukrea.com>
> +
> +       usbphy {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               compatible = "simple-bus";

I made this comment earlier: why do we place usbphy0/1 under simple-bus?

This is not documented in the the bindings.

> +
> +               usbphy0: usbphy@0 {
> +                       reg = <0>;
> +                       compatible = "usb-nop-xceiv";
> +               };
> +
> +               usbphy1: usbphy@1 {
> +                       reg = <1>;
> +                       compatible = "usb-nop-xceiv";
> +               };
> +       };

Regards,

Fabio Estevam
Denis Carikli March 13, 2014, 9:18 a.m. UTC | #2 
On 03/12/2014 12:08 PM, Fabio Estevam wrote:
> Hi Denis,
Hi,

> As you add me in the From field, you also need to add:
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> above your
> Signed-off-by line.
Thanks.

>> +       usbphy {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               compatible = "simple-bus";
>
> I made this comment earlier: why do we place usbphy0/1 under simple-bus?

The official ePAPR 1.1 standard talks about the system on a chip's 
internal I/O bus.
So, I wonder if, in general, it makes sense to group together, with a 
simple-bus compatible, potentially different usb phy, which are 
connected to potentially different usb controllers.

Still if I remove it from the usbphy node, I get the following messages 
more than once in dmesg:
 > ci_hdrc ci_hdrc.0: no usb2 phy configured
 > platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
 > ci_hdrc ci_hdrc.1: no usb2 phy configured
 > platform ci_hdrc.1: Driver ci_hdrc requests probe deferral

With at the end lsusb printing nothing.

> This is not documented in the the bindings.
I don't think that the simple-bus has to be added to 
Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt because that 
file only talks about what became usbphy's subnodes.

Denis.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 829791e..02697b2 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -482,22 +482,13 @@ 
 				clocks = <&clks 99>;
 			};
 
-			usbphy1: usbphy@1 {
-				compatible = "nop-usbphy";
-				status = "disabled";
-			};
-
-			usbphy2: usbphy@2 {
-				compatible = "nop-usbphy";
-				status = "disabled";
-			};
-
 			usbotg: usb@53ff4000 {
 				compatible = "fsl,imx25-usb", "fsl,imx27-usb";
 				reg = <0x53ff4000 0x0200>;
 				interrupts = <37>;
 				clocks = <&clks 70>;
 				fsl,usbmisc = <&usbmisc 0>;
+				usb-phy = <&usbphy0>;
 				status = "disabled";
 			};
 
@@ -507,6 +498,7 @@ 
 				interrupts = <35>;
 				clocks = <&clks 70>;
 				fsl,usbmisc = <&usbmisc 1>;
+				usb-phy = <&usbphy1>;
 				status = "disabled";
 			};
 
@@ -516,7 +508,6 @@ 
 				clocks = <&clks 9>, <&clks 70>, <&clks 8>;
 				clock-names = "ipg", "ahb", "per";
 				reg = <0x53ff4600 0x00f>;
-				status = "disabled";
 			};
 
 			dryice@53ffc000 {
@@ -548,4 +539,20 @@ 
 			};
 		};
 	};
+
+	usbphy {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "simple-bus";
+
+		usbphy0: usbphy@0 {
+			reg = <0>;
+			compatible = "usb-nop-xceiv";
+		};
+
+		usbphy1: usbphy@1 {
+			reg = <1>;
+			compatible = "usb-nop-xceiv";
+		};
+	};
 };