diff mbox series

[PATCHv3,2/5] ARM: dts: exynos: Add missing usbdrd3 suspend clk

Message ID 20200310194854.831-3-linux.amoon@gmail.com (mailing list archive)
State Not Applicable, archived
Headers show
Series Add support for FSYS power domain and enable suspend clk for Exynos542x SoC | expand

Commit Message

Anand Moon March 10, 2020, 7:48 p.m. UTC
Add new compatible strings for USBDRD3 for adding missing
suspend clk, exynos5422 usbdrd3 support two clk USBD300 and
SCLK_USBD300, so add missing suspemd_clk for Exynos542x DWC3 nodes.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
fix the commit message
---
 arch/arm/boot/dts/exynos5410.dtsi | 8 ++++----
 arch/arm/boot/dts/exynos5420.dtsi | 8 ++++----
 arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

Comments

Anand Moon March 14, 2020, 1:32 p.m. UTC | #1
Hi Krzysztof,

On Wed, 11 Mar 2020 at 01:19, Anand Moon <linux.amoon@gmail.com> wrote:
>
> Add new compatible strings for USBDRD3 for adding missing
> suspend clk, exynos5422 usbdrd3 support two clk USBD300 and
> SCLK_USBD300, so add missing suspemd_clk for Exynos542x DWC3 nodes.
>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>

My assumption based on the FSYS clock source diagram below was bit wrong.
[0] https://imgur.com/gallery/zAiBoyh

And again re-looking into the driver source code, it turn out their
are *6 clock*
Here is the correct mapping as per the Exynos5420 clock driver.

USB-(phy@12100000)
|___________________CLK_SCLK_USBD300
|___________________CLK_SCLK_USBPHY300

USB-(phy@12500000)
|___________________CLK_SCLK_USBD301
|___________________CLK_SCLK_USBPHY301

USB-(dwc3@12000000)
|___________________CLK_USBD300
USB-(dwc3@12400000)
|___________________CLK_USBD301

Note: As per Exynos 5422 user manual, There are some more USB CLK
configuration missing in GATE_IP_FSYS. So we could enable another dwc3 clk,
If needed I would like too add this missing clk code and enable this
clk for dwc3 driver.

For some reason we already use CLK_USBD300 and CLK_USBD301
for PHY nodes, which lead to this confusion. So we need to update PHY clock
CLK_USBD300 with CLK_SCLK_USBD300 and clock CLK_USBD301 with CLK_SCLK_USBD301.

Please share your thought on linking PHY nodes above and add new DWC3 clock
and enable this clock.

-Anand

