[PATCHv3 0/5] Add support for FSYS power domain and enable suspend clk for Exynos542x SoC

[PATCHv3 0/5] Add support for FSYS power domain and enable suspend clk for Exynos542x SoC

[Anand Moon]

Series build and tested on linux next-20200306.

This patch series tries to enable FSYS power domain
for USBDRD3, PDMA and USB2.0 devices.
Some new patches is added to enable this feature.

Summary: 
# powerdebug -d
...

FSYS:
current_state: on
active_time: 236786 ms
total_idle_time: 1914 ms
Idle States:
             State            Time
             S0               1914
Devices:
         /devices/platform/soc/10010000.clock-controller/exynos5-subcmu.6.auto
         /devices/platform/soc/12130000.phy
         /devices/platform/soc/12100000.phy
         /devices/platform/soc/12500000.phy
         /devices/platform/soc/soc:amba/121a0000.pdma
         /devices/platform/soc/soc:amba/121b0000.pdma
         /devices/platform/soc/12110000.usb
         /devices/platform/soc/12120000.usb
         /devices/platform/soc/soc:usb3-0
         /devices/platform/soc/soc:usb3-1

This patch series tries to enable suspend clk using
exynos dwc3 driver, for this I have added new
compatible string "samsung,exynos5420-dwusb3"
so that we could add new suspend clk in addition
to the core clk. exynos dwc3 driver will help
enable/disable these clk.

This series PatchV2.
--Added the clk names for exynos5420 compatible.
--Added missing support for Exyno5410 SoC suspend clock.
--Update the commit message to support suspend clk usages.

---
Long time ago I tried to add suspend clk for dwc3 phy
which was wrong appoch, see below.

[0] https://lore.kernel.org/patchwork/patch/837635/
[1] https://lore.kernel.org/patchwork/patch/837636/

Previous changes V3 (It was send with wrong Patch version)
[2] https://patchwork.kernel.org/cover/11373043/

-Anand

Anand Moon (5):
  devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3
    clocks support
  ARM: dts: exynos: Add missing usbdrd3 suspend clk
  ARM: dts: exynos: Add FSYS power domain to Exynos542x USB nodes
  usb: dwc3: exynos: Add support for Exynos5422 suspend clk
  clk: samsung: exynos542x: Move FSYS subsystem clocks to its sub-CMU

 .../devicetree/bindings/usb/exynos-usb.txt    |  5 ++-
 arch/arm/boot/dts/exynos5410.dtsi             |  8 ++--
 arch/arm/boot/dts/exynos5420.dtsi             | 24 ++++++++--
 arch/arm/boot/dts/exynos54xx.dtsi             |  4 +-
 drivers/clk/samsung/clk-exynos5420.c          | 45 ++++++++++++++-----
 drivers/usb/dwc3/dwc3-exynos.c                |  9 ++++
 6 files changed, 73 insertions(+), 22 deletions(-)

-- 
2.25.1

[PATCHv3 1/5] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support

[Anand Moon]

Add the new compatible string for Exynos5422 DWC3 to support
enable/disable of core and suspend clk by DWC3 driver.
Also updated the clock names for compatible samsung,exynos5420-dwusb3.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 Documentation/devicetree/bindings/usb/exynos-usb.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index 6aae1544f240..220f729ac8eb 100644
--- a/Documentation/devicetree/bindings/usb/exynos-usb.txt
+++ b/Documentation/devicetree/bindings/usb/exynos-usb.txt
@@ -69,7 +69,9 @@ DWC3
 Required properties:
  - compatible: should be one of the following -
 	       "samsung,exynos5250-dwusb3": for USB 3.0 DWC3 controller on
-					    Exynos5250/5420.
+					    Exynos5250.
+	       "samsung,exynos5420-dwusb3": for USB 3.0 DWC3 controller on
+					    Exynos5420.
 	       "samsung,exynos5433-dwusb3": for USB 3.0 DWC3 controller on
 					    Exynos5433.
 	       "samsung,exynos7-dwusb3": for USB 3.0 DWC3 controller on Exynos7.
@@ -82,6 +84,7 @@ Required properties:
                 Following clock names shall be provided for different
                 compatibles:
                  - samsung,exynos5250-dwusb3: "usbdrd30",
+                 - samsung,exynos5420-dwusb3: "usbdrd30", "usbdrd30_susp_clk",
                  - samsung,exynos5433-dwusb3: "aclk", "susp_clk", "pipe_pclk",
                                               "phyclk",
                  - samsung,exynos7-dwusb3: "usbdrd30", "usbdrd30_susp_clk",
-- 
2.25.1

Re: [PATCHv3 1/5] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]


Hi,

Anand Moon <linux.amoon@gmail.com> writes:

> Add the new compatible string for Exynos5422 DWC3 to support
> enable/disable of core and suspend clk by DWC3 driver.
> Also updated the clock names for compatible samsung,exynos5420-dwusb3.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>

What is the dependency here?

checking file Documentation/devicetree/bindings/usb/exynos-usb.txt
Hunk #2 FAILED at 84.
1 out of 2 hunks FAILED

Applying on top of v5.6-rc5

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCHv3 1/5] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support

[Anand Moon]

Hi Felipe,

On Sun, 15 Mar 2020 at 14:37, Felipe Balbi <balbi@kernel.org> wrote:
>
>
> Hi,
>
> Anand Moon <linux.amoon@gmail.com> writes:
>
> > Add the new compatible string for Exynos5422 DWC3 to support
> > enable/disable of core and suspend clk by DWC3 driver.
> > Also updated the clock names for compatible samsung,exynos5420-dwusb3.
> >
> > Acked-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>
> What is the dependency here?
>
> checking file Documentation/devicetree/bindings/usb/exynos-usb.txt
> Hunk #2 FAILED at 84.
> 1 out of 2 hunks FAILED
>
> Applying on top of v5.6-rc5
>
> --
> balbi

These patch were made on top linux next-20200306,
And with new updates in the clk driver configuration.
I will update these patchs later, plz drop these changes for now.

-Anand

Re: [PATCHv3 1/5] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support

[Krzysztof Kozlowski]

On Sun, 15 Mar 2020 at 10:08, Felipe Balbi <balbi@kernel.org> wrote:
>
>
> Hi,
>
> Anand Moon <linux.amoon@gmail.com> writes:
>
> > Add the new compatible string for Exynos5422 DWC3 to support
> > enable/disable of core and suspend clk by DWC3 driver.
> > Also updated the clock names for compatible samsung,exynos5420-dwusb3.
> >
> > Acked-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>
> What is the dependency here?

Felipe,

This patchset should not be applied. As of now, it is not needed and
not justified.

