diff mbox series

[1/1] clk: renesas: Mark concerned clocks as "ignore_unused"

Message ID 20230905140008.136263-1-aymeric.aillet@iot.bzh (mailing list archive)
State Rejected
Delegated to: Geert Uytterhoeven
Headers show
Series [1/1] clk: renesas: Mark concerned clocks as "ignore_unused" | expand

Commit Message

Aymeric Aillet Sept. 5, 2023, 2 p.m. UTC
In order to avoid Linux from gating clocks that are used by
another OS running at the same time (eg. RTOS), we are adding
the "CLK_IGNORE_UNUSED" flag to the concerned clocks.

As for now, list of clocks to be flagged have been completed
depending of features that are supported by Renesas SoCs/boards
port in Zephyr RTOS.

Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>
---
 drivers/clk/renesas/r8a7795-cpg-mssr.c  | 16 ++++++++++++++++
 drivers/clk/renesas/r8a779f0-cpg-mssr.c | 10 ++++++++++
 drivers/clk/renesas/renesas-cpg-mssr.c  |  9 +++++++++
 drivers/clk/renesas/renesas-cpg-mssr.h  |  4 ++++
 4 files changed, 39 insertions(+)

Comments

Geert Uytterhoeven Sept. 7, 2023, 10:07 a.m. UTC | #1
Hi Aymeric,

On Tue, Sep 5, 2023 at 4:00 PM Aymeric Aillet <aymeric.aillet@iot.bzh> wrote:
> In order to avoid Linux from gating clocks that are used by
> another OS running at the same time (eg. RTOS), we are adding
> the "CLK_IGNORE_UNUSED" flag to the concerned clocks.
>
> As for now, list of clocks to be flagged have been completed
> depending of features that are supported by Renesas SoCs/boards
> port in Zephyr RTOS.
>
> Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>

Thanks for your patch!

> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> @@ -288,6 +288,18 @@ static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
>         DEF_MOD("scu-src0",             1031,   MOD_CLK_ID(1017)),
>  };
>
> +static const unsigned int r8a7795_ignore_unused_mod_clks[] __initconst = {
> +       MOD_CLK_ID(206),        /* SCIF1 */
> +       MOD_CLK_ID(303),        /* CMT0 */
> +       MOD_CLK_ID(310),        /* SCIF2 */
> +       MOD_CLK_ID(523),        /* PWM */
> +       MOD_CLK_ID(906),        /* GPIO6 */
> +       MOD_CLK_ID(907),        /* GPIO5 */
> +       MOD_CLK_ID(916),        /* CAN0 */
> +       MOD_CLK_ID(929),        /* I2C2 */
> +       MOD_CLK_ID(927),        /* I2C4 */

All of this is board-specific, so it cannot be handled in the SoC
clock driver.

E.g. scif2 is the main serial console for Linux on the Salvator-X(S)
and ULCB development
boards.
Pwm, gpio6, gpio5, i2c2, and i2c4 are used on Salvator-X(S).
I2c2 and i2c4 are also used on ULCB.
Scif1 and can0 are used on the KingFisher extension board for ULCB.

> --- a/drivers/clk/renesas/r8a779f0-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a779f0-cpg-mssr.c
> @@ -168,6 +168,12 @@ static const struct mssr_mod_clk r8a779f0_mod_clks[] __initconst = {
>         DEF_MOD("ufs",          1514,   R8A779F0_CLK_S0D4_HSC),
>  };
>
> +static const unsigned int r8a779f0_ignore_unused_mod_clks[] __initconst = {
> +       MOD_CLK_ID(702),        /* SCIF0 */
> +       MOD_CLK_ID(704),        /* SCIF3 */
> +       MOD_CLK_ID(915),        /* PFC0 */

E.g. scif0 is available for use by Linux on the Spider development board.

> --- a/drivers/clk/renesas/renesas-cpg-mssr.h
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
> @@ -140,6 +140,10 @@ struct cpg_mssr_info {
>         unsigned int num_mod_clks;
>         unsigned int num_hw_mod_clks;
>
> +       /* Module Clocks that should not be gated */
> +       const unsigned int *ignore_unused_mod_clks;
> +       unsigned int num_ignore_unused_mod_clks;
> +
>         /* Critical Module Clocks that should not be disabled */
>         const unsigned int *crit_mod_clks;
>         unsigned int num_crit_mod_clks;

Even if this was considered a good solution, why couldn't these be
added to the existing crit_mod_clks[] array?

Fortunately, the Renesas SoC developers thought about this use case:
R-Car H3 has both System (SMSTPCRn) and Realtime  (RMSTPCRn) Module
Stop Control Registers, and a module clock is not gated unless it is
stopped in both sets.  Linux uses the System set, while an RTOS like
Zephyr running on the Cortex-R CPU core should use the Realtime set.
Note that that mechanism does not protect against both OSes changing
e.g. a divider for a parent clock.

R-Car S4-8 only has a single set of Module Stop Control Register
(MSTPCRn), but it does support multiple domains instead, and has Domain
x Write Access Control Register (DxWACR_yyy) to control access.

Gr{oetje,eeting}s,

                        Geert
Aymeric Aillet Sept. 7, 2023, 1:24 p.m. UTC | #2
Hi Geert,
Thanks for your quick answer !
> Hi Aymeric,
>
> On Tue, Sep 5, 2023 at 4:00 PM Aymeric Aillet <aymeric.aillet@iot.bzh> wrote:
>> In order to avoid Linux from gating clocks that are used by
>> another OS running at the same time (eg. RTOS), we are adding
>> the "CLK_IGNORE_UNUSED" flag to the concerned clocks.
>>
>> As for now, list of clocks to be flagged have been completed
>> depending of features that are supported by Renesas SoCs/boards
>> port in Zephyr RTOS.
>>
>> Signed-off-by: Aymeric Aillet <aymeric.aillet@iot.bzh>
> Thanks for your patch!
>
>> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> @@ -288,6 +288,18 @@ static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
>>          DEF_MOD("scu-src0",             1031,   MOD_CLK_ID(1017)),
>>   };
>>
>> +static const unsigned int r8a7795_ignore_unused_mod_clks[] __initconst = {
>> +       MOD_CLK_ID(206),        /* SCIF1 */
>> +       MOD_CLK_ID(303),        /* CMT0 */
>> +       MOD_CLK_ID(310),        /* SCIF2 */
>> +       MOD_CLK_ID(523),        /* PWM */
>> +       MOD_CLK_ID(906),        /* GPIO6 */
>> +       MOD_CLK_ID(907),        /* GPIO5 */
>> +       MOD_CLK_ID(916),        /* CAN0 */
>> +       MOD_CLK_ID(929),        /* I2C2 */
>> +       MOD_CLK_ID(927),        /* I2C4 */
> All of this is board-specific, so it cannot be handled in the SoC
> clock driver.
>
> E.g. scif2 is the main serial console for Linux on the Salvator-X(S)
> and ULCB development
> boards.
> Pwm, gpio6, gpio5, i2c2, and i2c4 are used on Salvator-X(S).
> I2c2 and i2c4 are also used on ULCB.
> Scif1 and can0 are used on the KingFisher extension board for ULCB.

True, my cases were specific boards we are (or will) support in Zephyr OS.

I didn't though about what would happen on other boards with the same 
SoC while testing, my bad.

>
>> --- a/drivers/clk/renesas/r8a779f0-cpg-mssr.c
>> +++ b/drivers/clk/renesas/r8a779f0-cpg-mssr.c
>> @@ -168,6 +168,12 @@ static const struct mssr_mod_clk r8a779f0_mod_clks[] __initconst = {
>>          DEF_MOD("ufs",          1514,   R8A779F0_CLK_S0D4_HSC),
>>   };
>>
>> +static const unsigned int r8a779f0_ignore_unused_mod_clks[] __initconst = {
>> +       MOD_CLK_ID(702),        /* SCIF0 */
>> +       MOD_CLK_ID(704),        /* SCIF3 */
>> +       MOD_CLK_ID(915),        /* PFC0 */
> E.g. scif0 is available for use by Linux on the Spider development board.
>
>> --- a/drivers/clk/renesas/renesas-cpg-mssr.h
>> +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
>> @@ -140,6 +140,10 @@ struct cpg_mssr_info {
>>          unsigned int num_mod_clks;
>>          unsigned int num_hw_mod_clks;
>>
>> +       /* Module Clocks that should not be gated */
>> +       const unsigned int *ignore_unused_mod_clks;
>> +       unsigned int num_ignore_unused_mod_clks;
>> +
>>          /* Critical Module Clocks that should not be disabled */
>>          const unsigned int *crit_mod_clks;
>>          unsigned int num_crit_mod_clks;
> Even if this was considered a good solution, why couldn't these be
> added to the existing crit_mod_clks[] array?
When working on this I understood that critical clocks were not only 
preserved but were also managed differently at boot.
My point was only to preserve these form gating so I though that marking 
them as critical was a bit overkill.
My understanding of the difference between critical and unused clocks is 
maybe not the good one !
>
> Fortunately, the Renesas SoC developers thought about this use case:
> R-Car H3 has both System (SMSTPCRn) and Realtime  (RMSTPCRn) Module
> Stop Control Registers, and a module clock is not gated unless it is
> stopped in both sets.  Linux uses the System set, while an RTOS like
> Zephyr running on the Cortex-R CPU core should use the Realtime set.
> Note that that mechanism does not protect against both OSes changing
> e.g. a divider for a parent clock.
>
> R-Car S4-8 only has a single set of Module Stop Control Register
> (MSTPCRn), but it does support multiple domains instead, and has Domain
> x Write Access Control Register (DxWACR_yyy) to control access.
I started working on this for an S4 based board as we are not yet 
supporting multiple domain control on our Zephyr support.

Concerning Gen3 SoCs, you are right, we are already using RMSTPCRn 
module on our H3 SoC implementation so this part of my patch is useless 
anyway !

I'll keep this patch on my side while working on multiple domains 
support on our Gen4 Zephyr support.

>
> Gr{oetje,eeting}s,
>
>                          Geert

Thanks again for your answer.

Regards,

Aymeric

>
diff mbox series

Patch

diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index ad20b3301ef6..82465354b100 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -288,6 +288,18 @@  static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
 	DEF_MOD("scu-src0",		1031,	MOD_CLK_ID(1017)),
 };
 
+static const unsigned int r8a7795_ignore_unused_mod_clks[] __initconst = {
+	MOD_CLK_ID(206),	/* SCIF1 */
+	MOD_CLK_ID(303),	/* CMT0 */
+	MOD_CLK_ID(310),	/* SCIF2 */
+	MOD_CLK_ID(523),	/* PWM */
+	MOD_CLK_ID(906),	/* GPIO6 */
+	MOD_CLK_ID(907),	/* GPIO5 */
+	MOD_CLK_ID(916),	/* CAN0 */
+	MOD_CLK_ID(929),	/* I2C2 */
+	MOD_CLK_ID(927),	/* I2C4 */
+};
+
 static const unsigned int r8a7795_crit_mod_clks[] __initconst = {
 	MOD_CLK_ID(402),	/* RWDT */
 	MOD_CLK_ID(408),	/* INTC-AP (GIC) */
@@ -388,6 +400,10 @@  const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
 	.num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks),
 	.num_hw_mod_clks = 12 * 32,
 
+	/* Ignore Unused Module Clocks */
+	.ignore_unused_mod_clks = r8a7795_ignore_unused_mod_clks,
+	.num_ignore_unused_mod_clks = ARRAY_SIZE(r8a7795_ignore_unused_mod_clks),
+
 	/* Critical Module Clocks */
 	.crit_mod_clks = r8a7795_crit_mod_clks,
 	.num_crit_mod_clks = ARRAY_SIZE(r8a7795_crit_mod_clks),
diff --git a/drivers/clk/renesas/r8a779f0-cpg-mssr.c b/drivers/clk/renesas/r8a779f0-cpg-mssr.c
index f721835c7e21..ceae94c1c7dc 100644
--- a/drivers/clk/renesas/r8a779f0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a779f0-cpg-mssr.c
@@ -168,6 +168,12 @@  static const struct mssr_mod_clk r8a779f0_mod_clks[] __initconst = {
 	DEF_MOD("ufs",		1514,	R8A779F0_CLK_S0D4_HSC),
 };
 
+static const unsigned int r8a779f0_ignore_unused_mod_clks[] __initconst = {
+	MOD_CLK_ID(702),	/* SCIF0 */
+	MOD_CLK_ID(704),	/* SCIF3 */
+	MOD_CLK_ID(915),	/* PFC0 */
+};
+
 static const unsigned int r8a779f0_crit_mod_clks[] __initconst = {
 	MOD_CLK_ID(907),	/* WDT */
 };
@@ -226,6 +232,10 @@  const struct cpg_mssr_info r8a779f0_cpg_mssr_info __initconst = {
 	.num_mod_clks = ARRAY_SIZE(r8a779f0_mod_clks),
 	.num_hw_mod_clks = 28 * 32,
 
+	/* Ignore Unused Module Clocks */
+	.ignore_unused_mod_clks = r8a779f0_ignore_unused_mod_clks,
+	.num_ignore_unused_mod_clks = ARRAY_SIZE(r8a779f0_ignore_unused_mod_clks),
+
 	/* Critical Module Clocks */
 	.crit_mod_clks = r8a779f0_crit_mod_clks,
 	.num_crit_mod_clks = ARRAY_SIZE(r8a779f0_crit_mod_clks),
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index cb80d1bf6c7c..8e08a13e6904 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -444,6 +444,15 @@  static void __init cpg_mssr_register_mod_clk(const struct mssr_mod_clk *mod,
 	clock->priv = priv;
 	clock->hw.init = &init;
 
+	for (i = 0; i < info->num_ignore_unused_mod_clks; i++) {
+		if (id == info->ignore_unused_mod_clks[i]) {
+			dev_dbg(dev, "MSTP %s setting CLK_IGNORE_UNUSED\n",
+				    mod->name);
+			init.flags |= CLK_IGNORE_UNUSED;
+			break;
+		}
+	}
+
 	for (i = 0; i < info->num_crit_mod_clks; i++)
 		if (id == info->crit_mod_clks[i] &&
 		    cpg_mstp_clock_is_enabled(&clock->hw)) {
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
index 80c5b462924a..da5e999c23b0 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.h
+++ b/drivers/clk/renesas/renesas-cpg-mssr.h
@@ -140,6 +140,10 @@  struct cpg_mssr_info {
 	unsigned int num_mod_clks;
 	unsigned int num_hw_mod_clks;
 
+	/* Module Clocks that should not be gated */
+	const unsigned int *ignore_unused_mod_clks;
+	unsigned int num_ignore_unused_mod_clks;
+
 	/* Critical Module Clocks that should not be disabled */
 	const unsigned int *crit_mod_clks;
 	unsigned int num_crit_mod_clks;