diff mbox

[v3] ARM: zynq: DT: Add USB to device tree

Message ID 1417536431-27759-1-git-send-email-soren.brinkmann@xilinx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Soren Brinkmann Dec. 2, 2014, 4:07 p.m. UTC
Add USB nodes to zc702, zc706 and zed device trees.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
v3:
 - rename phy nodes: usb_phy -> phy0
 - rebased onto zynq/dt
v2:
 - remove '@0' from phy node name
 - don't add bogus space
---
 arch/arm/boot/dts/zynq-7000.dtsi | 20 ++++++++++++++++++++
 arch/arm/boot/dts/zynq-zc702.dts | 11 +++++++++++
 arch/arm/boot/dts/zynq-zc706.dts | 10 ++++++++++
 arch/arm/boot/dts/zynq-zed.dts   | 10 ++++++++++
 4 files changed, 51 insertions(+)

Comments

Michal Simek Dec. 3, 2014, 8:39 a.m. UTC | #1
On 12/02/2014 05:07 PM, Soren Brinkmann wrote:
> Add USB nodes to zc702, zc706 and zed device trees.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> v3:
>  - rename phy nodes: usb_phy -> phy0
>  - rebased onto zynq/dt
> v2:
>  - remove '@0' from phy node name
>  - don't add bogus space
> ---
>  arch/arm/boot/dts/zynq-7000.dtsi | 20 ++++++++++++++++++++
>  arch/arm/boot/dts/zynq-zc702.dts | 11 +++++++++++
>  arch/arm/boot/dts/zynq-zc706.dts | 10 ++++++++++
>  arch/arm/boot/dts/zynq-zed.dts   | 10 ++++++++++
>  4 files changed, 51 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index f8e4a28adfc0..21aae01a3fe9 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -314,5 +314,25 @@
>  			reg = <0xf8f00600 0x20>;
>  			clocks = <&clkc 4>;
>  		};
> +
> +		usb0: usb@e0002000 {
> +			compatible = "xlnx,zynq-usb-2.20a", "chipidea,usb2";
> +			status = "disabled";
> +			clocks = <&clkc 28>;
> +			interrupt-parent = <&intc>;
> +			interrupts = <0 21 4>;
> +			reg = <0xe0002000 0x1000>;
> +			phy_type = "ulpi";
> +		};
> +
> +		usb1: usb@e0003000 {
> +			compatible = "xlnx,zynq-usb-2.20a", "chipidea,usb2";
> +			status = "disabled";
> +			clocks = <&clkc 29>;
> +			interrupt-parent = <&intc>;
> +			interrupts = <0 44 4>;
> +			reg = <0xe0003000 0x1000>;
> +			phy_type = "ulpi";
> +		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
> index 280f02dd4ddc..399fed4d9c19 100644
> --- a/arch/arm/boot/dts/zynq-zc702.dts
> +++ b/arch/arm/boot/dts/zynq-zc702.dts
> @@ -36,6 +36,11 @@
>  			linux,default-trigger = "heartbeat";
>  		};
>  	};
> +
> +	usb_phy0: phy0 {
> +		compatible = "usb-nop-xceiv";
> +		#phy-cells = <0>;
> +	};
>  };
>  
>  &can0 {
> @@ -139,3 +144,9 @@
>  &uart1 {
>  	status = "okay";
>  };
> +
> +&usb0 {
> +	status = "okay";
> +	dr_mode = "host";
> +	usb-phy = <&usb_phy0>;
> +};
> diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
> index 34f7812d2ee8..89cc9adc569d 100644
> --- a/arch/arm/boot/dts/zynq-zc706.dts
> +++ b/arch/arm/boot/dts/zynq-zc706.dts
> @@ -27,6 +27,10 @@
>  		bootargs = "console=ttyPS0,115200 earlyprintk";
>  	};
>  
> +	usb_phy0: phy0 {
> +		compatible = "usb-nop-xceiv";
> +		#phy-cells = <0>;
> +	};
>  };
>  
>  &clkc {
> @@ -118,3 +122,9 @@
>  &uart1 {
>  	status = "okay";
>  };
> +
> +&usb0 {
> +	status = "okay";
> +	dr_mode = "host";
> +	usb-phy = <&usb_phy0>;
> +};
> diff --git a/arch/arm/boot/dts/zynq-zed.dts b/arch/arm/boot/dts/zynq-zed.dts
> index 1c7cc990b47a..e20956e5e720 100644
> --- a/arch/arm/boot/dts/zynq-zed.dts
> +++ b/arch/arm/boot/dts/zynq-zed.dts
> @@ -27,6 +27,10 @@
>  		bootargs = "console=ttyPS0,115200 earlyprintk";
>  	};
>  
> +	usb_phy0: phy0 {
> +		compatible = "usb-nop-xceiv";
> +		#phy-cells = <0>;
> +	};
>  };
>  
>  &clkc {
> @@ -50,3 +54,9 @@
>  &uart1 {
>  	status = "okay";
>  };
> +
> +&usb0 {
> +	status = "okay";
> +	dr_mode = "host";
> +	usb-phy = <&usb_phy0>;
> +};
> 

Applied to zynq/dt.

Thanks,
Michal
Andreas Färber Jan. 26, 2015, 8:19 a.m. UTC | #2
Hi Michal,

Am 03.12.2014 um 09:39 schrieb Michal Simek:
> On 12/02/2014 05:07 PM, Soren Brinkmann wrote:
>> Add USB nodes to zc702, zc706 and zed device trees.
>>
>> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
>> ---
>> v3:
>>  - rename phy nodes: usb_phy -> phy0
>>  - rebased onto zynq/dt
>> v2:
>>  - remove '@0' from phy node name
>>  - don't add bogus space
>> ---
>>  arch/arm/boot/dts/zynq-7000.dtsi | 20 ++++++++++++++++++++
>>  arch/arm/boot/dts/zynq-zc702.dts | 11 +++++++++++
>>  arch/arm/boot/dts/zynq-zc706.dts | 10 ++++++++++
>>  arch/arm/boot/dts/zynq-zed.dts   | 10 ++++++++++
>>  4 files changed, 51 insertions(+)
[...]
> 
> Applied to zynq/dt.

Hm, I don't see this patch in linux-next next-20150123...

And if I apply it to my -next based tree, adding corresponding nodes to
zynq-parallella.dts, I get repeatedly:

[  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
[  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
f090e100 op: f090e140
[  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
[  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
[  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
f0910100 op: f0910140
[  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral

Am I missing any other patches or config options?
(I do notice that the pinctrl v3 patch that got merged has a trivial bug
for usb0, for which I'll send a patch later on.)

Regards,
Andreas
Michal Simek Jan. 26, 2015, 8:23 a.m. UTC | #3
On 01/26/2015 09:19 AM, Andreas Färber wrote:
> Hi Michal,
> 
> Am 03.12.2014 um 09:39 schrieb Michal Simek:
>> On 12/02/2014 05:07 PM, Soren Brinkmann wrote:
>>> Add USB nodes to zc702, zc706 and zed device trees.
>>>
>>> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
>>> ---
>>> v3:
>>>  - rename phy nodes: usb_phy -> phy0
>>>  - rebased onto zynq/dt
>>> v2:
>>>  - remove '@0' from phy node name
>>>  - don't add bogus space
>>> ---
>>>  arch/arm/boot/dts/zynq-7000.dtsi | 20 ++++++++++++++++++++
>>>  arch/arm/boot/dts/zynq-zc702.dts | 11 +++++++++++
>>>  arch/arm/boot/dts/zynq-zc706.dts | 10 ++++++++++
>>>  arch/arm/boot/dts/zynq-zed.dts   | 10 ++++++++++
>>>  4 files changed, 51 insertions(+)
> [...]
>>
>> Applied to zynq/dt.
> 
> Hm, I don't see this patch in linux-next next-20150123...
> 
> And if I apply it to my -next based tree, adding corresponding nodes to
> zynq-parallella.dts, I get repeatedly:
> 
> [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
> [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
> f090e100 op: f090e140
> [  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
> [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
> [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
> f0910100 op: f0910140
> [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
> 
> Am I missing any other patches or config options?
> (I do notice that the pinctrl v3 patch that got merged has a trivial bug
> for usb0, for which I'll send a patch later on.)

Why is it deferred? Is it because of pinmuxing stuff?

Thanks,
Michal
Andreas Färber Jan. 26, 2015, 8:33 a.m. UTC | #4
Am 26.01.2015 um 09:23 schrieb Michal Simek:
> On 01/26/2015 09:19 AM, Andreas Färber wrote:
>> Am 03.12.2014 um 09:39 schrieb Michal Simek:
>>> On 12/02/2014 05:07 PM, Soren Brinkmann wrote:
>>>> Add USB nodes to zc702, zc706 and zed device trees.
>>>>
>>>> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
>>>> ---
>>>> v3:
>>>>  - rename phy nodes: usb_phy -> phy0
>>>>  - rebased onto zynq/dt
>>>> v2:
>>>>  - remove '@0' from phy node name
>>>>  - don't add bogus space
>>>> ---
>>>>  arch/arm/boot/dts/zynq-7000.dtsi | 20 ++++++++++++++++++++
>>>>  arch/arm/boot/dts/zynq-zc702.dts | 11 +++++++++++
>>>>  arch/arm/boot/dts/zynq-zc706.dts | 10 ++++++++++
>>>>  arch/arm/boot/dts/zynq-zed.dts   | 10 ++++++++++
>>>>  4 files changed, 51 insertions(+)
>> [...]
>>>
>>> Applied to zynq/dt.
>>
>> Hm, I don't see this patch in linux-next next-20150123...
>>
>> And if I apply it to my -next based tree, adding corresponding nodes to
>> zynq-parallella.dts, I get repeatedly:
>>
>> [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
>> [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
>> f090e100 op: f090e140
>> [  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
>> [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
>> [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
>> f0910100 op: f0910140
>> [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
>>
>> Am I missing any other patches or config options?
>> (I do notice that the pinctrl v3 patch that got merged has a trivial bug
>> for usb0, for which I'll send a patch later on.)
> 
> Why is it deferred? Is it because of pinmuxing stuff?

No, happened without as well.

Looking at a different place in dmesg, I spot this:

[  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
[  +0,000012] usb_phy_generic phy0: using device tree for GPIO lookup
[  +0,000015] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
property
 of node '/phy0[0]'
[  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
property
of node '/phy0[0]'
[  +0,000010] usb_phy_generic phy0: using lookup tables for GPIO lookup
[  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
[  +0,000012] usb_phy_generic phy0: Error requesting RESET GPIO
[  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
[  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
[  +0,000012] usb_phy_generic phy1: using device tree for GPIO lookup
[  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
property
 of node '/phy1[0]'
[  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
property of node '/phy1[0]'
[  +0,000010] usb_phy_generic phy1: using lookup tables for GPIO lookup
[  +0,000012] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
[  +0,000011] usb_phy_generic phy1: Error requesting RESET GPIO
[  +0,004337] usb_phy_generic: probe of phy1 failed with error -2

So, I guess the chipidea driver is deferring because the phys want a
property that neither me nor you are specifying? Would that be the two
MDIO pins 52 and 53 that would need to be specified?

Regards,
Andreas
Andreas Färber Jan. 26, 2015, 9:35 a.m. UTC | #5
Am 26.01.2015 um 09:33 schrieb Andreas Färber:
> Am 26.01.2015 um 09:23 schrieb Michal Simek:
>> On 01/26/2015 09:19 AM, Andreas Färber wrote:
>>> And if I apply it to my -next based tree, adding corresponding nodes to
>>> zynq-parallella.dts, I get repeatedly:
>>>
>>> [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
>>> [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
>>> f090e100 op: f090e140
>>> [  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
>>> [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
>>> [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
>>> f0910100 op: f0910140
>>> [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
>>>
>>> Am I missing any other patches or config options?
>>> (I do notice that the pinctrl v3 patch that got merged has a trivial bug
>>> for usb0, for which I'll send a patch later on.)
>>
>> Why is it deferred? Is it because of pinmuxing stuff?
> 
> No, happened without as well.
> 
> Looking at a different place in dmesg, I spot this:
> 
> [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
> [  +0,000012] usb_phy_generic phy0: using device tree for GPIO lookup
> [  +0,000015] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
> property
>  of node '/phy0[0]'
> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
> property
> of node '/phy0[0]'
> [  +0,000010] usb_phy_generic phy0: using lookup tables for GPIO lookup
> [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
> [  +0,000012] usb_phy_generic phy0: Error requesting RESET GPIO
> [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
> [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
> [  +0,000012] usb_phy_generic phy1: using device tree for GPIO lookup
> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
> property
>  of node '/phy1[0]'
> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
> property of node '/phy1[0]'
> [  +0,000010] usb_phy_generic phy1: using lookup tables for GPIO lookup
> [  +0,000012] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
> [  +0,000011] usb_phy_generic phy1: Error requesting RESET GPIO
> [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
> 
> So, I guess the chipidea driver is deferring because the phys want a
> property that neither me nor you are specifying? Would that be the two
> MDIO pins 52 and 53 that would need to be specified?

Erm, scratch that last question - wrong PHY. Trying it resolved the
above phy errors but not the original problem. And so does an empty one:

@@ -99,11 +100,13 @@

        usb_phy0: phy0 {
                compatible = "usb-nop-xceiv";
+               reset-gpios = <>;
                #phy-cells = <0>;
        };

        usb_phy1: phy1 {
                compatible = "usb-nop-xceiv";
+               reset-gpios = <>;
                #phy-cells = <0>;
        };
 };

In my manuals and notes I can't find any GPIO being used as reset for
the USB PHYs. Any thoughts appreciated.

Regards,
Andreas
Soren Brinkmann Jan. 26, 2015, 3:50 p.m. UTC | #6
On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
> Am 26.01.2015 um 09:33 schrieb Andreas Färber:
> > Am 26.01.2015 um 09:23 schrieb Michal Simek:
> >> On 01/26/2015 09:19 AM, Andreas Färber wrote:
> >>> And if I apply it to my -next based tree, adding corresponding nodes to
> >>> zynq-parallella.dts, I get repeatedly:
> >>>
> >>> [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
> >>> [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
> >>> f090e100 op: f090e140
> >>> [  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
> >>> [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
> >>> [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
> >>> f0910100 op: f0910140
> >>> [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
> >>>
> >>> Am I missing any other patches or config options?
> >>> (I do notice that the pinctrl v3 patch that got merged has a trivial bug
> >>> for usb0, for which I'll send a patch later on.)
> >>
> >> Why is it deferred? Is it because of pinmuxing stuff?
> > 
> > No, happened without as well.
> > 
> > Looking at a different place in dmesg, I spot this:
> > 
> > [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
> > [  +0,000012] usb_phy_generic phy0: using device tree for GPIO lookup
> > [  +0,000015] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
> > property
> >  of node '/phy0[0]'
> > [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
> > property
> > of node '/phy0[0]'
> > [  +0,000010] usb_phy_generic phy0: using lookup tables for GPIO lookup
> > [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
> > [  +0,000012] usb_phy_generic phy0: Error requesting RESET GPIO
> > [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
> > [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
> > [  +0,000012] usb_phy_generic phy1: using device tree for GPIO lookup
> > [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
> > property
> >  of node '/phy1[0]'
> > [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
> > property of node '/phy1[0]'
> > [  +0,000010] usb_phy_generic phy1: using lookup tables for GPIO lookup
> > [  +0,000012] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
> > [  +0,000011] usb_phy_generic phy1: Error requesting RESET GPIO
> > [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
> > 
> > So, I guess the chipidea driver is deferring because the phys want a
> > property that neither me nor you are specifying? Would that be the two
> > MDIO pins 52 and 53 that would need to be specified?
> 
> Erm, scratch that last question - wrong PHY. Trying it resolved the
> above phy errors but not the original problem. And so does an empty one:
> 
> @@ -99,11 +100,13 @@
> 
>         usb_phy0: phy0 {
>                 compatible = "usb-nop-xceiv";
> +               reset-gpios = <>;
>                 #phy-cells = <0>;
>         };
> 
>         usb_phy1: phy1 {
>                 compatible = "usb-nop-xceiv";
> +               reset-gpios = <>;
>                 #phy-cells = <0>;
>         };
>  };
> 
> In my manuals and notes I can't find any GPIO being used as reset for
> the USB PHYs. Any thoughts appreciated.

Such a connection is optional. The platform might rely on its reset
circuit, though it might not work for warm reboots.
I haven't looked at parallela docs, but if there is a schematic
available, that should tell you if/what is connected to the PHY reset
pin.

	Soren
Andreas Färber Jan. 26, 2015, 4:21 p.m. UTC | #7
Am 26.01.2015 um 16:50 schrieb Sören Brinkmann:
> On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
>> Am 26.01.2015 um 09:33 schrieb Andreas Färber:
>>> Am 26.01.2015 um 09:23 schrieb Michal Simek:
>>>> On 01/26/2015 09:19 AM, Andreas Färber wrote:
>>>>> And if I apply it to my -next based tree, adding corresponding nodes to
>>>>> zynq-parallella.dts, I get repeatedly:
>>>>>
>>>>> [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
>>>>> [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
>>>>> f090e100 op: f090e140
>>>>> [  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
>>>>> [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
>>>>> [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
>>>>> f0910100 op: f0910140
>>>>> [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
>>>>>
>>>>> Am I missing any other patches or config options?
>>>>> (I do notice that the pinctrl v3 patch that got merged has a trivial bug
>>>>> for usb0, for which I'll send a patch later on.)
>>>>
>>>> Why is it deferred? Is it because of pinmuxing stuff?
>>>
>>> No, happened without as well.
>>>
>>> Looking at a different place in dmesg, I spot this:
>>>
>>> [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
>>> [  +0,000012] usb_phy_generic phy0: using device tree for GPIO lookup
>>> [  +0,000015] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
>>> property
>>>  of node '/phy0[0]'
>>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
>>> property
>>> of node '/phy0[0]'
>>> [  +0,000010] usb_phy_generic phy0: using lookup tables for GPIO lookup
>>> [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
>>> [  +0,000012] usb_phy_generic phy0: Error requesting RESET GPIO
>>> [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
>>> [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
>>> [  +0,000012] usb_phy_generic phy1: using device tree for GPIO lookup
>>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
>>> property
>>>  of node '/phy1[0]'
>>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
>>> property of node '/phy1[0]'
>>> [  +0,000010] usb_phy_generic phy1: using lookup tables for GPIO lookup
>>> [  +0,000012] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
>>> [  +0,000011] usb_phy_generic phy1: Error requesting RESET GPIO
>>> [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
>>>
>>> So, I guess the chipidea driver is deferring because the phys want a
>>> property that neither me nor you are specifying? Would that be the two
>>> MDIO pins 52 and 53 that would need to be specified?
>>
>> Erm, scratch that last question - wrong PHY. Trying it resolved the
>> above phy errors but not the original problem. And so does an empty one:
>>
>> @@ -99,11 +100,13 @@
>>
>>         usb_phy0: phy0 {
>>                 compatible = "usb-nop-xceiv";
>> +               reset-gpios = <>;
>>                 #phy-cells = <0>;
>>         };
>>
>>         usb_phy1: phy1 {
>>                 compatible = "usb-nop-xceiv";
>> +               reset-gpios = <>;
>>                 #phy-cells = <0>;
>>         };
>>  };
>>
>> In my manuals and notes I can't find any GPIO being used as reset for
>> the USB PHYs. Any thoughts appreciated.
> 
> Such a connection is optional. The platform might rely on its reset
> circuit, though it might not work for warm reboots.
> I haven't looked at parallela docs, but if there is a schematic
> available, that should tell you if/what is connected to the PHY reset
> pin.

I do have the schematic, and the way I read it, only the on-board reset
button resets the PHYs.

Yet it looks as if usb-nop-xceiv insists on a reset-gpios above, no?
Does it work on your boards with linux-next?

And regarding my previous reporting, I was mistaken: I had to increase
the log buffer size due to enabled pinctrl debugging, which made the phy
errors sometimes appear, depending on timing. They're still there.

Andreas
Soren Brinkmann Jan. 26, 2015, 4:23 p.m. UTC | #8
On Mon, 2015-01-26 at 05:21PM +0100, Andreas Färber wrote:
> Am 26.01.2015 um 16:50 schrieb Sören Brinkmann:
> > On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
> >> Am 26.01.2015 um 09:33 schrieb Andreas Färber:
> >>> Am 26.01.2015 um 09:23 schrieb Michal Simek:
> >>>> On 01/26/2015 09:19 AM, Andreas Färber wrote:
> >>>>> And if I apply it to my -next based tree, adding corresponding nodes to
> >>>>> zynq-parallella.dts, I get repeatedly:
> >>>>>
> >>>>> [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
> >>>>> [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
> >>>>> f090e100 op: f090e140
> >>>>> [  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
> >>>>> [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
> >>>>> [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
> >>>>> f0910100 op: f0910140
> >>>>> [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
> >>>>>
> >>>>> Am I missing any other patches or config options?
> >>>>> (I do notice that the pinctrl v3 patch that got merged has a trivial bug
> >>>>> for usb0, for which I'll send a patch later on.)
> >>>>
> >>>> Why is it deferred? Is it because of pinmuxing stuff?
> >>>
> >>> No, happened without as well.
> >>>
> >>> Looking at a different place in dmesg, I spot this:
> >>>
> >>> [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
> >>> [  +0,000012] usb_phy_generic phy0: using device tree for GPIO lookup
> >>> [  +0,000015] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
> >>> property
> >>>  of node '/phy0[0]'
> >>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
> >>> property
> >>> of node '/phy0[0]'
> >>> [  +0,000010] usb_phy_generic phy0: using lookup tables for GPIO lookup
> >>> [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
> >>> [  +0,000012] usb_phy_generic phy0: Error requesting RESET GPIO
> >>> [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
> >>> [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
> >>> [  +0,000012] usb_phy_generic phy1: using device tree for GPIO lookup
> >>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
> >>> property
> >>>  of node '/phy1[0]'
> >>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
> >>> property of node '/phy1[0]'
> >>> [  +0,000010] usb_phy_generic phy1: using lookup tables for GPIO lookup
> >>> [  +0,000012] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
> >>> [  +0,000011] usb_phy_generic phy1: Error requesting RESET GPIO
> >>> [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
> >>>
> >>> So, I guess the chipidea driver is deferring because the phys want a
> >>> property that neither me nor you are specifying? Would that be the two
> >>> MDIO pins 52 and 53 that would need to be specified?
> >>
> >> Erm, scratch that last question - wrong PHY. Trying it resolved the
> >> above phy errors but not the original problem. And so does an empty one:
> >>
> >> @@ -99,11 +100,13 @@
> >>
> >>         usb_phy0: phy0 {
> >>                 compatible = "usb-nop-xceiv";
> >> +               reset-gpios = <>;
> >>                 #phy-cells = <0>;
> >>         };
> >>
> >>         usb_phy1: phy1 {
> >>                 compatible = "usb-nop-xceiv";
> >> +               reset-gpios = <>;
> >>                 #phy-cells = <0>;
> >>         };
> >>  };
> >>
> >> In my manuals and notes I can't find any GPIO being used as reset for
> >> the USB PHYs. Any thoughts appreciated.
> > 
> > Such a connection is optional. The platform might rely on its reset
> > circuit, though it might not work for warm reboots.
> > I haven't looked at parallela docs, but if there is a schematic
> > available, that should tell you if/what is connected to the PHY reset
> > pin.
> 
> I do have the schematic, and the way I read it, only the on-board reset
> button resets the PHYs.
> 
> Yet it looks as if usb-nop-xceiv insists on a reset-gpios above, no?
> Does it work on your boards with linux-next?

I haven't re-tested it since I submitted the patches, but at that time
it worked. But I also didn't test USB with the pinctrl patches together.
I'll do some testing later today.

	Soren
Soren Brinkmann Jan. 26, 2015, 6:44 p.m. UTC | #9
On Mon, 2015-01-26 at 08:23AM -0800, Sören Brinkmann wrote:
> On Mon, 2015-01-26 at 05:21PM +0100, Andreas Färber wrote:
> > Am 26.01.2015 um 16:50 schrieb Sören Brinkmann:
> > > On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
> > >> Am 26.01.2015 um 09:33 schrieb Andreas Färber:
> > >>> Am 26.01.2015 um 09:23 schrieb Michal Simek:
> > >>>> On 01/26/2015 09:19 AM, Andreas Färber wrote:
> > >>>>> And if I apply it to my -next based tree, adding corresponding nodes to
> > >>>>> zynq-parallella.dts, I get repeatedly:
> > >>>>>
> > >>>>> [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
> > >>>>> [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
> > >>>>> f090e100 op: f090e140
> > >>>>> [  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
> > >>>>> [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
> > >>>>> [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
> > >>>>> f0910100 op: f0910140
> > >>>>> [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
> > >>>>>
> > >>>>> Am I missing any other patches or config options?
> > >>>>> (I do notice that the pinctrl v3 patch that got merged has a trivial bug
> > >>>>> for usb0, for which I'll send a patch later on.)
> > >>>>
> > >>>> Why is it deferred? Is it because of pinmuxing stuff?
> > >>>
> > >>> No, happened without as well.
> > >>>
> > >>> Looking at a different place in dmesg, I spot this:
> > >>>
> > >>> [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
> > >>> [  +0,000012] usb_phy_generic phy0: using device tree for GPIO lookup
> > >>> [  +0,000015] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
> > >>> property
> > >>>  of node '/phy0[0]'
> > >>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
> > >>> property
> > >>> of node '/phy0[0]'
> > >>> [  +0,000010] usb_phy_generic phy0: using lookup tables for GPIO lookup
> > >>> [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
> > >>> [  +0,000012] usb_phy_generic phy0: Error requesting RESET GPIO
> > >>> [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
> > >>> [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
> > >>> [  +0,000012] usb_phy_generic phy1: using device tree for GPIO lookup
> > >>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
> > >>> property
> > >>>  of node '/phy1[0]'
> > >>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
> > >>> property of node '/phy1[0]'
> > >>> [  +0,000010] usb_phy_generic phy1: using lookup tables for GPIO lookup
> > >>> [  +0,000012] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
> > >>> [  +0,000011] usb_phy_generic phy1: Error requesting RESET GPIO
> > >>> [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
> > >>>
> > >>> So, I guess the chipidea driver is deferring because the phys want a
> > >>> property that neither me nor you are specifying? Would that be the two
> > >>> MDIO pins 52 and 53 that would need to be specified?
> > >>
> > >> Erm, scratch that last question - wrong PHY. Trying it resolved the
> > >> above phy errors but not the original problem. And so does an empty one:
> > >>
> > >> @@ -99,11 +100,13 @@
> > >>
> > >>         usb_phy0: phy0 {
> > >>                 compatible = "usb-nop-xceiv";
> > >> +               reset-gpios = <>;
> > >>                 #phy-cells = <0>;
> > >>         };
> > >>
> > >>         usb_phy1: phy1 {
> > >>                 compatible = "usb-nop-xceiv";
> > >> +               reset-gpios = <>;
> > >>                 #phy-cells = <0>;
> > >>         };
> > >>  };
> > >>
> > >> In my manuals and notes I can't find any GPIO being used as reset for
> > >> the USB PHYs. Any thoughts appreciated.
> > > 
> > > Such a connection is optional. The platform might rely on its reset
> > > circuit, though it might not work for warm reboots.
> > > I haven't looked at parallela docs, but if there is a schematic
> > > available, that should tell you if/what is connected to the PHY reset
> > > pin.
> > 
> > I do have the schematic, and the way I read it, only the on-board reset
> > button resets the PHYs.
> > 
> > Yet it looks as if usb-nop-xceiv insists on a reset-gpios above, no?
> > Does it work on your boards with linux-next?
> 
> I haven't re-tested it since I submitted the patches, but at that time
> it worked. But I also didn't test USB with the pinctrl patches together.
> I'll do some testing later today.

So, just did a test. I took all the pinctrl stuff and this patch and ran
it on a zc702. I plugged in a thumb drive and that worked just fine. So,
basically this patch could go in, despite missing pinctrl properties.

Michal: How do you wanna handle this? Could you create a branch that has
all the DT updates I can base stuff on, please? We could either take the
pinctrl patches and this patch and add another one adding the pinctrl
properties to the USB nodes. Or leave this patch out for now and revise
it on top of the pinctrl patches.

	Thanks,
	Sören
diff mbox

Patch

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index f8e4a28adfc0..21aae01a3fe9 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -314,5 +314,25 @@ 
 			reg = <0xf8f00600 0x20>;
 			clocks = <&clkc 4>;
 		};
+
+		usb0: usb@e0002000 {
+			compatible = "xlnx,zynq-usb-2.20a", "chipidea,usb2";
+			status = "disabled";
+			clocks = <&clkc 28>;
+			interrupt-parent = <&intc>;
+			interrupts = <0 21 4>;
+			reg = <0xe0002000 0x1000>;
+			phy_type = "ulpi";
+		};
+
+		usb1: usb@e0003000 {
+			compatible = "xlnx,zynq-usb-2.20a", "chipidea,usb2";
+			status = "disabled";
+			clocks = <&clkc 29>;
+			interrupt-parent = <&intc>;
+			interrupts = <0 44 4>;
+			reg = <0xe0003000 0x1000>;
+			phy_type = "ulpi";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
index 280f02dd4ddc..399fed4d9c19 100644
--- a/arch/arm/boot/dts/zynq-zc702.dts
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -36,6 +36,11 @@ 
 			linux,default-trigger = "heartbeat";
 		};
 	};
+
+	usb_phy0: phy0 {
+		compatible = "usb-nop-xceiv";
+		#phy-cells = <0>;
+	};
 };
 
 &can0 {
@@ -139,3 +144,9 @@ 
 &uart1 {
 	status = "okay";
 };
+
+&usb0 {
+	status = "okay";
+	dr_mode = "host";
+	usb-phy = <&usb_phy0>;
+};
diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
index 34f7812d2ee8..89cc9adc569d 100644
--- a/arch/arm/boot/dts/zynq-zc706.dts
+++ b/arch/arm/boot/dts/zynq-zc706.dts
@@ -27,6 +27,10 @@ 
 		bootargs = "console=ttyPS0,115200 earlyprintk";
 	};
 
+	usb_phy0: phy0 {
+		compatible = "usb-nop-xceiv";
+		#phy-cells = <0>;
+	};
 };
 
 &clkc {
@@ -118,3 +122,9 @@ 
 &uart1 {
 	status = "okay";
 };
+
+&usb0 {
+	status = "okay";
+	dr_mode = "host";
+	usb-phy = <&usb_phy0>;
+};
diff --git a/arch/arm/boot/dts/zynq-zed.dts b/arch/arm/boot/dts/zynq-zed.dts
index 1c7cc990b47a..e20956e5e720 100644
--- a/arch/arm/boot/dts/zynq-zed.dts
+++ b/arch/arm/boot/dts/zynq-zed.dts
@@ -27,6 +27,10 @@ 
 		bootargs = "console=ttyPS0,115200 earlyprintk";
 	};
 
+	usb_phy0: phy0 {
+		compatible = "usb-nop-xceiv";
+		#phy-cells = <0>;
+	};
 };
 
 &clkc {
@@ -50,3 +54,9 @@ 
 &uart1 {
 	status = "okay";
 };
+
+&usb0 {
+	status = "okay";
+	dr_mode = "host";
+	usb-phy = <&usb_phy0>;
+};