2022-07-26 15:07:37

by Abel Vesa

[permalink] [raw]
Subject: [RFC 0/9] clk: qcom: gcc-sdm845: Swicth from expanded definitions to compact macros

Lets see where this goes.

This RFC is basically a proof-of-concept of how we could use more
compact macros rather than expanded definitions for clocks on QCOM
platforms. As the subject says, this is only for SDM845 GCC, for now.

Also, there are a couple of hacky solutions here that need to be
reworked before it could be merged (if ever). One that comes to mind is
the way the branch clock macros differentiate between having no parent,
a parent name, or a parent HW. I'm still looking into that, so ...

I tried to make them as compact and readable from the user's POV, but
there is some complexity needed in order to allow passing different
types (or number) of parents. Maybe that complexity is too crazy and
doesn't bring that much benefit.

I managed to put together some semi-automated way to do this for every
CC, on every platform, if it's decided so. The only testing I managed to
do so far was comparing the preprocessed output before and after. Oh,
and the MTP does boot. I still need time to make sure that nothing gets
broken.

Also, this series is still WIP, so everything can be reworked fast, if
needed.

Abel Vesa (9):
clk: qcom: qcc-sdm845: Collapse gdsc structs into macros
clk: qcom: gcc-sdm845: Switch from parent_hws to parent_data
clk: qcom: rcg: Add macros to collapse definition
clk: qcom: alpha-pll: Add macros to collapse definition
clk: qcom: branch: Add macros to collapse definition
clk: qcom: common: Add macro wrapper for all clock types
clk: qcom: gcc-sdm845: Switch to macros to collapse branch clocks
definitions
clk: qcom: gcc-sdm845: Switch to macros to collapse rcg2 clocks
definitions
clk: qcom: gcc-sdm845: Switch to macros to collapse alpha-pll clocks
definitions

drivers/clk/qcom/clk-alpha-pll.h | 61 +
drivers/clk/qcom/clk-branch.h | 82 +
drivers/clk/qcom/clk-rcg.h | 40 +
drivers/clk/qcom/common.h | 3 +
drivers/clk/qcom/gcc-sdm845.c | 3222 ++----------------------------
drivers/clk/qcom/gdsc.h | 10 +
6 files changed, 406 insertions(+), 3012 deletions(-)

--
2.34.3


2022-07-26 15:37:02

by Abel Vesa

[permalink] [raw]
Subject: [RFC 9/9] clk: qcom: gcc-sdm845: Switch to macros to collapse alpha-pll clocks definitions

Switch from the expanded alpha-pll clocks definitions to the more compact
macros.

Signed-off-by: Abel Vesa <[email protected]>
---
drivers/clk/qcom/gcc-sdm845.c | 54 +++--------------------------------
1 file changed, 4 insertions(+), 50 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
index d9751d7e617c..ed85d3ba771a 100644
--- a/drivers/clk/qcom/gcc-sdm845.c
+++ b/drivers/clk/qcom/gcc-sdm845.c
@@ -34,40 +34,6 @@ enum {
P_SLEEP_CLK,
};

-static struct clk_alpha_pll gpll0 = {
- .offset = 0x0,
- .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
- .clkr = {
- .enable_reg = 0x52000,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gpll0",
- .parent_data = &(const struct clk_parent_data){
- .fw_name = "bi_tcxo", .name = "bi_tcxo",
- },
- .num_parents = 1,
- .ops = &clk_alpha_pll_fixed_fabia_ops,
- },
- },
-};
-
-static struct clk_alpha_pll gpll4 = {
- .offset = 0x76000,
- .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
- .clkr = {
- .enable_reg = 0x52000,
- .enable_mask = BIT(4),
- .hw.init = &(struct clk_init_data){
- .name = "gpll4",
- .parent_data = &(const struct clk_parent_data){
- .fw_name = "bi_tcxo", .name = "bi_tcxo",
- },
- .num_parents = 1,
- .ops = &clk_alpha_pll_fixed_fabia_ops,
- },
- },
-};
-
static const struct clk_div_table post_div_table_fabia_even[] = {
{ 0x0, 1 },
{ 0x1, 2 },
@@ -76,22 +42,10 @@ static const struct clk_div_table post_div_table_fabia_even[] = {
{ }
};

-static struct clk_alpha_pll_postdiv gpll0_out_even = {
- .offset = 0x0,
- .post_div_shift = 8,
- .post_div_table = post_div_table_fabia_even,
- .num_post_div = ARRAY_SIZE(post_div_table_fabia_even),
- .width = 4,
- .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_FABIA],
- .clkr.hw.init = &(struct clk_init_data){
- .name = "gpll0_out_even",
- .parent_hws = (const struct clk_hw*[]){
- &gpll0.clkr.hw,
- },
- .num_parents = 1,
- .ops = &clk_alpha_pll_postdiv_fabia_ops,
- },
-};
+DEFINE_QCOM_CC_CLK(ALPHA_PLL, gpll0, 0x0, 0x52000, BIT(0), "bi_tcxo");
+DEFINE_QCOM_CC_CLK(ALPHA_PLL, gpll4, 0x76000, 0x52000, BIT(4), "bi_tcxo");
+
+DEFINE_QCOM_CC_CLK(ALPHA_PLL_POSTDIV, gpll0_out_even, 0x0, 8, post_div_table_fabia_even, 4, &gpll0.clkr.hw);

