Re: [RFC/RFT v3 07/14] clk: meson: g12a: add notifiers to handle cpu clock change
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date: 2019-07-03 00:30:58
Also in:
linux-amlogic, linux-clk, linux-gpio, lkml
Hi Stephen, Hi Neil, On Mon, Jul 1, 2019 at 11:13 AM Neil Armstrong [off-list ref] wrote:
In order to implement clock switching for the CLKID_CPU_CLK and CLKID_CPUB_CLK, notifiers are added on specific points of the clock tree : cpu_clk / cpub_clk | \- cpu_clk_dyn | | \- cpu_clk_premux0 | | |- cpu_clk_postmux0 | | | |- cpu_clk_dyn0_div | | | \- xtal/fclk_div2/fclk_div3 | | \- xtal/fclk_div2/fclk_div3 | \- cpu_clk_premux1 | |- cpu_clk_postmux1 | | |- cpu_clk_dyn1_div | | \- xtal/fclk_div2/fclk_div3 | \- xtal/fclk_div2/fclk_div3 \ sys_pll / sys1_pll This for each cluster, a single one for G12A, two for G12B. Each cpu_clk_premux1 tree is marked as read-only and CLK_SET_RATE_NO_REPARENT, to be used as "parking" clock in a safe clock frequency.
it seems that this is one case where the "coordinated clocks" feature would come handy: [0] Stephen, do you know if those patches stopped in March or if there's still some ongoing effort to get them ready? [...]
quoted hunk ↗ jump to hunk
-/* - * Internal sys pll emulation configuration parameters - */ -static const struct reg_sequence g12a_sys_init_regs[] = { - { .reg = HHI_SYS_PLL_CNTL1, .def = 0x00000000 }, - { .reg = HHI_SYS_PLL_CNTL2, .def = 0x00000000 }, - { .reg = HHI_SYS_PLL_CNTL3, .def = 0x48681c00 }, - { .reg = HHI_SYS_PLL_CNTL4, .def = 0x88770290 }, - { .reg = HHI_SYS_PLL_CNTL5, .def = 0x39272000 }, - { .reg = HHI_SYS_PLL_CNTL6, .def = 0x56540000 }, +static const struct pll_mult_range g12a_sys_pll_mult_range = { + .min = 128, + .max = 250, }; static struct clk_regmap g12a_sys_pll_dco = {@@ -124,14 +118,15 @@ static struct clk_regmap g12a_sys_pll_dco = { .shift = 29, .width = 1, }, - .init_regs = g12a_sys_init_regs, - .init_count = ARRAY_SIZE(g12a_sys_init_regs), + .range = &g12a_sys_pll_mult_range,
Neil, I believe that this should be a separate patch with a description which explains why we don't need the "init regs" anymore
},
.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 *[]){ IN_PREFIX "xtal" },
.num_parents = 1,
+ /* This clock feeds the CPU, avoid disabling it */
+ .flags = CLK_IS_CRITICAL,maybe we should have a separate patch for making the CPU clock tree mutable as well [...]
+/* This divider uses bit 26 to take change in account */
+static int g12b_cpub_clk_mux0_div_set_rate(struct clk_hw *hw,
+ unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct clk_regmap_div_data *div = clk_get_regmap_div_data(clk);
+ unsigned int val;
+ int ret;
+
+ ret = divider_get_val(rate, parent_rate, div->table, div->width,
+ div->flags);
+ if (ret < 0)
+ return ret;
+
+ val = (unsigned int)ret << div->shift;
+
+ regmap_update_bits(clk->map, HHI_SYS_CPUB_CLK_CNTL,
+ SYS_CPU_DYN_ENABLE, SYS_CPU_DYN_ENABLE);
+
+ return regmap_update_bits(clk->map, div->offset,
+ clk_div_mask(div->width) << div->shift |
+ SYS_CPU_DYN_ENABLE, val);
+};the public S922X datasheet doesn't mention bit 26 do I understand the semantics correctly?: - set SYS_CPU_DYN_ENABLE - update the divider - unset SYS_CPU_DYN_ENABLE too bad it's not a gate which we could model with CLK_SET_RATE_GATE/CLK_SET_RATE_UNGATE Martin [0] https://patchwork.kernel.org/patch/10838949/ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel