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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Hi All,
Sorry typo this patch series should be PATCHv1 and not PATCHv3
-Anand
On Mon, 10 Feb 2020 at 16:21, Anand Moon <[email protected]> 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
>
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 <[email protected]>
> ---
> 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
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
>
Hi Krzysztof,
On Mon, 10 Feb 2020 at 19:26, Krzysztof Kozlowski <[email protected]> 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
Hi Krzysztof,
Thanks for your review comments.
On Mon, 10 Feb 2020 at 19:20, Krzysztof Kozlowski <[email protected]> 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 <[email protected]>
> > ---
> > 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
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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
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 <[email protected]> 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