LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH v2 0/6] Meson8b: make the CPU clock mutable
@ 2018-11-15 22:40 Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 1/6] clk: meson: clk-pll: check if the clock is already enabled Martin Blumenstingl
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2018-11-15 22:40 UTC (permalink / raw)
  To: linux-amlogic, linux-clk, jbrunet, narmstrong
  Cc: linux-arm-kernel, linux-kernel, mturquette, sboyd, Martin Blumenstingl

This allows changing the CPU clock on the 32-bit Amlogic Meson SoCs
(Meson8, Meson8b and Meson8m2).
CPU frequency scaling will be enabled with a separate series by adding
the CPU clock and the OPP tables to meson8.dtsi and meson8b.dtsi.

While changing the CPU frequency (sys_pll or any of it's post-dividers)
we need to run the CPU clock off the XTAL clock. Otherwise the system
will lock up because we need to disable the sys_pll to change it's
rate.

This also makes the clk-pll's .enable hook a no-op if the clock is
already enabled. Otherwise we're getting lockups when calling the
first clk_{prepare_}enable on the sys_pll or any of it's children (as
the CCF propagates the enable event up to the sys_pll). This is because
the .enable hook unconditionally disables and enables the clock.
However, we can't disable that clock (not even temporarily) if the CPU
is running off sys_pll.

Additionally this adds support for more M/N combinations in sys_pll to
achieve all of the OPPs on Meson8b and all OPPs <= 1608 MHz on Meson8
and Meson8m2.

Compared to Amlogic's 3.10 kernel there's one notable difference: we
are actually allowing changes to the sys_pll. Amlogic's kernel sets
sys_pll to a fixed rate during boot and then uses a timer generate a
"virtual clock rate" by toggling between various dividers (for example:
sys_pll is set to 1536MHz. to achieve 1008MHz they are toggling every
2500us between 1536MHZ and 768MHz so the average over <period, for
example one second> is 1008MHz).
I could reproduce any situation where changing sys_pll failed (for
example due to high temperature). To prove that I ran "stress --cpu 4"
for multiple hours and then cycled through all available CPU
frequencies (while keeping "stress" running in the background). This
worked fine on my Meson8b Odroid-C1 and EC-100 boards as well as my
Meson8m2 board.


Dependencies:
This series is built on top of the latest clk-meson.git tree and the
patches from my other series [1] "Meson8b: fixes for the cpu_scale_div
clock".


Changes since v1 at [0]:
- re-ordered patches as suggested by Jerome: keep all fixes first, then
  the new "features"
- squashed patches "clk-pll: check if the clock is already enabled" and
  "clk-pll: add the is_enabled function in the clk_ops". This also
  allows calling clk_hw_is_enabled() from meson_clk_pll_enable()
  instead of calling meson_clk_pll_is_enabled() directly (this matches
  the implementation of sclk-div.c)
- documented the dependencies of this series in the cover-letter
- dropped "RFC" prefix
- collected Jerome's Acked-/Reviewed-by's (thanks for the quick
  response!)


[0] https://patchwork.kernel.org/cover/10683317/
[1] https://patchwork.kernel.org/cover/10617617/


Martin Blumenstingl (6):
  clk: meson: clk-pll: check if the clock is already enabled
  clk: meson: meson8b: do not use cpu_div3 for cpu_scale_out_sel
  clk: meson: meson8b: mark the CPU clock as CLK_IS_CRITICAL
  clk: meson: meson8b: add support for more M/N values in sys_pll
  clk: meson: meson8b: run from the XTAL when changing the CPU frequency
  clk: meson: meson8b: allow changing the CPU clock tree

 drivers/clk/meson/clk-pll.c | 19 ++++++++
 drivers/clk/meson/meson8b.c | 94 +++++++++++++++++++++++++++++++++----
 2 files changed, 104 insertions(+), 9 deletions(-)

-- 
2.19.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/6] clk: meson: clk-pll: check if the clock is already enabled
  2018-11-15 22:40 [PATCH v2 0/6] Meson8b: make the CPU clock mutable Martin Blumenstingl
