2022-09-14 14:03:06

by Lad, Prabhakar

[permalink] [raw]
Subject: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop enabling wdt2

From: Lad Prabhakar <[email protected]>

WDT CH2 is specifically to check the operation of Cortex-M33 CPU so
don't enable WDT2 by default.

Signed-off-by: Lad Prabhakar <[email protected]>
---
arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi | 5 -----
1 file changed, 5 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi
index cf3b3d118ef1..2a0feb53f0dc 100644
--- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi
@@ -263,8 +263,3 @@ &wdt0 {
status = "okay";
timeout-sec = <60>;
};
-
-&wdt2 {
- status = "okay";
- timeout-sec = <60>;
-};
--
2.25.1


2022-09-18 09:15:51

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop enabling wdt2

On Wed, Sep 14, 2022 at 3:43 PM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> WDT CH2 is specifically to check the operation of Cortex-M33 CPU so
> don't enable WDT2 by default.
>
> Signed-off-by: Lad Prabhakar <[email protected]>

Reviewed-by: Geert Uytterhoeven <[email protected]>
i.e. will queue in renesas-devel for v6.1.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-09-19 10:17:34

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop enabling wdt2

Hi Geert,

Thank you for the review.

On Sun, Sep 18, 2022 at 10:04 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> On Wed, Sep 14, 2022 at 3:43 PM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > 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.

Cheers,
Prabhakar

2022-09-19 10:18:47

by Geert Uytterhoeven

[permalink] [raw]
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 <[email protected]> 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
> > <[email protected]> wrote:
> > > > From: Lad Prabhakar <[email protected]>
> > > >
> > > > 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.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2022-09-19 10:51:04

by Biju Das

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

Hi Geert,

Prabhakar Mahadev Lad <prabhakar.mahadev-
> [email protected]>
> 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 <[email protected]>
> 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
> > > <[email protected]> wrote:
> > > > > From: Lad Prabhakar <[email protected]>
> > > > >
> > > > > 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, both will do, I guess otherwise WDT enabled by Cortex-M33 will be disabled by
CORTEX-A55 during boot.

Cheers,
Biju

2022-09-19 11:12:08

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop enabling wdt2

On Mon, Sep 19, 2022 at 11:15 AM Biju Das <[email protected]> wrote:
>
> Hi Geert,
>
> Prabhakar Mahadev Lad <prabhakar.mahadev-
> > [email protected]>
> > 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 <[email protected]>
> > 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
> > > > <[email protected]> wrote:
> > > > > > From: Lad Prabhakar <[email protected]>
> > > > > >
> > > > > > 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.

> Yes, both will do, I guess otherwise WDT enabled by Cortex-M33 will be disabled by
> CORTEX-A55 during boot.
>
Agreed.

Cheers,
Prabhakar

2022-09-19 11:13:33

by Biju Das

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

Hi Prabhakar,

> -----Original Message-----
> From: Lad, Prabhakar <[email protected]>
> Sent: 19 September 2022 10:48
> To: Geert Uytterhoeven <[email protected]>
> Cc: Magnus Damm <[email protected]>; Rob Herring
> <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; linux-renesas-
> [email protected]; [email protected]; linux-
> [email protected]; Biju Das <[email protected]>;
> Prabhakar Mahadev Lad <[email protected]>
> Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> enabling wdt2
>
> Hi Geert,
>
> Thank you for the review.
>
> 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
> <[email protected]> wrote:
> > > From: Lad Prabhakar <[email protected]>
> > >
> > > 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?

Cheers,
Biju

2022-09-19 11:27:33

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop enabling wdt2

On Mon, Sep 19, 2022 at 11:55 AM Biju Das <[email protected]> wrote:
>
>
>
> > -----Original Message-----
> > From: Lad, Prabhakar <[email protected]>
> > Sent: 19 September 2022 11:44
> > To: Biju Das <[email protected]>
> > Cc: Geert Uytterhoeven <[email protected]>; Magnus Damm
> > <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> > Kozlowski <[email protected]>; linux-renesas-
> > [email protected]; [email protected]; linux-
> > [email protected]; Prabhakar Mahadev Lad <prabhakar.mahadev-
> > [email protected]>
> > Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> > enabling wdt2
> >
> > On Mon, Sep 19, 2022 at 11:15 AM Biju Das <[email protected]>
> > wrote:
> > >
> > > Hi Geert,
> > >
> > > Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > > [email protected]>
> > > > 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
> > > > <[email protected]>
> > > > 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
> > > > > > <[email protected]> wrote:
> > > > > > > > From: Lad Prabhakar
> > > > > > > > <[email protected]>
> > > > > > > >
> > > > > > > > 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?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220919&id=31989ece707adc453eb9f4f019fdac118f8fee72

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

2022-09-19 11:27:53

by Biju Das

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



> -----Original Message-----
> From: Lad, Prabhakar <[email protected]>
> Sent: 19 September 2022 11:44
> To: Biju Das <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>; Magnus Damm
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> Kozlowski <[email protected]>; linux-renesas-
> [email protected]; [email protected]; linux-
> [email protected]; Prabhakar Mahadev Lad <prabhakar.mahadev-
> [email protected]>
> Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> enabling wdt2
>
> On Mon, Sep 19, 2022 at 11:15 AM Biju Das <[email protected]>
> wrote:
> >
> > Hi Geert,
> >
> > Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > [email protected]>
> > > 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
> > > <[email protected]>
> > > 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
> > > > > <[email protected]> wrote:
> > > > > > > From: Lad Prabhakar
> > > > > > > <[email protected]>
> > > > > > >
> > > > > > > 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.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20220919&id=31989ece707adc453eb9f4f019fdac118f8fee72

