From: "Heiko Stübner" <heiko@sntech.de>
To: sboyd@kernel.org, "elaine.zhang" <zhangqing@rock-chips.com>
Cc: huangtao@rock-chips.com, linux-kernel@vger.kernel.org,
kever.yang@rock-chips.com, linux-rockchip@lists.infradead.org,
tony.xie@rock-chips.com, finley.xiao@rock-chips.com,
linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
cl@rock-chips.com
Subject: Re: [PATCH v1 3/4] clk: rockchip: support more core div setting
Date: Thu, 25 Feb 2021 10:07:36 +0100 [thread overview]
Message-ID: <3100125.44csPzL39Z@diego> (raw)
In-Reply-To: <6759a56c-56d1-da63-5299-0d76966329df@rock-chips.com>
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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2021-02-25 9:07 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-23 9:53 [PATCH v1 0/4] clk: rockchip: add clock controller for rk3568 Elaine Zhang
2021-02-23 9:53 ` [PATCH v1 1/4] dt-bindings: add bindings for rk3568 clock controller Elaine Zhang
2021-02-23 10:50 ` Johan Jonker
2021-02-23 10:54 ` Heiko Stübner
2021-02-23 9:53 ` [PATCH v1 2/4] clk: rockchip: add dt-binding header for rk3568 Elaine Zhang
2021-02-23 10:45 ` Heiko Stübner
2021-02-24 6:35 ` elaine.zhang
2021-02-24 7:27 ` Heiko Stübner
2021-02-23 9:53 ` [PATCH v1 3/4] clk: rockchip: support more core div setting Elaine Zhang
2021-02-23 10:22 ` Heiko Stübner
2021-02-25 2:59 ` elaine.zhang
2021-02-25 9:07 ` Heiko Stübner [this message]
2021-02-23 9:53 ` [PATCH v1 4/4] clk: rockchip: add clock controller for rk3568 Elaine Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3100125.44csPzL39Z@diego \
--to=heiko@sntech.de \
--cc=cl@rock-chips.com \
--cc=finley.xiao@rock-chips.com \
--cc=huangtao@rock-chips.com \
--cc=kever.yang@rock-chips.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=sboyd@kernel.org \
--cc=tony.xie@rock-chips.com \
--cc=zhangqing@rock-chips.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).