linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
@ 2022-04-09  7:51 Frank Wunderlich
  2022-04-09 10:14 ` Aw: " Frank Wunderlich
  0 siblings, 1 reply; 15+ messages in thread
From: Frank Wunderlich @ 2022-04-09  7:51 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
	Heiko Stuebner, Peter Geis, Michael Riesch, devicetree,
	linux-arm-kernel, linux-kernel

From: Frank Wunderlich <frank-w@public-files.de>

after these 2 commit different clock names are needed compared to 5.17

commit 5114c3ee2487 ("usb: dwc3: Calculate REFCLKPER based on reference clock")
commit 33fb697ec7e5 ("usb: dwc3: Get clocks individually")

change them in new rk356x usb support

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
this is a fix for this series not yet applied
https://patchwork.kernel.org/project/linux-rockchip/list/?series=630470
@peter
after testing you can squash it into your series and add a co-developed or similar
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 55e6dcb948cc..6dfe54e53be1 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -264,8 +264,8 @@ usb_host0_xhci: usb@fcc00000 {
 		interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru CLK_USB3OTG0_REF>, <&cru CLK_USB3OTG0_SUSPEND>,
 			 <&cru ACLK_USB3OTG0>;
-		clock-names = "ref_clk", "suspend_clk",
-			      "bus_clk";
+		clock-names = "ref", "suspend_clk",
+			      "bus_early";
 		dr_mode = "host";
 		phy_type = "utmi_wide";
 		power-domains = <&power RK3568_PD_PIPE>;
@@ -280,8 +280,8 @@ usb_host1_xhci: usb@fd000000 {
 		interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru CLK_USB3OTG1_REF>, <&cru CLK_USB3OTG1_SUSPEND>,
 			 <&cru ACLK_USB3OTG1>;
-		clock-names = "ref_clk", "suspend_clk",
-			      "bus_clk";
+		clock-names = "ref", "suspend_clk",
+			      "bus_early";
 		dr_mode = "host";
 		phys = <&usb2phy0_host>, <&combphy1 PHY_TYPE_USB3>;
 		phy-names = "usb2-phy", "usb3-phy";
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Aw: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09  7:51 [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb Frank Wunderlich
@ 2022-04-09 10:14 ` Frank Wunderlich
  2022-04-09 10:23   ` Heiko Stuebner
  0 siblings, 1 reply; 15+ messages in thread
From: Frank Wunderlich @ 2022-04-09 10:14 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: linux-rockchip, Rob Herring, Krzysztof Kozlowski, Heiko Stuebner,
	Peter Geis, Michael Riesch, devicetree, linux-arm-kernel,
	linux-kernel

Hi,

> Gesendet: Samstag, 09. April 2022 um 09:51 Uhr
> Von: "Frank Wunderlich" <linux@fw-web.de>

> -		clock-names = "ref_clk", "suspend_clk",
> -			      "bus_clk";
> +		clock-names = "ref", "suspend_clk",
> +			      "bus_early";
>  		dr_mode = "host";
>  		phy_type = "utmi_wide";
>  		power-domains = <&power RK3568_PD_PIPE>;
> @@ -280,8 +280,8 @@ usb_host1_xhci: usb@fd000000 {
>  		interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&cru CLK_USB3OTG1_REF>, <&cru CLK_USB3OTG1_SUSPEND>,
>  			 <&cru ACLK_USB3OTG1>;
> -		clock-names = "ref_clk", "suspend_clk",
> -			      "bus_clk";
> +		clock-names = "ref", "suspend_clk",
> +			      "bus_early";
>  		dr_mode = "host";

this is the patch breaking it:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=33fb697ec7e58c4f9b6a68d2786441189cd2df92

suspend clock needs to be renamed too from "suspend_clk" to "suspend"

else i get this error on poweroff

xhci-hcd xhci-hcd.1.auto: Host halt failed, -110

regards Frank

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Aw: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 10:14 ` Aw: " Frank Wunderlich
@ 2022-04-09 10:23   ` Heiko Stuebner
  2022-04-09 10:32     ` Aw: " Frank Wunderlich
  0 siblings, 1 reply; 15+ messages in thread
