[PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[Michael Riesch]

All nodes and handles related to USB have the prefix usb or usb2,
whereas the phy handles are prefixed with u2phy. Rename for
consistency reasons and to facilitate sorting.

This patch also updates the handles in the only board file that
uses them (rk3566-quartz64-a.dts).

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   | 18 ++++++++---------
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      | 20 +++++++++----------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
index f1d6bf10c650..3e65465ac7d5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
@@ -574,32 +574,32 @@ &uart2 {
 	status = "okay";
 };
 
-&u2phy1_host {
-	phy-supply = <&vcc5v0_usb20_host>;
+&usb_host0_ehci {
 	status = "okay";
 };
 
-&u2phy1_otg {
-	phy-supply = <&vcc5v0_usb20_host>;
+&usb_host0_ohci {
 	status = "okay";
 };
 
-&u2phy1 {
+&usb_host1_ehci {
 	status = "okay";
 };
 
-&usb_host0_ehci {
+&usb_host1_ohci {
 	status = "okay";
 };
 
-&usb_host0_ohci {
+&usb2phy1 {
 	status = "okay";
 };
 
-&usb_host1_ehci {
+&usb2phy1_host {
+	phy-supply = <&vcc5v0_usb20_host>;
 	status = "okay";
 };
 
-&usb_host1_ohci {
+&usb2phy1_otg {
+	phy-supply = <&vcc5v0_usb20_host>;
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 8ee2fab676f4..69c30992ced2 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -214,7 +214,7 @@ usb_host0_ehci: usb@fd800000 {
 		interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru HCLK_USB2HOST0>, <&cru HCLK_USB2HOST0_ARB>,
 			 <&cru PCLK_USB>;
-		phys = <&u2phy1_otg>;
+		phys = <&usb2phy1_otg>;
 		phy-names = "usb";
 		status = "disabled";
 	};
@@ -225,7 +225,7 @@ usb_host0_ohci: usb@fd840000 {
 		interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru HCLK_USB2HOST0>, <&cru HCLK_USB2HOST0_ARB>,
 			 <&cru PCLK_USB>;
-		phys = <&u2phy1_otg>;
+		phys = <&usb2phy1_otg>;
 		phy-names = "usb";
 		status = "disabled";
 	};
@@ -236,7 +236,7 @@ usb_host1_ehci: usb@fd880000 {
 		interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru HCLK_USB2HOST1>, <&cru HCLK_USB2HOST1_ARB>,
 			 <&cru PCLK_USB>;
-		phys = <&u2phy1_host>;
+		phys = <&usb2phy1_host>;
 		phy-names = "usb";
 		status = "disabled";
 	};
@@ -247,7 +247,7 @@ usb_host1_ohci: usb@fd8c0000 {
 		interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru HCLK_USB2HOST1>, <&cru HCLK_USB2HOST1_ARB>,
 			 <&cru PCLK_USB>;
-		phys = <&u2phy1_host>;
+		phys = <&usb2phy1_host>;
 		phy-names = "usb";
 		status = "disabled";
 	};
@@ -1195,7 +1195,7 @@ pwm15: pwm@fe700030 {
 		status = "disabled";
 	};
 
-	u2phy0: usb2phy@fe8a0000 {
+	usb2phy0: usb2phy@fe8a0000 {
 		compatible = "rockchip,rk3568-usb2phy";
 		reg = <0x0 0xfe8a0000 0x0 0x10000>;
 		clocks = <&pmucru CLK_USBPHY0_REF>;
@@ -1206,18 +1206,18 @@ u2phy0: usb2phy@fe8a0000 {
 		#clock-cells = <0>;
 		status = "disabled";
 
-		u2phy0_host: host-port {
+		usb2phy0_host: host-port {
 			#phy-cells = <0>;
 			status = "disabled";
 		};
 
-		u2phy0_otg: otg-port {
+		usb2phy0_otg: otg-port {
 			#phy-cells = <0>;
 			status = "disabled";
 		};
 	};
 
-	u2phy1: usb2phy@fe8b0000 {
+	usb2phy1: usb2phy@fe8b0000 {
 		compatible = "rockchip,rk3568-usb2phy";
 		reg = <0x0 0xfe8b0000 0x0 0x10000>;
 		clocks = <&pmucru CLK_USBPHY1_REF>;
@@ -1228,12 +1228,12 @@ u2phy1: usb2phy@fe8b0000 {
 		#clock-cells = <0>;
 		status = "disabled";
 
-		u2phy1_host: host-port {
+		usb2phy1_host: host-port {
 			#phy-cells = <0>;
 			status = "disabled";
 		};
 
-		u2phy1_otg: otg-port {
+		usb2phy1_otg: otg-port {
 			#phy-cells = <0>;
 			status = "disabled";
 		};
-- 
2.30.2

[PATCH 2/2] arm64: dts: rockchip: add usb2 support to rk3568-evb1-v10

[Michael Riesch]

Activate the USB2 controller and phy nodes in the device tree of the
RK3568 EVB1.

Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
---
 .../boot/dts/rockchip/rk3568-evb1-v10.dts     | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
index 184e2aa2416a..c68bade0d99b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
@@ -53,6 +53,28 @@ vcc5v0_sys: vcc5v0-sys {
 		vin-supply = <&dc_12v>;
 	};
 
+	vcc5v0_usb: vcc5v0-usb {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc5v0_usb";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&dc_12v>;
+	};
+
+	vcc5v0_usb_host: vcc5v0-usb-host {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpio = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&vcc5v0_usb_host_en>;
+		regulator-name = "vcc5v0_usb_host";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		vin-supply = <&vcc5v0_usb>;
+	};
+
 	vcc3v3_lcd0_n: vcc3v3-lcd0-n {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc3v3_lcd0_n";
@@ -345,6 +367,12 @@ pmic_int: pmic_int {
 				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
 		};
 	};
+
+	usb {
+		vcc5v0_usb_host_en: vcc5v0_usb_host_en {
+			rockchip,pins = <0 RK_PA6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
 };
 
 &pmu_io_domains {
@@ -390,3 +418,33 @@ &sdmmc0 {
 &uart2 {
 	status = "okay";
 };
+
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};
+
+&usb_host1_ehci {
+	status = "okay";
+};
+
+&usb_host1_ohci {
+	status = "okay";
+};
+
+&usb2phy1 {
+	status = "okay";
+};
+
+&usb2phy1_host {
+	phy-supply = <&vcc5v0_usb_host>;
+	status = "okay";
+};
+
+&usb2phy1_otg {
+	phy-supply = <&vcc5v0_usb_host>;
+	status = "okay";
+};
-- 
2.30.2

Re: [PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[Peter Geis]

On Thu, Jan 27, 2022 at 2:05 PM Michael Riesch
<michael.riesch@wolfvision.net> wrote:
>
> All nodes and handles related to USB have the prefix usb or usb2,
> whereas the phy handles are prefixed with u2phy. Rename for
> consistency reasons and to facilitate sorting.
>
> This patch also updates the handles in the only board file that
> uses them (rk3566-quartz64-a.dts).

Good Evening,

While I'm not against this idea, my main concern still stands.
I spent a great deal of thought on this, and decided to go the route I
did to maintain consistency with previous generations.
As such, I see one of three paths here:
- Pull this patch only and depart rk356x from previous SoCs.
- Do the same for previous SoCs to maintain consistency.
- Drop this patch to maintain consistency with previous SoCs.

I ask that others weigh in here, as offline discussion has produced
mixed results already.

Thanks,
Peter

>
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  .../boot/dts/rockchip/rk3566-quartz64-a.dts   | 18 ++++++++---------
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi      | 20 +++++++++----------
>  2 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
> index f1d6bf10c650..3e65465ac7d5 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
> @@ -574,32 +574,32 @@ &uart2 {
>         status = "okay";
>  };
>
> -&u2phy1_host {
> -       phy-supply = <&vcc5v0_usb20_host>;
> +&usb_host0_ehci {
>         status = "okay";
>  };
>
> -&u2phy1_otg {
> -       phy-supply = <&vcc5v0_usb20_host>;
> +&usb_host0_ohci {
>         status = "okay";
>  };
>
> -&u2phy1 {
> +&usb_host1_ehci {
>         status = "okay";
>  };
>
> -&usb_host0_ehci {
> +&usb_host1_ohci {
>         status = "okay";
>  };
>
> -&usb_host0_ohci {
> +&usb2phy1 {
>         status = "okay";
>  };
>
> -&usb_host1_ehci {
> +&usb2phy1_host {
> +       phy-supply = <&vcc5v0_usb20_host>;
>         status = "okay";
>  };
>
> -&usb_host1_ohci {
> +&usb2phy1_otg {
> +       phy-supply = <&vcc5v0_usb20_host>;
>         status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 8ee2fab676f4..69c30992ced2 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -214,7 +214,7 @@ usb_host0_ehci: usb@fd800000 {
>                 interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>;
>                 clocks = <&cru HCLK_USB2HOST0>, <&cru HCLK_USB2HOST0_ARB>,
>                          <&cru PCLK_USB>;
> -               phys = <&u2phy1_otg>;
> +               phys = <&usb2phy1_otg>;
>                 phy-names = "usb";
>                 status = "disabled";
>         };
> @@ -225,7 +225,7 @@ usb_host0_ohci: usb@fd840000 {
>                 interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
>                 clocks = <&cru HCLK_USB2HOST0>, <&cru HCLK_USB2HOST0_ARB>,
>                          <&cru PCLK_USB>;
> -               phys = <&u2phy1_otg>;
> +               phys = <&usb2phy1_otg>;
>                 phy-names = "usb";
>                 status = "disabled";
>         };
> @@ -236,7 +236,7 @@ usb_host1_ehci: usb@fd880000 {
>                 interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
>                 clocks = <&cru HCLK_USB2HOST1>, <&cru HCLK_USB2HOST1_ARB>,
>                          <&cru PCLK_USB>;
> -               phys = <&u2phy1_host>;
> +               phys = <&usb2phy1_host>;
>                 phy-names = "usb";
>                 status = "disabled";
>         };
> @@ -247,7 +247,7 @@ usb_host1_ohci: usb@fd8c0000 {
>                 interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
>                 clocks = <&cru HCLK_USB2HOST1>, <&cru HCLK_USB2HOST1_ARB>,
>                          <&cru PCLK_USB>;
> -               phys = <&u2phy1_host>;
> +               phys = <&usb2phy1_host>;
>                 phy-names = "usb";
>                 status = "disabled";
>         };
> @@ -1195,7 +1195,7 @@ pwm15: pwm@fe700030 {
>                 status = "disabled";
>         };
>
> -       u2phy0: usb2phy@fe8a0000 {
> +       usb2phy0: usb2phy@fe8a0000 {
>                 compatible = "rockchip,rk3568-usb2phy";
>                 reg = <0x0 0xfe8a0000 0x0 0x10000>;
>                 clocks = <&pmucru CLK_USBPHY0_REF>;
> @@ -1206,18 +1206,18 @@ u2phy0: usb2phy@fe8a0000 {
>                 #clock-cells = <0>;
>                 status = "disabled";
>
> -               u2phy0_host: host-port {
> +               usb2phy0_host: host-port {
>                         #phy-cells = <0>;
>                         status = "disabled";
>                 };
>
> -               u2phy0_otg: otg-port {
> +               usb2phy0_otg: otg-port {
>                         #phy-cells = <0>;
>                         status = "disabled";
>                 };
>         };
>
> -       u2phy1: usb2phy@fe8b0000 {
> +       usb2phy1: usb2phy@fe8b0000 {
>                 compatible = "rockchip,rk3568-usb2phy";
>                 reg = <0x0 0xfe8b0000 0x0 0x10000>;
>                 clocks = <&pmucru CLK_USBPHY1_REF>;
> @@ -1228,12 +1228,12 @@ u2phy1: usb2phy@fe8b0000 {
>                 #clock-cells = <0>;
>                 status = "disabled";
>
> -               u2phy1_host: host-port {
> +               usb2phy1_host: host-port {
>                         #phy-cells = <0>;
>                         status = "disabled";
>                 };
>
> -               u2phy1_otg: otg-port {
> +               usb2phy1_otg: otg-port {
>                         #phy-cells = <0>;
>                         status = "disabled";
>                 };
> --
> 2.30.2
>

Re: [PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[Piotr Oniszczuk]

> 
> Good Evening,
> 
> While I'm not against this idea, my main concern still stands.
> I spent a great deal of thought on this, and decided to go the route I
> did to maintain consistency with previous generations.
> As such, I see one of three paths here:
> - Pull this patch only and depart rk356x from previous SoCs.
> - Do the same for previous SoCs to maintain consistency.
> - Drop this patch to maintain consistency with previous SoCs.
> 
> I ask that others weigh in here, as offline discussion has produced
> mixed results already.

just pure user perspective

(who spent last weeks considerable time to develop DT for rk3566 tvbox. 99% of my work was by reading/learning from other boards existing DT's. Any inconsistencies in DTs makes work for such ppl like me much more harder):

For option 1 - i don't see value
For option 2 - what is reward for extra work needs to be done on all other SoCs?

so option 3 seems to be natural choice...

in other words:

for me:
option 1 brings practically zero value + increased inconsistency.
option 2: extra work - but consistency is like in option 3 (so where is value?)

so option 3 offers the same consistency - but without extra work...
 
just my 0.02$

 

Re: [PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[Michael Riesch]

Hello Peter and Piotr,

On 1/29/22 10:23, Piotr Oniszczuk wrote:
> 
> 
>>
>> Good Evening,
>>
>> While I'm not against this idea, my main concern still stands.
>> I spent a great deal of thought on this, and decided to go the route I
>> did to maintain consistency with previous generations.
>> As such, I see one of three paths here:
>> - Pull this patch only and depart rk356x from previous SoCs.
>> - Do the same for previous SoCs to maintain consistency.
>> - Drop this patch to maintain consistency with previous SoCs.
>>
>> I ask that others weigh in here, as offline discussion has produced
>> mixed results already.
> 
> just pure user perspective
> 
> (who spent last weeks considerable time to develop DT for rk3566 tvbox. 99% of my work was by reading/learning from other boards existing DT's. Any inconsistencies in DTs makes work for such ppl like me much more harder):
> 
> For option 1 - i don't see value
> For option 2 - what is reward for extra work needs to be done on all other SoCs?
> 
> so option 3 seems to be natural choice...
> 
> in other words:
> 
> for me:
> option 1 brings practically zero value + increased inconsistency.
> option 2: extra work - but consistency is like in option 3 (so where is value?)
> 
> so option 3 offers the same consistency - but without extra work...
>  
> just my 0.02$

Of course this change is purely cosmetic and it is reasonable to ask for
the practical value. It is just that technically the quartz64 dts is not
sorted alphabetically at the moment. The u2phy* nodes should be but
before the uart* nodes to follow the convention. On the other hand, it
may be nice to have the usb2 phys and controllers grouped in the dts.
The proposed renaming would allow all the mentioned nodes sorted
alphabetically and grouped logically.

Therefore I had option 1 in mind. I don't see any dependencies between
the different SoCs and think we can make a fresh start here.

Option 2 is not really feasible, we would almost definitely break
something existent.

Option 3 is feasible, of course. However, I would sort the nodes
alphabetically (u2phy*, then uart*, then usb*). Works for me as well,
although it is not that nice IMHO.

Since many boards with the RK3566 and RK3568 will pop up in near future
we should do the change right now (if we want to do it), as of course
all the board files need to be changed. Therefore I wanted to bring this
matter up now. Let's agree on something and move on.

Best regards,
Michael

Re: [PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[Heiko Stübner]

Am Samstag, 29. Januar 2022, 10:59:32 CET schrieb Michael Riesch:
> Hello Peter and Piotr,
> 
> On 1/29/22 10:23, Piotr Oniszczuk wrote:
> > 
> > 
> >>
> >> Good Evening,
> >>
> >> While I'm not against this idea, my main concern still stands.
> >> I spent a great deal of thought on this, and decided to go the route I
> >> did to maintain consistency with previous generations.
> >> As such, I see one of three paths here:
> >> - Pull this patch only and depart rk356x from previous SoCs.
> >> - Do the same for previous SoCs to maintain consistency.
> >> - Drop this patch to maintain consistency with previous SoCs.
> >>
> >> I ask that others weigh in here, as offline discussion has produced
> >> mixed results already.
> > 
> > just pure user perspective
> > 
> > (who spent last weeks considerable time to develop DT for rk3566 tvbox. 99% of my work was by reading/learning from other boards existing DT's. Any inconsistencies in DTs makes work for such ppl like me much more harder):
> > 
> > For option 1 - i don't see value
> > For option 2 - what is reward for extra work needs to be done on all other SoCs?
> > 
> > so option 3 seems to be natural choice...
> > 
> > in other words:
> > 
> > for me:
> > option 1 brings practically zero value + increased inconsistency.
> > option 2: extra work - but consistency is like in option 3 (so where is value?)
> > 
> > so option 3 offers the same consistency - but without extra work...
> >  
> > just my 0.02$
> 
> Of course this change is purely cosmetic and it is reasonable to ask for
> the practical value. It is just that technically the quartz64 dts is not
> sorted alphabetically at the moment. The u2phy* nodes should be but
> before the uart* nodes to follow the convention. On the other hand, it
> may be nice to have the usb2 phys and controllers grouped in the dts.
> The proposed renaming would allow all the mentioned nodes sorted
> alphabetically and grouped logically.
> 
> Therefore I had option 1 in mind. I don't see any dependencies between
> the different SoCs and think we can make a fresh start here.

correct :-) .

I do see each SoC individually and while I try to have people follow some
styling guidelines everywhere (ordering of properties, ordering of nodes)
I don't really want people to fear what some other SoC has done before.

But even these rules evolve sometimes, when something seems to work
better than before.

We have nowadays 9 years of Rockchip SoC history in the kernel.
Thanks to general dt-binding conventions most nodes have specific
names anyway (mmc@... etc), but for example trying to rename stuff
in older SoCs that has worked for years now is for one error-prone
as Michael pointed out, but also introduces unnecessary churn,
when these old SoCs (thinking of rk3188, rk3288 and friends but also things
like the rk3368) are essentially "finished" and most likely won't see that
much additional support for stuff added.


Heiko


> Option 2 is not really feasible, we would almost definitely break
> something existent.
> 
> Option 3 is feasible, of course. However, I would sort the nodes
> alphabetically (u2phy*, then uart*, then usb*). Works for me as well,
> although it is not that nice IMHO.
> 
> Since many boards with the RK3566 and RK3568 will pop up in near future
> we should do the change right now (if we want to do it), as of course
> all the board files need to be changed. Therefore I wanted to bring this
> matter up now. Let's agree on something and move on.
> 
> Best regards,
> Michael
> 

Re: [PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[Michael Riesch]

Hello Heiko,

On 1/29/22 16:28, Heiko Stübner wrote:
> Am Samstag, 29. Januar 2022, 10:59:32 CET schrieb Michael Riesch:
>> Hello Peter and Piotr,
>>
>> On 1/29/22 10:23, Piotr Oniszczuk wrote:
>>>
>>>
>>>>
>>>> Good Evening,
>>>>
>>>> While I'm not against this idea, my main concern still stands.
>>>> I spent a great deal of thought on this, and decided to go the route I
>>>> did to maintain consistency with previous generations.
>>>> As such, I see one of three paths here:
>>>> - Pull this patch only and depart rk356x from previous SoCs.
>>>> - Do the same for previous SoCs to maintain consistency.
>>>> - Drop this patch to maintain consistency with previous SoCs.
>>>>
>>>> I ask that others weigh in here, as offline discussion has produced
>>>> mixed results already.
>>>
>>> just pure user perspective
>>>
>>> (who spent last weeks considerable time to develop DT for rk3566 tvbox. 99% of my work was by reading/learning from other boards existing DT's. Any inconsistencies in DTs makes work for such ppl like me much more harder):
>>>
>>> For option 1 - i don't see value
>>> For option 2 - what is reward for extra work needs to be done on all other SoCs?
>>>
>>> so option 3 seems to be natural choice...
>>>
>>> in other words:
>>>
>>> for me:
>>> option 1 brings practically zero value + increased inconsistency.
>>> option 2: extra work - but consistency is like in option 3 (so where is value?)
>>>
>>> so option 3 offers the same consistency - but without extra work...
>>>  
>>> just my 0.02$
>>
>> Of course this change is purely cosmetic and it is reasonable to ask for
>> the practical value. It is just that technically the quartz64 dts is not
>> sorted alphabetically at the moment. The u2phy* nodes should be but
>> before the uart* nodes to follow the convention. On the other hand, it
>> may be nice to have the usb2 phys and controllers grouped in the dts.
>> The proposed renaming would allow all the mentioned nodes sorted
>> alphabetically and grouped logically.
>>
>> Therefore I had option 1 in mind. I don't see any dependencies between
>> the different SoCs and think we can make a fresh start here.
> 
> correct :-) .
> 
> I do see each SoC individually and while I try to have people follow some
> styling guidelines everywhere (ordering of properties, ordering of nodes)
> I don't really want people to fear what some other SoC has done before.
> 
> But even these rules evolve sometimes, when something seems to work
> better than before.
> 
> We have nowadays 9 years of Rockchip SoC history in the kernel.
> Thanks to general dt-binding conventions most nodes have specific
> names anyway (mmc@... etc), but for example trying to rename stuff
> in older SoCs that has worked for years now is for one error-prone
> as Michael pointed out, but also introduces unnecessary churn,
> when these old SoCs (thinking of rk3188, rk3288 and friends but also things
> like the rk3368) are essentially "finished" and most likely won't see that
> much additional support for stuff added.

So... may I take it that you are going to apply the patches in this series?

Or should I switch to option 3 and re-submit?

Thanks and best regards,
Michael

> 
> 
> Heiko
> 
> 
>> Option 2 is not really feasible, we would almost definitely break
>> something existent.
>>
>> Option 3 is feasible, of course. However, I would sort the nodes
>> alphabetically (u2phy*, then uart*, then usb*). Works for me as well,
>> although it is not that nice IMHO.
>>
>> Since many boards with the RK3566 and RK3568 will pop up in near future
>> we should do the change right now (if we want to do it), as of course
>> all the board files need to be changed. Therefore I wanted to bring this
>> matter up now. Let's agree on something and move on.
>>
>> Best regards,
>> Michael
>>
> 
> 
> 
> 

Re: [PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[Heiko Stübner]

Am Sonntag, 30. Januar 2022, 10:56:08 CET schrieb Michael Riesch:
> Hello Heiko,
> 
> On 1/29/22 16:28, Heiko Stübner wrote:
> > Am Samstag, 29. Januar 2022, 10:59:32 CET schrieb Michael Riesch:
> >> Hello Peter and Piotr,
> >>
> >> On 1/29/22 10:23, Piotr Oniszczuk wrote:
> >>>
> >>>
> >>>>
> >>>> Good Evening,
> >>>>
> >>>> While I'm not against this idea, my main concern still stands.
> >>>> I spent a great deal of thought on this, and decided to go the route I
> >>>> did to maintain consistency with previous generations.
> >>>> As such, I see one of three paths here:
> >>>> - Pull this patch only and depart rk356x from previous SoCs.
> >>>> - Do the same for previous SoCs to maintain consistency.
> >>>> - Drop this patch to maintain consistency with previous SoCs.
> >>>>
> >>>> I ask that others weigh in here, as offline discussion has produced
> >>>> mixed results already.
> >>>
> >>> just pure user perspective
> >>>
> >>> (who spent last weeks considerable time to develop DT for rk3566 tvbox. 99% of my work was by reading/learning from other boards existing DT's. Any inconsistencies in DTs makes work for such ppl like me much more harder):
> >>>
> >>> For option 1 - i don't see value
> >>> For option 2 - what is reward for extra work needs to be done on all other SoCs?
> >>>
> >>> so option 3 seems to be natural choice...
> >>>
> >>> in other words:
> >>>
> >>> for me:
> >>> option 1 brings practically zero value + increased inconsistency.
> >>> option 2: extra work - but consistency is like in option 3 (so where is value?)
> >>>
> >>> so option 3 offers the same consistency - but without extra work...
> >>>  
> >>> just my 0.02$
> >>
> >> Of course this change is purely cosmetic and it is reasonable to ask for
> >> the practical value. It is just that technically the quartz64 dts is not
> >> sorted alphabetically at the moment. The u2phy* nodes should be but
> >> before the uart* nodes to follow the convention. On the other hand, it
> >> may be nice to have the usb2 phys and controllers grouped in the dts.
> >> The proposed renaming would allow all the mentioned nodes sorted
> >> alphabetically and grouped logically.
> >>
> >> Therefore I had option 1 in mind. I don't see any dependencies between
> >> the different SoCs and think we can make a fresh start here.
> > 
> > correct :-) .
> > 
> > I do see each SoC individually and while I try to have people follow some
> > styling guidelines everywhere (ordering of properties, ordering of nodes)
> > I don't really want people to fear what some other SoC has done before.
> > 
> > But even these rules evolve sometimes, when something seems to work
> > better than before.
> > 
> > We have nowadays 9 years of Rockchip SoC history in the kernel.
> > Thanks to general dt-binding conventions most nodes have specific
> > names anyway (mmc@... etc), but for example trying to rename stuff
> > in older SoCs that has worked for years now is for one error-prone
> > as Michael pointed out, but also introduces unnecessary churn,
> > when these old SoCs (thinking of rk3188, rk3288 and friends but also things
> > like the rk3368) are essentially "finished" and most likely won't see that
> > much additional support for stuff added.
> 
> So... may I take it that you are going to apply the patches in this series?

that was the intention behind that "wall of text" :-D

Heiko


> Or should I switch to option 3 and re-submit?
> 
> Thanks and best regards,
> Michael
> 
> > 
> > 
> > Heiko
> > 
> > 
> >> Option 2 is not really feasible, we would almost definitely break
> >> something existent.
> >>
> >> Option 3 is feasible, of course. However, I would sort the nodes
> >> alphabetically (u2phy*, then uart*, then usb*). Works for me as well,
> >> although it is not that nice IMHO.
> >>
> >> Since many boards with the RK3566 and RK3568 will pop up in near future
> >> we should do the change right now (if we want to do it), as of course
> >> all the board files need to be changed. Therefore I wanted to bring this
> >> matter up now. Let's agree on something and move on.
> >>
> >> Best regards,
> >> Michael
> >>
> > 
> > 
> > 
> > 
> 

Re: [PATCH 1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles

[Heiko Stuebner]

On Thu, 27 Jan 2022 20:04:55 +0100, Michael Riesch wrote:
> All nodes and handles related to USB have the prefix usb or usb2,
> whereas the phy handles are prefixed with u2phy. Rename for
> consistency reasons and to facilitate sorting.
> 
> This patch also updates the handles in the only board file that
> uses them (rk3566-quartz64-a.dts).

Applied, thanks!

[1/2] arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles
[2/2] arm64: dts: rockchip: add usb2 support to rk3568-evb1-v10

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>