@ 2018-11-15 22:40 ` Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 2/6] clk: meson: meson8b: do not use cpu_div3 for cpu_scale_out_sel Martin Blumenstingl
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2018-11-15 22:40 UTC (permalink / raw)
  To: linux-amlogic, linux-clk, jbrunet, narmstrong
  Cc: linux-arm-kernel, linux-kernel, mturquette, sboyd, Martin Blumenstingl

Since commit 6f888e7bc7bd58 ("clk: meson: clk-pll: add enable bit") our
PLLs also support the "enable" bit. Currently meson_clk_pll_enable
unconditionally resets the PLL, enables it, takes it out of reset and
waits until it is locked.

This works fine for our current clock trees. However, there will be a
problem once we allow modifications to sys_pll on Meson8, Meson8b and
Meson8m2 (which will be required for CPU frequency scaling):
the CPU clock is derived from the sys_pll clock. Once clk_enable is
called on the CPU clock this will be propagated by the common clock
framework up until the sys_pll clock. If we reset the PLL
unconditionally in meson_clk_pll_enable the CPU will be stopped (on
Meson8, Meson8b and Meson8m2).
To prevent this we simply check if the PLL is already enabled and do
reset the PLL if it's already enabled and locked.

Now that we have a utility function to check whether the PLL is enabled
we can also pass that to our clk_ops to let the common clock framework
know about the status of the hardware clock.
For now this is of limited use since the only common clock framework's
internal "disabled unused clocks" mechanism checks for this. Everything
else still uses the ref-counting (internal to the common clock
framework) when clk_enable is called.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/clk-pll.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index f5b5b3fabe3c..afffc1547e20 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -200,11 +200,28 @@ static void meson_clk_pll_init(struct clk_hw *hw)
 	}
 }
 
+static int meson_clk_pll_is_enabled(struct clk_hw *hw)
+{
+	struct clk_regmap *clk = to_clk_regmap(hw);
+	struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
+
+	if (meson_parm_read(clk->map, &pll->rst) ||
+	    !meson_parm_read(clk->map, &pll->en) ||
+	    !meson_parm_read(clk->map, &pll->l))
+		return 0;
+
+	return 1;
+}
+
 static int meson_clk_pll_enable(struct clk_hw *hw)
 {
 	struct clk_regmap *clk = to_clk_regmap(hw);
 	struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
 
+	/* do nothing if the PLL is already enabled */
+	if (clk_hw_is_enabled(hw))
+		return 0;
+
 	/* Make sure the pll is in reset */
 	meson_parm_write(clk->map, &pll->rst, 1);
 
@@ -288,10 +305,12 @@ const struct clk_ops meson_clk_pll_ops = {
 	.recalc_rate	= meson_clk_pll_recalc_rate,
 	.round_rate	= meson_clk_pll_round_rate,
 	.set_rate	= meson_clk_pll_set_rate,
+	.is_enabled	= meson_clk_pll_is_enabled,
 	.enable		= meson_clk_pll_enable,
 	.disable	= meson_clk_pll_disable
 };
 
 const struct clk_ops meson_clk_pll_ro_ops = {
 	.recalc_rate	= meson_clk_pll_recalc_rate,
+	.is_enabled	= meson_clk_pll_is_enabled,
 };
-- 
2.19.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/6] clk: meson: meson8b: do not use cpu_div3 for cpu_scale_out_sel
  2018-11-15 22:40 [PATCH v2 0/6] Meson8b: make the CPU clock mutable Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 1/6] clk: meson: clk-pll: check if the clock is already enabled Martin Blumenstingl
@ 2018-11-15 22:40 ` Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 3/6] clk: meson: meson8b: mark the CPU clock as CLK_IS_CRITICAL Martin Blumenstingl
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2018-11-15 22:40 UTC (permalink / raw)
  To: linux-amlogic, linux-clk, jbrunet, narmstrong
  Cc: linux-arm-kernel, linux-kernel, mturquette, sboyd, Martin Blumenstingl

The cpu_div3 clock (cpu_in divided by 3) generates a signal with a duty
cycle of 33%. The CPU clock however requires a clock signal with a duty
cycle of 50% to run stable.
cpu_div3 was observed to be problematic when cycling through all
available CPU frequencies (with additional patches on top of this one)
while running "stress --cpu 4" in the background. This caused sporadic
hangs where the whole system would fully lock up.

Amlogic's 3.10 kernel code also does not use the cpu_div3 clock either
when changing the CPU clock.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/meson8b.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 9bd5920da0ff..a96bfee58a61 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -608,20 +608,27 @@ static struct clk_regmap meson8b_cpu_scale_div = {
 	},
 };
 
+static u32 mux_table_cpu_scale_out_sel[] = { 0, 1, 3 };
 static struct clk_regmap meson8b_cpu_scale_out_sel = {
 	.data = &(struct clk_regmap_mux_data){
 		.offset = HHI_SYS_CPU_CLK_CNTL0,
 		.mask = 0x3,
 		.shift = 2,
+		.table = mux_table_cpu_scale_out_sel,
 	},
 	.hw.init = &(struct clk_init_data){
 		.name = "cpu_scale_out_sel",
 		.ops = &clk_regmap_mux_ro_ops,
+		/*
+		 * NOTE: We are skipping the parent with value 0x2 (which is
+		 * "cpu_div3") because it results in a duty cycle of 33% which
+		 * makes the system unstable and can result in a lockup of the
+		 * whole system.
+		 */
 		.parent_names = (const char *[]) { "cpu_in_sel",
 						   "cpu_div2",
-						   "cpu_div3",
 						   "cpu_scale_div" },
-		.num_parents = 4,
+		.num_parents = 3,
 		.flags = CLK_SET_RATE_PARENT,
 	},
 };
-- 
2.19.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/6] clk: meson: meson8b: mark the CPU clock as CLK_IS_CRITICAL
  2018-11-15 22:40 [PATCH v2 0/6] Meson8b: make the CPU clock mutable Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 1/6] clk: meson: clk-pll: check if the clock is already enabled Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 2/6] clk: meson: meson8b: do not use cpu_div3 for cpu_scale_out_sel Martin Blumenstingl
@ 2018-11-15 22:40 ` Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 4/6] clk: meson: meson8b: add support for more M/N values in sys_pll Martin Blumenstingl
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2018-11-15 22:40 UTC (permalink / raw)
  To: linux-amlogic, linux-clk, jbrunet, narmstrong
  Cc: linux-arm-kernel, linux-kernel, mturquette, sboyd, Martin Blumenstingl

We don't want the common clock framework to disable the "cpu_clk" if
it's not used by any device. The cpufreq-dt driver does not enable the
CPU clocks. However, even if it would we would still want the CPU clock
to be enabled at all times because the CPU clock is also required even
if we disable CPU frequency scaling on a specific board.

The reason why we want the CPU clock to be enabled is a clock further up
in the tree:
Since commit 6f888e7bc7bd58 ("clk: meson: clk-pll: add enable bit") the
sys_pll can be disabled. However, since the CPU clock is derived from
sys_pll we don't want sys_pll to get disabled. The common clock
framework takes care of that for us by enabling all parent clocks of our
CPU clock when we mark the CPU clock with CLK_IS_CRITICAL.

Until now this is not a problem yet because all clocks in the CPU
clock's tree (including sys_pll) are read-only. However, once we allow
modifications to the clocks in that tree we will need this.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/meson8b.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index a96bfee58a61..41a5025364f9 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -646,7 +646,8 @@ static struct clk_regmap meson8b_cpu_clk = {
 						  "cpu_scale_out_sel" },
 		.num_parents = 2,
 		.flags = (CLK_SET_RATE_PARENT |
-			  CLK_SET_RATE_NO_REPARENT),
+			  CLK_SET_RATE_NO_REPARENT |
+			  CLK_IS_CRITICAL),
 	},
 };
 
-- 
2.19.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 4/6] clk: meson: meson8b: add support for more M/N values in sys_pll
  2018-11-15 22:40 [PATCH v2 0/6] Meson8b: make the CPU clock mutable Martin Blumenstingl
                   ` (2 preceding siblings ...)
  2018-11-15 22:40 ` [PATCH v2 3/6] clk: meson: meson8b: mark the CPU clock as CLK_IS_CRITICAL Martin Blumenstingl
@ 2018-11-15 22:40 ` Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 5/6] clk: meson: meson8b: run from the XTAL when changing the CPU frequency Martin Blumenstingl
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2018-11-15 22:40 UTC (permalink / raw)
  To: linux-amlogic, linux-clk, jbrunet, narmstrong
  Cc: linux-arm-kernel, linux-kernel, mturquette, sboyd, Martin Blumenstingl

The sys_pll on the EC-100 board is configured to 1584MHz at boot
(either by u-boot, firmware or chip defaults). This is achieved by using
M = 66, N = 1 (24MHz * 66 / 1).
At boot the CPU clock is running off sys_pll divided by 2 which results
in 792MHz. Thus M = 66 is considered to be a "safe" value for Meson8b.

To achieve 1608MHz (one of the CPU OPPs on Meson8 and Meson8m2) we need
M = 67, N = 1. I ran "stress --cpu 4" while infinitely cycling through
all available frequencies on my Meson8m2 board and could not spot any
issues with this setting (after ~12 hours of running this).

On Meson8, Meson8b and Meson8m2 we also want to be able to use 408MHz
and 816MHz CPU frequencies. These can be achieved by dividing sys_pll by
4 (for 408MHz) or 2 (for 816MHz). That means that sys_pll has to run at
1632MHz which can be generated using M = 68, N = 1.
Similarily we also want to be able to use 1008MHz as CPU frequency. This
means that sys_pll has to run either at 1008MHz or 2016MHz. The former
would result in an M value of 42, which is lower than the smallest value
used by the 3.10 GPL kernel sources from Amlogic (50 is the lower limit
there). Thus we need to run sys_pll at 2016MHz which can ge generated
using M = 84, N = 1.
I tested M = 68 and M = 84 on my Meson8b Odroid-C1 and my Meson8m2 board
by running "stress --cpu 4" while infinitely cycling thorugh all
available frequencies. I could not spot any issues after ~12 hours of
running this.

Amlogic's 3.10 GPL kernel sources have more M/N combinations. I did not
add them yet because M = 74 (to achieve close to 1800MHz on Meson8) and
M = 82 (to achieve close to 1992MHz on Meson8 as well) caused my
Meson8m2 board to hang randomly. It's not clear why this is (for example
because the board's voltage regulator design is bad, some missing bits
for these values in our clk-pll driver, etc.). Thus the following M
values from the Amlogic 3.10 GPL kernel sources are skipped as of now:
69, 70, 71, 72, 73, 74, 76, 78, 80, 82, 84, 86, 88, 90, 92, 94, 96, 98

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/meson8b.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index 41a5025364f9..b07a92ed7de3 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -43,6 +43,11 @@ static const struct pll_params_table sys_pll_params_table[] = {
 	PLL_PARAMS(62, 1),
 	PLL_PARAMS(63, 1),
 	PLL_PARAMS(64, 1),
+	PLL_PARAMS(65, 1),
+	PLL_PARAMS(66, 1),
+	PLL_PARAMS(67, 1),
+	PLL_PARAMS(68, 1),
+	PLL_PARAMS(84, 1),
 	{ /* sentinel */ },
 };
 
-- 
2.19.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 5/6] clk: meson: meson8b: run from the XTAL when changing the CPU frequency
  2018-11-15 22:40 [PATCH v2 0/6] Meson8b: make the CPU clock mutable Martin Blumenstingl
                   ` (3 preceding siblings ...)
  2018-11-15 22:40 ` [PATCH v2 4/6] clk: meson: meson8b: add support for more M/N values in sys_pll Martin Blumenstingl
