2021-02-23 10:05:52

by zhangqing

[permalink] [raw]
Subject: [PATCH v1 3/4] clk: rockchip: support more core div setting

A55 supports each core to work at different frequencies, and each core
has an independent divider control.

Signed-off-by: Elaine Zhang <[email protected]>
---
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);
} 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;
--
2.17.1




2021-02-23 10:26:28

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v1 3/4] clk: rockchip: support more core div setting

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 <[email protected]>
> ---
> 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;
>




2021-02-25 06:09:23

by zhangqing

[permalink] [raw]
Subject: Re: [PATCH v1 3/4] clk: rockchip: support more core div setting

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 <[email protected]>
>> ---
>> 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;
>>
>
>
>
>
>


2021-02-25 09:59:43

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v1 3/4] clk: rockchip: support more core div setting

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 <[email protected]>
> >> ---
> >> 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