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
