Re: [PATCH v12 3/3] dt-bindings: mfd: Document Renesas R-Car Gen3 RPC-IF MFD bindings

From: Geert Uytterhoeven
Date: Mon May 20 2019 - 03:47:23 EST


Hi Mason,

On Mon, May 20, 2019 at 9:24 AM <masonccyang@xxxxxxxxxxx> wrote:
> > >>> - clocks: should contain 1 entries for the module's clock
> > >>> - clock-names: should contain "rpc"
> > >>
> > >> I suspect we'd need the RPC/RPCD2 clocks mentioned as well (not
> sure
> > > yet)...
> > >
> > > Need it ?
> >
> > You seem to call clk_get_rate() on the module clock, I doubt that's
> > correct topologically...
>
> I think it's correct but just like Geert mentioned that there is no any
> patch
> in drivers/clk/renesas/r8a77995-cpg-mssr.c adding RPC-related clocks.
>
>
> I patched dt-bindings/clock/r8a77995-cpg-mssr.h for some simple testing
>
> -#define R8A77995_CLK_RPC 29
> -#define R8A77995_CLK_RPCD2 30
> +#define R8A77995_CLK_RPC 31
> +#define R8A77995_CLK_RPCD2 32

That change doesn't make sense to me...

> by clk_prepare_enable() & clk_disable_unprepare() with CPG_MOD 917
> on D3 draak board, it is working fine.

... and is not sufficient to allow the above two calls.

Besides, making explicit clk_prepare_enable() calls bypasses Runtime PM
and the automatic disabling of unused clocks, thus hiding bugs related to
Runtime PM. Which is probably why your driver doesn't work for Sergei...


Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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