| Message ID | 20220621160621.24415-3-y.oudjana@protonmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | clk: qcom: msm8996-cpu: Cleanup and migrate to parent_data | expand |
On Tue, Jun 21 2022 at 20:02:28 +0300, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > On Tue, 21 Jun 2022 at 19:07, Yassine Oudjana > <yassine.oudjana@gmail.com> wrote: >> >> From: Yassine Oudjana <y.oudjana@protonmail.com> >> >> This will allow for adding them to clk_parent_data arrays >> in an upcoming patch. >> >> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> >> --- >> drivers/clk/qcom/clk-cpu-8996.c | 66 >> +++++++++++++++++++++------------ >> 1 file changed, 42 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/clk/qcom/clk-cpu-8996.c >> b/drivers/clk/qcom/clk-cpu-8996.c >> index 5dc68dc3621f..217f9392c23d 100644 >> --- a/drivers/clk/qcom/clk-cpu-8996.c >> +++ b/drivers/clk/qcom/clk-cpu-8996.c >> @@ -135,6 +135,34 @@ static struct clk_alpha_pll pwrcl_pll = { >> }, >> }; >> >> +static struct clk_fixed_factor pwrcl_pll_postdiv = { >> + .mult = 1, >> + .div = 2, >> + .hw.init = &(struct clk_init_data){ >> + .name = "pwrcl_pll_postdiv", >> + .parent_data = &(const struct clk_parent_data){ >> + .hw = &pwrcl_pll.clkr.hw >> + }, >> + .num_parents = 1, >> + .ops = &clk_fixed_factor_ops, >> + .flags = CLK_SET_RATE_PARENT, >> + }, >> +}; >> + >> +static struct clk_fixed_factor perfcl_pll_postdiv = { >> + .mult = 1, >> + .div = 2, >> + .hw.init = &(struct clk_init_data){ >> + .name = "perfcl_pll_postdiv", >> + .parent_data = &(const struct clk_parent_data){ >> + .hw = &perfcl_pll.clkr.hw >> + }, >> + .num_parents = 1, >> + .ops = &clk_fixed_factor_ops, >> + .flags = CLK_SET_RATE_PARENT, >> + }, >> +}; >> + >> static const struct pll_vco alt_pll_vco_modes[] = { >> VCO(3, 250000000, 500000000), >> VCO(2, 500000000, 750000000), >> @@ -261,7 +289,7 @@ static struct clk_cpu_8996_mux pwrcl_smux = { >> .name = "pwrcl_smux", >> .parent_names = (const char *[]){ >> "xo", >> - "pwrcl_pll_main", >> + "pwrcl_pll_postdiv", >> }, >> .num_parents = 2, >> .ops = &clk_cpu_8996_mux_ops, >> @@ -277,7 +305,7 @@ static struct clk_cpu_8996_mux perfcl_smux = { >> .name = "perfcl_smux", >> .parent_names = (const char *[]){ >> "xo", >> - "perfcl_pll_main", >> + "perfcl_pll_postdiv", >> }, >> .num_parents = 2, >> .ops = &clk_cpu_8996_mux_ops, >> @@ -354,32 +382,25 @@ static int >> qcom_cpu_clk_msm8996_register_clks(struct device *dev, >> { >> int i, ret; >> >> - perfcl_smux.pll = clk_hw_register_fixed_factor(dev, >> "perfcl_pll_main", >> - "perfcl_pll", >> - >> CLK_SET_RATE_PARENT, >> - 1, 2); >> - if (IS_ERR(perfcl_smux.pll)) { >> - dev_err(dev, "Failed to initialize >> perfcl_pll_main\n"); >> - return PTR_ERR(perfcl_smux.pll); >> + ret = devm_clk_hw_register(dev, &pwrcl_pll_postdiv.hw); > > No need to. I'd suggest picking up the > devm_clk_hw_register_fixed_factor patch from my patchset and using > this API. I did it this way to be able to define it statically in the `parent_data` arrays of the secondary muxes in patch 6/6. How would I do it this way? Do I define global `static struct clk_hw *`s for the postdivs and use them in the `parent_data` arrays, or perhaps un-constify the arrays and insert the returned `struct clk_hw *`s into them here? Also can you send a link to your patch? or is it already applied? > >> + if (ret) { >> + dev_err(dev, "Failed to register pwrcl_pll_postdiv: >> %d", ret); >> + return ret; >> } >> >> - pwrcl_smux.pll = clk_hw_register_fixed_factor(dev, >> "pwrcl_pll_main", >> - "pwrcl_pll", >> - >> CLK_SET_RATE_PARENT, >> - 1, 2); >> - if (IS_ERR(pwrcl_smux.pll)) { >> - dev_err(dev, "Failed to initialize >> pwrcl_pll_main\n"); >> - clk_hw_unregister(perfcl_smux.pll); >> - return PTR_ERR(pwrcl_smux.pll); >> + ret = devm_clk_hw_register(dev, &perfcl_pll_postdiv.hw); >> + if (ret) { >> + dev_err(dev, "Failed to register >> perfcl_pll_postdiv: %d", ret); >> + return ret; >> } >> >> + pwrcl_smux.pll = &pwrcl_pll_postdiv.hw; >> + perfcl_smux.pll = &perfcl_pll_postdiv.hw; >> + >> for (i = 0; i < ARRAY_SIZE(cpu_msm8996_clks); i++) { >> ret = devm_clk_register_regmap(dev, >> cpu_msm8996_clks[i]); >> - if (ret) { >> - clk_hw_unregister(perfcl_smux.pll); >> - clk_hw_unregister(pwrcl_smux.pll); >> + if (ret) >> return ret; >> - } >> } >> >> clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config); >> @@ -409,9 +430,6 @@ static int >> qcom_cpu_clk_msm8996_unregister_clks(void) >> if (ret) >> return ret; >> >> - clk_hw_unregister(perfcl_smux.pll); >> - clk_hw_unregister(pwrcl_smux.pll); >> - >> return 0; >> } >> >> -- >> 2.36.1 >> > > > -- > With best wishes > Dmitry
On 14/07/2022 11:32, Yassine Oudjana wrote: > > On Tue, Jun 21 2022 at 20:02:28 +0300, Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> On Tue, 21 Jun 2022 at 19:07, Yassine Oudjana >> <yassine.oudjana@gmail.com> wrote: >>> >>> From: Yassine Oudjana <y.oudjana@protonmail.com> >>> >>> This will allow for adding them to clk_parent_data arrays >>> in an upcoming patch. >>> >>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> >>> --- >>> drivers/clk/qcom/clk-cpu-8996.c | 66 +++++++++++++++++++++------------ >>> 1 file changed, 42 insertions(+), 24 deletions(-) >>> >>> diff --git a/drivers/clk/qcom/clk-cpu-8996.c >>> b/drivers/clk/qcom/clk-cpu-8996.c >>> index 5dc68dc3621f..217f9392c23d 100644 >>> --- a/drivers/clk/qcom/clk-cpu-8996.c >>> +++ b/drivers/clk/qcom/clk-cpu-8996.c >>> @@ -135,6 +135,34 @@ static struct clk_alpha_pll pwrcl_pll = { >>> }, >>> }; >>> >>> +static struct clk_fixed_factor pwrcl_pll_postdiv = { >>> + .mult = 1, >>> + .div = 2, >>> + .hw.init = &(struct clk_init_data){ >>> + .name = "pwrcl_pll_postdiv", >>> + .parent_data = &(const struct clk_parent_data){ >>> + .hw = &pwrcl_pll.clkr.hw >>> + }, >>> + .num_parents = 1, >>> + .ops = &clk_fixed_factor_ops, >>> + .flags = CLK_SET_RATE_PARENT, >>> + }, >>> +}; >>> + >>> +static struct clk_fixed_factor perfcl_pll_postdiv = { >>> + .mult = 1, >>> + .div = 2, >>> + .hw.init = &(struct clk_init_data){ >>> + .name = "perfcl_pll_postdiv", >>> + .parent_data = &(const struct clk_parent_data){ >>> + .hw = &perfcl_pll.clkr.hw >>> + }, >>> + .num_parents = 1, >>> + .ops = &clk_fixed_factor_ops, >>> + .flags = CLK_SET_RATE_PARENT, >>> + }, >>> +}; >>> + >>> static const struct pll_vco alt_pll_vco_modes[] = { >>> VCO(3, 250000000, 500000000), >>> VCO(2, 500000000, 750000000), >>> @@ -261,7 +289,7 @@ static struct clk_cpu_8996_mux pwrcl_smux = { >>> .name = "pwrcl_smux", >>> .parent_names = (const char *[]){ >>> "xo", >>> - "pwrcl_pll_main", >>> + "pwrcl_pll_postdiv", >>> }, >>> .num_parents = 2, >>> .ops = &clk_cpu_8996_mux_ops, >>> @@ -277,7 +305,7 @@ static struct clk_cpu_8996_mux perfcl_smux = { >>> .name = "perfcl_smux", >>> .parent_names = (const char *[]){ >>> "xo", >>> - "perfcl_pll_main", >>> + "perfcl_pll_postdiv", >>> }, >>> .num_parents = 2, >>> .ops = &clk_cpu_8996_mux_ops, >>> @@ -354,32 +382,25 @@ static int >>> qcom_cpu_clk_msm8996_register_clks(struct device *dev, >>> { >>> int i, ret; >>> >>> - perfcl_smux.pll = clk_hw_register_fixed_factor(dev, >>> "perfcl_pll_main", >>> - "perfcl_pll", >>> - CLK_SET_RATE_PARENT, >>> - 1, 2); >>> - if (IS_ERR(perfcl_smux.pll)) { >>> - dev_err(dev, "Failed to initialize perfcl_pll_main\n"); >>> - return PTR_ERR(perfcl_smux.pll); >>> + ret = devm_clk_hw_register(dev, &pwrcl_pll_postdiv.hw); >> >> No need to. I'd suggest picking up the >> devm_clk_hw_register_fixed_factor patch from my patchset and using >> this API. > > I did it this way to be able to define it statically in the > `parent_data` arrays of the secondary muxes in patch 6/6. How > would I do it this way? Do I define global `static struct clk_hw *`s > for the postdivs and use them in the `parent_data` arrays, or > perhaps un-constify the arrays and insert the returned > `struct clk_hw *`s into them here? Also can you send a link to > your patch? or is it already applied? I have been playing with your patchset. In the end I have dropped the idea of using devm_clk_hw_register_fixed_factor() and just used devm_clk_hw_register too. So: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >> >>> + if (ret) { >>> + dev_err(dev, "Failed to register pwrcl_pll_postdiv: >>> %d", ret); >>> + return ret; >>> } >>> >>> - pwrcl_smux.pll = clk_hw_register_fixed_factor(dev, >>> "pwrcl_pll_main", >>> - "pwrcl_pll", >>> - CLK_SET_RATE_PARENT, >>> - 1, 2); >>> - if (IS_ERR(pwrcl_smux.pll)) { >>> - dev_err(dev, "Failed to initialize pwrcl_pll_main\n"); >>> - clk_hw_unregister(perfcl_smux.pll); >>> - return PTR_ERR(pwrcl_smux.pll); >>> + ret = devm_clk_hw_register(dev, &perfcl_pll_postdiv.hw); >>> + if (ret) { >>> + dev_err(dev, "Failed to register perfcl_pll_postdiv: >>> %d", ret); >>> + return ret; >>> } >>> >>> + pwrcl_smux.pll = &pwrcl_pll_postdiv.hw; >>> + perfcl_smux.pll = &perfcl_pll_postdiv.hw; >>> + >>> for (i = 0; i < ARRAY_SIZE(cpu_msm8996_clks); i++) { >>> ret = devm_clk_register_regmap(dev, >>> cpu_msm8996_clks[i]); >>> - if (ret) { >>> - clk_hw_unregister(perfcl_smux.pll); >>> - clk_hw_unregister(pwrcl_smux.pll); >>> + if (ret) >>> return ret; >>> - } >>> } >>> >>> clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config); >>> @@ -409,9 +430,6 @@ static int >>> qcom_cpu_clk_msm8996_unregister_clks(void) >>> if (ret) >>> return ret; >>> >>> - clk_hw_unregister(perfcl_smux.pll); >>> - clk_hw_unregister(pwrcl_smux.pll); >>> - >>> return 0; >>> } >>> >>> -- >>> 2.36.1 >>> >> >> >> -- >> With best wishes >> Dmitry > >
diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c index 5dc68dc3621f..217f9392c23d 100644 --- a/drivers/clk/qcom/clk-cpu-8996.c +++ b/drivers/clk/qcom/clk-cpu-8996.c @@ -135,6 +135,34 @@ static struct clk_alpha_pll pwrcl_pll = { }, }; +static struct clk_fixed_factor pwrcl_pll_postdiv = { + .mult = 1, + .div = 2, + .hw.init = &(struct clk_init_data){ + .name = "pwrcl_pll_postdiv", + .parent_data = &(const struct clk_parent_data){ + .hw = &pwrcl_pll.clkr.hw + }, + .num_parents = 1, + .ops = &clk_fixed_factor_ops, + .flags = CLK_SET_RATE_PARENT, + }, +}; + +static struct clk_fixed_factor perfcl_pll_postdiv = { + .mult = 1, + .div = 2, + .hw.init = &(struct clk_init_data){ + .name = "perfcl_pll_postdiv", + .parent_data = &(const struct clk_parent_data){ + .hw = &perfcl_pll.clkr.hw + }, + .num_parents = 1, + .ops = &clk_fixed_factor_ops, + .flags = CLK_SET_RATE_PARENT, + }, +}; + static const struct pll_vco alt_pll_vco_modes[] = { VCO(3, 250000000, 500000000), VCO(2, 500000000, 750000000), @@ -261,7 +289,7 @@ static struct clk_cpu_8996_mux pwrcl_smux = { .name = "pwrcl_smux", .parent_names = (const char *[]){ "xo", - "pwrcl_pll_main", + "pwrcl_pll_postdiv", }, .num_parents = 2, .ops = &clk_cpu_8996_mux_ops, @@ -277,7 +305,7 @@ static struct clk_cpu_8996_mux perfcl_smux = { .name = "perfcl_smux", .parent_names = (const char *[]){ "xo", - "perfcl_pll_main", + "perfcl_pll_postdiv", }, .num_parents = 2, .ops = &clk_cpu_8996_mux_ops, @@ -354,32 +382,25 @@ static int qcom_cpu_clk_msm8996_register_clks(struct device *dev, { int i, ret; - perfcl_smux.pll = clk_hw_register_fixed_factor(dev, "perfcl_pll_main", - "perfcl_pll", - CLK_SET_RATE_PARENT, - 1, 2); - if (IS_ERR(perfcl_smux.pll)) { - dev_err(dev, "Failed to initialize perfcl_pll_main\n"); - return PTR_ERR(perfcl_smux.pll); + ret = devm_clk_hw_register(dev, &pwrcl_pll_postdiv.hw); + if (ret) { + dev_err(dev, "Failed to register pwrcl_pll_postdiv: %d", ret); + return ret; } - pwrcl_smux.pll = clk_hw_register_fixed_factor(dev, "pwrcl_pll_main", - "pwrcl_pll", - CLK_SET_RATE_PARENT, - 1, 2); - if (IS_ERR(pwrcl_smux.pll)) { - dev_err(dev, "Failed to initialize pwrcl_pll_main\n"); - clk_hw_unregister(perfcl_smux.pll); - return PTR_ERR(pwrcl_smux.pll); + ret = devm_clk_hw_register(dev, &perfcl_pll_postdiv.hw); + if (ret) { + dev_err(dev, "Failed to register perfcl_pll_postdiv: %d", ret); + return ret; } + pwrcl_smux.pll = &pwrcl_pll_postdiv.hw; + perfcl_smux.pll = &perfcl_pll_postdiv.hw; + for (i = 0; i < ARRAY_SIZE(cpu_msm8996_clks); i++) { ret = devm_clk_register_regmap(dev, cpu_msm8996_clks[i]); - if (ret) { - clk_hw_unregister(perfcl_smux.pll); - clk_hw_unregister(pwrcl_smux.pll); + if (ret) return ret; - } } clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config); @@ -409,9 +430,6 @@ static int qcom_cpu_clk_msm8996_unregister_clks(void) if (ret) return ret; - clk_hw_unregister(perfcl_smux.pll); - clk_hw_unregister(pwrcl_smux.pll); - return 0; }