This patchset adds support for the clock tree feeding the 4xCortex A53
cpu cluster.
This patchet does not handle clock changing, this will be added in a
secondary patchset.
The CPU clock can either use the SYS_PLL for > 1GHz frequencies or
use a couple of div+mux from 1GHz/667MHz/24MHz source with 2 non-glitch
muxes.
The CPU clock must be switched to a safe clock while changing the clocks
before the non-glitch muxes. Proper support will be added later.
In this patchset, clocks are set read-only.
Neil Armstrong (2):
clk: meson-g12a: add cpu clock bindings
clk: meson: g12a: add cpu clocks
drivers/clk/meson/g12a.c | 348 ++++++++++++++++++++++++++
drivers/clk/meson/g12a.h | 25 +-
include/dt-bindings/clock/g12a-clkc.h | 1 +
3 files changed, 373 insertions(+), 1 deletion(-)
--
2.20.1
Add the Amlogic G12A Family CPU Clock tree in read/only for now.
The CPU clock can either use the SYS_PLL for > 1GHz frequencies or
use a couple of div+mux from 1GHz/667MHz/24MHz source with 2 non-glitch
muxes.
Proper DVFS support will come in a second time.
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/clk/meson/g12a.c | 348 +++++++++++++++++++++++++++++++++++++++
drivers/clk/meson/g12a.h | 1 +
2 files changed, 349 insertions(+)
diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 0e1ce8c03259..4c938f1b8421 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -150,6 +150,316 @@ static struct clk_regmap g12a_sys_pll = {
},
};
+static struct clk_regmap g12a_sys_pll_div16_en = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .bit_idx = 24,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "sys_pll_div16_en",
+ .ops = &clk_regmap_gate_ops,
+ .parent_names = (const char *[]){ "sys_pll" },
+ .num_parents = 1,
+ /*
+ * This clock is used to debug the sys_pll range
+ * Linux should not change it at runtime
+ */
+ .flags = CLK_IGNORE_UNUSED,
+ },
+};
+
+static struct clk_fixed_factor g12a_sys_pll_div16 = {
+ .mult = 1,
+ .div = 16,
+ .hw.init = &(struct clk_init_data){
+ .name = "sys_pll_div16",
+ .ops = &clk_fixed_factor_ops,
+ .parent_names = (const char *[]){ "sys_pll_div16_en" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_dyn0_sel = {
+ .data = &(struct clk_regmap_mux_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL0,
+ .mask = 0x3,
+ .shift = 0,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_dyn0_sel",
+ .ops = &clk_regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ IN_PREFIX "xtal",
+ "fclk_div2",
+ "fclk_div3" },
+ .num_parents = 3,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_dyn0_div = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL0,
+ .shift = 4,
+ .width = 6,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_dyn0_div",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk_dyn0_sel" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_dyn0 = {
+ .data = &(struct clk_regmap_mux_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL0,
+ .mask = 0x1,
+ .shift = 2,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_dyn0",
+ .ops = &clk_regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk_dyn0_sel",
+ "cpu_clk_dyn0_div" },
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_dyn1_sel = {
+ .data = &(struct clk_regmap_mux_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL0,
+ .mask = 0x3,
+ .shift = 16,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_dyn1_sel",
+ .ops = &clk_regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ IN_PREFIX "xtal",
+ "fclk_div2",
+ "fclk_div3" },
+ .num_parents = 3,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_dyn1_div = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL0,
+ .shift = 20,
+ .width = 6,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_dyn1_div",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk_dyn1_sel" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_dyn1 = {
+ .data = &(struct clk_regmap_mux_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL0,
+ .mask = 0x1,
+ .shift = 18,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_dyn1",
+ .ops = &clk_regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk_dyn1_sel",
+ "cpu_clk_dyn1_div" },
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_dyn = {
+ .data = &(struct clk_regmap_mux_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL0,
+ .mask = 0x1,
+ .shift = 10,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_dyn",
+ .ops = &clk_regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk_dyn0",
+ "cpu_clk_dyn1" },
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk = {
+ .data = &(struct clk_regmap_mux_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL0,
+ .mask = 0x1,
+ .shift = 11,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk",
+ .ops = &clk_regmap_mux_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk_dyn",
+ "sys_pll" },
+ .num_parents = 2,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_div16_en = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .bit_idx = 1,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "cpu_clk_div16_en",
+ .ops = &clk_regmap_gate_ops,
+ .parent_names = (const char *[]){ "cpu_clk" },
+ .num_parents = 1,
+ /*
+ * This clock is used to debug the cpu_clk range
+ * Linux should not change it at runtime
+ */
+ .flags = CLK_IGNORE_UNUSED,
+ },
+};
+
+static struct clk_fixed_factor g12a_cpu_clk_div16 = {
+ .mult = 1,
+ .div = 16,
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_div16",
+ .ops = &clk_fixed_factor_ops,
+ .parent_names = (const char *[]){ "cpu_clk_div16_en" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_apb_div = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .shift = 3,
+ .width = 3,
+ .flags = CLK_DIVIDER_POWER_OF_TWO,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_apb_div",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_apb = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .bit_idx = 1,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "cpu_clk_apb",
+ .ops = &clk_regmap_gate_ops,
+ .parent_names = (const char *[]){ "cpu_clk_apb_div" },
+ .num_parents = 1,
+ /*
+ * This clock is set by the ROM monitor code,
+ * Linux should not change it at runtime
+ */
+ .flags = CLK_IGNORE_UNUSED,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_atb_div = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .shift = 6,
+ .width = 3,
+ .flags = CLK_DIVIDER_POWER_OF_TWO,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_atb_div",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_atb = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .bit_idx = 17,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "cpu_clk_atb",
+ .ops = &clk_regmap_gate_ops,
+ .parent_names = (const char *[]){ "cpu_clk_atb_div" },
+ .num_parents = 1,
+ /*
+ * This clock is set by the ROM monitor code,
+ * Linux should not change it at runtime
+ */
+ .flags = CLK_IGNORE_UNUSED,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_axi_div = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .shift = 9,
+ .width = 3,
+ .flags = CLK_DIVIDER_POWER_OF_TWO,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_axi_div",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_axi = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .bit_idx = 18,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "cpu_clk_axi",
+ .ops = &clk_regmap_gate_ops,
+ .parent_names = (const char *[]){ "cpu_clk_axi_div" },
+ .num_parents = 1,
+ /*
+ * This clock is set by the ROM monitor code,
+ * Linux should not change it at runtime
+ */
+ .flags = CLK_IGNORE_UNUSED,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_trace_div = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .shift = 20,
+ .width = 3,
+ .flags = CLK_DIVIDER_POWER_OF_TWO,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "cpu_clk_trace_div",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_names = (const char *[]){ "cpu_clk" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap g12a_cpu_clk_trace = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = HHI_SYS_CPU_CLK_CNTL1,
+ .bit_idx = 23,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "cpu_clk_trace",
+ .ops = &clk_regmap_gate_ops,
+ .parent_names = (const char *[]){ "cpu_clk_trace_div" },
+ .num_parents = 1,
+ /*
+ * This clock is set by the ROM monitor code,
+ * Linux should not change it at runtime
+ */
+ .flags = CLK_IGNORE_UNUSED,
+ },
+};
+
static const struct pll_mult_range g12a_gp0_pll_mult_range = {
.min = 55,
.max = 255,
@@ -2167,6 +2477,26 @@ static struct clk_hw_onecell_data g12a_hw_onecell_data = {
[CLKID_MALI] = &g12a_mali.hw,
[CLKID_MPLL_5OM_DIV] = &g12a_mpll_50m_div.hw,
[CLKID_MPLL_5OM] = &g12a_mpll_50m.hw,
+ [CLKID_SYS_PLL_DIV16_EN] = &g12a_sys_pll_div16_en.hw,
+ [CLKID_SYS_PLL_DIV16] = &g12a_sys_pll_div16.hw,
+ [CLKID_CPU_CLK_DYN0_SEL] = &g12a_cpu_clk_dyn0_sel.hw,
+ [CLKID_CPU_CLK_DYN0_DIV] = &g12a_cpu_clk_dyn0_div.hw,
+ [CLKID_CPU_CLK_DYN0] = &g12a_cpu_clk_dyn0.hw,
+ [CLKID_CPU_CLK_DYN1_SEL] = &g12a_cpu_clk_dyn1_sel.hw,
+ [CLKID_CPU_CLK_DYN1_DIV] = &g12a_cpu_clk_dyn1_div.hw,
+ [CLKID_CPU_CLK_DYN1] = &g12a_cpu_clk_dyn1.hw,
+ [CLKID_CPU_CLK_DYN] = &g12a_cpu_clk_dyn.hw,
+ [CLKID_CPU_CLK] = &g12a_cpu_clk.hw,
+ [CLKID_CPU_CLK_DIV16_EN] = &g12a_cpu_clk_div16_en.hw,
+ [CLKID_CPU_CLK_DIV16] = &g12a_cpu_clk_div16.hw,
+ [CLKID_CPU_CLK_APB_DIV] = &g12a_cpu_clk_apb_div.hw,
+ [CLKID_CPU_CLK_APB] = &g12a_cpu_clk_apb.hw,
+ [CLKID_CPU_CLK_ATB_DIV] = &g12a_cpu_clk_atb_div.hw,
+ [CLKID_CPU_CLK_ATB] = &g12a_cpu_clk_atb.hw,
+ [CLKID_CPU_CLK_AXI_DIV] = &g12a_cpu_clk_axi_div.hw,
+ [CLKID_CPU_CLK_AXI] = &g12a_cpu_clk_axi.hw,
+ [CLKID_CPU_CLK_TRACE_DIV] = &g12a_cpu_clk_trace_div.hw,
+ [CLKID_CPU_CLK_TRACE] = &g12a_cpu_clk_trace.hw,
[NR_CLKS] = NULL,
},
.num = NR_CLKS,
@@ -2335,6 +2665,24 @@ static struct clk_regmap *const g12a_clk_regmaps[] = {
&g12a_mali_1,
&g12a_mali,
&g12a_mpll_50m,
+ &g12a_sys_pll_div16_en,
+ &g12a_cpu_clk_dyn0_sel,
+ &g12a_cpu_clk_dyn0_div,
+ &g12a_cpu_clk_dyn0,
+ &g12a_cpu_clk_dyn1_sel,
+ &g12a_cpu_clk_dyn1_div,
+ &g12a_cpu_clk_dyn1,
+ &g12a_cpu_clk_dyn,
+ &g12a_cpu_clk,
+ &g12a_cpu_clk_div16_en,
+ &g12a_cpu_clk_apb_div,
+ &g12a_cpu_clk_apb,
+ &g12a_cpu_clk_atb_div,
+ &g12a_cpu_clk_atb,
+ &g12a_cpu_clk_axi_div,
+ &g12a_cpu_clk_axi,
+ &g12a_cpu_clk_trace_div,
+ &g12a_cpu_clk_trace,
};
static const struct meson_eeclkc_data g12a_clkc_data = {
diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
index 4854750df902..0ba3bfe4d46d 100644
--- a/drivers/clk/meson/g12a.h
+++ b/drivers/clk/meson/g12a.h
@@ -50,6 +50,7 @@
#define HHI_GCLK_MPEG2 0x148
#define HHI_GCLK_OTHER 0x150
#define HHI_GCLK_OTHER2 0x154
+#define HHI_SYS_CPU_CLK_CNTL1 0x15c
#define HHI_VID_CLK_DIV 0x164
#define HHI_MPEG_CLK_CNTL 0x174
#define HHI_AUD_CLK_CNTL 0x178
--
2.20.1
Add Amlogic G12A Family CPU clocks bindings, only export CPU_CLK since
it should be the only ID used.
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/clk/meson/g12a.h | 24 +++++++++++++++++++++++-
include/dt-bindings/clock/g12a-clkc.h | 1 +
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
index f399dfe1401c..4854750df902 100644
--- a/drivers/clk/meson/g12a.h
+++ b/drivers/clk/meson/g12a.h
@@ -166,8 +166,30 @@
#define CLKID_MALI_0_DIV 170
#define CLKID_MALI_1_DIV 173
#define CLKID_MPLL_5OM_DIV 176
+#define CLKID_PCIE_PLL_DCO 178
+#define CLKID_PCIE_PLL_DCO_DIV2 179
+#define CLKID_PCIE_PLL_OD 180
+#define CLKID_SYS_PLL_DIV16_EN 181
+#define CLKID_SYS_PLL_DIV16 182
+#define CLKID_CPU_CLK_DYN0_SEL 183
+#define CLKID_CPU_CLK_DYN0_DIV 184
+#define CLKID_CPU_CLK_DYN0 185
+#define CLKID_CPU_CLK_DYN1_SEL 186
+#define CLKID_CPU_CLK_DYN1_DIV 187
+#define CLKID_CPU_CLK_DYN1 188
+#define CLKID_CPU_CLK_DYN 189
+#define CLKID_CPU_CLK_DIV16_EN 191
+#define CLKID_CPU_CLK_DIV16 192
+#define CLKID_CPU_CLK_APB_DIV 193
+#define CLKID_CPU_CLK_APB 194
+#define CLKID_CPU_CLK_ATB_DIV 195
+#define CLKID_CPU_CLK_ATB 196
+#define CLKID_CPU_CLK_AXI_DIV 197
+#define CLKID_CPU_CLK_AXI 198
+#define CLKID_CPU_CLK_TRACE_DIV 299
+#define CLKID_CPU_CLK_TRACE 200
-#define NR_CLKS 178
+#define NR_CLKS 201
/* include the CLKIDs that have been made part of the DT binding */
#include <dt-bindings/clock/g12a-clkc.h>
diff --git a/include/dt-bindings/clock/g12a-clkc.h b/include/dt-bindings/clock/g12a-clkc.h
index 83b657038d1e..33aba232282c 100644
--- a/include/dt-bindings/clock/g12a-clkc.h
+++ b/include/dt-bindings/clock/g12a-clkc.h
@@ -131,5 +131,6 @@
#define CLKID_MALI_1 174
#define CLKID_MALI 175
#define CLKID_MPLL_5OM 177
+#define CLKID_CPU_CLK 190
#endif /* __G12A_CLKC_H */
--
2.20.1
Hi Neil,
it's great to see the progress on G12A!
On Fri, Mar 1, 2019 at 11:22 AM Neil Armstrong <[email protected]> wrote:
>
> Add the Amlogic G12A Family CPU Clock tree in read/only for now.
>
> The CPU clock can either use the SYS_PLL for > 1GHz frequencies or
> use a couple of div+mux from 1GHz/667MHz/24MHz source with 2 non-glitch
> muxes.
>
> Proper DVFS support will come in a second time.
can you please also mention that this adds various CPU clock
post-dividers (APB, ATB, AXI and CPU trace)?
I don't mind them being int his patchset
disclaimer for my code-review:
- I don't have access to the datasheet so I can't verify if the clock
tree from this patch is correct
- the latest buildroot code with G12A support
(buildroot_openlinux_kernel_4.9_fbdev_20180706) doesn't have proper
names for all clocks
- based on my experience with Meson8* this looks good overall, some
questions and comments below
> Signed-off-by: Neil Armstrong <[email protected]>
>
> ---
> drivers/clk/meson/g12a.c | 348 +++++++++++++++++++++++++++++++++++++++
> drivers/clk/meson/g12a.h | 1 +
> 2 files changed, 349 insertions(+)
>
> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
> index 0e1ce8c03259..4c938f1b8421 100644
> --- a/drivers/clk/meson/g12a.c
> +++ b/drivers/clk/meson/g12a.c
> @@ -150,6 +150,316 @@ static struct clk_regmap g12a_sys_pll = {
> },
> };
>
> +static struct clk_regmap g12a_sys_pll_div16_en = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .bit_idx = 24,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "sys_pll_div16_en",
> + .ops = &clk_regmap_gate_ops,
> + .parent_names = (const char *[]){ "sys_pll" },
> + .num_parents = 1,
> + /*
> + * This clock is used to debug the sys_pll range
> + * Linux should not change it at runtime
> + */
if we're not supposed to touch this the enable bit, can you switch to
clk_regmap_gate_ro_ops ?
> + .flags = CLK_IGNORE_UNUSED,
> + },
> +};
> +
> +static struct clk_fixed_factor g12a_sys_pll_div16 = {
> + .mult = 1,
> + .div = 16,
> + .hw.init = &(struct clk_init_data){
> + .name = "sys_pll_div16",
> + .ops = &clk_fixed_factor_ops,
> + .parent_names = (const char *[]){ "sys_pll_div16_en" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_dyn0_sel = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> + .mask = 0x3,
> + .shift = 0,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_dyn0_sel",
the buildroot code has a variable with the name "p_premux"
I'm not sure what the datasheet states, but maybe this should be
cpu_clk_dyn0_pre_sel
same applies to the corresponding dyn1 clock below
> + .ops = &clk_regmap_mux_ro_ops,
> + .parent_names = (const char *[]){ IN_PREFIX "xtal",
> + "fclk_div2",
> + "fclk_div3" },
> + .num_parents = 3,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_dyn0_div = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> + .shift = 4,
> + .width = 6,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_dyn0_div",
> + .ops = &clk_regmap_divider_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk_dyn0_sel" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_dyn0 = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> + .mask = 0x1,
> + .shift = 2,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_dyn0",
the buildroot code has a variable with the name "p_postmux". in this
case I would leave the name "cpu_clk_dyn0" because it's the "output"
of this specific "clock tree/branch".
same applies to the corresponding dyn1 clock below
> + .ops = &clk_regmap_mux_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk_dyn0_sel",
> + "cpu_clk_dyn0_div" },
> + .num_parents = 2,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_dyn1_sel = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> + .mask = 0x3,
> + .shift = 16,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_dyn1_sel",
> + .ops = &clk_regmap_mux_ro_ops,
> + .parent_names = (const char *[]){ IN_PREFIX "xtal",
> + "fclk_div2",
> + "fclk_div3" },
> + .num_parents = 3,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_dyn1_div = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> + .shift = 20,
> + .width = 6,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_dyn1_div",
> + .ops = &clk_regmap_divider_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk_dyn1_sel" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_dyn1 = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> + .mask = 0x1,
> + .shift = 18,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_dyn1",
> + .ops = &clk_regmap_mux_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk_dyn1_sel",
> + "cpu_clk_dyn1_div" },
> + .num_parents = 2,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_dyn = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> + .mask = 0x1,
> + .shift = 10,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_dyn",
> + .ops = &clk_regmap_mux_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk_dyn0",
> + "cpu_clk_dyn1" },
> + .num_parents = 2,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> + .mask = 0x1,
> + .shift = 11,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk",
> + .ops = &clk_regmap_mux_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk_dyn",
> + "sys_pll" },
> + .num_parents = 2,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_div16_en = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .bit_idx = 1,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "cpu_clk_div16_en",
> + .ops = &clk_regmap_gate_ops,
> + .parent_names = (const char *[]){ "cpu_clk" },
> + .num_parents = 1,
> + /*
> + * This clock is used to debug the cpu_clk range
> + * Linux should not change it at runtime
> + */
same as above: if we're not supposed to touch this the enable bit, can
you switch to clk_regmap_gate_ro_ops ?
> + .flags = CLK_IGNORE_UNUSED,
> + },
> +};
> +
> +static struct clk_fixed_factor g12a_cpu_clk_div16 = {
> + .mult = 1,
> + .div = 16,
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_div16",
> + .ops = &clk_fixed_factor_ops,
> + .parent_names = (const char *[]){ "cpu_clk_div16_en" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_apb_div = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .shift = 3,
> + .width = 3,
> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_apb_div",
> + .ops = &clk_regmap_divider_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_apb = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .bit_idx = 1,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "cpu_clk_apb",
> + .ops = &clk_regmap_gate_ops,
> + .parent_names = (const char *[]){ "cpu_clk_apb_div" },
> + .num_parents = 1,
> + /*
> + * This clock is set by the ROM monitor code,
> + * Linux should not change it at runtime
> + */
same as above: if we're not supposed to touch this the enable bit, can
you switch to clk_regmap_gate_ro_ops ?
> + .flags = CLK_IGNORE_UNUSED,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_atb_div = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .shift = 6,
> + .width = 3,
> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_atb_div",
> + .ops = &clk_regmap_divider_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_atb = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .bit_idx = 17,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "cpu_clk_atb",
> + .ops = &clk_regmap_gate_ops,
> + .parent_names = (const char *[]){ "cpu_clk_atb_div" },
> + .num_parents = 1,
> + /*
> + * This clock is set by the ROM monitor code,
> + * Linux should not change it at runtime
> + */
same as above: if we're not supposed to touch this the enable bit, can
you switch to clk_regmap_gate_ro_ops ?
> + .flags = CLK_IGNORE_UNUSED,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_axi_div = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .shift = 9,
> + .width = 3,
> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_axi_div",
> + .ops = &clk_regmap_divider_ro_ops,
out of curiosity (this applies to all CPU clock post-dividers):
did you check whether CLK_DIVIDER_POWER_OF_TWO is correct on G12A?
I'm asking because on Meson8b the post-dividers select between one of:
"cpu_clk divided by 2, 3, 4, 5, 6, 7 or 8". also some of the
post-dividers use register value 0 for cpu_clk_div2 while others use
register value 1 for cpu_clk_div2.
> + .parent_names = (const char *[]){ "cpu_clk" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_axi = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .bit_idx = 18,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "cpu_clk_axi",
> + .ops = &clk_regmap_gate_ops,
> + .parent_names = (const char *[]){ "cpu_clk_axi_div" },
> + .num_parents = 1,
> + /*
> + * This clock is set by the ROM monitor code,
> + * Linux should not change it at runtime
> + */
same as above: if we're not supposed to touch this the enable bit, can
you switch to clk_regmap_gate_ro_ops ?
> + .flags = CLK_IGNORE_UNUSED,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_trace_div = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .shift = 20,
> + .width = 3,
> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "cpu_clk_trace_div",
> + .ops = &clk_regmap_divider_ro_ops,
> + .parent_names = (const char *[]){ "cpu_clk" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap g12a_cpu_clk_trace = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> + .bit_idx = 23,
> + },
> + .hw.init = &(struct clk_init_data) {
> + .name = "cpu_clk_trace",
> + .ops = &clk_regmap_gate_ops,
> + .parent_names = (const char *[]){ "cpu_clk_trace_div" },
> + .num_parents = 1,
> + /*
> + * This clock is set by the ROM monitor code,
> + * Linux should not change it at runtime
> + */
same as above: if we're not supposed to touch this the enable bit, can
you switch to clk_regmap_gate_ro_ops ?
> + .flags = CLK_IGNORE_UNUSED,
> + },
> +};
> +
> static const struct pll_mult_range g12a_gp0_pll_mult_range = {
> .min = 55,
> .max = 255,
> @@ -2167,6 +2477,26 @@ static struct clk_hw_onecell_data g12a_hw_onecell_data = {
> [CLKID_MALI] = &g12a_mali.hw,
> [CLKID_MPLL_5OM_DIV] = &g12a_mpll_50m_div.hw,
> [CLKID_MPLL_5OM] = &g12a_mpll_50m.hw,
> + [CLKID_SYS_PLL_DIV16_EN] = &g12a_sys_pll_div16_en.hw,
> + [CLKID_SYS_PLL_DIV16] = &g12a_sys_pll_div16.hw,
> + [CLKID_CPU_CLK_DYN0_SEL] = &g12a_cpu_clk_dyn0_sel.hw,
> + [CLKID_CPU_CLK_DYN0_DIV] = &g12a_cpu_clk_dyn0_div.hw,
> + [CLKID_CPU_CLK_DYN0] = &g12a_cpu_clk_dyn0.hw,
> + [CLKID_CPU_CLK_DYN1_SEL] = &g12a_cpu_clk_dyn1_sel.hw,
> + [CLKID_CPU_CLK_DYN1_DIV] = &g12a_cpu_clk_dyn1_div.hw,
> + [CLKID_CPU_CLK_DYN1] = &g12a_cpu_clk_dyn1.hw,
> + [CLKID_CPU_CLK_DYN] = &g12a_cpu_clk_dyn.hw,
> + [CLKID_CPU_CLK] = &g12a_cpu_clk.hw,
> + [CLKID_CPU_CLK_DIV16_EN] = &g12a_cpu_clk_div16_en.hw,
> + [CLKID_CPU_CLK_DIV16] = &g12a_cpu_clk_div16.hw,
> + [CLKID_CPU_CLK_APB_DIV] = &g12a_cpu_clk_apb_div.hw,
> + [CLKID_CPU_CLK_APB] = &g12a_cpu_clk_apb.hw,
> + [CLKID_CPU_CLK_ATB_DIV] = &g12a_cpu_clk_atb_div.hw,
> + [CLKID_CPU_CLK_ATB] = &g12a_cpu_clk_atb.hw,
> + [CLKID_CPU_CLK_AXI_DIV] = &g12a_cpu_clk_axi_div.hw,
> + [CLKID_CPU_CLK_AXI] = &g12a_cpu_clk_axi.hw,
> + [CLKID_CPU_CLK_TRACE_DIV] = &g12a_cpu_clk_trace_div.hw,
> + [CLKID_CPU_CLK_TRACE] = &g12a_cpu_clk_trace.hw,
> [NR_CLKS] = NULL,
> },
> .num = NR_CLKS,
> @@ -2335,6 +2665,24 @@ static struct clk_regmap *const g12a_clk_regmaps[] = {
> &g12a_mali_1,
> &g12a_mali,
> &g12a_mpll_50m,
> + &g12a_sys_pll_div16_en,
> + &g12a_cpu_clk_dyn0_sel,
> + &g12a_cpu_clk_dyn0_div,
> + &g12a_cpu_clk_dyn0,
> + &g12a_cpu_clk_dyn1_sel,
> + &g12a_cpu_clk_dyn1_div,
> + &g12a_cpu_clk_dyn1,
> + &g12a_cpu_clk_dyn,
> + &g12a_cpu_clk,
> + &g12a_cpu_clk_div16_en,
> + &g12a_cpu_clk_apb_div,
> + &g12a_cpu_clk_apb,
> + &g12a_cpu_clk_atb_div,
> + &g12a_cpu_clk_atb,
> + &g12a_cpu_clk_axi_div,
> + &g12a_cpu_clk_axi,
> + &g12a_cpu_clk_trace_div,
> + &g12a_cpu_clk_trace,
> };
>
> static const struct meson_eeclkc_data g12a_clkc_data = {
> diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
> index 4854750df902..0ba3bfe4d46d 100644
> --- a/drivers/clk/meson/g12a.h
> +++ b/drivers/clk/meson/g12a.h
> @@ -50,6 +50,7 @@
> #define HHI_GCLK_MPEG2 0x148
> #define HHI_GCLK_OTHER 0x150
> #define HHI_GCLK_OTHER2 0x154
> +#define HHI_SYS_CPU_CLK_CNTL1 0x15c
> #define HHI_VID_CLK_DIV 0x164
> #define HHI_MPEG_CLK_CNTL 0x174
> #define HHI_AUD_CLK_CNTL 0x178
> --
> 2.20.1
>
>
> _______________________________________________
> linux-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
Hi Martin,
On 01/03/2019 16:21, Martin Blumenstingl wrote:
> Hi Neil,
>
> it's great to see the progress on G12A!
>
> On Fri, Mar 1, 2019 at 11:22 AM Neil Armstrong <[email protected]> wrote:
>>
>> Add the Amlogic G12A Family CPU Clock tree in read/only for now.
>>
>> The CPU clock can either use the SYS_PLL for > 1GHz frequencies or
>> use a couple of div+mux from 1GHz/667MHz/24MHz source with 2 non-glitch
>> muxes.
>>
>> Proper DVFS support will come in a second time.
> can you please also mention that this adds various CPU clock
> post-dividers (APB, ATB, AXI and CPU trace)?
> I don't mind them being int his patchset
indeed, I forgot this !
>
> disclaimer for my code-review:
> - I don't have access to the datasheet so I can't verify if the clock
> tree from this patch is correct
> - the latest buildroot code with G12A support
> (buildroot_openlinux_kernel_4.9_fbdev_20180706) doesn't have proper
> names for all clocks
> - based on my experience with Meson8* this looks good overall, some
> questions and comments below
>
>> Signed-off-by: Neil Armstrong <[email protected]>
>>
>> ---
>> drivers/clk/meson/g12a.c | 348 +++++++++++++++++++++++++++++++++++++++
>> drivers/clk/meson/g12a.h | 1 +
>> 2 files changed, 349 insertions(+)
>>
>> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
>> index 0e1ce8c03259..4c938f1b8421 100644
>> --- a/drivers/clk/meson/g12a.c
>> +++ b/drivers/clk/meson/g12a.c
>> @@ -150,6 +150,316 @@ static struct clk_regmap g12a_sys_pll = {
>> },
>> };
>>
>> +static struct clk_regmap g12a_sys_pll_div16_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .bit_idx = 24,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "sys_pll_div16_en",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_names = (const char *[]){ "sys_pll" },
>> + .num_parents = 1,
>> + /*
>> + * This clock is used to debug the sys_pll range
>> + * Linux should not change it at runtime
>> + */
> if we're not supposed to touch this the enable bit, can you switch to
> clk_regmap_gate_ro_ops ?
exact
>
>> + .flags = CLK_IGNORE_UNUSED,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor g12a_sys_pll_div16 = {
>> + .mult = 1,
>> + .div = 16,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "sys_pll_div16",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_names = (const char *[]){ "sys_pll_div16_en" },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_dyn0_sel = {
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL0,
>> + .mask = 0x3,
>> + .shift = 0,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cpu_clk_dyn0_sel",
> the buildroot code has a variable with the name "p_premux"
> I'm not sure what the datasheet states, but maybe this should be
> cpu_clk_dyn0_pre_sel
> same applies to the corresponding dyn1 clock below
these bit are named "premux1", and cpu_clk_dyn0 names "postmux1",
which has no sense because there is no mux in between.
clk_dyn0_sel is the actual source selector of the dyn0 tree,
clkdyn0 is the top of the dyn0 tree, this is why i did not add "sel"
in it.
>
>> + .ops = &clk_regmap_mux_ro_ops,
>> + .parent_names = (const char *[]){ IN_PREFIX "xtal",
>> + "fclk_div2",
>> + "fclk_div3" },
>> + .num_parents = 3,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_dyn0_div = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL0,
>> + .shift = 4,
>> + .width = 6,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cpu_clk_dyn0_div",
>> + .ops = &clk_regmap_divider_ro_ops,
>> + .parent_names = (const char *[]){ "cpu_clk_dyn0_sel" },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_dyn0 = {
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL0,
>> + .mask = 0x1,
>> + .shift = 2,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cpu_clk_dyn0",
> the buildroot code has a variable with the name "p_postmux". in this
> case I would leave the name "cpu_clk_dyn0" because it's the "output"
> of this specific "clock tree/branch".
> same applies to the corresponding dyn1 clock below
>
>> + .ops = &clk_regmap_mux_ro_ops,
>> + .parent_names = (const char *[]){ "cpu_clk_dyn0_sel",
>> + "cpu_clk_dyn0_div" },
>> + .num_parents = 2,
>> + },
>> +};
>> +
[...]
>> +
>> +static struct clk_regmap g12a_cpu_clk_div16_en = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .bit_idx = 1,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "cpu_clk_div16_en",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_names = (const char *[]){ "cpu_clk" },
>> + .num_parents = 1,
>> + /*
>> + * This clock is used to debug the cpu_clk range
>> + * Linux should not change it at runtime
>> + */
> same as above: if we're not supposed to touch this the enable bit, can
> you switch to clk_regmap_gate_ro_ops ?
Yep
>
>> + .flags = CLK_IGNORE_UNUSED,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor g12a_cpu_clk_div16 = {
>> + .mult = 1,
>> + .div = 16,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cpu_clk_div16",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_names = (const char *[]){ "cpu_clk_div16_en" },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_apb_div = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .shift = 3,
>> + .width = 3,
>> + .flags = CLK_DIVIDER_POWER_OF_TWO,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cpu_clk_apb_div",
>> + .ops = &clk_regmap_divider_ro_ops,
>> + .parent_names = (const char *[]){ "cpu_clk" },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_apb = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .bit_idx = 1,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "cpu_clk_apb",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_names = (const char *[]){ "cpu_clk_apb_div" },
>> + .num_parents = 1,
>> + /*
>> + * This clock is set by the ROM monitor code,
>> + * Linux should not change it at runtime
>> + */
> same as above: if we're not supposed to touch this the enable bit, can
> you switch to clk_regmap_gate_ro_ops ?
Yep
>
>> + .flags = CLK_IGNORE_UNUSED,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_atb_div = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .shift = 6,
>> + .width = 3,
>> + .flags = CLK_DIVIDER_POWER_OF_TWO,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cpu_clk_atb_div",
>> + .ops = &clk_regmap_divider_ro_ops,
>> + .parent_names = (const char *[]){ "cpu_clk" },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_atb = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .bit_idx = 17,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "cpu_clk_atb",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_names = (const char *[]){ "cpu_clk_atb_div" },
>> + .num_parents = 1,
>> + /*
>> + * This clock is set by the ROM monitor code,
>> + * Linux should not change it at runtime
>> + */
> same as above: if we're not supposed to touch this the enable bit, can
> you switch to clk_regmap_gate_ro_ops ?
Yep
>
>> + .flags = CLK_IGNORE_UNUSED,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_axi_div = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .shift = 9,
>> + .width = 3,
>> + .flags = CLK_DIVIDER_POWER_OF_TWO,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cpu_clk_axi_div",
>> + .ops = &clk_regmap_divider_ro_ops,
> out of curiosity (this applies to all CPU clock post-dividers):
> did you check whether CLK_DIVIDER_POWER_OF_TWO is correct on G12A?
> I'm asking because on Meson8b the post-dividers select between one of:
> "cpu_clk divided by 2, 3, 4, 5, 6, 7 or 8". also some of the
> post-dividers use register value 0 for cpu_clk_div2 while others use
> register value 1 for cpu_clk_div2.
It's correct !
>
>> + .parent_names = (const char *[]){ "cpu_clk" },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_axi = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .bit_idx = 18,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "cpu_clk_axi",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_names = (const char *[]){ "cpu_clk_axi_div" },
>> + .num_parents = 1,
>> + /*
>> + * This clock is set by the ROM monitor code,
>> + * Linux should not change it at runtime
>> + */
> same as above: if we're not supposed to touch this the enable bit, can
> you switch to clk_regmap_gate_ro_ops ?
Yep
>
>> + .flags = CLK_IGNORE_UNUSED,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_trace_div = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .shift = 20,
>> + .width = 3,
>> + .flags = CLK_DIVIDER_POWER_OF_TWO,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cpu_clk_trace_div",
>> + .ops = &clk_regmap_divider_ro_ops,
>> + .parent_names = (const char *[]){ "cpu_clk" },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_cpu_clk_trace = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = HHI_SYS_CPU_CLK_CNTL1,
>> + .bit_idx = 23,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "cpu_clk_trace",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_names = (const char *[]){ "cpu_clk_trace_div" },
>> + .num_parents = 1,
>> + /*
>> + * This clock is set by the ROM monitor code,
>> + * Linux should not change it at runtime
>> + */
> same as above: if we're not supposed to touch this the enable bit, can
> you switch to clk_regmap_gate_ro_ops ?
Yep
>
>> + .flags = CLK_IGNORE_UNUSED,
>> + },
>> +};
>> +
>> static const struct pll_mult_range g12a_gp0_pll_mult_range = {
>> .min = 55,
>> .max = 255,
Thanks !
Neil
Hi Martin,
On 01/03/2019 16:26, Martin Blumenstingl wrote:
> Hi Neil,
>
> On Fri, Mar 1, 2019 at 11:22 AM Neil Armstrong <[email protected]> wrote:
>>
>> Add Amlogic G12A Family CPU clocks bindings, only export CPU_CLK since
>> it should be the only ID used.
> is this also true for the CPU post-dividers (APB, ATB, AXI, CPU CLK TRACE)?
Do we need these to be exported ?
>
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> drivers/clk/meson/g12a.h | 24 +++++++++++++++++++++++-
>> include/dt-bindings/clock/g12a-clkc.h | 1 +
>> 2 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
>> index f399dfe1401c..4854750df902 100644
>> --- a/drivers/clk/meson/g12a.h
>> +++ b/drivers/clk/meson/g12a.h
>> @@ -166,8 +166,30 @@
>> #define CLKID_MALI_0_DIV 170
>> #define CLKID_MALI_1_DIV 173
>> #define CLKID_MPLL_5OM_DIV 176
>> +#define CLKID_PCIE_PLL_DCO 178
>> +#define CLKID_PCIE_PLL_DCO_DIV2 179
>> +#define CLKID_PCIE_PLL_OD 180
> how are these PCIe clock related to the CPU clocks?
Oops, bad merge...
>
>> +#define CLKID_SYS_PLL_DIV16_EN 181
>> +#define CLKID_SYS_PLL_DIV16 182
>> +#define CLKID_CPU_CLK_DYN0_SEL 183
>> +#define CLKID_CPU_CLK_DYN0_DIV 184
>> +#define CLKID_CPU_CLK_DYN0 185
>> +#define CLKID_CPU_CLK_DYN1_SEL 186
>> +#define CLKID_CPU_CLK_DYN1_DIV 187
>> +#define CLKID_CPU_CLK_DYN1 188
>> +#define CLKID_CPU_CLK_DYN 189
>> +#define CLKID_CPU_CLK_DIV16_EN 191
>> +#define CLKID_CPU_CLK_DIV16 192
>> +#define CLKID_CPU_CLK_APB_DIV 193
>> +#define CLKID_CPU_CLK_APB 194
>> +#define CLKID_CPU_CLK_ATB_DIV 195
>> +#define CLKID_CPU_CLK_ATB 196
>> +#define CLKID_CPU_CLK_AXI_DIV 197
>> +#define CLKID_CPU_CLK_AXI 198
>> +#define CLKID_CPU_CLK_TRACE_DIV 299
>> +#define CLKID_CPU_CLK_TRACE 200
>>
>> -#define NR_CLKS 178
>> +#define NR_CLKS 201
> shouldn't all changes to this file (drivers/clk/meson/g12a.h) be part
> of the patch which adds the actual clocks so the dt-bindings patch is
> independent of the clock driver patches?
> in this case the subject should also be updated to "dt-bindings:
> clock: g12a: ..."
I don't have any opinion, Jerome ?
>
>
> Regards
> Martin
>
Thanks !
Neil
Hi Neil,
On Fri, Mar 1, 2019 at 5:42 PM Neil Armstrong <[email protected]> wrote:
[...]
> >> +static struct clk_regmap g12a_cpu_clk_dyn0_sel = {
> >> + .data = &(struct clk_regmap_mux_data){
> >> + .offset = HHI_SYS_CPU_CLK_CNTL0,
> >> + .mask = 0x3,
> >> + .shift = 0,
> >> + },
> >> + .hw.init = &(struct clk_init_data){
> >> + .name = "cpu_clk_dyn0_sel",
> > the buildroot code has a variable with the name "p_premux"
> > I'm not sure what the datasheet states, but maybe this should be
> > cpu_clk_dyn0_pre_sel
> > same applies to the corresponding dyn1 clock below
>
> these bit are named "premux1", and cpu_clk_dyn0 names "postmux1",
> which has no sense because there is no mux in between.
>
> clk_dyn0_sel is the actual source selector of the dyn0 tree,
> clkdyn0 is the top of the dyn0 tree, this is why i did not add "sel"
> in it.
OK, thank you for the explanation.
can you please add a comment with the name from the datasheet in that
case? that'll make it easier (at least for me) to compare the
datasheet (and also buildroot kernel, since similar names are used
there) with the mainline drivers
[...]
> >
> >> + .flags = CLK_IGNORE_UNUSED,
> >> + },
> >> +};
> >> +
> >> +static struct clk_regmap g12a_cpu_clk_axi_div = {
> >> + .data = &(struct clk_regmap_div_data){
> >> + .offset = HHI_SYS_CPU_CLK_CNTL1,
> >> + .shift = 9,
> >> + .width = 3,
> >> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> >> + },
> >> + .hw.init = &(struct clk_init_data){
> >> + .name = "cpu_clk_axi_div",
> >> + .ops = &clk_regmap_divider_ro_ops,
> > out of curiosity (this applies to all CPU clock post-dividers):
> > did you check whether CLK_DIVIDER_POWER_OF_TWO is correct on G12A?
> > I'm asking because on Meson8b the post-dividers select between one of:
> > "cpu_clk divided by 2, 3, 4, 5, 6, 7 or 8". also some of the
> > post-dividers use register value 0 for cpu_clk_div2 while others use
> > register value 1 for cpu_clk_div2.
>
> It's correct !
thanks for looking this up again and double-checking!
Martin
Hi Neil,
On Fri, Mar 1, 2019 at 5:43 PM Neil Armstrong <[email protected]> wrote:
>
> Hi Martin,
>
> On 01/03/2019 16:26, Martin Blumenstingl wrote:
> > Hi Neil,
> >
> > On Fri, Mar 1, 2019 at 11:22 AM Neil Armstrong <[email protected]> wrote:
> >>
> >> Add Amlogic G12A Family CPU clocks bindings, only export CPU_CLK since
> >> it should be the only ID used.
> > is this also true for the CPU post-dividers (APB, ATB, AXI, CPU CLK TRACE)?
>
> Do we need these to be exported ?
I'm not sure as I couldn't find more details about APB, ATB and AXI on G12A:
- APB and ATB may be needed by the CoreSight bindings
(Documentation/devicetree/bindings/arm/coresight.txt)
- AXI may be needed by the VPU driver (the S912 datasheet mentions in
OSD1_AFBCD_ENABLE: "id_fifo_thrd : unsigned , default = 64, axi id
fifo threshold")
if you don't know either then I'm fine with skipping them for now, we
can still export them later.
Martin
Hi Neil,
On Fri, Mar 1, 2019 at 11:22 AM Neil Armstrong <[email protected]> wrote:
>
> Add Amlogic G12A Family CPU clocks bindings, only export CPU_CLK since
> it should be the only ID used.
is this also true for the CPU post-dividers (APB, ATB, AXI, CPU CLK TRACE)?
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> drivers/clk/meson/g12a.h | 24 +++++++++++++++++++++++-
> include/dt-bindings/clock/g12a-clkc.h | 1 +
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
> index f399dfe1401c..4854750df902 100644
> --- a/drivers/clk/meson/g12a.h
> +++ b/drivers/clk/meson/g12a.h
> @@ -166,8 +166,30 @@
> #define CLKID_MALI_0_DIV 170
> #define CLKID_MALI_1_DIV 173
> #define CLKID_MPLL_5OM_DIV 176
> +#define CLKID_PCIE_PLL_DCO 178
> +#define CLKID_PCIE_PLL_DCO_DIV2 179
> +#define CLKID_PCIE_PLL_OD 180
how are these PCIe clock related to the CPU clocks?
> +#define CLKID_SYS_PLL_DIV16_EN 181
> +#define CLKID_SYS_PLL_DIV16 182
> +#define CLKID_CPU_CLK_DYN0_SEL 183
> +#define CLKID_CPU_CLK_DYN0_DIV 184
> +#define CLKID_CPU_CLK_DYN0 185
> +#define CLKID_CPU_CLK_DYN1_SEL 186
> +#define CLKID_CPU_CLK_DYN1_DIV 187
> +#define CLKID_CPU_CLK_DYN1 188
> +#define CLKID_CPU_CLK_DYN 189
> +#define CLKID_CPU_CLK_DIV16_EN 191
> +#define CLKID_CPU_CLK_DIV16 192
> +#define CLKID_CPU_CLK_APB_DIV 193
> +#define CLKID_CPU_CLK_APB 194
> +#define CLKID_CPU_CLK_ATB_DIV 195
> +#define CLKID_CPU_CLK_ATB 196
> +#define CLKID_CPU_CLK_AXI_DIV 197
> +#define CLKID_CPU_CLK_AXI 198
> +#define CLKID_CPU_CLK_TRACE_DIV 299
> +#define CLKID_CPU_CLK_TRACE 200
>
> -#define NR_CLKS 178
> +#define NR_CLKS 201
shouldn't all changes to this file (drivers/clk/meson/g12a.h) be part
of the patch which adds the actual clocks so the dt-bindings patch is
independent of the clock driver patches?
in this case the subject should also be updated to "dt-bindings:
clock: g12a: ..."
Regards
Martin