[RFC PATCH 0/4] Add CPG wrapper for Renesas RZ/Five SoC

[RFC PATCH 0/4] Add CPG wrapper for Renesas RZ/Five SoC

[Lad Prabhakar]

Hi All,

This patch series adds CPG wrapper for Renesas RZ/Five SoC. RZ/Five SoC
has almost identical clock structure compared to RZ/G2UL, so
r9a07g043-cpg.c file is re-used to add support for Renesas RZ/Five SoC.

Sending this as RFC in case there is a better approach for patch#2 to
avoid looping. This patch has been tested on Renesas RZ/Five SMARC EVK.
Below is the clock structure reported by Linux with this patch series:

/ # cat /sys/devices/soc0/family 
RZ/Five
/ # cat /sys/devices/soc0/machine 
Renesas SMARC EVK based on r9a07g043
/ # cat /sys/devices/soc0/revision 
0
/ # cat /sys/devices/soc0/soc_id 
r9a07g043
/ # cat /sys/kernel/debug/clk/clk_summary 
                                 enable  prepare  protect                                duty  hardware
   clock                          count    count    count        rate   accuracy phase  cycle    enable
-------------------------------------------------------------------------------------------------------
 extal                                3        3        0    24000000          0     0  50000         Y
    .pll6                             0        0        0   500000000          0     0  50000         Y
       .pll6_250                      0        0        0   250000000          0     0  50000         Y
          HP                          0        0        0   250000000          0     0  50000         Y
    .pll3                             1        1        0  1600000000          0     0  50000         Y
       .pll3_533                      0        0        0   533333333          0     0  50000         Y
          .sel_pll3_3                 0        0        0   533333333          0     0  50000         Y
             divpl3c                  0        0        0   266666667          0     0  50000         Y
                SPI1                  0        0        0    66666666          0     0  50000         Y
                   spi_clk2           0        0        0    66666666          0     0  50000         N
                SPI0                  0        0        0   133333333          0     0  50000         Y
                   spi_clk            0        0        0   133333333          0     0  50000         N
       .pll3_400                      0        0        0   400000000          0     0  50000         Y
       .pll3_div2                     1        1        0   800000000          0     0  50000         Y
          .pll3_div2_4                1        1        0   200000000          0     0  50000         Y
             M0                       0        0        0   200000000          0     0  50000         Y
                eth1_axi              0        0        0   200000000          0     0  50000         N
                eth0_axi              0        0        0   200000000          0     0  50000         N
             P1                       2        2        0   200000000          0     0  50000         Y
                iax45_clk             1        1        0   200000000          0     0  50000         Y
                usb_pclk              0        0        0   200000000          0     0  50000         N
                usb0_func             0        0        0   200000000          0     0  50000         N
                usb1_host             0        0        0   200000000          0     0  50000         N
                usb0_host             0        0        0   200000000          0     0  50000         N
                sdhi1_aclk            0        0        0   200000000          0     0  50000         N
                sdhi0_aclk            0        0        0   200000000          0     0  50000         N
                dmac_aclk             1        1        0   200000000          0     0  50000         Y
                P1_DIV2               0        0        0   100000000          0     0  50000         Y
                   dmac_pclk          0        0        0   100000000          0     0  50000         N
             .pll3_div2_4_2           0        0        0   100000000          0     0  50000         Y
                ZT                    0        0        0   100000000          0     0  50000         Y
                   eth1_chi           0        0        0   100000000          0     0  50000         N
                   eth0_chi           0        0        0   100000000          0     0  50000         N
                P2                    0        0        0   100000000          0     0  50000         Y
                   iax45_pclk         0        0        0   100000000          0     0  50000         N
    .pll2                             1        1        0  1600000000          0     0  50000         Y
       .clk_533                       0        0        0   533333333          0     0  50000         Y
          sd1                         0        0        0   533333333          0     0  50000         Y
             sdhi1_clk_hs             0        0        0   533333333          0     0  50000         N
             SD1_DIV4                 0        0        0   133333333          0     0  50000         Y
                sdhi1_imclk2          0        0        0   133333333          0     0  50000         N
                sdhi1_imclk           0        0        0   133333333          0     0  50000         N
          sd0                         0        0        0   533333333          0     0  50000         Y
             sdhi0_clk_hs             0        0        0   533333333          0     0  50000         N
             SD0_DIV4                 0        0        0   133333333          0     0  50000         Y
                sdhi0_imclk2          0        0        0   133333333          0     0  50000         N
                sdhi0_imclk           0        0        0   133333333          0     0  50000         N
          .clk_266                    0        0        0   266666666          0     0  50000         Y
       .clk_800                       0        0        0   800000000          0     0  50000         Y
          .clk_400                    0        0        0   400000000          0     0  50000         Y
       .pll2_div2                     1        1        0   800000000          0     0  50000         Y
          .pll2_div2_10               0        0        0    80000000          0     0  50000         Y
             TSU                      0        0        0    80000000          0     0  50000         Y
                tsu_pclk              0        0        0    80000000          0     0  50000         N
                adc_adclk             0        0        0    80000000          0     0  50000         N
          .pll2_div2_8                1        1        0   100000000          0     0  50000         Y
             P0                       1        1        0   100000000          0     0  50000         Y
                adc_pclk              0        0        0   100000000          0     0  50000         N
                canfd                 0        0        0   100000000          0     0  50000         N
                rspi2                 0        0        0   100000000          0     0  50000         N
                rspi1                 0        0        0   100000000          0     0  50000         N
                rspi0                 0        0        0   100000000          0     0  50000         N
                sci1                  0        0        0   100000000          0     0  50000         N
                sci0                  0        0        0   100000000          0     0  50000         N
                scif4                 0        0        0   100000000          0     0  50000         N
                scif3                 0        0        0   100000000          0     0  50000         N
                scif2                 0        0        0   100000000          0     0  50000         N
                scif1                 0        0        0   100000000          0     0  50000         N
                scif0                 2        2        0   100000000          0     0  50000         Y
                i2c3                  0        0        0   100000000          0     0  50000         N
                i2c2                  0        0        0   100000000          0     0  50000         N
                i2c1                  0        0        0   100000000          0     0  50000         N
                i2c0                  0        0        0   100000000          0     0  50000         N
                ssi3_sfr              0        0        0   100000000          0     0  50000         N
                ssi3_pclk             0        0        0   100000000          0     0  50000         N
                ssi2_sfr              0        0        0   100000000          0     0  50000         N
                ssi2_pclk             0        0        0   100000000          0     0  50000         N
                ssi1_sfr              0        0        0   100000000          0     0  50000         N
                ssi1_pclk             0        0        0   100000000          0     0  50000         N
                ssi0_sfr              0        0        0   100000000          0     0  50000         N
                ssi0_pclk             0        0        0   100000000          0     0  50000         N
                wdt2_pclk             0        0        0   100000000          0     0  50000         N
                wdt0_pclk             0        0        0   100000000          0     0  50000         N
                ostm2                 0        0        0   100000000          0     0  50000         N
                ostm1                 0        0        0   100000000          0     0  50000         N
                ostm0                 0        0        0   100000000          0     0  50000         N
                P0_DIV2               0        0        0    50000000          0     0  50000         Y
    .pll1                             0        0        0  1000000000          0     0  50000         Y
       I                              0        0        0  1000000000          0     0  50000         Y
    .osc_div1000                      0        0        0       24000          0     0  50000         Y
    .osc                              1        1        0    24000000          0     0  50000         Y
       gpio                           1        2        0    24000000          0     0  50000         Y
       wdt2_clk                       0        0        0    24000000          0     0  50000         N
       wdt0_clk                       0        0        0    24000000          0     0  50000         N
/ # 
/ # 

This patch series depends on branch [0].

[0] https://git.kernel.org/pub/scm/linux/kernel/git/geert/
renesas-drivers.git/log/?h=renesas-clk-for-v5.19

Cheers,
Prabhakar

Lad Prabhakar (4):
  dt-bindings: clock: r9a07g043-cpg: Add Renesas RZ/Five CPG Clock and
    Reset Definitions
  clk: renesas: rzg2l-cpg: Add support to stack the resets instead of
    indexing
  clk: renesas: r9a07g043: Split up core, module and resets array
  clk: renesas: r9a07g043: Add support for RZ/Five SoC

 drivers/clk/renesas/r9a07g043-cpg.c       | 514 ++++++++++++----------
 drivers/clk/renesas/rzg2l-cpg.c           |  76 +++-
 drivers/clk/renesas/rzg2l-cpg.h           |   4 +-
 include/dt-bindings/clock/r9a07g043-cpg.h |  20 +
 4 files changed, 376 insertions(+), 238 deletions(-)

-- 
2.25.1

[RFC PATCH 1/4] dt-bindings: clock: r9a07g043-cpg: Add Renesas RZ/Five CPG Clock and Reset Definitions

[Lad Prabhakar]

Renesas RZ/Five SoC has almost the same clock structure compared to the
Renesas RZ/G2UL SoC, re-use the r9a07g043-cpg.h header file and just
ammend the RZ/Five CPG clock and reset definitions.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 include/dt-bindings/clock/r9a07g043-cpg.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/dt-bindings/clock/r9a07g043-cpg.h b/include/dt-bindings/clock/r9a07g043-cpg.h
index 27e232733096..77cde8effdc7 100644
--- a/include/dt-bindings/clock/r9a07g043-cpg.h
+++ b/include/dt-bindings/clock/r9a07g043-cpg.h
@@ -108,6 +108,15 @@
 #define R9A07G043_ADC_ADCLK		76
 #define R9A07G043_ADC_PCLK		77
 #define R9A07G043_TSU_PCLK		78
+#define R9A07G043_NCEPLDM_DM_CLK	79	/* RZ/Five Only */
+#define R9A07G043_NCEPLDM_ACLK		80	/* RZ/Five Only */
+#define R9A07G043_NCEPLDM_TCK		81	/* RZ/Five Only */
+#define R9A07G043_NCEPLMT_ACLK		82	/* RZ/Five Only */
+#define R9A07G043_NCEPLIC_ACLK		83	/* RZ/Five Only */
+#define R9A07G043_AX45MP_CORE0_CLK	84	/* RZ/Five Only */
+#define R9A07G043_AX45MP_ACLK		85	/* RZ/Five Only */
+#define R9A07G043_IAX45_CLK		86	/* RZ/Five Only */
+#define R9A07G043_IAX45_PCLK		87	/* RZ/Five Only */
 
 /* R9A07G043 Resets */
 #define R9A07G043_CA55_RST_1_0		0	/* RZ/G2UL Only */
@@ -180,5 +189,16 @@
 #define R9A07G043_ADC_PRESETN		67
 #define R9A07G043_ADC_ADRST_N		68
 #define R9A07G043_TSU_PRESETN		69
+#define R9A07G043_NCEPLDM_DTM_PWR_RST_N	70	/* RZ/Five Only */
+#define R9A07G043_NCEPLDM_ARESETN	71	/* RZ/Five Only */
+#define R9A07G043_NCEPLMT_POR_RSTN	72	/* RZ/Five Only */
+#define R9A07G043_NCEPLMT_ARESETN	73	/* RZ/Five Only */
+#define R9A07G043_NCEPLIC_ARESETN	74	/* RZ/Five Only */
+#define R9A07G043_AX45MP_ARESETNM	75	/* RZ/Five Only */
+#define R9A07G043_AX45MP_ARESETNS	76	/* RZ/Five Only */
+#define R9A07G043_AX45MP_L2_RESETN	77	/* RZ/Five Only */
+#define R9A07G043_AX45MP_CORE0_RESETN	78	/* RZ/Five Only */
+#define R9A07G043_IAX45_RESETN		79	/* RZ/Five Only */
+
 
 #endif /* __DT_BINDINGS_CLOCK_R9A07G043_CPG_H__ */
-- 
2.25.1

Re: [RFC PATCH 1/4] dt-bindings: clock: r9a07g043-cpg: Add Renesas RZ/Five CPG Clock and Reset Definitions

[Geert Uytterhoeven]

Hi Prabhakar,

Thanks for your patch!

On Thu, May 5, 2022 at 9:32 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Renesas RZ/Five SoC has almost the same clock structure compared to the
> Renesas RZ/G2UL SoC, re-use the r9a07g043-cpg.h header file and just
> ammend the RZ/Five CPG clock and reset definitions.

amend

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> --- a/include/dt-bindings/clock/r9a07g043-cpg.h
> +++ b/include/dt-bindings/clock/r9a07g043-cpg.h
> @@ -108,6 +108,15 @@
>  #define R9A07G043_ADC_ADCLK            76
>  #define R9A07G043_ADC_PCLK             77
>  #define R9A07G043_TSU_PCLK             78
> +#define R9A07G043_NCEPLDM_DM_CLK       79      /* RZ/Five Only */

While NCEPLDM_DM_CLK is listed in the clock list spreadsheet, its
control bit is not documented.

> +#define R9A07G043_NCEPLDM_ACLK         80      /* RZ/Five Only */
> +#define R9A07G043_NCEPLDM_TCK          81      /* RZ/Five Only */

While NCEPLDM_TCK is listed in the clock list spreadsheet, its
control bit is not documented.

The rest LGTM, so with the above clarified
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [RFC PATCH 1/4] dt-bindings: clock: r9a07g043-cpg: Add Renesas RZ/Five CPG Clock and Reset Definitions

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Tue, May 10, 2022 at 3:02 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Thu, May 5, 2022 at 9:32 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Renesas RZ/Five SoC has almost the same clock structure compared to the
> > Renesas RZ/G2UL SoC, re-use the r9a07g043-cpg.h header file and just
> > ammend the RZ/Five CPG clock and reset definitions.
>
> amend
>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> > --- a/include/dt-bindings/clock/r9a07g043-cpg.h
> > +++ b/include/dt-bindings/clock/r9a07g043-cpg.h
> > @@ -108,6 +108,15 @@
> >  #define R9A07G043_ADC_ADCLK            76
> >  #define R9A07G043_ADC_PCLK             77
> >  #define R9A07G043_TSU_PCLK             78
> > +#define R9A07G043_NCEPLDM_DM_CLK       79      /* RZ/Five Only */
>
> While NCEPLDM_DM_CLK is listed in the clock list spreadsheet, its
> control bit is not documented.
>
> > +#define R9A07G043_NCEPLDM_ACLK         80      /* RZ/Five Only */
> > +#define R9A07G043_NCEPLDM_TCK          81      /* RZ/Five Only */
>
> While NCEPLDM_TCK is listed in the clock list spreadsheet, its
> control bit is not documented.
>
I have got the feedback for the above, NCEPLDM_DM_CLK and NCEPLDM_TCK
clocks cannot be stopped as a result there are no register bits for it
in the HW manual (clock spreadsheet will be updated). I will drop this
and send a v2 including your RB.

Cheers,
Prabhakar

> The rest LGTM, so with the above clarified
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> 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

Re: [RFC PATCH 1/4] dt-bindings: clock: r9a07g043-cpg: Add Renesas RZ/Five CPG Clock and Reset Definitions

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, May 19, 2022 at 7:45 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Tue, May 10, 2022 at 3:02 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, May 5, 2022 at 9:32 PM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > Renesas RZ/Five SoC has almost the same clock structure compared to the
> > > Renesas RZ/G2UL SoC, re-use the r9a07g043-cpg.h header file and just
> > > ammend the RZ/Five CPG clock and reset definitions.
> >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > > --- a/include/dt-bindings/clock/r9a07g043-cpg.h
> > > +++ b/include/dt-bindings/clock/r9a07g043-cpg.h
> > > @@ -108,6 +108,15 @@
> > >  #define R9A07G043_ADC_ADCLK            76
> > >  #define R9A07G043_ADC_PCLK             77
> > >  #define R9A07G043_TSU_PCLK             78
> > > +#define R9A07G043_NCEPLDM_DM_CLK       79      /* RZ/Five Only */
> >
> > While NCEPLDM_DM_CLK is listed in the clock list spreadsheet, its
> > control bit is not documented.
> >
> > > +#define R9A07G043_NCEPLDM_ACLK         80      /* RZ/Five Only */
> > > +#define R9A07G043_NCEPLDM_TCK          81      /* RZ/Five Only */
> >
> > While NCEPLDM_TCK is listed in the clock list spreadsheet, its
> > control bit is not documented.
> >
> I have got the feedback for the above, NCEPLDM_DM_CLK and NCEPLDM_TCK
> clocks cannot be stopped as a result there are no register bits for it
> in the HW manual (clock spreadsheet will be updated). I will drop this
> and send a v2 including your RB.

The question is not if the clocks can be stopped or not, but if there
is any need to refer to them from a DT node.
What's the nature of the future update to the clock spreadsheet?

Of course, if we don't add these clock definitions now, they can
still be added later. DT binding definitions are append-only.

Thanks!

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

Re: [RFC PATCH 1/4] dt-bindings: clock: r9a07g043-cpg: Add Renesas RZ/Five CPG Clock and Reset Definitions

[Lad, Prabhakar]

Hi Geert,

Sorry for the late reply.

On Thu, May 19, 2022 at 7:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, May 19, 2022 at 7:45 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Tue, May 10, 2022 at 3:02 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Thu, May 5, 2022 at 9:32 PM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > Renesas RZ/Five SoC has almost the same clock structure compared to the
> > > > Renesas RZ/G2UL SoC, re-use the r9a07g043-cpg.h header file and just
> > > > ammend the RZ/Five CPG clock and reset definitions.
> > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > > --- a/include/dt-bindings/clock/r9a07g043-cpg.h
> > > > +++ b/include/dt-bindings/clock/r9a07g043-cpg.h
> > > > @@ -108,6 +108,15 @@
> > > >  #define R9A07G043_ADC_ADCLK            76
> > > >  #define R9A07G043_ADC_PCLK             77
> > > >  #define R9A07G043_TSU_PCLK             78
> > > > +#define R9A07G043_NCEPLDM_DM_CLK       79      /* RZ/Five Only */
> > >
> > > While NCEPLDM_DM_CLK is listed in the clock list spreadsheet, its
> > > control bit is not documented.
> > >
> > > > +#define R9A07G043_NCEPLDM_ACLK         80      /* RZ/Five Only */
> > > > +#define R9A07G043_NCEPLDM_TCK          81      /* RZ/Five Only */
> > >
> > > While NCEPLDM_TCK is listed in the clock list spreadsheet, its
> > > control bit is not documented.
> > >
> > I have got the feedback for the above, NCEPLDM_DM_CLK and NCEPLDM_TCK
> > clocks cannot be stopped as a result there are no register bits for it
> > in the HW manual (clock spreadsheet will be updated). I will drop this
> > and send a v2 including your RB.
>
> The question is not if the clocks can be stopped or not, but if there
> is any need to refer to them from a DT node.
As per DT rule we have to add ;)

> What's the nature of the future update to the clock spreadsheet?
>
I have got confirmation from HW team, the UM and clock list will not be updated,

* NCEPLDM_DM_CLK, NCEPLDM_TCK and NCEPLDM_ACLK actually exist, and
should be listed on the clock list. Only NCEPLDM_ACLK has a mechanism
to stop.
* Therefore, only NCEPLDM_ACLK should appear on the Clock Control
register on the UM.

> Of course, if we don't add these clock definitions now, they can
> still be added later. DT binding definitions are append-only.
>
For now I will keep them.

Cheers,
Prabhakar

[RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Lad Prabhakar]

Instead of indexing the resets, stack them and instead create an id member
in struct rzg2l_reset to store the index. With this approach for every id
we will have to loop through the resets array to match the id.

This in preparation to add support for Renesas RZ/Five CPG in
r9a07g043-cpg.c file where the resets array will be split up into three
i.e. common and two SoC specific.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/rzg2l-cpg.c | 76 ++++++++++++++++++++++++++-------
 drivers/clk/renesas/rzg2l-cpg.h |  4 +-
 2 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 1ce35f65682b..94fe307ec4c5 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -681,14 +681,37 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk *mod,
 
 #define rcdev_to_priv(x)	container_of(x, struct rzg2l_cpg_priv, rcdev)
 
+static const struct rzg2l_reset
+*rzg2l_get_reset_ptr(struct rzg2l_cpg_priv *priv,
+		     unsigned long id)
+
+{
+	const struct rzg2l_cpg_info *info = priv->info;
+	unsigned int i;
+
+	for (i = 0; i < priv->num_resets; i++) {
+		if (info->resets[i].id == id)
+			return &info->resets[i];
+	}
+
+	return NULL;
+}
+
 static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
 			   unsigned long id)
 {
 	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
-	const struct rzg2l_cpg_info *info = priv->info;
-	unsigned int reg = info->resets[id].off;
-	u32 dis = BIT(info->resets[id].bit);
-	u32 we = dis << 16;
+	const struct rzg2l_reset *reset;
+	unsigned int reg;
+	u32 dis, we;
+
+	reset = rzg2l_get_reset_ptr(priv, id);
+	if (!reset)
+		return -EINVAL;
+
+	reg = reset->off;
+	dis = BIT(reset->bit);
+	we = dis << 16;
 
 	dev_dbg(rcdev->dev, "reset id:%ld offset:0x%x\n", id, CLK_RST_R(reg));
 
@@ -708,9 +731,16 @@ static int rzg2l_cpg_assert(struct reset_controller_dev *rcdev,
 			    unsigned long id)
 {
 	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
-	const struct rzg2l_cpg_info *info = priv->info;
-	unsigned int reg = info->resets[id].off;
-	u32 value = BIT(info->resets[id].bit) << 16;
+	const struct rzg2l_reset *reset;
+	unsigned int reg;
+	u32 value;
+
+	reset = rzg2l_get_reset_ptr(priv, id);
+	if (!reset)
+		return -EINVAL;
+
+	reg = reset->off;
+	value = BIT(reset->bit) << 16;
 
 	dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id, CLK_RST_R(reg));
 
@@ -722,11 +752,17 @@ static int rzg2l_cpg_deassert(struct reset_controller_dev *rcdev,
 			      unsigned long id)
 {
 	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
-	const struct rzg2l_cpg_info *info = priv->info;
-	unsigned int reg = info->resets[id].off;
-	u32 dis = BIT(info->resets[id].bit);
-	u32 value = (dis << 16) | dis;
+	const struct rzg2l_reset *reset;
+	unsigned int reg;
+	u32 dis, value;
+
+	reset = rzg2l_get_reset_ptr(priv, id);
+	if (!reset)
+		return -EINVAL;
 
+	reg = reset->off;
+	dis = BIT(reset->bit);
+	value = (dis << 16) | dis;
 	dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
 		CLK_RST_R(reg));
 
@@ -738,9 +774,16 @@ static int rzg2l_cpg_status(struct reset_controller_dev *rcdev,
 			    unsigned long id)
 {
 	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
-	const struct rzg2l_cpg_info *info = priv->info;
-	unsigned int reg = info->resets[id].off;
-	u32 bitmask = BIT(info->resets[id].bit);
+	const struct rzg2l_reset *reset;
+	unsigned int reg;
+	u32 bitmask;
+
+	reset = rzg2l_get_reset_ptr(priv, id);
+	if (!reset)
+		return -EINVAL;
+
+	reg = reset->off;
+	bitmask = BIT(reset->bit);
 
 	return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);
 }
@@ -756,10 +799,11 @@ static int rzg2l_cpg_reset_xlate(struct reset_controller_dev *rcdev,
 				 const struct of_phandle_args *reset_spec)
 {
 	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
-	const struct rzg2l_cpg_info *info = priv->info;
 	unsigned int id = reset_spec->args[0];
+	const struct rzg2l_reset *reset;
 
-	if (id >= rcdev->nr_resets || !info->resets[id].off) {
+	reset = rzg2l_get_reset_ptr(priv, id);
+	if (!reset) {
 		dev_err(rcdev->dev, "Invalid reset index %u\n", id);
 		return -EINVAL;
 	}
diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
index 92c88f42ca7f..a99f2ba7868f 100644
--- a/drivers/clk/renesas/rzg2l-cpg.h
+++ b/drivers/clk/renesas/rzg2l-cpg.h
@@ -152,12 +152,14 @@ struct rzg2l_mod_clk {
  * @bit: reset bit
  */
 struct rzg2l_reset {
+	unsigned int id;
 	u16 off;
 	u8 bit;
 };
 
 #define DEF_RST(_id, _off, _bit)	\
-	[_id] = { \
+	{ \
+		.id = (_id), \
 		.off = (_off), \
 		.bit = (_bit) \
 	}
-- 
2.25.1

RE: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Biju Das]

Hi Lad Prabhakar,

Thanks for the patch.

> Subject: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the
> resets instead of indexing
> 
> Instead of indexing the resets, stack them and instead create an id member
> in struct rzg2l_reset to store the index. With this approach for every id
> we will have to loop through the resets array to match the id.
> 
> This in preparation to add support for Renesas RZ/Five CPG in r9a07g043-
> cpg.c file where the resets array will be split up into three i.e. common
> and two SoC specific.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/clk/renesas/rzg2l-cpg.c | 76 ++++++++++++++++++++++++++-------
> drivers/clk/renesas/rzg2l-cpg.h |  4 +-
>  2 files changed, 63 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-
> cpg.c index 1ce35f65682b..94fe307ec4c5 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -681,14 +681,37 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk
> *mod,
> 
>  #define rcdev_to_priv(x)	container_of(x, struct rzg2l_cpg_priv,
> rcdev)
> 
> +static const struct rzg2l_reset
> +*rzg2l_get_reset_ptr(struct rzg2l_cpg_priv *priv,
> +		     unsigned long id)
> +
> +{
> +	const struct rzg2l_cpg_info *info = priv->info;
> +	unsigned int i;
> +
> +	for (i = 0; i < priv->num_resets; i++) {
> +		if (info->resets[i].id == id)
> +			return &info->resets[i];
> +	}

Is it not possible to use shared reset like RZ/G2L and RZ/V2L?, which
has optimal memory and performance wise we can avoid bigger loop.

Like adding Last index of RZ/Five as last reset index and
Handle RZ/G2UL specific as invalid reset index in xlate??


> +
> +	return NULL;
> +}
> +
>  static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
>  			   unsigned long id)
>  {
>  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> -	const struct rzg2l_cpg_info *info = priv->info;
> -	unsigned int reg = info->resets[id].off;
> -	u32 dis = BIT(info->resets[id].bit);
> -	u32 we = dis << 16;
> +	const struct rzg2l_reset *reset;
> +	unsigned int reg;
> +	u32 dis, we;
> +
> +	reset = rzg2l_get_reset_ptr(priv, id);
> +	if (!reset)
> +		return -EINVAL;
> +
> +	reg = reset->off;
> +	dis = BIT(reset->bit);
> +	we = dis << 16;
> 
>  	dev_dbg(rcdev->dev, "reset id:%ld offset:0x%x\n", id,
> CLK_RST_R(reg));
> 
> @@ -708,9 +731,16 @@ static int rzg2l_cpg_assert(struct
> reset_controller_dev *rcdev,
>  			    unsigned long id)
>  {
>  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> -	const struct rzg2l_cpg_info *info = priv->info;
> -	unsigned int reg = info->resets[id].off;
> -	u32 value = BIT(info->resets[id].bit) << 16;
> +	const struct rzg2l_reset *reset;
> +	unsigned int reg;
> +	u32 value;
> +
> +	reset = rzg2l_get_reset_ptr(priv, id);
> +	if (!reset)
> +		return -EINVAL;
> +
> +	reg = reset->off;
> +	value = BIT(reset->bit) << 16;
> 
>  	dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id,
> CLK_RST_R(reg));
> 
> @@ -722,11 +752,17 @@ static int rzg2l_cpg_deassert(struct
> reset_controller_dev *rcdev,
>  			      unsigned long id)
>  {
>  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> -	const struct rzg2l_cpg_info *info = priv->info;
> -	unsigned int reg = info->resets[id].off;
> -	u32 dis = BIT(info->resets[id].bit);
> -	u32 value = (dis << 16) | dis;
> +	const struct rzg2l_reset *reset;
> +	unsigned int reg;
> +	u32 dis, value;
> +
> +	reset = rzg2l_get_reset_ptr(priv, id);
> +	if (!reset)
> +		return -EINVAL;
> 
> +	reg = reset->off;
> +	dis = BIT(reset->bit);
> +	value = (dis << 16) | dis;
>  	dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
>  		CLK_RST_R(reg));
> 
> @@ -738,9 +774,16 @@ static int rzg2l_cpg_status(struct
> reset_controller_dev *rcdev,
>  			    unsigned long id)
>  {
>  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> -	const struct rzg2l_cpg_info *info = priv->info;
> -	unsigned int reg = info->resets[id].off;
> -	u32 bitmask = BIT(info->resets[id].bit);
> +	const struct rzg2l_reset *reset;
> +	unsigned int reg;
> +	u32 bitmask;
> +
> +	reset = rzg2l_get_reset_ptr(priv, id);
> +	if (!reset)
> +		return -EINVAL;
> +
> +	reg = reset->off;
> +	bitmask = BIT(reset->bit);
> 
>  	return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);  } @@ -
> 756,10 +799,11 @@ static int rzg2l_cpg_reset_xlate(struct
> reset_controller_dev *rcdev,
>  				 const struct of_phandle_args *reset_spec)  {
>  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> -	const struct rzg2l_cpg_info *info = priv->info;
>  	unsigned int id = reset_spec->args[0];
> +	const struct rzg2l_reset *reset;
> 
> -	if (id >= rcdev->nr_resets || !info->resets[id].off) {
> +	reset = rzg2l_get_reset_ptr(priv, id);
> +	if (!reset) {
>  		dev_err(rcdev->dev, "Invalid reset index %u\n", id);
>  		return -EINVAL;
>  	}
> diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-
> cpg.h index 92c88f42ca7f..a99f2ba7868f 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.h
> +++ b/drivers/clk/renesas/rzg2l-cpg.h
> @@ -152,12 +152,14 @@ struct rzg2l_mod_clk {
>   * @bit: reset bit
>   */
>  struct rzg2l_reset {
> +	unsigned int id;

Now you are adding 4 bytes to each reset entry in the LUT.

Cheers,
Biju

>  	u16 off;
>  	u8 bit;
>  };

> 
>  #define DEF_RST(_id, _off, _bit)	\
> -	[_id] = { \
> +	{ \
> +		.id = (_id), \
>  		.off = (_off), \
>  		.bit = (_bit) \
>  	}
> --
> 2.25.1

RE: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Biju Das]

> Subject: RE: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack
> the resets instead of indexing
> 
> Hi Lad Prabhakar,
> 
> Thanks for the patch.
> 
> > Subject: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack
> > the resets instead of indexing
> >
> > Instead of indexing the resets, stack them and instead create an id
> > member in struct rzg2l_reset to store the index. With this approach
> > for every id we will have to loop through the resets array to match the
> id.
> >
> > This in preparation to add support for Renesas RZ/Five CPG in
> > r9a07g043- cpg.c file where the resets array will be split up into
> > three i.e. common and two SoC specific.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/clk/renesas/rzg2l-cpg.c | 76
> > ++++++++++++++++++++++++++------- drivers/clk/renesas/rzg2l-cpg.h |  4
> > +-
> >  2 files changed, 63 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/clk/renesas/rzg2l-cpg.c
> > b/drivers/clk/renesas/rzg2l- cpg.c index 1ce35f65682b..94fe307ec4c5
> > 100644
> > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > @@ -681,14 +681,37 @@ rzg2l_cpg_register_mod_clk(const struct
> > rzg2l_mod_clk *mod,
> >
> >  #define rcdev_to_priv(x)	container_of(x, struct rzg2l_cpg_priv,
> > rcdev)
> >
> > +static const struct rzg2l_reset
> > +*rzg2l_get_reset_ptr(struct rzg2l_cpg_priv *priv,
> > +		     unsigned long id)
> > +
> > +{
> > +	const struct rzg2l_cpg_info *info = priv->info;
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < priv->num_resets; i++) {
> > +		if (info->resets[i].id == id)
> > +			return &info->resets[i];
> > +	}
> 
> Is it not possible to use shared reset like RZ/G2L and RZ/V2L?, which has
> optimal memory and performance wise we can avoid bigger loop.
> 
> Like adding Last index of RZ/Five as last reset index and Handle RZ/G2UL
> specific as invalid reset index in xlate??

Or add a invalidate_resets() callback to info structure and in probe, call this
Callback,if present to invalidate the resets which are not specific to this SoC.


> 
> 
> > +
> > +	return NULL;
> > +}
> > +
> >  static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
> >  			   unsigned long id)
> >  {
> >  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -	const struct rzg2l_cpg_info *info = priv->info;
> > -	unsigned int reg = info->resets[id].off;
> > -	u32 dis = BIT(info->resets[id].bit);
> > -	u32 we = dis << 16;
> > +	const struct rzg2l_reset *reset;
> > +	unsigned int reg;
> > +	u32 dis, we;
> > +
> > +	reset = rzg2l_get_reset_ptr(priv, id);
> > +	if (!reset)
> > +		return -EINVAL;
> > +
> > +	reg = reset->off;
> > +	dis = BIT(reset->bit);
> > +	we = dis << 16;
> >
> >  	dev_dbg(rcdev->dev, "reset id:%ld offset:0x%x\n", id,
> > CLK_RST_R(reg));
> >
> > @@ -708,9 +731,16 @@ static int rzg2l_cpg_assert(struct
> > reset_controller_dev *rcdev,
> >  			    unsigned long id)
> >  {
> >  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -	const struct rzg2l_cpg_info *info = priv->info;
> > -	unsigned int reg = info->resets[id].off;
> > -	u32 value = BIT(info->resets[id].bit) << 16;
> > +	const struct rzg2l_reset *reset;
> > +	unsigned int reg;
> > +	u32 value;
> > +
> > +	reset = rzg2l_get_reset_ptr(priv, id);
> > +	if (!reset)
> > +		return -EINVAL;
> > +
> > +	reg = reset->off;
> > +	value = BIT(reset->bit) << 16;
> >
> >  	dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id,
> > CLK_RST_R(reg));
> >
> > @@ -722,11 +752,17 @@ static int rzg2l_cpg_deassert(struct
> > reset_controller_dev *rcdev,
> >  			      unsigned long id)
> >  {
> >  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -	const struct rzg2l_cpg_info *info = priv->info;
> > -	unsigned int reg = info->resets[id].off;
> > -	u32 dis = BIT(info->resets[id].bit);
> > -	u32 value = (dis << 16) | dis;
> > +	const struct rzg2l_reset *reset;
> > +	unsigned int reg;
> > +	u32 dis, value;
> > +
> > +	reset = rzg2l_get_reset_ptr(priv, id);
> > +	if (!reset)
> > +		return -EINVAL;
> >
> > +	reg = reset->off;
> > +	dis = BIT(reset->bit);
> > +	value = (dis << 16) | dis;
> >  	dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
> >  		CLK_RST_R(reg));
> >
> > @@ -738,9 +774,16 @@ static int rzg2l_cpg_status(struct
> > reset_controller_dev *rcdev,
> >  			    unsigned long id)
> >  {
> >  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -	const struct rzg2l_cpg_info *info = priv->info;
> > -	unsigned int reg = info->resets[id].off;
> > -	u32 bitmask = BIT(info->resets[id].bit);
> > +	const struct rzg2l_reset *reset;
> > +	unsigned int reg;
> > +	u32 bitmask;
> > +
> > +	reset = rzg2l_get_reset_ptr(priv, id);
> > +	if (!reset)
> > +		return -EINVAL;
> > +
> > +	reg = reset->off;
> > +	bitmask = BIT(reset->bit);
> >
> >  	return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);  } @@ -
> > 756,10 +799,11 @@ static int rzg2l_cpg_reset_xlate(struct
> > reset_controller_dev *rcdev,
> >  				 const struct of_phandle_args *reset_spec)  {
> >  	struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -	const struct rzg2l_cpg_info *info = priv->info;
> >  	unsigned int id = reset_spec->args[0];
> > +	const struct rzg2l_reset *reset;
> >
> > -	if (id >= rcdev->nr_resets || !info->resets[id].off) {
> > +	reset = rzg2l_get_reset_ptr(priv, id);
> > +	if (!reset) {
> >  		dev_err(rcdev->dev, "Invalid reset index %u\n", id);
> >  		return -EINVAL;
> >  	}
> > diff --git a/drivers/clk/renesas/rzg2l-cpg.h
> > b/drivers/clk/renesas/rzg2l- cpg.h index 92c88f42ca7f..a99f2ba7868f
> > 100644
> > --- a/drivers/clk/renesas/rzg2l-cpg.h
> > +++ b/drivers/clk/renesas/rzg2l-cpg.h
> > @@ -152,12 +152,14 @@ struct rzg2l_mod_clk {
> >   * @bit: reset bit
> >   */
> >  struct rzg2l_reset {
> > +	unsigned int id;
> 
> Now you are adding 4 bytes to each reset entry in the LUT.
> 
> Cheers,
> Biju
> 
> >  	u16 off;
> >  	u8 bit;
> >  };
> 
> >
> >  #define DEF_RST(_id, _off, _bit)	\
> > -	[_id] = { \
> > +	{ \
> > +		.id = (_id), \
> >  		.off = (_off), \
> >  		.bit = (_bit) \
> >  	}
> > --
> > 2.25.1

Re: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Lad, Prabhakar]

On Fri, May 6, 2022 at 7:17 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> > Subject: RE: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack
> > the resets instead of indexing
> >
> > Hi Lad Prabhakar,
> >
> > Thanks for the patch.
> >
> > > Subject: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack
> > > the resets instead of indexing
> > >
> > > Instead of indexing the resets, stack them and instead create an id
> > > member in struct rzg2l_reset to store the index. With this approach
> > > for every id we will have to loop through the resets array to match the
> > id.
> > >
> > > This in preparation to add support for Renesas RZ/Five CPG in
> > > r9a07g043- cpg.c file where the resets array will be split up into
> > > three i.e. common and two SoC specific.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  drivers/clk/renesas/rzg2l-cpg.c | 76
> > > ++++++++++++++++++++++++++------- drivers/clk/renesas/rzg2l-cpg.h |  4
> > > +-
> > >  2 files changed, 63 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/clk/renesas/rzg2l-cpg.c
> > > b/drivers/clk/renesas/rzg2l- cpg.c index 1ce35f65682b..94fe307ec4c5
> > > 100644
> > > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > > @@ -681,14 +681,37 @@ rzg2l_cpg_register_mod_clk(const struct
> > > rzg2l_mod_clk *mod,
> > >
> > >  #define rcdev_to_priv(x)   container_of(x, struct rzg2l_cpg_priv,
> > > rcdev)
> > >
> > > +static const struct rzg2l_reset
> > > +*rzg2l_get_reset_ptr(struct rzg2l_cpg_priv *priv,
> > > +                unsigned long id)
> > > +
> > > +{
> > > +   const struct rzg2l_cpg_info *info = priv->info;
> > > +   unsigned int i;
> > > +
> > > +   for (i = 0; i < priv->num_resets; i++) {
> > > +           if (info->resets[i].id == id)
> > > +                   return &info->resets[i];
> > > +   }
> >
> > Is it not possible to use shared reset like RZ/G2L and RZ/V2L?, which has
> > optimal memory and performance wise we can avoid bigger loop.
> >
> > Like adding Last index of RZ/Five as last reset index and Handle RZ/G2UL
> > specific as invalid reset index in xlate??
>
> Or add a invalidate_resets() callback to info structure and in probe, call this
> Callback,if present to invalidate the resets which are not specific to this SoC.
>
Could you please elaborate on this.

Cheers,
Prabhakar

>
> >
> >
> > > +
> > > +   return NULL;
> > > +}
> > > +
> > >  static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
> > >                        unsigned long id)
> > >  {
> > >     struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -   const struct rzg2l_cpg_info *info = priv->info;
> > > -   unsigned int reg = info->resets[id].off;
> > > -   u32 dis = BIT(info->resets[id].bit);
> > > -   u32 we = dis << 16;
> > > +   const struct rzg2l_reset *reset;
> > > +   unsigned int reg;
> > > +   u32 dis, we;
> > > +
> > > +   reset = rzg2l_get_reset_ptr(priv, id);
> > > +   if (!reset)
> > > +           return -EINVAL;
> > > +
> > > +   reg = reset->off;
> > > +   dis = BIT(reset->bit);
> > > +   we = dis << 16;
> > >
> > >     dev_dbg(rcdev->dev, "reset id:%ld offset:0x%x\n", id,
> > > CLK_RST_R(reg));
> > >
> > > @@ -708,9 +731,16 @@ static int rzg2l_cpg_assert(struct
> > > reset_controller_dev *rcdev,
> > >                         unsigned long id)
> > >  {
> > >     struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -   const struct rzg2l_cpg_info *info = priv->info;
> > > -   unsigned int reg = info->resets[id].off;
> > > -   u32 value = BIT(info->resets[id].bit) << 16;
> > > +   const struct rzg2l_reset *reset;
> > > +   unsigned int reg;
> > > +   u32 value;
> > > +
> > > +   reset = rzg2l_get_reset_ptr(priv, id);
> > > +   if (!reset)
> > > +           return -EINVAL;
> > > +
> > > +   reg = reset->off;
> > > +   value = BIT(reset->bit) << 16;
> > >
> > >     dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id,
> > > CLK_RST_R(reg));
> > >
> > > @@ -722,11 +752,17 @@ static int rzg2l_cpg_deassert(struct
> > > reset_controller_dev *rcdev,
> > >                           unsigned long id)
> > >  {
> > >     struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -   const struct rzg2l_cpg_info *info = priv->info;
> > > -   unsigned int reg = info->resets[id].off;
> > > -   u32 dis = BIT(info->resets[id].bit);
> > > -   u32 value = (dis << 16) | dis;
> > > +   const struct rzg2l_reset *reset;
> > > +   unsigned int reg;
> > > +   u32 dis, value;
> > > +
> > > +   reset = rzg2l_get_reset_ptr(priv, id);
> > > +   if (!reset)
> > > +           return -EINVAL;
> > >
> > > +   reg = reset->off;
> > > +   dis = BIT(reset->bit);
> > > +   value = (dis << 16) | dis;
> > >     dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
> > >             CLK_RST_R(reg));
> > >
> > > @@ -738,9 +774,16 @@ static int rzg2l_cpg_status(struct
> > > reset_controller_dev *rcdev,
> > >                         unsigned long id)
> > >  {
> > >     struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -   const struct rzg2l_cpg_info *info = priv->info;
> > > -   unsigned int reg = info->resets[id].off;
> > > -   u32 bitmask = BIT(info->resets[id].bit);
> > > +   const struct rzg2l_reset *reset;
> > > +   unsigned int reg;
> > > +   u32 bitmask;
> > > +
> > > +   reset = rzg2l_get_reset_ptr(priv, id);
> > > +   if (!reset)
> > > +           return -EINVAL;
> > > +
> > > +   reg = reset->off;
> > > +   bitmask = BIT(reset->bit);
> > >
> > >     return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);  } @@ -
> > > 756,10 +799,11 @@ static int rzg2l_cpg_reset_xlate(struct
> > > reset_controller_dev *rcdev,
> > >                              const struct of_phandle_args *reset_spec)  {
> > >     struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -   const struct rzg2l_cpg_info *info = priv->info;
> > >     unsigned int id = reset_spec->args[0];
> > > +   const struct rzg2l_reset *reset;
> > >
> > > -   if (id >= rcdev->nr_resets || !info->resets[id].off) {
> > > +   reset = rzg2l_get_reset_ptr(priv, id);
> > > +   if (!reset) {
> > >             dev_err(rcdev->dev, "Invalid reset index %u\n", id);
> > >             return -EINVAL;
> > >     }
> > > diff --git a/drivers/clk/renesas/rzg2l-cpg.h
> > > b/drivers/clk/renesas/rzg2l- cpg.h index 92c88f42ca7f..a99f2ba7868f
> > > 100644
> > > --- a/drivers/clk/renesas/rzg2l-cpg.h
> > > +++ b/drivers/clk/renesas/rzg2l-cpg.h
> > > @@ -152,12 +152,14 @@ struct rzg2l_mod_clk {
> > >   * @bit: reset bit
> > >   */
> > >  struct rzg2l_reset {
> > > +   unsigned int id;
> >
> > Now you are adding 4 bytes to each reset entry in the LUT.
> >
> > Cheers,
> > Biju
> >
> > >     u16 off;
> > >     u8 bit;
> > >  };
> >
> > >
> > >  #define DEF_RST(_id, _off, _bit)   \
> > > -   [_id] = { \
> > > +   { \
> > > +           .id = (_id), \
> > >             .off = (_off), \
> > >             .bit = (_bit) \
> > >     }
> > > --
> > > 2.25.1
>

Re: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Lad, Prabhakar]

Hi Biju,

Thank you for the review.

On Thu, May 5, 2022 at 8:48 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Lad Prabhakar,
>
> Thanks for the patch.
>
> > Subject: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the
> > resets instead of indexing
> >
> > Instead of indexing the resets, stack them and instead create an id member
> > in struct rzg2l_reset to store the index. With this approach for every id
> > we will have to loop through the resets array to match the id.
> >
> > This in preparation to add support for Renesas RZ/Five CPG in r9a07g043-
> > cpg.c file where the resets array will be split up into three i.e. common
> > and two SoC specific.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/clk/renesas/rzg2l-cpg.c | 76 ++++++++++++++++++++++++++-------
> > drivers/clk/renesas/rzg2l-cpg.h |  4 +-
> >  2 files changed, 63 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-
> > cpg.c index 1ce35f65682b..94fe307ec4c5 100644
> > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > @@ -681,14 +681,37 @@ rzg2l_cpg_register_mod_clk(const struct rzg2l_mod_clk
> > *mod,
> >
> >  #define rcdev_to_priv(x)     container_of(x, struct rzg2l_cpg_priv,
> > rcdev)
> >
> > +static const struct rzg2l_reset
> > +*rzg2l_get_reset_ptr(struct rzg2l_cpg_priv *priv,
> > +                  unsigned long id)
> > +
> > +{
> > +     const struct rzg2l_cpg_info *info = priv->info;
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < priv->num_resets; i++) {
> > +             if (info->resets[i].id == id)
> > +                     return &info->resets[i];
> > +     }
>
> Is it not possible to use shared reset like RZ/G2L and RZ/V2L?, which
> has optimal memory and performance wise we can avoid bigger loop.
>
> Like adding Last index of RZ/Five as last reset index and
> Handle RZ/G2UL specific as invalid reset index in xlate??
>
So we will have to maintain an array id's which are invalid to RZ/Five
SoC. For this too we will have to loop at runtime itself. The array
for invalid index will be big too.

>
> > +
> > +     return NULL;
> > +}
> > +
> >  static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
> >                          unsigned long id)
> >  {
> >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -     const struct rzg2l_cpg_info *info = priv->info;
> > -     unsigned int reg = info->resets[id].off;
> > -     u32 dis = BIT(info->resets[id].bit);
> > -     u32 we = dis << 16;
> > +     const struct rzg2l_reset *reset;
> > +     unsigned int reg;
> > +     u32 dis, we;
> > +
> > +     reset = rzg2l_get_reset_ptr(priv, id);
> > +     if (!reset)
> > +             return -EINVAL;
> > +
> > +     reg = reset->off;
> > +     dis = BIT(reset->bit);
> > +     we = dis << 16;
> >
> >       dev_dbg(rcdev->dev, "reset id:%ld offset:0x%x\n", id,
> > CLK_RST_R(reg));
> >
> > @@ -708,9 +731,16 @@ static int rzg2l_cpg_assert(struct
> > reset_controller_dev *rcdev,
> >                           unsigned long id)
> >  {
> >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -     const struct rzg2l_cpg_info *info = priv->info;
> > -     unsigned int reg = info->resets[id].off;
> > -     u32 value = BIT(info->resets[id].bit) << 16;
> > +     const struct rzg2l_reset *reset;
> > +     unsigned int reg;
> > +     u32 value;
> > +
> > +     reset = rzg2l_get_reset_ptr(priv, id);
> > +     if (!reset)
> > +             return -EINVAL;
> > +
> > +     reg = reset->off;
> > +     value = BIT(reset->bit) << 16;
> >
> >       dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id,
> > CLK_RST_R(reg));
> >
> > @@ -722,11 +752,17 @@ static int rzg2l_cpg_deassert(struct
> > reset_controller_dev *rcdev,
> >                             unsigned long id)
> >  {
> >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -     const struct rzg2l_cpg_info *info = priv->info;
> > -     unsigned int reg = info->resets[id].off;
> > -     u32 dis = BIT(info->resets[id].bit);
> > -     u32 value = (dis << 16) | dis;
> > +     const struct rzg2l_reset *reset;
> > +     unsigned int reg;
> > +     u32 dis, value;
> > +
> > +     reset = rzg2l_get_reset_ptr(priv, id);
> > +     if (!reset)
> > +             return -EINVAL;
> >
> > +     reg = reset->off;
> > +     dis = BIT(reset->bit);
> > +     value = (dis << 16) | dis;
> >       dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
> >               CLK_RST_R(reg));
> >
> > @@ -738,9 +774,16 @@ static int rzg2l_cpg_status(struct
> > reset_controller_dev *rcdev,
> >                           unsigned long id)
> >  {
> >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -     const struct rzg2l_cpg_info *info = priv->info;
> > -     unsigned int reg = info->resets[id].off;
> > -     u32 bitmask = BIT(info->resets[id].bit);
> > +     const struct rzg2l_reset *reset;
> > +     unsigned int reg;
> > +     u32 bitmask;
> > +
> > +     reset = rzg2l_get_reset_ptr(priv, id);
> > +     if (!reset)
> > +             return -EINVAL;
> > +
> > +     reg = reset->off;
> > +     bitmask = BIT(reset->bit);
> >
> >       return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);  } @@ -
> > 756,10 +799,11 @@ static int rzg2l_cpg_reset_xlate(struct
> > reset_controller_dev *rcdev,
> >                                const struct of_phandle_args *reset_spec)  {
> >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > -     const struct rzg2l_cpg_info *info = priv->info;
> >       unsigned int id = reset_spec->args[0];
> > +     const struct rzg2l_reset *reset;
> >
> > -     if (id >= rcdev->nr_resets || !info->resets[id].off) {
> > +     reset = rzg2l_get_reset_ptr(priv, id);
> > +     if (!reset) {
> >               dev_err(rcdev->dev, "Invalid reset index %u\n", id);
> >               return -EINVAL;
> >       }
> > diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-
> > cpg.h index 92c88f42ca7f..a99f2ba7868f 100644
> > --- a/drivers/clk/renesas/rzg2l-cpg.h
> > +++ b/drivers/clk/renesas/rzg2l-cpg.h
> > @@ -152,12 +152,14 @@ struct rzg2l_mod_clk {
> >   * @bit: reset bit
> >   */
> >  struct rzg2l_reset {
> > +     unsigned int id;
>
> Now you are adding 4 bytes to each reset entry in the LUT.
>
Agreed, on the other hand we are saving space on the entries (for
example not all the reset entries are listed in the array and the
array length will always be equal to last index)

Cheers,
Prabhakar

> Cheers,
> Biju
>
> >       u16 off;
> >       u8 bit;
> >  };
>
> >
> >  #define DEF_RST(_id, _off, _bit)     \
> > -     [_id] = { \
> > +     { \
> > +             .id = (_id), \
> >               .off = (_off), \
> >               .bit = (_bit) \
> >       }
> > --
> > 2.25.1
>

RE: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Biju Das]

Hi Prabhakar,
> Subject: Re: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack
> the resets instead of indexing
> 
> Hi Biju,
> 
> Thank you for the review.
> 
> On Thu, May 5, 2022 at 8:48 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Lad Prabhakar,
> >
> > Thanks for the patch.
> >
> > > Subject: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to
> > > stack the resets instead of indexing
> > >
> > > Instead of indexing the resets, stack them and instead create an id
> > > member in struct rzg2l_reset to store the index. With this approach
> > > for every id we will have to loop through the resets array to match the
> id.
> > >
> > > This in preparation to add support for Renesas RZ/Five CPG in
> > > r9a07g043- cpg.c file where the resets array will be split up into
> > > three i.e. common and two SoC specific.
> > >
> > > Signed-off-by: Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  drivers/clk/renesas/rzg2l-cpg.c | 76
> > > ++++++++++++++++++++++++++------- drivers/clk/renesas/rzg2l-cpg.h |
> > > 4 +-
> > >  2 files changed, 63 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/clk/renesas/rzg2l-cpg.c
> > > b/drivers/clk/renesas/rzg2l- cpg.c index 1ce35f65682b..94fe307ec4c5
> > > 100644
> > > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > > @@ -681,14 +681,37 @@ rzg2l_cpg_register_mod_clk(const struct
> > > rzg2l_mod_clk *mod,
> > >
> > >  #define rcdev_to_priv(x)     container_of(x, struct rzg2l_cpg_priv,
> > > rcdev)
> > >
> > > +static const struct rzg2l_reset
> > > +*rzg2l_get_reset_ptr(struct rzg2l_cpg_priv *priv,
> > > +                  unsigned long id)
> > > +
> > > +{
> > > +     const struct rzg2l_cpg_info *info = priv->info;
> > > +     unsigned int i;
> > > +
> > > +     for (i = 0; i < priv->num_resets; i++) {
> > > +             if (info->resets[i].id == id)
> > > +                     return &info->resets[i];
> > > +     }
> >
> > Is it not possible to use shared reset like RZ/G2L and RZ/V2L?, which
> > has optimal memory and performance wise we can avoid bigger loop.
> >
> > Like adding Last index of RZ/Five as last reset index and Handle
> > RZ/G2UL specific as invalid reset index in xlate??
> >
> So we will have to maintain an array id's which are invalid to RZ/Five SoC.
> For this too we will have to loop at runtime itself. The array for invalid
> index will be big too.

As per [1], it will be 25 resets.

if you invalidate RZ/G2L specific resets in probe, there is no runtime overhead.
when a device match found, the info->reset_callback() which is mentioned in the next mail
and invalidate the resets(resets[id].off = 0)

ie,

if(info->reset_callback)
 info->reset_callback();

and on r9a07g043-cpg.c, make resets[id].off = 0 to invalidate the resets.

https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tree/include/dt-bindings/clock/r9a07g043-cpg.h




> 
> >
> > > +
> > > +     return NULL;
> > > +}
> > > +
> > >  static int rzg2l_cpg_reset(struct reset_controller_dev *rcdev,
> > >                          unsigned long id)  {
> > >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -     const struct rzg2l_cpg_info *info = priv->info;
> > > -     unsigned int reg = info->resets[id].off;
> > > -     u32 dis = BIT(info->resets[id].bit);
> > > -     u32 we = dis << 16;
> > > +     const struct rzg2l_reset *reset;
> > > +     unsigned int reg;
> > > +     u32 dis, we;
> > > +
> > > +     reset = rzg2l_get_reset_ptr(priv, id);
> > > +     if (!reset)
> > > +             return -EINVAL;
> > > +
> > > +     reg = reset->off;
> > > +     dis = BIT(reset->bit);
> > > +     we = dis << 16;
> > >
> > >       dev_dbg(rcdev->dev, "reset id:%ld offset:0x%x\n", id,
> > > CLK_RST_R(reg));
> > >
> > > @@ -708,9 +731,16 @@ static int rzg2l_cpg_assert(struct
> > > reset_controller_dev *rcdev,
> > >                           unsigned long id)  {
> > >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -     const struct rzg2l_cpg_info *info = priv->info;
> > > -     unsigned int reg = info->resets[id].off;
> > > -     u32 value = BIT(info->resets[id].bit) << 16;
> > > +     const struct rzg2l_reset *reset;
> > > +     unsigned int reg;
> > > +     u32 value;
> > > +
> > > +     reset = rzg2l_get_reset_ptr(priv, id);
> > > +     if (!reset)
> > > +             return -EINVAL;
> > > +
> > > +     reg = reset->off;
> > > +     value = BIT(reset->bit) << 16;
> > >
> > >       dev_dbg(rcdev->dev, "assert id:%ld offset:0x%x\n", id,
> > > CLK_RST_R(reg));
> > >
> > > @@ -722,11 +752,17 @@ static int rzg2l_cpg_deassert(struct
> > > reset_controller_dev *rcdev,
> > >                             unsigned long id)  {
> > >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -     const struct rzg2l_cpg_info *info = priv->info;
> > > -     unsigned int reg = info->resets[id].off;
> > > -     u32 dis = BIT(info->resets[id].bit);
> > > -     u32 value = (dis << 16) | dis;
> > > +     const struct rzg2l_reset *reset;
> > > +     unsigned int reg;
> > > +     u32 dis, value;
> > > +
> > > +     reset = rzg2l_get_reset_ptr(priv, id);
> > > +     if (!reset)
> > > +             return -EINVAL;
> > >
> > > +     reg = reset->off;
> > > +     dis = BIT(reset->bit);
> > > +     value = (dis << 16) | dis;
> > >       dev_dbg(rcdev->dev, "deassert id:%ld offset:0x%x\n", id,
> > >               CLK_RST_R(reg));
> > >
> > > @@ -738,9 +774,16 @@ static int rzg2l_cpg_status(struct
> > > reset_controller_dev *rcdev,
> > >                           unsigned long id)  {
> > >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -     const struct rzg2l_cpg_info *info = priv->info;
> > > -     unsigned int reg = info->resets[id].off;
> > > -     u32 bitmask = BIT(info->resets[id].bit);
> > > +     const struct rzg2l_reset *reset;
> > > +     unsigned int reg;
> > > +     u32 bitmask;
> > > +
> > > +     reset = rzg2l_get_reset_ptr(priv, id);
> > > +     if (!reset)
> > > +             return -EINVAL;
> > > +
> > > +     reg = reset->off;
> > > +     bitmask = BIT(reset->bit);
> > >
> > >       return !(readl(priv->base + CLK_MRST_R(reg)) & bitmask);  } @@
> > > -
> > > 756,10 +799,11 @@ static int rzg2l_cpg_reset_xlate(struct
> > > reset_controller_dev *rcdev,
> > >                                const struct of_phandle_args
> *reset_spec)  {
> > >       struct rzg2l_cpg_priv *priv = rcdev_to_priv(rcdev);
> > > -     const struct rzg2l_cpg_info *info = priv->info;
> > >       unsigned int id = reset_spec->args[0];
> > > +     const struct rzg2l_reset *reset;
> > >
> > > -     if (id >= rcdev->nr_resets || !info->resets[id].off) {
> > > +     reset = rzg2l_get_reset_ptr(priv, id);
> > > +     if (!reset) {
> > >               dev_err(rcdev->dev, "Invalid reset index %u\n", id);
> > >               return -EINVAL;
> > >       }
> > > diff --git a/drivers/clk/renesas/rzg2l-cpg.h
> > > b/drivers/clk/renesas/rzg2l- cpg.h index 92c88f42ca7f..a99f2ba7868f
> > > 100644
> > > --- a/drivers/clk/renesas/rzg2l-cpg.h
> > > +++ b/drivers/clk/renesas/rzg2l-cpg.h
> > > @@ -152,12 +152,14 @@ struct rzg2l_mod_clk {
> > >   * @bit: reset bit
> > >   */
> > >  struct rzg2l_reset {
> > > +     unsigned int id;
> >
> > Now you are adding 4 bytes to each reset entry in the LUT.
> >
> Agreed, on the other hand we are saving space on the entries (for example
> not all the reset entries are listed in the array and the array length will
> always be equal to last index)

Not now, But eventually we will fill all the resets, that is reason it is declared in header file.

Cheers,
Biju


> >
> > >       u16 off;
> > >       u8 bit;
> > >  };
> >
> > >
> > >  #define DEF_RST(_id, _off, _bit)     \
> > > -     [_id] = { \
> > > +     { \
> > > +             .id = (_id), \
> > >               .off = (_off), \
> > >               .bit = (_bit) \
> > >       }
> > > --
> > > 2.25.1
> >

Re: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Lad, Prabhakar]

Hi Biju,

On Fri, May 6, 2022 at 1:11 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Prabhakar,
> > Subject: Re: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack
> > the resets instead of indexing
> >
> > Hi Biju,
> >
> > Thank you for the review.
> >
> > On Thu, May 5, 2022 at 8:48 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > >
> > > Hi Lad Prabhakar,
> > >
> > > Thanks for the patch.
> > >
> > > > Subject: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to
> > > > stack the resets instead of indexing
> > > >
> > > > Instead of indexing the resets, stack them and instead create an id
> > > > member in struct rzg2l_reset to store the index. With this approach
> > > > for every id we will have to loop through the resets array to match the
> > id.
> > > >
> > > > This in preparation to add support for Renesas RZ/Five CPG in
> > > > r9a07g043- cpg.c file where the resets array will be split up into
> > > > three i.e. common and two SoC specific.
> > > >
> > > > Signed-off-by: Lad Prabhakar
> > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > >  drivers/clk/renesas/rzg2l-cpg.c | 76
> > > > ++++++++++++++++++++++++++------- drivers/clk/renesas/rzg2l-cpg.h |
> > > > 4 +-
> > > >  2 files changed, 63 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/drivers/clk/renesas/rzg2l-cpg.c
> > > > b/drivers/clk/renesas/rzg2l- cpg.c index 1ce35f65682b..94fe307ec4c5
> > > > 100644
> > > > --- a/drivers/clk/renesas/rzg2l-cpg.c
> > > > +++ b/drivers/clk/renesas/rzg2l-cpg.c
> > > > @@ -681,14 +681,37 @@ rzg2l_cpg_register_mod_clk(const struct
> > > > rzg2l_mod_clk *mod,
> > > >
> > > >  #define rcdev_to_priv(x)     container_of(x, struct rzg2l_cpg_priv,
> > > > rcdev)
> > > >
> > > > +static const struct rzg2l_reset
> > > > +*rzg2l_get_reset_ptr(struct rzg2l_cpg_priv *priv,
> > > > +                  unsigned long id)
> > > > +
> > > > +{
> > > > +     const struct rzg2l_cpg_info *info = priv->info;
> > > > +     unsigned int i;
> > > > +
> > > > +     for (i = 0; i < priv->num_resets; i++) {
> > > > +             if (info->resets[i].id == id)
> > > > +                     return &info->resets[i];
> > > > +     }
> > >
> > > Is it not possible to use shared reset like RZ/G2L and RZ/V2L?, which
> > > has optimal memory and performance wise we can avoid bigger loop.
> > >
> > > Like adding Last index of RZ/Five as last reset index and Handle
> > > RZ/G2UL specific as invalid reset index in xlate??
> > >
> > So we will have to maintain an array id's which are invalid to RZ/Five SoC.
> > For this too we will have to loop at runtime itself. The array for invalid
> > index will be big too.
>
> As per [1], it will be 25 resets.
>
> if you invalidate RZ/G2L specific resets in probe, there is no runtime overhead.
> when a device match found, the info->reset_callback() which is mentioned in the next mail
> and invalidate the resets(resets[id].off = 0)
>
Ahh right got that. I'll wait for Geert if he has more cunning ideas.
If not I'll go with your suggested approach.

> ie,
>
> if(info->reset_callback)
>  info->reset_callback();
>
> and on r9a07g043-cpg.c, make resets[id].off = 0 to invalidate the resets.
>
OK.

> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tree/include/dt-bindings/clock/r9a07g043-cpg.h
>
>
Cheers,
Prabhakar

Re: [RFC PATCH 2/4] clk: renesas: rzg2l-cpg: Add support to stack the resets instead of indexing

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, May 5, 2022 at 9:32 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Instead of indexing the resets, stack them and instead create an id member
> in struct rzg2l_reset to store the index. With this approach for every id
> we will have to loop through the resets array to match the id.
>
> This in preparation to add support for Renesas RZ/Five CPG in
> r9a07g043-cpg.c file where the resets array will be split up into three
> i.e. common and two SoC specific.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

An obvious alternative would be to allocate an array with pointers to
the individual resets, like is done for clocks.

Please see the suggestion in my reply to "[RFC PATCH 3/4] clk: renesas:
r9a07g043: Split up core, module and resets array", which would make
this patch unnecessary.

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

[RFC PATCH 3/4] clk: renesas: r9a07g043: Split up core, module and resets array

[Lad Prabhakar]

In preparation for adding support for Renesas RZ/Five SoC as part of
r9a07g043-cpg.c file, split up the core clock, module clock and resets
array into common and SoC specific.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g043-cpg.c | 488 +++++++++++++++-------------
 1 file changed, 267 insertions(+), 221 deletions(-)

diff --git a/drivers/clk/renesas/r9a07g043-cpg.c b/drivers/clk/renesas/r9a07g043-cpg.c
index 59d43c3b6a41..27b47ecfe4d8 100644
--- a/drivers/clk/renesas/r9a07g043-cpg.c
+++ b/drivers/clk/renesas/r9a07g043-cpg.c
@@ -36,9 +36,11 @@ enum clk_ids {
 	CLK_PLL3_DIV2_4_2,
 	CLK_SEL_PLL3_3,
 	CLK_DIV_PLL3_C,
+#ifndef CONFIG_RISCV
 	CLK_PLL5,
 	CLK_PLL5_500,
 	CLK_PLL5_250,
+#endif
 	CLK_PLL6,
 	CLK_PLL6_250,
 	CLK_P1_DIV2,
@@ -76,227 +78,271 @@ static const char * const sel_pll3_3[] = { ".pll3_533", ".pll3_400" };
 static const char * const sel_pll6_2[]	= { ".pll6_250", ".pll5_250" };
 static const char * const sel_shdi[] = { ".clk_533", ".clk_400", ".clk_266" };
 
-static const struct cpg_core_clk r9a07g043_core_clks[] __initconst = {
-	/* External Clock Inputs */
-	DEF_INPUT("extal", CLK_EXTAL),
+static const struct {
+	struct cpg_core_clk common[38];
+#ifndef CONFIG_RISCV
+	struct cpg_core_clk rzg2ul[3];
+#endif
+} core_clks __initconst = {
+	.common = {
+		/* External Clock Inputs */
+		DEF_INPUT("extal", CLK_EXTAL),
 
-	/* Internal Core Clocks */
-	DEF_FIXED(".osc", R9A07G043_OSCCLK, CLK_EXTAL, 1, 1),
-	DEF_FIXED(".osc_div1000", CLK_OSC_DIV1000, CLK_EXTAL, 1, 1000),
-	DEF_SAMPLL(".pll1", CLK_PLL1, CLK_EXTAL, PLL146_CONF(0)),
-	DEF_FIXED(".pll2", CLK_PLL2, CLK_EXTAL, 200, 3),
-	DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 1, 2),
-	DEF_FIXED(".clk_800", CLK_PLL2_800, CLK_PLL2, 1, 2),
-	DEF_FIXED(".clk_533", CLK_PLL2_SDHI_533, CLK_PLL2, 1, 3),
-	DEF_FIXED(".clk_400", CLK_PLL2_SDHI_400, CLK_PLL2_800, 1, 2),
-	DEF_FIXED(".clk_266", CLK_PLL2_SDHI_266, CLK_PLL2_SDHI_533, 1, 2),
-	DEF_FIXED(".pll2_div2_8", CLK_PLL2_DIV2_8, CLK_PLL2_DIV2, 1, 8),
-	DEF_FIXED(".pll2_div2_10", CLK_PLL2_DIV2_10, CLK_PLL2_DIV2, 1, 10),
-	DEF_FIXED(".pll3", CLK_PLL3, CLK_EXTAL, 200, 3),
-	DEF_FIXED(".pll3_div2", CLK_PLL3_DIV2, CLK_PLL3, 1, 2),
-	DEF_FIXED(".pll3_div2_4", CLK_PLL3_DIV2_4, CLK_PLL3_DIV2, 1, 4),
-	DEF_FIXED(".pll3_div2_4_2", CLK_PLL3_DIV2_4_2, CLK_PLL3_DIV2_4, 1, 2),
-	DEF_FIXED(".pll3_400", CLK_PLL3_400, CLK_PLL3, 1, 4),
-	DEF_FIXED(".pll3_533", CLK_PLL3_533, CLK_PLL3, 1, 3),
-	DEF_MUX(".sel_pll3_3", CLK_SEL_PLL3_3, SEL_PLL3_3,
-		sel_pll3_3, ARRAY_SIZE(sel_pll3_3), 0, CLK_MUX_READ_ONLY),
-	DEF_DIV("divpl3c", CLK_DIV_PLL3_C, CLK_SEL_PLL3_3,
-		DIVPL3C, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
-	DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
-	DEF_FIXED(".pll5_500", CLK_PLL5_500, CLK_PLL5, 1, 6),
-	DEF_FIXED(".pll5_250", CLK_PLL5_250, CLK_PLL5_500, 1, 2),
-	DEF_FIXED(".pll6", CLK_PLL6, CLK_EXTAL, 125, 6),
-	DEF_FIXED(".pll6_250", CLK_PLL6_250, CLK_PLL6, 1, 2),
+		/* Internal Core Clocks */
+		DEF_FIXED(".osc", R9A07G043_OSCCLK, CLK_EXTAL, 1, 1),
+		DEF_FIXED(".osc_div1000", CLK_OSC_DIV1000, CLK_EXTAL, 1, 1000),
+		DEF_SAMPLL(".pll1", CLK_PLL1, CLK_EXTAL, PLL146_CONF(0)),
+		DEF_FIXED(".pll2", CLK_PLL2, CLK_EXTAL, 200, 3),
+		DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 1, 2),
+		DEF_FIXED(".clk_800", CLK_PLL2_800, CLK_PLL2, 1, 2),
+		DEF_FIXED(".clk_533", CLK_PLL2_SDHI_533, CLK_PLL2, 1, 3),
+		DEF_FIXED(".clk_400", CLK_PLL2_SDHI_400, CLK_PLL2_800, 1, 2),
+		DEF_FIXED(".clk_266", CLK_PLL2_SDHI_266, CLK_PLL2_SDHI_533, 1, 2),
+		DEF_FIXED(".pll2_div2_8", CLK_PLL2_DIV2_8, CLK_PLL2_DIV2, 1, 8),
+		DEF_FIXED(".pll2_div2_10", CLK_PLL2_DIV2_10, CLK_PLL2_DIV2, 1, 10),
+		DEF_FIXED(".pll3", CLK_PLL3, CLK_EXTAL, 200, 3),
+		DEF_FIXED(".pll3_div2", CLK_PLL3_DIV2, CLK_PLL3, 1, 2),
+		DEF_FIXED(".pll3_div2_4", CLK_PLL3_DIV2_4, CLK_PLL3_DIV2, 1, 4),
+		DEF_FIXED(".pll3_div2_4_2", CLK_PLL3_DIV2_4_2, CLK_PLL3_DIV2_4, 1, 2),
+		DEF_FIXED(".pll3_400", CLK_PLL3_400, CLK_PLL3, 1, 4),
+		DEF_FIXED(".pll3_533", CLK_PLL3_533, CLK_PLL3, 1, 3),
+		DEF_MUX(".sel_pll3_3", CLK_SEL_PLL3_3, SEL_PLL3_3,
+			sel_pll3_3, ARRAY_SIZE(sel_pll3_3), 0, CLK_MUX_READ_ONLY),
+		DEF_DIV("divpl3c", CLK_DIV_PLL3_C, CLK_SEL_PLL3_3,
+			DIVPL3C, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
+		DEF_FIXED(".pll6", CLK_PLL6, CLK_EXTAL, 125, 6),
+		DEF_FIXED(".pll6_250", CLK_PLL6_250, CLK_PLL6, 1, 2),
+
+		/* Core output clk */
+		DEF_DIV("I", R9A07G043_CLK_I, CLK_PLL1, DIVPL1A, dtable_1_8,
+			CLK_DIVIDER_HIWORD_MASK),
+		DEF_DIV("P0", R9A07G043_CLK_P0, CLK_PLL2_DIV2_8, DIVPL2A,
+			dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
+		DEF_FIXED("P0_DIV2", R9A07G043_CLK_P0_DIV2, R9A07G043_CLK_P0, 1, 2),
+		DEF_FIXED("TSU", R9A07G043_CLK_TSU, CLK_PLL2_DIV2_10, 1, 1),
+		DEF_DIV("P1", R9A07G043_CLK_P1, CLK_PLL3_DIV2_4,
+			DIVPL3B, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
+		DEF_FIXED("P1_DIV2", CLK_P1_DIV2, R9A07G043_CLK_P1, 1, 2),
+		DEF_DIV("P2", R9A07G043_CLK_P2, CLK_PLL3_DIV2_4_2,
+			DIVPL3A, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
+		DEF_FIXED("M0", R9A07G043_CLK_M0, CLK_PLL3_DIV2_4, 1, 1),
+		DEF_FIXED("ZT", R9A07G043_CLK_ZT, CLK_PLL3_DIV2_4_2, 1, 1),
+		DEF_MUX("HP", R9A07G043_CLK_HP, SEL_PLL6_2,
+			sel_pll6_2, ARRAY_SIZE(sel_pll6_2), 0, CLK_MUX_HIWORD_MASK),
+		DEF_FIXED("SPI0", R9A07G043_CLK_SPI0, CLK_DIV_PLL3_C, 1, 2),
+		DEF_FIXED("SPI1", R9A07G043_CLK_SPI1, CLK_DIV_PLL3_C, 1, 4),
+		DEF_SD_MUX("SD0", R9A07G043_CLK_SD0, SEL_SDHI0,
+			   sel_shdi, ARRAY_SIZE(sel_shdi)),
+		DEF_SD_MUX("SD1", R9A07G043_CLK_SD1, SEL_SDHI1,
+			   sel_shdi, ARRAY_SIZE(sel_shdi)),
+		DEF_FIXED("SD0_DIV4", CLK_SD0_DIV4, R9A07G043_CLK_SD0, 1, 4),
+		DEF_FIXED("SD1_DIV4", CLK_SD1_DIV4, R9A07G043_CLK_SD1, 1, 4),
+	},
+#ifndef CONFIG_RISCV
+	.rzg2ul = {
+		DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
+		DEF_FIXED(".pll5_500", CLK_PLL5_500, CLK_PLL5, 1, 6),
+		DEF_FIXED(".pll5_250", CLK_PLL5_250, CLK_PLL5_500, 1, 2),
+	},
+#endif
 
-	/* Core output clk */
-	DEF_DIV("I", R9A07G043_CLK_I, CLK_PLL1, DIVPL1A, dtable_1_8,
-		CLK_DIVIDER_HIWORD_MASK),
-	DEF_DIV("P0", R9A07G043_CLK_P0, CLK_PLL2_DIV2_8, DIVPL2A,
-		dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
-	DEF_FIXED("P0_DIV2", R9A07G043_CLK_P0_DIV2, R9A07G043_CLK_P0, 1, 2),
-	DEF_FIXED("TSU", R9A07G043_CLK_TSU, CLK_PLL2_DIV2_10, 1, 1),
-	DEF_DIV("P1", R9A07G043_CLK_P1, CLK_PLL3_DIV2_4,
-		DIVPL3B, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
-	DEF_FIXED("P1_DIV2", CLK_P1_DIV2, R9A07G043_CLK_P1, 1, 2),
-	DEF_DIV("P2", R9A07G043_CLK_P2, CLK_PLL3_DIV2_4_2,
-		DIVPL3A, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
-	DEF_FIXED("M0", R9A07G043_CLK_M0, CLK_PLL3_DIV2_4, 1, 1),
-	DEF_FIXED("ZT", R9A07G043_CLK_ZT, CLK_PLL3_DIV2_4_2, 1, 1),
-	DEF_MUX("HP", R9A07G043_CLK_HP, SEL_PLL6_2,
-		sel_pll6_2, ARRAY_SIZE(sel_pll6_2), 0, CLK_MUX_HIWORD_MASK),
-	DEF_FIXED("SPI0", R9A07G043_CLK_SPI0, CLK_DIV_PLL3_C, 1, 2),
-	DEF_FIXED("SPI1", R9A07G043_CLK_SPI1, CLK_DIV_PLL3_C, 1, 4),
-	DEF_SD_MUX("SD0", R9A07G043_CLK_SD0, SEL_SDHI0,
-		   sel_shdi, ARRAY_SIZE(sel_shdi)),
-	DEF_SD_MUX("SD1", R9A07G043_CLK_SD1, SEL_SDHI1,
-		   sel_shdi, ARRAY_SIZE(sel_shdi)),
-	DEF_FIXED("SD0_DIV4", CLK_SD0_DIV4, R9A07G043_CLK_SD0, 1, 4),
-	DEF_FIXED("SD1_DIV4", CLK_SD1_DIV4, R9A07G043_CLK_SD1, 1, 4),
 };
 
-static struct rzg2l_mod_clk r9a07g043_mod_clks[] = {
-	DEF_MOD("gic",		R9A07G043_GIC600_GICCLK, R9A07G043_CLK_P1,
-				0x514, 0),
-	DEF_MOD("ia55_pclk",	R9A07G043_IA55_PCLK, R9A07G043_CLK_P2,
-				0x518, 0),
-	DEF_MOD("ia55_clk",	R9A07G043_IA55_CLK, R9A07G043_CLK_P1,
-				0x518, 1),
-	DEF_MOD("dmac_aclk",	R9A07G043_DMAC_ACLK, R9A07G043_CLK_P1,
-				0x52c, 0),
-	DEF_MOD("dmac_pclk",	R9A07G043_DMAC_PCLK, CLK_P1_DIV2,
-				0x52c, 1),
-	DEF_MOD("ostm0",	R9A07G043_OSTM0_PCLK, R9A07G043_CLK_P0,
-				0x534, 0),
-	DEF_MOD("ostm1",	R9A07G043_OSTM1_PCLK, R9A07G043_CLK_P0,
-				0x534, 1),
-	DEF_MOD("ostm2",	R9A07G043_OSTM2_PCLK, R9A07G043_CLK_P0,
-				0x534, 2),
-	DEF_MOD("wdt0_pclk",	R9A07G043_WDT0_PCLK, R9A07G043_CLK_P0,
-				0x548, 0),
-	DEF_MOD("wdt0_clk",	R9A07G043_WDT0_CLK, R9A07G043_OSCCLK,
-				0x548, 1),
-	DEF_MOD("wdt2_pclk",	R9A07G043_WDT2_PCLK, R9A07G043_CLK_P0,
-				0x548, 4),
-	DEF_MOD("wdt2_clk",	R9A07G043_WDT2_CLK, R9A07G043_OSCCLK,
-				0x548, 5),
-	DEF_MOD("spi_clk2",	R9A07G043_SPI_CLK2, R9A07G043_CLK_SPI1,
-				0x550, 0),
-	DEF_MOD("spi_clk",	R9A07G043_SPI_CLK, R9A07G043_CLK_SPI0,
-				0x550, 1),
-	DEF_MOD("sdhi0_imclk",	R9A07G043_SDHI0_IMCLK, CLK_SD0_DIV4,
-				0x554, 0),
-	DEF_MOD("sdhi0_imclk2",	R9A07G043_SDHI0_IMCLK2, CLK_SD0_DIV4,
-				0x554, 1),
-	DEF_MOD("sdhi0_clk_hs",	R9A07G043_SDHI0_CLK_HS, R9A07G043_CLK_SD0,
-				0x554, 2),
-	DEF_MOD("sdhi0_aclk",	R9A07G043_SDHI0_ACLK, R9A07G043_CLK_P1,
-				0x554, 3),
-	DEF_MOD("sdhi1_imclk",	R9A07G043_SDHI1_IMCLK, CLK_SD1_DIV4,
-				0x554, 4),
-	DEF_MOD("sdhi1_imclk2",	R9A07G043_SDHI1_IMCLK2, CLK_SD1_DIV4,
-				0x554, 5),
-	DEF_MOD("sdhi1_clk_hs",	R9A07G043_SDHI1_CLK_HS, R9A07G043_CLK_SD1,
-				0x554, 6),
-	DEF_MOD("sdhi1_aclk",	R9A07G043_SDHI1_ACLK, R9A07G043_CLK_P1,
-				0x554, 7),
-	DEF_MOD("ssi0_pclk",	R9A07G043_SSI0_PCLK2, R9A07G043_CLK_P0,
-				0x570, 0),
-	DEF_MOD("ssi0_sfr",	R9A07G043_SSI0_PCLK_SFR, R9A07G043_CLK_P0,
-				0x570, 1),
-	DEF_MOD("ssi1_pclk",	R9A07G043_SSI1_PCLK2, R9A07G043_CLK_P0,
-				0x570, 2),
-	DEF_MOD("ssi1_sfr",	R9A07G043_SSI1_PCLK_SFR, R9A07G043_CLK_P0,
-				0x570, 3),
-	DEF_MOD("ssi2_pclk",	R9A07G043_SSI2_PCLK2, R9A07G043_CLK_P0,
-				0x570, 4),
-	DEF_MOD("ssi2_sfr",	R9A07G043_SSI2_PCLK_SFR, R9A07G043_CLK_P0,
-				0x570, 5),
-	DEF_MOD("ssi3_pclk",	R9A07G043_SSI3_PCLK2, R9A07G043_CLK_P0,
-				0x570, 6),
-	DEF_MOD("ssi3_sfr",	R9A07G043_SSI3_PCLK_SFR, R9A07G043_CLK_P0,
-				0x570, 7),
-	DEF_MOD("usb0_host",	R9A07G043_USB_U2H0_HCLK, R9A07G043_CLK_P1,
-				0x578, 0),
-	DEF_MOD("usb1_host",	R9A07G043_USB_U2H1_HCLK, R9A07G043_CLK_P1,
-				0x578, 1),
-	DEF_MOD("usb0_func",	R9A07G043_USB_U2P_EXR_CPUCLK, R9A07G043_CLK_P1,
-				0x578, 2),
-	DEF_MOD("usb_pclk",	R9A07G043_USB_PCLK, R9A07G043_CLK_P1,
-				0x578, 3),
-	DEF_COUPLED("eth0_axi",	R9A07G043_ETH0_CLK_AXI, R9A07G043_CLK_M0,
-				0x57c, 0),
-	DEF_COUPLED("eth0_chi",	R9A07G043_ETH0_CLK_CHI, R9A07G043_CLK_ZT,
-				0x57c, 0),
-	DEF_COUPLED("eth1_axi",	R9A07G043_ETH1_CLK_AXI, R9A07G043_CLK_M0,
-				0x57c, 1),
-	DEF_COUPLED("eth1_chi",	R9A07G043_ETH1_CLK_CHI, R9A07G043_CLK_ZT,
-				0x57c, 1),
-	DEF_MOD("i2c0",		R9A07G043_I2C0_PCLK, R9A07G043_CLK_P0,
-				0x580, 0),
-	DEF_MOD("i2c1",		R9A07G043_I2C1_PCLK, R9A07G043_CLK_P0,
-				0x580, 1),
-	DEF_MOD("i2c2",		R9A07G043_I2C2_PCLK, R9A07G043_CLK_P0,
-				0x580, 2),
-	DEF_MOD("i2c3",		R9A07G043_I2C3_PCLK, R9A07G043_CLK_P0,
-				0x580, 3),
-	DEF_MOD("scif0",	R9A07G043_SCIF0_CLK_PCK, R9A07G043_CLK_P0,
-				0x584, 0),
-	DEF_MOD("scif1",	R9A07G043_SCIF1_CLK_PCK, R9A07G043_CLK_P0,
-				0x584, 1),
-	DEF_MOD("scif2",	R9A07G043_SCIF2_CLK_PCK, R9A07G043_CLK_P0,
-				0x584, 2),
-	DEF_MOD("scif3",	R9A07G043_SCIF3_CLK_PCK, R9A07G043_CLK_P0,
-				0x584, 3),
-	DEF_MOD("scif4",	R9A07G043_SCIF4_CLK_PCK, R9A07G043_CLK_P0,
-				0x584, 4),
-	DEF_MOD("sci0",		R9A07G043_SCI0_CLKP, R9A07G043_CLK_P0,
-				0x588, 0),
-	DEF_MOD("sci1",		R9A07G043_SCI1_CLKP, R9A07G043_CLK_P0,
-				0x588, 1),
-	DEF_MOD("rspi0",	R9A07G043_RSPI0_CLKB, R9A07G043_CLK_P0,
-				0x590, 0),
-	DEF_MOD("rspi1",	R9A07G043_RSPI1_CLKB, R9A07G043_CLK_P0,
-				0x590, 1),
-	DEF_MOD("rspi2",	R9A07G043_RSPI2_CLKB, R9A07G043_CLK_P0,
-				0x590, 2),
-	DEF_MOD("canfd",	R9A07G043_CANFD_PCLK, R9A07G043_CLK_P0,
-				0x594, 0),
-	DEF_MOD("gpio",		R9A07G043_GPIO_HCLK, R9A07G043_OSCCLK,
-				0x598, 0),
-	DEF_MOD("adc_adclk",	R9A07G043_ADC_ADCLK, R9A07G043_CLK_TSU,
-				0x5a8, 0),
-	DEF_MOD("adc_pclk",	R9A07G043_ADC_PCLK, R9A07G043_CLK_P0,
-				0x5a8, 1),
-	DEF_MOD("tsu_pclk",	R9A07G043_TSU_PCLK, R9A07G043_CLK_TSU,
-				0x5ac, 0),
+static const struct {
+	struct rzg2l_mod_clk common[54];
+#ifdef CONFIG_RISCV
+	struct rzg2l_mod_clk rzfive[0];
+#else
+	struct rzg2l_mod_clk rzg2ul[3];
+#endif
+} mod_clks = {
+	.common = {
+		DEF_MOD("dmac_aclk",	R9A07G043_DMAC_ACLK, R9A07G043_CLK_P1,
+					0x52c, 0),
+		DEF_MOD("dmac_pclk",	R9A07G043_DMAC_PCLK, CLK_P1_DIV2,
+					0x52c, 1),
+		DEF_MOD("ostm0",	R9A07G043_OSTM0_PCLK, R9A07G043_CLK_P0,
+					0x534, 0),
+		DEF_MOD("ostm1",	R9A07G043_OSTM1_PCLK, R9A07G043_CLK_P0,
+					0x534, 1),
+		DEF_MOD("ostm2",	R9A07G043_OSTM2_PCLK, R9A07G043_CLK_P0,
+					0x534, 2),
+		DEF_MOD("wdt0_pclk",	R9A07G043_WDT0_PCLK, R9A07G043_CLK_P0,
+					0x548, 0),
+		DEF_MOD("wdt0_clk",	R9A07G043_WDT0_CLK, R9A07G043_OSCCLK,
+					0x548, 1),
+		DEF_MOD("wdt2_pclk",	R9A07G043_WDT2_PCLK, R9A07G043_CLK_P0,
+					0x548, 4),
+		DEF_MOD("wdt2_clk",	R9A07G043_WDT2_CLK, R9A07G043_OSCCLK,
+					0x548, 5),
+		DEF_MOD("spi_clk2",	R9A07G043_SPI_CLK2, R9A07G043_CLK_SPI1,
+					0x550, 0),
+		DEF_MOD("spi_clk",	R9A07G043_SPI_CLK, R9A07G043_CLK_SPI0,
+					0x550, 1),
+		DEF_MOD("sdhi0_imclk",	R9A07G043_SDHI0_IMCLK, CLK_SD0_DIV4,
+					0x554, 0),
+		DEF_MOD("sdhi0_imclk2",	R9A07G043_SDHI0_IMCLK2, CLK_SD0_DIV4,
+					0x554, 1),
+		DEF_MOD("sdhi0_clk_hs",	R9A07G043_SDHI0_CLK_HS, R9A07G043_CLK_SD0,
+					0x554, 2),
+		DEF_MOD("sdhi0_aclk",	R9A07G043_SDHI0_ACLK, R9A07G043_CLK_P1,
+					0x554, 3),
+		DEF_MOD("sdhi1_imclk",	R9A07G043_SDHI1_IMCLK, CLK_SD1_DIV4,
+					0x554, 4),
+		DEF_MOD("sdhi1_imclk2",	R9A07G043_SDHI1_IMCLK2, CLK_SD1_DIV4,
+					0x554, 5),
+		DEF_MOD("sdhi1_clk_hs",	R9A07G043_SDHI1_CLK_HS, R9A07G043_CLK_SD1,
+					0x554, 6),
+		DEF_MOD("sdhi1_aclk",	R9A07G043_SDHI1_ACLK, R9A07G043_CLK_P1,
+					0x554, 7),
+		DEF_MOD("ssi0_pclk",	R9A07G043_SSI0_PCLK2, R9A07G043_CLK_P0,
+					0x570, 0),
+		DEF_MOD("ssi0_sfr",	R9A07G043_SSI0_PCLK_SFR, R9A07G043_CLK_P0,
+					0x570, 1),
+		DEF_MOD("ssi1_pclk",	R9A07G043_SSI1_PCLK2, R9A07G043_CLK_P0,
+					0x570, 2),
+		DEF_MOD("ssi1_sfr",	R9A07G043_SSI1_PCLK_SFR, R9A07G043_CLK_P0,
+					0x570, 3),
+		DEF_MOD("ssi2_pclk",	R9A07G043_SSI2_PCLK2, R9A07G043_CLK_P0,
+					0x570, 4),
+		DEF_MOD("ssi2_sfr",	R9A07G043_SSI2_PCLK_SFR, R9A07G043_CLK_P0,
+					0x570, 5),
+		DEF_MOD("ssi3_pclk",	R9A07G043_SSI3_PCLK2, R9A07G043_CLK_P0,
+					0x570, 6),
+		DEF_MOD("ssi3_sfr",	R9A07G043_SSI3_PCLK_SFR, R9A07G043_CLK_P0,
+					0x570, 7),
+		DEF_MOD("usb0_host",	R9A07G043_USB_U2H0_HCLK, R9A07G043_CLK_P1,
+					0x578, 0),
+		DEF_MOD("usb1_host",	R9A07G043_USB_U2H1_HCLK, R9A07G043_CLK_P1,
+					0x578, 1),
+		DEF_MOD("usb0_func",	R9A07G043_USB_U2P_EXR_CPUCLK, R9A07G043_CLK_P1,
+					0x578, 2),
+		DEF_MOD("usb_pclk",	R9A07G043_USB_PCLK, R9A07G043_CLK_P1,
+					0x578, 3),
+		DEF_COUPLED("eth0_axi",	R9A07G043_ETH0_CLK_AXI, R9A07G043_CLK_M0,
+					0x57c, 0),
+		DEF_COUPLED("eth0_chi",	R9A07G043_ETH0_CLK_CHI, R9A07G043_CLK_ZT,
+					0x57c, 0),
+		DEF_COUPLED("eth1_axi",	R9A07G043_ETH1_CLK_AXI, R9A07G043_CLK_M0,
+					0x57c, 1),
+		DEF_COUPLED("eth1_chi",	R9A07G043_ETH1_CLK_CHI, R9A07G043_CLK_ZT,
+					0x57c, 1),
+		DEF_MOD("i2c0",		R9A07G043_I2C0_PCLK, R9A07G043_CLK_P0,
+					0x580, 0),
+		DEF_MOD("i2c1",		R9A07G043_I2C1_PCLK, R9A07G043_CLK_P0,
+					0x580, 1),
+		DEF_MOD("i2c2",		R9A07G043_I2C2_PCLK, R9A07G043_CLK_P0,
+					0x580, 2),
+		DEF_MOD("i2c3",		R9A07G043_I2C3_PCLK, R9A07G043_CLK_P0,
+					0x580, 3),
+		DEF_MOD("scif0",	R9A07G043_SCIF0_CLK_PCK, R9A07G043_CLK_P0,
+					0x584, 0),
+		DEF_MOD("scif1",	R9A07G043_SCIF1_CLK_PCK, R9A07G043_CLK_P0,
+					0x584, 1),
+		DEF_MOD("scif2",	R9A07G043_SCIF2_CLK_PCK, R9A07G043_CLK_P0,
+					0x584, 2),
+		DEF_MOD("scif3",	R9A07G043_SCIF3_CLK_PCK, R9A07G043_CLK_P0,
+					0x584, 3),
+		DEF_MOD("scif4",	R9A07G043_SCIF4_CLK_PCK, R9A07G043_CLK_P0,
+					0x584, 4),
+		DEF_MOD("sci0",		R9A07G043_SCI0_CLKP, R9A07G043_CLK_P0,
+					0x588, 0),
+		DEF_MOD("sci1",		R9A07G043_SCI1_CLKP, R9A07G043_CLK_P0,
+					0x588, 1),
+		DEF_MOD("rspi0",	R9A07G043_RSPI0_CLKB, R9A07G043_CLK_P0,
+					0x590, 0),
+		DEF_MOD("rspi1",	R9A07G043_RSPI1_CLKB, R9A07G043_CLK_P0,
+					0x590, 1),
+		DEF_MOD("rspi2",	R9A07G043_RSPI2_CLKB, R9A07G043_CLK_P0,
+					0x590, 2),
+		DEF_MOD("canfd",	R9A07G043_CANFD_PCLK, R9A07G043_CLK_P0,
+					0x594, 0),
+		DEF_MOD("gpio",		R9A07G043_GPIO_HCLK, R9A07G043_OSCCLK,
+					0x598, 0),
+		DEF_MOD("adc_adclk",	R9A07G043_ADC_ADCLK, R9A07G043_CLK_TSU,
+					0x5a8, 0),
+		DEF_MOD("adc_pclk",	R9A07G043_ADC_PCLK, R9A07G043_CLK_P0,
+					0x5a8, 1),
+		DEF_MOD("tsu_pclk",	R9A07G043_TSU_PCLK, R9A07G043_CLK_TSU,
+					0x5ac, 0),
+	},
+#ifdef CONFIG_RISCV
+	.rzfive = {
+	},
+#else
+	.rzg2ul = {
+		DEF_MOD("gic",		R9A07G043_GIC600_GICCLK, R9A07G043_CLK_P1,
+					0x514, 0),
+		DEF_MOD("ia55_pclk",	R9A07G043_IA55_PCLK, R9A07G043_CLK_P2,
+					0x518, 0),
+		DEF_MOD("ia55_clk",	R9A07G043_IA55_CLK, R9A07G043_CLK_P1,
+					0x518, 1),
+	},
+#endif
 };
 
-static struct rzg2l_reset r9a07g043_resets[] = {
-	DEF_RST(R9A07G043_GIC600_GICRESET_N, 0x814, 0),
-	DEF_RST(R9A07G043_GIC600_DBG_GICRESET_N, 0x814, 1),
-	DEF_RST(R9A07G043_IA55_RESETN, 0x818, 0),
-	DEF_RST(R9A07G043_DMAC_ARESETN, 0x82c, 0),
-	DEF_RST(R9A07G043_DMAC_RST_ASYNC, 0x82c, 1),
-	DEF_RST(R9A07G043_OSTM0_PRESETZ, 0x834, 0),
-	DEF_RST(R9A07G043_OSTM1_PRESETZ, 0x834, 1),
-	DEF_RST(R9A07G043_OSTM2_PRESETZ, 0x834, 2),
-	DEF_RST(R9A07G043_WDT0_PRESETN, 0x848, 0),
-	DEF_RST(R9A07G043_WDT2_PRESETN, 0x848, 2),
-	DEF_RST(R9A07G043_SPI_RST, 0x850, 0),
-	DEF_RST(R9A07G043_SDHI0_IXRST, 0x854, 0),
-	DEF_RST(R9A07G043_SDHI1_IXRST, 0x854, 1),
-	DEF_RST(R9A07G043_SSI0_RST_M2_REG, 0x870, 0),
-	DEF_RST(R9A07G043_SSI1_RST_M2_REG, 0x870, 1),
-	DEF_RST(R9A07G043_SSI2_RST_M2_REG, 0x870, 2),
-	DEF_RST(R9A07G043_SSI3_RST_M2_REG, 0x870, 3),
-	DEF_RST(R9A07G043_USB_U2H0_HRESETN, 0x878, 0),
-	DEF_RST(R9A07G043_USB_U2H1_HRESETN, 0x878, 1),
-	DEF_RST(R9A07G043_USB_U2P_EXL_SYSRST, 0x878, 2),
-	DEF_RST(R9A07G043_USB_PRESETN, 0x878, 3),
-	DEF_RST(R9A07G043_ETH0_RST_HW_N, 0x87c, 0),
-	DEF_RST(R9A07G043_ETH1_RST_HW_N, 0x87c, 1),
-	DEF_RST(R9A07G043_I2C0_MRST, 0x880, 0),
-	DEF_RST(R9A07G043_I2C1_MRST, 0x880, 1),
-	DEF_RST(R9A07G043_I2C2_MRST, 0x880, 2),
-	DEF_RST(R9A07G043_I2C3_MRST, 0x880, 3),
-	DEF_RST(R9A07G043_SCIF0_RST_SYSTEM_N, 0x884, 0),
-	DEF_RST(R9A07G043_SCIF1_RST_SYSTEM_N, 0x884, 1),
-	DEF_RST(R9A07G043_SCIF2_RST_SYSTEM_N, 0x884, 2),
-	DEF_RST(R9A07G043_SCIF3_RST_SYSTEM_N, 0x884, 3),
-	DEF_RST(R9A07G043_SCIF4_RST_SYSTEM_N, 0x884, 4),
-	DEF_RST(R9A07G043_SCI0_RST, 0x888, 0),
-	DEF_RST(R9A07G043_SCI1_RST, 0x888, 1),
-	DEF_RST(R9A07G043_RSPI0_RST, 0x890, 0),
-	DEF_RST(R9A07G043_RSPI1_RST, 0x890, 1),
-	DEF_RST(R9A07G043_RSPI2_RST, 0x890, 2),
-	DEF_RST(R9A07G043_CANFD_RSTP_N, 0x894, 0),
-	DEF_RST(R9A07G043_CANFD_RSTC_N, 0x894, 1),
-	DEF_RST(R9A07G043_GPIO_RSTN, 0x898, 0),
-	DEF_RST(R9A07G043_GPIO_PORT_RESETN, 0x898, 1),
-	DEF_RST(R9A07G043_GPIO_SPARE_RESETN, 0x898, 2),
-	DEF_RST(R9A07G043_ADC_PRESETN, 0x8a8, 0),
-	DEF_RST(R9A07G043_ADC_ADRST_N, 0x8a8, 1),
-	DEF_RST(R9A07G043_TSU_PRESETN, 0x8ac, 0),
+static const struct {
+	struct rzg2l_reset common[42];
+#ifdef CONFIG_RISCV
+	struct rzg2l_reset rzfive[0];
+#else
+	struct rzg2l_reset rzg2ul[3];
+#endif
+} resets = {
+	.common = {
+		DEF_RST(R9A07G043_DMAC_ARESETN, 0x82c, 0),
+		DEF_RST(R9A07G043_DMAC_RST_ASYNC, 0x82c, 1),
+		DEF_RST(R9A07G043_OSTM0_PRESETZ, 0x834, 0),
+		DEF_RST(R9A07G043_OSTM1_PRESETZ, 0x834, 1),
+		DEF_RST(R9A07G043_OSTM2_PRESETZ, 0x834, 2),
+		DEF_RST(R9A07G043_WDT0_PRESETN, 0x848, 0),
+		DEF_RST(R9A07G043_WDT2_PRESETN, 0x848, 2),
+		DEF_RST(R9A07G043_SPI_RST, 0x850, 0),
+		DEF_RST(R9A07G043_SDHI0_IXRST, 0x854, 0),
+		DEF_RST(R9A07G043_SDHI1_IXRST, 0x854, 1),
+		DEF_RST(R9A07G043_SSI0_RST_M2_REG, 0x870, 0),
+		DEF_RST(R9A07G043_SSI1_RST_M2_REG, 0x870, 1),
+		DEF_RST(R9A07G043_SSI2_RST_M2_REG, 0x870, 2),
+		DEF_RST(R9A07G043_SSI3_RST_M2_REG, 0x870, 3),
+		DEF_RST(R9A07G043_USB_U2H0_HRESETN, 0x878, 0),
+		DEF_RST(R9A07G043_USB_U2H1_HRESETN, 0x878, 1),
+		DEF_RST(R9A07G043_USB_U2P_EXL_SYSRST, 0x878, 2),
+		DEF_RST(R9A07G043_USB_PRESETN, 0x878, 3),
+		DEF_RST(R9A07G043_ETH0_RST_HW_N, 0x87c, 0),
+		DEF_RST(R9A07G043_ETH1_RST_HW_N, 0x87c, 1),
+		DEF_RST(R9A07G043_I2C0_MRST, 0x880, 0),
+		DEF_RST(R9A07G043_I2C1_MRST, 0x880, 1),
+		DEF_RST(R9A07G043_I2C2_MRST, 0x880, 2),
+		DEF_RST(R9A07G043_I2C3_MRST, 0x880, 3),
+		DEF_RST(R9A07G043_SCIF0_RST_SYSTEM_N, 0x884, 0),
+		DEF_RST(R9A07G043_SCIF1_RST_SYSTEM_N, 0x884, 1),
+		DEF_RST(R9A07G043_SCIF2_RST_SYSTEM_N, 0x884, 2),
+		DEF_RST(R9A07G043_SCIF3_RST_SYSTEM_N, 0x884, 3),
+		DEF_RST(R9A07G043_SCIF4_RST_SYSTEM_N, 0x884, 4),
+		DEF_RST(R9A07G043_SCI0_RST, 0x888, 0),
+		DEF_RST(R9A07G043_SCI1_RST, 0x888, 1),
+		DEF_RST(R9A07G043_RSPI0_RST, 0x890, 0),
+		DEF_RST(R9A07G043_RSPI1_RST, 0x890, 1),
+		DEF_RST(R9A07G043_RSPI2_RST, 0x890, 2),
+		DEF_RST(R9A07G043_CANFD_RSTP_N, 0x894, 0),
+		DEF_RST(R9A07G043_CANFD_RSTC_N, 0x894, 1),
+		DEF_RST(R9A07G043_GPIO_RSTN, 0x898, 0),
+		DEF_RST(R9A07G043_GPIO_PORT_RESETN, 0x898, 1),
+		DEF_RST(R9A07G043_GPIO_SPARE_RESETN, 0x898, 2),
+		DEF_RST(R9A07G043_ADC_PRESETN, 0x8a8, 0),
+		DEF_RST(R9A07G043_ADC_ADRST_N, 0x8a8, 1),
+		DEF_RST(R9A07G043_TSU_PRESETN, 0x8ac, 0),
+	},
+#ifdef CONFIG_RISCV
+	.rzfive = {
+	},
+#else
+	.rzg2ul = {
+		DEF_RST(R9A07G043_GIC600_GICRESET_N, 0x814, 0),
+		DEF_RST(R9A07G043_GIC600_DBG_GICRESET_N, 0x814, 1),
+		DEF_RST(R9A07G043_IA55_RESETN, 0x818, 0),
+	},
+#endif
 };
 
 static const unsigned int r9a07g043_crit_mod_clks[] __initconst = {
@@ -307,8 +353,8 @@ static const unsigned int r9a07g043_crit_mod_clks[] __initconst = {
 
 const struct rzg2l_cpg_info r9a07g043_cpg_info = {
 	/* Core Clocks */
-	.core_clks = r9a07g043_core_clks,
-	.num_core_clks = ARRAY_SIZE(r9a07g043_core_clks),
+	.core_clks = core_clks.common,
+	.num_core_clks = ARRAY_SIZE(core_clks.common) + ARRAY_SIZE(core_clks.rzg2ul),
 	.last_dt_core_clk = LAST_DT_CORE_CLK,
 	.num_total_core_clks = MOD_CLK_BASE,
 
@@ -317,11 +363,11 @@ const struct rzg2l_cpg_info r9a07g043_cpg_info = {
 	.num_crit_mod_clks = ARRAY_SIZE(r9a07g043_crit_mod_clks),
 
 	/* Module Clocks */
-	.mod_clks = r9a07g043_mod_clks,
-	.num_mod_clks = ARRAY_SIZE(r9a07g043_mod_clks),
+	.mod_clks = mod_clks.common,
+	.num_mod_clks = ARRAY_SIZE(mod_clks.common) + ARRAY_SIZE(mod_clks.rzg2ul),
 	.num_hw_mod_clks = R9A07G043_TSU_PCLK + 1,
 
 	/* Resets */
-	.resets = r9a07g043_resets,
-	.num_resets = R9A07G043_TSU_PRESETN + 1, /* Last reset ID + 1 */
+	.resets = resets.common,
+	ARRAY_SIZE(resets.common) + ARRAY_SIZE(resets.rzg2ul),
 };
-- 
2.25.1

Re: [RFC PATCH 3/4] clk: renesas: r9a07g043: Split up core, module and resets array

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, May 5, 2022 at 9:32 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> In preparation for adding support for Renesas RZ/Five SoC as part of
> r9a07g043-cpg.c file, split up the core clock, module clock and resets
> array into common and SoC specific.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r9a07g043-cpg.c
> +++ b/drivers/clk/renesas/r9a07g043-cpg.c
> @@ -36,9 +36,11 @@ enum clk_ids {
>         CLK_PLL3_DIV2_4_2,
>         CLK_SEL_PLL3_3,
>         CLK_DIV_PLL3_C,
> +#ifndef CONFIG_RISCV

Perhaps make this a positive check, i.e. check for CONFIG_ARM64?
Just in case Renesas spins out another variant with a
non-arm64/non-riscv core ;-)

>         CLK_PLL5,
>         CLK_PLL5_500,
>         CLK_PLL5_250,
> +#endif

Technically, this #ifdef protection is not needed, but it helps to
catch errors below.

>         CLK_PLL6,
>         CLK_PLL6_250,
>         CLK_P1_DIV2,
> @@ -76,227 +78,271 @@ static const char * const sel_pll3_3[] = { ".pll3_533", ".pll3_400" };
>  static const char * const sel_pll6_2[] = { ".pll6_250", ".pll5_250" };
>  static const char * const sel_shdi[] = { ".clk_533", ".clk_400", ".clk_266" };
>
> -static const struct cpg_core_clk r9a07g043_core_clks[] __initconst = {
> -       /* External Clock Inputs */
> -       DEF_INPUT("extal", CLK_EXTAL),
> +static const struct {
> +       struct cpg_core_clk common[38];
> +#ifndef CONFIG_RISCV
> +       struct cpg_core_clk rzg2ul[3];
> +#endif

Unlike in the r9a07g044 vs. r9a07g054 case, there is no need to have
access to the individual arrays at runtime.  So you can just keep the
single r9a07g043_core_clks[] array, and add #ifdef/#endif around the
clock definitions for clocks that are present on one variant only.

> +} core_clks __initconst = {
> +       .common = {
> +               /* External Clock Inputs */
> +               DEF_INPUT("extal", CLK_EXTAL),
>
> -       /* Internal Core Clocks */
> -       DEF_FIXED(".osc", R9A07G043_OSCCLK, CLK_EXTAL, 1, 1),
> -       DEF_FIXED(".osc_div1000", CLK_OSC_DIV1000, CLK_EXTAL, 1, 1000),
> -       DEF_SAMPLL(".pll1", CLK_PLL1, CLK_EXTAL, PLL146_CONF(0)),
> -       DEF_FIXED(".pll2", CLK_PLL2, CLK_EXTAL, 200, 3),
> -       DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 1, 2),
> -       DEF_FIXED(".clk_800", CLK_PLL2_800, CLK_PLL2, 1, 2),
> -       DEF_FIXED(".clk_533", CLK_PLL2_SDHI_533, CLK_PLL2, 1, 3),
> -       DEF_FIXED(".clk_400", CLK_PLL2_SDHI_400, CLK_PLL2_800, 1, 2),
> -       DEF_FIXED(".clk_266", CLK_PLL2_SDHI_266, CLK_PLL2_SDHI_533, 1, 2),
> -       DEF_FIXED(".pll2_div2_8", CLK_PLL2_DIV2_8, CLK_PLL2_DIV2, 1, 8),
> -       DEF_FIXED(".pll2_div2_10", CLK_PLL2_DIV2_10, CLK_PLL2_DIV2, 1, 10),
> -       DEF_FIXED(".pll3", CLK_PLL3, CLK_EXTAL, 200, 3),
> -       DEF_FIXED(".pll3_div2", CLK_PLL3_DIV2, CLK_PLL3, 1, 2),
> -       DEF_FIXED(".pll3_div2_4", CLK_PLL3_DIV2_4, CLK_PLL3_DIV2, 1, 4),
> -       DEF_FIXED(".pll3_div2_4_2", CLK_PLL3_DIV2_4_2, CLK_PLL3_DIV2_4, 1, 2),
> -       DEF_FIXED(".pll3_400", CLK_PLL3_400, CLK_PLL3, 1, 4),
> -       DEF_FIXED(".pll3_533", CLK_PLL3_533, CLK_PLL3, 1, 3),
> -       DEF_MUX(".sel_pll3_3", CLK_SEL_PLL3_3, SEL_PLL3_3,
> -               sel_pll3_3, ARRAY_SIZE(sel_pll3_3), 0, CLK_MUX_READ_ONLY),
> -       DEF_DIV("divpl3c", CLK_DIV_PLL3_C, CLK_SEL_PLL3_3,
> -               DIVPL3C, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),

I.e. add
#ifdef CONFIG_ARM64

> -       DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
> -       DEF_FIXED(".pll5_500", CLK_PLL5_500, CLK_PLL5, 1, 6),
> -       DEF_FIXED(".pll5_250", CLK_PLL5_250, CLK_PLL5_500, 1, 2),

#endif

> -       DEF_FIXED(".pll6", CLK_PLL6, CLK_EXTAL, 125, 6),
> -       DEF_FIXED(".pll6_250", CLK_PLL6_250, CLK_PLL6, 1, 2),
> +               /* Internal Core Clocks */
> +               DEF_FIXED(".osc", R9A07G043_OSCCLK, CLK_EXTAL, 1, 1),

[...]

> +               DEF_FIXED("SD1_DIV4", CLK_SD1_DIV4, R9A07G043_CLK_SD1, 1, 4),
> +       },
> +#ifndef CONFIG_RISCV
> +       .rzg2ul = {
> +               DEF_FIXED(".pll5", CLK_PLL5, CLK_EXTAL, 125, 1),
> +               DEF_FIXED(".pll5_500", CLK_PLL5_500, CLK_PLL5, 1, 6),
> +               DEF_FIXED(".pll5_250", CLK_PLL5_250, CLK_PLL5_500, 1, 2),
> +       },
> +#endif
>
> -       /* Core output clk */
> -       DEF_DIV("I", R9A07G043_CLK_I, CLK_PLL1, DIVPL1A, dtable_1_8,
> -               CLK_DIVIDER_HIWORD_MASK),
> -       DEF_DIV("P0", R9A07G043_CLK_P0, CLK_PLL2_DIV2_8, DIVPL2A,
> -               dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> -       DEF_FIXED("P0_DIV2", R9A07G043_CLK_P0_DIV2, R9A07G043_CLK_P0, 1, 2),
> -       DEF_FIXED("TSU", R9A07G043_CLK_TSU, CLK_PLL2_DIV2_10, 1, 1),
> -       DEF_DIV("P1", R9A07G043_CLK_P1, CLK_PLL3_DIV2_4,
> -               DIVPL3B, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> -       DEF_FIXED("P1_DIV2", CLK_P1_DIV2, R9A07G043_CLK_P1, 1, 2),
> -       DEF_DIV("P2", R9A07G043_CLK_P2, CLK_PLL3_DIV2_4_2,
> -               DIVPL3A, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> -       DEF_FIXED("M0", R9A07G043_CLK_M0, CLK_PLL3_DIV2_4, 1, 1),
> -       DEF_FIXED("ZT", R9A07G043_CLK_ZT, CLK_PLL3_DIV2_4_2, 1, 1),
> -       DEF_MUX("HP", R9A07G043_CLK_HP, SEL_PLL6_2,
> -               sel_pll6_2, ARRAY_SIZE(sel_pll6_2), 0, CLK_MUX_HIWORD_MASK),
> -       DEF_FIXED("SPI0", R9A07G043_CLK_SPI0, CLK_DIV_PLL3_C, 1, 2),
> -       DEF_FIXED("SPI1", R9A07G043_CLK_SPI1, CLK_DIV_PLL3_C, 1, 4),
> -       DEF_SD_MUX("SD0", R9A07G043_CLK_SD0, SEL_SDHI0,
> -                  sel_shdi, ARRAY_SIZE(sel_shdi)),
> -       DEF_SD_MUX("SD1", R9A07G043_CLK_SD1, SEL_SDHI1,
> -                  sel_shdi, ARRAY_SIZE(sel_shdi)),
> -       DEF_FIXED("SD0_DIV4", CLK_SD0_DIV4, R9A07G043_CLK_SD0, 1, 4),
> -       DEF_FIXED("SD1_DIV4", CLK_SD1_DIV4, R9A07G043_CLK_SD1, 1, 4),
>  };
>
> -static struct rzg2l_mod_clk r9a07g043_mod_clks[] = {

Likewise, you can keep r9a07g043_mod_clks[].

#ifdef CONFIG_ARM64

> -       DEF_MOD("gic",          R9A07G043_GIC600_GICCLK, R9A07G043_CLK_P1,
> -                               0x514, 0),
> -       DEF_MOD("ia55_pclk",    R9A07G043_IA55_PCLK, R9A07G043_CLK_P2,
> -                               0x518, 0),
> -       DEF_MOD("ia55_clk",     R9A07G043_IA55_CLK, R9A07G043_CLK_P1,
> -                               0x518, 1),

#endif

and add the RZ/Five-only module clocks later protected by
#ifdef CONFIG_RISCV.

> -       DEF_MOD("dmac_aclk",    R9A07G043_DMAC_ACLK, R9A07G043_CLK_P1,
> -                               0x52c, 0),

[...]

> +static const struct {
> +       struct rzg2l_mod_clk common[54];
> +#ifdef CONFIG_RISCV
> +       struct rzg2l_mod_clk rzfive[0];
> +#else
> +       struct rzg2l_mod_clk rzg2ul[3];
> +#endif
> +} mod_clks = {
> +       .common = {
> +               DEF_MOD("dmac_aclk",    R9A07G043_DMAC_ACLK, R9A07G043_CLK_P1,
> +                                       0x52c, 0),

[...]

> +               DEF_MOD("tsu_pclk",     R9A07G043_TSU_PCLK, R9A07G043_CLK_TSU,
> +                                       0x5ac, 0),
> +       },
> +#ifdef CONFIG_RISCV
> +       .rzfive = {
> +       },
> +#else
> +       .rzg2ul = {
> +               DEF_MOD("gic",          R9A07G043_GIC600_GICCLK, R9A07G043_CLK_P1,
> +                                       0x514, 0),
> +               DEF_MOD("ia55_pclk",    R9A07G043_IA55_PCLK, R9A07G043_CLK_P2,
> +                                       0x518, 0),
> +               DEF_MOD("ia55_clk",     R9A07G043_IA55_CLK, R9A07G043_CLK_P1,
> +                                       0x518, 1),
> +       },
> +#endif
>  };
>
> -static struct rzg2l_reset r9a07g043_resets[] = {

If you do the same for the resets, you can drop "[RFC PATCH 2/4]
clk: renesas: rzg2l-cpg: Add support to stack the resets instead
of indexing".

> @@ -307,8 +353,8 @@ static const unsigned int r9a07g043_crit_mod_clks[] __initconst = {
>
>  const struct rzg2l_cpg_info r9a07g043_cpg_info = {
>         /* Core Clocks */
> -       .core_clks = r9a07g043_core_clks,
> -       .num_core_clks = ARRAY_SIZE(r9a07g043_core_clks),
> +       .core_clks = core_clks.common,
> +       .num_core_clks = ARRAY_SIZE(core_clks.common) + ARRAY_SIZE(core_clks.rzg2ul),

Then this change is not needed...

>         .last_dt_core_clk = LAST_DT_CORE_CLK,
>         .num_total_core_clks = MOD_CLK_BASE,
>
> @@ -317,11 +363,11 @@ const struct rzg2l_cpg_info r9a07g043_cpg_info = {
>         .num_crit_mod_clks = ARRAY_SIZE(r9a07g043_crit_mod_clks),
>
>         /* Module Clocks */
> -       .mod_clks = r9a07g043_mod_clks,
> -       .num_mod_clks = ARRAY_SIZE(r9a07g043_mod_clks),
> +       .mod_clks = mod_clks.common,
> +       .num_mod_clks = ARRAY_SIZE(mod_clks.common) + ARRAY_SIZE(mod_clks.rzg2ul),

Same here.

#ifdef CONFIG_ARM64

>         .num_hw_mod_clks = R9A07G043_TSU_PCLK + 1,

#endif

Note that compile-testing on non-arm64/non-riscv still works fine,
as .num_hw_mod_clks = 0 is not an issue at build-time.

>
>         /* Resets */
> -       .resets = r9a07g043_resets,

#ifdef CONFIG_ARM64

> -       .num_resets = R9A07G043_TSU_PRESETN + 1, /* Last reset ID + 1 */

#endif

> +       .resets = resets.common,
> +       ARRAY_SIZE(resets.common) + ARRAY_SIZE(resets.rzg2ul),

(BTW, ".num_resets = " was lost here)

>  };

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

[RFC PATCH 4/4] clk: renesas: r9a07g043: Add support for RZ/Five SoC

[Lad Prabhakar]

Add minimal clock and resets entries required to boot RZ/Five SoC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g043-cpg.c | 30 +++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/renesas/r9a07g043-cpg.c b/drivers/clk/renesas/r9a07g043-cpg.c
index 27b47ecfe4d8..95ea639490ef 100644
--- a/drivers/clk/renesas/r9a07g043-cpg.c
+++ b/drivers/clk/renesas/r9a07g043-cpg.c
@@ -151,7 +151,7 @@ static const struct {
 static const struct {
 	struct rzg2l_mod_clk common[54];
 #ifdef CONFIG_RISCV
-	struct rzg2l_mod_clk rzfive[0];
+	struct rzg2l_mod_clk rzfive[2];
 #else
 	struct rzg2l_mod_clk rzg2ul[3];
 #endif
@@ -268,6 +268,10 @@ static const struct {
 	},
 #ifdef CONFIG_RISCV
 	.rzfive = {
+		DEF_MOD("iax45_pclk",	R9A07G043_IAX45_PCLK, R9A07G043_CLK_P2,
+					0x518, 0),
+		DEF_MOD("iax45_clk",	R9A07G043_IAX45_CLK, R9A07G043_CLK_P1,
+					0x518, 1),
 	},
 #else
 	.rzg2ul = {
@@ -284,7 +288,7 @@ static const struct {
 static const struct {
 	struct rzg2l_reset common[42];
 #ifdef CONFIG_RISCV
-	struct rzg2l_reset rzfive[0];
+	struct rzg2l_reset rzfive[1];
 #else
 	struct rzg2l_reset rzg2ul[3];
 #endif
@@ -335,6 +339,7 @@ static const struct {
 	},
 #ifdef CONFIG_RISCV
 	.rzfive = {
+		DEF_RST(R9A07G043_IAX45_RESETN, 0x818, 0),
 	},
 #else
 	.rzg2ul = {
@@ -345,16 +350,27 @@ static const struct {
 #endif
 };
 
+#ifdef CONFIG_RISCV
+static const unsigned int r9a07g043_crit_mod_clks[] __initconst = {
+	MOD_CLK_BASE + R9A07G043_IAX45_CLK,
+	MOD_CLK_BASE + R9A07G043_DMAC_ACLK,
+};
+#else
 static const unsigned int r9a07g043_crit_mod_clks[] __initconst = {
 	MOD_CLK_BASE + R9A07G043_GIC600_GICCLK,
 	MOD_CLK_BASE + R9A07G043_IA55_CLK,
 	MOD_CLK_BASE + R9A07G043_DMAC_ACLK,
 };
+#endif
 
 const struct rzg2l_cpg_info r9a07g043_cpg_info = {
 	/* Core Clocks */
 	.core_clks = core_clks.common,
+#ifdef CONFIG_RISCV
+	.num_core_clks = ARRAY_SIZE(core_clks.common),
+#else
 	.num_core_clks = ARRAY_SIZE(core_clks.common) + ARRAY_SIZE(core_clks.rzg2ul),
+#endif
 	.last_dt_core_clk = LAST_DT_CORE_CLK,
 	.num_total_core_clks = MOD_CLK_BASE,
 
@@ -364,10 +380,20 @@ const struct rzg2l_cpg_info r9a07g043_cpg_info = {
 
 	/* Module Clocks */
 	.mod_clks = mod_clks.common,
+#ifdef CONFIG_RISCV
+	.num_mod_clks = ARRAY_SIZE(mod_clks.common) + ARRAY_SIZE(mod_clks.rzfive),
+	.num_hw_mod_clks = R9A07G043_IAX45_PCLK + 1,
+#else
+
 	.num_mod_clks = ARRAY_SIZE(mod_clks.common) + ARRAY_SIZE(mod_clks.rzg2ul),
 	.num_hw_mod_clks = R9A07G043_TSU_PCLK + 1,
+#endif
 
 	/* Resets */
 	.resets = resets.common,
+#ifdef CONFIG_RISCV
+	.num_resets = ARRAY_SIZE(resets.common) + ARRAY_SIZE(resets.rzfive),
+#else
 	ARRAY_SIZE(resets.common) + ARRAY_SIZE(resets.rzg2ul),
+#endif
 };
-- 
2.25.1

Re: [RFC PATCH 4/4] clk: renesas: r9a07g043: Add support for RZ/Five SoC

[Geert Uytterhoeven]

Hi Prabhakar,

On Thu, May 5, 2022 at 9:32 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add minimal clock and resets entries required to boot RZ/Five SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

I'm only commenting on the actual clock/reset definitions, as I expect
the structure of this file to change...

> --- a/drivers/clk/renesas/r9a07g043-cpg.c
> +++ b/drivers/clk/renesas/r9a07g043-cpg.c
> @@ -151,7 +151,7 @@ static const struct {
>  static const struct {
>         struct rzg2l_mod_clk common[54];
>  #ifdef CONFIG_RISCV
> -       struct rzg2l_mod_clk rzfive[0];
> +       struct rzg2l_mod_clk rzfive[2];
>  #else
>         struct rzg2l_mod_clk rzg2ul[3];
>  #endif
> @@ -268,6 +268,10 @@ static const struct {
>         },
>  #ifdef CONFIG_RISCV
>         .rzfive = {
> +               DEF_MOD("iax45_pclk",   R9A07G043_IAX45_PCLK, R9A07G043_CLK_P2,
> +                                       0x518, 0),
> +               DEF_MOD("iax45_clk",    R9A07G043_IAX45_CLK, R9A07G043_CLK_P1,
> +                                       0x518, 1),

OK.

Note that the register offset and bits for the IAX45 block on riscv
are exactly the same as for the IA55 block on arm64.  Hence it
would be an option to use the exact same values in the DT binding
definitions for R9A07G043_IAX45_(P)CLK and R9A07G043_IA55_(P)CLK,
and avoid the #ifdefs and duplicated DEF_MOD() entries here.

I'm undecided what's the best option...

>         },
>  #else
>         .rzg2ul = {
> @@ -284,7 +288,7 @@ static const struct {
>  static const struct {
>         struct rzg2l_reset common[42];
>  #ifdef CONFIG_RISCV
> -       struct rzg2l_reset rzfive[0];
> +       struct rzg2l_reset rzfive[1];
>  #else
>         struct rzg2l_reset rzg2ul[3];
>  #endif
> @@ -335,6 +339,7 @@ static const struct {
>         },
>  #ifdef CONFIG_RISCV
>         .rzfive = {
> +               DEF_RST(R9A07G043_IAX45_RESETN, 0x818, 0),

Same for R9A07G043_IAX45_RESETN and R9A07G043_IAA55_RESETN.

>         },
>  #else
>         .rzg2ul = {
> @@ -345,16 +350,27 @@ static const struct {
>  #endif
>  };
>
> +#ifdef CONFIG_RISCV
> +static const unsigned int r9a07g043_crit_mod_clks[] __initconst = {
> +       MOD_CLK_BASE + R9A07G043_IAX45_CLK,
> +       MOD_CLK_BASE + R9A07G043_DMAC_ACLK,
> +};
> +#else
>  static const unsigned int r9a07g043_crit_mod_clks[] __initconst = {
>         MOD_CLK_BASE + R9A07G043_GIC600_GICCLK,
>         MOD_CLK_BASE + R9A07G043_IA55_CLK,
>         MOD_CLK_BASE + R9A07G043_DMAC_ACLK,
>  };
> +#endif

Please keep a single r9a07g043_crit_mod_clks[] array, and protect the
entries inside by #ifdef CONFIG_ARM64 or CONFIG_RISCV.

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