Hi Geert, Thank you for having a look at this. On Monday, August 27, 2018 1, Geert Uytterhoeven wrote: > Given the differences, and the limited amount of RAM on RZ/A2, I think you > would be better off with a separate renesas-cpg-stbcr driver, and an > r7s9210-cpg-stbcr counterpart. If you really think there will be a lot of wasted RAM, then I will look into it. So are you saying I should first copy/rename renesas-cpg-mssr.c to renesas-cpg-stbr.c and then start hacking away at it? > 4. Your module clocks can use e.g. "36" instead of "306" (also in the DTS), > matching the datasheet. Yes, that would be much nicer! Just FYI, I like this new driver method, but the one downfall is that I have to go back and change my timer driver (renesas-ostm.c). The current OSTM driver uses TIMER_OF_DECLARE(). But when you use TIMER_OF_DECLARE, your timer driver gets probed VERY early in boot, before subsys_initcall, so none of your clocks are registered yet and the probe fails. Chris > > That means: > > > .../devicetree/bindings/clock/renesas,cpg-mssr.txt | 3 +- > > 1. A separate binding document. > > > drivers/clk/renesas/Kconfig | 5 + > > drivers/clk/renesas/Makefile | 1 + > > drivers/clk/renesas/r7s9210-cpg-mssr.c | 189 > +++++++++++++++++++++ > > drivers/clk/renesas/renesas-cpg-mssr.c | 66 +++++-- > > drivers/clk/renesas/renesas-cpg-mssr.h | 6 + > > include/dt-bindings/clock/r7s9210-cpg-mssr.h | 21 +++ > > 7 files changed, 280 insertions(+), 11 deletions(-) > > create mode 100644 drivers/clk/renesas/r7s9210-cpg-mssr.c > > create mode 100644 include/dt-bindings/clock/r7s9210-cpg-mssr.h > > > > diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg- > mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > > index db542abadb75..66ca973edd77 100644 > > --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > > +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > > @@ -13,6 +13,7 @@ They provide the following functionalities: > > > > Required Properties: > > - compatible: Must be one of: > > + - "renesas,r7s9210-cpg-mssr" for the r7s9210 SoC (RZ/A2) > > - "renesas,r8a7743-cpg-mssr" for the r8a7743 SoC (RZ/G1M) > > - "renesas,r8a7745-cpg-mssr" for the r8a7745 SoC (RZ/G1E) > > - "renesas,r8a77470-cpg-mssr" for the r8a77470 SoC (RZ/G1C) > > @@ -37,7 +38,7 @@ Required Properties: > > - clock-names: List of external parent clock names. Valid names are: > > - "extal" (r8a7743, r8a7745, r8a77470, r8a7790, r8a7791, r8a7792, > > r8a7793, r8a7794, r8a7795, r8a7796, r8a77965, r8a77970, > > - r8a77980, r8a77990, r8a77995) > > + r8a77980, r8a77990, r8a77995, r7s9210) > > - "extalr" (r8a7795, r8a7796, r8a77965, r8a77970, r8a77980) > > - "usb_extal" (r8a7743, r8a7745, r8a77470, r8a7790, r8a7791, > r8a7793, > > r8a7794) > > diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig > > index 9022bbe1297e..d8ccdaba5103 100644 > > --- a/drivers/clk/renesas/Kconfig > > +++ b/drivers/clk/renesas/Kconfig > > > @@ -45,6 +46,10 @@ config CLK_RZA1 > > bool "RZ/A1H clock support" if COMPILE_TEST > > select CLK_RENESAS_CPG_MSTP > > > > +config CLK_R7S9210 > > + bool "RZ/A2 clock support" if COMPILE_TEST > > + select CLK_RENESAS_CPG_MSSR > > 2. a separate CLK_RENESAS_CPG_STBCR symbol. > > > --- /dev/null > > +++ b/drivers/clk/renesas/r7s9210-cpg-mssr.c > > 3. Almost all of this can stay the same, modulo some renames. > > > +static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = { > > + DEF_MOD("ostm0", 306, R7S9210_CLK_P1C), > > 4. Your module clocks can use e.g. "36" instead of "306" (also in the DTS), > matching the datasheet. > > > --- /dev/null > > +++ b/include/dt-bindings/clock/r7s9210-cpg-mssr.h > > 5. Almost all of this can stay the same, modulo some renames. > > What do you think? > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > 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