* [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399
@ 2016-07-19 7:28 Frank Wang
2016-07-20 21:33 ` Doug Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Frank Wang @ 2016-07-19 7:28 UTC (permalink / raw)
To: heiko, dianders, linux, groeck, jwerner, kishon, robh+dt,
pawel.moll, mark.rutland, ijc+devicetree, galak
Cc: linux-kernel, devicetree, linux-usb, linux-rockchip, xzy.xu,
kever.yang, huangtao, william.wu, daniel.meng, frank.wang
Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
---
arch/arm64/boot/dts/rockchip/rk3399-evb.dts | 19 +++++++++++
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 47 ++++++++++++++++++++++++++-
2 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
index 1a3eb14..31d4828 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
@@ -69,6 +69,15 @@
regulator-max-microvolt = <3300000>;
};
+ vbus_host: vbus-host-regulator {
+ compatible = "regulator-fixed";
+ enable-active-high;
+ gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&host_vbus_drv>;
+ regulator-name = "vbus_host";
+ };
+
vcc_phy: vcc-phy-regulator {
compatible = "regulator-fixed";
regulator-name = "vcc_phy";
@@ -93,6 +102,16 @@
status = "okay";
};
+&u2phy0_host {
+ phy-supply = <&vbus_host>;
+ status = "okay";
+};
+
+&u2phy1_host {
+ phy-supply = <&vbus_host>;
+ status = "okay";
+};
+
&usb_host0_ehci {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d7f8e06..0383785 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -221,6 +221,8 @@
interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>;
clock-names = "hclk_host0", "hclk_host0_arb";
+ phys = <&u2phy0_host>;
+ phy-names = "u2phy0";
status = "disabled";
};
@@ -239,6 +241,8 @@
interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru HCLK_HOST1>, <&cru HCLK_HOST1_ARB>;
clock-names = "hclk_host1", "hclk_host1_arb";
+ phys = <&u2phy1_host>;
+ phy-names = "u2phy1";
status = "disabled";
};
@@ -481,8 +485,42 @@
};
grf: syscon@ff770000 {
- compatible = "rockchip,rk3399-grf", "syscon";
+ compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
reg = <0x0 0xff770000 0x0 0x10000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ u2phy0: usb2-phy@e450 {
+ compatible = "rockchip,rk3399-usb2phy";
+ reg = <0xe450 0x10>;
+ clocks = <&cru SCLK_USB2PHY0_REF>;
+ clock-names = "phyclk";
+ #clock-cells = <0>;
+ clock-output-names = "clk_usbphy0_480m";
+
+ u2phy0_host: host-port {
+ #phy-cells = <0>;
+ interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "linestate";
+ status = "disabled";
+ };
+ };
+
+ u2phy1: usb2-phy@e460 {
+ compatible = "rockchip,rk3399-usb2phy";
+ reg = <0xe460 0x10>;
+ clocks = <&cru SCLK_USB2PHY1_REF>;
+ clock-names = "phyclk";
+ #clock-cells = <0>;
+ clock-output-names = "clk_usbphy1_480m";
+
+ u2phy1_host: host-port {
+ #phy-cells = <0>;
+ interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "linestate";
+ status = "disabled";
+ };
+ };
};
watchdog@ff840000 {
@@ -1009,5 +1047,12 @@
<1 14 RK_FUNC_1 &pcfg_pull_none>;
};
};
+
+ usb2 {
+ host_vbus_drv: host-vbus-drv {
+ rockchip,pins =
+ <4 25 RK_FUNC_GPIO &pcfg_pull_none>;
+ };
+ };
};
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399
2016-07-19 7:28 [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399 Frank Wang
@ 2016-07-20 21:33 ` Doug Anderson
2016-07-20 22:06 ` Heiko Stübner
2016-07-21 2:49 ` Frank Wang
0 siblings, 2 replies; 6+ messages in thread
From: Doug Anderson @ 2016-07-20 21:33 UTC (permalink / raw)
To: Frank Wang
Cc: Heiko Stübner, Guenter Roeck, Guenter Roeck, Julius Werner,
Kishon Vijay Abraham I, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-usb@vger.kernel.org,
open list:ARM/Rockchip SoC..., Ziyuan Xu, Kever Yang, Tao Huang,
吴良峰, daniel.meng
Hi,
On Tue, Jul 19, 2016 at 12:28 AM, Frank Wang <frank.wang@rock-chips.com> wrote:
You need a patch description here, even for simple patches. All you
have now is a subject.
> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> ---
> arch/arm64/boot/dts/rockchip/rk3399-evb.dts | 19 +++++++++++
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 47 ++++++++++++++++++++++++++-
Personally I'd prefer to see EVB in a separate patch.
> 2 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> index 1a3eb14..31d4828 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> @@ -69,6 +69,15 @@
> regulator-max-microvolt = <3300000>;
> };
>
> + vbus_host: vbus-host-regulator {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&host_vbus_drv>;
> + regulator-name = "vbus_host";
> + };
> +
To match my schematics, this would probably be "vcc5v0_host".
Technically there are two regulators but since they are the same
voltage and enabled by the same GPIO it seems like modeling it as one
regulator is fine.
If you really wanted to model things you could also include the input
supply (VCC5V0_SYS). Not sure how much you care to model in EVB.
> vcc_phy: vcc-phy-regulator {
> compatible = "regulator-fixed";
> regulator-name = "vcc_phy";
> @@ -93,6 +102,16 @@
> status = "okay";
> };
>
> +&u2phy0_host {
> + phy-supply = <&vbus_host>;
> + status = "okay";
> +};
> +
> +&u2phy1_host {
> + phy-supply = <&vbus_host>;
> + status = "okay";
> +};
> +
Technically "u2" sorts alphabetically before "uart".
> &usb_host0_ehci {
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index d7f8e06..0383785 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -221,6 +221,8 @@
> interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>;
> clock-names = "hclk_host0", "hclk_host0_arb";
> + phys = <&u2phy0_host>;
> + phy-names = "u2phy0";
This is wrong. From
"Documentation/devicetree/bindings/usb/usb-ehci.txt" phy-names should
be "usb".
> status = "disabled";
> };
>
> @@ -239,6 +241,8 @@
> interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&cru HCLK_HOST1>, <&cru HCLK_HOST1_ARB>;
> clock-names = "hclk_host1", "hclk_host1_arb";
> + phys = <&u2phy1_host>;
> + phy-names = "u2phy1";
This is wrong. From
"Documentation/devicetree/bindings/usb/usb-ehci.txt" phy-names should
be "usb".
> status = "disabled";
> };
>
> @@ -481,8 +485,42 @@
> };
>
> grf: syscon@ff770000 {
> - compatible = "rockchip,rk3399-grf", "syscon";
> + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
> reg = <0x0 0xff770000 0x0 0x10000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + u2phy0: usb2-phy@e450 {
> + compatible = "rockchip,rk3399-usb2phy";
> + reg = <0xe450 0x10>;
> + clocks = <&cru SCLK_USB2PHY0_REF>;
> + clock-names = "phyclk";
> + #clock-cells = <0>;
> + clock-output-names = "clk_usbphy0_480m";
Any reason why there isn't a 'status = "disabled";' here?
> + u2phy0_host: host-port {
> + #phy-cells = <0>;
> + interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "linestate";
> + status = "disabled";
> + };
> + };
> +
> + u2phy1: usb2-phy@e460 {
> + compatible = "rockchip,rk3399-usb2phy";
> + reg = <0xe460 0x10>;
> + clocks = <&cru SCLK_USB2PHY1_REF>;
> + clock-names = "phyclk";
> + #clock-cells = <0>;
> + clock-output-names = "clk_usbphy1_480m";
> +
> + u2phy1_host: host-port {
> + #phy-cells = <0>;
> + interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "linestate";
> + status = "disabled";
> + };
> + };
> };
>
> watchdog@ff840000 {
> @@ -1009,5 +1047,12 @@
> <1 14 RK_FUNC_1 &pcfg_pull_none>;
> };
> };
> +
> + usb2 {
> + host_vbus_drv: host-vbus-drv {
> + rockchip,pins =
> + <4 25 RK_FUNC_GPIO &pcfg_pull_none>;
> + };
> + };
Are you certain this belongs in rk3399.dtsi? It seems like it should
be in the EVB file.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399
2016-07-20 21:33 ` Doug Anderson
@ 2016-07-20 22:06 ` Heiko Stübner
2016-07-21 2:49 ` Frank Wang
1 sibling, 0 replies; 6+ messages in thread
From: Heiko Stübner @ 2016-07-20 22:06 UTC (permalink / raw)
To: Doug Anderson
Cc: Frank Wang, Guenter Roeck, Guenter Roeck, Julius Werner,
Kishon Vijay Abraham I, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-usb@vger.kernel.org,
open list:ARM/Rockchip SoC..., Ziyuan Xu, Kever Yang, Tao Huang,
吴良峰, daniel.meng
Am Mittwoch, 20. Juli 2016, 14:33:25 schrieb Doug Anderson:
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + u2phy0: usb2-phy@e450 {
> > + compatible = "rockchip,rk3399-usb2phy";
> > + reg = <0xe450 0x10>;
> > + clocks = <&cru SCLK_USB2PHY0_REF>;
> > + clock-names = "phyclk";
> > + #clock-cells = <0>;
> > + clock-output-names = "clk_usbphy0_480m";
>
> Any reason why there isn't a 'status = "disabled";' here?
my guess would be, because we might need the provided clock anyway, even in
the case where the actual phy is not used.
But that is better decided by the board/board-dts designer if the clock is
really needed, so add the disabled here and enable in the board, as Doug
suggested.
> > + u2phy0_host: host-port {
> > + #phy-cells = <0>;
> > + interrupts = <GIC_SPI 27
> > IRQ_TYPE_LEVEL_HIGH>; + interrupt-names =
> > "linestate";
> > + status = "disabled";
> > + };
> > + };
> > +
> > + u2phy1: usb2-phy@e460 {
> > + compatible = "rockchip,rk3399-usb2phy";
> > + reg = <0xe460 0x10>;
> > + clocks = <&cru SCLK_USB2PHY1_REF>;
> > + clock-names = "phyclk";
> > + #clock-cells = <0>;
> > + clock-output-names = "clk_usbphy1_480m";
> > +
> > + u2phy1_host: host-port {
> > + #phy-cells = <0>;
> > + interrupts = <GIC_SPI 31
> > IRQ_TYPE_LEVEL_HIGH>; + interrupt-names =
> > "linestate";
> > + status = "disabled";
> > + };
> > + };
> >
> > };
> >
> > watchdog@ff840000 {
> >
> > @@ -1009,5 +1047,12 @@
> >
> > <1 14 RK_FUNC_1 &pcfg_pull_none>;
> >
> > };
> >
> > };
> >
> > +
> > + usb2 {
> > + host_vbus_drv: host-vbus-drv {
> > + rockchip,pins =
> > + <4 25 RK_FUNC_GPIO
> > &pcfg_pull_none>; + };
> > + };
>
> Are you certain this belongs in rk3399.dtsi? It seems like it should
> be in the EVB file.
yep, while most boards follow a reference design, this is still highly board-
specific.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399
2016-07-20 21:33 ` Doug Anderson
2016-07-20 22:06 ` Heiko Stübner
@ 2016-07-21 2:49 ` Frank Wang
2016-07-21 9:26 ` Heiko Stübner
1 sibling, 1 reply; 6+ messages in thread
From: Frank Wang @ 2016-07-21 2:49 UTC (permalink / raw)
To: Doug Anderson, Heiko Stübner
Cc: Guenter Roeck, Guenter Roeck, Julius Werner,
Kishon Vijay Abraham I, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-usb@vger.kernel.org,
open list:ARM/Rockchip SoC..., Ziyuan Xu, Kever Yang, Tao Huang,
吴良峰, daniel.meng, frank.wang
Hi Doug,
On 2016/7/21 5:33, Doug Anderson wrote:
> Hi,
>
> On Tue, Jul 19, 2016 at 12:28 AM, Frank Wang <frank.wang@rock-chips.com> wrote:
>
> You need a patch description here, even for simple patches. All you
> have now is a subject.
>
OK, I will describe it next version.
>> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
>> ---
>> arch/arm64/boot/dts/rockchip/rk3399-evb.dts | 19 +++++++++++
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 47 ++++++++++++++++++++++++++-
> Personally I'd prefer to see EVB in a separate patch.
>
Yep, I would like to separate them :-) .
>> 2 files changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
>> index 1a3eb14..31d4828 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
>> @@ -69,6 +69,15 @@
>> regulator-max-microvolt = <3300000>;
>> };
>>
>> + vbus_host: vbus-host-regulator {
>> + compatible = "regulator-fixed";
>> + enable-active-high;
>> + gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&host_vbus_drv>;
>> + regulator-name = "vbus_host";
>> + };
>> +
> To match my schematics, this would probably be "vcc5v0_host".
> Technically there are two regulators but since they are the same
> voltage and enabled by the same GPIO it seems like modeling it as one
> regulator is fine.
Yep, you are right, I will rename it.
> If you really wanted to model things you could also include the input
> supply (VCC5V0_SYS). Not sure how much you care to model in EVB.
>
Actually, from
"Documentation/devicetree/bindings/regulator/fixed-regulator.txt" show,
input supply name is just optional property, and it seems that only do
assign "vin" value for input_supply (the second member of struct
fixed_voltage_config) if "vin-supply" is specified.
So is input supply name (VCC5V0_SYS) required here? Would you like to
give more comments please?
>> vcc_phy: vcc-phy-regulator {
>> compatible = "regulator-fixed";
>> regulator-name = "vcc_phy";
>> @@ -93,6 +102,16 @@
>> status = "okay";
>> };
>>
>> +&u2phy0_host {
>> + phy-supply = <&vbus_host>;
>> + status = "okay";
>> +};
>> +
>> +&u2phy1_host {
>> + phy-supply = <&vbus_host>;
>> + status = "okay";
>> +};
>> +
> Technically "u2" sorts alphabetically before "uart".
>
Well, It will be sorted next version.
>> &usb_host0_ehci {
>> status = "okay";
>> };
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index d7f8e06..0383785 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -221,6 +221,8 @@
>> interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>;
>> clock-names = "hclk_host0", "hclk_host0_arb";
>> + phys = <&u2phy0_host>;
>> + phy-names = "u2phy0";
> This is wrong. From
> "Documentation/devicetree/bindings/usb/usb-ehci.txt" phy-names should
> be "usb".
done.
>> status = "disabled";
>> };
>>
>> @@ -239,6 +241,8 @@
>> interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&cru HCLK_HOST1>, <&cru HCLK_HOST1_ARB>;
>> clock-names = "hclk_host1", "hclk_host1_arb";
>> + phys = <&u2phy1_host>;
>> + phy-names = "u2phy1";
> This is wrong. From
> "Documentation/devicetree/bindings/usb/usb-ehci.txt" phy-names should
> be "usb".
done
>> status = "disabled";
>> };
>>
>> @@ -481,8 +485,42 @@
>> };
>>
>> grf: syscon@ff770000 {
>> - compatible = "rockchip,rk3399-grf", "syscon";
>> + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
>> reg = <0x0 0xff770000 0x0 0x10000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + u2phy0: usb2-phy@e450 {
>> + compatible = "rockchip,rk3399-usb2phy";
>> + reg = <0xe450 0x10>;
>> + clocks = <&cru SCLK_USB2PHY0_REF>;
>> + clock-names = "phyclk";
>> + #clock-cells = <0>;
>> + clock-output-names = "clk_usbphy0_480m";
> Any reason why there isn't a 'status = "disabled";' here?
>
Refer to some explains from Heiko in another mail which sent at 6:07 AM
on 21th July, anyway, I will add ' status = "disabled" ' property next
version.
>> + u2phy0_host: host-port {
>> + #phy-cells = <0>;
>> + interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "linestate";
>> + status = "disabled";
>> + };
>> + };
>> +
>> + u2phy1: usb2-phy@e460 {
>> + compatible = "rockchip,rk3399-usb2phy";
>> + reg = <0xe460 0x10>;
>> + clocks = <&cru SCLK_USB2PHY1_REF>;
>> + clock-names = "phyclk";
>> + #clock-cells = <0>;
>> + clock-output-names = "clk_usbphy1_480m";
>> +
>> + u2phy1_host: host-port {
>> + #phy-cells = <0>;
>> + interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "linestate";
>> + status = "disabled";
>> + };
>> + };
>> };
>>
>> watchdog@ff840000 {
>> @@ -1009,5 +1047,12 @@
>> <1 14 RK_FUNC_1 &pcfg_pull_none>;
>> };
>> };
>> +
>> + usb2 {
>> + host_vbus_drv: host-vbus-drv {
>> + rockchip,pins =
>> + <4 25 RK_FUNC_GPIO &pcfg_pull_none>;
>> + };
>> + };
> Are you certain this belongs in rk3399.dtsi? It seems like it should
> be in the EVB file.
>
All right, It will be moved to EVB file next version.
BR.
Frank
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399
2016-07-21 2:49 ` Frank Wang
@ 2016-07-21 9:26 ` Heiko Stübner
2016-07-21 10:17 ` Frank Wang
0 siblings, 1 reply; 6+ messages in thread
From: Heiko Stübner @ 2016-07-21 9:26 UTC (permalink / raw)
To: Frank Wang
Cc: Doug Anderson, Guenter Roeck, Guenter Roeck, Julius Werner,
Kishon Vijay Abraham I, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-usb@vger.kernel.org,
open list:ARM/Rockchip SoC..., Ziyuan Xu, Kever Yang, Tao Huang,
吴良峰, daniel.meng
Hi Frank,
Am Donnerstag, 21. Juli 2016, 10:49:53 schrieb Frank Wang:
> >> @@ -69,6 +69,15 @@
> >>
> >> regulator-max-microvolt = <3300000>;
> >>
> >> };
> >>
> >> + vbus_host: vbus-host-regulator {
> >> + compatible = "regulator-fixed";
> >> + enable-active-high;
> >> + gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
> >> + pinctrl-names = "default";
> >> + pinctrl-0 = <&host_vbus_drv>;
> >> + regulator-name = "vbus_host";
> >> + };
> >> +
> >
> > To match my schematics, this would probably be "vcc5v0_host".
> > Technically there are two regulators but since they are the same
> > voltage and enabled by the same GPIO it seems like modeling it as one
> > regulator is fine.
>
> Yep, you are right, I will rename it.
>
> > If you really wanted to model things you could also include the input
> > supply (VCC5V0_SYS). Not sure how much you care to model in EVB.
>
> Actually, from
> "Documentation/devicetree/bindings/regulator/fixed-regulator.txt" show,
> input supply name is just optional property, and it seems that only do
> assign "vin" value for input_supply (the second member of struct
> fixed_voltage_config) if "vin-supply" is specified.
>
> So is input supply name (VCC5V0_SYS) required here? Would you like to
> give more comments please?
While vin-supply is optional, I think that is meant for real top-level
regulators (our vcc_sys or whatever) that really don't have a parent
regulator.
It is always nicer to model the whole power-tree [in a sane way], as it makes
following the schematics a lot easier. If you mount a debugfs these days you
can even get a nice tree graph of the regulator infrastructure ... where the
parent-relationship is also needed to create something meaningful.
Heiko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399
2016-07-21 9:26 ` Heiko Stübner
@ 2016-07-21 10:17 ` Frank Wang
0 siblings, 0 replies; 6+ messages in thread
From: Frank Wang @ 2016-07-21 10:17 UTC (permalink / raw)
To: Heiko Stübner, Doug Anderson
Cc: Guenter Roeck, Guenter Roeck, Julius Werner,
Kishon Vijay Abraham I, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-usb@vger.kernel.org,
open list:ARM/Rockchip SoC..., Ziyuan Xu, Kever Yang, Tao Huang,
吴良峰, daniel.meng, frank.wang
Hi Heiko,
On 2016/7/21 17:26, Heiko Stübner wrote:
> Hi Frank,
>
> Am Donnerstag, 21. Juli 2016, 10:49:53 schrieb Frank Wang:
>>>> @@ -69,6 +69,15 @@
>>>>
>>>> regulator-max-microvolt = <3300000>;
>>>>
>>>> };
>>>>
>>>> + vbus_host: vbus-host-regulator {
>>>> + compatible = "regulator-fixed";
>>>> + enable-active-high;
>>>> + gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
>>>> + pinctrl-names = "default";
>>>> + pinctrl-0 = <&host_vbus_drv>;
>>>> + regulator-name = "vbus_host";
>>>> + };
>>>> +
>>> To match my schematics, this would probably be "vcc5v0_host".
>>> Technically there are two regulators but since they are the same
>>> voltage and enabled by the same GPIO it seems like modeling it as one
>>> regulator is fine.
>> Yep, you are right, I will rename it.
>>
>>> If you really wanted to model things you could also include the input
>>> supply (VCC5V0_SYS). Not sure how much you care to model in EVB.
>> Actually, from
>> "Documentation/devicetree/bindings/regulator/fixed-regulator.txt" show,
>> input supply name is just optional property, and it seems that only do
>> assign "vin" value for input_supply (the second member of struct
>> fixed_voltage_config) if "vin-supply" is specified.
>>
>> So is input supply name (VCC5V0_SYS) required here? Would you like to
>> give more comments please?
> While vin-supply is optional, I think that is meant for real top-level
> regulators (our vcc_sys or whatever) that really don't have a parent
> regulator.
>
> It is always nicer to model the whole power-tree [in a sane way], as it makes
> following the schematics a lot easier. If you mount a debugfs these days you
> can even get a nice tree graph of the regulator infrastructure ... where the
> parent-relationship is also needed to create something meaningful.
>
Got it, many thanks for your explanation, I will add vin-supply next
version.
BR.
Frank
> Heiko
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-21 10:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-19 7:28 [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399 Frank Wang
2016-07-20 21:33 ` Doug Anderson
2016-07-20 22:06 ` Heiko Stübner
2016-07-21 2:49 ` Frank Wang
2016-07-21 9:26 ` Heiko Stübner
2016-07-21 10:17 ` Frank Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox