* [PATCH 0/3] clk: meson: g12a: fixes for DVFS
@ 2019-09-19 9:36 ` Neil Armstrong
0 siblings, 0 replies; 27+ messages in thread
From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw)
To: jbrunet
Cc: Neil Armstrong, linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel
This is the first serie of fixes for DVFS support on G12a:
- Patch 1 fixes a rebase issue where a CLK_SET_RATE_NO_REPARENT
appeared on the wrong clock and a SET_RATE_PARENT went missing
- Patch 2 helps CCF use the right clock tree for the sub 1GHz clock range
- Patch 3 fixes an issue when we enter suspend with a non-SYS_PLL CPU clock,
leading to a SYS_PLL never enabled again
Neil Armstrong (3):
clk: meson: g12a: fix cpu clock rate setting
clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes
clk: meson: clk-pll: always enable a critical PLL when setting the
rate
drivers/clk/meson/clk-pll.c | 2 +-
drivers/clk/meson/g12a.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 3 deletions(-)
--
2.22.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 0/3] clk: meson: g12a: fixes for DVFS @ 2019-09-19 9:36 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw) To: jbrunet Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong This is the first serie of fixes for DVFS support on G12a: - Patch 1 fixes a rebase issue where a CLK_SET_RATE_NO_REPARENT appeared on the wrong clock and a SET_RATE_PARENT went missing - Patch 2 helps CCF use the right clock tree for the sub 1GHz clock range - Patch 3 fixes an issue when we enter suspend with a non-SYS_PLL CPU clock, leading to a SYS_PLL never enabled again Neil Armstrong (3): clk: meson: g12a: fix cpu clock rate setting clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes clk: meson: clk-pll: always enable a critical PLL when setting the rate drivers/clk/meson/clk-pll.c | 2 +- drivers/clk/meson/g12a.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) -- 2.22.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 0/3] clk: meson: g12a: fixes for DVFS @ 2019-09-19 9:36 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw) To: jbrunet Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong This is the first serie of fixes for DVFS support on G12a: - Patch 1 fixes a rebase issue where a CLK_SET_RATE_NO_REPARENT appeared on the wrong clock and a SET_RATE_PARENT went missing - Patch 2 helps CCF use the right clock tree for the sub 1GHz clock range - Patch 3 fixes an issue when we enter suspend with a non-SYS_PLL CPU clock, leading to a SYS_PLL never enabled again Neil Armstrong (3): clk: meson: g12a: fix cpu clock rate setting clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes clk: meson: clk-pll: always enable a critical PLL when setting the rate drivers/clk/meson/clk-pll.c | 2 +- drivers/clk/meson/g12a.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) -- 2.22.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/3] clk: meson: g12a: fix cpu clock rate setting 2019-09-19 9:36 ` Neil Armstrong (?) @ 2019-09-19 9:36 ` Neil Armstrong -1 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw) To: jbrunet Cc: Neil Armstrong, linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel CLK_SET_RATE_NO_REPARENT is wrongly set on the g12a cpu premux0 clocks flags, and CLK_SET_RATE_PARENT is required for the g12a cpu premux0 clock and the g12b cpub premux0 clock, otherwise CCF always selects the SYS_PLL clock to feed the cpu cluster. Fixes: ffae8475b90c ("clk: meson: g12a: add notifiers to handle cpu clock change") Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/g12a.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c index ea4c791f106d..33c7e04b4a82 100644 --- a/drivers/clk/meson/g12a.c +++ b/drivers/clk/meson/g12a.c @@ -353,8 +353,7 @@ static struct clk_regmap g12a_cpu_clk_premux0 = { { .hw = &g12a_fclk_div3.hw }, }, .num_parents = 3, - /* This sub-tree is used a parking clock */ - .flags = CLK_SET_RATE_NO_REPARENT, + .flags = CLK_SET_RATE_PARENT, }, }; @@ -533,6 +532,7 @@ static struct clk_regmap g12b_cpub_clk_premux0 = { { .hw = &g12a_fclk_div3.hw }, }, .num_parents = 3, + .flags = CLK_SET_RATE_PARENT, }, }; -- 2.22.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 1/3] clk: meson: g12a: fix cpu clock rate setting @ 2019-09-19 9:36 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw) To: jbrunet Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong CLK_SET_RATE_NO_REPARENT is wrongly set on the g12a cpu premux0 clocks flags, and CLK_SET_RATE_PARENT is required for the g12a cpu premux0 clock and the g12b cpub premux0 clock, otherwise CCF always selects the SYS_PLL clock to feed the cpu cluster. Fixes: ffae8475b90c ("clk: meson: g12a: add notifiers to handle cpu clock change") Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/g12a.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c index ea4c791f106d..33c7e04b4a82 100644 --- a/drivers/clk/meson/g12a.c +++ b/drivers/clk/meson/g12a.c @@ -353,8 +353,7 @@ static struct clk_regmap g12a_cpu_clk_premux0 = { { .hw = &g12a_fclk_div3.hw }, }, .num_parents = 3, - /* This sub-tree is used a parking clock */ - .flags = CLK_SET_RATE_NO_REPARENT, + .flags = CLK_SET_RATE_PARENT, }, }; @@ -533,6 +532,7 @@ static struct clk_regmap g12b_cpub_clk_premux0 = { { .hw = &g12a_fclk_div3.hw }, }, .num_parents = 3, + .flags = CLK_SET_RATE_PARENT, }, }; -- 2.22.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 1/3] clk: meson: g12a: fix cpu clock rate setting @ 2019-09-19 9:36 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw) To: jbrunet Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong CLK_SET_RATE_NO_REPARENT is wrongly set on the g12a cpu premux0 clocks flags, and CLK_SET_RATE_PARENT is required for the g12a cpu premux0 clock and the g12b cpub premux0 clock, otherwise CCF always selects the SYS_PLL clock to feed the cpu cluster. Fixes: ffae8475b90c ("clk: meson: g12a: add notifiers to handle cpu clock change") Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/g12a.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c index ea4c791f106d..33c7e04b4a82 100644 --- a/drivers/clk/meson/g12a.c +++ b/drivers/clk/meson/g12a.c @@ -353,8 +353,7 @@ static struct clk_regmap g12a_cpu_clk_premux0 = { { .hw = &g12a_fclk_div3.hw }, }, .num_parents = 3, - /* This sub-tree is used a parking clock */ - .flags = CLK_SET_RATE_NO_REPARENT, + .flags = CLK_SET_RATE_PARENT, }, }; @@ -533,6 +532,7 @@ static struct clk_regmap g12b_cpub_clk_premux0 = { { .hw = &g12a_fclk_div3.hw }, }, .num_parents = 3, + .flags = CLK_SET_RATE_PARENT, }, }; -- 2.22.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/3] clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes 2019-09-19 9:36 ` Neil Armstrong (?) @ 2019-09-19 9:36 ` Neil Armstrong -1 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw) To: jbrunet Cc: Neil Armstrong, linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel When setting the 100MHz, 500MHz, 666MHz and 1GHz rate for CPU clocks, CCF will use the SYS_PLL to handle these frequencies, but: - using FIXED_PLL derived FCLK_DIV2/DIV3 clocks is more precise - the Amlogic G12A/G12B/SM1 Suspend handling in firmware doesn't handle entering suspend using SYS_PLL for these frequencies Adding CLK_MUX_ROUND_CLOSEST on all the muxes of the non-SYS_PLL cpu clock tree helps CCF always selecting the FCLK_DIV2/DIV3 as source for these frequencies. Fixes: ffae8475b90c ("clk: meson: g12a: add notifiers to handle cpu clock change") Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/g12a.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c index 33c7e04b4a82..b3af61cc6fb9 100644 --- a/drivers/clk/meson/g12a.c +++ b/drivers/clk/meson/g12a.c @@ -343,6 +343,7 @@ static struct clk_regmap g12a_cpu_clk_premux0 = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x3, .shift = 0, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn0_sel", @@ -409,6 +410,7 @@ static struct clk_regmap g12a_cpu_clk_postmux0 = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 2, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn0", @@ -465,6 +467,7 @@ static struct clk_regmap g12a_cpu_clk_dyn = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 10, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn", @@ -484,6 +487,7 @@ static struct clk_regmap g12a_cpu_clk = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk", @@ -503,6 +507,7 @@ static struct clk_regmap g12b_cpu_clk = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk", @@ -522,6 +527,7 @@ static struct clk_regmap g12b_cpub_clk_premux0 = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x3, .shift = 0, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn0_sel", @@ -567,6 +573,7 @@ static struct clk_regmap g12b_cpub_clk_postmux0 = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 2, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn0", @@ -644,6 +651,7 @@ static struct clk_regmap g12b_cpub_clk_dyn = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 10, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn", @@ -663,6 +671,7 @@ static struct clk_regmap g12b_cpub_clk = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk", -- 2.22.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/3] clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes @ 2019-09-19 9:36 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw) To: jbrunet Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong When setting the 100MHz, 500MHz, 666MHz and 1GHz rate for CPU clocks, CCF will use the SYS_PLL to handle these frequencies, but: - using FIXED_PLL derived FCLK_DIV2/DIV3 clocks is more precise - the Amlogic G12A/G12B/SM1 Suspend handling in firmware doesn't handle entering suspend using SYS_PLL for these frequencies Adding CLK_MUX_ROUND_CLOSEST on all the muxes of the non-SYS_PLL cpu clock tree helps CCF always selecting the FCLK_DIV2/DIV3 as source for these frequencies. Fixes: ffae8475b90c ("clk: meson: g12a: add notifiers to handle cpu clock change") Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/g12a.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c index 33c7e04b4a82..b3af61cc6fb9 100644 --- a/drivers/clk/meson/g12a.c +++ b/drivers/clk/meson/g12a.c @@ -343,6 +343,7 @@ static struct clk_regmap g12a_cpu_clk_premux0 = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x3, .shift = 0, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn0_sel", @@ -409,6 +410,7 @@ static struct clk_regmap g12a_cpu_clk_postmux0 = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 2, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn0", @@ -465,6 +467,7 @@ static struct clk_regmap g12a_cpu_clk_dyn = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 10, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn", @@ -484,6 +487,7 @@ static struct clk_regmap g12a_cpu_clk = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk", @@ -503,6 +507,7 @@ static struct clk_regmap g12b_cpu_clk = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk", @@ -522,6 +527,7 @@ static struct clk_regmap g12b_cpub_clk_premux0 = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x3, .shift = 0, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn0_sel", @@ -567,6 +573,7 @@ static struct clk_regmap g12b_cpub_clk_postmux0 = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 2, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn0", @@ -644,6 +651,7 @@ static struct clk_regmap g12b_cpub_clk_dyn = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 10, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn", @@ -663,6 +671,7 @@ static struct clk_regmap g12b_cpub_clk = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk", -- 2.22.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/3] clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes @ 2019-09-19 9:36 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:36 UTC (permalink / raw) To: jbrunet Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong When setting the 100MHz, 500MHz, 666MHz and 1GHz rate for CPU clocks, CCF will use the SYS_PLL to handle these frequencies, but: - using FIXED_PLL derived FCLK_DIV2/DIV3 clocks is more precise - the Amlogic G12A/G12B/SM1 Suspend handling in firmware doesn't handle entering suspend using SYS_PLL for these frequencies Adding CLK_MUX_ROUND_CLOSEST on all the muxes of the non-SYS_PLL cpu clock tree helps CCF always selecting the FCLK_DIV2/DIV3 as source for these frequencies. Fixes: ffae8475b90c ("clk: meson: g12a: add notifiers to handle cpu clock change") Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/g12a.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c index 33c7e04b4a82..b3af61cc6fb9 100644 --- a/drivers/clk/meson/g12a.c +++ b/drivers/clk/meson/g12a.c @@ -343,6 +343,7 @@ static struct clk_regmap g12a_cpu_clk_premux0 = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x3, .shift = 0, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn0_sel", @@ -409,6 +410,7 @@ static struct clk_regmap g12a_cpu_clk_postmux0 = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 2, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn0", @@ -465,6 +467,7 @@ static struct clk_regmap g12a_cpu_clk_dyn = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 10, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk_dyn", @@ -484,6 +487,7 @@ static struct clk_regmap g12a_cpu_clk = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk", @@ -503,6 +507,7 @@ static struct clk_regmap g12b_cpu_clk = { .offset = HHI_SYS_CPU_CLK_CNTL0, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpu_clk", @@ -522,6 +527,7 @@ static struct clk_regmap g12b_cpub_clk_premux0 = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x3, .shift = 0, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn0_sel", @@ -567,6 +573,7 @@ static struct clk_regmap g12b_cpub_clk_postmux0 = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 2, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn0", @@ -644,6 +651,7 @@ static struct clk_regmap g12b_cpub_clk_dyn = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 10, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk_dyn", @@ -663,6 +671,7 @@ static struct clk_regmap g12b_cpub_clk = { .offset = HHI_SYS_CPUB_CLK_CNTL, .mask = 0x1, .shift = 11, + .flags = CLK_MUX_ROUND_CLOSEST, }, .hw.init = &(struct clk_init_data){ .name = "cpub_clk", -- 2.22.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate 2019-09-19 9:36 ` Neil Armstrong (?) @ 2019-09-19 9:38 ` Neil Armstrong -1 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:38 UTC (permalink / raw) To: jbrunet Cc: Neil Armstrong, linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel Make sure we always enable a PLL on a set_rate() when the PLL is flagged as critical. This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the PSCI firmware when resuming from suspend-to-memory, in the case where the CPU was not clocked by the SYS_PLL, but by the fixed PLL fixed divisors. In this particular case, when changing the PLL rate, CCF doesn't handle the fact the PLL could have been disabled in the meantime and set_rate() only changes the rate and never enables it again. Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/clk-pll.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c index ddb1e5634739..8c5adccb7959 100644 --- a/drivers/clk/meson/clk-pll.c +++ b/drivers/clk/meson/clk-pll.c @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, } /* If the pll is stopped, bail out now */ - if (!enabled) + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) return 0; if (meson_clk_pll_enable(hw)) { -- 2.22.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-19 9:38 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:38 UTC (permalink / raw) To: jbrunet Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong Make sure we always enable a PLL on a set_rate() when the PLL is flagged as critical. This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the PSCI firmware when resuming from suspend-to-memory, in the case where the CPU was not clocked by the SYS_PLL, but by the fixed PLL fixed divisors. In this particular case, when changing the PLL rate, CCF doesn't handle the fact the PLL could have been disabled in the meantime and set_rate() only changes the rate and never enables it again. Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/clk-pll.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c index ddb1e5634739..8c5adccb7959 100644 --- a/drivers/clk/meson/clk-pll.c +++ b/drivers/clk/meson/clk-pll.c @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, } /* If the pll is stopped, bail out now */ - if (!enabled) + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) return 0; if (meson_clk_pll_enable(hw)) { -- 2.22.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-19 9:38 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-19 9:38 UTC (permalink / raw) To: jbrunet Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong Make sure we always enable a PLL on a set_rate() when the PLL is flagged as critical. This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the PSCI firmware when resuming from suspend-to-memory, in the case where the CPU was not clocked by the SYS_PLL, but by the fixed PLL fixed divisors. In this particular case, when changing the PLL rate, CCF doesn't handle the fact the PLL could have been disabled in the meantime and set_rate() only changes the rate and never enables it again. Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- drivers/clk/meson/clk-pll.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c index ddb1e5634739..8c5adccb7959 100644 --- a/drivers/clk/meson/clk-pll.c +++ b/drivers/clk/meson/clk-pll.c @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, } /* If the pll is stopped, bail out now */ - if (!enabled) + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) return 0; if (meson_clk_pll_enable(hw)) { -- 2.22.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate 2019-09-19 9:38 ` Neil Armstrong (?) @ 2019-09-19 13:01 ` Jerome Brunet -1 siblings, 0 replies; 27+ messages in thread From: Jerome Brunet @ 2019-09-19 13:01 UTC (permalink / raw) To: Neil Armstrong, Stephen Boyd Cc: Neil Armstrong, linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > Make sure we always enable a PLL on a set_rate() when the PLL is > flagged as critical. > > This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > PSCI firmware when resuming from suspend-to-memory, in the case > where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > fixed divisors. > In this particular case, when changing the PLL rate, CCF doesn't handle > the fact the PLL could have been disabled in the meantime and set_rate() > only changes the rate and never enables it again. > > Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > --- > drivers/clk/meson/clk-pll.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > index ddb1e5634739..8c5adccb7959 100644 > --- a/drivers/clk/meson/clk-pll.c > +++ b/drivers/clk/meson/clk-pll.c > @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > } > > /* If the pll is stopped, bail out now */ > - if (!enabled) > + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) This is surely a work around to the issue at hand but: * Enabling the clock, critical or not, should not be done but the set_rate() callback. This is not the purpose of this callback. * Enabling the clock in such way does not walk the tree. So, if there is ever another PSCI Fw which disable we would get into the same issue again. IOW, This is not specific to the PLL driver so it should not have to deal with this. Since this clock can change out of CCF maybe it should be marked with CLK_GET_RATE_NOCACHE ? When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, in addition to to calling get_rate(), CCF could also call is_enabled() if the clock has CLK_IS_CRITICAL and possibly .enable() ? Stephen, what do you think ? > return 0; > > if (meson_clk_pll_enable(hw)) { > -- > 2.22.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-19 13:01 ` Jerome Brunet 0 siblings, 0 replies; 27+ messages in thread From: Jerome Brunet @ 2019-09-19 13:01 UTC (permalink / raw) To: Neil Armstrong, Stephen Boyd Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > Make sure we always enable a PLL on a set_rate() when the PLL is > flagged as critical. > > This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > PSCI firmware when resuming from suspend-to-memory, in the case > where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > fixed divisors. > In this particular case, when changing the PLL rate, CCF doesn't handle > the fact the PLL could have been disabled in the meantime and set_rate() > only changes the rate and never enables it again. > > Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > --- > drivers/clk/meson/clk-pll.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > index ddb1e5634739..8c5adccb7959 100644 > --- a/drivers/clk/meson/clk-pll.c > +++ b/drivers/clk/meson/clk-pll.c > @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > } > > /* If the pll is stopped, bail out now */ > - if (!enabled) > + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) This is surely a work around to the issue at hand but: * Enabling the clock, critical or not, should not be done but the set_rate() callback. This is not the purpose of this callback. * Enabling the clock in such way does not walk the tree. So, if there is ever another PSCI Fw which disable we would get into the same issue again. IOW, This is not specific to the PLL driver so it should not have to deal with this. Since this clock can change out of CCF maybe it should be marked with CLK_GET_RATE_NOCACHE ? When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, in addition to to calling get_rate(), CCF could also call is_enabled() if the clock has CLK_IS_CRITICAL and possibly .enable() ? Stephen, what do you think ? > return 0; > > if (meson_clk_pll_enable(hw)) { > -- > 2.22.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-19 13:01 ` Jerome Brunet 0 siblings, 0 replies; 27+ messages in thread From: Jerome Brunet @ 2019-09-19 13:01 UTC (permalink / raw) To: Neil Armstrong, Stephen Boyd Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > Make sure we always enable a PLL on a set_rate() when the PLL is > flagged as critical. > > This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > PSCI firmware when resuming from suspend-to-memory, in the case > where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > fixed divisors. > In this particular case, when changing the PLL rate, CCF doesn't handle > the fact the PLL could have been disabled in the meantime and set_rate() > only changes the rate and never enables it again. > > Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > --- > drivers/clk/meson/clk-pll.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > index ddb1e5634739..8c5adccb7959 100644 > --- a/drivers/clk/meson/clk-pll.c > +++ b/drivers/clk/meson/clk-pll.c > @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > } > > /* If the pll is stopped, bail out now */ > - if (!enabled) > + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) This is surely a work around to the issue at hand but: * Enabling the clock, critical or not, should not be done but the set_rate() callback. This is not the purpose of this callback. * Enabling the clock in such way does not walk the tree. So, if there is ever another PSCI Fw which disable we would get into the same issue again. IOW, This is not specific to the PLL driver so it should not have to deal with this. Since this clock can change out of CCF maybe it should be marked with CLK_GET_RATE_NOCACHE ? When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, in addition to to calling get_rate(), CCF could also call is_enabled() if the clock has CLK_IS_CRITICAL and possibly .enable() ? Stephen, what do you think ? > return 0; > > if (meson_clk_pll_enable(hw)) { > -- > 2.22.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate 2019-09-19 13:01 ` Jerome Brunet (?) @ 2019-09-19 17:06 ` Stephen Boyd -1 siblings, 0 replies; 27+ messages in thread From: Stephen Boyd @ 2019-09-19 17:06 UTC (permalink / raw) To: Jerome Brunet, Neil Armstrong Cc: Neil Armstrong, linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel Quoting Jerome Brunet (2019-09-19 06:01:28) > On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > > > Make sure we always enable a PLL on a set_rate() when the PLL is > > flagged as critical. > > > > This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > > PSCI firmware when resuming from suspend-to-memory, in the case > > where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > > fixed divisors. > > In this particular case, when changing the PLL rate, CCF doesn't handle > > the fact the PLL could have been disabled in the meantime and set_rate() > > only changes the rate and never enables it again. > > > > Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > > --- > > drivers/clk/meson/clk-pll.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > > index ddb1e5634739..8c5adccb7959 100644 > > --- a/drivers/clk/meson/clk-pll.c > > +++ b/drivers/clk/meson/clk-pll.c > > @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > > } > > > > /* If the pll is stopped, bail out now */ > > - if (!enabled) > > + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) > > This is surely a work around to the issue at hand but: > > * Enabling the clock, critical or not, should not be done but the > set_rate() callback. This is not the purpose of this callback. > > * Enabling the clock in such way does not walk the tree. So, if there is > ever another PSCI Fw which disable we would get into the same issue > again. IOW, This is not specific to the PLL driver so it should not have > to deal with this. Exactly. > > Since this clock can change out of CCF maybe it should be marked with > CLK_GET_RATE_NOCACHE ? Yes, or figure out a way to make the clk state match what PSCI leaves it in on resume from suspend. > > When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, > in addition to to calling get_rate(), CCF could also call is_enabled() > if the clock has CLK_IS_CRITICAL and possibly .enable() ? This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag specifically means get rate shouldn't be a cached operation. It doesn't relate to the enable state. I hope that you can implement some sort of resume hook that synchronizes the state though so that you don't need to rely on clk_set_rate() or clk_get_rate() to trigger a sync. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-19 17:06 ` Stephen Boyd 0 siblings, 0 replies; 27+ messages in thread From: Stephen Boyd @ 2019-09-19 17:06 UTC (permalink / raw) To: Jerome Brunet, Neil Armstrong Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong Quoting Jerome Brunet (2019-09-19 06:01:28) > On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > > > Make sure we always enable a PLL on a set_rate() when the PLL is > > flagged as critical. > > > > This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > > PSCI firmware when resuming from suspend-to-memory, in the case > > where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > > fixed divisors. > > In this particular case, when changing the PLL rate, CCF doesn't handle > > the fact the PLL could have been disabled in the meantime and set_rate() > > only changes the rate and never enables it again. > > > > Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > > --- > > drivers/clk/meson/clk-pll.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > > index ddb1e5634739..8c5adccb7959 100644 > > --- a/drivers/clk/meson/clk-pll.c > > +++ b/drivers/clk/meson/clk-pll.c > > @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > > } > > > > /* If the pll is stopped, bail out now */ > > - if (!enabled) > > + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) > > This is surely a work around to the issue at hand but: > > * Enabling the clock, critical or not, should not be done but the > set_rate() callback. This is not the purpose of this callback. > > * Enabling the clock in such way does not walk the tree. So, if there is > ever another PSCI Fw which disable we would get into the same issue > again. IOW, This is not specific to the PLL driver so it should not have > to deal with this. Exactly. > > Since this clock can change out of CCF maybe it should be marked with > CLK_GET_RATE_NOCACHE ? Yes, or figure out a way to make the clk state match what PSCI leaves it in on resume from suspend. > > When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, > in addition to to calling get_rate(), CCF could also call is_enabled() > if the clock has CLK_IS_CRITICAL and possibly .enable() ? This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag specifically means get rate shouldn't be a cached operation. It doesn't relate to the enable state. I hope that you can implement some sort of resume hook that synchronizes the state though so that you don't need to rely on clk_set_rate() or clk_get_rate() to trigger a sync. _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-19 17:06 ` Stephen Boyd 0 siblings, 0 replies; 27+ messages in thread From: Stephen Boyd @ 2019-09-19 17:06 UTC (permalink / raw) To: Jerome Brunet, Neil Armstrong Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel, Neil Armstrong Quoting Jerome Brunet (2019-09-19 06:01:28) > On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > > > Make sure we always enable a PLL on a set_rate() when the PLL is > > flagged as critical. > > > > This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > > PSCI firmware when resuming from suspend-to-memory, in the case > > where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > > fixed divisors. > > In this particular case, when changing the PLL rate, CCF doesn't handle > > the fact the PLL could have been disabled in the meantime and set_rate() > > only changes the rate and never enables it again. > > > > Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > > --- > > drivers/clk/meson/clk-pll.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > > index ddb1e5634739..8c5adccb7959 100644 > > --- a/drivers/clk/meson/clk-pll.c > > +++ b/drivers/clk/meson/clk-pll.c > > @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > > } > > > > /* If the pll is stopped, bail out now */ > > - if (!enabled) > > + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) > > This is surely a work around to the issue at hand but: > > * Enabling the clock, critical or not, should not be done but the > set_rate() callback. This is not the purpose of this callback. > > * Enabling the clock in such way does not walk the tree. So, if there is > ever another PSCI Fw which disable we would get into the same issue > again. IOW, This is not specific to the PLL driver so it should not have > to deal with this. Exactly. > > Since this clock can change out of CCF maybe it should be marked with > CLK_GET_RATE_NOCACHE ? Yes, or figure out a way to make the clk state match what PSCI leaves it in on resume from suspend. > > When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, > in addition to to calling get_rate(), CCF could also call is_enabled() > if the clock has CLK_IS_CRITICAL and possibly .enable() ? This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag specifically means get rate shouldn't be a cached operation. It doesn't relate to the enable state. I hope that you can implement some sort of resume hook that synchronizes the state though so that you don't need to rely on clk_set_rate() or clk_get_rate() to trigger a sync. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate 2019-09-19 17:06 ` Stephen Boyd (?) @ 2019-09-20 8:06 ` Neil Armstrong -1 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-20 8:06 UTC (permalink / raw) To: Stephen Boyd, Jerome Brunet Cc: linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel Hi Stephen, On 19/09/2019 19:06, Stephen Boyd wrote: > Quoting Jerome Brunet (2019-09-19 06:01:28) >> On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: >> >>> Make sure we always enable a PLL on a set_rate() when the PLL is >>> flagged as critical. >>> >>> This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the >>> PSCI firmware when resuming from suspend-to-memory, in the case >>> where the CPU was not clocked by the SYS_PLL, but by the fixed PLL >>> fixed divisors. >>> In this particular case, when changing the PLL rate, CCF doesn't handle >>> the fact the PLL could have been disabled in the meantime and set_rate() >>> only changes the rate and never enables it again. >>> >>> Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') >>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >>> --- >>> drivers/clk/meson/clk-pll.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c >>> index ddb1e5634739..8c5adccb7959 100644 >>> --- a/drivers/clk/meson/clk-pll.c >>> +++ b/drivers/clk/meson/clk-pll.c >>> @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, >>> } >>> >>> /* If the pll is stopped, bail out now */ >>> - if (!enabled) >>> + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) >> >> This is surely a work around to the issue at hand but: >> >> * Enabling the clock, critical or not, should not be done but the >> set_rate() callback. This is not the purpose of this callback. >> >> * Enabling the clock in such way does not walk the tree. So, if there is >> ever another PSCI Fw which disable we would get into the same issue >> again. IOW, This is not specific to the PLL driver so it should not have >> to deal with this. > > Exactly. > >> >> Since this clock can change out of CCF maybe it should be marked with >> CLK_GET_RATE_NOCACHE ? > > Yes, or figure out a way to make the clk state match what PSCI leaves it > in on resume from suspend. > > >> >> When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, >> in addition to to calling get_rate(), CCF could also call is_enabled() >> if the clock has CLK_IS_CRITICAL and possibly .enable() ? > > This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag > specifically means get rate shouldn't be a cached operation. It doesn't > relate to the enable state. I hope that you can implement some sort of > resume hook that synchronizes the state though so that you don't need to > rely on clk_set_rate() or clk_get_rate() to trigger a sync. > It's exactly the goal of [1] where I resync a clock tree after a resume. But I don't check the enable state, would you mean that: if core->ops->enable && core->enable_count > 0 && !clk_core_is_enabled(core) core->ops->enable(core->hw) along the parent/rate resync ? Isn't that dangerous ? [1] https://patchwork.kernel.org/patch/11152101/ Neil ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-20 8:06 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-20 8:06 UTC (permalink / raw) To: Stephen Boyd, Jerome Brunet Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel Hi Stephen, On 19/09/2019 19:06, Stephen Boyd wrote: > Quoting Jerome Brunet (2019-09-19 06:01:28) >> On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: >> >>> Make sure we always enable a PLL on a set_rate() when the PLL is >>> flagged as critical. >>> >>> This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the >>> PSCI firmware when resuming from suspend-to-memory, in the case >>> where the CPU was not clocked by the SYS_PLL, but by the fixed PLL >>> fixed divisors. >>> In this particular case, when changing the PLL rate, CCF doesn't handle >>> the fact the PLL could have been disabled in the meantime and set_rate() >>> only changes the rate and never enables it again. >>> >>> Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') >>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >>> --- >>> drivers/clk/meson/clk-pll.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c >>> index ddb1e5634739..8c5adccb7959 100644 >>> --- a/drivers/clk/meson/clk-pll.c >>> +++ b/drivers/clk/meson/clk-pll.c >>> @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, >>> } >>> >>> /* If the pll is stopped, bail out now */ >>> - if (!enabled) >>> + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) >> >> This is surely a work around to the issue at hand but: >> >> * Enabling the clock, critical or not, should not be done but the >> set_rate() callback. This is not the purpose of this callback. >> >> * Enabling the clock in such way does not walk the tree. So, if there is >> ever another PSCI Fw which disable we would get into the same issue >> again. IOW, This is not specific to the PLL driver so it should not have >> to deal with this. > > Exactly. > >> >> Since this clock can change out of CCF maybe it should be marked with >> CLK_GET_RATE_NOCACHE ? > > Yes, or figure out a way to make the clk state match what PSCI leaves it > in on resume from suspend. > > >> >> When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, >> in addition to to calling get_rate(), CCF could also call is_enabled() >> if the clock has CLK_IS_CRITICAL and possibly .enable() ? > > This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag > specifically means get rate shouldn't be a cached operation. It doesn't > relate to the enable state. I hope that you can implement some sort of > resume hook that synchronizes the state though so that you don't need to > rely on clk_set_rate() or clk_get_rate() to trigger a sync. > It's exactly the goal of [1] where I resync a clock tree after a resume. But I don't check the enable state, would you mean that: if core->ops->enable && core->enable_count > 0 && !clk_core_is_enabled(core) core->ops->enable(core->hw) along the parent/rate resync ? Isn't that dangerous ? [1] https://patchwork.kernel.org/patch/11152101/ Neil _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-20 8:06 ` Neil Armstrong 0 siblings, 0 replies; 27+ messages in thread From: Neil Armstrong @ 2019-09-20 8:06 UTC (permalink / raw) To: Stephen Boyd, Jerome Brunet Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel Hi Stephen, On 19/09/2019 19:06, Stephen Boyd wrote: > Quoting Jerome Brunet (2019-09-19 06:01:28) >> On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: >> >>> Make sure we always enable a PLL on a set_rate() when the PLL is >>> flagged as critical. >>> >>> This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the >>> PSCI firmware when resuming from suspend-to-memory, in the case >>> where the CPU was not clocked by the SYS_PLL, but by the fixed PLL >>> fixed divisors. >>> In this particular case, when changing the PLL rate, CCF doesn't handle >>> the fact the PLL could have been disabled in the meantime and set_rate() >>> only changes the rate and never enables it again. >>> >>> Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') >>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >>> --- >>> drivers/clk/meson/clk-pll.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c >>> index ddb1e5634739..8c5adccb7959 100644 >>> --- a/drivers/clk/meson/clk-pll.c >>> +++ b/drivers/clk/meson/clk-pll.c >>> @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, >>> } >>> >>> /* If the pll is stopped, bail out now */ >>> - if (!enabled) >>> + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) >> >> This is surely a work around to the issue at hand but: >> >> * Enabling the clock, critical or not, should not be done but the >> set_rate() callback. This is not the purpose of this callback. >> >> * Enabling the clock in such way does not walk the tree. So, if there is >> ever another PSCI Fw which disable we would get into the same issue >> again. IOW, This is not specific to the PLL driver so it should not have >> to deal with this. > > Exactly. > >> >> Since this clock can change out of CCF maybe it should be marked with >> CLK_GET_RATE_NOCACHE ? > > Yes, or figure out a way to make the clk state match what PSCI leaves it > in on resume from suspend. > > >> >> When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, >> in addition to to calling get_rate(), CCF could also call is_enabled() >> if the clock has CLK_IS_CRITICAL and possibly .enable() ? > > This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag > specifically means get rate shouldn't be a cached operation. It doesn't > relate to the enable state. I hope that you can implement some sort of > resume hook that synchronizes the state though so that you don't need to > rely on clk_set_rate() or clk_get_rate() to trigger a sync. > It's exactly the goal of [1] where I resync a clock tree after a resume. But I don't check the enable state, would you mean that: if core->ops->enable && core->enable_count > 0 && !clk_core_is_enabled(core) core->ops->enable(core->hw) along the parent/rate resync ? Isn't that dangerous ? [1] https://patchwork.kernel.org/patch/11152101/ Neil _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate 2019-09-20 8:06 ` Neil Armstrong (?) @ 2019-09-20 16:52 ` Stephen Boyd -1 siblings, 0 replies; 27+ messages in thread From: Stephen Boyd @ 2019-09-20 16:52 UTC (permalink / raw) To: Jerome Brunet, Neil Armstrong Cc: linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel Quoting Neil Armstrong (2019-09-20 01:06:58) > Hi Stephen, > > On 19/09/2019 19:06, Stephen Boyd wrote: > > Quoting Jerome Brunet (2019-09-19 06:01:28) > >> On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > >> > >>> Make sure we always enable a PLL on a set_rate() when the PLL is > >>> flagged as critical. > >>> > >>> This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > >>> PSCI firmware when resuming from suspend-to-memory, in the case > >>> where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > >>> fixed divisors. > >>> In this particular case, when changing the PLL rate, CCF doesn't handle > >>> the fact the PLL could have been disabled in the meantime and set_rate() > >>> only changes the rate and never enables it again. > >>> > >>> Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > >>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > >>> --- > >>> drivers/clk/meson/clk-pll.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > >>> index ddb1e5634739..8c5adccb7959 100644 > >>> --- a/drivers/clk/meson/clk-pll.c > >>> +++ b/drivers/clk/meson/clk-pll.c > >>> @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > >>> } > >>> > >>> /* If the pll is stopped, bail out now */ > >>> - if (!enabled) > >>> + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) > >> > >> This is surely a work around to the issue at hand but: > >> > >> * Enabling the clock, critical or not, should not be done but the > >> set_rate() callback. This is not the purpose of this callback. > >> > >> * Enabling the clock in such way does not walk the tree. So, if there is > >> ever another PSCI Fw which disable we would get into the same issue > >> again. IOW, This is not specific to the PLL driver so it should not have > >> to deal with this. > > > > Exactly. > > > >> > >> Since this clock can change out of CCF maybe it should be marked with > >> CLK_GET_RATE_NOCACHE ? > > > > Yes, or figure out a way to make the clk state match what PSCI leaves it > > in on resume from suspend. > > > > > >> > >> When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, > >> in addition to to calling get_rate(), CCF could also call is_enabled() > >> if the clock has CLK_IS_CRITICAL and possibly .enable() ? > > > > This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag > > specifically means get rate shouldn't be a cached operation. It doesn't > > relate to the enable state. I hope that you can implement some sort of > > resume hook that synchronizes the state though so that you don't need to > > rely on clk_set_rate() or clk_get_rate() to trigger a sync. > > > > It's exactly the goal of [1] where I resync a clock tree after a resume. Ok. I haven't looked at that series yet. We can move this discussion there if you like. > > But I don't check the enable state, would you mean that: > if core->ops->enable && core->enable_count > 0 && !clk_core_is_enabled(core) > core->ops->enable(core->hw) > > along the parent/rate resync ? > > Isn't that dangerous ? > Why is it dangerous? If the clk state is lost across suspend/resume we need to restore the state of the clk somehow. One way would be to have the clk driver tell the framework that this clk is now off and to drop the enable counts for any consumers. Then consumers will need to call clk_enable() again to turn the clk back on in the consumer resume callback. Or we can try to be smarter/nicer and restore the clk state to what the consumer is expecting across suspend/resume. I haven't thought about what is better or worse. On the one hand, device drivers already handle some things not being saved/restored during system low power modes. But on the other hand I don't know what the policy is for external resources that a device uses, such as clks or regulators or pinctrl muxing, etc. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-20 16:52 ` Stephen Boyd 0 siblings, 0 replies; 27+ messages in thread From: Stephen Boyd @ 2019-09-20 16:52 UTC (permalink / raw) To: Jerome Brunet, Neil Armstrong Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel Quoting Neil Armstrong (2019-09-20 01:06:58) > Hi Stephen, > > On 19/09/2019 19:06, Stephen Boyd wrote: > > Quoting Jerome Brunet (2019-09-19 06:01:28) > >> On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > >> > >>> Make sure we always enable a PLL on a set_rate() when the PLL is > >>> flagged as critical. > >>> > >>> This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > >>> PSCI firmware when resuming from suspend-to-memory, in the case > >>> where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > >>> fixed divisors. > >>> In this particular case, when changing the PLL rate, CCF doesn't handle > >>> the fact the PLL could have been disabled in the meantime and set_rate() > >>> only changes the rate and never enables it again. > >>> > >>> Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > >>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > >>> --- > >>> drivers/clk/meson/clk-pll.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > >>> index ddb1e5634739..8c5adccb7959 100644 > >>> --- a/drivers/clk/meson/clk-pll.c > >>> +++ b/drivers/clk/meson/clk-pll.c > >>> @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > >>> } > >>> > >>> /* If the pll is stopped, bail out now */ > >>> - if (!enabled) > >>> + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) > >> > >> This is surely a work around to the issue at hand but: > >> > >> * Enabling the clock, critical or not, should not be done but the > >> set_rate() callback. This is not the purpose of this callback. > >> > >> * Enabling the clock in such way does not walk the tree. So, if there is > >> ever another PSCI Fw which disable we would get into the same issue > >> again. IOW, This is not specific to the PLL driver so it should not have > >> to deal with this. > > > > Exactly. > > > >> > >> Since this clock can change out of CCF maybe it should be marked with > >> CLK_GET_RATE_NOCACHE ? > > > > Yes, or figure out a way to make the clk state match what PSCI leaves it > > in on resume from suspend. > > > > > >> > >> When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, > >> in addition to to calling get_rate(), CCF could also call is_enabled() > >> if the clock has CLK_IS_CRITICAL and possibly .enable() ? > > > > This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag > > specifically means get rate shouldn't be a cached operation. It doesn't > > relate to the enable state. I hope that you can implement some sort of > > resume hook that synchronizes the state though so that you don't need to > > rely on clk_set_rate() or clk_get_rate() to trigger a sync. > > > > It's exactly the goal of [1] where I resync a clock tree after a resume. Ok. I haven't looked at that series yet. We can move this discussion there if you like. > > But I don't check the enable state, would you mean that: > if core->ops->enable && core->enable_count > 0 && !clk_core_is_enabled(core) > core->ops->enable(core->hw) > > along the parent/rate resync ? > > Isn't that dangerous ? > Why is it dangerous? If the clk state is lost across suspend/resume we need to restore the state of the clk somehow. One way would be to have the clk driver tell the framework that this clk is now off and to drop the enable counts for any consumers. Then consumers will need to call clk_enable() again to turn the clk back on in the consumer resume callback. Or we can try to be smarter/nicer and restore the clk state to what the consumer is expecting across suspend/resume. I haven't thought about what is better or worse. On the one hand, device drivers already handle some things not being saved/restored during system low power modes. But on the other hand I don't know what the policy is for external resources that a device uses, such as clks or regulators or pinctrl muxing, etc. _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate @ 2019-09-20 16:52 ` Stephen Boyd 0 siblings, 0 replies; 27+ messages in thread From: Stephen Boyd @ 2019-09-20 16:52 UTC (permalink / raw) To: Jerome Brunet, Neil Armstrong Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel Quoting Neil Armstrong (2019-09-20 01:06:58) > Hi Stephen, > > On 19/09/2019 19:06, Stephen Boyd wrote: > > Quoting Jerome Brunet (2019-09-19 06:01:28) > >> On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote: > >> > >>> Make sure we always enable a PLL on a set_rate() when the PLL is > >>> flagged as critical. > >>> > >>> This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the > >>> PSCI firmware when resuming from suspend-to-memory, in the case > >>> where the CPU was not clocked by the SYS_PLL, but by the fixed PLL > >>> fixed divisors. > >>> In this particular case, when changing the PLL rate, CCF doesn't handle > >>> the fact the PLL could have been disabled in the meantime and set_rate() > >>> only changes the rate and never enables it again. > >>> > >>> Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled') > >>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > >>> --- > >>> drivers/clk/meson/clk-pll.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > >>> index ddb1e5634739..8c5adccb7959 100644 > >>> --- a/drivers/clk/meson/clk-pll.c > >>> +++ b/drivers/clk/meson/clk-pll.c > >>> @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate, > >>> } > >>> > >>> /* If the pll is stopped, bail out now */ > >>> - if (!enabled) > >>> + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled) > >> > >> This is surely a work around to the issue at hand but: > >> > >> * Enabling the clock, critical or not, should not be done but the > >> set_rate() callback. This is not the purpose of this callback. > >> > >> * Enabling the clock in such way does not walk the tree. So, if there is > >> ever another PSCI Fw which disable we would get into the same issue > >> again. IOW, This is not specific to the PLL driver so it should not have > >> to deal with this. > > > > Exactly. > > > >> > >> Since this clock can change out of CCF maybe it should be marked with > >> CLK_GET_RATE_NOCACHE ? > > > > Yes, or figure out a way to make the clk state match what PSCI leaves it > > in on resume from suspend. > > > > > >> > >> When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree, > >> in addition to to calling get_rate(), CCF could also call is_enabled() > >> if the clock has CLK_IS_CRITICAL and possibly .enable() ? > > > > This logic should go under a new flag. The CLK_GET_RATE_NOCACHE flag > > specifically means get rate shouldn't be a cached operation. It doesn't > > relate to the enable state. I hope that you can implement some sort of > > resume hook that synchronizes the state though so that you don't need to > > rely on clk_set_rate() or clk_get_rate() to trigger a sync. > > > > It's exactly the goal of [1] where I resync a clock tree after a resume. Ok. I haven't looked at that series yet. We can move this discussion there if you like. > > But I don't check the enable state, would you mean that: > if core->ops->enable && core->enable_count > 0 && !clk_core_is_enabled(core) > core->ops->enable(core->hw) > > along the parent/rate resync ? > > Isn't that dangerous ? > Why is it dangerous? If the clk state is lost across suspend/resume we need to restore the state of the clk somehow. One way would be to have the clk driver tell the framework that this clk is now off and to drop the enable counts for any consumers. Then consumers will need to call clk_enable() again to turn the clk back on in the consumer resume callback. Or we can try to be smarter/nicer and restore the clk state to what the consumer is expecting across suspend/resume. I haven't thought about what is better or worse. On the one hand, device drivers already handle some things not being saved/restored during system low power modes. But on the other hand I don't know what the policy is for external resources that a device uses, such as clks or regulators or pinctrl muxing, etc. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] clk: meson: g12a: fixes for DVFS 2019-09-19 9:36 ` Neil Armstrong (?) @ 2019-10-01 13:15 ` Jerome Brunet -1 siblings, 0 replies; 27+ messages in thread From: Jerome Brunet @ 2019-10-01 13:15 UTC (permalink / raw) To: Neil Armstrong; +Cc: linux-clk, linux-amlogic, linux-arm-kernel, linux-kernel On Thu 19 Sep 2019 at 11:36, Neil Armstrong <narmstrong@baylibre.com> wrote: > This is the first serie of fixes for DVFS support on G12a: > - Patch 1 fixes a rebase issue where a CLK_SET_RATE_NO_REPARENT > appeared on the wrong clock and a SET_RATE_PARENT went missing > - Patch 2 helps CCF use the right clock tree for the sub 1GHz clock range > - Patch 3 fixes an issue when we enter suspend with a non-SYS_PLL CPU clock, > leading to a SYS_PLL never enabled again > > Neil Armstrong (3): > clk: meson: g12a: fix cpu clock rate setting > clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes > clk: meson: clk-pll: always enable a critical PLL when setting the > rate > > drivers/clk/meson/clk-pll.c | 2 +- > drivers/clk/meson/g12a.c | 13 +++++++++++-- > 2 files changed, 12 insertions(+), 3 deletions(-) Applied the 2 first fixes. Thx ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] clk: meson: g12a: fixes for DVFS @ 2019-10-01 13:15 ` Jerome Brunet 0 siblings, 0 replies; 27+ messages in thread From: Jerome Brunet @ 2019-10-01 13:15 UTC (permalink / raw) To: Neil Armstrong; +Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel On Thu 19 Sep 2019 at 11:36, Neil Armstrong <narmstrong@baylibre.com> wrote: > This is the first serie of fixes for DVFS support on G12a: > - Patch 1 fixes a rebase issue where a CLK_SET_RATE_NO_REPARENT > appeared on the wrong clock and a SET_RATE_PARENT went missing > - Patch 2 helps CCF use the right clock tree for the sub 1GHz clock range > - Patch 3 fixes an issue when we enter suspend with a non-SYS_PLL CPU clock, > leading to a SYS_PLL never enabled again > > Neil Armstrong (3): > clk: meson: g12a: fix cpu clock rate setting > clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes > clk: meson: clk-pll: always enable a critical PLL when setting the > rate > > drivers/clk/meson/clk-pll.c | 2 +- > drivers/clk/meson/g12a.c | 13 +++++++++++-- > 2 files changed, 12 insertions(+), 3 deletions(-) Applied the 2 first fixes. Thx _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] clk: meson: g12a: fixes for DVFS @ 2019-10-01 13:15 ` Jerome Brunet 0 siblings, 0 replies; 27+ messages in thread From: Jerome Brunet @ 2019-10-01 13:15 UTC (permalink / raw) To: Neil Armstrong; +Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel On Thu 19 Sep 2019 at 11:36, Neil Armstrong <narmstrong@baylibre.com> wrote: > This is the first serie of fixes for DVFS support on G12a: > - Patch 1 fixes a rebase issue where a CLK_SET_RATE_NO_REPARENT > appeared on the wrong clock and a SET_RATE_PARENT went missing > - Patch 2 helps CCF use the right clock tree for the sub 1GHz clock range > - Patch 3 fixes an issue when we enter suspend with a non-SYS_PLL CPU clock, > leading to a SYS_PLL never enabled again > > Neil Armstrong (3): > clk: meson: g12a: fix cpu clock rate setting > clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes > clk: meson: clk-pll: always enable a critical PLL when setting the > rate > > drivers/clk/meson/clk-pll.c | 2 +- > drivers/clk/meson/g12a.c | 13 +++++++++++-- > 2 files changed, 12 insertions(+), 3 deletions(-) Applied the 2 first fixes. Thx _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2019-10-01 13:16 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-19 9:36 [PATCH 0/3] clk: meson: g12a: fixes for DVFS Neil Armstrong 2019-09-19 9:36 ` Neil Armstrong 2019-09-19 9:36 ` Neil Armstrong 2019-09-19 9:36 ` [PATCH 1/3] clk: meson: g12a: fix cpu clock rate setting Neil Armstrong 2019-09-19 9:36 ` Neil Armstrong 2019-09-19 9:36 ` Neil Armstrong 2019-09-19 9:36 ` [PATCH 2/3] clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes Neil Armstrong 2019-09-19 9:36 ` Neil Armstrong 2019-09-19 9:36 ` Neil Armstrong 2019-09-19 9:38 ` [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate Neil Armstrong 2019-09-19 9:38 ` Neil Armstrong 2019-09-19 9:38 ` Neil Armstrong 2019-09-19 13:01 ` Jerome Brunet 2019-09-19 13:01 ` Jerome Brunet 2019-09-19 13:01 ` Jerome Brunet 2019-09-19 17:06 ` Stephen Boyd 2019-09-19 17:06 ` Stephen Boyd 2019-09-19 17:06 ` Stephen Boyd 2019-09-20 8:06 ` Neil Armstrong 2019-09-20 8:06 ` Neil Armstrong 2019-09-20 8:06 ` Neil Armstrong 2019-09-20 16:52 ` Stephen Boyd 2019-09-20 16:52 ` Stephen Boyd 2019-09-20 16:52 ` Stephen Boyd 2019-10-01 13:15 ` [PATCH 0/3] clk: meson: g12a: fixes for DVFS Jerome Brunet 2019-10-01 13:15 ` Jerome Brunet 2019-10-01 13:15 ` Jerome Brunet
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.