2022-09-19 13:48:29

by Lad, Prabhakar

[permalink] [raw]
Subject: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

From: Lad Prabhakar <[email protected]>

Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT CH2 is
specifically to check the operation of Cortex-M33 CPU on the
RZ/{G2L, G2LC, V2L} SoCs and we dont want to turn off the clocks of
WDT2 if it isn't enabled by Cortex-A55.

This patch is in preparation to disable WDT CH2 from the RZ/G2L
(alike SoCs) DTS/I by default.

Reported-by: Biju Das <[email protected]>
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/clk/renesas/r9a07g044-cpg.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index 02a4fc41bb6e..cf9b1bd73792 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -412,6 +412,8 @@ static const unsigned int r9a07g044_crit_mod_clks[] __initconst = {
MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
MOD_CLK_BASE + R9A07G044_IA55_CLK,
MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
+ MOD_CLK_BASE + R9A07G044_WDT2_PCLK,
+ MOD_CLK_BASE + R9A07G044_WDT2_CLK,
};

#ifdef CONFIG_CLK_R9A07G044
--
2.25.1


2022-09-19 14:12:56

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Biju,

On Mon, Sep 19, 2022 at 2:35 PM Biju Das <[email protected]> wrote:
>
> Hi Prabhakar,
>
> > Subject: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical
> > list
> >
> > From: Lad Prabhakar <[email protected]>
> >
> > Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT CH2 is
> > specifically to check the operation of Cortex-M33 CPU on the RZ/{G2L,
> > G2LC, V2L} SoCs and we dont want to turn off the clocks of
> > WDT2 if it isn't enabled by Cortex-A55.
> >
> > This patch is in preparation to disable WDT CH2 from the RZ/G2L (alike
> > SoCs) DTS/I by default.
> >
> > Reported-by: Biju Das <[email protected]>
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > drivers/clk/renesas/r9a07g044-cpg.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/clk/renesas/r9a07g044-cpg.c
> > b/drivers/clk/renesas/r9a07g044-cpg.c
> > index 02a4fc41bb6e..cf9b1bd73792 100644
> > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > @@ -412,6 +412,8 @@ static const unsigned int
> > r9a07g044_crit_mod_clks[] __initconst = {
> > MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
> > MOD_CLK_BASE + R9A07G044_IA55_CLK,
> > MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
> > + MOD_CLK_BASE + R9A07G044_WDT2_PCLK,
> > + MOD_CLK_BASE + R9A07G044_WDT2_CLK,
>
> Do we need to turn on this clock unnecessarily?
>
No, this is in preparation to disable WDT2 by default from RZ/G2L{C} DTS/I.

Cheers,
Prabhakar

2022-09-19 14:18:58

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Biju,

On Mon, Sep 19, 2022 at 2:52 PM Biju Das <[email protected]> wrote:
>
> Hi Prabhakar,
>
> > Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > critical list
> >
> > Hi Biju,
> >
> > On Mon, Sep 19, 2022 at 2:35 PM Biju Das <[email protected]>
> > wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > > Subject: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > > critical list
> > > >
> > > > From: Lad Prabhakar <[email protected]>
> > > >
> > > > Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT CH2
> > is
> > > > specifically to check the operation of Cortex-M33 CPU on the
> > > > RZ/{G2L, G2LC, V2L} SoCs and we dont want to turn off the clocks
> > of
> > > > WDT2 if it isn't enabled by Cortex-A55.
> > > >
> > > > This patch is in preparation to disable WDT CH2 from the RZ/G2L
> > > > (alike
> > > > SoCs) DTS/I by default.
> > > >
> > > > Reported-by: Biju Das <[email protected]>
> > > > Signed-off-by: Lad Prabhakar
> > > > <[email protected]>
> > > > ---
> > > > drivers/clk/renesas/r9a07g044-cpg.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/clk/renesas/r9a07g044-cpg.c
> > > > b/drivers/clk/renesas/r9a07g044-cpg.c
> > > > index 02a4fc41bb6e..cf9b1bd73792 100644
> > > > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > > > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > > > @@ -412,6 +412,8 @@ static const unsigned int
> > > > r9a07g044_crit_mod_clks[] __initconst = {
> > > > MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
> > > > MOD_CLK_BASE + R9A07G044_IA55_CLK,
> > > > MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
> > > > + MOD_CLK_BASE + R9A07G044_WDT2_PCLK,
> > > > + MOD_CLK_BASE + R9A07G044_WDT2_CLK,
> > >
> > > Do we need to turn on this clock unnecessarily?
> > >
> > No, this is in preparation to disable WDT2 by default from RZ/G2L{C}
> > DTS/I.
>
> But that will make WDT2 device is not enabled, but unnecessarily the clk is on.
>
Agreed the clocks will be ON, but didnt we agree earlier for r9a07g043-cpg.c?

> Not sure, If we have any use case for wdt2 controlling from CA-55??
> If there is no use case, why can't we remove it from SoC dtsi and clock tables,
> that why we don't unnecessarily use this clock anymore??
>
I dont have a clear use case of using WDT ch2 when trying to use it
from CA-55 either.

Cheers,
Prabhakar

2022-09-19 14:21:48

by Biju Das

[permalink] [raw]
Subject: RE: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Prabhakar,

> Subject: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical
> list
>
> From: Lad Prabhakar <[email protected]>
>
> Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT CH2 is
> specifically to check the operation of Cortex-M33 CPU on the RZ/{G2L,
> G2LC, V2L} SoCs and we dont want to turn off the clocks of
> WDT2 if it isn't enabled by Cortex-A55.
>
> This patch is in preparation to disable WDT CH2 from the RZ/G2L (alike
> SoCs) DTS/I by default.
>
> Reported-by: Biju Das <[email protected]>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> drivers/clk/renesas/r9a07g044-cpg.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/renesas/r9a07g044-cpg.c
> b/drivers/clk/renesas/r9a07g044-cpg.c
> index 02a4fc41bb6e..cf9b1bd73792 100644
> --- a/drivers/clk/renesas/r9a07g044-cpg.c
> +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> @@ -412,6 +412,8 @@ static const unsigned int
> r9a07g044_crit_mod_clks[] __initconst = {
> MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
> MOD_CLK_BASE + R9A07G044_IA55_CLK,
> MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
> + MOD_CLK_BASE + R9A07G044_WDT2_PCLK,
> + MOD_CLK_BASE + R9A07G044_WDT2_CLK,

Do we need to turn on this clock unnecessarily?

Cheers,
Biju

2022-09-19 14:25:13

by Biju Das

[permalink] [raw]
Subject: RE: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Prabhakar,

> Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> critical list
>
> Hi Biju,
>
> On Mon, Sep 19, 2022 at 2:35 PM Biju Das <[email protected]>
> wrote:
> >
> > Hi Prabhakar,
> >
> > > Subject: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > critical list
> > >
> > > From: Lad Prabhakar <[email protected]>
> > >
> > > Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT CH2
> is
> > > specifically to check the operation of Cortex-M33 CPU on the
> > > RZ/{G2L, G2LC, V2L} SoCs and we dont want to turn off the clocks
> of
> > > WDT2 if it isn't enabled by Cortex-A55.
> > >
> > > This patch is in preparation to disable WDT CH2 from the RZ/G2L
> > > (alike
> > > SoCs) DTS/I by default.
> > >
> > > Reported-by: Biju Das <[email protected]>
> > > Signed-off-by: Lad Prabhakar
> > > <[email protected]>
> > > ---
> > > drivers/clk/renesas/r9a07g044-cpg.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/clk/renesas/r9a07g044-cpg.c
> > > b/drivers/clk/renesas/r9a07g044-cpg.c
> > > index 02a4fc41bb6e..cf9b1bd73792 100644
> > > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > > @@ -412,6 +412,8 @@ static const unsigned int
> > > r9a07g044_crit_mod_clks[] __initconst = {
> > > MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
> > > MOD_CLK_BASE + R9A07G044_IA55_CLK,
> > > MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
> > > + MOD_CLK_BASE + R9A07G044_WDT2_PCLK,
> > > + MOD_CLK_BASE + R9A07G044_WDT2_CLK,
> >
> > Do we need to turn on this clock unnecessarily?
> >
> No, this is in preparation to disable WDT2 by default from RZ/G2L{C}
> DTS/I.

But that will make WDT2 device is not enabled, but unnecessarily the clk is on.

Not sure, If we have any use case for wdt2 controlling from CA-55??
If there is no use case, why can't we remove it from SoC dtsi and clock tables,
that why we don't unnecessarily use this clock anymore??

Cheers,
Biju


2022-09-19 14:50:21

by Biju Das

[permalink] [raw]
Subject: RE: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Prabhakar,

> Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> critical list
>
> Hi Biju,
>
> On Mon, Sep 19, 2022 at 2:52 PM Biju Das <[email protected]>
> wrote:
> >
> > Hi Prabhakar,
> >
> > > Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > critical list
> > >
> > > Hi Biju,
> > >
> > > On Mon, Sep 19, 2022 at 2:35 PM Biju Das
> > > <[email protected]>
> > > wrote:
> > > >
> > > > Hi Prabhakar,
> > > >
> > > > > Subject: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > > > critical list
> > > > >
> > > > > From: Lad Prabhakar <[email protected]>
> > > > >
> > > > > Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT
> CH2
> > > is
> > > > > specifically to check the operation of Cortex-M33 CPU on the
> > > > > RZ/{G2L, G2LC, V2L} SoCs and we dont want to turn off the
> clocks
> > > of
> > > > > WDT2 if it isn't enabled by Cortex-A55.
> > > > >
> > > > > This patch is in preparation to disable WDT CH2 from the
> RZ/G2L
> > > > > (alike
> > > > > SoCs) DTS/I by default.
> > > > >
> > > > > Reported-by: Biju Das <[email protected]>
> > > > > Signed-off-by: Lad Prabhakar
> > > > > <[email protected]>
> > > > > ---
> > > > > drivers/clk/renesas/r9a07g044-cpg.c | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > b/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > index 02a4fc41bb6e..cf9b1bd73792 100644
> > > > > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > @@ -412,6 +412,8 @@ static const unsigned int
> > > > > r9a07g044_crit_mod_clks[] __initconst = {
> > > > > MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
> > > > > MOD_CLK_BASE + R9A07G044_IA55_CLK,
> > > > > MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
> > > > > + MOD_CLK_BASE + R9A07G044_WDT2_PCLK,
> > > > > + MOD_CLK_BASE + R9A07G044_WDT2_CLK,
> > > >
> > > > Do we need to turn on this clock unnecessarily?
> > > >
> > > No, this is in preparation to disable WDT2 by default from
> RZ/G2L{C}
> > > DTS/I.
> >
> > But that will make WDT2 device is not enabled, but unnecessarily the
> clk is on.
> >
> Agreed the clocks will be ON, but didnt we agree earlier for
> r9a07g043-cpg.c?

Yep, still we have a chance to conclude, whether we need to make
this clk always on, if it is not enabled and there is
no use case for wdt2 controlling from CA-55??

Cheers,
Biju

>
> > Not sure, If we have any use case for wdt2 controlling from CA-55??
> > If there is no use case, why can't we remove it from SoC dtsi and
> > clock tables, that why we don't unnecessarily use this clock
> anymore??
> >
> I dont have a clear use case of using WDT ch2 when trying to use it
> from CA-55 either.
>
> Cheers,
> Prabhakar

2022-10-05 08:32:06

by Biju Das

[permalink] [raw]
Subject: RE: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Geert and Prabhakar,

> Subject: RE: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> critical list
>
> Hi Prabhakar,
>
> > Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > critical list
> >
> > Hi Biju,
> >
> > On Mon, Sep 19, 2022 at 2:52 PM Biju Das
> <[email protected]>
> > wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > > Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > > critical list
> > > >
> > > > Hi Biju,
> > > >
> > > > On Mon, Sep 19, 2022 at 2:35 PM Biju Das
> > > > <[email protected]>
> > > > wrote:
> > > > >
> > > > > Hi Prabhakar,
> > > > >
> > > > > > Subject: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > > > > critical list
> > > > > >
> > > > > > From: Lad Prabhakar <prabhakar.mahadev-
> [email protected]>
> > > > > >
> > > > > > Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT
> > CH2
> > > > is
> > > > > > specifically to check the operation of Cortex-M33 CPU on the
> > > > > > RZ/{G2L, G2LC, V2L} SoCs and we dont want to turn off the
> > clocks
> > > > of
> > > > > > WDT2 if it isn't enabled by Cortex-A55.
> > > > > >
> > > > > > This patch is in preparation to disable WDT CH2 from the
> > RZ/G2L
> > > > > > (alike
> > > > > > SoCs) DTS/I by default.
> > > > > >
> > > > > > Reported-by: Biju Das <[email protected]>
> > > > > > Signed-off-by: Lad Prabhakar
> > > > > > <[email protected]>
> > > > > > ---
> > > > > > drivers/clk/renesas/r9a07g044-cpg.c | 2 ++
> > > > > > 1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > > b/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > > index 02a4fc41bb6e..cf9b1bd73792 100644
> > > > > > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > > @@ -412,6 +412,8 @@ static const unsigned int
> > > > > > r9a07g044_crit_mod_clks[] __initconst = {
> > > > > > MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
> > > > > > MOD_CLK_BASE + R9A07G044_IA55_CLK,
> > > > > > MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
> > > > > > + MOD_CLK_BASE + R9A07G044_WDT2_PCLK,
> > > > > > + MOD_CLK_BASE + R9A07G044_WDT2_CLK,
> > > > >
> > > > > Do we need to turn on this clock unnecessarily?
> > > > >
> > > > No, this is in preparation to disable WDT2 by default from
> > RZ/G2L{C}
> > > > DTS/I.
> > >
> > > But that will make WDT2 device is not enabled, but unnecessarily
> the
> > clk is on.
> > >
> > Agreed the clocks will be ON, but didnt we agree earlier for
> > r9a07g043-cpg.c?
>
> Yep, still we have a chance to conclude, whether we need to make this
> clk always on, if it is not enabled and there is no use case for wdt2
> controlling from CA-55??
>

I got confirmation that that using WDT2 from CA55 is prohibited.
WDT2 is only for CM33.

With CPG register, we can select whether CM33 to trigger CM33 cpu reset, or trigger system reset
when WDT2 overflows.

If WDT2 is used by CA55, it may result in unexpected behaviour.

So we may need to take WDT2 entries from binding + dtsi + clock table??

Or

Added it to critical clock list, to avoid changes in binding + dtsi + clock table
at the expense of turning on the WDT2 clk unnecessarily.

Please share your views.

Cheers,
Biju

2022-10-05 13:04:58

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Biju,

On Wed, Oct 5, 2022 at 9:27 AM Biju Das <[email protected]> wrote:
>
> Hi Geert and Prabhakar,
>
> > Subject: RE: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > critical list
> >
> > Hi Prabhakar,
> >
> > > Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > critical list
> > >
> > > Hi Biju,
> > >
> > > On Mon, Sep 19, 2022 at 2:52 PM Biju Das
> > <[email protected]>
> > > wrote:
> > > >
> > > > Hi Prabhakar,
> > > >
> > > > > Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > > > critical list
> > > > >
> > > > > Hi Biju,
> > > > >
> > > > > On Mon, Sep 19, 2022 at 2:35 PM Biju Das
> > > > > <[email protected]>
> > > > > wrote:
> > > > > >
> > > > > > Hi Prabhakar,
> > > > > >
> > > > > > > Subject: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to
> > > > > > > critical list
> > > > > > >
> > > > > > > From: Lad Prabhakar <prabhakar.mahadev-
> > [email protected]>
> > > > > > >
> > > > > > > Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT
> > > CH2
> > > > > is
> > > > > > > specifically to check the operation of Cortex-M33 CPU on the
> > > > > > > RZ/{G2L, G2LC, V2L} SoCs and we dont want to turn off the
> > > clocks
> > > > > of
> > > > > > > WDT2 if it isn't enabled by Cortex-A55.
> > > > > > >
> > > > > > > This patch is in preparation to disable WDT CH2 from the
> > > RZ/G2L
> > > > > > > (alike
> > > > > > > SoCs) DTS/I by default.
> > > > > > >
> > > > > > > Reported-by: Biju Das <[email protected]>
> > > > > > > Signed-off-by: Lad Prabhakar
> > > > > > > <[email protected]>
> > > > > > > ---
> > > > > > > drivers/clk/renesas/r9a07g044-cpg.c | 2 ++
> > > > > > > 1 file changed, 2 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > > > b/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > > > index 02a4fc41bb6e..cf9b1bd73792 100644
> > > > > > > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > > > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > > > > > > @@ -412,6 +412,8 @@ static const unsigned int
> > > > > > > r9a07g044_crit_mod_clks[] __initconst = {
> > > > > > > MOD_CLK_BASE + R9A07G044_GIC600_GICCLK,
> > > > > > > MOD_CLK_BASE + R9A07G044_IA55_CLK,
> > > > > > > MOD_CLK_BASE + R9A07G044_DMAC_ACLK,
> > > > > > > + MOD_CLK_BASE + R9A07G044_WDT2_PCLK,
> > > > > > > + MOD_CLK_BASE + R9A07G044_WDT2_CLK,
> > > > > >
> > > > > > Do we need to turn on this clock unnecessarily?
> > > > > >
> > > > > No, this is in preparation to disable WDT2 by default from
> > > RZ/G2L{C}
> > > > > DTS/I.
> > > >
> > > > But that will make WDT2 device is not enabled, but unnecessarily
> > the
> > > clk is on.
> > > >
> > > Agreed the clocks will be ON, but didnt we agree earlier for
> > > r9a07g043-cpg.c?
> >
> > Yep, still we have a chance to conclude, whether we need to make this
> > clk always on, if it is not enabled and there is no use case for wdt2
> > controlling from CA-55??
> >
>
> I got confirmation that that using WDT2 from CA55 is prohibited.
> WDT2 is only for CM33.
>
> With CPG register, we can select whether CM33 to trigger CM33 cpu reset, or trigger system reset
> when WDT2 overflows.
>
> If WDT2 is used by CA55, it may result in unexpected behaviour.
>
Thanks.

> So we may need to take WDT2 entries from binding + dtsi + clock table??
>
> Or
>
> Added it to critical clock list, to avoid changes in binding + dtsi + clock table
> at the expense of turning on the WDT2 clk unnecessarily.
>
I'm in favour of option#1 except that we keep WDT2 entries in binding.
Said that I'll leave Geert to decide on this.

Cheers,
Prabhakar

2022-10-06 08:24:59

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Prabhakar, Biju,

On Wed, Oct 5, 2022 at 2:56 PM Lad, Prabhakar
<[email protected]> wrote:
> On Wed, Oct 5, 2022 at 9:27 AM Biju Das <[email protected]> wrote:
> > > > On Mon, Sep 19, 2022 at 2:52 PM Biju Das <[email protected]> wrote:
> > > > > > On Mon, Sep 19, 2022 at 2:35 PM Biju Das <[email protected]> wrote:
> > > > > > > > From: Lad Prabhakar <prabhakar.mahadev-
> > > [email protected]>
> > > > > > > > Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT CH2 is
> > > > > > > > specifically to check the operation of Cortex-M33 CPU on the
> > > > > > > > RZ/{G2L, G2LC, V2L} SoCs and we dont want to turn off the clocks of
> > > > > > > > WDT2 if it isn't enabled by Cortex-A55.
> > > > > > > >
> > > > > > > > This patch is in preparation to disable WDT CH2 from the RZ/G2L
> > > > > > > > (alike
> > > > > > > > SoCs) DTS/I by default.
> > > > > > > >
> > > > > > > > Reported-by: Biju Das <[email protected]>
> > > > > > > > Signed-off-by: Lad Prabhakar <[email protected]>
> >
> > I got confirmation that that using WDT2 from CA55 is prohibited.
> > WDT2 is only for CM33.
> >
> > With CPG register, we can select whether CM33 to trigger CM33 cpu reset, or trigger system reset
> > when WDT2 overflows.
> >
> > If WDT2 is used by CA55, it may result in unexpected behaviour.
> >
> Thanks.
>
> > So we may need to take WDT2 entries from binding + dtsi + clock table??
> >
> > Or
> >
> > Added it to critical clock list, to avoid changes in binding + dtsi + clock table
> > at the expense of turning on the WDT2 clk unnecessarily.

... plus still risking to interfere with the CM33.

> I'm in favour of option#1 except that we keep WDT2 entries in binding.

Agreed (bindings are append-only).

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-10-06 09:24:18

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] clk: renesas: r9a07g044: Add WDT2 clocks to critical list

Hi Geert,

On Thu, Oct 6, 2022 at 9:16 AM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar, Biju,
>
> On Wed, Oct 5, 2022 at 2:56 PM Lad, Prabhakar
> <[email protected]> wrote:
> > On Wed, Oct 5, 2022 at 9:27 AM Biju Das <[email protected]> wrote:
> > > > > On Mon, Sep 19, 2022 at 2:52 PM Biju Das <[email protected]> wrote:
> > > > > > > On Mon, Sep 19, 2022 at 2:35 PM Biju Das <[email protected]> wrote:
> > > > > > > > > From: Lad Prabhakar <prabhakar.mahadev-
> > > > [email protected]>
> > > > > > > > > Add the WDT2 clocks to r9a07g044_crit_mod_clks[] list as WDT CH2 is
> > > > > > > > > specifically to check the operation of Cortex-M33 CPU on the
> > > > > > > > > RZ/{G2L, G2LC, V2L} SoCs and we dont want to turn off the clocks of
> > > > > > > > > WDT2 if it isn't enabled by Cortex-A55.
> > > > > > > > >
> > > > > > > > > This patch is in preparation to disable WDT CH2 from the RZ/G2L
> > > > > > > > > (alike
> > > > > > > > > SoCs) DTS/I by default.
> > > > > > > > >
> > > > > > > > > Reported-by: Biju Das <[email protected]>
> > > > > > > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > >
> > > I got confirmation that that using WDT2 from CA55 is prohibited.
> > > WDT2 is only for CM33.
> > >
> > > With CPG register, we can select whether CM33 to trigger CM33 cpu reset, or trigger system reset
> > > when WDT2 overflows.
> > >
> > > If WDT2 is used by CA55, it may result in unexpected behaviour.
> > >
> > Thanks.
> >
> > > So we may need to take WDT2 entries from binding + dtsi + clock table??
> > >
> > > Or
> > >
> > > Added it to critical clock list, to avoid changes in binding + dtsi + clock table
> > > at the expense of turning on the WDT2 clk unnecessarily.
>
> ... plus still risking to interfere with the CM33.
>
> > I'm in favour of option#1 except that we keep WDT2 entries in binding.
>
> Agreed (bindings are append-only).
>
I'll get the patches out to get rid of WDT2.

Cheers,
Prabhakar