From: Heiko Stuebner @ 2022-04-09 10:23 UTC (permalink / raw)
  To: Frank Wunderlich, Frank Wunderlich
  Cc: linux-rockchip, Rob Herring, Krzysztof Kozlowski, Peter Geis,
	Michael Riesch, devicetree, linux-arm-kernel, linux-kernel

Am Samstag, 9. April 2022, 12:14:28 CEST schrieb Frank Wunderlich:
> Hi,
> 
> > Gesendet: Samstag, 09. April 2022 um 09:51 Uhr
> > Von: "Frank Wunderlich" <linux@fw-web.de>
> 
> > -		clock-names = "ref_clk", "suspend_clk",
> > -			      "bus_clk";
> > +		clock-names = "ref", "suspend_clk",
> > +			      "bus_early";
> >  		dr_mode = "host";
> >  		phy_type = "utmi_wide";
> >  		power-domains = <&power RK3568_PD_PIPE>;
> > @@ -280,8 +280,8 @@ usb_host1_xhci: usb@fd000000 {
> >  		interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&cru CLK_USB3OTG1_REF>, <&cru CLK_USB3OTG1_SUSPEND>,
> >  			 <&cru ACLK_USB3OTG1>;
> > -		clock-names = "ref_clk", "suspend_clk",
> > -			      "bus_clk";
> > +		clock-names = "ref", "suspend_clk",
> > +			      "bus_early";
> >  		dr_mode = "host";
> 
> this is the patch breaking it:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=33fb697ec7e58c4f9b6a68d2786441189cd2df92
> 
> suspend clock needs to be renamed too from "suspend_clk" to "suspend"
> 
> else i get this error on poweroff
> 
> xhci-hcd xhci-hcd.1.auto: Host halt failed, -110
> 
> regards Frank

ok, so do you want to send a v2 including that change?
Alternatively I can also add this change when applying.

Also for educational purposes, the format for referencing a commit
you're fixing would be

Fixes: 33fb697ec7e5 ("usb: dwc3: Get clocks individually")
Signed-off-by: ....


Heiko



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Aw: Re:  [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 10:23   ` Heiko Stuebner
@ 2022-04-09 10:32     ` Frank Wunderlich
  2022-04-09 10:40       ` Dan Johansen
  0 siblings, 1 reply; 15+ messages in thread
From: Frank Wunderlich @ 2022-04-09 10:32 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Frank Wunderlich, linux-rockchip, Rob Herring,
	Krzysztof Kozlowski, Peter Geis, Michael Riesch, devicetree,
	linux-arm-kernel, linux-kernel

> Gesendet: Samstag, 09. April 2022 um 12:23 Uhr
> Von: "Heiko Stuebner" <heiko@sntech.de>


> ok, so do you want to send a v2 including that change?
> Alternatively I can also add this change when applying.

imho the best way will be that peter includes my patch in his

"arm64: dts: rockchip: add rk356x dwc3 usb3 nodes"

https://patchwork.kernel.org/project/linux-rockchip/patch/20220408151237.3165046-4-pgwipeout@gmail.com/

i just posted the fix for those who want to test his series on 5.18 including himself.

but of course if this is not the right way, i post a v2.

> Also for educational purposes, the format for referencing a commit
> you're fixing would be
>
> Fixes: 33fb697ec7e5 ("usb: dwc3: Get clocks individually")
> Signed-off-by: ....

as the patch not really broke current mainline state, i thought Fixes-tag is not right.
Imho only the rk356x-usb-patch is not compatible with 5.18 due to this change, but this is not applied yet.

regards Frank

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Aw: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 10:32     ` Aw: " Frank Wunderlich
@ 2022-04-09 10:40       ` Dan Johansen
  2022-04-09 10:57         ` Aw: " Frank Wunderlich
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Johansen @ 2022-04-09 10:40 UTC (permalink / raw)
  To: Frank Wunderlich, Heiko Stuebner
  Cc: Frank Wunderlich, linux-rockchip, Rob Herring,
	Krzysztof Kozlowski, Peter Geis, Michael Riesch, devicetree,
	linux-arm-kernel, linux-kernel

Den 09.04.2022 kl. 12.32 skrev Frank Wunderlich:
>> Gesendet: Samstag, 09. April 2022 um 12:23 Uhr
>> Von: "Heiko Stuebner" <heiko@sntech.de>
>
>> ok, so do you want to send a v2 including that change?
>> Alternatively I can also add this change when applying.
> imho the best way will be that peter includes my patch in his
>
> "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes"
>
> https://patchwork.kernel.org/project/linux-rockchip/patch/20220408151237.3165046-4-pgwipeout@gmail.com/
>
> i just posted the fix for those who want to test his series on 5.18 including himself.
So the issue is only with usb 3 ports, not usb 2 ports?
>
> but of course if this is not the right way, i post a v2.
>
>> Also for educational purposes, the format for referencing a commit
>> you're fixing would be
>>
>> Fixes: 33fb697ec7e5 ("usb: dwc3: Get clocks individually")
>> Signed-off-by: ....
> as the patch not really broke current mainline state, i thought Fixes-tag is not right.
> Imho only the rk356x-usb-patch is not compatible with 5.18 due to this change, but this is not applied yet.
>
> regards Frank
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
-- 
Kind regards
*Dan Johansen*
Project lead of the *Manjaro ARM* project
Manjaro-ARM <https://manjaro.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Aw: Re:  Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 10:40       ` Dan Johansen
@ 2022-04-09 10:57         ` Frank Wunderlich
  2022-04-09 11:01           ` Heiko Stuebner
  0 siblings, 1 reply; 15+ messages in thread
From: Frank Wunderlich @ 2022-04-09 10:57 UTC (permalink / raw)
  To: Dan Johansen
  Cc: Heiko Stuebner, Frank Wunderlich, linux-rockchip, Rob Herring,
	Krzysztof Kozlowski, Peter Geis, Michael Riesch, devicetree,
	linux-arm-kernel, linux-kernel

Hi
> Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> Von: "Dan Johansen" <strit@manjaro.org>

> So the issue is only with usb 3 ports, not usb 2 ports?

my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.

afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.

regards Frank

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Aw: Re:  Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 10:57         ` Aw: " Frank Wunderlich
@ 2022-04-09 11:01           ` Heiko Stuebner
  2022-04-09 11:13             ` Aw: " Frank Wunderlich
  2022-04-09 11:14             ` Aw: " Peter Geis
  0 siblings, 2 replies; 15+ messages in thread
From: Heiko Stuebner @ 2022-04-09 11:01 UTC (permalink / raw)
  To: Dan Johansen, Frank Wunderlich
  Cc: Frank Wunderlich, linux-rockchip, Rob Herring,
	Krzysztof Kozlowski, Peter Geis, Michael Riesch, devicetree,
	linux-arm-kernel, linux-kernel

Am Samstag, 9. April 2022, 12:57:39 CEST schrieb Frank Wunderlich:
> Hi
> > Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> > Von: "Dan Johansen" <strit@manjaro.org>
> 
> > So the issue is only with usb 3 ports, not usb 2 ports?
> 
> my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.
> 
> afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.

As far as I understand the issue now after checking the code, this
patch actually fixes the usb3 series from Peter, right?

I.e. the usb-nodes that are fixed in this patch are not yet present
in the main rk356x dtsi and only get added in
"arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]

As we don't want to add broken changes, this fix should squashed
into a next version of the patch adding the nodes.


Heiko

[0] https://lore.kernel.org/r/20220408151237.3165046-4-pgwipeout@gmail.com



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Aw: Re:  Re:  Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 11:01           ` Heiko Stuebner
@ 2022-04-09 11:13             ` Frank Wunderlich
  2022-04-09 11:14             ` Aw: " Peter Geis
  1 sibling, 0 replies; 15+ messages in thread
From: Frank Wunderlich @ 2022-04-09 11:13 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Dan Johansen, Frank Wunderlich, linux-rockchip, Rob Herring,
	Krzysztof Kozlowski, Peter Geis, Michael Riesch, devicetree,
	linux-arm-kernel, linux-kernel

> Gesendet: Samstag, 09. April 2022 um 13:01 Uhr
> Von: "Heiko Stuebner" <heiko@sntech.de>

> As far as I understand the issue now after checking the code, this
> patch actually fixes the usb3 series from Peter, right?

right, due to the change the patches from 5.17 were no more directly compatible with 5.18.
for my board without having standalone usb2-ports it breaks both as usb2 is passed
through xhci controller.

> I.e. the usb-nodes that are fixed in this patch are not yet present
> in the main rk356x dtsi and only get added in
> "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]
>
> As we don't want to add broken changes, this fix should squashed
> into a next version of the patch adding the nodes.

right, that was my intention, but do not forget the suspend_clk change i've mentioned in [1]
but to leave all know that this bug is known and how to solve it i've posted official patch but
without fixes-tag as the "breaking" commit does not really break...more likely introduce a
incompatibility for the not yet applied patchset.

> [0] https://lore.kernel.org/r/20220408151237.3165046-4-pgwipeout@gmail.com

[1] https://patchwork.kernel.org/comment/24809714/

regards Frank

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Aw: Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 11:01           ` Heiko Stuebner
  2022-04-09 11:13             ` Aw: " Frank Wunderlich
@ 2022-04-09 11:14             ` Peter Geis
  2022-04-09 11:30               ` Peter Geis
  1 sibling, 1 reply; 15+ messages in thread
From: Peter Geis @ 2022-04-09 11:14 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Dan Johansen, Frank Wunderlich, Frank Wunderlich,
	open list:ARM/Rockchip SoC...,
	Rob Herring, Krzysztof Kozlowski, Michael Riesch, devicetree,
	arm-mail-list, Linux Kernel Mailing List

On Sat, Apr 9, 2022 at 7:01 AM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Am Samstag, 9. April 2022, 12:57:39 CEST schrieb Frank Wunderlich:
> > Hi
> > > Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> > > Von: "Dan Johansen" <strit@manjaro.org>
> >
> > > So the issue is only with usb 3 ports, not usb 2 ports?
> >
> > my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.
> >
> > afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.

Good Morning,

>
> As far as I understand the issue now after checking the code, this
> patch actually fixes the usb3 series from Peter, right?
>
> I.e. the usb-nodes that are fixed in this patch are not yet present
> in the main rk356x dtsi and only get added in
> "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]
>
> As we don't want to add broken changes, this fix should squashed
> into a next version of the patch adding the nodes.

Thank you for reporting this, I will squash this fix in and add your signed-off.

However the offending patch is in fact the clock separation patch, and
it breaks backwards compatibility with the rk3328 dtsi which is why my
series also is broken.

The rockchip,dwc3.yaml needs to be fixed to align with the
snps,dwc3.yaml, and both the rk3328 and rk3399 clock names updated.
Also the offending clock separation patch needs a fix to grab the old
clock names for rk3328 backwards compatibility to be retained.

This might also be a good time to look into moving rk3399 to the core
dwc3 driver?

This is a delightful mess.

>
>
> Heiko
>
> [0] https://lore.kernel.org/r/20220408151237.3165046-4-pgwipeout@gmail.com
>
>

Very Respectfully,
Peter Geis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Aw: Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 11:14             ` Aw: " Peter Geis
@ 2022-04-09 11:30               ` Peter Geis
  2022-04-09 11:35                 ` Heiko Stuebner
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Geis @ 2022-04-09 11:30 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Dan Johansen, Frank Wunderlich, Frank Wunderlich,
	open list:ARM/Rockchip SoC...,
	Rob Herring, Krzysztof Kozlowski, Michael Riesch, devicetree,
	arm-mail-list, Linux Kernel Mailing List

On Sat, Apr 9, 2022 at 7:14 AM Peter Geis <pgwipeout@gmail.com> wrote:
>
> On Sat, Apr 9, 2022 at 7:01 AM Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > Am Samstag, 9. April 2022, 12:57:39 CEST schrieb Frank Wunderlich:
> > > Hi
> > > > Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> > > > Von: "Dan Johansen" <strit@manjaro.org>
> > >
> > > > So the issue is only with usb 3 ports, not usb 2 ports?
> > >
> > > my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.
> > >
> > > afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.
>
> Good Morning,
>
> >
> > As far as I understand the issue now after checking the code, this
> > patch actually fixes the usb3 series from Peter, right?
> >
> > I.e. the usb-nodes that are fixed in this patch are not yet present
> > in the main rk356x dtsi and only get added in
> > "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]
> >
> > As we don't want to add broken changes, this fix should squashed
> > into a next version of the patch adding the nodes.
>
> Thank you for reporting this, I will squash this fix in and add your signed-off.
>
> However the offending patch is in fact the clock separation patch, and
> it breaks backwards compatibility with the rk3328 dtsi which is why my
> series also is broken.
>
> The rockchip,dwc3.yaml needs to be fixed to align with the
> snps,dwc3.yaml, and both the rk3328 and rk3399 clock names updated.
> Also the offending clock separation patch needs a fix to grab the old
> clock names for rk3328 backwards compatibility to be retained.
>
> This might also be a good time to look into moving rk3399 to the core
> dwc3 driver?
>
> This is a delightful mess.

In the idea of getting this series to land, if all parties agree, I'll
submit a patch that fixes the clock separation patch with this series
and leave the naming as is for now.
The renaming of clocks and alignment of everything can be addressed in
a future series once discussion on how best to handle it has happened.

Do you concur with this?

>
> >
> >
> > Heiko
> >
> > [0] https://lore.kernel.org/r/20220408151237.3165046-4-pgwipeout@gmail.com
> >
> >
>
> Very Respectfully,
> Peter Geis

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Aw: Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 11:30               ` Peter Geis
@ 2022-04-09 11:35                 ` Heiko Stuebner
  2022-04-09 11:51                   ` Peter Geis
  2022-04-09 11:56                   ` Aw: " Frank Wunderlich
  0 siblings, 2 replies; 15+ messages in thread
From: Heiko Stuebner @ 2022-04-09 11:35 UTC (permalink / raw)
  To: Peter Geis
  Cc: Dan Johansen, Frank Wunderlich, Frank Wunderlich,
	open list:ARM/Rockchip SoC...,
	Rob Herring, Krzysztof Kozlowski, Michael Riesch, devicetree,
	arm-mail-list, Linux Kernel Mailing List

Am Samstag, 9. April 2022, 13:30:44 CEST schrieb Peter Geis:
> On Sat, Apr 9, 2022 at 7:14 AM Peter Geis <pgwipeout@gmail.com> wrote:
> >
> > On Sat, Apr 9, 2022 at 7:01 AM Heiko Stuebner <heiko@sntech.de> wrote:
> > >
> > > Am Samstag, 9. April 2022, 12:57:39 CEST schrieb Frank Wunderlich:
> > > > Hi
> > > > > Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> > > > > Von: "Dan Johansen" <strit@manjaro.org>
> > > >
> > > > > So the issue is only with usb 3 ports, not usb 2 ports?
> > > >
> > > > my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.
> > > >
> > > > afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.
> >
> > Good Morning,
> >
> > >
> > > As far as I understand the issue now after checking the code, this
> > > patch actually fixes the usb3 series from Peter, right?
> > >
> > > I.e. the usb-nodes that are fixed in this patch are not yet present
> > > in the main rk356x dtsi and only get added in
> > > "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]
> > >
> > > As we don't want to add broken changes, this fix should squashed
> > > into a next version of the patch adding the nodes.
> >
> > Thank you for reporting this, I will squash this fix in and add your signed-off.
> >
> > However the offending patch is in fact the clock separation patch, and
> > it breaks backwards compatibility with the rk3328 dtsi which is why my
> > series also is broken.
> >
> > The rockchip,dwc3.yaml needs to be fixed to align with the
> > snps,dwc3.yaml, and both the rk3328 and rk3399 clock names updated.
> > Also the offending clock separation patch needs a fix to grab the old
> > clock names for rk3328 backwards compatibility to be retained.
> >
> > This might also be a good time to look into moving rk3399 to the core
> > dwc3 driver?
> >
> > This is a delightful mess.
> 
> In the idea of getting this series to land, if all parties agree, I'll
> submit a patch that fixes the clock separation patch with this series
> and leave the naming as is for now.
> The renaming of clocks and alignment of everything can be addressed in
> a future series once discussion on how best to handle it has happened.
> 
> Do you concur with this?

I'm not sure about that ... i.e. adding known-broken changes
(for the rk356x) feels somewhat wrong to me.

Heiko



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Aw: Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 11:35                 ` Heiko Stuebner
@ 2022-04-09 11:51                   ` Peter Geis
  2022-04-09 11:56                   ` Aw: " Frank Wunderlich
  1 sibling, 0 replies; 15+ messages in thread
From: Peter Geis @ 2022-04-09 11:51 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Dan Johansen, Frank Wunderlich, Frank Wunderlich,
	open list:ARM/Rockchip SoC...,
	Rob Herring, Krzysztof Kozlowski, Michael Riesch, devicetree,
	arm-mail-list, Linux Kernel Mailing List

On Sat, Apr 9, 2022 at 7:35 AM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Am Samstag, 9. April 2022, 13:30:44 CEST schrieb Peter Geis:
> > On Sat, Apr 9, 2022 at 7:14 AM Peter Geis <pgwipeout@gmail.com> wrote:
> > >
> > > On Sat, Apr 9, 2022 at 7:01 AM Heiko Stuebner <heiko@sntech.de> wrote:
> > > >
> > > > Am Samstag, 9. April 2022, 12:57:39 CEST schrieb Frank Wunderlich:
> > > > > Hi
> > > > > > Gesendet: Samstag, 09. April 2022 um 12:40 Uhr
> > > > > > Von: "Dan Johansen" <strit@manjaro.org>
> > > > >
> > > > > > So the issue is only with usb 3 ports, not usb 2 ports?
> > > > >
> > > > > my board has no standalone usb2-ports. usb2 is integrated into the usb3 ports (dual phy). here both were not working.
> > > > >
> > > > > afaik rk3566 has standalone usb2 ports that may not be broken, but i have no such board for testing.
> > >
> > > Good Morning,
> > >
> > > >
> > > > As far as I understand the issue now after checking the code, this
> > > > patch actually fixes the usb3 series from Peter, right?
> > > >
> > > > I.e. the usb-nodes that are fixed in this patch are not yet present
> > > > in the main rk356x dtsi and only get added in
> > > > "arm64: dts: rockchip: add rk356x dwc3 usb3 nodes" [0]
> > > >
> > > > As we don't want to add broken changes, this fix should squashed
> > > > into a next version of the patch adding the nodes.
> > >
> > > Thank you for reporting this, I will squash this fix in and add your signed-off.
> > >
> > > However the offending patch is in fact the clock separation patch, and
> > > it breaks backwards compatibility with the rk3328 dtsi which is why my
> > > series also is broken.
> > >
> > > The rockchip,dwc3.yaml needs to be fixed to align with the
> > > snps,dwc3.yaml, and both the rk3328 and rk3399 clock names updated.
> > > Also the offending clock separation patch needs a fix to grab the old
> > > clock names for rk3328 backwards compatibility to be retained.
> > >
> > > This might also be a good time to look into moving rk3399 to the core
> > > dwc3 driver?
> > >
> > > This is a delightful mess.
> >
> > In the idea of getting this series to land, if all parties agree, I'll
> > submit a patch that fixes the clock separation patch with this series
> > and leave the naming as is for now.
> > The renaming of clocks and alignment of everything can be addressed in
> > a future series once discussion on how best to handle it has happened.
> >
> > Do you concur with this?
>
> I'm not sure about that ... i.e. adding known-broken changes
> (for the rk356x) feels somewhat wrong to me.

My series as it is complies with the current dt-binding for Rockchip.
It just so happens the dt-binding describes two different devices that
are handled by two different drivers, and the binding itself was
wrong.
This becomes a whole lot more intrusive if we decide to do this now,
and the offending change still needs to be fixed if we want to retain
backwards compatibility with rk3328.dtsi

>
> Heiko
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Aw: Re:  Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 11:35                 ` Heiko Stuebner
  2022-04-09 11:51                   ` Peter Geis
@ 2022-04-09 11:56                   ` Frank Wunderlich
  2022-04-09 15:26                     ` Peter Geis
  1 sibling, 1 reply; 15+ messages in thread
From: Frank Wunderlich @ 2022-04-09 11:56 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Peter Geis, Dan Johansen, Frank Wunderlich,
	open list:ARM/Rockchip SoC...,
	Rob Herring, Krzysztof Kozlowski, Michael Riesch, devicetree,
	arm-mail-list, Linux Kernel Mailing List

Hi,

so to not break the binding and other boards the right Patch should be like this

+++ b/drivers/usb/dwc3/core.c
@@ -1691,17 +1691,17 @@ static int dwc3_probe(struct platform_device *pdev)
                 * Clocks are optional, but new DT platforms should support all
                 * clocks as required by the DT-binding.
                 */
-               dwc->bus_clk = devm_clk_get_optional(dev, "bus_early");
+               dwc->bus_clk = devm_clk_get_optional(dev, "bus_clk");
                if (IS_ERR(dwc->bus_clk))
                        return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
                                             "could not get bus clock\n");

-               dwc->ref_clk = devm_clk_get_optional(dev, "ref");
+               dwc->ref_clk = devm_clk_get_optional(dev, "ref_clk");
                if (IS_ERR(dwc->ref_clk))
                        return dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
                                             "could not get ref clock\n");

-               dwc->susp_clk = devm_clk_get_optional(dev, "suspend");
+               dwc->susp_clk = devm_clk_get_optional(dev, "suspend_clk");
                if (IS_ERR(dwc->susp_clk))
                        return dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
                                             "could not get suspend clock\n");

but this needs fixing dts using the new clock names

this is a link to the series moving from bulk_clk to named clocks:

https://patchwork.kernel.org/project/linux-usb/patch/20220127200636.1456175-3-sean.anderson@seco.com/

regards Frank

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Re: Re: Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 11:56                   ` Aw: " Frank Wunderlich
@ 2022-04-09 15:26                     ` Peter Geis
  2022-04-10 16:53                       ` Heiko Stuebner
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Geis @ 2022-04-09 15:26 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Heiko Stuebner, Dan Johansen, Frank Wunderlich,
	open list:ARM/Rockchip SoC...,
	Rob Herring, Krzysztof Kozlowski, Michael Riesch, devicetree,
	arm-mail-list, Linux Kernel Mailing List

On Sat, Apr 9, 2022 at 7:56 AM Frank Wunderlich <frank-w@public-files.de> wrote:
>
> Hi,
>
> so to not break the binding and other boards the right Patch should be like this
>
> +++ b/drivers/usb/dwc3/core.c
> @@ -1691,17 +1691,17 @@ static int dwc3_probe(struct platform_device *pdev)
>                  * Clocks are optional, but new DT platforms should support all
>                  * clocks as required by the DT-binding.
>                  */
> -               dwc->bus_clk = devm_clk_get_optional(dev, "bus_early");
> +               dwc->bus_clk = devm_clk_get_optional(dev, "bus_clk");
>                 if (IS_ERR(dwc->bus_clk))
>                         return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
>                                              "could not get bus clock\n");
>
> -               dwc->ref_clk = devm_clk_get_optional(dev, "ref");
> +               dwc->ref_clk = devm_clk_get_optional(dev, "ref_clk");
>                 if (IS_ERR(dwc->ref_clk))
>                         return dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
>                                              "could not get ref clock\n");
>
> -               dwc->susp_clk = devm_clk_get_optional(dev, "suspend");
> +               dwc->susp_clk = devm_clk_get_optional(dev, "suspend_clk");
>                 if (IS_ERR(dwc->susp_clk))
>                         return dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
>                                              "could not get suspend clock\n");
>
> but this needs fixing dts using the new clock names
>
> this is a link to the series moving from bulk_clk to named clocks:
>
> https://patchwork.kernel.org/project/linux-usb/patch/20220127200636.1456175-3-sean.anderson@seco.com/
>
> regards Frank

