RE: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop enabling wdt2

[Biju Das]

Hi Prabhakar,

Mahadev Lad <prabhakar.mahadev-
> lad.rj@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> enabling wdt2
> 
> On Mon, Sep 19, 2022 at 11:55 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx>
> > > Sent: 19 September 2022 11:44
> > > To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>; Magnus Damm
> > > <magnus.damm@xxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>;
> Krzysztof
> > > Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>; linux-renesas-
> > > soc@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> > > kernel@xxxxxxxxxxxxxxx; Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > lad.rj@xxxxxxxxxxxxxx>
> > > Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> > > enabling wdt2
> > >
> > > On Mon, Sep 19, 2022 at 11:15 AM Biju Das
> > > <biju.das.jz@xxxxxxxxxxxxxx>
> > > wrote:
> > > >
> > > > Hi Geert,
> > > >
> > > > Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > > > lad.rj@xxxxxxxxxxxxxx>
> > > > > Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som:
> Drop
> > > > > enabling wdt2
> > > > >
> > > > > Hi Biju,
> > > > >
> > > > > On Mon, Sep 19, 2022 at 12:06 PM Biju Das
> > > > > <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > wrote:
> > > > > > > On Sun, Sep 18, 2022 at 10:04 AM Geert Uytterhoeven
> > > > > > > <geert@linux- m68k.org> wrote:
> > > > > > > > On Wed, Sep 14, 2022 at 3:43 PM Prabhakar
> > > > > > > <prabhakar.csengg@xxxxxxxxx> wrote:
> > > > > > > > > From: Lad Prabhakar
> > > > > > > > > <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > > > > > > >
> > > > > > > > > WDT CH2 is specifically to check the operation of
> > > > > > > > > Cortex-
> > > M33
> > > > > CPU
> > > > > > > so
> > > > > > > > > don't enable WDT2 by default.
> > > > > > > > >
> > > > > > > For consistency I'll drop enabling WDT2 from the RZ/G2L{C}
> > > too.
> > > > > >
> > > > > > Does remove unused clocks turn off WDT2 clock during late
> init??
> > > > > >
> > > > > > If that is the case, we should remove entries from clock
> table
> > > as
> > > > > well?
> > > > >
> > > > > Or add it to *_crit_mod_clks[]?
> > > > >
> > > > > Note that drivers/clk/renesas/r9a07g043-cpg.c creates wdt2
> > > > > clocks and resets, so that should probably be moved inside the
> > > > > #ifdef
> > > > > CONFIG_ARM64 section.
> > > >
> > > Yes we dont want it to be listed on RZ/Five.
> >
> > OK, So this patch [1] will break existing RZ/G2UL till we have clk
> changes suggested by Geert.
> >
> No it won't, patch [1] just disables enabling wdt2 on A55. Whereas
> Geert is suggesting adding wdt2 clk R9A07G043_WDT2_PCLK (and also
> R9A07G043_WDT2_CLK?)  to crit_mod_clks so that we dont disable them in
> A55 (currently we dont have any scenario where M33 is up with WDT2 and
> patch [1] is breaking it). Also Geert's other suggestion was to add
> the WDT2 clocks under ARM64 as we dont want them to be visible under
> RZ/Five (where wdt2 is not present).
> 
> Or is it that I am missing something?

It all depends upon whether remove unused clocks turn off WDT2 clock during
late boot process. 

I guess, since we have wdt2 entries in clock table, it will turn off WDT clock,
since you have disable it from DT( it becomes unused).

By adding critical clocks, it won't turn off the WDT clock.

Cheers,
Biju

> 
> [1]
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-
> next.git%2Fcommit%2F%3Fh%3Dnext-
> 20220919%26id%3D31989ece707adc453eb9f4f019fdac118f8fee72&amp;data=05%7
> C01%7Cbiju.das.jz%40bp.renesas.com%7C55f37d25d74646611de908da9a2ecfac%
> 7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637991823087385873%7CUnkn
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=hP2u5NwutD8AhhrOrY6dPvlWW8PkB
> g3skyTMXBJQaWc%3D&amp;reserved=0
> 
> Cheers,
> Prabhakar
> >
> > >
> > > > Yes, both will do, I guess otherwise WDT enabled by Cortex-M33
> > > > will
> > > be
> > > > disabled by
> > > > CORTEX-A55 during boot.
> > > >
> > > Agreed.
> > >
> > > Cheers,
> > > Prabhakar