@ 2018-11-15 22:40 ` Martin Blumenstingl
  2018-11-15 22:40 ` [PATCH v2 6/6] clk: meson: meson8b: allow changing the CPU clock tree Martin Blumenstingl
  2018-11-16  8:49 ` [PATCH v2 0/6] Meson8b: make the CPU clock mutable Neil Armstrong
  6 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2018-11-15 22:40 UTC (permalink / raw)
  To: linux-amlogic, linux-clk, jbrunet, narmstrong
  Cc: linux-arm-kernel, linux-kernel, mturquette, sboyd, Martin Blumenstingl

Changing the CPU clock requires changing various clocks including the
SYS PLL. The existing meson clk-pll and clk-regmap drivers can change
all of the relevant clocks already.
However, changing for exampe the SYS PLL is problematic because as long
as the CPU is running off a clock derived from SYS PLL changing the
latter results in a full system lockup.
Fix this system lockup by switching the CPU clock to run off the XTAL
while we are changing the any of the clocks in the CPU clock tree.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/meson8b.c | 63 +++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index b07a92ed7de3..c06a1a7faa4c 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -1116,6 +1116,53 @@ static const struct reset_control_ops meson8b_clk_reset_ops = {
 	.deassert = meson8b_clk_reset_deassert,
 };
 
+struct meson8b_nb_data {
+	struct notifier_block nb;
+	struct clk_hw_onecell_data *onecell_data;
+};
+
+static int meson8b_cpu_clk_notifier_cb(struct notifier_block *nb,
+				       unsigned long event, void *data)
+{
+	struct meson8b_nb_data *nb_data =
+		container_of(nb, struct meson8b_nb_data, nb);
+	struct clk_hw **hws = nb_data->onecell_data->hws;
+	struct clk_hw *cpu_clk_hw, *parent_clk_hw;
+	struct clk *cpu_clk, *parent_clk;
+	int ret;
+
+	switch (event) {
+	case PRE_RATE_CHANGE:
+		parent_clk_hw = hws[CLKID_XTAL];
+		break;
+
+	case POST_RATE_CHANGE:
+		parent_clk_hw = hws[CLKID_CPU_SCALE_OUT_SEL];
+		break;
+
+	default:
+		return NOTIFY_DONE;
+	}
+
+	cpu_clk_hw = hws[CLKID_CPUCLK];
+	cpu_clk = __clk_lookup(clk_hw_get_name(cpu_clk_hw));
+
+	parent_clk = __clk_lookup(clk_hw_get_name(parent_clk_hw));
+
+	ret = clk_set_parent(cpu_clk, parent_clk);
+	if (ret)
+		return notifier_from_errno(ret);
+
+	udelay(100);
+
+	return NOTIFY_OK;
+}
+
+static struct meson8b_nb_data meson8b_cpu_nb_data = {
+	.nb.notifier_call = meson8b_cpu_clk_notifier_cb,
+	.onecell_data = &meson8b_hw_onecell_data,
+};
+
 static const struct regmap_config clkc_regmap_config = {
 	.reg_bits       = 32,
 	.val_bits       = 32,
@@ -1125,6 +1172,8 @@ static const struct regmap_config clkc_regmap_config = {
 static void __init meson8b_clkc_init(struct device_node *np)
 {
 	struct meson8b_clk_reset *rstc;
+	const char *notifier_clk_name;
+	struct clk *notifier_clk;
 	void __iomem *clk_base;
 	struct regmap *map;
 	int i, ret;
@@ -1179,6 +1228,20 @@ static void __init meson8b_clkc_init(struct device_node *np)
 			return;
 	}
 
+	/*
+	 * FIXME we shouldn't program the muxes in notifier handlers. The
+	 * tricky programming sequence will be handled by the forthcoming
+	 * coordinated clock rates mechanism once that feature is released.
+	 */
+	notifier_clk_name = clk_hw_get_name(&meson8b_cpu_scale_out_sel.hw);
+	notifier_clk = __clk_lookup(notifier_clk_name);
+	ret = clk_notifier_register(notifier_clk, &meson8b_cpu_nb_data.nb);
+	if (ret) {
+		pr_err("%s: failed to register the CPU clock notifier\n",
+		       __func__);
+		return;
+	}
+
 	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get,
 				     &meson8b_hw_onecell_data);
 	if (ret)
-- 
2.19.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 6/6] clk: meson: meson8b: allow changing the CPU clock tree
  2018-11-15 22:40 [PATCH v2 0/6] Meson8b: make the CPU clock mutable Martin Blumenstingl
                   ` (4 preceding siblings ...)
  2018-11-15 22:40 ` [PATCH v2 5/6] clk: meson: meson8b: run from the XTAL when changing the CPU frequency Martin Blumenstingl
@ 2018-11-15 22:40 ` Martin Blumenstingl
  2018-11-16  8:49 ` [PATCH v2 0/6] Meson8b: make the CPU clock mutable Neil Armstrong
  6 siblings, 0 replies; 8+ messages in thread
From: Martin Blumenstingl @ 2018-11-15 22:40 UTC (permalink / raw)
  To: linux-amlogic, linux-clk, jbrunet, narmstrong
  Cc: linux-arm-kernel, linux-kernel, mturquette, sboyd, Martin Blumenstingl

Currently all clocks in the CPU clock tree are marked as read-only
(using the corresponding _ro_ clk_ops). This was correct since changing
the clock tree could cause the system to lock up.
Switch all clocks to their corresponding clk_ops variant which is not
read-only to allow changing the CPU clock tree since the bug which
locked up the system is now fixed (by switching the CPU clock temporary
to run off XTAL while changing the CPU clock tree).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/meson/meson8b.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index c06a1a7faa4c..b3bdc7e05441 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -203,7 +203,7 @@ static struct clk_regmap meson8b_sys_pll_dco = {
 	},
 	.hw.init = &(struct clk_init_data){
 		.name = "sys_pll_dco",
-		.ops = &meson_clk_pll_ro_ops,
+		.ops = &meson_clk_pll_ops,
 		.parent_names = (const char *[]){ "xtal" },
 		.num_parents = 1,
 	},
@@ -218,7 +218,7 @@ static struct clk_regmap meson8b_sys_pll = {
 	},
 	.hw.init = &(struct clk_init_data){
 		.name = "sys_pll",
-		.ops = &clk_regmap_divider_ro_ops,
+		.ops = &clk_regmap_divider_ops,
 		.parent_names = (const char *[]){ "sys_pll_dco" },
 		.num_parents = 1,
 		.flags = CLK_SET_RATE_PARENT,
@@ -552,7 +552,7 @@ static struct clk_regmap meson8b_cpu_in_sel = {
 	},
 	.hw.init = &(struct clk_init_data){
 		.name = "cpu_in_sel",
-		.ops = &clk_regmap_mux_ro_ops,
+		.ops = &clk_regmap_mux_ops,
 		.parent_names = (const char *[]){ "xtal", "sys_pll" },
 		.num_parents = 2,
 		.flags = (CLK_SET_RATE_PARENT |
@@ -606,7 +606,7 @@ static struct clk_regmap meson8b_cpu_scale_div = {
 	},
 	.hw.init = &(struct clk_init_data){
 		.name = "cpu_scale_div",
-		.ops = &clk_regmap_divider_ro_ops,
+		.ops = &clk_regmap_divider_ops,
 		.parent_names = (const char *[]){ "cpu_in_sel" },
 		.num_parents = 1,
 		.flags = CLK_SET_RATE_PARENT,
@@ -623,7 +623,7 @@ static struct clk_regmap meson8b_cpu_scale_out_sel = {
 	},
 	.hw.init = &(struct clk_init_data){
 		.name = "cpu_scale_out_sel",
-		.ops = &clk_regmap_mux_ro_ops,
+		.ops = &clk_regmap_mux_ops,
 		/*
 		 * NOTE: We are skipping the parent with value 0x2 (which is
 		 * "cpu_div3") because it results in a duty cycle of 33% which
@@ -646,7 +646,7 @@ static struct clk_regmap meson8b_cpu_clk = {
 	},
 	.hw.init = &(struct clk_init_data){
 		.name = "cpu_clk",
-		.ops = &clk_regmap_mux_ro_ops,
+		.ops = &clk_regmap_mux_ops,
 		.parent_names = (const char *[]){ "xtal",
 						  "cpu_scale_out_sel" },
 		.num_parents = 2,
-- 
2.19.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/6] Meson8b: make the CPU clock mutable
  2018-11-15 22:40 [PATCH v2 0/6] Meson8b: make the CPU clock mutable Martin Blumenstingl
                   ` (5 preceding siblings ...)
  2018-11-15 22:40 ` [PATCH v2 6/6] clk: meson: meson8b: allow changing the CPU clock tree Martin Blumenstingl
@ 2018-11-16  8:49 ` Neil Armstrong
  6 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2018-11-16  8:49 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, linux-clk, jbrunet
  Cc: linux-arm-kernel, linux-kernel, mturquette, sboyd

On 15/11/2018 23:40, Martin Blumenstingl wrote:
> This allows changing the CPU clock on the 32-bit Amlogic Meson SoCs
> (Meson8, Meson8b and Meson8m2).
> CPU frequency scaling will be enabled with a separate series by adding
> the CPU clock and the OPP tables to meson8.dtsi and meson8b.dtsi.
> 
> While changing the CPU frequency (sys_pll or any of it's post-dividers)
> we need to run the CPU clock off the XTAL clock. Otherwise the system
> will lock up because we need to disable the sys_pll to change it's
> rate.
> 
> This also makes the clk-pll's .enable hook a no-op if the clock is
> already enabled. Otherwise we're getting lockups when calling the
> first clk_{prepare_}enable on the sys_pll or any of it's children (as
> the CCF propagates the enable event up to the sys_pll). This is because
> the .enable hook unconditionally disables and enables the clock.
> However, we can't disable that clock (not even temporarily) if the CPU
> is running off sys_pll.
> 
> Additionally this adds support for more M/N combinations in sys_pll to
> achieve all of the OPPs on Meson8b and all OPPs <= 1608 MHz on Meson8
> and Meson8m2.
> 
> Compared to Amlogic's 3.10 kernel there's one notable difference: we
> are actually allowing changes to the sys_pll. Amlogic's kernel sets
> sys_pll to a fixed rate during boot and then uses a timer generate a
> "virtual clock rate" by toggling between various dividers (for example:
> sys_pll is set to 1536MHz. to achieve 1008MHz they are toggling every
> 2500us between 1536MHZ and 768MHz so the average over <period, for
> example one second> is 1008MHz).
> I could reproduce any situation where changing sys_pll failed (for
> example due to high temperature). To prove that I ran "stress --cpu 4"
> for multiple hours and then cycled through all available CPU
> frequencies (while keeping "stress" running in the background). This
> worked fine on my Meson8b Odroid-C1 and EC-100 boards as well as my
> Meson8m2 board.
> 
> 
> Dependencies:
> This series is built on top of the latest clk-meson.git tree and the
> patches from my other series [1] "Meson8b: fixes for the cpu_scale_div
> clock".
> 
> 
> Changes since v1 at [0]:
> - re-ordered patches as suggested by Jerome: keep all fixes first, then
>   the new "features"
> - squashed patches "clk-pll: check if the clock is already enabled" and
>   "clk-pll: add the is_enabled function in the clk_ops". This also
>   allows calling clk_hw_is_enabled() from meson_clk_pll_enable()
>   instead of calling meson_clk_pll_is_enabled() directly (this matches
>   the implementation of sclk-div.c)
> - documented the dependencies of this series in the cover-letter
> - dropped "RFC" prefix
> - collected Jerome's Acked-/Reviewed-by's (thanks for the quick
>   response!)
> 
> 
> [0] https://patchwork.kernel.org/cover/10683317/
> [1] https://patchwork.kernel.org/cover/10617617/
> 
> 
> Martin Blumenstingl (6):
>   clk: meson: clk-pll: check if the clock is already enabled
>   clk: meson: meson8b: do not use cpu_div3 for cpu_scale_out_sel
>   clk: meson: meson8b: mark the CPU clock as CLK_IS_CRITICAL
>   clk: meson: meson8b: add support for more M/N values in sys_pll
>   clk: meson: meson8b: run from the XTAL when changing the CPU frequency
>   clk: meson: meson8b: allow changing the CPU clock tree
> 
>  drivers/clk/meson/clk-pll.c | 19 ++++++++
>  drivers/clk/meson/meson8b.c | 94 +++++++++++++++++++++++++++++++++----
>  2 files changed, 104 insertions(+), 9 deletions(-)
> 