I've submitted a fix for the backwards compatibility issue.
https://patchwork.kernel.org/project/linux-rockchip/patch/20220409152116.3834354-1-pgwipeout@gmail.com/

This fix is standalone and necessary no matter which route we decide
to go with this series (and the rk3328/rk3399 support as well).
With this patch, dwc3 is functional on the rk356x as the series was
submitted, so if we decide to fix everything all at once, that is a
viable option.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb
  2022-04-09 15:26                     ` Peter Geis
@ 2022-04-10 16:53                       ` Heiko Stuebner
  0 siblings, 0 replies; 15+ messages in thread
From: Heiko Stuebner @ 2022-04-10 16:53 UTC (permalink / raw)
  To: Frank Wunderlich, Peter Geis
  Cc: Dan Johansen, Frank Wunderlich, open list:ARM/Rockchip SoC...,
	Rob Herring, Krzysztof Kozlowski, Michael Riesch, devicetree,
	arm-mail-list, Linux Kernel Mailing List

Am Samstag, 9. April 2022, 17:26:01 CEST schrieb Peter Geis:
> On Sat, Apr 9, 2022 at 7:56 AM Frank Wunderlich <frank-w@public-files.de> wrote:
> >
> > Hi,
> >
> > so to not break the binding and other boards the right Patch should be like this
> >
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -1691,17 +1691,17 @@ static int dwc3_probe(struct platform_device *pdev)
> >                  * Clocks are optional, but new DT platforms should support all
> >                  * clocks as required by the DT-binding.
> >                  */
> > -               dwc->bus_clk = devm_clk_get_optional(dev, "bus_early");
> > +               dwc->bus_clk = devm_clk_get_optional(dev, "bus_clk");
> >                 if (IS_ERR(dwc->bus_clk))
> >                         return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
> >                                              "could not get bus clock\n");
> >
> > -               dwc->ref_clk = devm_clk_get_optional(dev, "ref");
> > +               dwc->ref_clk = devm_clk_get_optional(dev, "ref_clk");
> >                 if (IS_ERR(dwc->ref_clk))
> >                         return dev_err_probe(dev, PTR_ERR(dwc->ref_clk),
> >                                              "could not get ref clock\n");
> >
> > -               dwc->susp_clk = devm_clk_get_optional(dev, "suspend");
> > +               dwc->susp_clk = devm_clk_get_optional(dev, "suspend_clk");
> >                 if (IS_ERR(dwc->susp_clk))
> >                         return dev_err_probe(dev, PTR_ERR(dwc->susp_clk),
> >                                              "could not get suspend clock\n");
> >
> > but this needs fixing dts using the new clock names
> >
> > this is a link to the series moving from bulk_clk to named clocks:
> >
> > https://patchwork.kernel.org/project/linux-usb/patch/20220127200636.1456175-3-sean.anderson@seco.com/
> >
> > regards Frank
> 
> I've submitted a fix for the backwards compatibility issue.
> https://patchwork.kernel.org/project/linux-rockchip/patch/20220409152116.3834354-1-pgwipeout@gmail.com/
> 
> This fix is standalone and necessary no matter which route we decide
> to go with this series (and the rk3328/rk3399 support as well).
> With this patch, dwc3 is functional on the rk356x as the series was
> submitted, so if we decide to fix everything all at once, that is a
> viable option.

Thanks for doing that fix.

As the usb-dt-series is actually following the rockchip,dwc3 binding,
and "just" the driver does ignore it, I've now applied the usb series
and hope for a resolution of the general problem :-)


Heiko





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-04-10 16:53 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-09  7:51 [PATCH] arm64: dts: rockchip: Fix clocks for rk356x usb Frank Wunderlich
2022-04-09 10:14 ` Aw: " Frank Wunderlich
2022-04-09 10:23   ` Heiko Stuebner
2022-04-09 10:32     ` Aw: " Frank Wunderlich
2022-04-09 10:40       ` Dan Johansen
2022-04-09 10:57         ` Aw: " Frank Wunderlich
2022-04-09 11:01           ` Heiko Stuebner
2022-04-09 11:13             ` Aw: " Frank Wunderlich
2022-04-09 11:14             ` Aw: " Peter Geis
2022-04-09 11:30               ` Peter Geis
2022-04-09 11:35                 ` Heiko Stuebner
2022-04-09 11:51                   ` Peter Geis
2022-04-09 11:56                   ` Aw: " Frank Wunderlich
2022-04-09 15:26                     ` Peter Geis
2022-04-10 16:53                       ` Heiko Stuebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).