Best regards,
Krzysztof

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

[Anand Moon]

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(-)

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

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

[Anand Moon]

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
>

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

[Krzysztof Kozlowski]

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

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

[Anand Moon]

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

[PATCHv3 3/5] ARM: dts: exynos: Add FSYS power domain to Exynos542x USB nodes

[Anand Moon]

Add a power domain FSYS for USB 3.0 and USB 2.0 and pdma nodes present
on Exynos542x/5800 SoCs.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
New patch in this series.
---
 arch/arm/boot/dts/exynos5420.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index bd505256a223..4046b669b105 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -396,6 +396,13 @@ msc_pd: power-domain@10044120 {
 			label = "MSC";
 		};
 
+		fsys_pd: power-domain@10044140 {
+			compatible = "samsung,exynos4210-pd";
+			reg = <0x10044140 0x20>;
+			#power-domain-cells = <0>;
+			label = "FSYS";
+		};
+
 		pinctrl_0: pinctrl@13400000 {
 			compatible = "samsung,exynos5420-pinctrl";
 			reg = <0x13400000 0x1000>;
@@ -461,6 +468,7 @@ pdma0: pdma@121a0000 {
 				#dma-cells = <1>;
 				#dma-channels = <8>;
 				#dma-requests = <32>;
+				power-domains = <&fsys_pd>;
 			};
 
 			pdma1: pdma@121b0000 {
@@ -472,6 +480,7 @@ pdma1: pdma@121b0000 {
 				#dma-cells = <1>;
 				#dma-channels = <8>;
 				#dma-requests = <32>;
+				power-domains = <&fsys_pd>;
 			};
 
 			mdma0: mdma@10800000 {
@@ -1374,17 +1383,20 @@ &trng {
 &usbdrd3_0 {
 	clocks = <&clock CLK_USBD300>, <&clock CLK_SCLK_USBD300>;
 	clock-names = "usbdrd30", "usbdrd30_susp_clk";
+	power-domains = <&fsys_pd>;
 };
 
 &usbdrd_phy0 {
 	clocks = <&clock CLK_USBD300>, <&clock CLK_SCLK_USBPHY300>;
 	clock-names = "phy", "ref";
 	samsung,pmu-syscon = <&pmu_system_controller>;
+	power-domains = <&fsys_pd>;
 };
 
 &usbdrd3_1 {
 	clocks = <&clock CLK_USBD301>, <&clock CLK_SCLK_USBD301>;
 	clock-names = "usbdrd30", "usbdrd30_susp_clk";
+	power-domains = <&fsys_pd>;
 };
 
 &usbdrd_dwc3_1 {
@@ -1395,16 +1407,19 @@ &usbdrd_phy1 {
 	clocks = <&clock CLK_USBD301>, <&clock CLK_SCLK_USBPHY301>;
 	clock-names = "phy", "ref";
 	samsung,pmu-syscon = <&pmu_system_controller>;
+	power-domains = <&fsys_pd>;
 };
 
 &usbhost1 {
 	clocks = <&clock CLK_USBH20>;
 	clock-names = "usbhost";
+	power-domains = <&fsys_pd>;
 };
 
 &usbhost2 {
 	clocks = <&clock CLK_USBH20>;
 	clock-names = "usbhost";
+	power-domains = <&fsys_pd>;
 };
 
 &usb2_phy {
@@ -1412,6 +1427,7 @@ &usb2_phy {
 	clock-names = "phy", "ref";
 	samsung,sysreg-phandle = <&sysreg_system_controller>;
 	samsung,pmureg-phandle = <&pmu_system_controller>;
+	power-domains = <&fsys_pd>;
 };
 
 &watchdog {
-- 
2.25.1

[PATCHv3 4/5] usb: dwc3: exynos: Add support for Exynos5422 suspend clk

[Anand Moon]

Exynos5422 DWC3 module support two clk USBD300 and SCLK_USBD300
so add missing code to enable/disable code and suspend clk, for this
add a new compatible samsung,exynos5420-dwusb3 to help configure
dwc3 code and dwc3 suspend clock. Suspend clock controls the PHY power
change from P0 to P1/P2/P3 during U0 to U1/U2/U3 transition.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/usb/dwc3/dwc3-exynos.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 90bb022737da..48b68b6f0dc8 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -162,6 +162,12 @@ static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
 	.suspend_clk_idx = -1,
 };
 
+static const struct dwc3_exynos_driverdata exynos5420_drvdata = {
+	.clk_names = { "usbdrd30", "usbdrd30_susp_clk"},
+	.num_clks = 2,
+	.suspend_clk_idx = 1,
+};
+
 static const struct dwc3_exynos_driverdata exynos5433_drvdata = {
 	.clk_names = { "aclk", "susp_clk", "pipe_pclk", "phyclk" },
 	.num_clks = 4,
@@ -178,6 +184,9 @@ static const struct of_device_id exynos_dwc3_match[] = {
 	{
 		.compatible = "samsung,exynos5250-dwusb3",
 		.data = &exynos5250_drvdata,
+	}, {
+		.compatible = "samsung,exynos5420-dwusb3",
+		.data = &exynos5420_drvdata,
 	}, {
 		.compatible = "samsung,exynos5433-dwusb3",
 		.data = &exynos5433_drvdata,
-- 
2.25.1

[PATCHv3 5/5] clk: samsung: exynos542x: Move FSYS subsystem clocks to its sub-CMU

[Anand Moon]

FSYS power domain support usbdrd3, pdma and usb2 power gaiting,
hence move FSYS clk setting to sub-CMU block to support power domain
on/off sequences for device nodes.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
New patch in the series
---
 drivers/clk/samsung/clk-exynos5420.c | 45 +++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index c9e5a1fb6653..6c4c47dfcdce 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -859,12 +859,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
 	DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
 	DIV(0, "dout_maupcm0", "dout_maudio0", DIV_MAU, 24, 8),
 
-	/* USB3.0 */
-	DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
-	DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
-	DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
-	DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
-
 	/* MMC */
 	DIV(0, "dout_mmc0", "mout_mmc0", DIV_FSYS1, 0, 10),
 	DIV(0, "dout_mmc1", "mout_mmc1", DIV_FSYS1, 10, 10),
@@ -1031,8 +1025,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
 
 	/* FSYS Block */
 	GATE(CLK_TSI, "tsi", "aclk200_fsys", GATE_BUS_FSYS0, 0, 0, 0),
-	GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
-	GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
 	GATE(CLK_UFS, "ufs", "aclk200_fsys2", GATE_BUS_FSYS0, 3, 0, 0),
 	GATE(CLK_RTIC, "rtic", "aclk200_fsys", GATE_IP_FSYS, 9, 0, 0),
 	GATE(CLK_MMC0, "mmc0", "aclk200_fsys2", GATE_IP_FSYS, 12, 0, 0),
@@ -1040,9 +1032,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
 	GATE(CLK_MMC2, "mmc2", "aclk200_fsys2", GATE_IP_FSYS, 14, 0, 0),
 	GATE(CLK_SROMC, "sromc", "aclk200_fsys2",
 			GATE_IP_FSYS, 17, CLK_IGNORE_UNUSED, 0),
-	GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
-	GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
-	GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
 	GATE(CLK_SCLK_UNIPRO, "sclk_unipro", "dout_unipro",
 			SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
 
@@ -1258,6 +1247,28 @@ static struct exynos5_subcmu_reg_dump exynos5x_gsc_suspend_regs[] = {
 	{ DIV2_RATIO0, 0, 0x30 },	/* DIV dout_gscl_blk_300 */
 };
 
+/* USB3.0 */
+static const struct samsung_div_clock exynos5x_fsys_div_clks[] __initconst = {
+	DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
+	DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
+	DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
+	DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
+};
+
+static const struct samsung_gate_clock exynos5x_fsys_gate_clks[] __initconst = {
+	GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
+	GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
+	GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
+	GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
+	GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
+};
+
+static struct exynos5_subcmu_reg_dump exynos5x_fsys_suspend_regs[] = {
+	{ GATE_IP_FSYS, 0xffffffff, 0xffffffff }, /* FSYS gates */
+	{ SRC_TOP3, 0, BIT(24) },                 /* SW_MUX_PCLK_200_FSYS_SEL */
+	{ SRC_TOP3, 0, BIT(28) },                 /* SW_MUX_ACLK_200_FSYS_SEL */
+};
+
 static const struct samsung_gate_clock exynos5x_g3d_gate_clks[] __initconst = {
 	GATE(CLK_G3D, "g3d", "mout_user_aclk_g3d", GATE_IP_G3D, 9,
 	     CLK_SET_RATE_PARENT, 0),
@@ -1376,12 +1387,23 @@ static const struct exynos5_subcmu_info exynos5800_mau_subcmu = {
 	.pd_name	= "MAU",
 };
 
+static const struct exynos5_subcmu_info exynos5x_fsys_subcmu = {
+	.div_clks       = exynos5x_fsys_div_clks,
+	.nr_div_clks    = ARRAY_SIZE(exynos5x_fsys_div_clks),
+	.gate_clks	= exynos5x_fsys_gate_clks,
+	.nr_gate_clks	= ARRAY_SIZE(exynos5x_fsys_gate_clks),
+	.suspend_regs	= exynos5x_fsys_suspend_regs,
+	.nr_suspend_regs = ARRAY_SIZE(exynos5x_fsys_suspend_regs),
+	.pd_name	= "FSYS",
+};
+
 static const struct exynos5_subcmu_info *exynos5x_subcmus[] = {
 	&exynos5x_disp_subcmu,
 	&exynos5x_gsc_subcmu,
 	&exynos5x_g3d_subcmu,
 	&exynos5x_mfc_subcmu,
 	&exynos5x_mscl_subcmu,
+	&exynos5x_fsys_subcmu,
 };
 
 static const struct exynos5_subcmu_info *exynos5800_subcmus[] = {
@@ -1391,6 +1413,7 @@ static const struct exynos5_subcmu_info *exynos5800_subcmus[] = {
 	&exynos5x_mfc_subcmu,
 	&exynos5x_mscl_subcmu,
 	&exynos5800_mau_subcmu,
+	&exynos5x_fsys_subcmu,
 };
 
 static const struct samsung_pll_rate_table exynos5420_pll2550x_24mhz_tbl[] __initconst = {
-- 
2.25.1

Re: [PATCHv3 5/5] clk: samsung: exynos542x: Move FSYS subsystem clocks to its sub-CMU

[Krzysztof Kozlowski]

On Tue, Mar 10, 2020 at 07:48:54PM +0000, Anand Moon wrote:
> FSYS power domain support usbdrd3, pdma and usb2 power gaiting,
> hence move FSYS clk setting to sub-CMU block to support power domain
> on/off sequences for device nodes.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> New patch in the series
> ---
>  drivers/clk/samsung/clk-exynos5420.c | 45 +++++++++++++++++++++-------
>  1 file changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> index c9e5a1fb6653..6c4c47dfcdce 100644
> --- a/drivers/clk/samsung/clk-exynos5420.c
> +++ b/drivers/clk/samsung/clk-exynos5420.c
> @@ -859,12 +859,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
>  	DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
>  	DIV(0, "dout_maupcm0", "dout_maudio0", DIV_MAU, 24, 8),
>  
> -	/* USB3.0 */
> -	DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> -	DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> -	DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> -	DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),

According to clock diagram these are still in CMU TOP, not FSYS.

> -
>  	/* MMC */
>  	DIV(0, "dout_mmc0", "mout_mmc0", DIV_FSYS1, 0, 10),
>  	DIV(0, "dout_mmc1", "mout_mmc1", DIV_FSYS1, 10, 10),
> @@ -1031,8 +1025,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
/>  
>  	/* FSYS Block */
>  	GATE(CLK_TSI, "tsi", "aclk200_fsys", GATE_BUS_FSYS0, 0, 0, 0),
> -	GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> -	GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
>  	GATE(CLK_UFS, "ufs", "aclk200_fsys2", GATE_BUS_FSYS0, 3, 0, 0),
>  	GATE(CLK_RTIC, "rtic", "aclk200_fsys", GATE_IP_FSYS, 9, 0, 0),
>  	GATE(CLK_MMC0, "mmc0", "aclk200_fsys2", GATE_IP_FSYS, 12, 0, 0),
> @@ -1040,9 +1032,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
>  	GATE(CLK_MMC2, "mmc2", "aclk200_fsys2", GATE_IP_FSYS, 14, 0, 0),
>  	GATE(CLK_SROMC, "sromc", "aclk200_fsys2",
>  			GATE_IP_FSYS, 17, CLK_IGNORE_UNUSED, 0),
> -	GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> -	GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> -	GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
>  	GATE(CLK_SCLK_UNIPRO, "sclk_unipro", "dout_unipro",
>  			SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
>  
> @@ -1258,6 +1247,28 @@ static struct exynos5_subcmu_reg_dump exynos5x_gsc_suspend_regs[] = {
>  	{ DIV2_RATIO0, 0, 0x30 },	/* DIV dout_gscl_blk_300 */
>  };
>  
> +/* USB3.0 */
> +static const struct samsung_div_clock exynos5x_fsys_div_clks[] __initconst = {
> +	DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> +	DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> +	DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> +	DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> +};
> +
> +static const struct samsung_gate_clock exynos5x_fsys_gate_clks[] __initconst = {
> +	GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> +	GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> +	GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> +	GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> +	GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> +};
> +
> +static struct exynos5_subcmu_reg_dump exynos5x_fsys_suspend_regs[] = {
> +	{ GATE_IP_FSYS, 0xffffffff, 0xffffffff }, /* FSYS gates */

This looks wrong. GATE_IP_FSYS has fields also for FSYS2 clocks which
you are not suspending. They do not belong to this CMU.

Don't you need to save also parts of GATE_BUS_FSYS0?

> +	{ SRC_TOP3, 0, BIT(24) },                 /* SW_MUX_PCLK_200_FSYS_SEL */
> +	{ SRC_TOP3, 0, BIT(28) },                 /* SW_MUX_ACLK_200_FSYS_SEL */

Name of clocks from the driver please, not from datasheet. Look at other
examples.

Best regards,
Krzysztof


> +};
> +
>  static const struct samsung_gate_clock exynos5x_g3d_gate_clks[] __initconst = {
>  	GATE(CLK_G3D, "g3d", "mout_user_aclk_g3d", GATE_IP_G3D, 9,
>  	     CLK_SET_RATE_PARENT, 0),
> @@ -1376,12 +1387,23 @@ static const struct exynos5_subcmu_info exynos5800_mau_subcmu = {
>  	.pd_name	= "MAU",
>  };
>  
> +static const struct exynos5_subcmu_info exynos5x_fsys_subcmu = {
> +	.div_clks       = exynos5x_fsys_div_clks,
> +	.nr_div_clks    = ARRAY_SIZE(exynos5x_fsys_div_clks),
> +	.gate_clks	= exynos5x_fsys_gate_clks,
> +	.nr_gate_clks	= ARRAY_SIZE(exynos5x_fsys_gate_clks),
> +	.suspend_regs	= exynos5x_fsys_suspend_regs,
> +	.nr_suspend_regs = ARRAY_SIZE(exynos5x_fsys_suspend_regs),
> +	.pd_name	= "FSYS",
> +};
> +
>  static const struct exynos5_subcmu_info *exynos5x_subcmus[] = {
>  	&exynos5x_disp_subcmu,
>  	&exynos5x_gsc_subcmu,
>  	&exynos5x_g3d_subcmu,
>  	&exynos5x_mfc_subcmu,
>  	&exynos5x_mscl_subcmu,
> +	&exynos5x_fsys_subcmu,
>  };
>  
>  static const struct exynos5_subcmu_info *exynos5800_subcmus[] = {
> @@ -1391,6 +1413,7 @@ static const struct exynos5_subcmu_info *exynos5800_subcmus[] = {
>  	&exynos5x_mfc_subcmu,
>  	&exynos5x_mscl_subcmu,
>  	&exynos5800_mau_subcmu,
> +	&exynos5x_fsys_subcmu,
>  };
>  
>  static const struct samsung_pll_rate_table exynos5420_pll2550x_24mhz_tbl[] __initconst = {
> -- 
> 2.25.1
> 

Re: [PATCHv3 5/5] clk: samsung: exynos542x: Move FSYS subsystem clocks to its sub-CMU

[Anand Moon]

Hi Krzysztof,

Thanks for your review comments.

On Wed, 11 Mar 2020 at 20:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, Mar 10, 2020 at 07:48:54PM +0000, Anand Moon wrote:
> > FSYS power domain support usbdrd3, pdma and usb2 power gaiting,
> > hence move FSYS clk setting to sub-CMU block to support power domain
> > on/off sequences for device nodes.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > New patch in the series
> > ---
> >  drivers/clk/samsung/clk-exynos5420.c | 45 +++++++++++++++++++++-------
> >  1 file changed, 34 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> > index c9e5a1fb6653..6c4c47dfcdce 100644
> > --- a/drivers/clk/samsung/clk-exynos5420.c
> > +++ b/drivers/clk/samsung/clk-exynos5420.c
> > @@ -859,12 +859,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
> >       DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
> >       DIV(0, "dout_maupcm0", "dout_maudio0", DIV_MAU, 24, 8),
> >
> > -     /* USB3.0 */
> > -     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > -     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > -     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > -     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
>
> According to clock diagram these are still in CMU TOP, not FSYS.
>
> > -
> >       /* MMC */
> >       DIV(0, "dout_mmc0", "mout_mmc0", DIV_FSYS1, 0, 10),
> >       DIV(0, "dout_mmc1", "mout_mmc1", DIV_FSYS1, 10, 10),
> > @@ -1031,8 +1025,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> />
> >       /* FSYS Block */
> >       GATE(CLK_TSI, "tsi", "aclk200_fsys", GATE_BUS_FSYS0, 0, 0, 0),
> > -     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > -     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> >       GATE(CLK_UFS, "ufs", "aclk200_fsys2", GATE_BUS_FSYS0, 3, 0, 0),
> >       GATE(CLK_RTIC, "rtic", "aclk200_fsys", GATE_IP_FSYS, 9, 0, 0),
> >       GATE(CLK_MMC0, "mmc0", "aclk200_fsys2", GATE_IP_FSYS, 12, 0, 0),
> > @@ -1040,9 +1032,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> >       GATE(CLK_MMC2, "mmc2", "aclk200_fsys2", GATE_IP_FSYS, 14, 0, 0),
> >       GATE(CLK_SROMC, "sromc", "aclk200_fsys2",
> >                       GATE_IP_FSYS, 17, CLK_IGNORE_UNUSED, 0),
> > -     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > -     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > -     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> >       GATE(CLK_SCLK_UNIPRO, "sclk_unipro", "dout_unipro",
> >                       SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
> >
> > @@ -1258,6 +1247,28 @@ static struct exynos5_subcmu_reg_dump exynos5x_gsc_suspend_regs[] = {
> >       { DIV2_RATIO0, 0, 0x30 },       /* DIV dout_gscl_blk_300 */
> >  };
> >
> > +/* USB3.0 */
> > +static const struct samsung_div_clock exynos5x_fsys_div_clks[] __initconst = {
> > +     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > +     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > +     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > +     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> > +};
> > +
> > +static const struct samsung_gate_clock exynos5x_fsys_gate_clks[] __initconst = {
> > +     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > +     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > +     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > +     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > +     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > +};
> > +
> > +static struct exynos5_subcmu_reg_dump exynos5x_fsys_suspend_regs[] = {
> > +     { GATE_IP_FSYS, 0xffffffff, 0xffffffff }, /* FSYS gates */
>
> This looks wrong. GATE_IP_FSYS has fields also for FSYS2 clocks which
> you are not suspending. They do not belong to this CMU.
>

Ok. I change the from GATE_IP_FSYS to GATE_BUS_FSYS0 in the above
exynos5x_fsys_gate_clks to make this consistent to used GATE_BUS_FSYS0 for CMU,
with this change it works as per previously.

> Don't you need to save also parts of GATE_BUS_FSYS0?

GATE_BUS_FSYS0 and GATE_IP_FSYS are already part of list
of control register which are saved and restored during suspend and resume
so no point in adding this here, I should drop the GATE_IP_FSYS reg
dump over here.

>
> > +     { SRC_TOP3, 0, BIT(24) },                 /* SW_MUX_PCLK_200_FSYS_SEL */
> > +     { SRC_TOP3, 0, BIT(28) },                 /* SW_MUX_ACLK_200_FSYS_SEL */
>
> Name of clocks from the driver please, not from datasheet. Look at other
> examples.
>

Ok  I will update this as per the examples.

> Best regards,
> Krzysztof
>
>

-Anand

> > +};
> > +
> >  static const struct samsung_gate_clock exynos5x_g3d_gate_clks[] __initconst = {
> >       GATE(CLK_G3D, "g3d", "mout_user_aclk_g3d", GATE_IP_G3D, 9,
> >            CLK_SET_RATE_PARENT, 0),
> > @@ -1376,12 +1387,23 @@ static const struct exynos5_subcmu_info exynos5800_mau_subcmu = {
> >       .pd_name        = "MAU",
> >  };
> >
> > +static const struct exynos5_subcmu_info exynos5x_fsys_subcmu = {
> > +     .div_clks       = exynos5x_fsys_div_clks,
> > +     .nr_div_clks    = ARRAY_SIZE(exynos5x_fsys_div_clks),
> > +     .gate_clks      = exynos5x_fsys_gate_clks,
> > +     .nr_gate_clks   = ARRAY_SIZE(exynos5x_fsys_gate_clks),
> > +     .suspend_regs   = exynos5x_fsys_suspend_regs,
> > +     .nr_suspend_regs = ARRAY_SIZE(exynos5x_fsys_suspend_regs),
> > +     .pd_name        = "FSYS",
> > +};
> > +
> >  static const struct exynos5_subcmu_info *exynos5x_subcmus[] = {
> >       &exynos5x_disp_subcmu,
> >       &exynos5x_gsc_subcmu,
> >       &exynos5x_g3d_subcmu,
> >       &exynos5x_mfc_subcmu,
> >       &exynos5x_mscl_subcmu,
> > +     &exynos5x_fsys_subcmu,
> >  };
> >
> >  static const struct exynos5_subcmu_info *exynos5800_subcmus[] = {
> > @@ -1391,6 +1413,7 @@ static const struct exynos5_subcmu_info *exynos5800_subcmus[] = {
> >       &exynos5x_mfc_subcmu,
> >       &exynos5x_mscl_subcmu,
> >       &exynos5800_mau_subcmu,
> > +     &exynos5x_fsys_subcmu,
> >  };
> >
> >  static const struct samsung_pll_rate_table exynos5420_pll2550x_24mhz_tbl[] __initconst = {
> > --
> > 2.25.1
> >

Re: [PATCHv3 5/5] clk: samsung: exynos542x: Move FSYS subsystem clocks to its sub-CMU

[Krzysztof Kozlowski]

On Thu, Mar 12, 2020 at 04:04:57PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> Thanks for your review comments.
> 
> On Wed, 11 Mar 2020 at 20:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Tue, Mar 10, 2020 at 07:48:54PM +0000, Anand Moon wrote:
> > > FSYS power domain support usbdrd3, pdma and usb2 power gaiting,
> > > hence move FSYS clk setting to sub-CMU block to support power domain
> > > on/off sequences for device nodes.
> > >
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > > New patch in the series
> > > ---
> > >  drivers/clk/samsung/clk-exynos5420.c | 45 +++++++++++++++++++++-------
> > >  1 file changed, 34 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> > > index c9e5a1fb6653..6c4c47dfcdce 100644
> > > --- a/drivers/clk/samsung/clk-exynos5420.c
> > > +++ b/drivers/clk/samsung/clk-exynos5420.c
> > > @@ -859,12 +859,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
> > >       DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
> > >       DIV(0, "dout_maupcm0", "dout_maudio0", DIV_MAU, 24, 8),
> > >
> > > -     /* USB3.0 */
> > > -     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > > -     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > > -     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > > -     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> >
> > According to clock diagram these are still in CMU TOP, not FSYS.
> >
> > > -
> > >       /* MMC */
> > >       DIV(0, "dout_mmc0", "mout_mmc0", DIV_FSYS1, 0, 10),
> > >       DIV(0, "dout_mmc1", "mout_mmc1", DIV_FSYS1, 10, 10),
> > > @@ -1031,8 +1025,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> > />
> > >       /* FSYS Block */
> > >       GATE(CLK_TSI, "tsi", "aclk200_fsys", GATE_BUS_FSYS0, 0, 0, 0),
> > > -     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > > -     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > >       GATE(CLK_UFS, "ufs", "aclk200_fsys2", GATE_BUS_FSYS0, 3, 0, 0),
> > >       GATE(CLK_RTIC, "rtic", "aclk200_fsys", GATE_IP_FSYS, 9, 0, 0),
> > >       GATE(CLK_MMC0, "mmc0", "aclk200_fsys2", GATE_IP_FSYS, 12, 0, 0),
> > > @@ -1040,9 +1032,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> > >       GATE(CLK_MMC2, "mmc2", "aclk200_fsys2", GATE_IP_FSYS, 14, 0, 0),
> > >       GATE(CLK_SROMC, "sromc", "aclk200_fsys2",
> > >                       GATE_IP_FSYS, 17, CLK_IGNORE_UNUSED, 0),
> > > -     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > > -     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > > -     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > >       GATE(CLK_SCLK_UNIPRO, "sclk_unipro", "dout_unipro",
> > >                       SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
> > >
> > > @@ -1258,6 +1247,28 @@ static struct exynos5_subcmu_reg_dump exynos5x_gsc_suspend_regs[] = {
> > >       { DIV2_RATIO0, 0, 0x30 },       /* DIV dout_gscl_blk_300 */
> > >  };
> > >
> > > +/* USB3.0 */
> > > +static const struct samsung_div_clock exynos5x_fsys_div_clks[] __initconst = {
> > > +     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > > +     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > > +     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > > +     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> > > +};
> > > +
> > > +static const struct samsung_gate_clock exynos5x_fsys_gate_clks[] __initconst = {
> > > +     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > > +     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > > +     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > > +     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > > +     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > > +};
> > > +
> > > +static struct exynos5_subcmu_reg_dump exynos5x_fsys_suspend_regs[] = {
> > > +     { GATE_IP_FSYS, 0xffffffff, 0xffffffff }, /* FSYS gates */
> >
> > This looks wrong. GATE_IP_FSYS has fields also for FSYS2 clocks which
> > you are not suspending. They do not belong to this CMU.
> >
> 
> Ok. I change the from GATE_IP_FSYS to GATE_BUS_FSYS0 in the above
> exynos5x_fsys_gate_clks to make this consistent to used GATE_BUS_FSYS0 for CMU,
> with this change it works as per previously.

Wait, you should set here proper registers with proper mask.
> 
> > Don't you need to save also parts of GATE_BUS_FSYS0?
> 
> GATE_BUS_FSYS0 and GATE_IP_FSYS are already part of list
> of control register which are saved and restored during suspend and resume
> so no point in adding this here, I should drop the GATE_IP_FSYS reg
> dump over here.

Since registers are used in separate sub CMU devices, each should
save/restore its part.

Best regards,
Krzysztof

Re: [PATCHv3 5/5] clk: samsung: exynos542x: Move FSYS subsystem clocks to its sub-CMU

[Anand Moon]

Hi Krzysztof,

On Thu, 12 Mar 2020 at 17:06, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Thu, Mar 12, 2020 at 04:04:57PM +0530, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > Thanks for your review comments.
> >
> > On Wed, 11 Mar 2020 at 20:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On Tue, Mar 10, 2020 at 07:48:54PM +0000, Anand Moon wrote:
> > > > FSYS power domain support usbdrd3, pdma and usb2 power gaiting,
> > > > hence move FSYS clk setting to sub-CMU block to support power domain
> > > > on/off sequences for device nodes.
> > > >
> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > ---
> > > > New patch in the series
> > > > ---
> > > >  drivers/clk/samsung/clk-exynos5420.c | 45 +++++++++++++++++++++-------
> > > >  1 file changed, 34 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> > > > index c9e5a1fb6653..6c4c47dfcdce 100644
> > > > --- a/drivers/clk/samsung/clk-exynos5420.c
> > > > +++ b/drivers/clk/samsung/clk-exynos5420.c
> > > > @@ -859,12 +859,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
> > > >       DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
> > > >       DIV(0, "dout_maupcm0", "dout_maudio0", DIV_MAU, 24, 8),
> > > >
> > > > -     /* USB3.0 */
> > > > -     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > > > -     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > > > -     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > > > -     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> > >
> > > According to clock diagram these are still in CMU TOP, not FSYS.
> > >
> > > > -
> > > >       /* MMC */
> > > >       DIV(0, "dout_mmc0", "mout_mmc0", DIV_FSYS1, 0, 10),
> > > >       DIV(0, "dout_mmc1", "mout_mmc1", DIV_FSYS1, 10, 10),
> > > > @@ -1031,8 +1025,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> > > />
> > > >       /* FSYS Block */
> > > >       GATE(CLK_TSI, "tsi", "aclk200_fsys", GATE_BUS_FSYS0, 0, 0, 0),
> > > > -     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > > > -     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > > >       GATE(CLK_UFS, "ufs", "aclk200_fsys2", GATE_BUS_FSYS0, 3, 0, 0),
> > > >       GATE(CLK_RTIC, "rtic", "aclk200_fsys", GATE_IP_FSYS, 9, 0, 0),
> > > >       GATE(CLK_MMC0, "mmc0", "aclk200_fsys2", GATE_IP_FSYS, 12, 0, 0),
> > > > @@ -1040,9 +1032,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> > > >       GATE(CLK_MMC2, "mmc2", "aclk200_fsys2", GATE_IP_FSYS, 14, 0, 0),
> > > >       GATE(CLK_SROMC, "sromc", "aclk200_fsys2",
> > > >                       GATE_IP_FSYS, 17, CLK_IGNORE_UNUSED, 0),
> > > > -     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > > > -     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > > > -     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > > >       GATE(CLK_SCLK_UNIPRO, "sclk_unipro", "dout_unipro",
> > > >                       SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
> > > >
> > > > @@ -1258,6 +1247,28 @@ static struct exynos5_subcmu_reg_dump exynos5x_gsc_suspend_regs[] = {
> > > >       { DIV2_RATIO0, 0, 0x30 },       /* DIV dout_gscl_blk_300 */
> > > >  };
> > > >
> > > > +/* USB3.0 */
> > > > +static const struct samsung_div_clock exynos5x_fsys_div_clks[] __initconst = {
> > > > +     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > > > +     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > > > +     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > > > +     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> > > > +};
> > > > +
> > > > +static const struct samsung_gate_clock exynos5x_fsys_gate_clks[] __initconst = {
> > > > +     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > > > +     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > > > +     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > > > +     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > > > +     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > > > +};
> > > > +
> > > > +static struct exynos5_subcmu_reg_dump exynos5x_fsys_suspend_regs[] = {
> > > > +     { GATE_IP_FSYS, 0xffffffff, 0xffffffff }, /* FSYS gates */
> > >
> > > This looks wrong. GATE_IP_FSYS has fields also for FSYS2 clocks which
> > > you are not suspending. They do not belong to this CMU.
> > >
> >
> > Ok. I change the from GATE_IP_FSYS to GATE_BUS_FSYS0 in the above
> > exynos5x_fsys_gate_clks to make this consistent to used GATE_BUS_FSYS0 for CMU,
> > with this change it works as per previously.
>
> Wait, you should set here proper registers with proper mask.

Yes I will set the proper mask for each as per the Exynos 5422 User Manual.

Here is what I feel
CLK_GATE_BUS_FSYS0 controls the PHY clock
CLK_GATE_IP_FSYS controls the IP clock.

So both these field should be part of this FSYS CMU.

> >
> > > Don't you need to save also parts of GATE_BUS_FSYS0?
> >
> > GATE_BUS_FSYS0 and GATE_IP_FSYS are already part of list
> > of control register which are saved and restored during suspend and resume
> > so no point in adding this here, I should drop the GATE_IP_FSYS reg
> > dump over here.
>
> Since registers are used in separate sub CMU devices, each should
> save/restore its part.

Ok I will add both GATE_BUS_FSYS0 and GATE_IP_FSYS
reset value over here.

>
> Best regards,
> Krzysztof
>

-Anand

Re: [PATCHv3 5/5] clk: samsung: exynos542x: Move FSYS subsystem clocks to its sub-CMU

[Anand Moon]

Hi Krzysztof,

On Thu, 12 Mar 2020 at 18:24, Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Krzysztof,
>
> On Thu, 12 Mar 2020 at 17:06, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Thu, Mar 12, 2020 at 04:04:57PM +0530, Anand Moon wrote:
> > > Hi Krzysztof,
> > >
> > > Thanks for your review comments.
> > >
> > > On Wed, 11 Mar 2020 at 20:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >
> > > > On Tue, Mar 10, 2020 at 07:48:54PM +0000, Anand Moon wrote:
> > > > > FSYS power domain support usbdrd3, pdma and usb2 power gaiting,
> > > > > hence move FSYS clk setting to sub-CMU block to support power domain
> > > > > on/off sequences for device nodes.
> > > > >
> > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > > ---
> > > > > New patch in the series
> > > > > ---
> > > > >  drivers/clk/samsung/clk-exynos5420.c | 45 +++++++++++++++++++++-------
> > > > >  1 file changed, 34 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> > > > > index c9e5a1fb6653..6c4c47dfcdce 100644
> > > > > --- a/drivers/clk/samsung/clk-exynos5420.c
> > > > > +++ b/drivers/clk/samsung/clk-exynos5420.c
> > > > > @@ -859,12 +859,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
> > > > >       DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
> > > > >       DIV(0, "dout_maupcm0", "dout_maudio0", DIV_MAU, 24, 8),
> > > > >
> > > > > -     /* USB3.0 */
> > > > > -     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > > > > -     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > > > > -     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > > > > -     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> > > >
> > > > According to clock diagram these are still in CMU TOP, not FSYS.
> > > >
> > > > > -
> > > > >       /* MMC */
> > > > >       DIV(0, "dout_mmc0", "mout_mmc0", DIV_FSYS1, 0, 10),
> > > > >       DIV(0, "dout_mmc1", "mout_mmc1", DIV_FSYS1, 10, 10),
> > > > > @@ -1031,8 +1025,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> > > > />
> > > > >       /* FSYS Block */
> > > > >       GATE(CLK_TSI, "tsi", "aclk200_fsys", GATE_BUS_FSYS0, 0, 0, 0),
> > > > > -     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > > > > -     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > > > >       GATE(CLK_UFS, "ufs", "aclk200_fsys2", GATE_BUS_FSYS0, 3, 0, 0),
> > > > >       GATE(CLK_RTIC, "rtic", "aclk200_fsys", GATE_IP_FSYS, 9, 0, 0),
> > > > >       GATE(CLK_MMC0, "mmc0", "aclk200_fsys2", GATE_IP_FSYS, 12, 0, 0),
> > > > > @@ -1040,9 +1032,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> > > > >       GATE(CLK_MMC2, "mmc2", "aclk200_fsys2", GATE_IP_FSYS, 14, 0, 0),
> > > > >       GATE(CLK_SROMC, "sromc", "aclk200_fsys2",
> > > > >                       GATE_IP_FSYS, 17, CLK_IGNORE_UNUSED, 0),
> > > > > -     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > > > > -     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > > > > -     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > > > >       GATE(CLK_SCLK_UNIPRO, "sclk_unipro", "dout_unipro",
> > > > >                       SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
> > > > >
> > > > > @@ -1258,6 +1247,28 @@ static struct exynos5_subcmu_reg_dump exynos5x_gsc_suspend_regs[] = {
> > > > >       { DIV2_RATIO0, 0, 0x30 },       /* DIV dout_gscl_blk_300 */
> > > > >  };
> > > > >
> > > > > +/* USB3.0 */
> > > > > +static const struct samsung_div_clock exynos5x_fsys_div_clks[] __initconst = {
> > > > > +     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > > > > +     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > > > > +     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > > > > +     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> > > > > +};
> > > > > +
> > > > > +static const struct samsung_gate_clock exynos5x_fsys_gate_clks[] __initconst = {
> > > > > +     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > > > > +     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > > > > +     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > > > > +     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > > > > +     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > > > > +};
> > > > > +
> > > > > +static struct exynos5_subcmu_reg_dump exynos5x_fsys_suspend_regs[] = {
> > > > > +     { GATE_IP_FSYS, 0xffffffff, 0xffffffff }, /* FSYS gates */
> > > >
> > > > This looks wrong. GATE_IP_FSYS has fields also for FSYS2 clocks which
> > > > you are not suspending. They do not belong to this CMU.
> > > >
> > >
> > > Ok. I change the from GATE_IP_FSYS to GATE_BUS_FSYS0 in the above
> > > exynos5x_fsys_gate_clks to make this consistent to used GATE_BUS_FSYS0 for CMU,
> > > with this change it works as per previously.
> >
> > Wait, you should set here proper registers with proper mask.
>
> Yes I will set the proper mask for each as per the Exynos 5422 User Manual.
>
> Here is what I feel
> CLK_GATE_BUS_FSYS0 controls the PHY clock
> CLK_GATE_IP_FSYS controls the IP clock.
>

Sorry I cannot register both CLK_GATE_BUS_FSYS0 and CLK_GATE_IP_FSYS
to aclk200_fsys, so I got some error like below.

[    0.922693] samsung_clk_register_gate: failed to register clock usbh20
[    0.922857] samsung_clk_register_gate: failed to register clock usbd300
[    0.923000] samsung_clk_register_gate: failed to register clock usbd301

> So both these field should be part of this FSYS CMU.
>
> > >
> > > > Don't you need to save also parts of GATE_BUS_FSYS0?
> > >
> > > GATE_BUS_FSYS0 and GATE_IP_FSYS are already part of list
> > > of control register which are saved and restored during suspend and resume
> > > so no point in adding this here, I should drop the GATE_IP_FSYS reg
> > > dump over here.
> >
> > Since registers are used in separate sub CMU devices, each should
> > save/restore its part.
>
> Ok I will add both GATE_BUS_FSYS0 and GATE_IP_FSYS
> reset value over here.
>

So only changes to this patch is to set the above correctly.

-Anand

Re: [PATCHv3 5/5] clk: samsung: exynos542x: Move FSYS subsystem clocks to its sub-CMU

[Krzysztof Kozlowski]

On Thu, Mar 12, 2020 at 07:38:30PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Thu, 12 Mar 2020 at 18:24, Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Hi Krzysztof,
> >
> > On Thu, 12 Mar 2020 at 17:06, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On Thu, Mar 12, 2020 at 04:04:57PM +0530, Anand Moon wrote:
> > > > Hi Krzysztof,
> > > >
> > > > Thanks for your review comments.
> > > >
> > > > On Wed, 11 Mar 2020 at 20:12, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > >
> > > > > On Tue, Mar 10, 2020 at 07:48:54PM +0000, Anand Moon wrote:
> > > > > > FSYS power domain support usbdrd3, pdma and usb2 power gaiting,
> > > > > > hence move FSYS clk setting to sub-CMU block to support power domain
> > > > > > on/off sequences for device nodes.
> > > > > >
> > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > > > ---
> > > > > > New patch in the series
> > > > > > ---
> > > > > >  drivers/clk/samsung/clk-exynos5420.c | 45 +++++++++++++++++++++-------
> > > > > >  1 file changed, 34 insertions(+), 11 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
> > > > > > index c9e5a1fb6653..6c4c47dfcdce 100644
> > > > > > --- a/drivers/clk/samsung/clk-exynos5420.c
> > > > > > +++ b/drivers/clk/samsung/clk-exynos5420.c
> > > > > > @@ -859,12 +859,6 @@ static const struct samsung_div_clock exynos5x_div_clks[] __initconst = {
> > > > > >       DIV(0, "dout_maudio0", "mout_maudio0", DIV_MAU, 20, 4),
> > > > > >       DIV(0, "dout_maupcm0", "dout_maudio0", DIV_MAU, 24, 8),
> > > > > >
> > > > > > -     /* USB3.0 */
> > > > > > -     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > > > > > -     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > > > > > -     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > > > > > -     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> > > > >
> > > > > According to clock diagram these are still in CMU TOP, not FSYS.
> > > > >
> > > > > > -
> > > > > >       /* MMC */
> > > > > >       DIV(0, "dout_mmc0", "mout_mmc0", DIV_FSYS1, 0, 10),
> > > > > >       DIV(0, "dout_mmc1", "mout_mmc1", DIV_FSYS1, 10, 10),
> > > > > > @@ -1031,8 +1025,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> > > > > />
> > > > > >       /* FSYS Block */
> > > > > >       GATE(CLK_TSI, "tsi", "aclk200_fsys", GATE_BUS_FSYS0, 0, 0, 0),
> > > > > > -     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > > > > > -     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > > > > >       GATE(CLK_UFS, "ufs", "aclk200_fsys2", GATE_BUS_FSYS0, 3, 0, 0),
> > > > > >       GATE(CLK_RTIC, "rtic", "aclk200_fsys", GATE_IP_FSYS, 9, 0, 0),
> > > > > >       GATE(CLK_MMC0, "mmc0", "aclk200_fsys2", GATE_IP_FSYS, 12, 0, 0),
> > > > > > @@ -1040,9 +1032,6 @@ static const struct samsung_gate_clock exynos5x_gate_clks[] __initconst = {
> > > > > >       GATE(CLK_MMC2, "mmc2", "aclk200_fsys2", GATE_IP_FSYS, 14, 0, 0),
> > > > > >       GATE(CLK_SROMC, "sromc", "aclk200_fsys2",
> > > > > >                       GATE_IP_FSYS, 17, CLK_IGNORE_UNUSED, 0),
> > > > > > -     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > > > > > -     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > > > > > -     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > > > > >       GATE(CLK_SCLK_UNIPRO, "sclk_unipro", "dout_unipro",
> > > > > >                       SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
> > > > > >
> > > > > > @@ -1258,6 +1247,28 @@ static struct exynos5_subcmu_reg_dump exynos5x_gsc_suspend_regs[] = {
> > > > > >       { DIV2_RATIO0, 0, 0x30 },       /* DIV dout_gscl_blk_300 */
> > > > > >  };
> > > > > >
> > > > > > +/* USB3.0 */
> > > > > > +static const struct samsung_div_clock exynos5x_fsys_div_clks[] __initconst = {
> > > > > > +     DIV(0, "dout_usbphy301", "mout_usbd301", DIV_FSYS0, 12, 4),
> > > > > > +     DIV(0, "dout_usbphy300", "mout_usbd300", DIV_FSYS0, 16, 4),
> > > > > > +     DIV(0, "dout_usbd301", "mout_usbd301", DIV_FSYS0, 20, 4),
> > > > > > +     DIV(0, "dout_usbd300", "mout_usbd300", DIV_FSYS0, 24, 4),
> > > > > > +};
> > > > > > +
> > > > > > +static const struct samsung_gate_clock exynos5x_fsys_gate_clks[] __initconst = {
> > > > > > +     GATE(CLK_PDMA0, "pdma0", "aclk200_fsys", GATE_BUS_FSYS0, 1, 0, 0),
> > > > > > +     GATE(CLK_PDMA1, "pdma1", "aclk200_fsys", GATE_BUS_FSYS0, 2, 0, 0),
> > > > > > +     GATE(CLK_USBH20, "usbh20", "aclk200_fsys", GATE_IP_FSYS, 18, 0, 0),
> > > > > > +     GATE(CLK_USBD300, "usbd300", "aclk200_fsys", GATE_IP_FSYS, 19, 0, 0),
> > > > > > +     GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
> > > > > > +};
> > > > > > +
> > > > > > +static struct exynos5_subcmu_reg_dump exynos5x_fsys_suspend_regs[] = {
> > > > > > +     { GATE_IP_FSYS, 0xffffffff, 0xffffffff }, /* FSYS gates */
> > > > >
> > > > > This looks wrong. GATE_IP_FSYS has fields also for FSYS2 clocks which
> > > > > you are not suspending. They do not belong to this CMU.
> > > > >
> > > >
> > > > Ok. I change the from GATE_IP_FSYS to GATE_BUS_FSYS0 in the above
> > > > exynos5x_fsys_gate_clks to make this consistent to used GATE_BUS_FSYS0 for CMU,
> > > > with this change it works as per previously.
> > >
> > > Wait, you should set here proper registers with proper mask.
> >
> > Yes I will set the proper mask for each as per the Exynos 5422 User Manual.
> >
> > Here is what I feel
> > CLK_GATE_BUS_FSYS0 controls the PHY clock
> > CLK_GATE_IP_FSYS controls the IP clock.
> >
> 
> Sorry I cannot register both CLK_GATE_BUS_FSYS0 and CLK_GATE_IP_FSYS
> to aclk200_fsys, so I got some error like below.
> 
> [    0.922693] samsung_clk_register_gate: failed to register clock usbh20
> [    0.922857] samsung_clk_register_gate: failed to register clock usbd300
> [    0.923000] samsung_clk_register_gate: failed to register clock usbd301
> 
> > So both these field should be part of this FSYS CMU.

I am not sure if I understand your problem. I mentioned that you need
to put proper registers with proper masks into the
exynos5_subcmu_reg_dump. I don't know what have you changed to produce
such error logs.

Best regards,
Krzysztof