| Message ID | 1572591791-11280-4-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | clk: renesas: rcar-usb2-clock-sel: Fix clks/resets handling | expand |
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
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 --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);
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(-)