Message ID | 20210223095352.11544-4-zhangqing@rock-chips.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | clk: rockchip: add clock controller for rk3568 | expand |
Hi Elaine, Am Dienstag, 23. Februar 2021, 10:53:51 CET schrieb Elaine Zhang: > A55 supports each core to work at different frequencies, and each core > has an independent divider control. > > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > --- > drivers/clk/rockchip/clk-cpu.c | 25 +++++++++++++++++++++++++ > drivers/clk/rockchip/clk.h | 17 ++++++++++++++++- > 2 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c > index fa9027fb1920..cac06f4f7573 100644 > --- a/drivers/clk/rockchip/clk-cpu.c > +++ b/drivers/clk/rockchip/clk-cpu.c > @@ -164,6 +164,18 @@ static int rockchip_cpuclk_pre_rate_change(struct rockchip_cpuclk *cpuclk, > reg_data->mux_core_mask, > reg_data->mux_core_shift), > cpuclk->reg_base + reg_data->core_reg); > + if (reg_data->core1_reg) > + writel(HIWORD_UPDATE(alt_div, reg_data->div_core1_mask, > + reg_data->div_core1_shift), > + cpuclk->reg_base + reg_data->core1_reg); > + if (reg_data->core2_reg) > + writel(HIWORD_UPDATE(alt_div, reg_data->div_core2_mask, > + reg_data->div_core2_shift), > + cpuclk->reg_base + reg_data->core2_reg); > + if (reg_data->core3_reg) > + writel(HIWORD_UPDATE(alt_div, reg_data->div_core3_mask, > + reg_data->div_core3_shift), > + cpuclk->reg_base + reg_data->core3_reg); for (i = 0; i < reg_data->num_cores; i++) writel(...) > } else { > /* select alternate parent */ > writel(HIWORD_UPDATE(reg_data->mux_core_alt, > @@ -209,6 +221,19 @@ static int rockchip_cpuclk_post_rate_change(struct rockchip_cpuclk *cpuclk, > reg_data->mux_core_shift), > cpuclk->reg_base + reg_data->core_reg); > > + if (reg_data->core1_reg) > + writel(HIWORD_UPDATE(0, reg_data->div_core1_mask, > + reg_data->div_core1_shift), > + cpuclk->reg_base + reg_data->core1_reg); > + if (reg_data->core2_reg) > + writel(HIWORD_UPDATE(0, reg_data->div_core2_mask, > + reg_data->div_core2_shift), > + cpuclk->reg_base + reg_data->core2_reg); > + if (reg_data->core3_reg) > + writel(HIWORD_UPDATE(0, reg_data->div_core3_mask, > + reg_data->div_core3_shift), > + cpuclk->reg_base + reg_data->core3_reg); > + for (i = 0; i < reg_data->num_cores; i++) writel(...) > if (ndata->old_rate > ndata->new_rate) > rockchip_cpuclk_set_dividers(cpuclk, rate); > > diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h > index 2271a84124b0..b46c93fd0cb5 100644 > --- a/drivers/clk/rockchip/clk.h > +++ b/drivers/clk/rockchip/clk.h > @@ -322,7 +322,7 @@ struct rockchip_cpuclk_clksel { > u32 val; > }; > > -#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 2 > +#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 5 please move this into a separate patch, as yes the rk3568 needs more dividers but that isn't related to adding separate core divider controls. [...] add #define ROCKCHIP_CPUCLK_MAX_CORES 4 > struct rockchip_cpuclk_rate_table { > unsigned long prate; > struct rockchip_cpuclk_clksel divs[ROCKCHIP_CPUCLK_NUM_DIVIDERS]; > @@ -333,6 +333,12 @@ struct rockchip_cpuclk_rate_table { > * @core_reg: register offset of the core settings register > * @div_core_shift: core divider offset used to divide the pll value > * @div_core_mask: core divider mask > + * @div_core1_shift: core1 divider offset used to divide the pll value > + * @div_core1_mask: core1 divider mask > + * @div_core2_shift: core2 divider offset used to divide the pll value > + * @div_core2_mask: core2 divider mask > + * @div_core3_shift: core3 divider offset used to divide the pll value > + * @div_core3_mask: core3 divider mask > * @mux_core_alt: mux value to select alternate parent > * @mux_core_main: mux value to select main parent of core > * @mux_core_shift: offset of the core multiplexer > @@ -342,6 +348,15 @@ struct rockchip_cpuclk_reg_data { > int core_reg; > u8 div_core_shift; > u32 div_core_mask; > + int core1_reg; > + u8 div_core1_shift; > + u32 div_core1_mask; > + int core2_reg; > + u8 div_core2_shift; > + u32 div_core2_mask; > + int core3_reg; > + u8 div_core3_shift; > + u32 div_core3_mask; please make this instead like: int core_reg[ROCKCHIP_CPUCLK_MAX_CORES]; u8 div_core_shift[ROCKCHIP_CPUCLK_MAX_CORES]; u32 div_core_mask[ROCKCHIP_CPUCLK_MAX_CORES]; int num_cores; Thanks Heiko > u8 mux_core_alt; > u8 mux_core_main; > u8 mux_core_shift; >
Hi,Heiko: 在 2021/2/23 下午6:22, Heiko Stübner 写道: > Hi Elaine, > > Am Dienstag, 23. Februar 2021, 10:53:51 CET schrieb Elaine Zhang: >> A55 supports each core to work at different frequencies, and each core >> has an independent divider control. >> >> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> >> --- >> drivers/clk/rockchip/clk-cpu.c | 25 +++++++++++++++++++++++++ >> drivers/clk/rockchip/clk.h | 17 ++++++++++++++++- >> 2 files changed, 41 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c >> index fa9027fb1920..cac06f4f7573 100644 >> --- a/drivers/clk/rockchip/clk-cpu.c >> +++ b/drivers/clk/rockchip/clk-cpu.c >> @@ -164,6 +164,18 @@ static int rockchip_cpuclk_pre_rate_change(struct rockchip_cpuclk *cpuclk, >> reg_data->mux_core_mask, >> reg_data->mux_core_shift), >> cpuclk->reg_base + reg_data->core_reg); >> + if (reg_data->core1_reg) >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core1_mask, >> + reg_data->div_core1_shift), >> + cpuclk->reg_base + reg_data->core1_reg); >> + if (reg_data->core2_reg) >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core2_mask, >> + reg_data->div_core2_shift), >> + cpuclk->reg_base + reg_data->core2_reg); >> + if (reg_data->core3_reg) >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core3_mask, >> + reg_data->div_core3_shift), >> + cpuclk->reg_base + reg_data->core3_reg); > for (i = 0; i < reg_data->num_cores; i++) > writel(...) > >> } else { >> /* select alternate parent */ >> writel(HIWORD_UPDATE(reg_data->mux_core_alt, >> @@ -209,6 +221,19 @@ static int rockchip_cpuclk_post_rate_change(struct rockchip_cpuclk *cpuclk, >> reg_data->mux_core_shift), >> cpuclk->reg_base + reg_data->core_reg); >> >> + if (reg_data->core1_reg) >> + writel(HIWORD_UPDATE(0, reg_data->div_core1_mask, >> + reg_data->div_core1_shift), >> + cpuclk->reg_base + reg_data->core1_reg); >> + if (reg_data->core2_reg) >> + writel(HIWORD_UPDATE(0, reg_data->div_core2_mask, >> + reg_data->div_core2_shift), >> + cpuclk->reg_base + reg_data->core2_reg); >> + if (reg_data->core3_reg) >> + writel(HIWORD_UPDATE(0, reg_data->div_core3_mask, >> + reg_data->div_core3_shift), >> + cpuclk->reg_base + reg_data->core3_reg); >> + > for (i = 0; i < reg_data->num_cores; i++) > writel(...) > >> if (ndata->old_rate > ndata->new_rate) >> rockchip_cpuclk_set_dividers(cpuclk, rate); >> >> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h >> index 2271a84124b0..b46c93fd0cb5 100644 >> --- a/drivers/clk/rockchip/clk.h >> +++ b/drivers/clk/rockchip/clk.h >> @@ -322,7 +322,7 @@ struct rockchip_cpuclk_clksel { >> u32 val; >> }; >> >> -#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 2 >> +#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 5 > please move this into a separate patch, as yes the rk3568 needs more > dividers but that isn't related to adding separate core divider controls. > > [...] > add > > #define ROCKCHIP_CPUCLK_MAX_CORES 4 > >> struct rockchip_cpuclk_rate_table { >> unsigned long prate; >> struct rockchip_cpuclk_clksel divs[ROCKCHIP_CPUCLK_NUM_DIVIDERS]; >> @@ -333,6 +333,12 @@ struct rockchip_cpuclk_rate_table { >> * @core_reg: register offset of the core settings register >> * @div_core_shift: core divider offset used to divide the pll value >> * @div_core_mask: core divider mask >> + * @div_core1_shift: core1 divider offset used to divide the pll value >> + * @div_core1_mask: core1 divider mask >> + * @div_core2_shift: core2 divider offset used to divide the pll value >> + * @div_core2_mask: core2 divider mask >> + * @div_core3_shift: core3 divider offset used to divide the pll value >> + * @div_core3_mask: core3 divider mask >> * @mux_core_alt: mux value to select alternate parent >> * @mux_core_main: mux value to select main parent of core >> * @mux_core_shift: offset of the core multiplexer >> @@ -342,6 +348,15 @@ struct rockchip_cpuclk_reg_data { >> int core_reg; >> u8 div_core_shift; >> u32 div_core_mask; >> + int core1_reg; >> + u8 div_core1_shift; >> + u32 div_core1_mask; >> + int core2_reg; >> + u8 div_core2_shift; >> + u32 div_core2_mask; >> + int core3_reg; >> + u8 div_core3_shift; >> + u32 div_core3_mask; > please make this instead like: > > int core_reg[ROCKCHIP_CPUCLK_MAX_CORES]; > u8 div_core_shift[ROCKCHIP_CPUCLK_MAX_CORES]; > u32 div_core_mask[ROCKCHIP_CPUCLK_MAX_CORES]; > int num_cores; This is also my original intention, but with such modification, other SOCs of RK need to be modified, otherwise they cannot be compatible with the old SOC. > > > Thanks > Heiko > > >> u8 mux_core_alt; >> u8 mux_core_main; >> u8 mux_core_shift; >> > > > > >
Hi Elaine, Am Donnerstag, 25. Februar 2021, 03:59:32 CET schrieb elaine.zhang: > 在 2021/2/23 下午6:22, Heiko Stübner 写道: > > Am Dienstag, 23. Februar 2021, 10:53:51 CET schrieb Elaine Zhang: > >> A55 supports each core to work at different frequencies, and each core > >> has an independent divider control. > >> > >> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > >> --- > >> drivers/clk/rockchip/clk-cpu.c | 25 +++++++++++++++++++++++++ > >> drivers/clk/rockchip/clk.h | 17 ++++++++++++++++- > >> 2 files changed, 41 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c > >> index fa9027fb1920..cac06f4f7573 100644 > >> --- a/drivers/clk/rockchip/clk-cpu.c > >> +++ b/drivers/clk/rockchip/clk-cpu.c > >> @@ -164,6 +164,18 @@ static int rockchip_cpuclk_pre_rate_change(struct rockchip_cpuclk *cpuclk, > >> reg_data->mux_core_mask, > >> reg_data->mux_core_shift), > >> cpuclk->reg_base + reg_data->core_reg); > >> + if (reg_data->core1_reg) > >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core1_mask, > >> + reg_data->div_core1_shift), > >> + cpuclk->reg_base + reg_data->core1_reg); > >> + if (reg_data->core2_reg) > >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core2_mask, > >> + reg_data->div_core2_shift), > >> + cpuclk->reg_base + reg_data->core2_reg); > >> + if (reg_data->core3_reg) > >> + writel(HIWORD_UPDATE(alt_div, reg_data->div_core3_mask, > >> + reg_data->div_core3_shift), > >> + cpuclk->reg_base + reg_data->core3_reg); > > for (i = 0; i < reg_data->num_cores; i++) > > writel(...) > > > >> } else { > >> /* select alternate parent */ > >> writel(HIWORD_UPDATE(reg_data->mux_core_alt, > >> @@ -209,6 +221,19 @@ static int rockchip_cpuclk_post_rate_change(struct rockchip_cpuclk *cpuclk, > >> reg_data->mux_core_shift), > >> cpuclk->reg_base + reg_data->core_reg); > >> > >> + if (reg_data->core1_reg) > >> + writel(HIWORD_UPDATE(0, reg_data->div_core1_mask, > >> + reg_data->div_core1_shift), > >> + cpuclk->reg_base + reg_data->core1_reg); > >> + if (reg_data->core2_reg) > >> + writel(HIWORD_UPDATE(0, reg_data->div_core2_mask, > >> + reg_data->div_core2_shift), > >> + cpuclk->reg_base + reg_data->core2_reg); > >> + if (reg_data->core3_reg) > >> + writel(HIWORD_UPDATE(0, reg_data->div_core3_mask, > >> + reg_data->div_core3_shift), > >> + cpuclk->reg_base + reg_data->core3_reg); > >> + > > for (i = 0; i < reg_data->num_cores; i++) > > writel(...) > > > >> if (ndata->old_rate > ndata->new_rate) > >> rockchip_cpuclk_set_dividers(cpuclk, rate); > >> > >> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h > >> index 2271a84124b0..b46c93fd0cb5 100644 > >> --- a/drivers/clk/rockchip/clk.h > >> +++ b/drivers/clk/rockchip/clk.h > >> @@ -322,7 +322,7 @@ struct rockchip_cpuclk_clksel { > >> u32 val; > >> }; > >> > >> -#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 2 > >> +#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 5 > > please move this into a separate patch, as yes the rk3568 needs more > > dividers but that isn't related to adding separate core divider controls. > > > > [...] > > add > > > > #define ROCKCHIP_CPUCLK_MAX_CORES 4 > > > >> struct rockchip_cpuclk_rate_table { > >> unsigned long prate; > >> struct rockchip_cpuclk_clksel divs[ROCKCHIP_CPUCLK_NUM_DIVIDERS]; > >> @@ -333,6 +333,12 @@ struct rockchip_cpuclk_rate_table { > >> * @core_reg: register offset of the core settings register > >> * @div_core_shift: core divider offset used to divide the pll value > >> * @div_core_mask: core divider mask > >> + * @div_core1_shift: core1 divider offset used to divide the pll value > >> + * @div_core1_mask: core1 divider mask > >> + * @div_core2_shift: core2 divider offset used to divide the pll value > >> + * @div_core2_mask: core2 divider mask > >> + * @div_core3_shift: core3 divider offset used to divide the pll value > >> + * @div_core3_mask: core3 divider mask > >> * @mux_core_alt: mux value to select alternate parent > >> * @mux_core_main: mux value to select main parent of core > >> * @mux_core_shift: offset of the core multiplexer > >> @@ -342,6 +348,15 @@ struct rockchip_cpuclk_reg_data { > >> int core_reg; > >> u8 div_core_shift; > >> u32 div_core_mask; > >> + int core1_reg; > >> + u8 div_core1_shift; > >> + u32 div_core1_mask; > >> + int core2_reg; > >> + u8 div_core2_shift; > >> + u32 div_core2_mask; > >> + int core3_reg; > >> + u8 div_core3_shift; > >> + u32 div_core3_mask; > > please make this instead like: > > > > int core_reg[ROCKCHIP_CPUCLK_MAX_CORES]; > > u8 div_core_shift[ROCKCHIP_CPUCLK_MAX_CORES]; > > u32 div_core_mask[ROCKCHIP_CPUCLK_MAX_CORES]; > > int num_cores; > > This is also my original intention, but with such modification, other > SOCs of RK need to be modified, otherwise they cannot be compatible with > the old SOC. That is actually no problem, especially as the diff should be just a simple static const struct rockchip_cpuclk_reg_data px30_cpuclk_data = { - .core_reg = PX30_CLKSEL_CON(0), - .div_core_shift = 0, - .div_core_mask = 0xf, + .core_reg[0] = PX30_CLKSEL_CON(0), + .div_core_shift[0] = 0, + .div_core_mask[0] = 0xf, + .num_cores = 1, .mux_core_alt = 1, .mux_core_main = 0, .mux_core_shift = 7, .mux_core_mask = 0x1, }; So please go the clean way :-) Thanks Heiko
diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c index fa9027fb1920..cac06f4f7573 100644 --- a/drivers/clk/rockchip/clk-cpu.c +++ b/drivers/clk/rockchip/clk-cpu.c @@ -164,6 +164,18 @@ static int rockchip_cpuclk_pre_rate_change(struct rockchip_cpuclk *cpuclk, reg_data->mux_core_mask, reg_data->mux_core_shift), cpuclk->reg_base + reg_data->core_reg); + if (reg_data->core1_reg) + writel(HIWORD_UPDATE(alt_div, reg_data->div_core1_mask, + reg_data->div_core1_shift), + cpuclk->reg_base + reg_data->core1_reg); + if (reg_data->core2_reg) + writel(HIWORD_UPDATE(alt_div, reg_data->div_core2_mask, + reg_data->div_core2_shift), + cpuclk->reg_base + reg_data->core2_reg); + if (reg_data->core3_reg) + writel(HIWORD_UPDATE(alt_div, reg_data->div_core3_mask, + reg_data->div_core3_shift), + cpuclk->reg_base + reg_data->core3_reg); } else { /* select alternate parent */ writel(HIWORD_UPDATE(reg_data->mux_core_alt, @@ -209,6 +221,19 @@ static int rockchip_cpuclk_post_rate_change(struct rockchip_cpuclk *cpuclk, reg_data->mux_core_shift), cpuclk->reg_base + reg_data->core_reg); + if (reg_data->core1_reg) + writel(HIWORD_UPDATE(0, reg_data->div_core1_mask, + reg_data->div_core1_shift), + cpuclk->reg_base + reg_data->core1_reg); + if (reg_data->core2_reg) + writel(HIWORD_UPDATE(0, reg_data->div_core2_mask, + reg_data->div_core2_shift), + cpuclk->reg_base + reg_data->core2_reg); + if (reg_data->core3_reg) + writel(HIWORD_UPDATE(0, reg_data->div_core3_mask, + reg_data->div_core3_shift), + cpuclk->reg_base + reg_data->core3_reg); + if (ndata->old_rate > ndata->new_rate) rockchip_cpuclk_set_dividers(cpuclk, rate); diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h index 2271a84124b0..b46c93fd0cb5 100644 --- a/drivers/clk/rockchip/clk.h +++ b/drivers/clk/rockchip/clk.h @@ -322,7 +322,7 @@ struct rockchip_cpuclk_clksel { u32 val; }; -#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 2 +#define ROCKCHIP_CPUCLK_NUM_DIVIDERS 5 struct rockchip_cpuclk_rate_table { unsigned long prate; struct rockchip_cpuclk_clksel divs[ROCKCHIP_CPUCLK_NUM_DIVIDERS]; @@ -333,6 +333,12 @@ struct rockchip_cpuclk_rate_table { * @core_reg: register offset of the core settings register * @div_core_shift: core divider offset used to divide the pll value * @div_core_mask: core divider mask + * @div_core1_shift: core1 divider offset used to divide the pll value + * @div_core1_mask: core1 divider mask + * @div_core2_shift: core2 divider offset used to divide the pll value + * @div_core2_mask: core2 divider mask + * @div_core3_shift: core3 divider offset used to divide the pll value + * @div_core3_mask: core3 divider mask * @mux_core_alt: mux value to select alternate parent * @mux_core_main: mux value to select main parent of core * @mux_core_shift: offset of the core multiplexer @@ -342,6 +348,15 @@ struct rockchip_cpuclk_reg_data { int core_reg; u8 div_core_shift; u32 div_core_mask; + int core1_reg; + u8 div_core1_shift; + u32 div_core1_mask; + int core2_reg; + u8 div_core2_shift; + u32 div_core2_mask; + int core3_reg; + u8 div_core3_shift; + u32 div_core3_mask; u8 mux_core_alt; u8 mux_core_main; u8 mux_core_shift;
A55 supports each core to work at different frequencies, and each core has an independent divider control. Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> --- drivers/clk/rockchip/clk-cpu.c | 25 +++++++++++++++++++++++++ drivers/clk/rockchip/clk.h | 17 ++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-)