> ---
> fix the commit message
> ---
>  arch/arm/boot/dts/exynos5410.dtsi | 8 ++++----
>  arch/arm/boot/dts/exynos5420.dtsi | 8 ++++----
>  arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
>  3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
> index 2eab80bf5f3a..19845dcd528f 100644
> --- a/arch/arm/boot/dts/exynos5410.dtsi
> +++ b/arch/arm/boot/dts/exynos5410.dtsi
> @@ -396,8 +396,8 @@ &trng {
>  };
>
>  &usbdrd3_0 {
> -       clocks = <&clock CLK_USBD300>;
> -       clock-names = "usbdrd30";
> +       clocks = <&clock CLK_USBD300>, <&clock CLK_SCLK_USBD300>;
> +       clock-names = "usbdrd30", "usbdrd30_susp_clk";
>  };
>
>  &usbdrd_phy0 {
> @@ -407,8 +407,8 @@ &usbdrd_phy0 {
>  };
>
>  &usbdrd3_1 {
> -       clocks = <&clock CLK_USBD301>;
> -       clock-names = "usbdrd30";
> +       clocks = <&clock CLK_USBD301>, <&clock CLK_SCLK_USBD301>;
> +       clock-names = "usbdrd30", "usbdrd30_susp_clk";
>  };
>
>  &usbdrd_dwc3_1 {
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index b672080e7469..bd505256a223 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -1372,8 +1372,8 @@ &trng {
>  };
>
>  &usbdrd3_0 {
> -       clocks = <&clock CLK_USBD300>;
> -       clock-names = "usbdrd30";
> +       clocks = <&clock CLK_USBD300>, <&clock CLK_SCLK_USBD300>;
> +       clock-names = "usbdrd30", "usbdrd30_susp_clk";
>  };
>
>  &usbdrd_phy0 {
> @@ -1383,8 +1383,8 @@ &usbdrd_phy0 {
>  };
>
>  &usbdrd3_1 {
> -       clocks = <&clock CLK_USBD301>;
> -       clock-names = "usbdrd30";
> +       clocks = <&clock CLK_USBD301>, <&clock CLK_SCLK_USBD301>;
> +       clock-names = "usbdrd30", "usbdrd30_susp_clk";
>  };
>
>  &usbdrd_dwc3_1 {
> diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi
> index 8aa5117e58ce..0aac6255de5d 100644
> --- a/arch/arm/boot/dts/exynos54xx.dtsi
> +++ b/arch/arm/boot/dts/exynos54xx.dtsi
> @@ -143,7 +143,7 @@ hsi2c_7: i2c@12cd0000 {
>                 };
>
>                 usbdrd3_0: usb3-0 {
> -                       compatible = "samsung,exynos5250-dwusb3";
> +                       compatible = "samsung,exynos5420-dwusb3";
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges;
> @@ -165,7 +165,7 @@ usbdrd_phy0: phy@12100000 {
>                 };
>
>                 usbdrd3_1: usb3-1 {
> -                       compatible = "samsung,exynos5250-dwusb3";
> +                       compatible = "samsung,exynos5420-dwusb3";
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges;
> --
> 2.25.1
>
Krzysztof Kozlowski March 14, 2020, 6:20 p.m. UTC | #2
On Sat, Mar 14, 2020 at 07:02:33PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Wed, 11 Mar 2020 at 01:19, Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Add new compatible strings for USBDRD3 for adding missing
> > suspend clk, exynos5422 usbdrd3 support two clk USBD300 and
> > SCLK_USBD300, so add missing suspemd_clk for Exynos542x DWC3 nodes.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> 
> My assumption based on the FSYS clock source diagram below was bit wrong.
> [0] https://imgur.com/gallery/zAiBoyh
> 
> And again re-looking into the driver source code, it turn out their
> are *6 clock*
> Here is the correct mapping as per the Exynos5420 clock driver.
> 
> USB-(phy@12100000)
> |___________________CLK_SCLK_USBD300
> |___________________CLK_SCLK_USBPHY300
> 
> USB-(phy@12500000)
> |___________________CLK_SCLK_USBD301
> |___________________CLK_SCLK_USBPHY301
> 
> USB-(dwc3@12000000)
> |___________________CLK_USBD300
> USB-(dwc3@12400000)
> |___________________CLK_USBD301
> 
> Note: As per Exynos 5422 user manual, There are some more USB CLK
> configuration missing in GATE_IP_FSYS. So we could enable another dwc3 clk,
> If needed I would like too add this missing clk code and enable this
> clk for dwc3 driver.
> 
> For some reason we already use CLK_USBD300 and CLK_USBD301
> for PHY nodes, which lead to this confusion. So we need to update PHY clock
> CLK_USBD300 with CLK_SCLK_USBD300 and clock CLK_USBD301 with CLK_SCLK_USBD301.
> 
> Please share your thought on linking PHY nodes above and add new DWC3 clock
> and enable this clock.

The real clock topology of Exynos5422 is not properly reflected in the
kernel. However cleaning this up is quite big task.


Best regards,
Krzysztof
Anand Moon March 15, 2020, 9:46 a.m. UTC | #3
Hi Krzysztof,

On Sat, 14 Mar 2020 at 23:50, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Sat, Mar 14, 2020 at 07:02:33PM +0530, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > On Wed, 11 Mar 2020 at 01:19, Anand Moon <linux.amoon@gmail.com> wrote:
> > >
> > > Add new compatible strings for USBDRD3 for adding missing
> > > suspend clk, exynos5422 usbdrd3 support two clk USBD300 and
> > > SCLK_USBD300, so add missing suspemd_clk for Exynos542x DWC3 nodes.
> > >
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >
> > My assumption based on the FSYS clock source diagram below was bit wrong.
> > [0] https://imgur.com/gallery/zAiBoyh
> >
> > And again re-looking into the driver source code, it turn out their
> > are *6 clock*
> > Here is the correct mapping as per the Exynos5420 clock driver.
> >
> > USB-(phy@12100000)
> > |___________________CLK_SCLK_USBD300
> > |___________________CLK_SCLK_USBPHY300
> >
> > USB-(phy@12500000)
> > |___________________CLK_SCLK_USBD301
> > |___________________CLK_SCLK_USBPHY301
> >
> > USB-(dwc3@12000000)
> > |___________________CLK_USBD300
> > USB-(dwc3@12400000)
> > |___________________CLK_USBD301
> >
> > Note: As per Exynos 5422 user manual, There are some more USB CLK
> > configuration missing in GATE_IP_FSYS. So we could enable another dwc3 clk,
> > If needed I would like too add this missing clk code and enable this
> > clk for dwc3 driver.
> >
> > For some reason we already use CLK_USBD300 and CLK_USBD301
> > for PHY nodes, which lead to this confusion. So we need to update PHY clock
> > CLK_USBD300 with CLK_SCLK_USBD300 and clock CLK_USBD301 with CLK_SCLK_USBD301.
> >
> > Please share your thought on linking PHY nodes above and add new DWC3 clock
> > and enable this clock.
>
> The real clock topology of Exynos5422 is not properly reflected in the
> kernel. However cleaning this up is quite big task.
>
>
> Best regards,
> Krzysztof
>

I would like to fix all my patches with new finding and submit them
once again for review.

-Anand
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
index 2eab80bf5f3a..19845dcd528f 100644
--- a/arch/arm/boot/dts/exynos5410.dtsi
+++ b/arch/arm/boot/dts/exynos5410.dtsi
@@ -396,8 +396,8 @@  &trng {
 };
 
 &usbdrd3_0 {
-	clocks = <&clock CLK_USBD300>;
-	clock-names = "usbdrd30";
+	clocks = <&clock CLK_USBD300>, <&clock CLK_SCLK_USBD300>;
+	clock-names = "usbdrd30", "usbdrd30_susp_clk";
 };
 
 &usbdrd_phy0 {
@@ -407,8 +407,8 @@  &usbdrd_phy0 {
 };
 
 &usbdrd3_1 {
-	clocks = <&clock CLK_USBD301>;
-	clock-names = "usbdrd30";
+	clocks = <&clock CLK_USBD301>, <&clock CLK_SCLK_USBD301>;
+	clock-names = "usbdrd30", "usbdrd30_susp_clk";
 };
 
 &usbdrd_dwc3_1 {
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index b672080e7469..bd505256a223 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -1372,8 +1372,8 @@  &trng {
 };
 
 &usbdrd3_0 {
-	clocks = <&clock CLK_USBD300>;
-	clock-names = "usbdrd30";
+	clocks = <&clock CLK_USBD300>, <&clock CLK_SCLK_USBD300>;
+	clock-names = "usbdrd30", "usbdrd30_susp_clk";
 };
 
 &usbdrd_phy0 {
@@ -1383,8 +1383,8 @@  &usbdrd_phy0 {
 };
 
 &usbdrd3_1 {
-	clocks = <&clock CLK_USBD301>;
-	clock-names = "usbdrd30";
+	clocks = <&clock CLK_USBD301>, <&clock CLK_SCLK_USBD301>;
+	clock-names = "usbdrd30", "usbdrd30_susp_clk";
 };
 
 &usbdrd_dwc3_1 {
diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi
index 8aa5117e58ce..0aac6255de5d 100644
--- a/arch/arm/boot/dts/exynos54xx.dtsi
+++ b/arch/arm/boot/dts/exynos54xx.dtsi
@@ -143,7 +143,7 @@  hsi2c_7: i2c@12cd0000 {
 		};
 
 		usbdrd3_0: usb3-0 {
-			compatible = "samsung,exynos5250-dwusb3";
+			compatible = "samsung,exynos5420-dwusb3";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
@@ -165,7 +165,7 @@  usbdrd_phy0: phy@12100000 {
 		};
 
 		usbdrd3_1: usb3-1 {
-			compatible = "samsung,exynos5250-dwusb3";
+			compatible = "samsung,exynos5420-dwusb3";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;