static const struct parent_map gcc_parent_map_0[] = {
{ P_BI_TCXO, 0 },
--
2.34.3

2022-07-26 19:54:14

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [RFC 0/9] clk: qcom: gcc-sdm845: Swicth from expanded definitions to compact macros

Hello!

Probably an unnecessary nit: typo in the word 'Switch' in the title

On 26.07.2022 16:22, Abel Vesa wrote:
> Lets see where this goes.
>
> This RFC is basically a proof-of-concept of how we could use more
> compact macros rather than expanded definitions for clocks on QCOM
> platforms. As the subject says, this is only for SDM845 GCC, for now.
>
> Also, there are a couple of hacky solutions here that need to be
> reworked before it could be merged (if ever). One that comes to mind is
> the way the branch clock macros differentiate between having no parent,
> a parent name, or a parent HW. I'm still looking into that, so ...
>
> I tried to make them as compact and readable from the user's POV, but
> there is some complexity needed in order to allow passing different
> types (or number) of parents. Maybe that complexity is too crazy and
> doesn't bring that much benefit.
>
> I managed to put together some semi-automated way to do this for every
> CC, on every platform, if it's decided so. The only testing I managed to
> do so far was comparing the preprocessed output before and after. Oh,
> and the MTP does boot. I still need time to make sure that nothing gets
> broken.
>
> Also, this series is still WIP, so everything can be reworked fast, if
> needed.
>
> Abel Vesa (9):
> clk: qcom: qcc-sdm845: Collapse gdsc structs into macros
> clk: qcom: gcc-sdm845: Switch from parent_hws to parent_data
> clk: qcom: rcg: Add macros to collapse definition
> clk: qcom: alpha-pll: Add macros to collapse definition
> clk: qcom: branch: Add macros to collapse definition
> clk: qcom: common: Add macro wrapper for all clock types
> clk: qcom: gcc-sdm845: Switch to macros to collapse branch clocks
> definitions
> clk: qcom: gcc-sdm845: Switch to macros to collapse rcg2 clocks
> definitions
> clk: qcom: gcc-sdm845: Switch to macros to collapse alpha-pll clocks
> definitions
>
> drivers/clk/qcom/clk-alpha-pll.h | 61 +
> drivers/clk/qcom/clk-branch.h | 82 +
> drivers/clk/qcom/clk-rcg.h | 40 +
> drivers/clk/qcom/common.h | 3 +
> drivers/clk/qcom/gcc-sdm845.c | 3222 ++----------------------------
> drivers/clk/qcom/gdsc.h | 10 +
> 6 files changed, 406 insertions(+), 3012 deletions(-)
>
> --
At first I was against it, but now I like it. Saves on LoC and clutter.
And it will hopefully finally be reviewable, without 15 levels of
indentation flashing before your eyes :D

I am also happy you mentioned a "semi-automated" way to convert things,
especially since Qualcomm will take some time to adjust to this scheme
on their downstream kernel (it's already really good, I still have
arch/arm/mach-msm flashbacks..) and of course, when it comes to platform
support, the more the merrier!

Thanks for working on this.

Konrad
> 2.34.3
>