linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC
@ 2020-02-10 10:51 Anand Moon
  2020-02-10 10:51 ` [PATCHv3 1/3] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support Anand Moon
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Anand Moon @ 2020-02-10 10:51 UTC (permalink / raw)
  To: linux-usb, devicetree, linux-arm-kernel
  Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Krzysztof Kozlowski, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	linux-samsung-soc, linux-kernel

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/

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.

-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 | 4 +++-
 arch/arm/boot/dts/exynos5420.dtsi                    | 8 ++++----
 arch/arm/boot/dts/exynos54xx.dtsi                    | 4 ++--
 drivers/usb/dwc3/dwc3-exynos.c                       | 9 +++++++++
 4 files changed, 18 insertions(+), 7 deletions(-)

-- 
2.25.0


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

* [PATCHv3 1/3] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support
  2020-02-10 10:51 [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
@ 2020-02-10 10:51 ` Anand Moon
  2020-02-10 10:51 ` [PATCHv3 2/3] ARM: dts: exynos: Add missing usbdrd3 suspend clk Anand Moon
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Anand Moon @ 2020-02-10 10:51 UTC (permalink / raw)
  To: linux-usb, devicetree, linux-arm-kernel
  Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Krzysztof Kozlowski, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	linux-samsung-soc, linux-kernel

This patch adds the new compatible string for Exynos5422 DWC3
to support enable/disable of core and suspend clk by DWC3 driver.

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

diff --git a/Documentation/devicetree/bindings/usb/exynos-usb.txt b/Documentation/devicetree/bindings/usb/exynos-usb.txt
index 66c394f9e11f..6d27f4c0e5a2 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.
-- 
2.25.0


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

* [PATCHv3 2/3] ARM: dts: exynos: Add missing usbdrd3 suspend clk
  2020-02-10 10:51 [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
  2020-02-10 10:51 ` [PATCHv3 1/3] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support Anand Moon
@ 2020-02-10 10:51 ` Anand Moon
  2020-02-10 13:50   ` Krzysztof Kozlowski
  2020-02-10 10:51 ` [PATCHv3 3/3] usb: dwc3: exynos: Add support for Exynos5422 " Anand Moon
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Anand Moon @ 2020-02-10 10:51 UTC (permalink / raw)
  To: linux-usb, devicetree, linux-arm-kernel
  Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Krzysztof Kozlowski, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	linux-samsung-soc, linux-kernel

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.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 arch/arm/boot/dts/exynos5420.dtsi | 8 ++++----
 arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

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.0


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

* [PATCHv3 3/3] usb: dwc3: exynos: Add support for Exynos5422 suspend clk
  2020-02-10 10:51 [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
  2020-02-10 10:51 ` [PATCHv3 1/3] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support Anand Moon
  2020-02-10 10:51 ` [PATCHv3 2/3] ARM: dts: exynos: Add missing usbdrd3 suspend clk Anand Moon
@ 2020-02-10 10:51 ` Anand Moon
  2020-02-10 10:56 ` [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
  2020-02-10 13:56 ` Krzysztof Kozlowski
  4 siblings, 0 replies; 11+ messages in thread
From: Anand Moon @ 2020-02-10 10:51 UTC (permalink / raw)
  To: linux-usb, devicetree, linux-arm-kernel
  Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Krzysztof Kozlowski, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	linux-samsung-soc, linux-kernel

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 clk.

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.0


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

