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

[Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>]

Hi Geert-san,

Thank you for your comments!

> From: Geert Uytterhoeven, Sent: Thursday, October 24, 2019 8:35 PM
<snip>
> > --- a/drivers/clk/renesas/rcar-usb2-clock-sel.c
> > +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c
> > @@ -53,14 +60,32 @@ 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 i, ret;
> > +
> > +       for (i = 0; i < CLK_NUM; i++) {
> > +               ret = clk_prepare_enable(priv->clks[i]);
> > +               if (ret) {
> > +                       while (--i >= 0)
> > +                               clk_disable_unprepare(priv->clks[i]);
> > +                       return ret;
> > +               }
> > +       }
> 
> You can use the clk_bulk_* APIs, instead of open-coding the same
> operation.

I didn't know such APIs. I'll fix it.

> > @@ -131,6 +156,14 @@ static int rcar_usb2_clock_sel_probe(struct platform_device *pdev)
> >         pm_runtime_enable(dev);
> >         pm_runtime_get_sync(dev);
> 
> pm_runtime_get_sync() will have already enabled the first module clock listed in
> the DT "clocks" property.
> 
> If you want the driver to manage all clocks itself, perhaps the PM Runtime
> calls should be dropped?

I'm thinking PM Runtime calls are related to power domain control so that we cannot
drop it. Or, since the hardware is the Always-on domain, can we drop it anyway?

Best regards,
Yoshihiro Shimoda