Skip to contentSkip to conversation

Add support for suspend clk for Exynos5422 SoC

PATCH Superseded Anand Moon 4 participants 10 messages Mar 1, 2020 – Mar 15, 2020

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Anand Moon

author
#0 lore raw

v2 cover Add support for suspend clk for Exynos5422 SoC

Seried build and tested on linux next-20200228.

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 (3):
devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3
clocks support
ARM: dts: exynos: Add missing usbdrd3 suspend clk
usb: dwc3: exynos: Add support for Exynos5422 suspend clk

 Documentation/devicetree/bindings/usb/exynos-usb.txt | 5 ++++-
 arch/arm/boot/dts/exynos5410.dtsi                    | 8 ++++----
 arch/arm/boot/dts/exynos5420.dtsi                    | 8 ++++----
 arch/arm/boot/dts/exynos54xx.dtsi                    | 4 ++--
 drivers/usb/dwc3/dwc3-exynos.c                       | 9 +++++++++
 5 files changed, 23 insertions(+), 11 deletions(-)
Signature · 1 line
2.25.1
author
#1 lore raw

PATCH v2 1/3 devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support

This patch adds 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.

---
Previous changes:
Added the missing clock name for Exynos5420 complatible

---
 Documentation/devicetree/bindings/usb/exynos-usb.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/usb/exynos-usb.txt +4 -1
index 6aae1544f240..220f729ac8eb 100644
@@ -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",
Signature · 1 line
2.25.1
#2 lore raw

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

Quoted 3 lines · Anand Moon

This patch adds 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.

Some time ago I mentioned this... so once more:
Do not use "This patch adds" but simple "Add".
https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L151

Best regards,
Krzysztof

Quoted 35 lines

---
Previous changes:
Added the missing clock name for Exynos5420 complatible

---
 Documentation/devicetree/bindings/usb/exynos-usb.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/usb/exynos-usb.txt +4 -1
index 6aae1544f240..220f729ac8eb 100644
@@ -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",
Signature · 1 line
2.25.1
author
#3 lore raw

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

Hi Krzysztof,

Quoted 10 lines · Krzysztof Kozlowski
Quoted 3 lines · Anand Moon

This patch adds 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.

Some time ago I mentioned this... so once more:
Do not use "This patch adds" but simple "Add".
https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L151

Ok I will keep this in my mind next time I update my patchs.

Quoted 3 lines

Best regards,
Krzysztof

-Anand

maintainer Acked-by on v2 1/3
#4 lore raw

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

In reply to Anand Moon
Quoted 4 lines · Anand Moon

This patch adds 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.

Quoted 8 lines

---
Previous changes:
Added the missing clock name for Exynos5420 complatible

---
 Documentation/devicetree/bindings/usb/exynos-usb.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
#5 lore raw

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

In reply to Anand Moon
Quoted 5 lines · Anand Moon

This patch adds 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.

doesn't apply:

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

Signature · 1 line
balbi
author
#6 lore raw

PATCH v2 2/3 ARM: dts: exynos: Add missing usbdrd3 suspend clk

In reply to Anand Moon

This patch adds new combatible 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.

---
Previous changes:
Added missing suspend clk name Exynos5410 SoC.

---
 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(-)
arch/arm/boot/dts/exynos5410.dtsi +4 -4
index 2eab80bf5f3a..19845dcd528f 100644
@@ -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 {
arch/arm/boot/dts/exynos5420.dtsi +4 -4
index b672080e7469..bd505256a223 100644
@@ -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 {
arch/arm/boot/dts/exynos54xx.dtsi +2 -2
index 8aa5117e58ce..0aac6255de5d 100644
@@ -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;
Signature · 1 line
2.25.1
author
#7 lore raw

PATCH v2 3/3 usb: dwc3: exynos: Add support for Exynos5422 suspend clk

In reply to 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.

---
Append the Power Managment State control by the suspend clk for USB3.0

---
 drivers/usb/dwc3/dwc3-exynos.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/usb/dwc3/dwc3-exynos.c +9 -0
index 90bb022737da..48b68b6f0dc8 100644
@@ -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,
Signature · 1 line
2.25.1
#8 lore raw
In reply to Anand Moon
Quoted 8 lines · Anand Moon

Seried build and tested on linux next-20200228.

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.

That's not entirely correct. You enable there SCLK which is a "special clock", not a "suspend clock". You use word "suspend: in multiple places in commits making an impression that it is about some suspend clock... no, there is no suspend clock.

There is however a clock which driver calls suspend_clk (but it is just some name) and it is being enabled for entire lifetime of device (so also during suspend). AFAIU, this is not needed for Exynos5422 but I am not sure. So please convince me...

However I have still the same questions:
1. What problem are you trying to solve here?
2. Why this is needed?
3. What is fixed with this patch?

Best regards,
Krzysztof

Quoted 34 lines

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 (3):
devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3
clocks support
ARM: dts: exynos: Add missing usbdrd3 suspend clk
usb: dwc3: exynos: Add support for Exynos5422 suspend clk

 Documentation/devicetree/bindings/usb/exynos-usb.txt | 5 ++++-
 arch/arm/boot/dts/exynos5410.dtsi                    | 8 ++++----
 arch/arm/boot/dts/exynos5420.dtsi                    | 8 ++++----
 arch/arm/boot/dts/exynos54xx.dtsi                    | 4 ++--
 drivers/usb/dwc3/dwc3-exynos.c                       | 9 +++++++++
 5 files changed, 23 insertions(+), 11 deletions(-)
Signature · 1 line
2.25.1
author
#9 lore raw

Hi Krzysztof,

Quoted 11 lines · Krzysztof Kozlowski
Quoted 8 lines · Anand Moon

Seried build and tested on linux next-20200228.

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.

Quoted 5 lines

That's not entirely correct. You enable there SCLK which is a "special clock", not a "suspend clock". You use word "suspend: in multiple places in commits making an impression that it is about some suspend clock... no, there is no suspend clock.

Ok

Quoted 5 lines

There is however a clock which driver calls suspend_clk (but it is just some name) and it is being enabled for entire lifetime of device (so also during suspend). AFAIU, this is not needed for Exynos5422 but I am not sure. So please convince me...

Yep you are absolutely correct. Yes all the CLK_SLK* are call special clk's

Earlier I had share the FSYS clk diagram for Exynos5422
[0] https://imgur.com/gallery/zAiBoyh
from the diagram I mapped the naming terminology.

CLKMUX_USBDRD300 --->CLKDIV_USBDRD300 ---> SCLK_USBDRD300 (48 MHz) ---> USBDRD30_0 (SUSPEND_CLK)

                                   |
                                   |--->CLKDIV_USBPHY300--->

SCLK_USBPHY300 (48 MHZ) ---> USBDRD30_PHY_0 (USB30_SCLK_100M | USB20_PICO_CLKCORE)

CLKMUX_USBDRD301 --->CLKDIV_USBDRD301 ---> SCLK_USBDRD301 (48 MHz) ---> USBDRD30_1 (SUSPEND_CLK)

                                   |
                                   |--->CLKDIV_USBPHY301--->

SCLK_USBPHY301 (48 MHZ) ---> USBDRD30_PHY_1 (USB30_SCLK_100M)

SCLK_USBDRD300      USBDRD30_0             operating clock to 24 MHz
SCLK_USBDRD301      USBDRD30_PHY_0    operating clock to 24 MHz
SCLK_USBPHY300      USBPHY30_0             operating clock to 24 MHz
SCLK_USBPHY301      USBDRD30_PHY_1    operating clock to 24 Mhz
Quoted 3 lines

However I have still the same questions:
1. What problem are you trying to solve here?
2. Why this is needed?

I am trying to get the USB clk to get enabled for FSYS power domain to working efficiently.

Quoted 1 line

3. What is fixed with this patch?

Currently locally I tried to enable the FSYS power domain for USB 3.0 / USB 2.0. but it's not working as expected, need future study.

*Note:* For now plz discard these patches.
When I get the FSYS power domain to work correctly. I will link with those patch which will be better for testing.

-Anand

Quoted 4 lines

Best regards,
Krzysztof