* [RESEND PATCH 0/3] Clock fixes for Ingenic SoCs
@ 2022-04-11 10:14 Aidan MacDonald
2022-04-11 10:14 ` [RESEND PATCH 1/3] clk: ingenic: Allow specifying common clock flags Aidan MacDonald
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Aidan MacDonald @ 2022-04-11 10:14 UTC (permalink / raw)
To: paul, paulburton, tsbogend, mturquette, sboyd
Cc: linux-mips, linux-kernel, linux-clk
Resending this series since it appears none of the patches were picked up
the first time around.
I ran across a problem trying to get Linux running on an Ingenic X1000 SoC:
since the memory clock isn't referenced by any driver, it appears unused and
gets disabled automatically. After that, the system hangs on any RAM access.
There is a hack in board-ingenic.c to forcibly enable the CPU clock, but this
is insufficient for the X1000 since the memory clock has its own gate and mux
that isn't tied to the CPU.
This patch series fixes the bug by adding CLK_IS_CRITICAL flags to important
clocks, which seems to be the approach used in many other SoC clock drivers.
Original submission:
https://lore.kernel.org/linux-mips/20220208010048.211691-1-aidanmacdonald.0x0@gmail.com/
Aidan MacDonald (3):
clk: ingenic: Allow specifying common clock flags
clk: ingenic: Mark critical clocks in Ingenic SoCs
mips: ingenic: Do not manually reference the CPU clock
arch/mips/generic/board-ingenic.c | 26 --------------------------
drivers/clk/ingenic/cgu.c | 2 +-
drivers/clk/ingenic/cgu.h | 3 +++
drivers/clk/ingenic/jz4725b-cgu.c | 2 ++
drivers/clk/ingenic/jz4740-cgu.c | 2 ++
drivers/clk/ingenic/jz4760-cgu.c | 2 ++
drivers/clk/ingenic/jz4770-cgu.c | 1 +
drivers/clk/ingenic/jz4780-cgu.c | 3 +++
drivers/clk/ingenic/x1000-cgu.c | 3 +++
drivers/clk/ingenic/x1830-cgu.c | 3 +++
10 files changed, 20 insertions(+), 27 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [RESEND PATCH 1/3] clk: ingenic: Allow specifying common clock flags 2022-04-11 10:14 [RESEND PATCH 0/3] Clock fixes for Ingenic SoCs Aidan MacDonald @ 2022-04-11 10:14 ` Aidan MacDonald 2022-04-11 10:14 ` [RESEND PATCH 2/3] clk: ingenic: Mark critical clocks in Ingenic SoCs Aidan MacDonald 2022-04-11 10:14 ` [RESEND PATCH 3/3] mips: ingenic: Do not manually reference the CPU clock Aidan MacDonald 2 siblings, 0 replies; 6+ messages in thread From: Aidan MacDonald @ 2022-04-11 10:14 UTC (permalink / raw) To: paul, paulburton, tsbogend, mturquette, sboyd Cc: linux-mips, linux-kernel, linux-clk Provide a flags field for clocks under the ingenic-cgu driver, which can be used to set generic common clock framework flags on the created clocks. For example, the CLK_IS_CRITICAL flag is needed for some clocks (such as CPU or memory) to stop them being automatically disabled. Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> Reviewed-by: Paul Cercueil <paul@crapouillou.net> --- drivers/clk/ingenic/cgu.c | 2 +- drivers/clk/ingenic/cgu.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c index af31633a8862..861c50d6cb24 100644 --- a/drivers/clk/ingenic/cgu.c +++ b/drivers/clk/ingenic/cgu.c @@ -660,7 +660,7 @@ static int ingenic_register_clock(struct ingenic_cgu *cgu, unsigned idx) ingenic_clk->idx = idx; clk_init.name = clk_info->name; - clk_init.flags = 0; + clk_init.flags = clk_info->flags; clk_init.parent_names = parent_names; caps = clk_info->type; diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h index bfc2b9c38a41..147b7df0d657 100644 --- a/drivers/clk/ingenic/cgu.h +++ b/drivers/clk/ingenic/cgu.h @@ -136,6 +136,7 @@ struct ingenic_cgu_custom_info { * struct ingenic_cgu_clk_info - information about a clock * @name: name of the clock * @type: a bitmask formed from CGU_CLK_* values + * @flags: common clock flags to set on this clock * @parents: an array of the indices of potential parents of this clock * within the clock_info array of the CGU, or -1 in entries * which correspond to no valid parent @@ -161,6 +162,8 @@ struct ingenic_cgu_clk_info { CGU_CLK_CUSTOM = BIT(7), } type; + unsigned long flags; + int parents[4]; union { -- 2.35.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RESEND PATCH 2/3] clk: ingenic: Mark critical clocks in Ingenic SoCs 2022-04-11 10:14 [RESEND PATCH 0/3] Clock fixes for Ingenic SoCs Aidan MacDonald 2022-04-11 10:14 ` [RESEND PATCH 1/3] clk: ingenic: Allow specifying common clock flags Aidan MacDonald @ 2022-04-11 10:14 ` Aidan MacDonald 2022-04-22 2:33 ` Stephen Boyd 2022-04-11 10:14 ` [RESEND PATCH 3/3] mips: ingenic: Do not manually reference the CPU clock Aidan MacDonald 2 siblings, 1 reply; 6+ messages in thread From: Aidan MacDonald @ 2022-04-11 10:14 UTC (permalink / raw) To: paul, paulburton, tsbogend, mturquette, sboyd Cc: linux-mips, linux-kernel, linux-clk Consider the CPU, L2 cache, and memory as critical to ensure they are not disabled. Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> Reviewed-by: Paul Cercueil <paul@crapouillou.net> --- drivers/clk/ingenic/jz4725b-cgu.c | 2 ++ drivers/clk/ingenic/jz4740-cgu.c | 2 ++ drivers/clk/ingenic/jz4760-cgu.c | 2 ++ drivers/clk/ingenic/jz4770-cgu.c | 1 + drivers/clk/ingenic/jz4780-cgu.c | 3 +++ drivers/clk/ingenic/x1000-cgu.c | 3 +++ drivers/clk/ingenic/x1830-cgu.c | 3 +++ 7 files changed, 16 insertions(+) diff --git a/drivers/clk/ingenic/jz4725b-cgu.c b/drivers/clk/ingenic/jz4725b-cgu.c index 15d61793f53b..c209a170be5d 100644 --- a/drivers/clk/ingenic/jz4725b-cgu.c +++ b/drivers/clk/ingenic/jz4725b-cgu.c @@ -87,6 +87,7 @@ static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = { [JZ4725B_CLK_CCLK] = { "cclk", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4725B_CLK_PLL, -1, -1, -1 }, .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1, 0, @@ -114,6 +115,7 @@ static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = { [JZ4725B_CLK_MCLK] = { "mclk", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4725B_CLK_PLL, -1, -1, -1 }, .div = { CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1, 0, diff --git a/drivers/clk/ingenic/jz4740-cgu.c b/drivers/clk/ingenic/jz4740-cgu.c index 43ffb62c42bb..2b6f7a9fcea8 100644 --- a/drivers/clk/ingenic/jz4740-cgu.c +++ b/drivers/clk/ingenic/jz4740-cgu.c @@ -102,6 +102,7 @@ static const struct ingenic_cgu_clk_info jz4740_cgu_clocks[] = { [JZ4740_CLK_CCLK] = { "cclk", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4740_CLK_PLL, -1, -1, -1 }, .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1, 0, @@ -129,6 +130,7 @@ static const struct ingenic_cgu_clk_info jz4740_cgu_clocks[] = { [JZ4740_CLK_MCLK] = { "mclk", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4740_CLK_PLL, -1, -1, -1 }, .div = { CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1, 0, diff --git a/drivers/clk/ingenic/jz4760-cgu.c b/drivers/clk/ingenic/jz4760-cgu.c index 8fdd383560fb..7920df2cee1a 100644 --- a/drivers/clk/ingenic/jz4760-cgu.c +++ b/drivers/clk/ingenic/jz4760-cgu.c @@ -143,6 +143,7 @@ static const struct ingenic_cgu_clk_info jz4760_cgu_clocks[] = { [JZ4760_CLK_CCLK] = { "cclk", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4760_CLK_PLL0, }, .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1, 0, @@ -175,6 +176,7 @@ static const struct ingenic_cgu_clk_info jz4760_cgu_clocks[] = { }, [JZ4760_CLK_MCLK] = { "mclk", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4760_CLK_PLL0, }, .div = { CGU_REG_CPCCR, 12, 1, 4, 22, -1, -1, 0, diff --git a/drivers/clk/ingenic/jz4770-cgu.c b/drivers/clk/ingenic/jz4770-cgu.c index 7ef91257630e..02b266c2a537 100644 --- a/drivers/clk/ingenic/jz4770-cgu.c +++ b/drivers/clk/ingenic/jz4770-cgu.c @@ -149,6 +149,7 @@ static const struct ingenic_cgu_clk_info jz4770_cgu_clocks[] = { [JZ4770_CLK_CCLK] = { "cclk", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4770_CLK_PLL0, }, .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1, 0, diff --git a/drivers/clk/ingenic/jz4780-cgu.c b/drivers/clk/ingenic/jz4780-cgu.c index e357c228e0f1..014674486038 100644 --- a/drivers/clk/ingenic/jz4780-cgu.c +++ b/drivers/clk/ingenic/jz4780-cgu.c @@ -341,12 +341,14 @@ static const struct ingenic_cgu_clk_info jz4780_cgu_clocks[] = { [JZ4780_CLK_CPU] = { "cpu", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4780_CLK_CPUMUX, -1, -1, -1 }, .div = { CGU_REG_CLOCKCONTROL, 0, 1, 4, 22, -1, -1 }, }, [JZ4780_CLK_L2CACHE] = { "l2cache", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { JZ4780_CLK_CPUMUX, -1, -1, -1 }, .div = { CGU_REG_CLOCKCONTROL, 4, 1, 4, -1, -1, -1 }, }, @@ -380,6 +382,7 @@ static const struct ingenic_cgu_clk_info jz4780_cgu_clocks[] = { [JZ4780_CLK_DDR] = { "ddr", CGU_CLK_MUX | CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { -1, JZ4780_CLK_SCLKA, JZ4780_CLK_MPLL, -1 }, .mux = { CGU_REG_DDRCDR, 30, 2 }, .div = { CGU_REG_DDRCDR, 0, 1, 4, 29, 28, 27 }, diff --git a/drivers/clk/ingenic/x1000-cgu.c b/drivers/clk/ingenic/x1000-cgu.c index 3c4d5a77ccbd..1bd421cc2ab3 100644 --- a/drivers/clk/ingenic/x1000-cgu.c +++ b/drivers/clk/ingenic/x1000-cgu.c @@ -251,6 +251,7 @@ static const struct ingenic_cgu_clk_info x1000_cgu_clocks[] = { [X1000_CLK_CPU] = { "cpu", CGU_CLK_DIV | CGU_CLK_GATE, + .flags = CLK_IS_CRITICAL, .parents = { X1000_CLK_CPUMUX, -1, -1, -1 }, .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1 }, .gate = { CGU_REG_CLKGR, 30 }, @@ -258,6 +259,7 @@ static const struct ingenic_cgu_clk_info x1000_cgu_clocks[] = { [X1000_CLK_L2CACHE] = { "l2cache", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { X1000_CLK_CPUMUX, -1, -1, -1 }, .div = { CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1 }, }, @@ -290,6 +292,7 @@ static const struct ingenic_cgu_clk_info x1000_cgu_clocks[] = { [X1000_CLK_DDR] = { "ddr", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE, + .flags = CLK_IS_CRITICAL, .parents = { -1, X1000_CLK_SCLKA, X1000_CLK_MPLL, -1 }, .mux = { CGU_REG_DDRCDR, 30, 2 }, .div = { CGU_REG_DDRCDR, 0, 1, 4, 29, 28, 27 }, diff --git a/drivers/clk/ingenic/x1830-cgu.c b/drivers/clk/ingenic/x1830-cgu.c index e01ec2dc7a1a..b08e318aa095 100644 --- a/drivers/clk/ingenic/x1830-cgu.c +++ b/drivers/clk/ingenic/x1830-cgu.c @@ -225,6 +225,7 @@ static const struct ingenic_cgu_clk_info x1830_cgu_clocks[] = { [X1830_CLK_CPU] = { "cpu", CGU_CLK_DIV | CGU_CLK_GATE, + .flags = CLK_IS_CRITICAL, .parents = { X1830_CLK_CPUMUX, -1, -1, -1 }, .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1 }, .gate = { CGU_REG_CLKGR1, 15 }, @@ -232,6 +233,7 @@ static const struct ingenic_cgu_clk_info x1830_cgu_clocks[] = { [X1830_CLK_L2CACHE] = { "l2cache", CGU_CLK_DIV, + .flags = CLK_IS_CRITICAL, .parents = { X1830_CLK_CPUMUX, -1, -1, -1 }, .div = { CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1 }, }, @@ -264,6 +266,7 @@ static const struct ingenic_cgu_clk_info x1830_cgu_clocks[] = { [X1830_CLK_DDR] = { "ddr", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE, + .flags = CLK_IS_CRITICAL, .parents = { -1, X1830_CLK_SCLKA, X1830_CLK_MPLL, -1 }, .mux = { CGU_REG_DDRCDR, 30, 2 }, .div = { CGU_REG_DDRCDR, 0, 1, 4, 29, 28, 27 }, -- 2.35.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 2/3] clk: ingenic: Mark critical clocks in Ingenic SoCs 2022-04-11 10:14 ` [RESEND PATCH 2/3] clk: ingenic: Mark critical clocks in Ingenic SoCs Aidan MacDonald @ 2022-04-22 2:33 ` Stephen Boyd 2022-04-22 15:54 ` Aidan MacDonald 0 siblings, 1 reply; 6+ messages in thread From: Stephen Boyd @ 2022-04-22 2:33 UTC (permalink / raw) To: Aidan MacDonald, mturquette, paul, paulburton, tsbogend Cc: linux-mips, linux-kernel, linux-clk Quoting Aidan MacDonald (2022-04-11 03:14:40) > Consider the CPU, L2 cache, and memory as critical to ensure they > are not disabled. > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> > Reviewed-by: Paul Cercueil <paul@crapouillou.net> > --- General comment, please add a comment around CLK_IS_CRITICAL usage if it isn't very clear why such a clk shouldn't be turned off. Second, is there any point in describing these clks in the kernel and using memory to do that if they're just going to always be on? Wouldn't a dummy clk returned from clk_get() work just as well if anything is grabbing a reference with clk_get()? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH 2/3] clk: ingenic: Mark critical clocks in Ingenic SoCs 2022-04-22 2:33 ` Stephen Boyd @ 2022-04-22 15:54 ` Aidan MacDonald 0 siblings, 0 replies; 6+ messages in thread From: Aidan MacDonald @ 2022-04-22 15:54 UTC (permalink / raw) To: Stephen Boyd Cc: linux-clk, linux-kernel, linux-mips, mturquette, paul, paulburton, tsbogend On Thu, Apr 21, 2022 at 07:33:57PM -0700, Stephen Boyd wrote: > Quoting Aidan MacDonald (2022-04-11 03:14:40) > > Consider the CPU, L2 cache, and memory as critical to ensure they > > are not disabled. > > > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> > > Reviewed-by: Paul Cercueil <paul@crapouillou.net> > > --- > > General comment, please add a comment around CLK_IS_CRITICAL usage if it > isn't very clear why such a clk shouldn't be turned off. Second, is > there any point in describing these clks in the kernel and using memory > to do that if they're just going to always be on? Wouldn't a dummy clk > returned from clk_get() work just as well if anything is grabbing a > reference with clk_get()? I'd guess they're there to keep track of which PLLs are in use, at least for SoCs that have more than one PLL. Using a dummy clock sounds like a bad idea since it won't represent that, and besides the clock configuration is something that can change at runtime so hardcoding it would be foolish. I'll send a v2 with explanatory comments around CLK_IS_CRITICAL. Regards, Aidan ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RESEND PATCH 3/3] mips: ingenic: Do not manually reference the CPU clock 2022-04-11 10:14 [RESEND PATCH 0/3] Clock fixes for Ingenic SoCs Aidan MacDonald 2022-04-11 10:14 ` [RESEND PATCH 1/3] clk: ingenic: Allow specifying common clock flags Aidan MacDonald 2022-04-11 10:14 ` [RESEND PATCH 2/3] clk: ingenic: Mark critical clocks in Ingenic SoCs Aidan MacDonald @ 2022-04-11 10:14 ` Aidan MacDonald 2 siblings, 0 replies; 6+ messages in thread From: Aidan MacDonald @ 2022-04-11 10:14 UTC (permalink / raw) To: paul, paulburton, tsbogend, mturquette, sboyd Cc: linux-mips, linux-kernel, linux-clk It isn't necessary to manually walk the device tree and enable the CPU clock anymore. The CPU and other necessary clocks are now flagged as critical in the clock driver, which accomplishes the same thing in a more declarative fashion. Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> Reviewed-by: Paul Cercueil <paul@crapouillou.net> --- arch/mips/generic/board-ingenic.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/arch/mips/generic/board-ingenic.c b/arch/mips/generic/board-ingenic.c index 3f44f14bdb33..c422bbc890ed 100644 --- a/arch/mips/generic/board-ingenic.c +++ b/arch/mips/generic/board-ingenic.c @@ -131,36 +131,10 @@ static const struct platform_suspend_ops ingenic_pm_ops __maybe_unused = { static int __init ingenic_pm_init(void) { - struct device_node *cpu_node; - struct clk *cpu0_clk; - int ret; - if (boot_cpu_type() == CPU_XBURST) { if (IS_ENABLED(CONFIG_PM_SLEEP)) suspend_set_ops(&ingenic_pm_ops); _machine_halt = ingenic_halt; - - /* - * Unconditionally enable the clock for the first CPU. - * This makes sure that the PLL that feeds the CPU won't be - * stopped while the kernel is running. - */ - cpu_node = of_get_cpu_node(0, NULL); - if (!cpu_node) { - pr_err("Unable to get CPU node\n"); - } else { - cpu0_clk = of_clk_get(cpu_node, 0); - if (IS_ERR(cpu0_clk)) { - pr_err("Unable to get CPU0 clock\n"); - return PTR_ERR(cpu0_clk); - } - - ret = clk_prepare_enable(cpu0_clk); - if (ret) { - pr_err("Unable to enable CPU0 clock\n"); - return ret; - } - } } return 0; -- 2.35.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-22 15:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-11 10:14 [RESEND PATCH 0/3] Clock fixes for Ingenic SoCs Aidan MacDonald 2022-04-11 10:14 ` [RESEND PATCH 1/3] clk: ingenic: Allow specifying common clock flags Aidan MacDonald 2022-04-11 10:14 ` [RESEND PATCH 2/3] clk: ingenic: Mark critical clocks in Ingenic SoCs Aidan MacDonald 2022-04-22 2:33 ` Stephen Boyd 2022-04-22 15:54 ` Aidan MacDonald 2022-04-11 10:14 ` [RESEND PATCH 3/3] mips: ingenic: Do not manually reference the CPU clock Aidan MacDonald
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).