RE: [PATCH] clk: renesas: cpg-mssr: Add R7S9210 support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux