RE: [PATCH v5] clk: renesas: rzg2l-cpg: Add support for RZ/V2L SoC

Biju Das <biju.das.jz@xxxxxxxxxxxxxx> · Mon, 7 Feb 2022 14:35:02 +0000

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v5] clk: renesas: rzg2l-cpg: Add support for RZ/V2L
> SoC
> 
> Hi Biju,
> 
> On Sat, Feb 5, 2022 at 9:40 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > The clock structure for RZ/V2L is almost identical to RZ/G2L SoC. The
> > only difference being RZ/V2L has an additional registers to control
> > clock and reset for the DRP-AI block.
> >
> > This patch reuses r9a07g044-cpg.c, as the common clock IDS and rest
> > IDS are same between RZ/G2L and RZ/V2L and adding a separate
> > r9a07g054_cpg_info to take care of DRP-AI clocks/resets.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> > This patch depend upon [1]
> > [1]
> > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> > kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-next.git%
> > 2Fcommit%2Finclude%2Fdt-bindings%2Fclock%3Fh%3Dnext-20220204%26id%3D4d
> > ecd2e54b61686787f36b727d2772e067a46ea5&amp;data=04%7C01%7Cbiju.das.jz%
> > 40bp.renesas.com%7C820d967550ea4eedc6d108d9ea457c54%7C53d82571da1947e4
> > 9cb4625a166a4a2a%7C0%7C0%7C637798406429941649%7CUnknown%7CTWFpbGZsb3d8
> > eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> > 000&amp;sdata=HMut3PBVm4oGRV2yFbPvKhWecUvz2yIUOuqZSktoJwE%3D&amp;reser
> > ved=0
> > v4->v5:
> > * Rebased on latest clk-driver
> > * Added base commit ID
> > v3->v4:
> > * Added common/drp split for core clocks and resets
> 
> Thanks for the updates!
> 
> > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > @@ -11,12 +11,13 @@
> >  #include <linux/kernel.h>
> >
> >  #include <dt-bindings/clock/r9a07g044-cpg.h>
> > +#include <dt-bindings/clock/r9a07g054-cpg.h>
> >
> >  #include "rzg2l-cpg.h"
> >
> >  enum clk_ids {
> >         /* Core Clock Outputs exported to DT */
> > -       LAST_DT_CORE_CLK = R9A07G044_CLK_P0_DIV2,
> > +       LAST_DT_CORE_CLK = R9A07G054_CLK_DRP_A,
> >
> >         /* External Input Clocks */
> >         CLK_EXTAL,
> > @@ -80,252 +81,285 @@ 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 char * const sel_gpu2[] = { ".pll6",
> > ".pll3_div2_2" };
> >
> > -static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
> > -       /* External Clock Inputs */
> > -       DEF_INPUT("extal", CLK_EXTAL),
> > +static struct {
> > +       const struct cpg_core_clk common[44]; #ifdef
> > +CONFIG_CLK_R9A07G054
> > +       const struct cpg_core_clk drp[0]; #endif } const core_clks
> > +__initconst = {
> 
> Having a single "const" between "static" and "struct" is sufficient.

I got a check patch error "ERROR: Use of const init definition must use __initconst"
Which is fixed by "const core_clks __initconst". Looks like checkpatch needs this in one line.

Regards,
Biju

> 
> > +       .common = {
> > +               /* External Clock Inputs */
> > +               DEF_INPUT("extal", CLK_EXTAL),
> >
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> i.e. will queue
> in renesas-clk-for-v5.18, with the above fixed.