* Re: [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC
  2020-02-10 10:51 [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
                   ` (2 preceding siblings ...)
  2020-02-10 10:51 ` [PATCHv3 3/3] usb: dwc3: exynos: Add support for Exynos5422 " Anand Moon
@ 2020-02-10 10:56 ` Anand Moon
  2020-02-10 13:56 ` Krzysztof Kozlowski
  4 siblings, 0 replies; 11+ messages in thread
From: Anand Moon @ 2020-02-10 10:56 UTC (permalink / raw)
  To: Linux USB Mailing List, devicetree, linux-arm-kernel
  Cc: Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Krzysztof Kozlowski, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	linux-samsung-soc, Linux Kernel

Hi All,

Sorry typo this patch series should be PATCHv1 and not PATCHv3

-Anand

On Mon, 10 Feb 2020 at 16:21, Anand Moon <linux.amoon@gmail.com> wrote:
>
> 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/
>
> 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.
>
> -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 | 4 +++-
>  arch/arm/boot/dts/exynos5420.dtsi                    | 8 ++++----
>  arch/arm/boot/dts/exynos54xx.dtsi                    | 4 ++--
>  drivers/usb/dwc3/dwc3-exynos.c                       | 9 +++++++++
>  4 files changed, 18 insertions(+), 7 deletions(-)
>
> --
> 2.25.0
>

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

* Re: [PATCHv3 2/3] ARM: dts: exynos: Add missing usbdrd3 suspend clk
  2020-02-10 10:51 ` [PATCHv3 2/3] ARM: dts: exynos: Add missing usbdrd3 suspend clk Anand Moon
@ 2020-02-10 13:50   ` Krzysztof Kozlowski
  2020-02-10 17:13     ` Anand Moon
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2020-02-10 13:50 UTC (permalink / raw)
  To: Anand Moon
  Cc: linux-usb, devicetree, linux-arm-kernel, Greg Kroah-Hartman,
	Rob Herring, Mark Rutland, Felipe Balbi, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, linux-samsung-soc, linux-kernel

On Mon, Feb 10, 2020 at 10:51:07AM +0000, Anand Moon wrote:
> 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.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  arch/arm/boot/dts/exynos5420.dtsi | 8 ++++----
>  arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> 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";

This affects also Exynos5410 but you do not add new clock there.

Best regards,
Krzysztof


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

* Re: [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC
  2020-02-10 10:51 [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
                   ` (3 preceding siblings ...)
  2020-02-10 10:56 ` [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
@ 2020-02-10 13:56 ` Krzysztof Kozlowski
  2020-02-10 17:08   ` Anand Moon
  4 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2020-02-10 13:56 UTC (permalink / raw)
  To: Anand Moon
  Cc: linux-usb, devicetree, linux-arm-kernel, Greg Kroah-Hartman,
	Rob Herring, Mark Rutland, Felipe Balbi, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, linux-samsung-soc, linux-kernel

On Mon, Feb 10, 2020 at 10:51:05AM +0000, Anand Moon wrote:
> 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/
> 

You ignored parts of my review from these previous patches. I asked for
describing WHY are you doing this and WHAT problem are you trying to
solve. I asked for this multiple times. Unfortunately I cannot find the
answers to my questions in this patchset...

Best regards,
Krzysztof


> 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.
> 
> -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 | 4 +++-
>  arch/arm/boot/dts/exynos5420.dtsi                    | 8 ++++----
>  arch/arm/boot/dts/exynos54xx.dtsi                    | 4 ++--
>  drivers/usb/dwc3/dwc3-exynos.c                       | 9 +++++++++
>  4 files changed, 18 insertions(+), 7 deletions(-)
> 
> -- 
> 2.25.0
> 

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

* Re: [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC
  2020-02-10 13:56 ` Krzysztof Kozlowski
@ 2020-02-10 17:08   ` Anand Moon
  2020-02-10 19:02     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Anand Moon @ 2020-02-10 17:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Linux USB Mailing List, devicetree, linux-arm-kernel,
	Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Marek Szyprowski, Bartlomiej Zolnierkiewicz, linux-samsung-soc,
	Linux Kernel

Hi Krzysztof,

On Mon, 10 Feb 2020 at 19:26, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Feb 10, 2020 at 10:51:05AM +0000, Anand Moon wrote:
> > 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/
> >
>

Thanks for your review comments.

> You ignored parts of my review from these previous patches. I asked for
> describing WHY are you doing this and WHAT problem are you trying to
> solve. I asked for this multiple times. Unfortunately I cannot find the
> answers to my questions in this patchset...
>
> Best regards,
> Krzysztof

I dont know how to resolve this issue, but I want to re-post
some of my changes back for review. let me try again.

My future goal is to add #power-domain for FSYS and FSYS2
which I am trying to resolve some issue.
Also add run-time power management for USB3 drivers.

Here is the clk diagram for FSYS clk as per Exynos5422 user manual.
[0] https://imgur.com/gallery/zAiBoyh

As per the USB 3.0 Architecture T I.

2.13.1 PHY Power Management
The SS PHY has power states P0, P1, P2, and P3, corresponding to the
SS LPM states of U0, U1, U2,and U3. In the P3 state,SS PHY does not drive
the default functional clock,instead, the *susp_clk* is used in its place.

So enable the suspend clk help control the power management
states for the DWC3 controller.

-Anand

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

* Re: [PATCHv3 2/3] ARM: dts: exynos: Add missing usbdrd3 suspend clk
  2020-02-10 13:50   ` Krzysztof Kozlowski
@ 2020-02-10 17:13     ` Anand Moon
  2020-02-10 18:54       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Anand Moon @ 2020-02-10 17:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Linux USB Mailing List, devicetree, linux-arm-kernel,
	Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Marek Szyprowski, Bartlomiej Zolnierkiewicz, linux-samsung-soc,
	Linux Kernel

Hi Krzysztof,

Thanks for your review comments.

On Mon, 10 Feb 2020 at 19:20, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Feb 10, 2020 at 10:51:07AM +0000, Anand Moon wrote:
> > 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.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  arch/arm/boot/dts/exynos5420.dtsi | 8 ++++----
> >  arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > 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";
>
> This affects also Exynos5410 but you do not add new clock there.
>
> Best regards,
> Krzysztof
>

Ok I will update this Exynos5410 dts.

Is samsung,exynos54xx-dwusb3 is valid compatible string
for both the SoC.

-Anand

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

* Re: [PATCHv3 2/3] ARM: dts: exynos: Add missing usbdrd3 suspend clk
  2020-02-10 17:13     ` Anand Moon
@ 2020-02-10 18:54       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2020-02-10 18:54 UTC (permalink / raw)
  To: Anand Moon
  Cc: Linux USB Mailing List, devicetree, linux-arm-kernel,
	Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Marek Szyprowski, Bartlomiej Zolnierkiewicz, linux-samsung-soc,
	Linux Kernel

On Mon, Feb 10, 2020 at 10:43:45PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> Thanks for your review comments.
> 
> On Mon, 10 Feb 2020 at 19:20, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Mon, Feb 10, 2020 at 10:51:07AM +0000, Anand Moon wrote:
> > > 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.
> > >
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > >  arch/arm/boot/dts/exynos5420.dtsi | 8 ++++----
> > >  arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
> > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > >
> > > 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";
> >
> > This affects also Exynos5410 but you do not add new clock there.
> >
> > Best regards,
> > Krzysztof
> >
> 
> Ok I will update this Exynos5410 dts.
> 
> Is samsung,exynos54xx-dwusb3 is valid compatible string
> for both the SoC.

The compatible should not be wildcard so keep it as
samsung,exynos5420-dwusb3.

Best regards,
Krzysztof


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

* Re: [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC
  2020-02-10 17:08   ` Anand Moon
@ 2020-02-10 19:02     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2020-02-10 19:02 UTC (permalink / raw)
  To: Anand Moon
  Cc: Linux USB Mailing List, devicetree, linux-arm-kernel,
	Greg Kroah-Hartman, Rob Herring, Mark Rutland, Felipe Balbi,
	Marek Szyprowski, Bartlomiej Zolnierkiewicz, linux-samsung-soc,
	Linux Kernel

On Mon, Feb 10, 2020 at 10:38:52PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Mon, 10 Feb 2020 at 19:26, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Mon, Feb 10, 2020 at 10:51:05AM +0000, Anand Moon wrote:
> > > 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/
> > >
> >
> 
> Thanks for your review comments.
> 
> > You ignored parts of my review from these previous patches. I asked for
> > describing WHY are you doing this and WHAT problem are you trying to
> > solve. I asked for this multiple times. Unfortunately I cannot find the
> > answers to my questions in this patchset...
> >
> > Best regards,
> > Krzysztof
> 
> I dont know how to resolve this issue, but I want to re-post
> some of my changes back for review. let me try again.
> 
> My future goal is to add #power-domain for FSYS and FSYS2
> which I am trying to resolve some issue.
> Also add run-time power management for USB3 drivers.

You can start by describing why FSYS and FSYS2 power domains cannot be
added right now. Maybe this patchset allows this later?

> 
> Here is the clk diagram for FSYS clk as per Exynos5422 user manual.
> [0] https://imgur.com/gallery/zAiBoyh
> 
> As per the USB 3.0 Architecture T I.
> 
> 2.13.1 PHY Power Management
> The SS PHY has power states P0, P1, P2, and P3, corresponding to the
> SS LPM states of U0, U1, U2,and U3. In the P3 state,SS PHY does not drive
> the default functional clock,instead, the *susp_clk* is used in its place.
> 
> So enable the suspend clk help control the power management
> states for the DWC3 controller.

That's too vague because clock usually cannot "help"... The wording is
wrong and the actual problem is not described.

I could guess from your description and driver behavior that SCLK has to
be on during USB DRD suspend.

Best regards,
Krzysztof


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

end of thread, other threads:[~2020-02-10 19:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 10:51 [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
2020-02-10 10:51 ` [PATCHv3 1/3] devicetree: bindings: exynos: Add new compatible for Exynos5420 dwc3 clocks support Anand Moon
2020-02-10 10:51 ` [PATCHv3 2/3] ARM: dts: exynos: Add missing usbdrd3 suspend clk Anand Moon
2020-02-10 13:50   ` Krzysztof Kozlowski
2020-02-10 17:13     ` Anand Moon
2020-02-10 18:54       ` Krzysztof Kozlowski
2020-02-10 10:51 ` [PATCHv3 3/3] usb: dwc3: exynos: Add support for Exynos5422 " Anand Moon
2020-02-10 10:56 ` [PATCHv3 0/3] Add support for suspend clk for Exynos5422 SoC Anand Moon
2020-02-10 13:56 ` Krzysztof Kozlowski
2020-02-10 17:08   ` Anand Moon
2020-02-10 19:02     ` Krzysztof Kozlowski

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