[PATCH v3 0/4] clk: renesas: rcar-usb2-clock-sel: Fix clks/resets handling

[PATCH v3 0/4] clk: renesas: rcar-usb2-clock-sel: Fix clks/resets handling

[Yoshihiro Shimoda]

This patch series is based on the latest renesas-drivers.git /
clk-renesas-for-v5.5 branch.
The hardware also needs multiple clocks/resets management like
renesas_usbhs driver [1], so this patch series fixes it.

[1]
3df0e24 usb: renesas_usbhs: Add multiple clocks management
f181dbb usb: renesas_usbhs: Add reset_control

Changes from v2:
 - Add Rob's Reviewed-by into the patch [12]/4.
 - Fix typo in patch 2/4.
 https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=194309

Changes from v1:
 - Add Reviewed-by into this series' patch 1/4.
 - (new) Add resets and power-domains properties into the patch 2/4.
 - Use clk_bulk_* APIs (except clk_bulk_get() because this driver has
   4 clocks and used only 2 clocks).
 - Add "select RESET_CONTROLLER" into Kconfig
 - Use devm_reset_control_array_get() instead of optional API.
 https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=192869

Yoshihiro Shimoda (4):
  dt-bindings: clock: renesas: rcar-usb2-clock-sel: Fix clock[-name]s
    properties
  dt-bindings: clock: renesas: rcar-usb2-clock-sel: Add power-domains
    and resets properties
  clk: renesas: rcar-usb2-clock-sel: Add multiple clocks management
  clk: renesas: rcar-usb2-clock-sel: Add reset_control

 .../bindings/clock/renesas,rcar-usb2-clock-sel.txt | 13 +++++--
 drivers/clk/renesas/Kconfig                        |  1 +
 drivers/clk/renesas/rcar-usb2-clock-sel.c          | 43 +++++++++++++++++++++-
 3 files changed, 52 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH v3 1/4] dt-bindings: clock: renesas: rcar-usb2-clock-sel: Fix clock[-name]s properties

[Yoshihiro Shimoda]

Since the hardware requires to enable both USB 2.0 host and peripheral
functional clock, this patch fixes the documentation.
Fortunately, no one has this device node for now, so that we don't
need to think of backward compatibility.

Fixes: 311accb64570 ("clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt     | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
index 83f6c6a..5c1903f 100644
--- a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
@@ -38,7 +38,8 @@ Required properties:
 - reg: offset and length of the USB 2.0 clock selector register block.
 - clocks: A list of phandles and specifier pairs.
 - clock-names: Name of the clocks.
- - The functional clock must be "ehci_ohci"
+ - The functional clock of USB 2.0 host side must be "ehci_ohci"
+ - The functional clock of HS-USB side must be "hs-usb-if"
  - The USB_EXTAL clock pin must be "usb_extal"
  - The USB_XTAL clock pin must be "usb_xtal"
 - #clock-cells: Must be 0
@@ -49,7 +50,8 @@ Example (R-Car H3):
 		compatible = "renesas,r8a7795-rcar-usb2-clock-sel",
 			     "renesas,rcar-gen3-usb2-clock-sel";
 		reg = <0 0xe6590630 0 0x02>;
-		clocks = <&cpg CPG_MOD 703>, <&usb_extal>, <&usb_xtal>;
-		clock-names = "ehci_ohci", "usb_extal", "usb_xtal";
+		clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>,
+			 <&usb_extal>, <&usb_xtal>;
+		clock-names = "ehci_ohci", "hs-usb-if", "usb_extal", "usb_xtal";
 		#clock-cells = <0>;
 	};
-- 
2.7.4

[PATCH v3 2/4] dt-bindings: clock: renesas: rcar-usb2-clock-sel: Add power-domains and resets properties

[Yoshihiro Shimoda]

This patch adds missing required properties of power-domains and resets.
Fortunately, no one has this device node for now, so that we don't
need to think of backward compatibility.

Fixes: 311accb64570 ("clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt        | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
index 5c1903f..8e96640 100644
--- a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
@@ -43,6 +43,9 @@ Required properties:
  - The USB_EXTAL clock pin must be "usb_extal"
  - The USB_XTAL clock pin must be "usb_xtal"
 - #clock-cells: Must be 0
+- power-domains: A phandle and symbolic PM domain specifier.
+                 See power/renesas,rcar-sysc.txt.
+- resets: A list of phandles and specifier pairs.
 
 Example (R-Car H3):
 
@@ -54,4 +57,6 @@ Example (R-Car H3):
 			 <&usb_extal>, <&usb_xtal>;
 		clock-names = "ehci_ohci", "hs-usb-if", "usb_extal", "usb_xtal";
 		#clock-cells = <0>;
+		power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
+		resets = <&cpg 703>, <&cpg 704>;
 	};
-- 
2.7.4

Re: [PATCH v3 2/4] dt-bindings: clock: renesas: rcar-usb2-clock-sel: Add power-domains and resets properties

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Nov 1, 2019 at 8:03 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This patch adds missing required properties of power-domains and resets.
> Fortunately, no one has this device node for now, so that we don't
> need to think of backward compatibility.
>
> Fixes: 311accb64570 ("clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> @@ -43,6 +43,9 @@ Required properties:
>   - The USB_EXTAL clock pin must be "usb_extal"
>   - The USB_XTAL clock pin must be "usb_xtal"
>  - #clock-cells: Must be 0
> +- power-domains: A phandle and symbolic PM domain specifier.
> +                 See power/renesas,rcar-sysc.txt.
> +- resets: A list of phandles and specifier pairs.

Since there is more than one, I think it would be good to specify
reset-names, too ("ehci_ohci" and "hs-usb-if").

The rest looks good to me.

>  Example (R-Car H3):
>
> @@ -54,4 +57,6 @@ Example (R-Car H3):
>                          <&usb_extal>, <&usb_xtal>;
>                 clock-names = "ehci_ohci", "hs-usb-if", "usb_extal", "usb_xtal";
>                 #clock-cells = <0>;
> +               power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> +               resets = <&cpg 703>, <&cpg 704>;
>         };

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3 2/4] dt-bindings: clock: renesas: rcar-usb2-clock-sel: Add power-domains and resets properties

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for your review and I completely overlooked this email...

> From: Geert Uytterhoeven, Sent: Monday, November 18, 2019 7:12 PM
> 
> Hi Shimoda-san,
> 
> On Fri, Nov 1, 2019 at 8:03 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > This patch adds missing required properties of power-domains and resets.
> > Fortunately, no one has this device node for now, so that we don't
> > need to think of backward compatibility.
> >
> > Fixes: 311accb64570 ("clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY")
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> 
> Thanks for your patch!
> 
> > --- a/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> > +++ b/Documentation/devicetree/bindings/clock/renesas,rcar-usb2-clock-sel.txt
> > @@ -43,6 +43,9 @@ Required properties:
> >   - The USB_EXTAL clock pin must be "usb_extal"
> >   - The USB_XTAL clock pin must be "usb_xtal"
> >  - #clock-cells: Must be 0
> > +- power-domains: A phandle and symbolic PM domain specifier.
> > +                 See power/renesas,rcar-sysc.txt.
> > +- resets: A list of phandles and specifier pairs.
> 
> Since there is more than one, I think it would be good to specify
> reset-names, too ("ehci_ohci" and "hs-usb-if").

I got it. I'll add reset-names.

Best regards,
Yoshihiro Shimoda

> The rest looks good to me.
> 
> >  Example (R-Car H3):
> >
> > @@ -54,4 +57,6 @@ Example (R-Car H3):
> >                          <&usb_extal>, <&usb_xtal>;
> >                 clock-names = "ehci_ohci", "hs-usb-if", "usb_extal", "usb_xtal";
> >                 #clock-cells = <0>;
> > +               power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> > +               resets = <&cpg 703>, <&cpg 704>;
> >         };
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

[PATCH v3 3/4] clk: renesas: rcar-usb2-clock-sel: Add multiple clocks management

[Yoshihiro Shimoda]

This hardware needs to enable clocks of both host and peripheral.
So, this patch adds multiple clocks management.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/clk/renesas/rcar-usb2-clock-sel.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/renesas/rcar-usb2-clock-sel.c b/drivers/clk/renesas/rcar-usb2-clock-sel.c
index b97f5f9..4096506 100644
--- a/drivers/clk/renesas/rcar-usb2-clock-sel.c
+++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c
@@ -26,9 +26,16 @@
 #define CLKSET0_PRIVATE		BIT(0)
 #define CLKSET0_EXTAL_ONLY	(CLKSET0_INTCLK_EN | CLKSET0_PRIVATE)
 
+enum {
+	CLK_INDEX_EHCI_OHCI,
+	CLK_INDEX_HS_USB,
+	CLK_NUM
+};
+
 struct usb2_clock_sel_priv {
 	void __iomem *base;
 	struct clk_hw hw;
+	struct clk_bulk_data clks[CLK_NUM];
 	bool extal;
 	bool xtal;
 };
@@ -53,14 +60,25 @@ static void usb2_clock_sel_disable_extal_only(struct usb2_clock_sel_priv *priv)
 
 static int usb2_clock_sel_enable(struct clk_hw *hw)
 {
-	usb2_clock_sel_enable_extal_only(to_priv(hw));
+	struct usb2_clock_sel_priv *priv = to_priv(hw);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(CLK_NUM, priv->clks);
+	if (ret)
+		return ret;
+
+	usb2_clock_sel_enable_extal_only(priv);
 
 	return 0;
 }
 
 static void usb2_clock_sel_disable(struct clk_hw *hw)
 {
-	usb2_clock_sel_disable_extal_only(to_priv(hw));
+	struct usb2_clock_sel_priv *priv = to_priv(hw);
+
+	usb2_clock_sel_disable_extal_only(priv);
+
+	clk_bulk_disable_unprepare(CLK_NUM, priv->clks);
 }
 
 /*
@@ -128,6 +146,14 @@ static int rcar_usb2_clock_sel_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
+	priv->clks[CLK_INDEX_EHCI_OHCI].clk = devm_clk_get(dev, "ehci_ohci");
+	if (IS_ERR(priv->clks[CLK_INDEX_EHCI_OHCI].clk))
+		return PTR_ERR(priv->clks[CLK_INDEX_EHCI_OHCI].clk);
+
+	priv->clks[CLK_INDEX_HS_USB].clk = devm_clk_get(dev, "hs-usb-if");
+	if (IS_ERR(priv->clks[CLK_INDEX_HS_USB].clk))
+		return PTR_ERR(priv->clks[CLK_INDEX_HS_USB].clk);
+
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
-- 
2.7.4

Re: [PATCH v3 3/4] clk: renesas: rcar-usb2-clock-sel: Add multiple clocks management

[Geert Uytterhoeven]

Hi Shimoda-san,

On Fri, Nov 1, 2019 at 8:03 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This hardware needs to enable clocks of both host and peripheral.
> So, this patch adds multiple clocks management.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/rcar-usb2-clock-sel.c
> +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c

> @@ -128,6 +146,14 @@ static int rcar_usb2_clock_sel_probe(struct platform_device *pdev)
>         if (IS_ERR(priv->base))
>                 return PTR_ERR(priv->base);
>
> +       priv->clks[CLK_INDEX_EHCI_OHCI].clk = devm_clk_get(dev, "ehci_ohci");
> +       if (IS_ERR(priv->clks[CLK_INDEX_EHCI_OHCI].clk))
> +               return PTR_ERR(priv->clks[CLK_INDEX_EHCI_OHCI].clk);
> +
> +       priv->clks[CLK_INDEX_HS_USB].clk = devm_clk_get(dev, "hs-usb-if");
> +       if (IS_ERR(priv->clks[CLK_INDEX_HS_USB].clk))
> +               return PTR_ERR(priv->clks[CLK_INDEX_HS_USB].clk);
> +

Is these any specific reason you're not just filling in the .id fields first,
and calling devm_clk_bulk_get()?

    static const struct clk_bulk_data rcar_usb2_clocks[] = {
            { .id = "ehci_ohci", },
            { .id = "hs-usb-if", },
    };

    memcpy(priv->clks, rcar_usb2_clocks, sizeof(priv->clks));
    ... = devm_clk_bulk_get(dev, ARRAY_SIZE(priv->clks), priv->clks);
    ...

That way you can drop the enums, and use ARRAY_SIZE(rcar_usb2_clocks)
instead of CLK_NUM.

>         pm_runtime_enable(dev);
>         pm_runtime_get_sync(dev);
>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3 3/4] clk: renesas: rcar-usb2-clock-sel: Add multiple clocks management

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for your review!

> From: Geert Uytterhoeven, Sent: Monday, November 18, 2019 7:25 PM
> 
> Hi Shimoda-san,
> 
> On Fri, Nov 1, 2019 at 8:03 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
<snip>
> > --- a/drivers/clk/renesas/rcar-usb2-clock-sel.c
> > +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c
> 
> > @@ -128,6 +146,14 @@ static int rcar_usb2_clock_sel_probe(struct platform_device *pdev)
> >         if (IS_ERR(priv->base))
> >                 return PTR_ERR(priv->base);
> >
> > +       priv->clks[CLK_INDEX_EHCI_OHCI].clk = devm_clk_get(dev, "ehci_ohci");
> > +       if (IS_ERR(priv->clks[CLK_INDEX_EHCI_OHCI].clk))
> > +               return PTR_ERR(priv->clks[CLK_INDEX_EHCI_OHCI].clk);
> > +
> > +       priv->clks[CLK_INDEX_HS_USB].clk = devm_clk_get(dev, "hs-usb-if");
> > +       if (IS_ERR(priv->clks[CLK_INDEX_HS_USB].clk))
> > +               return PTR_ERR(priv->clks[CLK_INDEX_HS_USB].clk);
> > +
> 
> Is these any specific reason you're not just filling in the .id fields first,
> and calling devm_clk_bulk_get()?
> 
>     static const struct clk_bulk_data rcar_usb2_clocks[] = {
>             { .id = "ehci_ohci", },
>             { .id = "hs-usb-if", },
>     };
> 
>     memcpy(priv->clks, rcar_usb2_clocks, sizeof(priv->clks));
>     ... = devm_clk_bulk_get(dev, ARRAY_SIZE(priv->clks), priv->clks);
>     ...
> 
> That way you can drop the enums, and use ARRAY_SIZE(rcar_usb2_clocks)
> instead of CLK_NUM.

I don't know why I didn't use devm_clk_bulk_get()...
I'll fix the code.

Best regards,
Yoshihiro Shimoda

[PATCH v3 4/4] clk: renesas: rcar-usb2-clock-sel: Add reset_control

[Yoshihiro Shimoda]

This hardware needs to deassert resets of both host and peripheral.
So, this patch adds reset control.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/clk/renesas/Kconfig               |  1 +
 drivers/clk/renesas/rcar-usb2-clock-sel.c | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index be03bb7..a76d05af 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -156,6 +156,7 @@ config CLK_RCAR_GEN3_CPG
 config CLK_RCAR_USB2_CLOCK_SEL
 	bool "Renesas R-Car USB2 clock selector support"
 	depends on ARCH_RENESAS || COMPILE_TEST
+	select RESET_CONTROLLER
 	help
 	  This is a driver for R-Car USB2 clock selector
 
diff --git a/drivers/clk/renesas/rcar-usb2-clock-sel.c b/drivers/clk/renesas/rcar-usb2-clock-sel.c
index 4096506..1cdcc8f 100644
--- a/drivers/clk/renesas/rcar-usb2-clock-sel.c
+++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 
 #define USB20_CLKSET0		0x00
@@ -36,6 +37,7 @@ struct usb2_clock_sel_priv {
 	void __iomem *base;
 	struct clk_hw hw;
 	struct clk_bulk_data clks[CLK_NUM];
+	struct reset_control *rsts;
 	bool extal;
 	bool xtal;
 };
@@ -63,10 +65,16 @@ static int usb2_clock_sel_enable(struct clk_hw *hw)
 	struct usb2_clock_sel_priv *priv = to_priv(hw);
 	int ret;
 
-	ret = clk_bulk_prepare_enable(CLK_NUM, priv->clks);
+	ret = reset_control_deassert(priv->rsts);
 	if (ret)
 		return ret;
 
+	ret = clk_bulk_prepare_enable(CLK_NUM, priv->clks);
+	if (ret) {
+		reset_control_assert(priv->rsts);
+		return ret;
+	}
+
 	usb2_clock_sel_enable_extal_only(priv);
 
 	return 0;
@@ -79,6 +87,7 @@ static void usb2_clock_sel_disable(struct clk_hw *hw)
 	usb2_clock_sel_disable_extal_only(priv);
 
 	clk_bulk_disable_unprepare(CLK_NUM, priv->clks);
+	reset_control_assert(priv->rsts);
 }
 
 /*
@@ -154,6 +163,10 @@ static int rcar_usb2_clock_sel_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->clks[CLK_INDEX_HS_USB].clk))
 		return PTR_ERR(priv->clks[CLK_INDEX_HS_USB].clk);
 
+	priv->rsts = devm_reset_control_array_get(dev, true, false);
+	if (IS_ERR(priv->rsts))
+		return PTR_ERR(priv->rsts);
+
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
-- 
2.7.4

Re: [PATCH v3 4/4] clk: renesas: rcar-usb2-clock-sel: Add reset_control

[Geert Uytterhoeven]

On Fri, Nov 1, 2019 at 8:03 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This hardware needs to deassert resets of both host and peripheral.
> So, this patch adds reset control.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Not yet queueing in clk-renesas-for-v5.6, as this depends on earlier
patches in the series.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds