| Message ID | 20190301102140.7181-3-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clk: meson: g12a: Add CPU Clock support | expand |
Hi Neil, it's great to see the progress on G12A! On Fri, Mar 1, 2019 at 11:22 AM Neil Armstrong <narmstrong@baylibre.com> 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 <narmstrong@baylibre.com> > > --- > 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 > linux-amlogic@lists.infradead.org > 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 <narmstrong@baylibre.com> 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 <narmstrong@baylibre.com> >> >> --- >> 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 Neil, On Fri, Mar 1, 2019 at 5:42 PM Neil Armstrong <narmstrong@baylibre.com> 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
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
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 <narmstrong@baylibre.com> --- drivers/clk/meson/g12a.c | 348 +++++++++++++++++++++++++++++++++++++++ drivers/clk/meson/g12a.h | 1 + 2 files changed, 349 insertions(+)