>
> > Yes, both will do, I guess otherwise WDT enabled by Cortex-M33 will
> be
> > disabled by
> > CORTEX-A55 during boot.
> >
> Agreed.
>
> Cheers,
> Prabhakar

2022-09-19 11:29:35

by Biju Das

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

Hi Prabhakar,

Mahadev Lad <prabhakar.mahadev-
> [email protected]>
> Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> enabling wdt2
>
> On Mon, Sep 19, 2022 at 11:55 AM Biju Das <[email protected]>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Lad, Prabhakar <[email protected]>
> > > Sent: 19 September 2022 11:44
> > > To: Biju Das <[email protected]>
> > > Cc: Geert Uytterhoeven <[email protected]>; Magnus Damm
> > > <[email protected]>; Rob Herring <[email protected]>;
> Krzysztof
> > > Kozlowski <[email protected]>; linux-renesas-
> > > [email protected]; [email protected]; linux-
> > > [email protected]; Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > [email protected]>
> > > Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> > > enabling wdt2
> > >
> > > On Mon, Sep 19, 2022 at 11:15 AM Biju Das
> > > <[email protected]>
> > > wrote:
> > > >
> > > > Hi Geert,
> > > >
> > > > Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > > > [email protected]>
> > > > > 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
> > > > > <[email protected]>
> > > > > 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
> > > > > > > <[email protected]> wrote:
> > > > > > > > > From: Lad Prabhakar
> > > > > > > > > <[email protected]>
> > > > > > > > >
> > > > > > > > > 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

2022-09-19 12:08:38

by Biju Das

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

Hi Geert,

> Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> enabling wdt2
>
> Hi Biju,
>
> On Mon, Sep 19, 2022 at 1:10 PM Biju Das <[email protected]>
> wrote:
> > Mahadev Lad <prabhakar.mahadev-
> > > [email protected]>
> > > On Mon, Sep 19, 2022 at 11:55 AM Biju Das
> > > <[email protected]>
> > > wrote:
> > > > > -----Original Message-----
> > > > > From: Lad, Prabhakar <[email protected]>
> > > > > Sent: 19 September 2022 11:44
> > > > > To: Biju Das <[email protected]>
> > > > > Cc: Geert Uytterhoeven <[email protected]>; Magnus Damm
> > > > > <[email protected]>; Rob Herring <[email protected]>;
> > > Krzysztof
> > > > > Kozlowski <[email protected]>; linux-renesas-
> > > > > [email protected]; [email protected]; linux-
> > > > > [email protected]; Prabhakar Mahadev Lad
> > > > > <prabhakar.mahadev- [email protected]>
> > > > > Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som:
> Drop
> > > > > enabling wdt2
> > > > >
> > > > > On Mon, Sep 19, 2022 at 11:15 AM Biju Das
> > > > > <[email protected]>
> > > > > wrote:
> > > > > > Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > > > > > [email protected]>
> > > > > > > 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
> > > > > > > <[email protected]>
> > > > > > > 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
> > > > > > > > > <[email protected]> wrote:
> > > > > > > > > > > From: Lad Prabhakar
> > > > > > > > > > > <[email protected]>
> > > > > > > > > > >
> > > > > > > > > > > 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).
>
> AFAIU, the WDT cannot trigger when the clock is stopped, so RZ/G2UL
> should be fine?

If CA-33 turning on WDT clock (eg:- u-boot loading and booting CA-33) and
Later it will be disabled by CA-55 during late boot, then we will have problem.
Then WDT on CA-33 won't work.

Currently as far as I know, no one is using CA-33 with mainline code, So it is ok.

Cheers,
Biju

>
> > By adding critical clocks, it won't turn off the WDT clock.
>
> > > [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%7C55f37d25d74646611de908da9a2ecfa
> > > c%
> > >
> 7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637991823087385873%7CUn
> > > kn
> > >
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> > > wi
> > >
> LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=hP2u5NwutD8AhhrOrY6dPvlWW8P
> > > kB
> > > g3skyTMXBJQaWc%3D&amp;reserved=0
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> [email protected]
>
> 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

2022-09-19 12:12:05

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop enabling wdt2

Hi Biju,

On Mon, Sep 19, 2022 at 1:10 PM Biju Das <[email protected]> wrote:
> Mahadev Lad <prabhakar.mahadev-
> > [email protected]>
> > On Mon, Sep 19, 2022 at 11:55 AM Biju Das <[email protected]>
> > wrote:
> > > > -----Original Message-----
> > > > From: Lad, Prabhakar <[email protected]>
> > > > Sent: 19 September 2022 11:44
> > > > To: Biju Das <[email protected]>
> > > > Cc: Geert Uytterhoeven <[email protected]>; Magnus Damm
> > > > <[email protected]>; Rob Herring <[email protected]>;
> > Krzysztof
> > > > Kozlowski <[email protected]>; linux-renesas-
> > > > [email protected]; [email protected]; linux-
> > > > [email protected]; Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > > [email protected]>
> > > > Subject: Re: [PATCH] arm64: dts: renesas: rzg2ul-smarc-som: Drop
> > > > enabling wdt2
> > > >
> > > > On Mon, Sep 19, 2022 at 11:15 AM Biju Das
> > > > <[email protected]>
> > > > wrote:
> > > > > Prabhakar Mahadev Lad <prabhakar.mahadev-
> > > > > > [email protected]>
> > > > > > 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
> > > > > > <[email protected]>
> > > > > > 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
> > > > > > > > <[email protected]> wrote:
> > > > > > > > > > From: Lad Prabhakar
> > > > > > > > > > <[email protected]>
> > > > > > > > > >
> > > > > > > > > > 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).

AFAIU, the WDT cannot trigger when the clock is stopped, so RZ/G2UL
should be fine?

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

> > [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

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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