Re: [PATCH v1 3/4] clk: rockchip: support more core div setting
From: Heiko Stübner
Date: Thu Feb 25 2021 - 04:12:22 EST
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@xxxxxxxxxxxxxx>
>
>> ---
>
>> 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