Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1534622 > unrolled thread

[PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399

Started byBrian Norris <briannorris@chromium.org>
First post2016-12-02 03:40 +0100
Last post2016-12-07 20:10 +0100
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399 Brian Norris <briannorris@chromium.org> - 2016-12-02 03:40 +0100
    Re: [PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399 Heiko Stuebner <heiko@sntech.de> - 2016-12-07 18:10 +0100
      Re: [PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399 Brian Norris <briannorris@chromium.org> - 2016-12-07 19:00 +0100
        Re: [PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399 Brian Norris <briannorris@chromium.org> - 2016-12-07 20:10 +0100
          Re: [PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399 Heiko Stuebner <heiko@sntech.de> - 2016-12-07 20:10 +0100
        Re: [PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399 Heiko Stuebner <heiko@sntech.de> - 2016-12-07 20:10 +0100

#1534622 — [PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399

FromBrian Norris <briannorris@chromium.org>
Date2016-12-02 03:40 +0100
Subject[PATCH 4/9] arm64: dts: rockchip: support dwc3 USB for rk3399
Message-ID<sJLVv-5Ru-1@gated-at.bofh.it>
Add the dwc3 usb needed node information for rk3399.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
Somewhat rewritten from Caesar's reposting (v2) of my patch.

Changes:
 * Include USB2 PHY (which is now in -next)
 * Don't include USB3 PHY, as extcon support is not ready yet
 * Drop non-upstream properties
 * Fixup whitespace a bit
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 60 ++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 4ca8f9a7601c..1e97fb8c6415 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -316,6 +316,66 @@
 		};
 	};
 
+	usbdrd3_0: usb@fe800000 {
+		compatible = "rockchip,rk3399-dwc3";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
+			 <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
+			 <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;
+		clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
+			      "aclk_usb3otg0", "aclk_usb3_rksoc_axi_perf",
+			      "aclk_usb3", "aclk_usb3_grf";
+		resets = <&cru SRST_A_USB3_OTG0>;
+		reset-names = "usb3-otg";
+		status = "disabled";
+
+		usbdrd_dwc3_0: dwc3 {
+			compatible = "snps,dwc3";
+			reg = <0x0 0xfe800000 0x0 0x100000>;
+			interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
+			dr_mode = "otg";
+			phys = <&u2phy0_otg>;
+			phy-names = "usb2-phy";
+			snps,dis_enblslpm_quirk;
+			snps,dis-u2-freeclk-exists-quirk;
+			snps,dis_u2_susphy_quirk;
+			snps,dis-del-phy-power-chg-quirk;
+			status = "disabled";
+		};
+	};
+
+	usbdrd3_1: usb@fe900000 {
+		compatible = "rockchip,rk3399-dwc3";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		clocks = <&cru SCLK_USB3OTG1_REF>, <&cru SCLK_USB3OTG1_SUSPEND>,
+			 <&cru ACLK_USB3OTG1>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
+			 <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;
+		clock-names = "clk_usb3otg1_ref", "clk_usb3otg1_suspend",
+			      "aclk_usb3otg1", "aclk_usb3_rksoc_axi_perf",
+			      "aclk_usb3", "aclk_usb3_grf";
+		resets = <&cru SRST_A_USB3_OTG1>;
+		reset-names = "usb3-otg";
+		status = "disabled";
+
+		usbdrd_dwc3_1: dwc3 {
+			compatible = "snps,dwc3";
+			reg = <0x0 0xfe900000 0x0 0x100000>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
+			dr_mode = "otg";
+			phys = <&u2phy1_otg>;
+			phy-names = "usb2-phy";
+			snps,dis_enblslpm_quirk;
+			snps,dis-u2-freeclk-exists-quirk;
+			snps,dis_u2_susphy_quirk;
+			snps,dis-del-phy-power-chg-quirk;
+			status = "disabled";
+		};
+	};
+
 	usb_host0_ehci: usb@fe380000 {
 		compatible = "generic-ehci";
 		reg = <0x0 0xfe380000 0x0 0x20000>;
-- 
2.8.0.rc3.226.g39d4020

[toc] | [next] | [standalone]

#1537938

FromHeiko Stuebner <heiko@sntech.de>
Date2016-12-07 18:10 +0100
Message-ID<sLNTb-2UC-23@gated-at.bofh.it>
In reply to#1534622
Hi Brian,

Am Donnerstag, 1. Dezember 2016, 18:27:28 CET schrieb Brian Norris:
> Add the dwc3 usb needed node information for rk3399.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> Somewhat rewritten from Caesar's reposting (v2) of my patch.
> 
> Changes:
>  * Include USB2 PHY (which is now in -next)
>  * Don't include USB3 PHY, as extcon support is not ready yet
>  * Drop non-upstream properties
>  * Fixup whitespace a bit
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 60
> ++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index 4ca8f9a7601c..1e97fb8c6415
> 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -316,6 +316,66 @@
>  		};
>  	};
> 
> +	usbdrd3_0: usb@fe800000 {

insert location above usb@fe380000 is sorted wrong

> +		compatible = "rockchip,rk3399-dwc3";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +		clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
> +			 <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
> +			 <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;
> +		clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
> +			      "aclk_usb3otg0", "aclk_usb3_rksoc_axi_perf",
> +			      "aclk_usb3", "aclk_usb3_grf";

clock-names do not match binding. The dwc3-of-simple does not care, as it just 
enables all of them it seems, but binding doc states the clock names as

- clock-names:  Should contain the following:
  "ref_clk"     Controller reference clk, have to be 24 MHz
  "suspend_clk" Controller suspend clk, have to be 24 MHz or 32 KHz
  "bus_clk"     Master/Core clock, have to be >= 62.5 MHz for SS
                operation and >= 30MHz for HS operation
  "grf_clk"     Controller grf clk

> +		resets = <&cru SRST_A_USB3_OTG0>;
> +		reset-names = "usb3-otg";

you could update the binding documentation to list this one.


Heiko

> +		status = "disabled";
> +
> +		usbdrd_dwc3_0: dwc3 {
> +			compatible = "snps,dwc3";
> +			reg = <0x0 0xfe800000 0x0 0x100000>;
> +			interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> +			dr_mode = "otg";
> +			phys = <&u2phy0_otg>;
> +			phy-names = "usb2-phy";
> +			snps,dis_enblslpm_quirk;
> +			snps,dis-u2-freeclk-exists-quirk;
> +			snps,dis_u2_susphy_quirk;
> +			snps,dis-del-phy-power-chg-quirk;
> +			status = "disabled";
> +		};
> +	};
> +
> +	usbdrd3_1: usb@fe900000 {
> +		compatible = "rockchip,rk3399-dwc3";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +		clocks = <&cru SCLK_USB3OTG1_REF>, <&cru SCLK_USB3OTG1_SUSPEND>,
> +			 <&cru ACLK_USB3OTG1>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
> +			 <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;
> +		clock-names = "clk_usb3otg1_ref", "clk_usb3otg1_suspend",
> +			      "aclk_usb3otg1", "aclk_usb3_rksoc_axi_perf",
> +			      "aclk_usb3", "aclk_usb3_grf";
> +		resets = <&cru SRST_A_USB3_OTG1>;
> +		reset-names = "usb3-otg";
> +		status = "disabled";
> +
> +		usbdrd_dwc3_1: dwc3 {
> +			compatible = "snps,dwc3";
> +			reg = <0x0 0xfe900000 0x0 0x100000>;
> +			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
> +			dr_mode = "otg";
> +			phys = <&u2phy1_otg>;
> +			phy-names = "usb2-phy";
> +			snps,dis_enblslpm_quirk;
> +			snps,dis-u2-freeclk-exists-quirk;
> +			snps,dis_u2_susphy_quirk;
> +			snps,dis-del-phy-power-chg-quirk;
> +			status = "disabled";
> +		};
> +	};
> +
>  	usb_host0_ehci: usb@fe380000 {
>  		compatible = "generic-ehci";
>  		reg = <0x0 0xfe380000 0x0 0x20000>;

[toc] | [prev] | [next] | [standalone]

#1537961

FromBrian Norris <briannorris@chromium.org>
Date2016-12-07 19:00 +0100
Message-ID<sLOFA-3bg-7@gated-at.bofh.it>
In reply to#1537938
Hi,

On Wed, Dec 07, 2016 at 06:09:16PM +0100, Heiko Stuebner wrote:
> Am Donnerstag, 1. Dezember 2016, 18:27:28 CET schrieb Brian Norris:
> > Add the dwc3 usb needed node information for rk3399.
> > 
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > ---
> > Somewhat rewritten from Caesar's reposting (v2) of my patch.
> > 
> > Changes:
> >  * Include USB2 PHY (which is now in -next)
> >  * Don't include USB3 PHY, as extcon support is not ready yet
> >  * Drop non-upstream properties
> >  * Fixup whitespace a bit
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 60
> > ++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index 4ca8f9a7601c..1e97fb8c6415
> > 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > @@ -316,6 +316,66 @@
> >  		};
> >  	};
> > 
> > +	usbdrd3_0: usb@fe800000 {
> 
> insert location above usb@fe380000 is sorted wrong

So, *how* do you think things are sorted here? Alphabetical by label? Or
by node name? Or by unit address? I guess I'm seeing you meant unit
address. But pcie@f8000000 is also out of order then. I guess maybe
that's the only one then.

> > +		compatible = "rockchip,rk3399-dwc3";
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> > +		clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
> > +			 <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
> > +			 <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;
> > +		clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
> > +			      "aclk_usb3otg0", "aclk_usb3_rksoc_axi_perf",
> > +			      "aclk_usb3", "aclk_usb3_grf";
> 
> clock-names do not match binding. The dwc3-of-simple does not care, as it just 
> enables all of them it seems, but binding doc states the clock names as
> 
> - clock-names:  Should contain the following:
>   "ref_clk"     Controller reference clk, have to be 24 MHz
>   "suspend_clk" Controller suspend clk, have to be 24 MHz or 32 KHz
>   "bus_clk"     Master/Core clock, have to be >= 62.5 MHz for SS
>                 operation and >= 30MHz for HS operation
>   "grf_clk"     Controller grf clk

Ah, sorry. I'll try to go with the rockchip,dwc3.txt names better.

There are a few extra clocks here now, but I think those might only be
for USB3 support, which isn't really working yet. I'll either document
them or drop them.

> > +		resets = <&cru SRST_A_USB3_OTG0>;
> > +		reset-names = "usb3-otg";
> 
> you could update the binding documentation to list this one.

Similar story; this is only used for some of the hacky stuff Rockchip is
doing for USB3/TypeC stuff out of tree. I'll either document it or drop
it (as I'm not actually using it yet).

Thanks,
Brian

> Heiko
> 
> > +		status = "disabled";
> > +
> > +		usbdrd_dwc3_0: dwc3 {
> > +			compatible = "snps,dwc3";
> > +			reg = <0x0 0xfe800000 0x0 0x100000>;
> > +			interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> > +			dr_mode = "otg";
> > +			phys = <&u2phy0_otg>;
> > +			phy-names = "usb2-phy";
> > +			snps,dis_enblslpm_quirk;
> > +			snps,dis-u2-freeclk-exists-quirk;
> > +			snps,dis_u2_susphy_quirk;
> > +			snps,dis-del-phy-power-chg-quirk;
> > +			status = "disabled";
> > +		};
> > +	};
[...]

[toc] | [prev] | [next] | [standalone]

#1538007

FromBrian Norris <briannorris@chromium.org>
Date2016-12-07 20:10 +0100
Message-ID<sLPLj-46c-13@gated-at.bofh.it>
In reply to#1537961
On Wed, Dec 07, 2016 at 08:01:23PM +0100, Heiko Stuebner wrote:
> Am Mittwoch, 7. Dezember 2016, 09:52:08 CET schrieb Brian Norris:
> > But pcie@f8000000 is also out of order then. I guess maybe
> > that's the only one then.
> 
> Yep, pcie is misplaced as sadly sometimes I miss those errors as well.

OK. Do you want me to patch that at the end of my series, or is that
unnecessary churn?

Brian

[toc] | [prev] | [next] | [standalone]

#1538020

FromHeiko Stuebner <heiko@sntech.de>
Date2016-12-07 20:10 +0100
Message-ID<sLPLk-46c-49@gated-at.bofh.it>
In reply to#1538007
Am Mittwoch, 7. Dezember 2016, 11:03:02 CET schrieb Brian Norris:
> On Wed, Dec 07, 2016 at 08:01:23PM +0100, Heiko Stuebner wrote:
> > Am Mittwoch, 7. Dezember 2016, 09:52:08 CET schrieb Brian Norris:
> > > But pcie@f8000000 is also out of order then. I guess maybe
> > > that's the only one then.
> > 
> > Yep, pcie is misplaced as sadly sometimes I miss those errors as well.
> 
> OK. Do you want me to patch that at the end of my series, or is that
> unnecessary churn?

I'm still pondering [and am investing way to much brain cells in trying to 
decide this ;-) ], but tend to want to move it.

That way the dtsi gives the correct sorting impression for future dt-patches 
of which I guess we'll see a lot in the time to come.

So yes, please move it as well.


Thanks
Heiko

[toc] | [prev] | [next] | [standalone]

#1538015

FromHeiko Stuebner <heiko@sntech.de>
Date2016-12-07 20:10 +0100
Message-ID<sLPLj-46c-15@gated-at.bofh.it>
In reply to#1537961
Am Mittwoch, 7. Dezember 2016, 09:52:08 CET schrieb Brian Norris:
> Hi,
> 
> On Wed, Dec 07, 2016 at 06:09:16PM +0100, Heiko Stuebner wrote:
> > Am Donnerstag, 1. Dezember 2016, 18:27:28 CET schrieb Brian Norris:
> > > Add the dwc3 usb needed node information for rk3399.
> > > 
> > > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > > ---
> > > Somewhat rewritten from Caesar's reposting (v2) of my patch.
> > > 
> > > Changes:
> > >  * Include USB2 PHY (which is now in -next)
> > >  * Don't include USB3 PHY, as extcon support is not ready yet
> > >  * Drop non-upstream properties
> > >  * Fixup whitespace a bit
> > > 
> > > ---
> > > 
> > >  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 60
> > > 
> > > ++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index
> > > 4ca8f9a7601c..1e97fb8c6415
> > > 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> > > @@ -316,6 +316,66 @@
> > > 
> > >  		};
> > >  	
> > >  	};
> > > 
> > > +	usbdrd3_0: usb@fe800000 {
> > 
> > insert location above usb@fe380000 is sorted wrong
> 
> So, *how* do you think things are sorted here? Alphabetical by label? Or
> by node name? Or by unit address? I guess I'm seeing you meant unit
> address.

correct. Per unit-address first, nodes without address alphabetical by node-
name (above nodes with addresses), never by label.

> But pcie@f8000000 is also out of order then. I guess maybe
> that's the only one then.

Yep, pcie is misplaced as sadly sometimes I miss those errors as well.


Heiko

> > > +		compatible = "rockchip,rk3399-dwc3";
> > > +		#address-cells = <2>;
> > > +		#size-cells = <2>;
> > > +		ranges;
> > > +		clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
> > > +			 <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_RKSOC_AXI_PERF>,
> > > +			 <&cru ACLK_USB3>, <&cru ACLK_USB3_GRF>;
> > > +		clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
> > > +			      "aclk_usb3otg0", "aclk_usb3_rksoc_axi_perf",
> > > +			      "aclk_usb3", "aclk_usb3_grf";
> > 
> > clock-names do not match binding. The dwc3-of-simple does not care, as it
> > just enables all of them it seems, but binding doc states the clock names
> > as> 
> > - clock-names:  Should contain the following:
> >   "ref_clk"     Controller reference clk, have to be 24 MHz
> >   "suspend_clk" Controller suspend clk, have to be 24 MHz or 32 KHz
> >   "bus_clk"     Master/Core clock, have to be >= 62.5 MHz for SS
> >   
> >                 operation and >= 30MHz for HS operation
> >   
> >   "grf_clk"     Controller grf clk
> 
> Ah, sorry. I'll try to go with the rockchip,dwc3.txt names better.
> 
> There are a few extra clocks here now, but I think those might only be
> for USB3 support, which isn't really working yet. I'll either document
> them or drop them.
> 
> > > +		resets = <&cru SRST_A_USB3_OTG0>;
> > > +		reset-names = "usb3-otg";
> > 
> > you could update the binding documentation to list this one.
> 
> Similar story; this is only used for some of the hacky stuff Rockchip is
> doing for USB3/TypeC stuff out of tree. I'll either document it or drop
> it (as I'm not actually using it yet).
> 
> Thanks,
> Brian
> 
> > Heiko
> > 
> > > +		status = "disabled";
> > > +
> > > +		usbdrd_dwc3_0: dwc3 {
> > > +			compatible = "snps,dwc3";
> > > +			reg = <0x0 0xfe800000 0x0 0x100000>;
> > > +			interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> > > +			dr_mode = "otg";
> > > +			phys = <&u2phy0_otg>;
> > > +			phy-names = "usb2-phy";
> > > +			snps,dis_enblslpm_quirk;
> > > +			snps,dis-u2-freeclk-exists-quirk;
> > > +			snps,dis_u2_susphy_quirk;
> > > +			snps,dis-del-phy-power-chg-quirk;
> > > +			status = "disabled";
> > > +		};
> > > +	};
> 
> [...]

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web