diff mbox series

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

Message ID 1572591791-11280-4-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Awaiting Upstream, archived
Headers show
Series clk: renesas: rcar-usb2-clock-sel: Fix clks/resets handling | expand

Commit Message

Yoshihiro Shimoda Nov. 1, 2019, 7:03 a.m. UTC 
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(-)

Comments

Geert Uytterhoeven Nov. 18, 2019, 10:24 a.m. UTC | #1 
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
Yoshihiro Shimoda March 4, 2020, 6:32 a.m. UTC | #2 
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
diff mbox series

Patch

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