Applied to next/drivers !

Thanks for your work on meson8* SoCs !

Neil

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-15 22:40 [PATCH v2 0/6] Meson8b: make the CPU clock mutable Martin Blumenstingl
2018-11-15 22:40 ` [PATCH v2 1/6] clk: meson: clk-pll: check if the clock is already enabled Martin Blumenstingl
2018-11-15 22:40 ` [PATCH v2 2/6] clk: meson: meson8b: do not use cpu_div3 for cpu_scale_out_sel Martin Blumenstingl
2018-11-15 22:40 ` [PATCH v2 3/6] clk: meson: meson8b: mark the CPU clock as CLK_IS_CRITICAL Martin Blumenstingl
2018-11-15 22:40 ` [PATCH v2 4/6] clk: meson: meson8b: add support for more M/N values in sys_pll Martin Blumenstingl
2018-11-15 22:40 ` [PATCH v2 5/6] clk: meson: meson8b: run from the XTAL when changing the CPU frequency Martin Blumenstingl
2018-11-15 22:40 ` [PATCH v2 6/6] clk: meson: meson8b: allow changing the CPU clock tree Martin Blumenstingl
2018-11-16  8:49 ` [PATCH v2 0/6] Meson8b: make the CPU clock mutable Neil Armstrong

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox