* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-29 9:27 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 9:27 UTC (permalink / raw)
To: Dirk Behme
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
Hi Dirk,
CC clock, ARM, DT, PM people
TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
disables them as being unused.
On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> CLK_IS_CRITICAL") we are able to handle critical module clocks.
> Introduce the same logic for critical core clocks.
>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
> Commit
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>
> is quite nice to avoid *module* clocks being disabled. Unfortunately,
> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> core of the r8a7795, the 'canfd' is a quite popular core clock which
> shouldn't be disabled by Linux.
>
> Therefore, this patch is a proposal to use the same 'mark clocks as
> critical' logic implemented for the module clocks for the core
> clocks, too.
>
> Opinions?
On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
core which may run another OS.
This is an interesting issue, and relevant to other SoCs, too.
In this particular case, the "canfd" clock is a core clock used as an
auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
to three scenarios:
1. Linux controls all CAN interfaces
=> no issue,
2. The OS on the RT CPU controls all CAN interfaces
=> issue, Linux disables the clock
3. Mix of 1 and 2
=> More issues.
Of course this is not limited to clocks, but also to e.g. PM domains.
How can this be handled?
I believe just marking the "canfd" clock critical is not the right solution,
as about any clock could be used by the RT CPU.
Still, Linux needs to be made aware that devices (clocks and PM domains) are
controlled by another CPU/OS.
Should this be described in DT? It feels like software policy to me.
Note that we (mainline) currently don't describe the Cortex R7 core in DT.
Dirk: do you describe it?
Summary:
1. Core/module clocks are described in the clock driver (not in DT),
2. Unused clocks are disabled by CCF,
3. Clocks may be in use by the Real-Time CPU core, running another OS,
4. How to communicate to Linux which clocks are under control of the RT CPU?
Thanks for your comments!
> drivers/clk/renesas/clk-div6.c | 17 +++++++++++++++--
> drivers/clk/renesas/clk-div6.h | 4 +++-
> drivers/clk/renesas/r8a7795-cpg-mssr.c | 7 +++++++
> drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
> drivers/clk/renesas/renesas-cpg-mssr.h | 8 ++++++++
> 5 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/renesas/clk-div6.c b/drivers/clk/renesas/clk-div6.c
> index 0627860..5917e05 100644
> --- a/drivers/clk/renesas/clk-div6.c
> +++ b/drivers/clk/renesas/clk-div6.c
> @@ -18,6 +18,7 @@
> #include <linux/of_address.h>
> #include <linux/slab.h>
>
> +#include "renesas-cpg-mssr.h"
> #include "clk-div6.h"
>
> #define CPG_DIV6_CKSTP BIT(8)
> @@ -184,7 +185,9 @@ static const struct clk_ops cpg_div6_clock_ops = {
> struct clk * __init cpg_div6_register(const char *name,
> unsigned int num_parents,
> const char **parent_names,
> - void __iomem *reg)
> + void __iomem *reg,
> + const struct cpg_mssr_info *info,
> + unsigned int id)
> {
> unsigned int valid_parents;
> struct clk_init_data init;
> @@ -246,6 +249,15 @@ struct clk * __init cpg_div6_register(const char *name,
> init.name = name;
> init.ops = &cpg_div6_clock_ops;
> init.flags = CLK_IS_BASIC;
> + if (info) {
> + for (i = 0; i < info->num_crit_core_clks; i++)
> + if (id == info->crit_core_clks[i]) {
> + pr_devel("DIV6 %s setting CLK_IS_CRITICAL\n",
> + name);
> + init.flags |= CLK_IS_CRITICAL;
> + break;
> + }
> + }
> init.parent_names = parent_names;
> init.num_parents = valid_parents;
>
> @@ -298,7 +310,8 @@ static void __init cpg_div6_clock_init(struct device_node *np)
> for (i = 0; i < num_parents; i++)
> parent_names[i] = of_clk_get_parent_name(np, i);
>
> - clk = cpg_div6_register(clk_name, num_parents, parent_names, reg);
> + clk = cpg_div6_register(clk_name, num_parents, parent_names, reg,
> + NULL, 0);
> if (IS_ERR(clk)) {
> pr_err("%s: failed to register %s DIV6 clock (%ld)\n",
> __func__, np->name, PTR_ERR(clk));
> diff --git a/drivers/clk/renesas/clk-div6.h b/drivers/clk/renesas/clk-div6.h
> index 567b31d..b619d6b4 100644
> --- a/drivers/clk/renesas/clk-div6.h
> +++ b/drivers/clk/renesas/clk-div6.h
> @@ -2,6 +2,8 @@
> #define __RENESAS_CLK_DIV6_H__
>
> struct clk *cpg_div6_register(const char *name, unsigned int num_parents,
> - const char **parent_names, void __iomem *reg);
> + const char **parent_names, void __iomem *reg,
> + const struct cpg_mssr_info *info,
> + unsigned int id);
>
> #endif
> diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> index eaa98b4..a54fed6 100644
> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> @@ -114,6 +114,9 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
> DEF_BASE("r", R8A7795_CLK_R, CLK_TYPE_GEN3_R, CLK_RINT),
> };
>
> +static const unsigned int r8a7795_crit_core_clks[] __initconst = {
> +};
> +
> static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
> DEF_MOD("fdp1-2", 117, R8A7795_CLK_S2D1), /* ES1.x */
> DEF_MOD("fdp1-1", 118, R8A7795_CLK_S0D1),
> @@ -441,6 +444,10 @@ const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
> .last_dt_core_clk = LAST_DT_CORE_CLK,
> .num_total_core_clks = MOD_CLK_BASE,
>
> + /* Critical Core Clocks */
> + .crit_core_clks = r8a7795_crit_core_clks,
> + .num_crit_core_clks = ARRAY_SIZE(r8a7795_crit_core_clks),
> +
> /* Module Clocks */
> .mod_clks = r8a7795_mod_clks,
> .num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks),
> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> index 99eeec6..80be019 100644
> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> @@ -293,7 +293,8 @@ static void __init cpg_mssr_register_core_clk(const struct cpg_core_clk *core,
>
> if (core->type == CLK_TYPE_DIV6P1) {
> clk = cpg_div6_register(core->name, 1, &parent_name,
> - priv->base + core->offset);
> + priv->base + core->offset, info,
> + core->id);
> } else {
> clk = clk_register_fixed_factor(NULL, core->name,
> parent_name, 0,
> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
> index 148f4f0a..a723fdd 100644
> --- a/drivers/clk/renesas/renesas-cpg-mssr.h
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
> @@ -86,6 +86,10 @@ struct device_node;
> * @last_dt_core_clk: ID of the last Core Clock exported to DT
> * @num_total_core_clks: Total number of Core Clocks (exported + internal)
> *
> + * @crit_core_clks: Array with Core Clock IDs of critical clocks that
> + * should not be disabled without a knowledgeable driver
> + * @num_core_mod_clks: Number of entries in crit_core_clks[]
> + *
> * @mod_clks: Array of Module Clock definitions
> * @num_mod_clks: Number of entries in mod_clks[]
> * @num_hw_mod_clks: Number of Module Clocks supported by the hardware
> @@ -109,6 +113,10 @@ struct cpg_mssr_info {
> unsigned int last_dt_core_clk;
> unsigned int num_total_core_clks;
>
> + /* Critical Core Clocks that should not be disabled */
> + const unsigned int *crit_core_clks;
> + unsigned int num_crit_core_clks;
> +
> /* Module Clocks */
> const struct mssr_mod_clk *mod_clks;
> unsigned int num_mod_clks;
> --
> 2.8.0
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-29 9:27 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 9:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dirk,
CC clock, ARM, DT, PM people
TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
disables them as being unused.
On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> CLK_IS_CRITICAL") we are able to handle critical module clocks.
> Introduce the same logic for critical core clocks.
>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
> Commit
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>
> is quite nice to avoid *module* clocks being disabled. Unfortunately,
> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> core of the r8a7795, the 'canfd' is a quite popular core clock which
> shouldn't be disabled by Linux.
>
> Therefore, this patch is a proposal to use the same 'mark clocks as
> critical' logic implemented for the module clocks for the core
> clocks, too.
>
> Opinions?
On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
core which may run another OS.
This is an interesting issue, and relevant to other SoCs, too.
In this particular case, the "canfd" clock is a core clock used as an
auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
to three scenarios:
1. Linux controls all CAN interfaces
=> no issue,
2. The OS on the RT CPU controls all CAN interfaces
=> issue, Linux disables the clock
3. Mix of 1 and 2
=> More issues.
Of course this is not limited to clocks, but also to e.g. PM domains.
How can this be handled?
I believe just marking the "canfd" clock critical is not the right solution,
as about any clock could be used by the RT CPU.
Still, Linux needs to be made aware that devices (clocks and PM domains) are
controlled by another CPU/OS.
Should this be described in DT? It feels like software policy to me.
Note that we (mainline) currently don't describe the Cortex R7 core in DT.
Dirk: do you describe it?
Summary:
1. Core/module clocks are described in the clock driver (not in DT),
2. Unused clocks are disabled by CCF,
3. Clocks may be in use by the Real-Time CPU core, running another OS,
4. How to communicate to Linux which clocks are under control of the RT CPU?
Thanks for your comments!
> drivers/clk/renesas/clk-div6.c | 17 +++++++++++++++--
> drivers/clk/renesas/clk-div6.h | 4 +++-
> drivers/clk/renesas/r8a7795-cpg-mssr.c | 7 +++++++
> drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
> drivers/clk/renesas/renesas-cpg-mssr.h | 8 ++++++++
> 5 files changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/renesas/clk-div6.c b/drivers/clk/renesas/clk-div6.c
> index 0627860..5917e05 100644
> --- a/drivers/clk/renesas/clk-div6.c
> +++ b/drivers/clk/renesas/clk-div6.c
> @@ -18,6 +18,7 @@
> #include <linux/of_address.h>
> #include <linux/slab.h>
>
> +#include "renesas-cpg-mssr.h"
> #include "clk-div6.h"
>
> #define CPG_DIV6_CKSTP BIT(8)
> @@ -184,7 +185,9 @@ static const struct clk_ops cpg_div6_clock_ops = {
> struct clk * __init cpg_div6_register(const char *name,
> unsigned int num_parents,
> const char **parent_names,
> - void __iomem *reg)
> + void __iomem *reg,
> + const struct cpg_mssr_info *info,
> + unsigned int id)
> {
> unsigned int valid_parents;
> struct clk_init_data init;
> @@ -246,6 +249,15 @@ struct clk * __init cpg_div6_register(const char *name,
> init.name = name;
> init.ops = &cpg_div6_clock_ops;
> init.flags = CLK_IS_BASIC;
> + if (info) {
> + for (i = 0; i < info->num_crit_core_clks; i++)
> + if (id == info->crit_core_clks[i]) {
> + pr_devel("DIV6 %s setting CLK_IS_CRITICAL\n",
> + name);
> + init.flags |= CLK_IS_CRITICAL;
> + break;
> + }
> + }
> init.parent_names = parent_names;
> init.num_parents = valid_parents;
>
> @@ -298,7 +310,8 @@ static void __init cpg_div6_clock_init(struct device_node *np)
> for (i = 0; i < num_parents; i++)
> parent_names[i] = of_clk_get_parent_name(np, i);
>
> - clk = cpg_div6_register(clk_name, num_parents, parent_names, reg);
> + clk = cpg_div6_register(clk_name, num_parents, parent_names, reg,
> + NULL, 0);
> if (IS_ERR(clk)) {
> pr_err("%s: failed to register %s DIV6 clock (%ld)\n",
> __func__, np->name, PTR_ERR(clk));
> diff --git a/drivers/clk/renesas/clk-div6.h b/drivers/clk/renesas/clk-div6.h
> index 567b31d..b619d6b4 100644
> --- a/drivers/clk/renesas/clk-div6.h
> +++ b/drivers/clk/renesas/clk-div6.h
> @@ -2,6 +2,8 @@
> #define __RENESAS_CLK_DIV6_H__
>
> struct clk *cpg_div6_register(const char *name, unsigned int num_parents,
> - const char **parent_names, void __iomem *reg);
> + const char **parent_names, void __iomem *reg,
> + const struct cpg_mssr_info *info,
> + unsigned int id);
>
> #endif
> diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> index eaa98b4..a54fed6 100644
> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> @@ -114,6 +114,9 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
> DEF_BASE("r", R8A7795_CLK_R, CLK_TYPE_GEN3_R, CLK_RINT),
> };
>
> +static const unsigned int r8a7795_crit_core_clks[] __initconst = {
> +};
> +
> static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
> DEF_MOD("fdp1-2", 117, R8A7795_CLK_S2D1), /* ES1.x */
> DEF_MOD("fdp1-1", 118, R8A7795_CLK_S0D1),
> @@ -441,6 +444,10 @@ const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
> .last_dt_core_clk = LAST_DT_CORE_CLK,
> .num_total_core_clks = MOD_CLK_BASE,
>
> + /* Critical Core Clocks */
> + .crit_core_clks = r8a7795_crit_core_clks,
> + .num_crit_core_clks = ARRAY_SIZE(r8a7795_crit_core_clks),
> +
> /* Module Clocks */
> .mod_clks = r8a7795_mod_clks,
> .num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks),
> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> index 99eeec6..80be019 100644
> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> @@ -293,7 +293,8 @@ static void __init cpg_mssr_register_core_clk(const struct cpg_core_clk *core,
>
> if (core->type == CLK_TYPE_DIV6P1) {
> clk = cpg_div6_register(core->name, 1, &parent_name,
> - priv->base + core->offset);
> + priv->base + core->offset, info,
> + core->id);
> } else {
> clk = clk_register_fixed_factor(NULL, core->name,
> parent_name, 0,
> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
> index 148f4f0a..a723fdd 100644
> --- a/drivers/clk/renesas/renesas-cpg-mssr.h
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
> @@ -86,6 +86,10 @@ struct device_node;
> * @last_dt_core_clk: ID of the last Core Clock exported to DT
> * @num_total_core_clks: Total number of Core Clocks (exported + internal)
> *
> + * @crit_core_clks: Array with Core Clock IDs of critical clocks that
> + * should not be disabled without a knowledgeable driver
> + * @num_core_mod_clks: Number of entries in crit_core_clks[]
> + *
> * @mod_clks: Array of Module Clock definitions
> * @num_mod_clks: Number of entries in mod_clks[]
> * @num_hw_mod_clks: Number of Module Clocks supported by the hardware
> @@ -109,6 +113,10 @@ struct cpg_mssr_info {
> unsigned int last_dt_core_clk;
> unsigned int num_total_core_clks;
>
> + /* Critical Core Clocks that should not be disabled */
> + const unsigned int *crit_core_clks;
> + unsigned int num_crit_core_clks;
> +
> /* Module Clocks */
> const struct mssr_mod_clk *mod_clks;
> unsigned int num_mod_clks;
> --
> 2.8.0
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-06-29 9:27 ` Geert Uytterhoeven
@ 2017-06-29 10:28 ` Dirk Behme
-1 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-06-29 10:28 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
On 29.06.2017 11:27, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
>
> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>> Introduce the same logic for critical core clocks.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>> Commit
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>
>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>> shouldn't be disabled by Linux.
>>
>> Therefore, this patch is a proposal to use the same 'mark clocks as
>> critical' logic implemented for the module clocks for the core
>> clocks, too.
>>
>> Opinions?
>
> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> core which may run another OS.
> This is an interesting issue, and relevant to other SoCs, too.
>
> In this particular case, the "canfd" clock is a core clock used as an
> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> to three scenarios:
> 1. Linux controls all CAN interfaces
> => no issue,
> 2. The OS on the RT CPU controls all CAN interfaces
> => issue, Linux disables the clock
> 3. Mix of 1 and 2
> => More issues.
> Of course this is not limited to clocks, but also to e.g. PM domains.
>
> How can this be handled?
> I believe just marking the "canfd" clock critical is not the right solution,
> as about any clock could be used by the RT CPU.
>
> Still, Linux needs to be made aware that devices (clocks and PM domains) are
> controlled by another CPU/OS.
>
> Should this be described in DT? It feels like software policy to me.
>
> Note that we (mainline) currently don't describe the Cortex R7 core in DT.
> Dirk: do you describe it?
No, we don't describe anything R7 related in DT, too.
> Summary:
> 1. Core/module clocks are described in the clock driver (not in DT),
> 2. Unused clocks are disabled by CCF,
> 3. Clocks may be in use by the Real-Time CPU core, running another OS,
> 4. How to communicate to Linux which clocks are under control of the RT CPU?
>
> Thanks for your comments!
While I appreciated that the overall issue is discussed, I'm not sure if
there is anything really special we don't support generally, yet.
We have an infrastructure to mark clocks enabled anywhere else to be not
disabled by Linux kernel (CLK_IS_CRITICAL). From my point of view, for
this infrastructure, it doesn't matter, where this 'anywhere else' is.
To take some concrete Renesas RCar3 examples, from my point of view it
doesn't matter if its a GIC-400 clock enabled in the boot loader
(U-Boot) or a CAN clock enabled by the R7. In both cases marking them as
critical on Linux side does the trick.
The issue I just want to address (discuss) with this RFC patch is that
for Renesas RCar3 we have CLK_IS_CRITICAL support for module clocks, but
not for core clocks. From my point of view, this is a completely Renesas
implementation specific discussion.
So I would rephrase the initial sentence above
"Clocks may be in use by another CPU not running Linux, while Linux
disables them as being unused."
to anything like
"Clocks may be enabled (used) by others (U-Boot/CPUs/Hypervisors), while
Linux disables them as being unused"
And I think this is completely addressed by CLK_IS_CRITICAL, as far as I
can see :)
Best regards
Dirk
>> drivers/clk/renesas/clk-div6.c | 17 +++++++++++++++--
>> drivers/clk/renesas/clk-div6.h | 4 +++-
>> drivers/clk/renesas/r8a7795-cpg-mssr.c | 7 +++++++
>> drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
>> drivers/clk/renesas/renesas-cpg-mssr.h | 8 ++++++++
>> 5 files changed, 35 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/renesas/clk-div6.c b/drivers/clk/renesas/clk-div6.c
>> index 0627860..5917e05 100644
>> --- a/drivers/clk/renesas/clk-div6.c
>> +++ b/drivers/clk/renesas/clk-div6.c
>> @@ -18,6 +18,7 @@
>> #include <linux/of_address.h>
>> #include <linux/slab.h>
>>
>> +#include "renesas-cpg-mssr.h"
>> #include "clk-div6.h"
>>
>> #define CPG_DIV6_CKSTP BIT(8)
>> @@ -184,7 +185,9 @@ static const struct clk_ops cpg_div6_clock_ops = {
>> struct clk * __init cpg_div6_register(const char *name,
>> unsigned int num_parents,
>> const char **parent_names,
>> - void __iomem *reg)
>> + void __iomem *reg,
>> + const struct cpg_mssr_info *info,
>> + unsigned int id)
>> {
>> unsigned int valid_parents;
>> struct clk_init_data init;
>> @@ -246,6 +249,15 @@ struct clk * __init cpg_div6_register(const char *name,
>> init.name = name;
>> init.ops = &cpg_div6_clock_ops;
>> init.flags = CLK_IS_BASIC;
>> + if (info) {
>> + for (i = 0; i < info->num_crit_core_clks; i++)
>> + if (id == info->crit_core_clks[i]) {
>> + pr_devel("DIV6 %s setting CLK_IS_CRITICAL\n",
>> + name);
>> + init.flags |= CLK_IS_CRITICAL;
>> + break;
>> + }
>> + }
>> init.parent_names = parent_names;
>> init.num_parents = valid_parents;
>>
>> @@ -298,7 +310,8 @@ static void __init cpg_div6_clock_init(struct device_node *np)
>> for (i = 0; i < num_parents; i++)
>> parent_names[i] = of_clk_get_parent_name(np, i);
>>
>> - clk = cpg_div6_register(clk_name, num_parents, parent_names, reg);
>> + clk = cpg_div6_register(clk_name, num_parents, parent_names, reg,
>> + NULL, 0);
>> if (IS_ERR(clk)) {
>> pr_err("%s: failed to register %s DIV6 clock (%ld)\n",
>> __func__, np->name, PTR_ERR(clk));
>> diff --git a/drivers/clk/renesas/clk-div6.h b/drivers/clk/renesas/clk-div6.h
>> index 567b31d..b619d6b4 100644
>> --- a/drivers/clk/renesas/clk-div6.h
>> +++ b/drivers/clk/renesas/clk-div6.h
>> @@ -2,6 +2,8 @@
>> #define __RENESAS_CLK_DIV6_H__
>>
>> struct clk *cpg_div6_register(const char *name, unsigned int num_parents,
>> - const char **parent_names, void __iomem *reg);
>> + const char **parent_names, void __iomem *reg,
>> + const struct cpg_mssr_info *info,
>> + unsigned int id);
>>
>> #endif
>> diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> index eaa98b4..a54fed6 100644
>> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> @@ -114,6 +114,9 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
>> DEF_BASE("r", R8A7795_CLK_R, CLK_TYPE_GEN3_R, CLK_RINT),
>> };
>>
>> +static const unsigned int r8a7795_crit_core_clks[] __initconst = {
>> +};
>> +
>> static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
>> DEF_MOD("fdp1-2", 117, R8A7795_CLK_S2D1), /* ES1.x */
>> DEF_MOD("fdp1-1", 118, R8A7795_CLK_S0D1),
>> @@ -441,6 +444,10 @@ const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
>> .last_dt_core_clk = LAST_DT_CORE_CLK,
>> .num_total_core_clks = MOD_CLK_BASE,
>>
>> + /* Critical Core Clocks */
>> + .crit_core_clks = r8a7795_crit_core_clks,
>> + .num_crit_core_clks = ARRAY_SIZE(r8a7795_crit_core_clks),
>> +
>> /* Module Clocks */
>> .mod_clks = r8a7795_mod_clks,
>> .num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks),
>> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
>> index 99eeec6..80be019 100644
>> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
>> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
>> @@ -293,7 +293,8 @@ static void __init cpg_mssr_register_core_clk(const struct cpg_core_clk *core,
>>
>> if (core->type == CLK_TYPE_DIV6P1) {
>> clk = cpg_div6_register(core->name, 1, &parent_name,
>> - priv->base + core->offset);
>> + priv->base + core->offset, info,
>> + core->id);
>> } else {
>> clk = clk_register_fixed_factor(NULL, core->name,
>> parent_name, 0,
>> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
>> index 148f4f0a..a723fdd 100644
>> --- a/drivers/clk/renesas/renesas-cpg-mssr.h
>> +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
>> @@ -86,6 +86,10 @@ struct device_node;
>> * @last_dt_core_clk: ID of the last Core Clock exported to DT
>> * @num_total_core_clks: Total number of Core Clocks (exported + internal)
>> *
>> + * @crit_core_clks: Array with Core Clock IDs of critical clocks that
>> + * should not be disabled without a knowledgeable driver
>> + * @num_core_mod_clks: Number of entries in crit_core_clks[]
>> + *
>> * @mod_clks: Array of Module Clock definitions
>> * @num_mod_clks: Number of entries in mod_clks[]
>> * @num_hw_mod_clks: Number of Module Clocks supported by the hardware
>> @@ -109,6 +113,10 @@ struct cpg_mssr_info {
>> unsigned int last_dt_core_clk;
>> unsigned int num_total_core_clks;
>>
>> + /* Critical Core Clocks that should not be disabled */
>> + const unsigned int *crit_core_clks;
>> + unsigned int num_crit_core_clks;
>> +
>> /* Module Clocks */
>> const struct mssr_mod_clk *mod_clks;
>> unsigned int num_mod_clks;
>> --
>> 2.8.0
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-29 10:28 ` Dirk Behme
0 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-06-29 10:28 UTC (permalink / raw)
To: linux-arm-kernel
On 29.06.2017 11:27, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
>
> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>> Introduce the same logic for critical core clocks.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>> Commit
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>
>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>> shouldn't be disabled by Linux.
>>
>> Therefore, this patch is a proposal to use the same 'mark clocks as
>> critical' logic implemented for the module clocks for the core
>> clocks, too.
>>
>> Opinions?
>
> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> core which may run another OS.
> This is an interesting issue, and relevant to other SoCs, too.
>
> In this particular case, the "canfd" clock is a core clock used as an
> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> to three scenarios:
> 1. Linux controls all CAN interfaces
> => no issue,
> 2. The OS on the RT CPU controls all CAN interfaces
> => issue, Linux disables the clock
> 3. Mix of 1 and 2
> => More issues.
> Of course this is not limited to clocks, but also to e.g. PM domains.
>
> How can this be handled?
> I believe just marking the "canfd" clock critical is not the right solution,
> as about any clock could be used by the RT CPU.
>
> Still, Linux needs to be made aware that devices (clocks and PM domains) are
> controlled by another CPU/OS.
>
> Should this be described in DT? It feels like software policy to me.
>
> Note that we (mainline) currently don't describe the Cortex R7 core in DT.
> Dirk: do you describe it?
No, we don't describe anything R7 related in DT, too.
> Summary:
> 1. Core/module clocks are described in the clock driver (not in DT),
> 2. Unused clocks are disabled by CCF,
> 3. Clocks may be in use by the Real-Time CPU core, running another OS,
> 4. How to communicate to Linux which clocks are under control of the RT CPU?
>
> Thanks for your comments!
While I appreciated that the overall issue is discussed, I'm not sure if
there is anything really special we don't support generally, yet.
We have an infrastructure to mark clocks enabled anywhere else to be not
disabled by Linux kernel (CLK_IS_CRITICAL). From my point of view, for
this infrastructure, it doesn't matter, where this 'anywhere else' is.
To take some concrete Renesas RCar3 examples, from my point of view it
doesn't matter if its a GIC-400 clock enabled in the boot loader
(U-Boot) or a CAN clock enabled by the R7. In both cases marking them as
critical on Linux side does the trick.
The issue I just want to address (discuss) with this RFC patch is that
for Renesas RCar3 we have CLK_IS_CRITICAL support for module clocks, but
not for core clocks. From my point of view, this is a completely Renesas
implementation specific discussion.
So I would rephrase the initial sentence above
"Clocks may be in use by another CPU not running Linux, while Linux
disables them as being unused."
to anything like
"Clocks may be enabled (used) by others (U-Boot/CPUs/Hypervisors), while
Linux disables them as being unused"
And I think this is completely addressed by CLK_IS_CRITICAL, as far as I
can see :)
Best regards
Dirk
>> drivers/clk/renesas/clk-div6.c | 17 +++++++++++++++--
>> drivers/clk/renesas/clk-div6.h | 4 +++-
>> drivers/clk/renesas/r8a7795-cpg-mssr.c | 7 +++++++
>> drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
>> drivers/clk/renesas/renesas-cpg-mssr.h | 8 ++++++++
>> 5 files changed, 35 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/renesas/clk-div6.c b/drivers/clk/renesas/clk-div6.c
>> index 0627860..5917e05 100644
>> --- a/drivers/clk/renesas/clk-div6.c
>> +++ b/drivers/clk/renesas/clk-div6.c
>> @@ -18,6 +18,7 @@
>> #include <linux/of_address.h>
>> #include <linux/slab.h>
>>
>> +#include "renesas-cpg-mssr.h"
>> #include "clk-div6.h"
>>
>> #define CPG_DIV6_CKSTP BIT(8)
>> @@ -184,7 +185,9 @@ static const struct clk_ops cpg_div6_clock_ops = {
>> struct clk * __init cpg_div6_register(const char *name,
>> unsigned int num_parents,
>> const char **parent_names,
>> - void __iomem *reg)
>> + void __iomem *reg,
>> + const struct cpg_mssr_info *info,
>> + unsigned int id)
>> {
>> unsigned int valid_parents;
>> struct clk_init_data init;
>> @@ -246,6 +249,15 @@ struct clk * __init cpg_div6_register(const char *name,
>> init.name = name;
>> init.ops = &cpg_div6_clock_ops;
>> init.flags = CLK_IS_BASIC;
>> + if (info) {
>> + for (i = 0; i < info->num_crit_core_clks; i++)
>> + if (id == info->crit_core_clks[i]) {
>> + pr_devel("DIV6 %s setting CLK_IS_CRITICAL\n",
>> + name);
>> + init.flags |= CLK_IS_CRITICAL;
>> + break;
>> + }
>> + }
>> init.parent_names = parent_names;
>> init.num_parents = valid_parents;
>>
>> @@ -298,7 +310,8 @@ static void __init cpg_div6_clock_init(struct device_node *np)
>> for (i = 0; i < num_parents; i++)
>> parent_names[i] = of_clk_get_parent_name(np, i);
>>
>> - clk = cpg_div6_register(clk_name, num_parents, parent_names, reg);
>> + clk = cpg_div6_register(clk_name, num_parents, parent_names, reg,
>> + NULL, 0);
>> if (IS_ERR(clk)) {
>> pr_err("%s: failed to register %s DIV6 clock (%ld)\n",
>> __func__, np->name, PTR_ERR(clk));
>> diff --git a/drivers/clk/renesas/clk-div6.h b/drivers/clk/renesas/clk-div6.h
>> index 567b31d..b619d6b4 100644
>> --- a/drivers/clk/renesas/clk-div6.h
>> +++ b/drivers/clk/renesas/clk-div6.h
>> @@ -2,6 +2,8 @@
>> #define __RENESAS_CLK_DIV6_H__
>>
>> struct clk *cpg_div6_register(const char *name, unsigned int num_parents,
>> - const char **parent_names, void __iomem *reg);
>> + const char **parent_names, void __iomem *reg,
>> + const struct cpg_mssr_info *info,
>> + unsigned int id);
>>
>> #endif
>> diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> index eaa98b4..a54fed6 100644
>> --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
>> @@ -114,6 +114,9 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
>> DEF_BASE("r", R8A7795_CLK_R, CLK_TYPE_GEN3_R, CLK_RINT),
>> };
>>
>> +static const unsigned int r8a7795_crit_core_clks[] __initconst = {
>> +};
>> +
>> static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
>> DEF_MOD("fdp1-2", 117, R8A7795_CLK_S2D1), /* ES1.x */
>> DEF_MOD("fdp1-1", 118, R8A7795_CLK_S0D1),
>> @@ -441,6 +444,10 @@ const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
>> .last_dt_core_clk = LAST_DT_CORE_CLK,
>> .num_total_core_clks = MOD_CLK_BASE,
>>
>> + /* Critical Core Clocks */
>> + .crit_core_clks = r8a7795_crit_core_clks,
>> + .num_crit_core_clks = ARRAY_SIZE(r8a7795_crit_core_clks),
>> +
>> /* Module Clocks */
>> .mod_clks = r8a7795_mod_clks,
>> .num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks),
>> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
>> index 99eeec6..80be019 100644
>> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
>> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
>> @@ -293,7 +293,8 @@ static void __init cpg_mssr_register_core_clk(const struct cpg_core_clk *core,
>>
>> if (core->type == CLK_TYPE_DIV6P1) {
>> clk = cpg_div6_register(core->name, 1, &parent_name,
>> - priv->base + core->offset);
>> + priv->base + core->offset, info,
>> + core->id);
>> } else {
>> clk = clk_register_fixed_factor(NULL, core->name,
>> parent_name, 0,
>> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
>> index 148f4f0a..a723fdd 100644
>> --- a/drivers/clk/renesas/renesas-cpg-mssr.h
>> +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
>> @@ -86,6 +86,10 @@ struct device_node;
>> * @last_dt_core_clk: ID of the last Core Clock exported to DT
>> * @num_total_core_clks: Total number of Core Clocks (exported + internal)
>> *
>> + * @crit_core_clks: Array with Core Clock IDs of critical clocks that
>> + * should not be disabled without a knowledgeable driver
>> + * @num_core_mod_clks: Number of entries in crit_core_clks[]
>> + *
>> * @mod_clks: Array of Module Clock definitions
>> * @num_mod_clks: Number of entries in mod_clks[]
>> * @num_hw_mod_clks: Number of Module Clocks supported by the hardware
>> @@ -109,6 +113,10 @@ struct cpg_mssr_info {
>> unsigned int last_dt_core_clk;
>> unsigned int num_total_core_clks;
>>
>> + /* Critical Core Clocks that should not be disabled */
>> + const unsigned int *crit_core_clks;
>> + unsigned int num_crit_core_clks;
>> +
>> /* Module Clocks */
>> const struct mssr_mod_clk *mod_clks;
>> unsigned int num_mod_clks;
>> --
>> 2.8.0
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-06-29 10:28 ` Dirk Behme
@ 2017-06-29 11:18 ` Geert Uytterhoeven
-1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 11:18 UTC (permalink / raw)
To: Dirk Behme
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
Hi Dirk,
On Thu, Jun 29, 2017 at 12:28 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 29.06.2017 11:27, Geert Uytterhoeven wrote:
>> CC clock, ARM, DT, PM people
>>
>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>> disables them as being unused.
>>
>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>> Introduce the same logic for critical core clocks.
>>>
>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>> ---
>>> Commit
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>
>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>> shouldn't be disabled by Linux.
>>>
>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>> critical' logic implemented for the module clocks for the core
>>> clocks, too.
>>>
>>> Opinions?
>>
>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex
>> R7
>> core which may run another OS.
>> This is an interesting issue, and relevant to other SoCs, too.
>>
>> In this particular case, the "canfd" clock is a core clock used as an
>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>> to three scenarios:
>> 1. Linux controls all CAN interfaces
>> => no issue,
>> 2. The OS on the RT CPU controls all CAN interfaces
>> => issue, Linux disables the clock
>> 3. Mix of 1 and 2
>> => More issues.
>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>
>> How can this be handled?
>> I believe just marking the "canfd" clock critical is not the right
>> solution,
>> as about any clock could be used by the RT CPU.
>>
>> Still, Linux needs to be made aware that devices (clocks and PM domains)
>> are
>> controlled by another CPU/OS.
>>
>> Should this be described in DT? It feels like software policy to me.
>>
>> Note that we (mainline) currently don't describe the Cortex R7 core in DT.
>> Dirk: do you describe it?
>
> No, we don't describe anything R7 related in DT, too.
>
>> Summary:
>> 1. Core/module clocks are described in the clock driver (not in DT),
>> 2. Unused clocks are disabled by CCF,
>> 3. Clocks may be in use by the Real-Time CPU core, running another OS,
>> 4. How to communicate to Linux which clocks are under control of the RT
>> CPU?
>
>> Thanks for your comments!
>
> While I appreciated that the overall issue is discussed, I'm not sure if
> there is anything really special we don't support generally, yet.
>
> We have an infrastructure to mark clocks enabled anywhere else to be not
> disabled by Linux kernel (CLK_IS_CRITICAL). From my point of view, for this
> infrastructure, it doesn't matter, where this 'anywhere else' is. To take
> some concrete Renesas RCar3 examples, from my point of view it doesn't
> matter if its a GIC-400 clock enabled in the boot loader (U-Boot) or a CAN
> clock enabled by the R7. In both cases marking them as critical on Linux
> side does the trick.
Yes, it does the trick. But is it the proper solution?
> The issue I just want to address (discuss) with this RFC patch is that for
> Renesas RCar3 we have CLK_IS_CRITICAL support for module clocks, but not for
> core clocks. From my point of view, this is a completely Renesas
> implementation specific discussion.
Correct. How the Renesas CPG/MSSR driver decides which clocks are marked
critical is a Renesas-specific implementation issue.
And indeed, currently the driver only handles critical module clocks
(more specifically, the GIC module clock, due to the lack of runtime PM
support in the GIC driver).
It does not handle critical core clocks, as so far no use case required
such support.
> So I would rephrase the initial sentence above
>
> "Clocks may be in use by another CPU not running Linux, while Linux disables
> them as being unused."
>
> to anything like
>
> "Clocks may be enabled (used) by others (U-Boot/CPUs/Hypervisors), while
> Linux disables them as being unused"
If a clock is used by Linux, Linux must make sure it is enabled when
needed.
If a clock is needed to run Linux, without driver support, Linux must still
make sure it is enabled. This is were critical clocks enter the game.
The above includes CPU clocks.
If a clock is used by U-Boot, U-Boot must make sure it is enabled when
needed. Once U-Boot has transferred control to Linux, this no longer
matters.
if a clock is used by a HV, the HV must make sure it is enabled when
needed. I believe this is handled in secure mode? R-Car Gen3 SoCs already
have several module clocks that cannot be disabled (i.e. MSTPSRx != OR of
all yMSTPCRx).
If a clock is used by the RT CPU, the RT CPU must make sure it is enabled
when needed.
There are several options to do that:
- Secure mode,
- Communicating the clock requirements to Linux, either statically or
dynamically,
- DT? (but this is software policy?)
- ???
> And I think this is completely addressed by CLK_IS_CRITICAL, as far as I can
> see :)
While this is a solution that works (for your use case), IMHO it's not the
right thing to do.
However, if we decide to go this route, which other clocks should be marked
critical? Almost (all?) peripherals can be used by the RT CPU. Hence all
clocks should be enabled and marked critical, unconditionally?
So I still think we need a more general solution.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-29 11:18 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 11:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dirk,
On Thu, Jun 29, 2017 at 12:28 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 29.06.2017 11:27, Geert Uytterhoeven wrote:
>> CC clock, ARM, DT, PM people
>>
>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>> disables them as being unused.
>>
>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>> Introduce the same logic for critical core clocks.
>>>
>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>> ---
>>> Commit
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>
>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>> shouldn't be disabled by Linux.
>>>
>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>> critical' logic implemented for the module clocks for the core
>>> clocks, too.
>>>
>>> Opinions?
>>
>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex
>> R7
>> core which may run another OS.
>> This is an interesting issue, and relevant to other SoCs, too.
>>
>> In this particular case, the "canfd" clock is a core clock used as an
>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>> to three scenarios:
>> 1. Linux controls all CAN interfaces
>> => no issue,
>> 2. The OS on the RT CPU controls all CAN interfaces
>> => issue, Linux disables the clock
>> 3. Mix of 1 and 2
>> => More issues.
>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>
>> How can this be handled?
>> I believe just marking the "canfd" clock critical is not the right
>> solution,
>> as about any clock could be used by the RT CPU.
>>
>> Still, Linux needs to be made aware that devices (clocks and PM domains)
>> are
>> controlled by another CPU/OS.
>>
>> Should this be described in DT? It feels like software policy to me.
>>
>> Note that we (mainline) currently don't describe the Cortex R7 core in DT.
>> Dirk: do you describe it?
>
> No, we don't describe anything R7 related in DT, too.
>
>> Summary:
>> 1. Core/module clocks are described in the clock driver (not in DT),
>> 2. Unused clocks are disabled by CCF,
>> 3. Clocks may be in use by the Real-Time CPU core, running another OS,
>> 4. How to communicate to Linux which clocks are under control of the RT
>> CPU?
>
>> Thanks for your comments!
>
> While I appreciated that the overall issue is discussed, I'm not sure if
> there is anything really special we don't support generally, yet.
>
> We have an infrastructure to mark clocks enabled anywhere else to be not
> disabled by Linux kernel (CLK_IS_CRITICAL). From my point of view, for this
> infrastructure, it doesn't matter, where this 'anywhere else' is. To take
> some concrete Renesas RCar3 examples, from my point of view it doesn't
> matter if its a GIC-400 clock enabled in the boot loader (U-Boot) or a CAN
> clock enabled by the R7. In both cases marking them as critical on Linux
> side does the trick.
Yes, it does the trick. But is it the proper solution?
> The issue I just want to address (discuss) with this RFC patch is that for
> Renesas RCar3 we have CLK_IS_CRITICAL support for module clocks, but not for
> core clocks. From my point of view, this is a completely Renesas
> implementation specific discussion.
Correct. How the Renesas CPG/MSSR driver decides which clocks are marked
critical is a Renesas-specific implementation issue.
And indeed, currently the driver only handles critical module clocks
(more specifically, the GIC module clock, due to the lack of runtime PM
support in the GIC driver).
It does not handle critical core clocks, as so far no use case required
such support.
> So I would rephrase the initial sentence above
>
> "Clocks may be in use by another CPU not running Linux, while Linux disables
> them as being unused."
>
> to anything like
>
> "Clocks may be enabled (used) by others (U-Boot/CPUs/Hypervisors), while
> Linux disables them as being unused"
If a clock is used by Linux, Linux must make sure it is enabled when
needed.
If a clock is needed to run Linux, without driver support, Linux must still
make sure it is enabled. This is were critical clocks enter the game.
The above includes CPU clocks.
If a clock is used by U-Boot, U-Boot must make sure it is enabled when
needed. Once U-Boot has transferred control to Linux, this no longer
matters.
if a clock is used by a HV, the HV must make sure it is enabled when
needed. I believe this is handled in secure mode? R-Car Gen3 SoCs already
have several module clocks that cannot be disabled (i.e. MSTPSRx != OR of
all yMSTPCRx).
If a clock is used by the RT CPU, the RT CPU must make sure it is enabled
when needed.
There are several options to do that:
- Secure mode,
- Communicating the clock requirements to Linux, either statically or
dynamically,
- DT? (but this is software policy?)
- ???
> And I think this is completely addressed by CLK_IS_CRITICAL, as far as I can
> see :)
While this is a solution that works (for your use case), IMHO it's not the
right thing to do.
However, if we decide to go this route, which other clocks should be marked
critical? Almost (all?) peripherals can be used by the RT CPU. Hence all
clocks should be enabled and marked critical, unconditionally?
So I still think we need a more general solution.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-06-29 9:27 ` Geert Uytterhoeven
(?)
@ 2017-06-29 11:56 ` Geert Uytterhoeven
-1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 11:56 UTC (permalink / raw)
To: Dirk Behme
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux PM list
Hi Dirk,
On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
> Of course this is not limited to clocks, but also to e.g. PM domains.
BTW, how do you prevent Linux from powering down the CR7 PM domain,
which contains the Cortex R7?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-29 11:56 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 11:56 UTC (permalink / raw)
To: Dirk Behme
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
Hi Dirk,
On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
> Of course this is not limited to clocks, but also to e.g. PM domains.
BTW, how do you prevent Linux from powering down the CR7 PM domain,
which contains the Cortex R7?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-29 11:56 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 11:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dirk,
On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
> Of course this is not limited to clocks, but also to e.g. PM domains.
BTW, how do you prevent Linux from powering down the CR7 PM domain,
which contains the Cortex R7?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU
2017-06-29 11:56 ` Geert Uytterhoeven
@ 2017-06-29 12:07 ` Dirk Behme
-1 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-06-29 12:07 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
On 29.06.2017 13:56, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> CC clock, ARM, DT, PM people
>>
>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>> disables them as being unused.
>
>> Of course this is not limited to clocks, but also to e.g. PM domains.
>
> BTW, how do you prevent Linux from powering down the CR7 PM domain,
> which contains the Cortex R7?
The idea is
--- a/drivers/soc/renesas/r8a7795-sysc.c
+++ b/drivers/soc/renesas/r8a7795-sysc.c
@@ -38,7 +38,8 @@ static const struct rcar_sysc_area r8a7795_areas[]
__initconst = {
{ "ca53-cpu3", 0x200, 3, R8A7795_PD_CA53_CPU3, R8A7795_PD_CA53_SCU,
PD_CPU_NOCR },
{ "a3vp", 0x340, 0, R8A7795_PD_A3VP, R8A7795_PD_ALWAYS_ON },
- { "cr7", 0x240, 0, R8A7795_PD_CR7, R8A7795_PD_ALWAYS_ON },
+ { "cr7", 0x240, 0, R8A7795_PD_CR7, R8A7795_PD_ALWAYS_ON,
+ PD_CPU_NOCR },
{ "a3vc", 0x380, 0, R8A7795_PD_A3VC, R8A7795_PD_ALWAYS_ON },
{ "a2vc0", 0x3c0, 0, R8A7795_PD_A2VC0, R8A7795_PD_A3VC },
{ "a2vc1", 0x3c0, 1, R8A7795_PD_A2VC1, R8A7795_PD_A3VC },
Not finally tested, though.
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU
@ 2017-06-29 12:07 ` Dirk Behme
0 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-06-29 12:07 UTC (permalink / raw)
To: linux-arm-kernel
On 29.06.2017 13:56, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> CC clock, ARM, DT, PM people
>>
>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>> disables them as being unused.
>
>> Of course this is not limited to clocks, but also to e.g. PM domains.
>
> BTW, how do you prevent Linux from powering down the CR7 PM domain,
> which contains the Cortex R7?
The idea is
--- a/drivers/soc/renesas/r8a7795-sysc.c
+++ b/drivers/soc/renesas/r8a7795-sysc.c
@@ -38,7 +38,8 @@ static const struct rcar_sysc_area r8a7795_areas[]
__initconst = {
{ "ca53-cpu3", 0x200, 3, R8A7795_PD_CA53_CPU3, R8A7795_PD_CA53_SCU,
PD_CPU_NOCR },
{ "a3vp", 0x340, 0, R8A7795_PD_A3VP, R8A7795_PD_ALWAYS_ON },
- { "cr7", 0x240, 0, R8A7795_PD_CR7, R8A7795_PD_ALWAYS_ON },
+ { "cr7", 0x240, 0, R8A7795_PD_CR7, R8A7795_PD_ALWAYS_ON,
+ PD_CPU_NOCR },
{ "a3vc", 0x380, 0, R8A7795_PD_A3VC, R8A7795_PD_ALWAYS_ON },
{ "a2vc0", 0x3c0, 0, R8A7795_PD_A2VC0, R8A7795_PD_A3VC },
{ "a2vc1", 0x3c0, 1, R8A7795_PD_A2VC1, R8A7795_PD_A3VC },
Not finally tested, though.
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU
2017-06-29 12:07 ` Dirk Behme
@ 2017-06-29 12:45 ` Geert Uytterhoeven
-1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 12:45 UTC (permalink / raw)
To: Dirk Behme
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
Hi Dirk,
On Thu, Jun 29, 2017 at 2:07 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 29.06.2017 13:56, Geert Uytterhoeven wrote:
>> On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> CC clock, ARM, DT, PM people
>>>
>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>> disables them as being unused.
>>
>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>
>> BTW, how do you prevent Linux from powering down the CR7 PM domain,
>> which contains the Cortex R7?
>
> The idea is
>
> --- a/drivers/soc/renesas/r8a7795-sysc.c
> +++ b/drivers/soc/renesas/r8a7795-sysc.c
> @@ -38,7 +38,8 @@ static const struct rcar_sysc_area r8a7795_areas[]
> __initconst = {
> { "ca53-cpu3", 0x200, 3, R8A7795_PD_CA53_CPU3, R8A7795_PD_CA53_SCU,
> PD_CPU_NOCR },
> { "a3vp", 0x340, 0, R8A7795_PD_A3VP, R8A7795_PD_ALWAYS_ON
> },
> - { "cr7", 0x240, 0, R8A7795_PD_CR7, R8A7795_PD_ALWAYS_ON
> },
> + { "cr7", 0x240, 0, R8A7795_PD_CR7,
> R8A7795_PD_ALWAYS_ON,
> + PD_CPU_NOCR },
> { "a3vc", 0x380, 0, R8A7795_PD_A3VC, R8A7795_PD_ALWAYS_ON
> },
> { "a2vc0", 0x3c0, 0, R8A7795_PD_A2VC0, R8A7795_PD_A3VC },
> { "a2vc1", 0x3c0, 1, R8A7795_PD_A2VC1, R8A7795_PD_A3VC },
>
>
> Not finally tested, though.
Pretending the CR7 PM Domain can only be controlled through WFI/APMU only should
work.
But IMHO it is a hack, just like your solution for the CANFD clock.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU
@ 2017-06-29 12:45 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 12:45 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dirk,
On Thu, Jun 29, 2017 at 2:07 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 29.06.2017 13:56, Geert Uytterhoeven wrote:
>> On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> CC clock, ARM, DT, PM people
>>>
>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>> disables them as being unused.
>>
>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>
>> BTW, how do you prevent Linux from powering down the CR7 PM domain,
>> which contains the Cortex R7?
>
> The idea is
>
> --- a/drivers/soc/renesas/r8a7795-sysc.c
> +++ b/drivers/soc/renesas/r8a7795-sysc.c
> @@ -38,7 +38,8 @@ static const struct rcar_sysc_area r8a7795_areas[]
> __initconst = {
> { "ca53-cpu3", 0x200, 3, R8A7795_PD_CA53_CPU3, R8A7795_PD_CA53_SCU,
> PD_CPU_NOCR },
> { "a3vp", 0x340, 0, R8A7795_PD_A3VP, R8A7795_PD_ALWAYS_ON
> },
> - { "cr7", 0x240, 0, R8A7795_PD_CR7, R8A7795_PD_ALWAYS_ON
> },
> + { "cr7", 0x240, 0, R8A7795_PD_CR7,
> R8A7795_PD_ALWAYS_ON,
> + PD_CPU_NOCR },
> { "a3vc", 0x380, 0, R8A7795_PD_A3VC, R8A7795_PD_ALWAYS_ON
> },
> { "a2vc0", 0x3c0, 0, R8A7795_PD_A2VC0, R8A7795_PD_A3VC },
> { "a2vc1", 0x3c0, 1, R8A7795_PD_A2VC1, R8A7795_PD_A3VC },
>
>
> Not finally tested, though.
Pretending the CR7 PM Domain can only be controlled through WFI/APMU only should
work.
But IMHO it is a hack, just like your solution for the CANFD clock.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU
2017-06-29 12:45 ` Geert Uytterhoeven
@ 2017-06-29 12:55 ` Dirk Behme
-1 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-06-29 12:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
On 29.06.2017 14:45, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Thu, Jun 29, 2017 at 2:07 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 29.06.2017 13:56, Geert Uytterhoeven wrote:
>>> On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> CC clock, ARM, DT, PM people
>>>>
>>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>>> disables them as being unused.
>>>
>>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>
>>> BTW, how do you prevent Linux from powering down the CR7 PM domain,
>>> which contains the Cortex R7?
>>
>> The idea is
>>
>> --- a/drivers/soc/renesas/r8a7795-sysc.c
>> +++ b/drivers/soc/renesas/r8a7795-sysc.c
>> @@ -38,7 +38,8 @@ static const struct rcar_sysc_area r8a7795_areas[]
>> __initconst = {
>> { "ca53-cpu3", 0x200, 3, R8A7795_PD_CA53_CPU3, R8A7795_PD_CA53_SCU,
>> PD_CPU_NOCR },
>> { "a3vp", 0x340, 0, R8A7795_PD_A3VP, R8A7795_PD_ALWAYS_ON
>> },
>> - { "cr7", 0x240, 0, R8A7795_PD_CR7, R8A7795_PD_ALWAYS_ON
>> },
>> + { "cr7", 0x240, 0, R8A7795_PD_CR7,
>> R8A7795_PD_ALWAYS_ON,
>> + PD_CPU_NOCR },
>> { "a3vc", 0x380, 0, R8A7795_PD_A3VC, R8A7795_PD_ALWAYS_ON
>> },
>> { "a2vc0", 0x3c0, 0, R8A7795_PD_A2VC0, R8A7795_PD_A3VC },
>> { "a2vc1", 0x3c0, 1, R8A7795_PD_A2VC1, R8A7795_PD_A3VC },
>>
>>
>> Not finally tested, though.
>
> Pretending the CR7 PM Domain can only be controlled through WFI/APMU only should
> work.
> But IMHO it is a hack, just like your solution for the CANFD clock.
Well, it seems to be the best solution recent mainline gives us and
seems to work ;)
And for me it looks at least better than the other solution I've seen so
far: Remove the 'cr7', 'canfd' etc lines completely from the kernel.
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU
@ 2017-06-29 12:55 ` Dirk Behme
0 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-06-29 12:55 UTC (permalink / raw)
To: linux-arm-kernel
On 29.06.2017 14:45, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Thu, Jun 29, 2017 at 2:07 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 29.06.2017 13:56, Geert Uytterhoeven wrote:
>>> On Thu, Jun 29, 2017 at 11:27 AM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> CC clock, ARM, DT, PM people
>>>>
>>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>>> disables them as being unused.
>>>
>>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>
>>> BTW, how do you prevent Linux from powering down the CR7 PM domain,
>>> which contains the Cortex R7?
>>
>> The idea is
>>
>> --- a/drivers/soc/renesas/r8a7795-sysc.c
>> +++ b/drivers/soc/renesas/r8a7795-sysc.c
>> @@ -38,7 +38,8 @@ static const struct rcar_sysc_area r8a7795_areas[]
>> __initconst = {
>> { "ca53-cpu3", 0x200, 3, R8A7795_PD_CA53_CPU3, R8A7795_PD_CA53_SCU,
>> PD_CPU_NOCR },
>> { "a3vp", 0x340, 0, R8A7795_PD_A3VP, R8A7795_PD_ALWAYS_ON
>> },
>> - { "cr7", 0x240, 0, R8A7795_PD_CR7, R8A7795_PD_ALWAYS_ON
>> },
>> + { "cr7", 0x240, 0, R8A7795_PD_CR7,
>> R8A7795_PD_ALWAYS_ON,
>> + PD_CPU_NOCR },
>> { "a3vc", 0x380, 0, R8A7795_PD_A3VC, R8A7795_PD_ALWAYS_ON
>> },
>> { "a2vc0", 0x3c0, 0, R8A7795_PD_A2VC0, R8A7795_PD_A3VC },
>> { "a2vc1", 0x3c0, 1, R8A7795_PD_A2VC1, R8A7795_PD_A3VC },
>>
>>
>> Not finally tested, though.
>
> Pretending the CR7 PM Domain can only be controlled through WFI/APMU only should
> work.
> But IMHO it is a hack, just like your solution for the CANFD clock.
Well, it seems to be the best solution recent mainline gives us and
seems to work ;)
And for me it looks at least better than the other solution I've seen so
far: Remove the 'cr7', 'canfd' etc lines completely from the kernel.
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU
2017-06-29 11:18 ` Geert Uytterhoeven
@ 2017-06-29 13:18 ` Dirk Behme
-1 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-06-29 13:18 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
On 29.06.2017 13:18, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Thu, Jun 29, 2017 at 12:28 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 29.06.2017 11:27, Geert Uytterhoeven wrote:
>>> CC clock, ARM, DT, PM people
>>>
>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>> disables them as being unused.
>>>
>>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>>> Introduce the same logic for critical core clocks.
>>>>
>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>> ---
>>>> Commit
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>>
>>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>>> shouldn't be disabled by Linux.
>>>>
>>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>>> critical' logic implemented for the module clocks for the core
>>>> clocks, too.
>>>>
>>>> Opinions?
>>>
>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex
>>> R7
>>> core which may run another OS.
>>> This is an interesting issue, and relevant to other SoCs, too.
>>>
>>> In this particular case, the "canfd" clock is a core clock used as an
>>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>>> to three scenarios:
>>> 1. Linux controls all CAN interfaces
>>> => no issue,
>>> 2. The OS on the RT CPU controls all CAN interfaces
>>> => issue, Linux disables the clock
>>> 3. Mix of 1 and 2
>>> => More issues.
>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>
>>> How can this be handled?
>>> I believe just marking the "canfd" clock critical is not the right
>>> solution,
>>> as about any clock could be used by the RT CPU.
>>>
>>> Still, Linux needs to be made aware that devices (clocks and PM domains)
>>> are
>>> controlled by another CPU/OS.
>>>
>>> Should this be described in DT? It feels like software policy to me.
>>>
>>> Note that we (mainline) currently don't describe the Cortex R7 core in DT.
>>> Dirk: do you describe it?
>>
>> No, we don't describe anything R7 related in DT, too.
>>
>>> Summary:
>>> 1. Core/module clocks are described in the clock driver (not in DT),
>>> 2. Unused clocks are disabled by CCF,
>>> 3. Clocks may be in use by the Real-Time CPU core, running another OS,
>>> 4. How to communicate to Linux which clocks are under control of the RT
>>> CPU?
>>
>>> Thanks for your comments!
>>
>> While I appreciated that the overall issue is discussed, I'm not sure if
>> there is anything really special we don't support generally, yet.
>>
>> We have an infrastructure to mark clocks enabled anywhere else to be not
>> disabled by Linux kernel (CLK_IS_CRITICAL). From my point of view, for this
>> infrastructure, it doesn't matter, where this 'anywhere else' is. To take
>> some concrete Renesas RCar3 examples, from my point of view it doesn't
>> matter if its a GIC-400 clock enabled in the boot loader (U-Boot) or a CAN
>> clock enabled by the R7. In both cases marking them as critical on Linux
>> side does the trick.
>
> Yes, it does the trick. But is it the proper solution?
>
>> The issue I just want to address (discuss) with this RFC patch is that for
>> Renesas RCar3 we have CLK_IS_CRITICAL support for module clocks, but not for
>> core clocks. From my point of view, this is a completely Renesas
>> implementation specific discussion.
>
> Correct. How the Renesas CPG/MSSR driver decides which clocks are marked
> critical is a Renesas-specific implementation issue.
> And indeed, currently the driver only handles critical module clocks
> (more specifically, the GIC module clock, due to the lack of runtime PM
> support in the GIC driver).
> It does not handle critical core clocks, as so far no use case required
> such support.
>
>> So I would rephrase the initial sentence above
>>
>> "Clocks may be in use by another CPU not running Linux, while Linux disables
>> them as being unused."
>>
>> to anything like
>>
>> "Clocks may be enabled (used) by others (U-Boot/CPUs/Hypervisors), while
>> Linux disables them as being unused"
>
> If a clock is used by Linux, Linux must make sure it is enabled when
> needed.
> If a clock is needed to run Linux, without driver support, Linux must still
> make sure it is enabled. This is were critical clocks enter the game.
> The above includes CPU clocks.
>
> If a clock is used by U-Boot, U-Boot must make sure it is enabled when
> needed. Once U-Boot has transferred control to Linux, this no longer
> matters.
>
> if a clock is used by a HV, the HV must make sure it is enabled when
> needed. I believe this is handled in secure mode? R-Car Gen3 SoCs already
> have several module clocks that cannot be disabled (i.e. MSTPSRx != OR of
> all yMSTPCRx).
>
> If a clock is used by the RT CPU, the RT CPU must make sure it is enabled
> when needed.
> There are several options to do that:
> - Secure mode,
> - Communicating the clock requirements to Linux, either statically or
> dynamically,
> - DT? (but this is software policy?)
> - ???
>
>> And I think this is completely addressed by CLK_IS_CRITICAL, as far as I can
>> see :)
>
> While this is a solution that works (for your use case), IMHO it's not the
> right thing to do.
> However, if we decide to go this route, which other clocks should be marked
> critical? Almost (all?) peripherals can be used by the RT CPU. Hence all
> clocks should be enabled and marked critical, unconditionally?
First of all, just the clocks we know (*) are used by the R7 side *and*
are disabled by the kernel (for a practical demo this are e.g. ~7 clocks).
(*) Yes, this is board (and R7 software) related. And yes, this might
result in board specific r8a7795_crit_mod_clks/r8a7795_crit_core_clks[]
tables. So yes, most probably device tree would be an option. While there is
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk.c#n3402
unfortunately
a) its marked as "Do not use this function"
and
b) even if we would ignore (a) we couldn't figure how to use this for
the RCar3 clocks (being no clock expert)
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU
@ 2017-06-29 13:18 ` Dirk Behme
0 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-06-29 13:18 UTC (permalink / raw)
To: linux-arm-kernel
On 29.06.2017 13:18, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Thu, Jun 29, 2017 at 12:28 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 29.06.2017 11:27, Geert Uytterhoeven wrote:
>>> CC clock, ARM, DT, PM people
>>>
>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>> disables them as being unused.
>>>
>>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>>> Introduce the same logic for critical core clocks.
>>>>
>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>> ---
>>>> Commit
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>>
>>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>>> shouldn't be disabled by Linux.
>>>>
>>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>>> critical' logic implemented for the module clocks for the core
>>>> clocks, too.
>>>>
>>>> Opinions?
>>>
>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex
>>> R7
>>> core which may run another OS.
>>> This is an interesting issue, and relevant to other SoCs, too.
>>>
>>> In this particular case, the "canfd" clock is a core clock used as an
>>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>>> to three scenarios:
>>> 1. Linux controls all CAN interfaces
>>> => no issue,
>>> 2. The OS on the RT CPU controls all CAN interfaces
>>> => issue, Linux disables the clock
>>> 3. Mix of 1 and 2
>>> => More issues.
>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>
>>> How can this be handled?
>>> I believe just marking the "canfd" clock critical is not the right
>>> solution,
>>> as about any clock could be used by the RT CPU.
>>>
>>> Still, Linux needs to be made aware that devices (clocks and PM domains)
>>> are
>>> controlled by another CPU/OS.
>>>
>>> Should this be described in DT? It feels like software policy to me.
>>>
>>> Note that we (mainline) currently don't describe the Cortex R7 core in DT.
>>> Dirk: do you describe it?
>>
>> No, we don't describe anything R7 related in DT, too.
>>
>>> Summary:
>>> 1. Core/module clocks are described in the clock driver (not in DT),
>>> 2. Unused clocks are disabled by CCF,
>>> 3. Clocks may be in use by the Real-Time CPU core, running another OS,
>>> 4. How to communicate to Linux which clocks are under control of the RT
>>> CPU?
>>
>>> Thanks for your comments!
>>
>> While I appreciated that the overall issue is discussed, I'm not sure if
>> there is anything really special we don't support generally, yet.
>>
>> We have an infrastructure to mark clocks enabled anywhere else to be not
>> disabled by Linux kernel (CLK_IS_CRITICAL). From my point of view, for this
>> infrastructure, it doesn't matter, where this 'anywhere else' is. To take
>> some concrete Renesas RCar3 examples, from my point of view it doesn't
>> matter if its a GIC-400 clock enabled in the boot loader (U-Boot) or a CAN
>> clock enabled by the R7. In both cases marking them as critical on Linux
>> side does the trick.
>
> Yes, it does the trick. But is it the proper solution?
>
>> The issue I just want to address (discuss) with this RFC patch is that for
>> Renesas RCar3 we have CLK_IS_CRITICAL support for module clocks, but not for
>> core clocks. From my point of view, this is a completely Renesas
>> implementation specific discussion.
>
> Correct. How the Renesas CPG/MSSR driver decides which clocks are marked
> critical is a Renesas-specific implementation issue.
> And indeed, currently the driver only handles critical module clocks
> (more specifically, the GIC module clock, due to the lack of runtime PM
> support in the GIC driver).
> It does not handle critical core clocks, as so far no use case required
> such support.
>
>> So I would rephrase the initial sentence above
>>
>> "Clocks may be in use by another CPU not running Linux, while Linux disables
>> them as being unused."
>>
>> to anything like
>>
>> "Clocks may be enabled (used) by others (U-Boot/CPUs/Hypervisors), while
>> Linux disables them as being unused"
>
> If a clock is used by Linux, Linux must make sure it is enabled when
> needed.
> If a clock is needed to run Linux, without driver support, Linux must still
> make sure it is enabled. This is were critical clocks enter the game.
> The above includes CPU clocks.
>
> If a clock is used by U-Boot, U-Boot must make sure it is enabled when
> needed. Once U-Boot has transferred control to Linux, this no longer
> matters.
>
> if a clock is used by a HV, the HV must make sure it is enabled when
> needed. I believe this is handled in secure mode? R-Car Gen3 SoCs already
> have several module clocks that cannot be disabled (i.e. MSTPSRx != OR of
> all yMSTPCRx).
>
> If a clock is used by the RT CPU, the RT CPU must make sure it is enabled
> when needed.
> There are several options to do that:
> - Secure mode,
> - Communicating the clock requirements to Linux, either statically or
> dynamically,
> - DT? (but this is software policy?)
> - ???
>
>> And I think this is completely addressed by CLK_IS_CRITICAL, as far as I can
>> see :)
>
> While this is a solution that works (for your use case), IMHO it's not the
> right thing to do.
> However, if we decide to go this route, which other clocks should be marked
> critical? Almost (all?) peripherals can be used by the RT CPU. Hence all
> clocks should be enabled and marked critical, unconditionally?
First of all, just the clocks we know (*) are used by the R7 side *and*
are disabled by the kernel (for a practical demo this are e.g. ~7 clocks).
(*) Yes, this is board (and R7 software) related. And yes, this might
result in board specific r8a7795_crit_mod_clks/r8a7795_crit_core_clks[]
tables. So yes, most probably device tree would be an option. While there is
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk.c#n3402
unfortunately
a) its marked as "Do not use this function"
and
b) even if we would ignore (a) we couldn't figure how to use this for
the RCar3 clocks (being no clock expert)
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU
2017-06-29 13:18 ` Dirk Behme
@ 2017-06-29 13:22 ` Geert Uytterhoeven
-1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 13:22 UTC (permalink / raw)
To: Dirk Behme
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
Hi Dirk,
On Thu, Jun 29, 2017 at 3:18 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 29.06.2017 13:18, Geert Uytterhoeven wrote:
>> On Thu, Jun 29, 2017 at 12:28 PM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> On 29.06.2017 11:27, Geert Uytterhoeven wrote:
>>>> TL;DR: Clocks may be in use by another CPU not running Linux, while
>>>> Linux
>>>> disables them as being unused.
>>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex
>>>> R7
>>>> core which may run another OS.
>>>> This is an interesting issue, and relevant to other SoCs, too.
> First of all, just the clocks we know (*) are used by the R7 side *and* are
> disabled by the kernel (for a practical demo this are e.g. ~7 clocks).
>
> (*) Yes, this is board (and R7 software) related. And yes, this might result
> in board specific r8a7795_crit_mod_clks/r8a7795_crit_core_clks[] tables. So
> yes, most probably device tree would be an option. While there is
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk.c#n3402
>
> unfortunately
>
> a) its marked as "Do not use this function"
>
> and
>
> b) even if we would ignore (a) we couldn't figure how to use this for the
> RCar3 clocks (being no clock expert)
You cannot, as it only supports the legacy one-clock-per-node style.
You can use it on e.g. sd2_clk in arch/arm/boot/dts/r8a7791.dtsi, until R-Car
Gen2 is switched to the new CPG/MSSR driver.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU
@ 2017-06-29 13:22 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-06-29 13:22 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dirk,
On Thu, Jun 29, 2017 at 3:18 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 29.06.2017 13:18, Geert Uytterhoeven wrote:
>> On Thu, Jun 29, 2017 at 12:28 PM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> On 29.06.2017 11:27, Geert Uytterhoeven wrote:
>>>> TL;DR: Clocks may be in use by another CPU not running Linux, while
>>>> Linux
>>>> disables them as being unused.
>>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex
>>>> R7
>>>> core which may run another OS.
>>>> This is an interesting issue, and relevant to other SoCs, too.
> First of all, just the clocks we know (*) are used by the R7 side *and* are
> disabled by the kernel (for a practical demo this are e.g. ~7 clocks).
>
> (*) Yes, this is board (and R7 software) related. And yes, this might result
> in board specific r8a7795_crit_mod_clks/r8a7795_crit_core_clks[] tables. So
> yes, most probably device tree would be an option. While there is
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk.c#n3402
>
> unfortunately
>
> a) its marked as "Do not use this function"
>
> and
>
> b) even if we would ignore (a) we couldn't figure how to use this for the
> RCar3 clocks (being no clock expert)
You cannot, as it only supports the legacy one-clock-per-node style.
You can use it on e.g. sd2_clk in arch/arm/boot/dts/r8a7791.dtsi, until R-Car
Gen2 is switched to the new CPG/MSSR driver.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-06-29 9:27 ` Geert Uytterhoeven
@ 2017-06-30 8:02 ` Peter De Schrijver
-1 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-06-30 8:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Dirk Behme, Michael Turquette, Stephen Boyd, Rob Herring,
Mark Rutland, Ulf Hansson, Rafael J. Wysocki, Kevin Hilman,
Linux-Renesas, linux-clk, linux-arm-kernel, devicetree,
Linux PM list
On Thu, Jun 29, 2017 at 11:27:28AM +0200, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
>
There is that but also Linux should not be allowed to change the rate and
parent. Otherwise your R7 sw will likely fail as well. I think it makes sense
to have some DT property which informs linux which clocks it should not touch.
At least assuming clock control isn't moved to a separate coprocessor. In that
case any policy can ofcourse be implemented in the coprocessor.
Peter.
> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> > With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> > CLK_IS_CRITICAL") we are able to handle critical module clocks.
> > Introduce the same logic for critical core clocks.
> >
> > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> > ---
> > Commit
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
> >
> > is quite nice to avoid *module* clocks being disabled. Unfortunately,
> > there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> > core of the r8a7795, the 'canfd' is a quite popular core clock which
> > shouldn't be disabled by Linux.
> >
> > Therefore, this patch is a proposal to use the same 'mark clocks as
> > critical' logic implemented for the module clocks for the core
> > clocks, too.
> >
> > Opinions?
>
> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> core which may run another OS.
> This is an interesting issue, and relevant to other SoCs, too.
>
> In this particular case, the "canfd" clock is a core clock used as an
> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> to three scenarios:
> 1. Linux controls all CAN interfaces
> => no issue,
> 2. The OS on the RT CPU controls all CAN interfaces
> => issue, Linux disables the clock
> 3. Mix of 1 and 2
> => More issues.
> Of course this is not limited to clocks, but also to e.g. PM domains.
>
> How can this be handled?
> I believe just marking the "canfd" clock critical is not the right solution,
> as about any clock could be used by the RT CPU.
>
> Still, Linux needs to be made aware that devices (clocks and PM domains) are
> controlled by another CPU/OS.
>
> Should this be described in DT? It feels like software policy to me.
>
> Note that we (mainline) currently don't describe the Cortex R7 core in DT.
> Dirk: do you describe it?
>
> Summary:
> 1. Core/module clocks are described in the clock driver (not in DT),
> 2. Unused clocks are disabled by CCF,
> 3. Clocks may be in use by the Real-Time CPU core, running another OS,
> 4. How to communicate to Linux which clocks are under control of the RT CPU?
>
> Thanks for your comments!
>
> > drivers/clk/renesas/clk-div6.c | 17 +++++++++++++++--
> > drivers/clk/renesas/clk-div6.h | 4 +++-
> > drivers/clk/renesas/r8a7795-cpg-mssr.c | 7 +++++++
> > drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
> > drivers/clk/renesas/renesas-cpg-mssr.h | 8 ++++++++
> > 5 files changed, 35 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clk/renesas/clk-div6.c b/drivers/clk/renesas/clk-div6.c
> > index 0627860..5917e05 100644
> > --- a/drivers/clk/renesas/clk-div6.c
> > +++ b/drivers/clk/renesas/clk-div6.c
> > @@ -18,6 +18,7 @@
> > #include <linux/of_address.h>
> > #include <linux/slab.h>
> >
> > +#include "renesas-cpg-mssr.h"
> > #include "clk-div6.h"
> >
> > #define CPG_DIV6_CKSTP BIT(8)
> > @@ -184,7 +185,9 @@ static const struct clk_ops cpg_div6_clock_ops = {
> > struct clk * __init cpg_div6_register(const char *name,
> > unsigned int num_parents,
> > const char **parent_names,
> > - void __iomem *reg)
> > + void __iomem *reg,
> > + const struct cpg_mssr_info *info,
> > + unsigned int id)
> > {
> > unsigned int valid_parents;
> > struct clk_init_data init;
> > @@ -246,6 +249,15 @@ struct clk * __init cpg_div6_register(const char *name,
> > init.name = name;
> > init.ops = &cpg_div6_clock_ops;
> > init.flags = CLK_IS_BASIC;
> > + if (info) {
> > + for (i = 0; i < info->num_crit_core_clks; i++)
> > + if (id == info->crit_core_clks[i]) {
> > + pr_devel("DIV6 %s setting CLK_IS_CRITICAL\n",
> > + name);
> > + init.flags |= CLK_IS_CRITICAL;
> > + break;
> > + }
> > + }
> > init.parent_names = parent_names;
> > init.num_parents = valid_parents;
> >
> > @@ -298,7 +310,8 @@ static void __init cpg_div6_clock_init(struct device_node *np)
> > for (i = 0; i < num_parents; i++)
> > parent_names[i] = of_clk_get_parent_name(np, i);
> >
> > - clk = cpg_div6_register(clk_name, num_parents, parent_names, reg);
> > + clk = cpg_div6_register(clk_name, num_parents, parent_names, reg,
> > + NULL, 0);
> > if (IS_ERR(clk)) {
> > pr_err("%s: failed to register %s DIV6 clock (%ld)\n",
> > __func__, np->name, PTR_ERR(clk));
> > diff --git a/drivers/clk/renesas/clk-div6.h b/drivers/clk/renesas/clk-div6.h
> > index 567b31d..b619d6b4 100644
> > --- a/drivers/clk/renesas/clk-div6.h
> > +++ b/drivers/clk/renesas/clk-div6.h
> > @@ -2,6 +2,8 @@
> > #define __RENESAS_CLK_DIV6_H__
> >
> > struct clk *cpg_div6_register(const char *name, unsigned int num_parents,
> > - const char **parent_names, void __iomem *reg);
> > + const char **parent_names, void __iomem *reg,
> > + const struct cpg_mssr_info *info,
> > + unsigned int id);
> >
> > #endif
> > diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > index eaa98b4..a54fed6 100644
> > --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > @@ -114,6 +114,9 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
> > DEF_BASE("r", R8A7795_CLK_R, CLK_TYPE_GEN3_R, CLK_RINT),
> > };
> >
> > +static const unsigned int r8a7795_crit_core_clks[] __initconst = {
> > +};
> > +
> > static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
> > DEF_MOD("fdp1-2", 117, R8A7795_CLK_S2D1), /* ES1.x */
> > DEF_MOD("fdp1-1", 118, R8A7795_CLK_S0D1),
> > @@ -441,6 +444,10 @@ const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
> > .last_dt_core_clk = LAST_DT_CORE_CLK,
> > .num_total_core_clks = MOD_CLK_BASE,
> >
> > + /* Critical Core Clocks */
> > + .crit_core_clks = r8a7795_crit_core_clks,
> > + .num_crit_core_clks = ARRAY_SIZE(r8a7795_crit_core_clks),
> > +
> > /* Module Clocks */
> > .mod_clks = r8a7795_mod_clks,
> > .num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks),
> > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> > index 99eeec6..80be019 100644
> > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> > @@ -293,7 +293,8 @@ static void __init cpg_mssr_register_core_clk(const struct cpg_core_clk *core,
> >
> > if (core->type == CLK_TYPE_DIV6P1) {
> > clk = cpg_div6_register(core->name, 1, &parent_name,
> > - priv->base + core->offset);
> > + priv->base + core->offset, info,
> > + core->id);
> > } else {
> > clk = clk_register_fixed_factor(NULL, core->name,
> > parent_name, 0,
> > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
> > index 148f4f0a..a723fdd 100644
> > --- a/drivers/clk/renesas/renesas-cpg-mssr.h
> > +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
> > @@ -86,6 +86,10 @@ struct device_node;
> > * @last_dt_core_clk: ID of the last Core Clock exported to DT
> > * @num_total_core_clks: Total number of Core Clocks (exported + internal)
> > *
> > + * @crit_core_clks: Array with Core Clock IDs of critical clocks that
> > + * should not be disabled without a knowledgeable driver
> > + * @num_core_mod_clks: Number of entries in crit_core_clks[]
> > + *
> > * @mod_clks: Array of Module Clock definitions
> > * @num_mod_clks: Number of entries in mod_clks[]
> > * @num_hw_mod_clks: Number of Module Clocks supported by the hardware
> > @@ -109,6 +113,10 @@ struct cpg_mssr_info {
> > unsigned int last_dt_core_clk;
> > unsigned int num_total_core_clks;
> >
> > + /* Critical Core Clocks that should not be disabled */
> > + const unsigned int *crit_core_clks;
> > + unsigned int num_crit_core_clks;
> > +
> > /* Module Clocks */
> > const struct mssr_mod_clk *mod_clks;
> > unsigned int num_mod_clks;
> > --
> > 2.8.0
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-30 8:02 ` Peter De Schrijver
0 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-06-30 8:02 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 29, 2017 at 11:27:28AM +0200, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
>
There is that but also Linux should not be allowed to change the rate and
parent. Otherwise your R7 sw will likely fail as well. I think it makes sense
to have some DT property which informs linux which clocks it should not touch.
At least assuming clock control isn't moved to a separate coprocessor. In that
case any policy can ofcourse be implemented in the coprocessor.
Peter.
> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> > With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> > CLK_IS_CRITICAL") we are able to handle critical module clocks.
> > Introduce the same logic for critical core clocks.
> >
> > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> > ---
> > Commit
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
> >
> > is quite nice to avoid *module* clocks being disabled. Unfortunately,
> > there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> > core of the r8a7795, the 'canfd' is a quite popular core clock which
> > shouldn't be disabled by Linux.
> >
> > Therefore, this patch is a proposal to use the same 'mark clocks as
> > critical' logic implemented for the module clocks for the core
> > clocks, too.
> >
> > Opinions?
>
> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> core which may run another OS.
> This is an interesting issue, and relevant to other SoCs, too.
>
> In this particular case, the "canfd" clock is a core clock used as an
> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> to three scenarios:
> 1. Linux controls all CAN interfaces
> => no issue,
> 2. The OS on the RT CPU controls all CAN interfaces
> => issue, Linux disables the clock
> 3. Mix of 1 and 2
> => More issues.
> Of course this is not limited to clocks, but also to e.g. PM domains.
>
> How can this be handled?
> I believe just marking the "canfd" clock critical is not the right solution,
> as about any clock could be used by the RT CPU.
>
> Still, Linux needs to be made aware that devices (clocks and PM domains) are
> controlled by another CPU/OS.
>
> Should this be described in DT? It feels like software policy to me.
>
> Note that we (mainline) currently don't describe the Cortex R7 core in DT.
> Dirk: do you describe it?
>
> Summary:
> 1. Core/module clocks are described in the clock driver (not in DT),
> 2. Unused clocks are disabled by CCF,
> 3. Clocks may be in use by the Real-Time CPU core, running another OS,
> 4. How to communicate to Linux which clocks are under control of the RT CPU?
>
> Thanks for your comments!
>
> > drivers/clk/renesas/clk-div6.c | 17 +++++++++++++++--
> > drivers/clk/renesas/clk-div6.h | 4 +++-
> > drivers/clk/renesas/r8a7795-cpg-mssr.c | 7 +++++++
> > drivers/clk/renesas/renesas-cpg-mssr.c | 3 ++-
> > drivers/clk/renesas/renesas-cpg-mssr.h | 8 ++++++++
> > 5 files changed, 35 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clk/renesas/clk-div6.c b/drivers/clk/renesas/clk-div6.c
> > index 0627860..5917e05 100644
> > --- a/drivers/clk/renesas/clk-div6.c
> > +++ b/drivers/clk/renesas/clk-div6.c
> > @@ -18,6 +18,7 @@
> > #include <linux/of_address.h>
> > #include <linux/slab.h>
> >
> > +#include "renesas-cpg-mssr.h"
> > #include "clk-div6.h"
> >
> > #define CPG_DIV6_CKSTP BIT(8)
> > @@ -184,7 +185,9 @@ static const struct clk_ops cpg_div6_clock_ops = {
> > struct clk * __init cpg_div6_register(const char *name,
> > unsigned int num_parents,
> > const char **parent_names,
> > - void __iomem *reg)
> > + void __iomem *reg,
> > + const struct cpg_mssr_info *info,
> > + unsigned int id)
> > {
> > unsigned int valid_parents;
> > struct clk_init_data init;
> > @@ -246,6 +249,15 @@ struct clk * __init cpg_div6_register(const char *name,
> > init.name = name;
> > init.ops = &cpg_div6_clock_ops;
> > init.flags = CLK_IS_BASIC;
> > + if (info) {
> > + for (i = 0; i < info->num_crit_core_clks; i++)
> > + if (id == info->crit_core_clks[i]) {
> > + pr_devel("DIV6 %s setting CLK_IS_CRITICAL\n",
> > + name);
> > + init.flags |= CLK_IS_CRITICAL;
> > + break;
> > + }
> > + }
> > init.parent_names = parent_names;
> > init.num_parents = valid_parents;
> >
> > @@ -298,7 +310,8 @@ static void __init cpg_div6_clock_init(struct device_node *np)
> > for (i = 0; i < num_parents; i++)
> > parent_names[i] = of_clk_get_parent_name(np, i);
> >
> > - clk = cpg_div6_register(clk_name, num_parents, parent_names, reg);
> > + clk = cpg_div6_register(clk_name, num_parents, parent_names, reg,
> > + NULL, 0);
> > if (IS_ERR(clk)) {
> > pr_err("%s: failed to register %s DIV6 clock (%ld)\n",
> > __func__, np->name, PTR_ERR(clk));
> > diff --git a/drivers/clk/renesas/clk-div6.h b/drivers/clk/renesas/clk-div6.h
> > index 567b31d..b619d6b4 100644
> > --- a/drivers/clk/renesas/clk-div6.h
> > +++ b/drivers/clk/renesas/clk-div6.h
> > @@ -2,6 +2,8 @@
> > #define __RENESAS_CLK_DIV6_H__
> >
> > struct clk *cpg_div6_register(const char *name, unsigned int num_parents,
> > - const char **parent_names, void __iomem *reg);
> > + const char **parent_names, void __iomem *reg,
> > + const struct cpg_mssr_info *info,
> > + unsigned int id);
> >
> > #endif
> > diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > index eaa98b4..a54fed6 100644
> > --- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > +++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
> > @@ -114,6 +114,9 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
> > DEF_BASE("r", R8A7795_CLK_R, CLK_TYPE_GEN3_R, CLK_RINT),
> > };
> >
> > +static const unsigned int r8a7795_crit_core_clks[] __initconst = {
> > +};
> > +
> > static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
> > DEF_MOD("fdp1-2", 117, R8A7795_CLK_S2D1), /* ES1.x */
> > DEF_MOD("fdp1-1", 118, R8A7795_CLK_S0D1),
> > @@ -441,6 +444,10 @@ const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
> > .last_dt_core_clk = LAST_DT_CORE_CLK,
> > .num_total_core_clks = MOD_CLK_BASE,
> >
> > + /* Critical Core Clocks */
> > + .crit_core_clks = r8a7795_crit_core_clks,
> > + .num_crit_core_clks = ARRAY_SIZE(r8a7795_crit_core_clks),
> > +
> > /* Module Clocks */
> > .mod_clks = r8a7795_mod_clks,
> > .num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks),
> > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> > index 99eeec6..80be019 100644
> > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> > @@ -293,7 +293,8 @@ static void __init cpg_mssr_register_core_clk(const struct cpg_core_clk *core,
> >
> > if (core->type == CLK_TYPE_DIV6P1) {
> > clk = cpg_div6_register(core->name, 1, &parent_name,
> > - priv->base + core->offset);
> > + priv->base + core->offset, info,
> > + core->id);
> > } else {
> > clk = clk_register_fixed_factor(NULL, core->name,
> > parent_name, 0,
> > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
> > index 148f4f0a..a723fdd 100644
> > --- a/drivers/clk/renesas/renesas-cpg-mssr.h
> > +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
> > @@ -86,6 +86,10 @@ struct device_node;
> > * @last_dt_core_clk: ID of the last Core Clock exported to DT
> > * @num_total_core_clks: Total number of Core Clocks (exported + internal)
> > *
> > + * @crit_core_clks: Array with Core Clock IDs of critical clocks that
> > + * should not be disabled without a knowledgeable driver
> > + * @num_core_mod_clks: Number of entries in crit_core_clks[]
> > + *
> > * @mod_clks: Array of Module Clock definitions
> > * @num_mod_clks: Number of entries in mod_clks[]
> > * @num_hw_mod_clks: Number of Module Clocks supported by the hardware
> > @@ -109,6 +113,10 @@ struct cpg_mssr_info {
> > unsigned int last_dt_core_clk;
> > unsigned int num_total_core_clks;
> >
> > + /* Critical Core Clocks that should not be disabled */
> > + const unsigned int *crit_core_clks;
> > + unsigned int num_crit_core_clks;
> > +
> > /* Module Clocks */
> > const struct mssr_mod_clk *mod_clks;
> > unsigned int num_mod_clks;
> > --
> > 2.8.0
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
>
> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-06-29 9:27 ` Geert Uytterhoeven
@ 2017-06-30 15:58 ` Rob Herring
-1 siblings, 0 replies; 54+ messages in thread
From: Rob Herring @ 2017-06-30 15:58 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Dirk Behme, Michael Turquette, Stephen Boyd, Mark Rutland,
Ulf Hansson, Rafael J. Wysocki, Kevin Hilman, Linux-Renesas,
linux-clk, linux-arm-kernel, devicetree, Linux PM list
On Thu, Jun 29, 2017 at 4:27 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Dirk,
>
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
>
> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>> Introduce the same logic for critical core clocks.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>> Commit
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>
>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>> shouldn't be disabled by Linux.
>>
>> Therefore, this patch is a proposal to use the same 'mark clocks as
>> critical' logic implemented for the module clocks for the core
>> clocks, too.
>>
>> Opinions?
>
> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> core which may run another OS.
> This is an interesting issue, and relevant to other SoCs, too.
>
> In this particular case, the "canfd" clock is a core clock used as an
> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> to three scenarios:
> 1. Linux controls all CAN interfaces
> => no issue,
> 2. The OS on the RT CPU controls all CAN interfaces
> => issue, Linux disables the clock
> 3. Mix of 1 and 2
> => More issues.
> Of course this is not limited to clocks, but also to e.g. PM domains.
>
> How can this be handled?
> I believe just marking the "canfd" clock critical is not the right solution,
> as about any clock could be used by the RT CPU.
>
> Still, Linux needs to be made aware that devices (clocks and PM domains) are
> controlled by another CPU/OS.
>
> Should this be described in DT? It feels like software policy to me.
No, it shouldn't. It is Linux policy to disable all unused clocks, so
Linux gets to deal with the consequences.
Rob
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-30 15:58 ` Rob Herring
0 siblings, 0 replies; 54+ messages in thread
From: Rob Herring @ 2017-06-30 15:58 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 29, 2017 at 4:27 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Dirk,
>
> CC clock, ARM, DT, PM people
>
> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> disables them as being unused.
>
> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>> Introduce the same logic for critical core clocks.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>> Commit
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>
>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>> shouldn't be disabled by Linux.
>>
>> Therefore, this patch is a proposal to use the same 'mark clocks as
>> critical' logic implemented for the module clocks for the core
>> clocks, too.
>>
>> Opinions?
>
> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> core which may run another OS.
> This is an interesting issue, and relevant to other SoCs, too.
>
> In this particular case, the "canfd" clock is a core clock used as an
> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> to three scenarios:
> 1. Linux controls all CAN interfaces
> => no issue,
> 2. The OS on the RT CPU controls all CAN interfaces
> => issue, Linux disables the clock
> 3. Mix of 1 and 2
> => More issues.
> Of course this is not limited to clocks, but also to e.g. PM domains.
>
> How can this be handled?
> I believe just marking the "canfd" clock critical is not the right solution,
> as about any clock could be used by the RT CPU.
>
> Still, Linux needs to be made aware that devices (clocks and PM domains) are
> controlled by another CPU/OS.
>
> Should this be described in DT? It feels like software policy to me.
No, it shouldn't. It is Linux policy to disable all unused clocks, so
Linux gets to deal with the consequences.
Rob
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-06-30 15:58 ` Rob Herring
(?)
@ 2017-06-30 20:24 ` Uwe Kleine-König
-1 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-06-30 20:24 UTC (permalink / raw)
To: Rob Herring
Cc: Geert Uytterhoeven, Mark Rutland, devicetree, Ulf Hansson,
Dirk Behme, Linux PM list, Stephen Boyd, Michael Turquette,
Rafael J. Wysocki, Linux-Renesas, Kevin Hilman, linux-clk,
linux-arm-kernel
Hello,
On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
> > TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> > disables them as being unused.
not long ago I thought with a few colleagues about this. The scenario is
to start a Linux kernel on a Cortex-M companion to a Cortex-A.
> > On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> >> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> >> CLK_IS_CRITICAL") we are able to handle critical module clocks.
> >> Introduce the same logic for critical core clocks.
> >>
> >> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> >> ---
> >> Commit
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
> >>
> >> is quite nice to avoid *module* clocks being disabled. Unfortunately,
> >> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> >> core of the r8a7795, the 'canfd' is a quite popular core clock which
> >> shouldn't be disabled by Linux.
> >>
> >> Therefore, this patch is a proposal to use the same 'mark clocks as
> >> critical' logic implemented for the module clocks for the core
> >> clocks, too.
> >>
> >> Opinions?
> >
> > On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> > core which may run another OS.
> > This is an interesting issue, and relevant to other SoCs, too.
> >
> > In this particular case, the "canfd" clock is a core clock used as an
> > auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> > to three scenarios:
> > 1. Linux controls all CAN interfaces
> > => no issue,
> > 2. The OS on the RT CPU controls all CAN interfaces
> > => issue, Linux disables the clock
> > 3. Mix of 1 and 2
> > => More issues.
> > Of course this is not limited to clocks, but also to e.g. PM domains.
> >
> > How can this be handled?
> > I believe just marking the "canfd" clock critical is not the right solution,
> > as about any clock could be used by the RT CPU.
> >
> > Still, Linux needs to be made aware that devices (clocks and PM domains) are
> > controlled by another CPU/OS.
> >
> > Should this be described in DT? It feels like software policy to me.
>
> No, it shouldn't. It is Linux policy to disable all unused clocks, so
> Linux gets to deal with the consequences.
The ideal solution I imagine is to make the other CPU's OS a consumer of
the Linux clock driver. This would require a generic device driver on the
companion CPU that forwards clk requests via inter-cpu communication to
the Linux clk driver. It could be feed with the necessary information by
the rproc glue. So when the companion cpu is supposed to care for the
can0 device, the steps that should happen are:
- make sure can0 isn't occupied by the Linux Host
- reroute the can irq to the companion cpu (if necessary)
- create a dtb containing something like this for the companion CPU:
clks: virtclk {
compatible = ???
#clock-cells = <1>;
...
};
can@$address {
compatible = ...
regs = ...
clocks = <&clks 3>;
clock-names = ...
...
};
where the driver binding to the virtclk device just forwards clk
requests to the Linux host side which then knows that clk 3 is the
can clock and does the necessary stuff.
This way the can clock doesn't need special handling in the host's dtb
and no clock necessary for the companion is disabled as unused because
it is requested and enabled.
The only problem I see is that implementing such a driver/protocol
probably is time consuming.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-30 20:24 ` Uwe Kleine-König
0 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-06-30 20:24 UTC (permalink / raw)
To: Rob Herring
Cc: Geert Uytterhoeven, Mark Rutland, devicetree, Ulf Hansson,
Dirk Behme, Linux PM list, Stephen Boyd, Michael Turquette,
Rafael J. Wysocki, Linux-Renesas, Kevin Hilman, linux-clk,
linux-arm-kernel
Hello,
On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
> > TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> > disables them as being unused.
not long ago I thought with a few colleagues about this. The scenario is
to start a Linux kernel on a Cortex-M companion to a Cortex-A.
> > On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> >> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> >> CLK_IS_CRITICAL") we are able to handle critical module clocks.
> >> Introduce the same logic for critical core clocks.
> >>
> >> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> >> ---
> >> Commit
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
> >>
> >> is quite nice to avoid *module* clocks being disabled. Unfortunately,
> >> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> >> core of the r8a7795, the 'canfd' is a quite popular core clock which
> >> shouldn't be disabled by Linux.
> >>
> >> Therefore, this patch is a proposal to use the same 'mark clocks as
> >> critical' logic implemented for the module clocks for the core
> >> clocks, too.
> >>
> >> Opinions?
> >
> > On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> > core which may run another OS.
> > This is an interesting issue, and relevant to other SoCs, too.
> >
> > In this particular case, the "canfd" clock is a core clock used as an
> > auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> > to three scenarios:
> > 1. Linux controls all CAN interfaces
> > => no issue,
> > 2. The OS on the RT CPU controls all CAN interfaces
> > => issue, Linux disables the clock
> > 3. Mix of 1 and 2
> > => More issues.
> > Of course this is not limited to clocks, but also to e.g. PM domains.
> >
> > How can this be handled?
> > I believe just marking the "canfd" clock critical is not the right solution,
> > as about any clock could be used by the RT CPU.
> >
> > Still, Linux needs to be made aware that devices (clocks and PM domains) are
> > controlled by another CPU/OS.
> >
> > Should this be described in DT? It feels like software policy to me.
>
> No, it shouldn't. It is Linux policy to disable all unused clocks, so
> Linux gets to deal with the consequences.
The ideal solution I imagine is to make the other CPU's OS a consumer of
the Linux clock driver. This would require a generic device driver on the
companion CPU that forwards clk requests via inter-cpu communication to
the Linux clk driver. It could be feed with the necessary information by
the rproc glue. So when the companion cpu is supposed to care for the
can0 device, the steps that should happen are:
- make sure can0 isn't occupied by the Linux Host
- reroute the can irq to the companion cpu (if necessary)
- create a dtb containing something like this for the companion CPU:
clks: virtclk {
compatible = ???
#clock-cells = <1>;
...
};
can@$address {
compatible = ...
regs = ...
clocks = <&clks 3>;
clock-names = ...
...
};
where the driver binding to the virtclk device just forwards clk
requests to the Linux host side which then knows that clk 3 is the
can clock and does the necessary stuff.
This way the can clock doesn't need special handling in the host's dtb
and no clock necessary for the companion is disabled as unused because
it is requested and enabled.
The only problem I see is that implementing such a driver/protocol
probably is time consuming.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K�nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-06-30 20:24 ` Uwe Kleine-König
0 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-06-30 20:24 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
> > TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> > disables them as being unused.
not long ago I thought with a few colleagues about this. The scenario is
to start a Linux kernel on a Cortex-M companion to a Cortex-A.
> > On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> >> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> >> CLK_IS_CRITICAL") we are able to handle critical module clocks.
> >> Introduce the same logic for critical core clocks.
> >>
> >> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> >> ---
> >> Commit
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
> >>
> >> is quite nice to avoid *module* clocks being disabled. Unfortunately,
> >> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> >> core of the r8a7795, the 'canfd' is a quite popular core clock which
> >> shouldn't be disabled by Linux.
> >>
> >> Therefore, this patch is a proposal to use the same 'mark clocks as
> >> critical' logic implemented for the module clocks for the core
> >> clocks, too.
> >>
> >> Opinions?
> >
> > On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> > core which may run another OS.
> > This is an interesting issue, and relevant to other SoCs, too.
> >
> > In this particular case, the "canfd" clock is a core clock used as an
> > auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> > to three scenarios:
> > 1. Linux controls all CAN interfaces
> > => no issue,
> > 2. The OS on the RT CPU controls all CAN interfaces
> > => issue, Linux disables the clock
> > 3. Mix of 1 and 2
> > => More issues.
> > Of course this is not limited to clocks, but also to e.g. PM domains.
> >
> > How can this be handled?
> > I believe just marking the "canfd" clock critical is not the right solution,
> > as about any clock could be used by the RT CPU.
> >
> > Still, Linux needs to be made aware that devices (clocks and PM domains) are
> > controlled by another CPU/OS.
> >
> > Should this be described in DT? It feels like software policy to me.
>
> No, it shouldn't. It is Linux policy to disable all unused clocks, so
> Linux gets to deal with the consequences.
The ideal solution I imagine is to make the other CPU's OS a consumer of
the Linux clock driver. This would require a generic device driver on the
companion CPU that forwards clk requests via inter-cpu communication to
the Linux clk driver. It could be feed with the necessary information by
the rproc glue. So when the companion cpu is supposed to care for the
can0 device, the steps that should happen are:
- make sure can0 isn't occupied by the Linux Host
- reroute the can irq to the companion cpu (if necessary)
- create a dtb containing something like this for the companion CPU:
clks: virtclk {
compatible = ???
#clock-cells = <1>;
...
};
can@$address {
compatible = ...
regs = ...
clocks = <&clks 3>;
clock-names = ...
...
};
where the driver binding to the virtclk device just forwards clk
requests to the Linux host side which then knows that clk 3 is the
can clock and does the necessary stuff.
This way the can clock doesn't need special handling in the host's dtb
and no clock necessary for the companion is disabled as unused because
it is requested and enabled.
The only problem I see is that implementing such a driver/protocol
probably is time consuming.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-06-30 20:24 ` Uwe Kleine-König
(?)
@ 2017-07-01 5:02 ` Dirk Behme
-1 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-07-01 5:02 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring
Cc: Geert Uytterhoeven, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson, Dirk Behme,
Linux PM list, Stephen Boyd, Michael Turquette,
Rafael J. Wysocki, Linux-Renesas, Kevin Hilman, linux-clk,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 30.06.2017 22:24, Uwe Kleine-König wrote:
> Hello,
>
> On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>> disables them as being unused.
>
> not long ago I thought with a few colleagues about this. The scenario is
> to start a Linux kernel on a Cortex-M companion to a Cortex-A.
>
>>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org> wrote:
>>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>>> Introduce the same logic for critical core clocks.
>>>>
>>>> Signed-off-by: Dirk Behme <dirk.behme-V5te9oGctAVWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>> Commit
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>>
>>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>>> shouldn't be disabled by Linux.
>>>>
>>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>>> critical' logic implemented for the module clocks for the core
>>>> clocks, too.
>>>>
>>>> Opinions?
>>>
>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
>>> core which may run another OS.
>>> This is an interesting issue, and relevant to other SoCs, too.
>>>
>>> In this particular case, the "canfd" clock is a core clock used as an
>>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>>> to three scenarios:
>>> 1. Linux controls all CAN interfaces
>>> => no issue,
>>> 2. The OS on the RT CPU controls all CAN interfaces
>>> => issue, Linux disables the clock
>>> 3. Mix of 1 and 2
>>> => More issues.
>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>
>>> How can this be handled?
>>> I believe just marking the "canfd" clock critical is not the right solution,
>>> as about any clock could be used by the RT CPU.
>>>
>>> Still, Linux needs to be made aware that devices (clocks and PM domains) are
>>> controlled by another CPU/OS.
>>>
>>> Should this be described in DT? It feels like software policy to me.
>>
>> No, it shouldn't. It is Linux policy to disable all unused clocks, so
>> Linux gets to deal with the consequences.
>
> The ideal solution I imagine is to make the other CPU's OS a consumer of
> the Linux clock driver. This would require a generic device driver on the
> companion CPU that forwards clk requests via inter-cpu communication to
> the Linux clk driver. It could be feed with the necessary information by
> the rproc glue. So when the companion cpu is supposed to care for the
> can0 device, the steps that should happen are:
>
> - make sure can0 isn't occupied by the Linux Host
> - reroute the can irq to the companion cpu (if necessary)
> - create a dtb containing something like this for the companion CPU:
>
> clks: virtclk {
> compatible = ???
> #clock-cells = <1>;
> ...
> };
>
> can@$address {
> compatible = ...
> regs = ...
> clocks = <&clks 3>;
> clock-names = ...
> ...
> };
>
> where the driver binding to the virtclk device just forwards clk
> requests to the Linux host side which then knows that clk 3 is the
> can clock and does the necessary stuff.
>
> This way the can clock doesn't need special handling in the host's dtb
> and no clock necessary for the companion is disabled as unused because
> it is requested and enabled.
>
> The only problem I see is that implementing such a driver/protocol
> probably is time consuming.
The other problem is security related. If, at all, you have to do it
the other way around, then:
Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
secured) OS clock driver.
Best regards
Dirk
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-01 5:02 ` Dirk Behme
0 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-07-01 5:02 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring
Cc: Geert Uytterhoeven, Mark Rutland, devicetree, Ulf Hansson,
Dirk Behme, Linux PM list, Stephen Boyd, Michael Turquette,
Rafael J. Wysocki, Linux-Renesas, Kevin Hilman, linux-clk,
linux-arm-kernel
On 30.06.2017 22:24, Uwe Kleine-König wrote:
> Hello,
>
> On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>> disables them as being unused.
>
> not long ago I thought with a few colleagues about this. The scenario is
> to start a Linux kernel on a Cortex-M companion to a Cortex-A.
>
>>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>>> Introduce the same logic for critical core clocks.
>>>>
>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>> ---
>>>> Commit
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>>
>>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>>> shouldn't be disabled by Linux.
>>>>
>>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>>> critical' logic implemented for the module clocks for the core
>>>> clocks, too.
>>>>
>>>> Opinions?
>>>
>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
>>> core which may run another OS.
>>> This is an interesting issue, and relevant to other SoCs, too.
>>>
>>> In this particular case, the "canfd" clock is a core clock used as an
>>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>>> to three scenarios:
>>> 1. Linux controls all CAN interfaces
>>> => no issue,
>>> 2. The OS on the RT CPU controls all CAN interfaces
>>> => issue, Linux disables the clock
>>> 3. Mix of 1 and 2
>>> => More issues.
>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>
>>> How can this be handled?
>>> I believe just marking the "canfd" clock critical is not the right solution,
>>> as about any clock could be used by the RT CPU.
>>>
>>> Still, Linux needs to be made aware that devices (clocks and PM domains) are
>>> controlled by another CPU/OS.
>>>
>>> Should this be described in DT? It feels like software policy to me.
>>
>> No, it shouldn't. It is Linux policy to disable all unused clocks, so
>> Linux gets to deal with the consequences.
>
> The ideal solution I imagine is to make the other CPU's OS a consumer of
> the Linux clock driver. This would require a generic device driver on the
> companion CPU that forwards clk requests via inter-cpu communication to
> the Linux clk driver. It could be feed with the necessary information by
> the rproc glue. So when the companion cpu is supposed to care for the
> can0 device, the steps that should happen are:
>
> - make sure can0 isn't occupied by the Linux Host
> - reroute the can irq to the companion cpu (if necessary)
> - create a dtb containing something like this for the companion CPU:
>
> clks: virtclk {
> compatible = ???
> #clock-cells = <1>;
> ...
> };
>
> can@$address {
> compatible = ...
> regs = ...
> clocks = <&clks 3>;
> clock-names = ...
> ...
> };
>
> where the driver binding to the virtclk device just forwards clk
> requests to the Linux host side which then knows that clk 3 is the
> can clock and does the necessary stuff.
>
> This way the can clock doesn't need special handling in the host's dtb
> and no clock necessary for the companion is disabled as unused because
> it is requested and enabled.
>
> The only problem I see is that implementing such a driver/protocol
> probably is time consuming.
The other problem is security related. If, at all, you have to do it
the other way around, then:
Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
secured) OS clock driver.
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-01 5:02 ` Dirk Behme
0 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-07-01 5:02 UTC (permalink / raw)
To: linux-arm-kernel
On 30.06.2017 22:24, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>> disables them as being unused.
>
> not long ago I thought with a few colleagues about this. The scenario is
> to start a Linux kernel on a Cortex-M companion to a Cortex-A.
>
>>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>>> Introduce the same logic for critical core clocks.
>>>>
>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>> ---
>>>> Commit
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>>
>>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>>> shouldn't be disabled by Linux.
>>>>
>>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>>> critical' logic implemented for the module clocks for the core
>>>> clocks, too.
>>>>
>>>> Opinions?
>>>
>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
>>> core which may run another OS.
>>> This is an interesting issue, and relevant to other SoCs, too.
>>>
>>> In this particular case, the "canfd" clock is a core clock used as an
>>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>>> to three scenarios:
>>> 1. Linux controls all CAN interfaces
>>> => no issue,
>>> 2. The OS on the RT CPU controls all CAN interfaces
>>> => issue, Linux disables the clock
>>> 3. Mix of 1 and 2
>>> => More issues.
>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>
>>> How can this be handled?
>>> I believe just marking the "canfd" clock critical is not the right solution,
>>> as about any clock could be used by the RT CPU.
>>>
>>> Still, Linux needs to be made aware that devices (clocks and PM domains) are
>>> controlled by another CPU/OS.
>>>
>>> Should this be described in DT? It feels like software policy to me.
>>
>> No, it shouldn't. It is Linux policy to disable all unused clocks, so
>> Linux gets to deal with the consequences.
>
> The ideal solution I imagine is to make the other CPU's OS a consumer of
> the Linux clock driver. This would require a generic device driver on the
> companion CPU that forwards clk requests via inter-cpu communication to
> the Linux clk driver. It could be feed with the necessary information by
> the rproc glue. So when the companion cpu is supposed to care for the
> can0 device, the steps that should happen are:
>
> - make sure can0 isn't occupied by the Linux Host
> - reroute the can irq to the companion cpu (if necessary)
> - create a dtb containing something like this for the companion CPU:
>
> clks: virtclk {
> compatible = ???
> #clock-cells = <1>;
> ...
> };
>
> can@$address {
> compatible = ...
> regs = ...
> clocks = <&clks 3>;
> clock-names = ...
> ...
> };
>
> where the driver binding to the virtclk device just forwards clk
> requests to the Linux host side which then knows that clk 3 is the
> can clock and does the necessary stuff.
>
> This way the can clock doesn't need special handling in the host's dtb
> and no clock necessary for the companion is disabled as unused because
> it is requested and enabled.
>
> The only problem I see is that implementing such a driver/protocol
> probably is time consuming.
The other problem is security related. If, at all, you have to do it
the other way around, then:
Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
secured) OS clock driver.
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-07-01 5:02 ` Dirk Behme
(?)
@ 2017-07-01 18:14 ` Uwe Kleine-König
-1 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-07-01 18:14 UTC (permalink / raw)
To: Dirk Behme
Cc: Rob Herring, Mark Rutland, devicetree, Ulf Hansson, Dirk Behme,
Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Geert Uytterhoeven, Kevin Hilman,
linux-clk, linux-arm-kernel
Hello,
On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
> On 30.06.2017 22:24, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
> > > > TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> > > > disables them as being unused.
> >
> > not long ago I thought with a few colleagues about this. The scenario is
> > to start a Linux kernel on a Cortex-M companion to a Cortex-A.
> >
> > > > On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> > > > > With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> > > > > CLK_IS_CRITICAL") we are able to handle critical module clocks.
> > > > > Introduce the same logic for critical core clocks.
> > > > >
> > > > > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> > > > > ---
> > > > > Commit
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
> > > > >
> > > > > is quite nice to avoid *module* clocks being disabled. Unfortunately,
> > > > > there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> > > > > core of the r8a7795, the 'canfd' is a quite popular core clock which
> > > > > shouldn't be disabled by Linux.
> > > > >
> > > > > Therefore, this patch is a proposal to use the same 'mark clocks as
> > > > > critical' logic implemented for the module clocks for the core
> > > > > clocks, too.
> > > > >
> > > > > Opinions?
> > > >
> > > > On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> > > > core which may run another OS.
> > > > This is an interesting issue, and relevant to other SoCs, too.
> > > >
> > > > In this particular case, the "canfd" clock is a core clock used as an
> > > > auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> > > > to three scenarios:
> > > > 1. Linux controls all CAN interfaces
> > > > => no issue,
> > > > 2. The OS on the RT CPU controls all CAN interfaces
> > > > => issue, Linux disables the clock
> > > > 3. Mix of 1 and 2
> > > > => More issues.
> > > > Of course this is not limited to clocks, but also to e.g. PM domains.
> > > >
> > > > How can this be handled?
> > > > I believe just marking the "canfd" clock critical is not the right solution,
> > > > as about any clock could be used by the RT CPU.
> > > >
> > > > Still, Linux needs to be made aware that devices (clocks and PM domains) are
> > > > controlled by another CPU/OS.
> > > >
> > > > Should this be described in DT? It feels like software policy to me.
> > >
> > > No, it shouldn't. It is Linux policy to disable all unused clocks, so
> > > Linux gets to deal with the consequences.
> >
> > The ideal solution I imagine is to make the other CPU's OS a consumer of
> > the Linux clock driver. This would require a generic device driver on the
> > companion CPU that forwards clk requests via inter-cpu communication to
> > the Linux clk driver. It could be feed with the necessary information by
> > the rproc glue. So when the companion cpu is supposed to care for the
> > can0 device, the steps that should happen are:
> >
> > - make sure can0 isn't occupied by the Linux Host
> > - reroute the can irq to the companion cpu (if necessary)
> > - create a dtb containing something like this for the companion CPU:
> >
> > clks: virtclk {
> > compatible = ???
> > #clock-cells = <1>;
> > ...
> > };
> >
> > can@$address {
> > compatible = ...
> > regs = ...
> > clocks = <&clks 3>;
> > clock-names = ...
> > ...
> > };
> >
> > where the driver binding to the virtclk device just forwards clk
> > requests to the Linux host side which then knows that clk 3 is the
> > can clock and does the necessary stuff.
> >
> > This way the can clock doesn't need special handling in the host's dtb
> > and no clock necessary for the companion is disabled as unused because
> > it is requested and enabled.
> >
> > The only problem I see is that implementing such a driver/protocol
> > probably is time consuming.
>
>
> The other problem is security related. If, at all, you have to do it the
> other way around, then:
>
> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> secured) OS clock driver.
That doesn't matter much. Either way the first CPU has to provide the
master side of this device (as it needs clks for booting up) and the 2nd
gets this virtual clk device that forwards clk requests to the first
CPU.
On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
started by the bootrom. If I want the M4 being the primary device I'd
need support in the bootloader to wait long enough (i.e. until the M4 is
up) before letting the A9 jump into Linux. Managable I'd say. This way
would even make sense if the M4 runs a rt critical OS that shouldn't be
forced to wait on the non-rt A9 to enable a clk.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-01 18:14 ` Uwe Kleine-König
0 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-07-01 18:14 UTC (permalink / raw)
To: Dirk Behme
Cc: Rob Herring, Mark Rutland, devicetree, Ulf Hansson, Dirk Behme,
Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Geert Uytterhoeven, Kevin Hilman,
linux-clk, linux-arm-kernel
Hello,
On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
> On 30.06.2017 22:24, Uwe Kleine-K�nig wrote:
> > Hello,
> >
> > On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
> > > > TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> > > > disables them as being unused.
> >
> > not long ago I thought with a few colleagues about this. The scenario is
> > to start a Linux kernel on a Cortex-M companion to a Cortex-A.
> >
> > > > On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> > > > > With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> > > > > CLK_IS_CRITICAL") we are able to handle critical module clocks.
> > > > > Introduce the same logic for critical core clocks.
> > > > >
> > > > > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> > > > > ---
> > > > > Commit
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
> > > > >
> > > > > is quite nice to avoid *module* clocks being disabled. Unfortunately,
> > > > > there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> > > > > core of the r8a7795, the 'canfd' is a quite popular core clock which
> > > > > shouldn't be disabled by Linux.
> > > > >
> > > > > Therefore, this patch is a proposal to use the same 'mark clocks as
> > > > > critical' logic implemented for the module clocks for the core
> > > > > clocks, too.
> > > > >
> > > > > Opinions?
> > > >
> > > > On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> > > > core which may run another OS.
> > > > This is an interesting issue, and relevant to other SoCs, too.
> > > >
> > > > In this particular case, the "canfd" clock is a core clock used as an
> > > > auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> > > > to three scenarios:
> > > > 1. Linux controls all CAN interfaces
> > > > => no issue,
> > > > 2. The OS on the RT CPU controls all CAN interfaces
> > > > => issue, Linux disables the clock
> > > > 3. Mix of 1 and 2
> > > > => More issues.
> > > > Of course this is not limited to clocks, but also to e.g. PM domains.
> > > >
> > > > How can this be handled?
> > > > I believe just marking the "canfd" clock critical is not the right solution,
> > > > as about any clock could be used by the RT CPU.
> > > >
> > > > Still, Linux needs to be made aware that devices (clocks and PM domains) are
> > > > controlled by another CPU/OS.
> > > >
> > > > Should this be described in DT? It feels like software policy to me.
> > >
> > > No, it shouldn't. It is Linux policy to disable all unused clocks, so
> > > Linux gets to deal with the consequences.
> >
> > The ideal solution I imagine is to make the other CPU's OS a consumer of
> > the Linux clock driver. This would require a generic device driver on the
> > companion CPU that forwards clk requests via inter-cpu communication to
> > the Linux clk driver. It could be feed with the necessary information by
> > the rproc glue. So when the companion cpu is supposed to care for the
> > can0 device, the steps that should happen are:
> >
> > - make sure can0 isn't occupied by the Linux Host
> > - reroute the can irq to the companion cpu (if necessary)
> > - create a dtb containing something like this for the companion CPU:
> >
> > clks: virtclk {
> > compatible = ???
> > #clock-cells = <1>;
> > ...
> > };
> >
> > can@$address {
> > compatible = ...
> > regs = ...
> > clocks = <&clks 3>;
> > clock-names = ...
> > ...
> > };
> >
> > where the driver binding to the virtclk device just forwards clk
> > requests to the Linux host side which then knows that clk 3 is the
> > can clock and does the necessary stuff.
> >
> > This way the can clock doesn't need special handling in the host's dtb
> > and no clock necessary for the companion is disabled as unused because
> > it is requested and enabled.
> >
> > The only problem I see is that implementing such a driver/protocol
> > probably is time consuming.
>
>
> The other problem is security related. If, at all, you have to do it the
> other way around, then:
>
> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> secured) OS clock driver.
That doesn't matter much. Either way the first CPU has to provide the
master side of this device (as it needs clks for booting up) and the 2nd
gets this virtual clk device that forwards clk requests to the first
CPU.
On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
started by the bootrom. If I want the M4 being the primary device I'd
need support in the bootloader to wait long enough (i.e. until the M4 is
up) before letting the A9 jump into Linux. Managable I'd say. This way
would even make sense if the M4 runs a rt critical OS that shouldn't be
forced to wait on the non-rt A9 to enable a clk.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K�nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-01 18:14 ` Uwe Kleine-König
0 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-07-01 18:14 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
> On 30.06.2017 22:24, Uwe Kleine-K?nig wrote:
> > Hello,
> >
> > On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
> > > > TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
> > > > disables them as being unused.
> >
> > not long ago I thought with a few colleagues about this. The scenario is
> > to start a Linux kernel on a Cortex-M companion to a Cortex-A.
> >
> > > > On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> > > > > With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
> > > > > CLK_IS_CRITICAL") we are able to handle critical module clocks.
> > > > > Introduce the same logic for critical core clocks.
> > > > >
> > > > > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> > > > > ---
> > > > > Commit
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
> > > > >
> > > > > is quite nice to avoid *module* clocks being disabled. Unfortunately,
> > > > > there are *core* clocks, too. E.g. using an other OS on the Cortex R7
> > > > > core of the r8a7795, the 'canfd' is a quite popular core clock which
> > > > > shouldn't be disabled by Linux.
> > > > >
> > > > > Therefore, this patch is a proposal to use the same 'mark clocks as
> > > > > critical' logic implemented for the module clocks for the core
> > > > > clocks, too.
> > > > >
> > > > > Opinions?
> > > >
> > > > On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
> > > > core which may run another OS.
> > > > This is an interesting issue, and relevant to other SoCs, too.
> > > >
> > > > In this particular case, the "canfd" clock is a core clock used as an
> > > > auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
> > > > to three scenarios:
> > > > 1. Linux controls all CAN interfaces
> > > > => no issue,
> > > > 2. The OS on the RT CPU controls all CAN interfaces
> > > > => issue, Linux disables the clock
> > > > 3. Mix of 1 and 2
> > > > => More issues.
> > > > Of course this is not limited to clocks, but also to e.g. PM domains.
> > > >
> > > > How can this be handled?
> > > > I believe just marking the "canfd" clock critical is not the right solution,
> > > > as about any clock could be used by the RT CPU.
> > > >
> > > > Still, Linux needs to be made aware that devices (clocks and PM domains) are
> > > > controlled by another CPU/OS.
> > > >
> > > > Should this be described in DT? It feels like software policy to me.
> > >
> > > No, it shouldn't. It is Linux policy to disable all unused clocks, so
> > > Linux gets to deal with the consequences.
> >
> > The ideal solution I imagine is to make the other CPU's OS a consumer of
> > the Linux clock driver. This would require a generic device driver on the
> > companion CPU that forwards clk requests via inter-cpu communication to
> > the Linux clk driver. It could be feed with the necessary information by
> > the rproc glue. So when the companion cpu is supposed to care for the
> > can0 device, the steps that should happen are:
> >
> > - make sure can0 isn't occupied by the Linux Host
> > - reroute the can irq to the companion cpu (if necessary)
> > - create a dtb containing something like this for the companion CPU:
> >
> > clks: virtclk {
> > compatible = ???
> > #clock-cells = <1>;
> > ...
> > };
> >
> > can@$address {
> > compatible = ...
> > regs = ...
> > clocks = <&clks 3>;
> > clock-names = ...
> > ...
> > };
> >
> > where the driver binding to the virtclk device just forwards clk
> > requests to the Linux host side which then knows that clk 3 is the
> > can clock and does the necessary stuff.
> >
> > This way the can clock doesn't need special handling in the host's dtb
> > and no clock necessary for the companion is disabled as unused because
> > it is requested and enabled.
> >
> > The only problem I see is that implementing such a driver/protocol
> > probably is time consuming.
>
>
> The other problem is security related. If, at all, you have to do it the
> other way around, then:
>
> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> secured) OS clock driver.
That doesn't matter much. Either way the first CPU has to provide the
master side of this device (as it needs clks for booting up) and the 2nd
gets this virtual clk device that forwards clk requests to the first
CPU.
On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
started by the bootrom. If I want the M4 being the primary device I'd
need support in the bootloader to wait long enough (i.e. until the M4 is
up) before letting the A9 jump into Linux. Managable I'd say. This way
would even make sense if the M4 runs a rt critical OS that shouldn't be
forced to wait on the non-rt A9 to enable a clk.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-07-01 18:14 ` Uwe Kleine-König
@ 2017-07-02 5:48 ` Dirk Behme
-1 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-07-02 5:48 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Rob Herring, Mark Rutland, devicetree, Ulf Hansson, Dirk Behme,
Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Geert Uytterhoeven, Kevin Hilman,
linux-clk, linux-arm-kernel
On 01.07.2017 20:14, Uwe Kleine-König wrote:
> Hello,
>
> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
>> On 30.06.2017 22:24, Uwe Kleine-König wrote:
>>> Hello,
>>>
>>> On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
>>>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>>>> disables them as being unused.
>>>
>>> not long ago I thought with a few colleagues about this. The scenario is
>>> to start a Linux kernel on a Cortex-M companion to a Cortex-A.
>>>
>>>>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>>>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>>>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>>>>> Introduce the same logic for critical core clocks.
>>>>>>
>>>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>>>> ---
>>>>>> Commit
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>>>>
>>>>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>>>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>>>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>>>>> shouldn't be disabled by Linux.
>>>>>>
>>>>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>>>>> critical' logic implemented for the module clocks for the core
>>>>>> clocks, too.
>>>>>>
>>>>>> Opinions?
>>>>>
>>>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
>>>>> core which may run another OS.
>>>>> This is an interesting issue, and relevant to other SoCs, too.
>>>>>
>>>>> In this particular case, the "canfd" clock is a core clock used as an
>>>>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>>>>> to three scenarios:
>>>>> 1. Linux controls all CAN interfaces
>>>>> => no issue,
>>>>> 2. The OS on the RT CPU controls all CAN interfaces
>>>>> => issue, Linux disables the clock
>>>>> 3. Mix of 1 and 2
>>>>> => More issues.
>>>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>>>
>>>>> How can this be handled?
>>>>> I believe just marking the "canfd" clock critical is not the right solution,
>>>>> as about any clock could be used by the RT CPU.
>>>>>
>>>>> Still, Linux needs to be made aware that devices (clocks and PM domains) are
>>>>> controlled by another CPU/OS.
>>>>>
>>>>> Should this be described in DT? It feels like software policy to me.
>>>>
>>>> No, it shouldn't. It is Linux policy to disable all unused clocks, so
>>>> Linux gets to deal with the consequences.
>>>
>>> The ideal solution I imagine is to make the other CPU's OS a consumer of
>>> the Linux clock driver. This would require a generic device driver on the
>>> companion CPU that forwards clk requests via inter-cpu communication to
>>> the Linux clk driver. It could be feed with the necessary information by
>>> the rproc glue. So when the companion cpu is supposed to care for the
>>> can0 device, the steps that should happen are:
>>>
>>> - make sure can0 isn't occupied by the Linux Host
>>> - reroute the can irq to the companion cpu (if necessary)
>>> - create a dtb containing something like this for the companion CPU:
>>>
>>> clks: virtclk {
>>> compatible = ???
>>> #clock-cells = <1>;
>>> ...
>>> };
>>>
>>> can@$address {
>>> compatible = ...
>>> regs = ...
>>> clocks = <&clks 3>;
>>> clock-names = ...
>>> ...
>>> };
>>>
>>> where the driver binding to the virtclk device just forwards clk
>>> requests to the Linux host side which then knows that clk 3 is the
>>> can clock and does the necessary stuff.
>>>
>>> This way the can clock doesn't need special handling in the host's dtb
>>> and no clock necessary for the companion is disabled as unused because
>>> it is requested and enabled.
>>>
>>> The only problem I see is that implementing such a driver/protocol
>>> probably is time consuming.
>>
>>
>> The other problem is security related. If, at all, you have to do it the
>> other way around, then:
>>
>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
>> secured) OS clock driver.
>
> That doesn't matter much. Either way the first CPU has to provide the
> master side of this device (as it needs clks for booting up) and the 2nd
> gets this virtual clk device that forwards clk requests to the first
> CPU.
>
> On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> started by the bootrom. If I want the M4 being the primary device I'd
> need support in the bootloader to wait long enough (i.e. until the M4 is
> up) before letting the A9 jump into Linux. Managable I'd say. This way
> would even make sense if the M4 runs a rt critical OS that shouldn't be
> forced to wait on the non-rt A9 to enable a clk.
Overall, assuming that the issue we are discussing here can be solved
quite easily in hardware (a set of clock registers for each CPU/OS
domain, connected cleverly to effectively control each clock, with
access protection for each set of registers) I tend to think that for
a SoC supposed to run different OS on different cores this is a
missing hardware feature (bug?).
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-02 5:48 ` Dirk Behme
0 siblings, 0 replies; 54+ messages in thread
From: Dirk Behme @ 2017-07-02 5:48 UTC (permalink / raw)
To: linux-arm-kernel
On 01.07.2017 20:14, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
>> On 30.06.2017 22:24, Uwe Kleine-K?nig wrote:
>>> Hello,
>>>
>>> On Fri, Jun 30, 2017 at 10:58:26AM -0500, Rob Herring wrote:
>>>>> TL;DR: Clocks may be in use by another CPU not running Linux, while Linux
>>>>> disables them as being unused.
>>>
>>> not long ago I thought with a few colleagues about this. The scenario is
>>> to start a Linux kernel on a Cortex-M companion to a Cortex-A.
>>>
>>>>> On Mon, Jun 26, 2017 at 1:30 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>>>>> With commit 72f5df2c2bbb6 ("clk: renesas: cpg-mssr: Migrate to
>>>>>> CLK_IS_CRITICAL") we are able to handle critical module clocks.
>>>>>> Introduce the same logic for critical core clocks.
>>>>>>
>>>>>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>>>>>> ---
>>>>>> Commit
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas?id=72f5df2c2bbb66d4a555cb51eb9f412abf1af77f
>>>>>>
>>>>>> is quite nice to avoid *module* clocks being disabled. Unfortunately,
>>>>>> there are *core* clocks, too. E.g. using an other OS on the Cortex R7
>>>>>> core of the r8a7795, the 'canfd' is a quite popular core clock which
>>>>>> shouldn't be disabled by Linux.
>>>>>>
>>>>>> Therefore, this patch is a proposal to use the same 'mark clocks as
>>>>>> critical' logic implemented for the module clocks for the core
>>>>>> clocks, too.
>>>>>>
>>>>>> Opinions?
>>>>>
>>>>> On r8a7795, there are several Cortex A cores running Linux, and a Cortex R7
>>>>> core which may run another OS.
>>>>> This is an interesting issue, and relevant to other SoCs, too.
>>>>>
>>>>> In this particular case, the "canfd" clock is a core clock used as an
>>>>> auxiliary clock for the CAN0, CAN1, and CANFD interfaces. This can lead
>>>>> to three scenarios:
>>>>> 1. Linux controls all CAN interfaces
>>>>> => no issue,
>>>>> 2. The OS on the RT CPU controls all CAN interfaces
>>>>> => issue, Linux disables the clock
>>>>> 3. Mix of 1 and 2
>>>>> => More issues.
>>>>> Of course this is not limited to clocks, but also to e.g. PM domains.
>>>>>
>>>>> How can this be handled?
>>>>> I believe just marking the "canfd" clock critical is not the right solution,
>>>>> as about any clock could be used by the RT CPU.
>>>>>
>>>>> Still, Linux needs to be made aware that devices (clocks and PM domains) are
>>>>> controlled by another CPU/OS.
>>>>>
>>>>> Should this be described in DT? It feels like software policy to me.
>>>>
>>>> No, it shouldn't. It is Linux policy to disable all unused clocks, so
>>>> Linux gets to deal with the consequences.
>>>
>>> The ideal solution I imagine is to make the other CPU's OS a consumer of
>>> the Linux clock driver. This would require a generic device driver on the
>>> companion CPU that forwards clk requests via inter-cpu communication to
>>> the Linux clk driver. It could be feed with the necessary information by
>>> the rproc glue. So when the companion cpu is supposed to care for the
>>> can0 device, the steps that should happen are:
>>>
>>> - make sure can0 isn't occupied by the Linux Host
>>> - reroute the can irq to the companion cpu (if necessary)
>>> - create a dtb containing something like this for the companion CPU:
>>>
>>> clks: virtclk {
>>> compatible = ???
>>> #clock-cells = <1>;
>>> ...
>>> };
>>>
>>> can@$address {
>>> compatible = ...
>>> regs = ...
>>> clocks = <&clks 3>;
>>> clock-names = ...
>>> ...
>>> };
>>>
>>> where the driver binding to the virtclk device just forwards clk
>>> requests to the Linux host side which then knows that clk 3 is the
>>> can clock and does the necessary stuff.
>>>
>>> This way the can clock doesn't need special handling in the host's dtb
>>> and no clock necessary for the companion is disabled as unused because
>>> it is requested and enabled.
>>>
>>> The only problem I see is that implementing such a driver/protocol
>>> probably is time consuming.
>>
>>
>> The other problem is security related. If, at all, you have to do it the
>> other way around, then:
>>
>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
>> secured) OS clock driver.
>
> That doesn't matter much. Either way the first CPU has to provide the
> master side of this device (as it needs clks for booting up) and the 2nd
> gets this virtual clk device that forwards clk requests to the first
> CPU.
>
> On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> started by the bootrom. If I want the M4 being the primary device I'd
> need support in the bootloader to wait long enough (i.e. until the M4 is
> up) before letting the A9 jump into Linux. Managable I'd say. This way
> would even make sense if the M4 runs a rt critical OS that shouldn't be
> forced to wait on the non-rt A9 to enable a clk.
Overall, assuming that the issue we are discussing here can be solved
quite easily in hardware (a set of clock registers for each CPU/OS
domain, connected cleverly to effectively control each clock, with
access protection for each set of registers) I tend to think that for
a SoC supposed to run different OS on different cores this is a
missing hardware feature (bug?).
Best regards
Dirk
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-07-02 5:48 ` Dirk Behme
(?)
(?)
@ 2017-07-02 9:23 ` Uwe Kleine-König
-1 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-07-02 9:23 UTC (permalink / raw)
To: Dirk Behme
Cc: Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
Ulf Hansson, Dirk Behme, Linux PM list, Rafael J. Wysocki,
Michael Turquette, Stephen Boyd, Linux-Renesas,
Geert Uytterhoeven, Kevin Hilman, linux-clk,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
Hello Dirk,
On Sun, Jul 02, 2017 at 07:48:41AM +0200, Dirk Behme wrote:
> > On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> > started by the bootrom. If I want the M4 being the primary device I'd
> > need support in the bootloader to wait long enough (i.e. until the M4 is
> > up) before letting the A9 jump into Linux. Managable I'd say. This way
> > would even make sense if the M4 runs a rt critical OS that shouldn't be
> > forced to wait on the non-rt A9 to enable a clk.
>
>
> Overall, assuming that the issue we are discussing here can be solved quite
> easily in hardware (a set of clock registers for each CPU/OS domain,
> connected cleverly to effectively control each clock, with access protection
> for each set of registers) I tend to think that for a SoC supposed to run
> different OS on different cores this is a missing hardware feature (bug?).
So you want to enable bits for your CAN clock, one in each cpu's domain.
I'd say that is a nice idea that a hardware engineer might be proud to
pick up but that results in more headache than fun for the software
colleague.
There are several problems that come immediately to mind:
- You can switch of a clk because you don't need it on, or because you
need it off. I guess you want to have the clock on if at least one
cpu wants it on. So you take away the freedom from the other cpu to
force the clock off. (Yeah, the currently available clk framework
doesn't allow that either.)
- What if cpu 0 sets the parent of the can clk to pll2 but cpu 1 wants
it set to pll1? How does cpu 1 notice the change?
- On off might be relatively easy, what about clk dividers? cpu 0 sets
2 which cpu 1 sets 6.
That convinces me that the disadvantages of having two views on the clk
core have more weight and you really want a single view and share that
by software.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-02 9:23 ` Uwe Kleine-König
0 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-07-02 9:23 UTC (permalink / raw)
To: Dirk Behme
Cc: Rob Herring, Mark Rutland, devicetree, Ulf Hansson, Dirk Behme,
Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Geert Uytterhoeven, Kevin Hilman,
linux-clk, linux-arm-kernel, kernel
Hello Dirk,
On Sun, Jul 02, 2017 at 07:48:41AM +0200, Dirk Behme wrote:
> > On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> > started by the bootrom. If I want the M4 being the primary device I'd
> > need support in the bootloader to wait long enough (i.e. until the M4 is
> > up) before letting the A9 jump into Linux. Managable I'd say. This way
> > would even make sense if the M4 runs a rt critical OS that shouldn't be
> > forced to wait on the non-rt A9 to enable a clk.
>
>
> Overall, assuming that the issue we are discussing here can be solved quite
> easily in hardware (a set of clock registers for each CPU/OS domain,
> connected cleverly to effectively control each clock, with access protection
> for each set of registers) I tend to think that for a SoC supposed to run
> different OS on different cores this is a missing hardware feature (bug?).
So you want to enable bits for your CAN clock, one in each cpu's domain.
I'd say that is a nice idea that a hardware engineer might be proud to
pick up but that results in more headache than fun for the software
colleague.
There are several problems that come immediately to mind:
- You can switch of a clk because you don't need it on, or because you
need it off. I guess you want to have the clock on if at least one
cpu wants it on. So you take away the freedom from the other cpu to
force the clock off. (Yeah, the currently available clk framework
doesn't allow that either.)
- What if cpu 0 sets the parent of the can clk to pll2 but cpu 1 wants
it set to pll1? How does cpu 1 notice the change?
- On off might be relatively easy, what about clk dividers? cpu 0 sets
2 which cpu 1 sets 6.
That convinces me that the disadvantages of having two views on the clk
core have more weight and you really want a single view and share that
by software.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K�nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-02 9:23 ` Uwe Kleine-König
0 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-07-02 9:23 UTC (permalink / raw)
To: Dirk Behme
Cc: Rob Herring, Mark Rutland, devicetree, Ulf Hansson, Dirk Behme,
Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Geert Uytterhoeven, Kevin Hilman,
linux-clk, linux-arm-kernel, kernel
Hello Dirk,
On Sun, Jul 02, 2017 at 07:48:41AM +0200, Dirk Behme wrote:
> > On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> > started by the bootrom. If I want the M4 being the primary device I'd
> > need support in the bootloader to wait long enough (i.e. until the M4 is
> > up) before letting the A9 jump into Linux. Managable I'd say. This way
> > would even make sense if the M4 runs a rt critical OS that shouldn't be
> > forced to wait on the non-rt A9 to enable a clk.
>
>
> Overall, assuming that the issue we are discussing here can be solved quite
> easily in hardware (a set of clock registers for each CPU/OS domain,
> connected cleverly to effectively control each clock, with access protection
> for each set of registers) I tend to think that for a SoC supposed to run
> different OS on different cores this is a missing hardware feature (bug?).
So you want to enable bits for your CAN clock, one in each cpu's domain.
I'd say that is a nice idea that a hardware engineer might be proud to
pick up but that results in more headache than fun for the software
colleague.
There are several problems that come immediately to mind:
- You can switch of a clk because you don't need it on, or because you
need it off. I guess you want to have the clock on if at least one
cpu wants it on. So you take away the freedom from the other cpu to
force the clock off. (Yeah, the currently available clk framework
doesn't allow that either.)
- What if cpu 0 sets the parent of the can clk to pll2 but cpu 1 wants
it set to pll1? How does cpu 1 notice the change?
- On off might be relatively easy, what about clk dividers? cpu 0 sets
2 which cpu 1 sets 6.
That convinces me that the disadvantages of having two views on the clk
core have more weight and you really want a single view and share that
by software.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-02 9:23 ` Uwe Kleine-König
0 siblings, 0 replies; 54+ messages in thread
From: Uwe Kleine-König @ 2017-07-02 9:23 UTC (permalink / raw)
To: linux-arm-kernel
Hello Dirk,
On Sun, Jul 02, 2017 at 07:48:41AM +0200, Dirk Behme wrote:
> > On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> > started by the bootrom. If I want the M4 being the primary device I'd
> > need support in the bootloader to wait long enough (i.e. until the M4 is
> > up) before letting the A9 jump into Linux. Managable I'd say. This way
> > would even make sense if the M4 runs a rt critical OS that shouldn't be
> > forced to wait on the non-rt A9 to enable a clk.
>
>
> Overall, assuming that the issue we are discussing here can be solved quite
> easily in hardware (a set of clock registers for each CPU/OS domain,
> connected cleverly to effectively control each clock, with access protection
> for each set of registers) I tend to think that for a SoC supposed to run
> different OS on different cores this is a missing hardware feature (bug?).
So you want to enable bits for your CAN clock, one in each cpu's domain.
I'd say that is a nice idea that a hardware engineer might be proud to
pick up but that results in more headache than fun for the software
colleague.
There are several problems that come immediately to mind:
- You can switch of a clk because you don't need it on, or because you
need it off. I guess you want to have the clock on if at least one
cpu wants it on. So you take away the freedom from the other cpu to
force the clock off. (Yeah, the currently available clk framework
doesn't allow that either.)
- What if cpu 0 sets the parent of the can clk to pll2 but cpu 1 wants
it set to pll1? How does cpu 1 notice the change?
- On off might be relatively easy, what about clk dividers? cpu 0 sets
2 which cpu 1 sets 6.
That convinces me that the disadvantages of having two views on the clk
core have more weight and you really want a single view and share that
by software.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-07-02 9:23 ` Uwe Kleine-König
(?)
@ 2017-07-03 7:40 ` Geert Uytterhoeven
-1 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-07-03 7:40 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Dirk Behme, Rob Herring, Mark Rutland, devicetree, Ulf Hansson,
Dirk Behme, Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Kevin Hilman, linux-clk,
linux-arm-kernel, Sascha Hauer
Hi Uwe,
On Sun, Jul 2, 2017 at 11:23 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Sun, Jul 02, 2017 at 07:48:41AM +0200, Dirk Behme wrote:
>> > On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
>> > started by the bootrom. If I want the M4 being the primary device I'd
>> > need support in the bootloader to wait long enough (i.e. until the M4 is
>> > up) before letting the A9 jump into Linux. Managable I'd say. This way
>> > would even make sense if the M4 runs a rt critical OS that shouldn't be
>> > forced to wait on the non-rt A9 to enable a clk.
>>
>> Overall, assuming that the issue we are discussing here can be solved quite
>> easily in hardware (a set of clock registers for each CPU/OS domain,
>> connected cleverly to effectively control each clock, with access protection
>> for each set of registers) I tend to think that for a SoC supposed to run
>> different OS on different cores this is a missing hardware feature (bug?).
>
> So you want to enable bits for your CAN clock, one in each cpu's domain.
>
> I'd say that is a nice idea that a hardware engineer might be proud to
> pick up but that results in more headache than fun for the software
> colleague.
>
> There are several problems that come immediately to mind:
>
> - You can switch of a clk because you don't need it on, or because you
> need it off. I guess you want to have the clock on if at least one
> cpu wants it on. So you take away the freedom from the other cpu to
> force the clock off. (Yeah, the currently available clk framework
> doesn't allow that either.)
> - What if cpu 0 sets the parent of the can clk to pll2 but cpu 1 wants
> it set to pll1? How does cpu 1 notice the change?
> - On off might be relatively easy, what about clk dividers? cpu 0 sets
> 2 which cpu 1 sets 6.
>
> That convinces me that the disadvantages of having two views on the clk
> core have more weight and you really want a single view and share that
> by software.
Renesas ARM SoCs already implement (parts of) that.
There are separate sets of the Module SToP register bits for each set of
CPUs. Only if all sets agree the clock supply to a module will be stopped,
which can be monitored using the status Registers.
So for the CAN modules itself there is no issue, as the clock supply to the
modules will not be stopped as long as the RT CPU keeps it enabled.
There must be more to it (secure mode?), as some R-Car Gen3 module clocks
cannot seem to be disabled by disabling them for all documented CPU sets.
There's also a global bit to prevent modifying any clock register.
This could be used to prevent changing the CAN parent clock (changing
its divider and/or stopping it). However, as this is a global bit, it would
affect the full Linux clock driver.
Again, probably there's more to it when using secure mode...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-03 7:40 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-07-03 7:40 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Dirk Behme, Rob Herring, Mark Rutland, devicetree, Ulf Hansson,
Dirk Behme, Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Kevin Hilman, linux-clk,
linux-arm-kernel, Sascha Hauer
Hi Uwe,
On Sun, Jul 2, 2017 at 11:23 AM, Uwe Kleine-K=C3=B6nig
<u.kleine-koenig@pengutronix.de> wrote:
> On Sun, Jul 02, 2017 at 07:48:41AM +0200, Dirk Behme wrote:
>> > On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
>> > started by the bootrom. If I want the M4 being the primary device I'd
>> > need support in the bootloader to wait long enough (i.e. until the M4 =
is
>> > up) before letting the A9 jump into Linux. Managable I'd say. This way
>> > would even make sense if the M4 runs a rt critical OS that shouldn't b=
e
>> > forced to wait on the non-rt A9 to enable a clk.
>>
>> Overall, assuming that the issue we are discussing here can be solved qu=
ite
>> easily in hardware (a set of clock registers for each CPU/OS domain,
>> connected cleverly to effectively control each clock, with access protec=
tion
>> for each set of registers) I tend to think that for a SoC supposed to ru=
n
>> different OS on different cores this is a missing hardware feature (bug?=
).
>
> So you want to enable bits for your CAN clock, one in each cpu's domain.
>
> I'd say that is a nice idea that a hardware engineer might be proud to
> pick up but that results in more headache than fun for the software
> colleague.
>
> There are several problems that come immediately to mind:
>
> - You can switch of a clk because you don't need it on, or because you
> need it off. I guess you want to have the clock on if at least one
> cpu wants it on. So you take away the freedom from the other cpu to
> force the clock off. (Yeah, the currently available clk framework
> doesn't allow that either.)
> - What if cpu 0 sets the parent of the can clk to pll2 but cpu 1 wants
> it set to pll1? How does cpu 1 notice the change?
> - On off might be relatively easy, what about clk dividers? cpu 0 sets
> 2 which cpu 1 sets 6.
>
> That convinces me that the disadvantages of having two views on the clk
> core have more weight and you really want a single view and share that
> by software.
Renesas ARM SoCs already implement (parts of) that.
There are separate sets of the Module SToP register bits for each set of
CPUs. Only if all sets agree the clock supply to a module will be stopped,
which can be monitored using the status Registers.
So for the CAN modules itself there is no issue, as the clock supply to the
modules will not be stopped as long as the RT CPU keeps it enabled.
There must be more to it (secure mode?), as some R-Car Gen3 module clocks
cannot seem to be disabled by disabling them for all documented CPU sets.
There's also a global bit to prevent modifying any clock register.
This could be used to prevent changing the CAN parent clock (changing
its divider and/or stopping it). However, as this is a global bit, it would
affect the full Linux clock driver.
Again, probably there's more to it when using secure mode...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org
In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-03 7:40 ` Geert Uytterhoeven
0 siblings, 0 replies; 54+ messages in thread
From: Geert Uytterhoeven @ 2017-07-03 7:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Uwe,
On Sun, Jul 2, 2017 at 11:23 AM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> On Sun, Jul 02, 2017 at 07:48:41AM +0200, Dirk Behme wrote:
>> > On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
>> > started by the bootrom. If I want the M4 being the primary device I'd
>> > need support in the bootloader to wait long enough (i.e. until the M4 is
>> > up) before letting the A9 jump into Linux. Managable I'd say. This way
>> > would even make sense if the M4 runs a rt critical OS that shouldn't be
>> > forced to wait on the non-rt A9 to enable a clk.
>>
>> Overall, assuming that the issue we are discussing here can be solved quite
>> easily in hardware (a set of clock registers for each CPU/OS domain,
>> connected cleverly to effectively control each clock, with access protection
>> for each set of registers) I tend to think that for a SoC supposed to run
>> different OS on different cores this is a missing hardware feature (bug?).
>
> So you want to enable bits for your CAN clock, one in each cpu's domain.
>
> I'd say that is a nice idea that a hardware engineer might be proud to
> pick up but that results in more headache than fun for the software
> colleague.
>
> There are several problems that come immediately to mind:
>
> - You can switch of a clk because you don't need it on, or because you
> need it off. I guess you want to have the clock on if at least one
> cpu wants it on. So you take away the freedom from the other cpu to
> force the clock off. (Yeah, the currently available clk framework
> doesn't allow that either.)
> - What if cpu 0 sets the parent of the can clk to pll2 but cpu 1 wants
> it set to pll1? How does cpu 1 notice the change?
> - On off might be relatively easy, what about clk dividers? cpu 0 sets
> 2 which cpu 1 sets 6.
>
> That convinces me that the disadvantages of having two views on the clk
> core have more weight and you really want a single view and share that
> by software.
Renesas ARM SoCs already implement (parts of) that.
There are separate sets of the Module SToP register bits for each set of
CPUs. Only if all sets agree the clock supply to a module will be stopped,
which can be monitored using the status Registers.
So for the CAN modules itself there is no issue, as the clock supply to the
modules will not be stopped as long as the RT CPU keeps it enabled.
There must be more to it (secure mode?), as some R-Car Gen3 module clocks
cannot seem to be disabled by disabling them for all documented CPU sets.
There's also a global bit to prevent modifying any clock register.
This could be used to prevent changing the CAN parent clock (changing
its divider and/or stopping it). However, as this is a global bit, it would
affect the full Linux clock driver.
Again, probably there's more to it when using secure mode...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
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
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-07-01 18:14 ` Uwe Kleine-König
(?)
@ 2017-07-03 9:17 ` Sudeep Holla
-1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2017-07-03 9:17 UTC (permalink / raw)
To: Uwe Kleine-König, Dirk Behme
Cc: Sudeep Holla, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson, Dirk Behme,
Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Geert Uytterhoeven, Kevin Hilman,
linux-clk, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 01/07/17 19:14, Uwe Kleine-König wrote:
> Hello,
>
> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
[...]
>>
>>
>> The other problem is security related. If, at all, you have to do it the
>> other way around, then:
>>
>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
>> secured) OS clock driver.
Yes, that's better and is getting common on newer platforms. They have
separate M-class(or even low A-class e.g. A5/A7) processors to handle
all the system management.
The new ARM SCMI specification[0][1] is designed to standardize the
interface. It covers the clocks in clock protocol.
>
> That doesn't matter much. Either way the first CPU has to provide the
> master side of this device (as it needs clks for booting up) and the 2nd
> gets this virtual clk device that forwards clk requests to the first
> CPU.
>
> On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> started by the bootrom. If I want the M4 being the primary device I'd
> need support in the bootloader to wait long enough (i.e. until the M4 is
> up) before letting the A9 jump into Linux.
I think that is platform specific. On few platforms I have seen
recently, it's M4 or whatever core that handles system power management
boots first and is responsible to even boot secondaries.
> Managable I'd say. This way would even make sense if the M4 runs a
> rt critical OS that shouldn't be forced to wait on the non-rt A9 to> enable a clk.
>
Exactly.
--
Regards,
Sudeep
[0] http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/index.html
[1] https://marc.info/?l=devicetree&m=149849482623492&w=2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-03 9:17 ` Sudeep Holla
0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2017-07-03 9:17 UTC (permalink / raw)
To: Uwe Kleine-König, Dirk Behme
Cc: Sudeep Holla, Rob Herring, Mark Rutland, devicetree, Ulf Hansson,
Dirk Behme, Linux PM list, Rafael J. Wysocki, Michael Turquette,
Stephen Boyd, Linux-Renesas, Geert Uytterhoeven, Kevin Hilman,
linux-clk, linux-arm-kernel
On 01/07/17 19:14, Uwe Kleine-König wrote:
> Hello,
>
> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
[...]
>>
>>
>> The other problem is security related. If, at all, you have to do it the
>> other way around, then:
>>
>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
>> secured) OS clock driver.
Yes, that's better and is getting common on newer platforms. They have
separate M-class(or even low A-class e.g. A5/A7) processors to handle
all the system management.
The new ARM SCMI specification[0][1] is designed to standardize the
interface. It covers the clocks in clock protocol.
>
> That doesn't matter much. Either way the first CPU has to provide the
> master side of this device (as it needs clks for booting up) and the 2nd
> gets this virtual clk device that forwards clk requests to the first
> CPU.
>
> On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> started by the bootrom. If I want the M4 being the primary device I'd
> need support in the bootloader to wait long enough (i.e. until the M4 is
> up) before letting the A9 jump into Linux.
I think that is platform specific. On few platforms I have seen
recently, it's M4 or whatever core that handles system power management
boots first and is responsible to even boot secondaries.
> Managable I'd say. This way would even make sense if the M4 runs a
> rt critical OS that shouldn't be forced to wait on the non-rt A9 to> enable a clk.
>
Exactly.
--
Regards,
Sudeep
[0] http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/index.html
[1] https://marc.info/?l=devicetree&m=149849482623492&w=2
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-03 9:17 ` Sudeep Holla
0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2017-07-03 9:17 UTC (permalink / raw)
To: linux-arm-kernel
On 01/07/17 19:14, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
[...]
>>
>>
>> The other problem is security related. If, at all, you have to do it the
>> other way around, then:
>>
>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
>> secured) OS clock driver.
Yes, that's better and is getting common on newer platforms. They have
separate M-class(or even low A-class e.g. A5/A7) processors to handle
all the system management.
The new ARM SCMI specification[0][1] is designed to standardize the
interface. It covers the clocks in clock protocol.
>
> That doesn't matter much. Either way the first CPU has to provide the
> master side of this device (as it needs clks for booting up) and the 2nd
> gets this virtual clk device that forwards clk requests to the first
> CPU.
>
> On my machine (Udoo Neo, A9 + M4) the A9 is the primary CPU that is
> started by the bootrom. If I want the M4 being the primary device I'd
> need support in the bootloader to wait long enough (i.e. until the M4 is
> up) before letting the A9 jump into Linux.
I think that is platform specific. On few platforms I have seen
recently, it's M4 or whatever core that handles system power management
boots first and is responsible to even boot secondaries.
> Managable I'd say. This way would even make sense if the M4 runs a
> rt critical OS that shouldn't be forced to wait on the non-rt A9 to> enable a clk.
>
Exactly.
--
Regards,
Sudeep
[0] http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/index.html
[1] https://marc.info/?l=devicetree&m=149849482623492&w=2
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-07-03 9:17 ` Sudeep Holla
(?)
(?)
@ 2017-07-04 7:31 ` Peter De Schrijver
-1 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-07-04 7:31 UTC (permalink / raw)
To: Sudeep Holla
Cc: Uwe Kleine-König, Dirk Behme, Rob Herring, Mark Rutland,
devicetree, Ulf Hansson, Dirk Behme, Linux PM list,
Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
Linux-Renesas, Geert Uytterhoeven, Kevin Hilman, linux-clk,
linux-arm-kernel
On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
>
>
> On 01/07/17 19:14, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
>
> [...]
>
> >>
> >>
> >> The other problem is security related. If, at all, you have to do it the
> >> other way around, then:
> >>
> >> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> >> secured) OS clock driver.
>
> Yes, that's better and is getting common on newer platforms. They have
> separate M-class(or even low A-class e.g. A5/A7) processors to handle
> all the system management.
>
> The new ARM SCMI specification[0][1] is designed to standardize the
> interface. It covers the clocks in clock protocol.
>
Yes, however this doesn't exist on older SoCs which still have multiple CPU's
Peter.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-04 7:31 ` Peter De Schrijver
0 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-07-04 7:31 UTC (permalink / raw)
To: Sudeep Holla
Cc: Uwe Kleine-König, Dirk Behme, Rob Herring, Mark Rutland,
devicetree, Ulf Hansson, Dirk Behme, Linux PM list,
Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
Linux-Renesas, Geert Uytterhoeven, Kevin Hilman, linux-clk,
linux-arm-kernel
On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
>
>
> On 01/07/17 19:14, Uwe Kleine-K�nig wrote:
> > Hello,
> >
> > On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
>
> [...]
>
> >>
> >>
> >> The other problem is security related. If, at all, you have to do it the
> >> other way around, then:
> >>
> >> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> >> secured) OS clock driver.
>
> Yes, that's better and is getting common on newer platforms. They have
> separate M-class(or even low A-class e.g. A5/A7) processors to handle
> all the system management.
>
> The new ARM SCMI specification[0][1] is designed to standardize the
> interface. It covers the clocks in clock protocol.
>
Yes, however this doesn't exist on older SoCs which still have multiple CPU's
Peter.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-04 7:31 ` Peter De Schrijver
0 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-07-04 7:31 UTC (permalink / raw)
To: Sudeep Holla
Cc: Uwe Kleine-König, Dirk Behme, Rob Herring, Mark Rutland,
devicetree, Ulf Hansson, Dirk Behme, Linux PM list,
Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
Linux-Renesas, Geert Uytterhoeven, Kevin Hilman, linux-clk,
linux-arm-kernel
On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
>=20
>=20
> On 01/07/17 19:14, Uwe Kleine-K=F6nig wrote:
> > Hello,
> >=20
> > On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
>=20
> [...]
>=20
> >>
> >>
> >> The other problem is security related. If, at all, you have to do it t=
he
> >> other way around, then:
> >>
> >> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> >> secured) OS clock driver.
>=20
> Yes, that's better and is getting common on newer platforms. They have
> separate M-class(or even low A-class e.g. A5/A7) processors to handle
> all the system management.
>=20
> The new ARM SCMI specification[0][1] is designed to standardize the
> interface. It covers the clocks in clock protocol.
>=20
Yes, however this doesn't exist on older SoCs which still have multiple CPU=
's
Peter.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-04 7:31 ` Peter De Schrijver
0 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-07-04 7:31 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
>
>
> On 01/07/17 19:14, Uwe Kleine-K?nig wrote:
> > Hello,
> >
> > On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
>
> [...]
>
> >>
> >>
> >> The other problem is security related. If, at all, you have to do it the
> >> other way around, then:
> >>
> >> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> >> secured) OS clock driver.
>
> Yes, that's better and is getting common on newer platforms. They have
> separate M-class(or even low A-class e.g. A5/A7) processors to handle
> all the system management.
>
> The new ARM SCMI specification[0][1] is designed to standardize the
> interface. It covers the clocks in clock protocol.
>
Yes, however this doesn't exist on older SoCs which still have multiple CPU's
Peter.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-07-04 7:31 ` Peter De Schrijver
@ 2017-07-04 8:49 ` Sudeep Holla
-1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2017-07-04 8:49 UTC (permalink / raw)
To: Peter De Schrijver
Cc: Sudeep Holla, Uwe Kleine-König, Dirk Behme, Rob Herring,
Mark Rutland, devicetree, Ulf Hansson, Dirk Behme, Linux PM list,
Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
Linux-Renesas, Geert Uytterhoeven, Kevin Hilman, linux-clk,
linux-arm-kernel
On 04/07/17 08:31, Peter De Schrijver wrote:
> On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
>>
>>
>> On 01/07/17 19:14, Uwe Kleine-König wrote:
>>> Hello,
>>>
>>> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
>>
>> [...]
>>
>>>>
>>>>
>>>> The other problem is security related. If, at all, you have to do it the
>>>> other way around, then:
>>>>
>>>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
>>>> secured) OS clock driver.
>>
>> Yes, that's better and is getting common on newer platforms. They have
>> separate M-class(or even low A-class e.g. A5/A7) processors to handle
>> all the system management.
>>
>> The new ARM SCMI specification[0][1] is designed to standardize the
>> interface. It covers the clocks in clock protocol.
>>
>
> Yes, however this doesn't exist on older SoCs which still have multiple CPU's
>
Agreed. But if someone is fixing/adding support in Linux as well as in
the other OS running on those cores, why not consider this interface
instead of trying to generalize something which will invariably SoC
specific.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-04 8:49 ` Sudeep Holla
0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2017-07-04 8:49 UTC (permalink / raw)
To: linux-arm-kernel
On 04/07/17 08:31, Peter De Schrijver wrote:
> On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
>>
>>
>> On 01/07/17 19:14, Uwe Kleine-K?nig wrote:
>>> Hello,
>>>
>>> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
>>
>> [...]
>>
>>>>
>>>>
>>>> The other problem is security related. If, at all, you have to do it the
>>>> other way around, then:
>>>>
>>>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
>>>> secured) OS clock driver.
>>
>> Yes, that's better and is getting common on newer platforms. They have
>> separate M-class(or even low A-class e.g. A5/A7) processors to handle
>> all the system management.
>>
>> The new ARM SCMI specification[0][1] is designed to standardize the
>> interface. It covers the clocks in clock protocol.
>>
>
> Yes, however this doesn't exist on older SoCs which still have multiple CPU's
>
Agreed. But if someone is fixing/adding support in Linux as well as in
the other OS running on those cores, why not consider this interface
instead of trying to generalize something which will invariably SoC
specific.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
2017-07-04 8:49 ` Sudeep Holla
(?)
(?)
@ 2017-07-05 7:25 ` Peter De Schrijver
-1 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-07-05 7:25 UTC (permalink / raw)
To: Sudeep Holla
Cc: Uwe Kleine-König, Dirk Behme, Rob Herring, Mark Rutland,
devicetree, Ulf Hansson, Dirk Behme, Linux PM list,
Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
Linux-Renesas, Geert Uytterhoeven, Kevin Hilman, linux-clk,
linux-arm-kernel
On Tue, Jul 04, 2017 at 09:49:25AM +0100, Sudeep Holla wrote:
>
>
> On 04/07/17 08:31, Peter De Schrijver wrote:
> > On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
> >>
> >>
> >> On 01/07/17 19:14, Uwe Kleine-König wrote:
> >>> Hello,
> >>>
> >>> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
> >>
> >> [...]
> >>
> >>>>
> >>>>
> >>>> The other problem is security related. If, at all, you have to do it the
> >>>> other way around, then:
> >>>>
> >>>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> >>>> secured) OS clock driver.
> >>
> >> Yes, that's better and is getting common on newer platforms. They have
> >> separate M-class(or even low A-class e.g. A5/A7) processors to handle
> >> all the system management.
> >>
> >> The new ARM SCMI specification[0][1] is designed to standardize the
> >> interface. It covers the clocks in clock protocol.
> >>
> >
> > Yes, however this doesn't exist on older SoCs which still have multiple CPU's
> >
>
> Agreed. But if someone is fixing/adding support in Linux as well as in
> the other OS running on those cores, why not consider this interface
> instead of trying to generalize something which will invariably SoC
> specific.
Because the firmware of the other CPUs might not be able to support this
or is frozen and cannot be changed.
Peter.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-05 7:25 ` Peter De Schrijver
0 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-07-05 7:25 UTC (permalink / raw)
To: Sudeep Holla
Cc: Uwe Kleine-König, Dirk Behme, Rob Herring, Mark Rutland,
devicetree, Ulf Hansson, Dirk Behme, Linux PM list,
Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
Linux-Renesas, Geert Uytterhoeven, Kevin Hilman, linux-clk,
linux-arm-kernel
On Tue, Jul 04, 2017 at 09:49:25AM +0100, Sudeep Holla wrote:
>
>
> On 04/07/17 08:31, Peter De Schrijver wrote:
> > On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
> >>
> >>
> >> On 01/07/17 19:14, Uwe Kleine-K�nig wrote:
> >>> Hello,
> >>>
> >>> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
> >>
> >> [...]
> >>
> >>>>
> >>>>
> >>>> The other problem is security related. If, at all, you have to do it the
> >>>> other way around, then:
> >>>>
> >>>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> >>>> secured) OS clock driver.
> >>
> >> Yes, that's better and is getting common on newer platforms. They have
> >> separate M-class(or even low A-class e.g. A5/A7) processors to handle
> >> all the system management.
> >>
> >> The new ARM SCMI specification[0][1] is designed to standardize the
> >> interface. It covers the clocks in clock protocol.
> >>
> >
> > Yes, however this doesn't exist on older SoCs which still have multiple CPU's
> >
>
> Agreed. But if someone is fixing/adding support in Linux as well as in
> the other OS running on those cores, why not consider this interface
> instead of trying to generalize something which will invariably SoC
> specific.
Because the firmware of the other CPUs might not be able to support this
or is frozen and cannot be changed.
Peter.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-05 7:25 ` Peter De Schrijver
0 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-07-05 7:25 UTC (permalink / raw)
To: Sudeep Holla
Cc: Uwe Kleine-König, Dirk Behme, Rob Herring, Mark Rutland,
devicetree, Ulf Hansson, Dirk Behme, Linux PM list,
Rafael J. Wysocki, Michael Turquette, Stephen Boyd,
Linux-Renesas, Geert Uytterhoeven, Kevin Hilman, linux-clk,
linux-arm-kernel
On Tue, Jul 04, 2017 at 09:49:25AM +0100, Sudeep Holla wrote:
>=20
>=20
> On 04/07/17 08:31, Peter De Schrijver wrote:
> > On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
> >>
> >>
> >> On 01/07/17 19:14, Uwe Kleine-K=F6nig wrote:
> >>> Hello,
> >>>
> >>> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
> >>
> >> [...]
> >>
> >>>>
> >>>>
> >>>> The other problem is security related. If, at all, you have to do it=
the
> >>>> other way around, then:
> >>>>
> >>>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> >>>> secured) OS clock driver.
> >>
> >> Yes, that's better and is getting common on newer platforms. They have
> >> separate M-class(or even low A-class e.g. A5/A7) processors to handle
> >> all the system management.
> >>
> >> The new ARM SCMI specification[0][1] is designed to standardize the
> >> interface. It covers the clocks in clock protocol.
> >>
> >=20
> > Yes, however this doesn't exist on older SoCs which still have multiple=
CPU's
> >=20
>=20
> Agreed. But if someone is fixing/adding support in Linux as well as in
> the other OS running on those cores, why not consider this interface
> instead of trying to generalize something which will invariably SoC
> specific.
Because the firmware of the other CPUs might not be able to support this
or is frozen and cannot be changed.
Peter.
^ permalink raw reply [flat|nested] 54+ messages in thread
* Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks)
@ 2017-07-05 7:25 ` Peter De Schrijver
0 siblings, 0 replies; 54+ messages in thread
From: Peter De Schrijver @ 2017-07-05 7:25 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 04, 2017 at 09:49:25AM +0100, Sudeep Holla wrote:
>
>
> On 04/07/17 08:31, Peter De Schrijver wrote:
> > On Mon, Jul 03, 2017 at 10:17:22AM +0100, Sudeep Holla wrote:
> >>
> >>
> >> On 01/07/17 19:14, Uwe Kleine-K?nig wrote:
> >>> Hello,
> >>>
> >>> On Sat, Jul 01, 2017 at 07:02:48AM +0200, Dirk Behme wrote:
> >>
> >> [...]
> >>
> >>>>
> >>>>
> >>>> The other problem is security related. If, at all, you have to do it the
> >>>> other way around, then:
> >>>>
> >>>> Make Linux a consumer of the other CPU's (trusted/trustzone/whatever
> >>>> secured) OS clock driver.
> >>
> >> Yes, that's better and is getting common on newer platforms. They have
> >> separate M-class(or even low A-class e.g. A5/A7) processors to handle
> >> all the system management.
> >>
> >> The new ARM SCMI specification[0][1] is designed to standardize the
> >> interface. It covers the clocks in clock protocol.
> >>
> >
> > Yes, however this doesn't exist on older SoCs which still have multiple CPU's
> >
>
> Agreed. But if someone is fixing/adding support in Linux as well as in
> the other OS running on those cores, why not consider this interface
> instead of trying to generalize something which will invariably SoC
> specific.
Because the firmware of the other CPUs might not be able to support this
or is frozen and cannot be changed.
Peter.
^ permalink raw reply [flat|nested] 54+ messages in thread
end of thread, other threads:[~2017-07-05 7:25 UTC | newest]
Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-29 9:27 Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks) Geert Uytterhoeven
2017-06-29 9:27 ` Geert Uytterhoeven
2017-06-29 10:28 ` Dirk Behme
2017-06-29 10:28 ` Dirk Behme
2017-06-29 11:18 ` Geert Uytterhoeven
2017-06-29 11:18 ` Geert Uytterhoeven
2017-06-29 13:18 ` Clocks used by another OS/CPU Dirk Behme
2017-06-29 13:18 ` Dirk Behme
2017-06-29 13:22 ` Geert Uytterhoeven
2017-06-29 13:22 ` Geert Uytterhoeven
[not found] ` <CAMuHMdW9+CNTTOVO4SRRUxuz3ajLbY2j1uG8b_RpHX52NPwXrQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-29 11:56 ` Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks) Geert Uytterhoeven
2017-06-29 11:56 ` Geert Uytterhoeven
2017-06-29 11:56 ` Geert Uytterhoeven
2017-06-29 12:07 ` Clocks used by another OS/CPU Dirk Behme
2017-06-29 12:07 ` Dirk Behme
2017-06-29 12:45 ` Geert Uytterhoeven
2017-06-29 12:45 ` Geert Uytterhoeven
2017-06-29 12:55 ` Dirk Behme
2017-06-29 12:55 ` Dirk Behme
2017-06-30 8:02 ` Clocks used by another OS/CPU (was: Re: [RFC PATCH] clk: renesas: cpg-mssr: Add interface for critical core clocks) Peter De Schrijver
2017-06-30 8:02 ` Peter De Schrijver
2017-06-30 15:58 ` Rob Herring
2017-06-30 15:58 ` Rob Herring
2017-06-30 20:24 ` Uwe Kleine-König
2017-06-30 20:24 ` Uwe Kleine-König
2017-06-30 20:24 ` Uwe Kleine-König
[not found] ` <20170630202453.eh6vaehkap3as4np-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2017-07-01 5:02 ` Dirk Behme
2017-07-01 5:02 ` Dirk Behme
2017-07-01 5:02 ` Dirk Behme
2017-07-01 18:14 ` Uwe Kleine-König
2017-07-01 18:14 ` Uwe Kleine-König
2017-07-01 18:14 ` Uwe Kleine-König
2017-07-02 5:48 ` Dirk Behme
2017-07-02 5:48 ` Dirk Behme
[not found] ` <6098d579-f206-5a23-bbfc-ac13e0448479-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-07-02 9:23 ` Uwe Kleine-König
2017-07-02 9:23 ` Uwe Kleine-König
2017-07-02 9:23 ` Uwe Kleine-König
2017-07-02 9:23 ` Uwe Kleine-König
2017-07-03 7:40 ` Geert Uytterhoeven
2017-07-03 7:40 ` Geert Uytterhoeven
2017-07-03 7:40 ` Geert Uytterhoeven
[not found] ` <20170701181408.yuocymwtj5dgt74d-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2017-07-03 9:17 ` Sudeep Holla
2017-07-03 9:17 ` Sudeep Holla
2017-07-03 9:17 ` Sudeep Holla
2017-07-04 7:31 ` Peter De Schrijver
2017-07-04 7:31 ` Peter De Schrijver
2017-07-04 7:31 ` Peter De Schrijver
2017-07-04 7:31 ` Peter De Schrijver
2017-07-04 8:49 ` Sudeep Holla
2017-07-04 8:49 ` Sudeep Holla
2017-07-05 7:25 ` Peter De Schrijver
2017-07-05 7:25 ` Peter De Schrijver
2017-07-05 7:25 ` Peter De Schrijver
2017-07-05 7:25 ` Peter De Schrijver
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.