[PATCH 0/6] clk: qcom: msm8996-cpu: Cleanup and migrate to parent_data

[PATCH 0/6] clk: qcom: msm8996-cpu: Cleanup and migrate to parent_data

[Yassine Oudjana]

This series includes some cleanup of the MSM8996 CPU clock driver, as well as
migration from parent_names to parent_data for all of its clocks. The DT schema
is also fixed in this series to show the actual clocks consumed by the clock
controller and pass checks.

Yassine Oudjana (6):
  clk: qcom: msm8996-cpu: Rename DIV_2_INDEX to SMUX_INDEX
  clk: qcom: msm8996-cpu: Statically define PLL dividers
  clk: qcom: msm8996-cpu: Unify cluster order
  clk: qcom: msm8996-cpu: Convert secondary muxes to clk_regmap_mux
  dt-bindings: clock: qcom,msm8996-apcc: Fix clocks
  clk: qcom: msm8996-cpu: Use parent_data for all clocks

 .../bindings/clock/qcom,msm8996-apcc.yaml     |  15 +-
 drivers/clk/qcom/clk-cpu-8996.c               | 235 ++++++++++--------
 2 files changed, 140 insertions(+), 110 deletions(-)

-- 
2.36.1

[PATCH 1/6] clk: qcom: msm8996-cpu: Rename DIV_2_INDEX to SMUX_INDEX

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

The parent at this index is the secondary mux, which can connect
not only to primary PLL/2 but also to XO. Rename the index to SMUX_INDEX
to better reflect the parent.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/clk/qcom/clk-cpu-8996.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index 4a4fde8dd12d..5dc68dc3621f 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -61,7 +61,7 @@
 #include "clk-regmap.h"
 
 enum _pmux_input {
-	DIV_2_INDEX = 0,
+	SMUX_INDEX = 0,
 	PLL_INDEX,
 	ACD_INDEX,
 	ALT_INDEX,
@@ -468,7 +468,7 @@ static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 	case POST_RATE_CHANGE:
 		if (cnd->new_rate < DIV_2_THRESHOLD)
 			ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
-							  DIV_2_INDEX);
+							  SMUX_INDEX);
 		else
 			ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
 							  ACD_INDEX);
-- 
2.36.1

Re: [PATCH 1/6] clk: qcom: msm8996-cpu: Rename DIV_2_INDEX to SMUX_INDEX

[Dmitry Baryshkov]

On Tue, 21 Jun 2022 at 19:07, Yassine Oudjana <yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> The parent at this index is the secondary mux, which can connect
> not only to primary PLL/2 but also to XO. Rename the index to SMUX_INDEX
> to better reflect the parent.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/clk/qcom/clk-cpu-8996.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)


-- 
With best wishes
Dmitry

[PATCH 2/6] clk: qcom: msm8996-cpu: Statically define PLL dividers

[Yassine Oudjana]

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);
+	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

Re: [PATCH 2/6] clk: qcom: msm8996-cpu: Statically define PLL dividers

[Dmitry Baryshkov]

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.

> +       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

Re: [PATCH 2/6] clk: qcom: msm8996-cpu: Statically define PLL dividers

[Yassine Oudjana]

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

Re: [PATCH 2/6] clk: qcom: msm8996-cpu: Statically define PLL dividers

[Dmitry Baryshkov]

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
> 
> 


-- 
With best wishes
Dmitry

[PATCH 3/6] clk: qcom: msm8996-cpu: Unify cluster order

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

The power cluster comes before the performance cluster. Make
everything in the driver follow this order.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/clk/qcom/clk-cpu-8996.c | 36 ++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index 217f9392c23d..b6761a74d5ac 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -111,24 +111,24 @@ static const struct alpha_pll_config hfpll_config = {
 	.early_output_mask = BIT(3),
 };
 
-static struct clk_alpha_pll perfcl_pll = {
-	.offset = PERFCL_REG_OFFSET,
+static struct clk_alpha_pll pwrcl_pll = {
+	.offset = PWRCL_REG_OFFSET,
 	.regs = prim_pll_regs,
 	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data){
-		.name = "perfcl_pll",
+		.name = "pwrcl_pll",
 		.parent_names = (const char *[]){ "xo" },
 		.num_parents = 1,
 		.ops = &clk_alpha_pll_huayra_ops,
 	},
 };
 
-static struct clk_alpha_pll pwrcl_pll = {
-	.offset = PWRCL_REG_OFFSET,
+static struct clk_alpha_pll perfcl_pll = {
+	.offset = PERFCL_REG_OFFSET,
 	.regs = prim_pll_regs,
 	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data){
-		.name = "pwrcl_pll",
+		.name = "perfcl_pll",
 		.parent_names = (const char *[]){ "xo" },
 		.num_parents = 1,
 		.ops = &clk_alpha_pll_huayra_ops,
@@ -181,28 +181,28 @@ static const struct alpha_pll_config altpll_config = {
 	.early_output_mask = BIT(3),
 };
 
-static struct clk_alpha_pll perfcl_alt_pll = {
-	.offset = PERFCL_REG_OFFSET + ALT_PLL_OFFSET,
+static struct clk_alpha_pll pwrcl_alt_pll = {
+	.offset = PWRCL_REG_OFFSET + ALT_PLL_OFFSET,
 	.regs = alt_pll_regs,
 	.vco_table = alt_pll_vco_modes,
 	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
 	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data) {
-		.name = "perfcl_alt_pll",
+		.name = "pwrcl_alt_pll",
 		.parent_names = (const char *[]){ "xo" },
 		.num_parents = 1,
 		.ops = &clk_alpha_pll_hwfsm_ops,
 	},
 };
 
-static struct clk_alpha_pll pwrcl_alt_pll = {
-	.offset = PWRCL_REG_OFFSET + ALT_PLL_OFFSET,
+static struct clk_alpha_pll perfcl_alt_pll = {
+	.offset = PERFCL_REG_OFFSET + ALT_PLL_OFFSET,
 	.regs = alt_pll_regs,
 	.vco_table = alt_pll_vco_modes,
 	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
 	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data) {
-		.name = "pwrcl_alt_pll",
+		.name = "perfcl_alt_pll",
 		.parent_names = (const char *[]){ "xo" },
 		.num_parents = 1,
 		.ops = &clk_alpha_pll_hwfsm_ops,
@@ -367,14 +367,14 @@ static const struct regmap_config cpu_msm8996_regmap_config = {
 };
 
 static struct clk_regmap *cpu_msm8996_clks[] = {
-	&perfcl_pll.clkr,
 	&pwrcl_pll.clkr,
-	&perfcl_alt_pll.clkr,
+	&perfcl_pll.clkr,
 	&pwrcl_alt_pll.clkr,
-	&perfcl_smux.clkr,
+	&perfcl_alt_pll.clkr,
 	&pwrcl_smux.clkr,
-	&perfcl_pmux.clkr,
+	&perfcl_smux.clkr,
 	&pwrcl_pmux.clkr,
+	&perfcl_pmux.clkr,
 };
 
 static int qcom_cpu_clk_msm8996_register_clks(struct device *dev,
@@ -403,10 +403,10 @@ static int qcom_cpu_clk_msm8996_register_clks(struct device *dev,
 			return ret;
 	}
 
-	clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config);
 	clk_alpha_pll_configure(&pwrcl_pll, regmap, &hfpll_config);
-	clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);
+	clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config);
 	clk_alpha_pll_configure(&pwrcl_alt_pll, regmap, &altpll_config);
+	clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);
 
 	/* Enable alt PLLs */
 	clk_prepare_enable(pwrcl_alt_pll.clkr.hw.clk);
-- 
2.36.1

Re: [PATCH 3/6] clk: qcom: msm8996-cpu: Unify cluster order

[Dmitry Baryshkov]

On Tue, 21 Jun 2022 at 19:07, Yassine Oudjana <yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> The power cluster comes before the performance cluster. Make
> everything in the driver follow this order.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/clk/qcom/clk-cpu-8996.c | 36 ++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 18 deletions(-)

-- 
With best wishes
Dmitry

[PATCH 4/6] clk: qcom: msm8996-cpu: Convert secondary muxes to clk_regmap_mux

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

There is nothing special about the secondary muxes, unlike the
primary muxes which need some extra logic to handle ACD and
switching between primary PLL and secondary mux sources. Turn
them into clk_regmap_mux and rename cpu_clk_msm8996_mux into
cpu_clk_msm8996_pmux to make it specific to primary muxes.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/clk/qcom/clk-cpu-8996.c | 62 ++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index b6761a74d5ac..b3ad9245874d 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -59,6 +59,7 @@
 
 #include "clk-alpha-pll.h"
 #include "clk-regmap.h"
+#include "clk-regmap-mux.h"
 
 enum _pmux_input {
 	SMUX_INDEX = 0,
@@ -209,7 +210,7 @@ static struct clk_alpha_pll perfcl_alt_pll = {
 	},
 };
 
-struct clk_cpu_8996_mux {
+struct clk_cpu_8996_pmux {
 	u32	reg;
 	u8	shift;
 	u8	width;
@@ -222,18 +223,18 @@ struct clk_cpu_8996_mux {
 static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 			       void *data);
 
-#define to_clk_cpu_8996_mux_nb(_nb) \
-	container_of(_nb, struct clk_cpu_8996_mux, nb)
+#define to_clk_cpu_8996_pmux_nb(_nb) \
+	container_of(_nb, struct clk_cpu_8996_pmux, nb)
 
-static inline struct clk_cpu_8996_mux *to_clk_cpu_8996_mux_hw(struct clk_hw *hw)
+static inline struct clk_cpu_8996_pmux *to_clk_cpu_8996_pmux_hw(struct clk_hw *hw)
 {
-	return container_of(to_clk_regmap(hw), struct clk_cpu_8996_mux, clkr);
+	return container_of(to_clk_regmap(hw), struct clk_cpu_8996_pmux, clkr);
 }
 
-static u8 clk_cpu_8996_mux_get_parent(struct clk_hw *hw)
+static u8 clk_cpu_8996_pmux_get_parent(struct clk_hw *hw)
 {
 	struct clk_regmap *clkr = to_clk_regmap(hw);
-	struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
+	struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_hw(hw);
 	u32 mask = GENMASK(cpuclk->width - 1, 0);
 	u32 val;
 
@@ -243,10 +244,10 @@ static u8 clk_cpu_8996_mux_get_parent(struct clk_hw *hw)
 	return val & mask;
 }
 
-static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
+static int clk_cpu_8996_pmux_set_parent(struct clk_hw *hw, u8 index)
 {
 	struct clk_regmap *clkr = to_clk_regmap(hw);
-	struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
+	struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_hw(hw);
 	u32 mask = GENMASK(cpuclk->width + cpuclk->shift - 1, cpuclk->shift);
 	u32 val;
 
@@ -256,10 +257,10 @@ static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
 	return regmap_update_bits(clkr->regmap, cpuclk->reg, mask, val);
 }
 
-static int clk_cpu_8996_mux_determine_rate(struct clk_hw *hw,
+static int clk_cpu_8996_pmux_determine_rate(struct clk_hw *hw,
 					   struct clk_rate_request *req)
 {
-	struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
+	struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_hw(hw);
 	struct clk_hw *parent = cpuclk->pll;
 
 	if (cpuclk->pll_div_2 && req->rate < DIV_2_THRESHOLD) {
@@ -275,13 +276,13 @@ static int clk_cpu_8996_mux_determine_rate(struct clk_hw *hw,
 	return 0;
 }
 
-static const struct clk_ops clk_cpu_8996_mux_ops = {
-	.set_parent = clk_cpu_8996_mux_set_parent,
-	.get_parent = clk_cpu_8996_mux_get_parent,
-	.determine_rate = clk_cpu_8996_mux_determine_rate,
+static const struct clk_ops clk_cpu_8996_pmux_ops = {
+	.set_parent = clk_cpu_8996_pmux_set_parent,
+	.get_parent = clk_cpu_8996_pmux_get_parent,
+	.determine_rate = clk_cpu_8996_pmux_determine_rate,
 };
 
-static struct clk_cpu_8996_mux pwrcl_smux = {
+static struct clk_regmap_mux pwrcl_smux = {
 	.reg = PWRCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 2,
 	.width = 2,
@@ -292,12 +293,12 @@ static struct clk_cpu_8996_mux pwrcl_smux = {
 			"pwrcl_pll_postdiv",
 		},
 		.num_parents = 2,
-		.ops = &clk_cpu_8996_mux_ops,
+		.ops = &clk_regmap_mux_closest_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	},
 };
 
-static struct clk_cpu_8996_mux perfcl_smux = {
+static struct clk_regmap_mux perfcl_smux = {
 	.reg = PERFCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 2,
 	.width = 2,
@@ -308,12 +309,12 @@ static struct clk_cpu_8996_mux perfcl_smux = {
 			"perfcl_pll_postdiv",
 		},
 		.num_parents = 2,
-		.ops = &clk_cpu_8996_mux_ops,
+		.ops = &clk_regmap_mux_closest_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	},
 };
 
-static struct clk_cpu_8996_mux pwrcl_pmux = {
+static struct clk_cpu_8996_pmux pwrcl_pmux = {
 	.reg = PWRCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 0,
 	.width = 2,
@@ -329,13 +330,13 @@ static struct clk_cpu_8996_mux pwrcl_pmux = {
 			"pwrcl_alt_pll",
 		},
 		.num_parents = 4,
-		.ops = &clk_cpu_8996_mux_ops,
+		.ops = &clk_cpu_8996_pmux_ops,
 		/* CPU clock is critical and should never be gated */
 		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 	},
 };
 
-static struct clk_cpu_8996_mux perfcl_pmux = {
+static struct clk_cpu_8996_pmux perfcl_pmux = {
 	.reg = PERFCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 0,
 	.width = 2,
@@ -351,7 +352,7 @@ static struct clk_cpu_8996_mux perfcl_pmux = {
 			"perfcl_alt_pll",
 		},
 		.num_parents = 4,
-		.ops = &clk_cpu_8996_mux_ops,
+		.ops = &clk_cpu_8996_pmux_ops,
 		/* CPU clock is critical and should never be gated */
 		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 	},
@@ -394,9 +395,6 @@ static int qcom_cpu_clk_msm8996_register_clks(struct device *dev,
 		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)
@@ -474,22 +472,22 @@ static void qcom_cpu_clk_msm8996_acd_init(void __iomem *base)
 static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 			       void *data)
 {
-	struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_nb(nb);
+	struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_nb(nb);
 	struct clk_notifier_data *cnd = data;
 	int ret;
 
 	switch (event) {
 	case PRE_RATE_CHANGE:
-		ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
+		ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
 		qcom_cpu_clk_msm8996_acd_init(base);
 		break;
 	case POST_RATE_CHANGE:
 		if (cnd->new_rate < DIV_2_THRESHOLD)
-			ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
-							  SMUX_INDEX);
+			ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw,
+							   SMUX_INDEX);
 		else
-			ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
-							  ACD_INDEX);
+			ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw,
+							   ACD_INDEX);
 		break;
 	default:
 		ret = 0;
-- 
2.36.1

Re: [PATCH 4/6] clk: qcom: msm8996-cpu: Convert secondary muxes to clk_regmap_mux

[Dmitry Baryshkov]

On Tue, 21 Jun 2022 at 19:07, Yassine Oudjana <yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> There is nothing special about the secondary muxes, unlike the
> primary muxes which need some extra logic to handle ACD and
> switching between primary PLL and secondary mux sources. Turn
> them into clk_regmap_mux and rename cpu_clk_msm8996_mux into
> cpu_clk_msm8996_pmux to make it specific to primary muxes.

Nice idea!
I think this also allows us to clean up the pmux_ops to remove extra
conditions, doesn't it? (as a followup patch)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
>  drivers/clk/qcom/clk-cpu-8996.c | 62 ++++++++++++++++-----------------
>  1 file changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
> index b6761a74d5ac..b3ad9245874d 100644
> --- a/drivers/clk/qcom/clk-cpu-8996.c
> +++ b/drivers/clk/qcom/clk-cpu-8996.c
> @@ -59,6 +59,7 @@
>
>  #include "clk-alpha-pll.h"
>  #include "clk-regmap.h"
> +#include "clk-regmap-mux.h"
>
>  enum _pmux_input {
>         SMUX_INDEX = 0,
> @@ -209,7 +210,7 @@ static struct clk_alpha_pll perfcl_alt_pll = {
>         },
>  };
>
> -struct clk_cpu_8996_mux {
> +struct clk_cpu_8996_pmux {
>         u32     reg;
>         u8      shift;
>         u8      width;
> @@ -222,18 +223,18 @@ struct clk_cpu_8996_mux {
>  static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
>                                void *data);
>
> -#define to_clk_cpu_8996_mux_nb(_nb) \
> -       container_of(_nb, struct clk_cpu_8996_mux, nb)
> +#define to_clk_cpu_8996_pmux_nb(_nb) \
> +       container_of(_nb, struct clk_cpu_8996_pmux, nb)
>
> -static inline struct clk_cpu_8996_mux *to_clk_cpu_8996_mux_hw(struct clk_hw *hw)
> +static inline struct clk_cpu_8996_pmux *to_clk_cpu_8996_pmux_hw(struct clk_hw *hw)
>  {
> -       return container_of(to_clk_regmap(hw), struct clk_cpu_8996_mux, clkr);
> +       return container_of(to_clk_regmap(hw), struct clk_cpu_8996_pmux, clkr);
>  }
>
> -static u8 clk_cpu_8996_mux_get_parent(struct clk_hw *hw)
> +static u8 clk_cpu_8996_pmux_get_parent(struct clk_hw *hw)
>  {
>         struct clk_regmap *clkr = to_clk_regmap(hw);
> -       struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
> +       struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_hw(hw);
>         u32 mask = GENMASK(cpuclk->width - 1, 0);
>         u32 val;
>
> @@ -243,10 +244,10 @@ static u8 clk_cpu_8996_mux_get_parent(struct clk_hw *hw)
>         return val & mask;
>  }
>
> -static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
> +static int clk_cpu_8996_pmux_set_parent(struct clk_hw *hw, u8 index)
>  {
>         struct clk_regmap *clkr = to_clk_regmap(hw);
> -       struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
> +       struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_hw(hw);
>         u32 mask = GENMASK(cpuclk->width + cpuclk->shift - 1, cpuclk->shift);
>         u32 val;
>
> @@ -256,10 +257,10 @@ static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
>         return regmap_update_bits(clkr->regmap, cpuclk->reg, mask, val);
>  }
>
> -static int clk_cpu_8996_mux_determine_rate(struct clk_hw *hw,
> +static int clk_cpu_8996_pmux_determine_rate(struct clk_hw *hw,
>                                            struct clk_rate_request *req)
>  {
> -       struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
> +       struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_hw(hw);
>         struct clk_hw *parent = cpuclk->pll;
>
>         if (cpuclk->pll_div_2 && req->rate < DIV_2_THRESHOLD) {
> @@ -275,13 +276,13 @@ static int clk_cpu_8996_mux_determine_rate(struct clk_hw *hw,
>         return 0;
>  }
>
> -static const struct clk_ops clk_cpu_8996_mux_ops = {
> -       .set_parent = clk_cpu_8996_mux_set_parent,
> -       .get_parent = clk_cpu_8996_mux_get_parent,
> -       .determine_rate = clk_cpu_8996_mux_determine_rate,
> +static const struct clk_ops clk_cpu_8996_pmux_ops = {
> +       .set_parent = clk_cpu_8996_pmux_set_parent,
> +       .get_parent = clk_cpu_8996_pmux_get_parent,
> +       .determine_rate = clk_cpu_8996_pmux_determine_rate,
>  };
>
> -static struct clk_cpu_8996_mux pwrcl_smux = {
> +static struct clk_regmap_mux pwrcl_smux = {
>         .reg = PWRCL_REG_OFFSET + MUX_OFFSET,
>         .shift = 2,
>         .width = 2,
> @@ -292,12 +293,12 @@ static struct clk_cpu_8996_mux pwrcl_smux = {
>                         "pwrcl_pll_postdiv",
>                 },
>                 .num_parents = 2,
> -               .ops = &clk_cpu_8996_mux_ops,
> +               .ops = &clk_regmap_mux_closest_ops,
>                 .flags = CLK_SET_RATE_PARENT,
>         },
>  };
>
> -static struct clk_cpu_8996_mux perfcl_smux = {
> +static struct clk_regmap_mux perfcl_smux = {
>         .reg = PERFCL_REG_OFFSET + MUX_OFFSET,
>         .shift = 2,
>         .width = 2,
> @@ -308,12 +309,12 @@ static struct clk_cpu_8996_mux perfcl_smux = {
>                         "perfcl_pll_postdiv",
>                 },
>                 .num_parents = 2,
> -               .ops = &clk_cpu_8996_mux_ops,
> +               .ops = &clk_regmap_mux_closest_ops,
>                 .flags = CLK_SET_RATE_PARENT,
>         },
>  };
>
> -static struct clk_cpu_8996_mux pwrcl_pmux = {
> +static struct clk_cpu_8996_pmux pwrcl_pmux = {
>         .reg = PWRCL_REG_OFFSET + MUX_OFFSET,
>         .shift = 0,
>         .width = 2,
> @@ -329,13 +330,13 @@ static struct clk_cpu_8996_mux pwrcl_pmux = {
>                         "pwrcl_alt_pll",
>                 },
>                 .num_parents = 4,
> -               .ops = &clk_cpu_8996_mux_ops,
> +               .ops = &clk_cpu_8996_pmux_ops,
>                 /* CPU clock is critical and should never be gated */
>                 .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>         },
>  };
>
> -static struct clk_cpu_8996_mux perfcl_pmux = {
> +static struct clk_cpu_8996_pmux perfcl_pmux = {
>         .reg = PERFCL_REG_OFFSET + MUX_OFFSET,
>         .shift = 0,
>         .width = 2,
> @@ -351,7 +352,7 @@ static struct clk_cpu_8996_mux perfcl_pmux = {
>                         "perfcl_alt_pll",
>                 },
>                 .num_parents = 4,
> -               .ops = &clk_cpu_8996_mux_ops,
> +               .ops = &clk_cpu_8996_pmux_ops,
>                 /* CPU clock is critical and should never be gated */
>                 .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>         },
> @@ -394,9 +395,6 @@ static int qcom_cpu_clk_msm8996_register_clks(struct device *dev,
>                 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)
> @@ -474,22 +472,22 @@ static void qcom_cpu_clk_msm8996_acd_init(void __iomem *base)
>  static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
>                                void *data)
>  {
> -       struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_nb(nb);
> +       struct clk_cpu_8996_pmux *cpuclk = to_clk_cpu_8996_pmux_nb(nb);
>         struct clk_notifier_data *cnd = data;
>         int ret;
>
>         switch (event) {
>         case PRE_RATE_CHANGE:
> -               ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
> +               ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, ALT_INDEX);
>                 qcom_cpu_clk_msm8996_acd_init(base);
>                 break;
>         case POST_RATE_CHANGE:
>                 if (cnd->new_rate < DIV_2_THRESHOLD)
> -                       ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
> -                                                         SMUX_INDEX);
> +                       ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw,
> +                                                          SMUX_INDEX);
>                 else
> -                       ret = clk_cpu_8996_mux_set_parent(&cpuclk->clkr.hw,
> -                                                         ACD_INDEX);
> +                       ret = clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw,
> +                                                          ACD_INDEX);
>                 break;
>         default:
>                 ret = 0;
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

[PATCH 5/6] dt-bindings: clock: qcom,msm8996-apcc: Fix clocks

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

The clocks currently listed in clocks and clock-names are the ones
supplied by this clock controller, not the ones it consumes. Replace
them with the only clock it consumes - the on-board oscillator (XO),
and make the properties required.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 .../bindings/clock/qcom,msm8996-apcc.yaml         | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
index a20cb10636dd..c4971234fef8 100644
--- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
@@ -26,22 +26,18 @@ properties:
 
   clocks:
     items:
-      - description: Primary PLL clock for power cluster (little)
-      - description: Primary PLL clock for perf cluster (big)
-      - description: Alternate PLL clock for power cluster (little)
-      - description: Alternate PLL clock for perf cluster (big)
+      - description: XO source
 
   clock-names:
     items:
-      - const: pwrcl_pll
-      - const: perfcl_pll
-      - const: pwrcl_alt_pll
-      - const: perfcl_alt_pll
+      - const: xo
 
 required:
   - compatible
   - reg
   - '#clock-cells'
+  - clocks
+  - clock-names
 
 additionalProperties: false
 
@@ -51,4 +47,7 @@ examples:
         compatible = "qcom,msm8996-apcc";
         reg = <0x6400000 0x90000>;
         #clock-cells = <1>;
+
+        clocks = <&xo_board>;
+        clock-names = "xo";
     };
-- 
2.36.1

Re: [PATCH 5/6] dt-bindings: clock: qcom,msm8996-apcc: Fix clocks

[Dmitry Baryshkov]

On Tue, 21 Jun 2022 at 19:07, Yassine Oudjana <yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> The clocks currently listed in clocks and clock-names are the ones
> supplied by this clock controller, not the ones it consumes. Replace
> them with the only clock it consumes - the on-board oscillator (XO),
> and make the properties required.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
>  .../bindings/clock/qcom,msm8996-apcc.yaml         | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> index a20cb10636dd..c4971234fef8 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> @@ -26,22 +26,18 @@ properties:
>
>    clocks:
>      items:
> -      - description: Primary PLL clock for power cluster (little)
> -      - description: Primary PLL clock for perf cluster (big)
> -      - description: Alternate PLL clock for power cluster (little)
> -      - description: Alternate PLL clock for perf cluster (big)
> +      - description: XO source
>
>    clock-names:
>      items:
> -      - const: pwrcl_pll
> -      - const: perfcl_pll
> -      - const: pwrcl_alt_pll
> -      - const: perfcl_alt_pll
> +      - const: xo
>
>  required:
>    - compatible
>    - reg
>    - '#clock-cells'
> +  - clocks
> +  - clock-names

I think we can not list them as required, as then older DT files won't
pass schema validation. But I'll leave this into the hands of Rob and
Krzyshtof.

>
>  additionalProperties: false
>
> @@ -51,4 +47,7 @@ examples:
>          compatible = "qcom,msm8996-apcc";
>          reg = <0x6400000 0x90000>;
>          #clock-cells = <1>;
> +
> +        clocks = <&xo_board>;
> +        clock-names = "xo";
>      };
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 5/6] dt-bindings: clock: qcom,msm8996-apcc: Fix clocks

[Yassine Oudjana]

On Tue, Jun 21 2022 at 20:07:50 +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>
>> 
>>  The clocks currently listed in clocks and clock-names are the ones
>>  supplied by this clock controller, not the ones it consumes. Replace
>>  them with the only clock it consumes - the on-board oscillator (XO),
>>  and make the properties required.
>> 
>>  Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>>  ---
>>   .../bindings/clock/qcom,msm8996-apcc.yaml         | 15 
>> +++++++--------
>>   1 file changed, 7 insertions(+), 8 deletions(-)
>> 
>>  diff --git 
>> a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml 
>> b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>  index a20cb10636dd..c4971234fef8 100644
>>  --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>  +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>  @@ -26,22 +26,18 @@ properties:
>> 
>>     clocks:
>>       items:
>>  -      - description: Primary PLL clock for power cluster (little)
>>  -      - description: Primary PLL clock for perf cluster (big)
>>  -      - description: Alternate PLL clock for power cluster (little)
>>  -      - description: Alternate PLL clock for perf cluster (big)
>>  +      - description: XO source
>> 
>>     clock-names:
>>       items:
>>  -      - const: pwrcl_pll
>>  -      - const: perfcl_pll
>>  -      - const: pwrcl_alt_pll
>>  -      - const: perfcl_alt_pll
>>  +      - const: xo
>> 
>>   required:
>>     - compatible
>>     - reg
>>     - '#clock-cells'
>>  +  - clocks
>>  +  - clock-names
> 
> I think we can not list them as required, as then older DT files won't
> pass schema validation. But I'll leave this into the hands of Rob and
> Krzyshtof.

The old DT files that didn't have XO defined had a wrong
compatible string to begin with (fixed in [1]), so I don't
think it's a problem.

>>   additionalProperties: false
>> 
>>  @@ -51,4 +47,7 @@ examples:
>>           compatible = "qcom,msm8996-apcc";
>>           reg = <0x6400000 0x90000>;
>>           #clock-cells = <1>;
>>  +
>>  +        clocks = <&xo_board>;
>>  +        clock-names = "xo";
>>       };
>>  --
>>  2.36.1
>> 
> 
> 
> --
> With best wishes
> Dmitry

[1] 
https://lore.kernel.org/linux-arm-msm/20210527192958.775434-1-konrad.dybcio@somainline.org/

Re: [PATCH 5/6] dt-bindings: clock: qcom,msm8996-apcc: Fix clocks

[Dmitry Baryshkov]

On Tue, 21 Jun 2022 at 20:29, Yassine Oudjana <yassine.oudjana@gmail.com> wrote:
>
>
> On Tue, Jun 21 2022 at 20:07:50 +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>
> >>
> >>  The clocks currently listed in clocks and clock-names are the ones
> >>  supplied by this clock controller, not the ones it consumes. Replace
> >>  them with the only clock it consumes - the on-board oscillator (XO),
> >>  and make the properties required.
> >>
> >>  Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> >>  ---
> >>   .../bindings/clock/qcom,msm8996-apcc.yaml         | 15
> >> +++++++--------
> >>   1 file changed, 7 insertions(+), 8 deletions(-)
> >>
> >>  diff --git
> >> a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> >> b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> >>  index a20cb10636dd..c4971234fef8 100644
> >>  --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> >>  +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
> >>  @@ -26,22 +26,18 @@ properties:
> >>
> >>     clocks:
> >>       items:
> >>  -      - description: Primary PLL clock for power cluster (little)
> >>  -      - description: Primary PLL clock for perf cluster (big)
> >>  -      - description: Alternate PLL clock for power cluster (little)
> >>  -      - description: Alternate PLL clock for perf cluster (big)
> >>  +      - description: XO source
> >>
> >>     clock-names:
> >>       items:
> >>  -      - const: pwrcl_pll
> >>  -      - const: perfcl_pll
> >>  -      - const: pwrcl_alt_pll
> >>  -      - const: perfcl_alt_pll
> >>  +      - const: xo
> >>
> >>   required:
> >>     - compatible
> >>     - reg
> >>     - '#clock-cells'
> >>  +  - clocks
> >>  +  - clock-names
> >
> > I think we can not list them as required, as then older DT files won't
> > pass schema validation. But I'll leave this into the hands of Rob and
> > Krzyshtof.
>
> The old DT files that didn't have XO defined had a wrong
> compatible string to begin with (fixed in [1]), so I don't
> think it's a problem.

Looks fine to me then. (Though Rob and Krzysztof have the deciding voice here).

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> >>   additionalProperties: false
> >>
> >>  @@ -51,4 +47,7 @@ examples:
> >>           compatible = "qcom,msm8996-apcc";
> >>           reg = <0x6400000 0x90000>;
> >>           #clock-cells = <1>;
> >>  +
> >>  +        clocks = <&xo_board>;
> >>  +        clock-names = "xo";
> >>       };
> >>  --
> >>  2.36.1
> >>
> >
> >
> > --
> > With best wishes
> > Dmitry
>
> [1]
> https://lore.kernel.org/linux-arm-msm/20210527192958.775434-1-konrad.dybcio@somainline.org/
>
>


-- 
With best wishes
Dmitry

Re: [PATCH 5/6] dt-bindings: clock: qcom,msm8996-apcc: Fix clocks

[Krzysztof Kozlowski]

On 21/06/2022 19:28, Yassine Oudjana wrote:
> 
> On Tue, Jun 21 2022 at 20:07:50 +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>
>>>
>>>  The clocks currently listed in clocks and clock-names are the ones
>>>  supplied by this clock controller, not the ones it consumes. Replace
>>>  them with the only clock it consumes - the on-board oscillator (XO),
>>>  and make the properties required.
>>>
>>>  Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>>>  ---
>>>   .../bindings/clock/qcom,msm8996-apcc.yaml         | 15 
>>> +++++++--------
>>>   1 file changed, 7 insertions(+), 8 deletions(-)
>>>
>>>  diff --git 
>>> a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml 
>>> b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>>  index a20cb10636dd..c4971234fef8 100644
>>>  --- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>>  +++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
>>>  @@ -26,22 +26,18 @@ properties:
>>>
>>>     clocks:
>>>       items:
>>>  -      - description: Primary PLL clock for power cluster (little)
>>>  -      - description: Primary PLL clock for perf cluster (big)
>>>  -      - description: Alternate PLL clock for power cluster (little)
>>>  -      - description: Alternate PLL clock for perf cluster (big)
>>>  +      - description: XO source
>>>
>>>     clock-names:
>>>       items:
>>>  -      - const: pwrcl_pll
>>>  -      - const: perfcl_pll
>>>  -      - const: pwrcl_alt_pll
>>>  -      - const: perfcl_alt_pll
>>>  +      - const: xo
>>>
>>>   required:
>>>     - compatible
>>>     - reg
>>>     - '#clock-cells'
>>>  +  - clocks
>>>  +  - clock-names
>>
>> I think we can not list them as required, as then older DT files won't
>> pass schema validation. But I'll leave this into the hands of Rob and
>> Krzyshtof.
> 
> The old DT files that didn't have XO defined had a wrong
> compatible string to begin with (fixed in [1]), so I don't
> think it's a problem.
> 

Reasonable.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 6/6] clk: qcom: msm8996-cpu: Use parent_data for all clocks

[Yassine Oudjana]

From: Yassine Oudjana <y.oudjana@protonmail.com>

Replace parent_names in PLLs, secondary muxes and primary muxes with
parent_data. For primary muxes there were never any *cl_pll_acd clocks,
so instead of adding them, put the primary PLLs in both PLL_INDEX and
ACD_INDEX, then make sure ACD_INDEX is always picked over PLL_INDEX when
setting parent since we always want ACD when using the primary PLLs.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/clk/qcom/clk-cpu-8996.c | 79 ++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index b3ad9245874d..cdb7b2ef3367 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -112,14 +112,18 @@ static const struct alpha_pll_config hfpll_config = {
 	.early_output_mask = BIT(3),
 };
 
+static const struct clk_parent_data pll_parent[] = {
+	{ .fw_name = "xo" },
+};
+
 static struct clk_alpha_pll pwrcl_pll = {
 	.offset = PWRCL_REG_OFFSET,
 	.regs = prim_pll_regs,
 	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "pwrcl_pll",
-		.parent_names = (const char *[]){ "xo" },
-		.num_parents = 1,
+		.parent_data = pll_parent,
+		.num_parents = ARRAY_SIZE(pll_parent),
 		.ops = &clk_alpha_pll_huayra_ops,
 	},
 };
@@ -130,8 +134,8 @@ static struct clk_alpha_pll perfcl_pll = {
 	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "perfcl_pll",
-		.parent_names = (const char *[]){ "xo" },
-		.num_parents = 1,
+		.parent_data = pll_parent,
+		.num_parents = ARRAY_SIZE(pll_parent),
 		.ops = &clk_alpha_pll_huayra_ops,
 	},
 };
@@ -190,8 +194,8 @@ static struct clk_alpha_pll pwrcl_alt_pll = {
 	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "pwrcl_alt_pll",
-		.parent_names = (const char *[]){ "xo" },
-		.num_parents = 1,
+		.parent_data = pll_parent,
+		.num_parents = ARRAY_SIZE(pll_parent),
 		.ops = &clk_alpha_pll_hwfsm_ops,
 	},
 };
@@ -204,8 +208,8 @@ static struct clk_alpha_pll perfcl_alt_pll = {
 	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "perfcl_alt_pll",
-		.parent_names = (const char *[]){ "xo" },
-		.num_parents = 1,
+		.parent_data = pll_parent,
+		.num_parents = ARRAY_SIZE(pll_parent),
 		.ops = &clk_alpha_pll_hwfsm_ops,
 	},
 };
@@ -252,6 +256,9 @@ static int clk_cpu_8996_pmux_set_parent(struct clk_hw *hw, u8 index)
 	u32 val;
 
 	val = index;
+	/* We always want ACD when using the primary PLL */
+	if (val == PLL_INDEX)
+		val = ACD_INDEX;
 	val <<= cpuclk->shift;
 
 	return regmap_update_bits(clkr->regmap, cpuclk->reg, mask, val);
@@ -282,17 +289,24 @@ static const struct clk_ops clk_cpu_8996_pmux_ops = {
 	.determine_rate = clk_cpu_8996_pmux_determine_rate,
 };
 
+static const struct clk_parent_data pwrcl_smux_parents[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &pwrcl_pll_postdiv.hw },
+};
+
+static const struct clk_parent_data perfcl_smux_parents[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &perfcl_pll_postdiv.hw },
+};
+
 static struct clk_regmap_mux pwrcl_smux = {
 	.reg = PWRCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 2,
 	.width = 2,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "pwrcl_smux",
-		.parent_names = (const char *[]){
-			"xo",
-			"pwrcl_pll_postdiv",
-		},
-		.num_parents = 2,
+		.parent_data = pwrcl_smux_parents,
+		.num_parents = ARRAY_SIZE(pwrcl_smux_parents),
 		.ops = &clk_regmap_mux_closest_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	},
@@ -304,16 +318,27 @@ static struct clk_regmap_mux perfcl_smux = {
 	.width = 2,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "perfcl_smux",
-		.parent_names = (const char *[]){
-			"xo",
-			"perfcl_pll_postdiv",
-		},
-		.num_parents = 2,
+		.parent_data = perfcl_smux_parents,
+		.num_parents = ARRAY_SIZE(perfcl_smux_parents),
 		.ops = &clk_regmap_mux_closest_ops,
 		.flags = CLK_SET_RATE_PARENT,
 	},
 };
 
+static const struct clk_parent_data pwrcl_pmux_parents[] = {
+	[SMUX_INDEX] = { .hw = &pwrcl_smux.clkr.hw },
+	[PLL_INDEX] = { .hw = &pwrcl_pll.clkr.hw },
+	[ACD_INDEX] = { .hw = &pwrcl_pll.clkr.hw },
+	[ALT_INDEX] = { .hw = &pwrcl_alt_pll.clkr.hw },
+};
+
+static const struct clk_parent_data perfcl_pmux_parents[] = {
+	[SMUX_INDEX] = { .hw = &perfcl_smux.clkr.hw },
+	[PLL_INDEX] = { .hw = &perfcl_pll.clkr.hw },
+	[ACD_INDEX] = { .hw = &perfcl_pll.clkr.hw },
+	[ALT_INDEX] = { .hw = &perfcl_alt_pll.clkr.hw },
+};
+
 static struct clk_cpu_8996_pmux pwrcl_pmux = {
 	.reg = PWRCL_REG_OFFSET + MUX_OFFSET,
 	.shift = 0,
@@ -323,13 +348,8 @@ static struct clk_cpu_8996_pmux pwrcl_pmux = {
 	.nb.notifier_call = cpu_clk_notifier_cb,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "pwrcl_pmux",
-		.parent_names = (const char *[]){
-			"pwrcl_smux",
-			"pwrcl_pll",
-			"pwrcl_pll_acd",
-			"pwrcl_alt_pll",
-		},
-		.num_parents = 4,
+		.parent_data = pwrcl_pmux_parents,
+		.num_parents = ARRAY_SIZE(pwrcl_pmux_parents),
 		.ops = &clk_cpu_8996_pmux_ops,
 		/* CPU clock is critical and should never be gated */
 		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
@@ -345,13 +365,8 @@ static struct clk_cpu_8996_pmux perfcl_pmux = {
 	.nb.notifier_call = cpu_clk_notifier_cb,
 	.clkr.hw.init = &(struct clk_init_data) {
 		.name = "perfcl_pmux",
-		.parent_names = (const char *[]){
-			"perfcl_smux",
-			"perfcl_pll",
-			"perfcl_pll_acd",
-			"perfcl_alt_pll",
-		},
-		.num_parents = 4,
+		.parent_data = perfcl_pmux_parents,
+		.num_parents = ARRAY_SIZE(perfcl_pmux_parents),
 		.ops = &clk_cpu_8996_pmux_ops,
 		/* CPU clock is critical and should never be gated */
 		.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
-- 
2.36.1

Re: [PATCH 6/6] clk: qcom: msm8996-cpu: Use parent_data for all clocks

[Dmitry Baryshkov]

On Tue, 21 Jun 2022 at 19:07, Yassine Oudjana <yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> Replace parent_names in PLLs, secondary muxes and primary muxes with
> parent_data. For primary muxes there were never any *cl_pll_acd clocks,
> so instead of adding them, put the primary PLLs in both PLL_INDEX and
> ACD_INDEX, then make sure ACD_INDEX is always picked over PLL_INDEX when
> setting parent since we always want ACD when using the primary PLLs.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
>  drivers/clk/qcom/clk-cpu-8996.c | 79 ++++++++++++++++++++-------------
>  1 file changed, 47 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
> index b3ad9245874d..cdb7b2ef3367 100644
> --- a/drivers/clk/qcom/clk-cpu-8996.c
> +++ b/drivers/clk/qcom/clk-cpu-8996.c
> @@ -112,14 +112,18 @@ static const struct alpha_pll_config hfpll_config = {
>         .early_output_mask = BIT(3),
>  };
>
> +static const struct clk_parent_data pll_parent[] = {
> +       { .fw_name = "xo" },
> +};
> +
>  static struct clk_alpha_pll pwrcl_pll = {
>         .offset = PWRCL_REG_OFFSET,
>         .regs = prim_pll_regs,
>         .flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,
>         .clkr.hw.init = &(struct clk_init_data){
>                 .name = "pwrcl_pll",
> -               .parent_names = (const char *[]){ "xo" },
> -               .num_parents = 1,
> +               .parent_data = pll_parent,
> +               .num_parents = ARRAY_SIZE(pll_parent),
>                 .ops = &clk_alpha_pll_huayra_ops,
>         },
>  };
> @@ -130,8 +134,8 @@ static struct clk_alpha_pll perfcl_pll = {
>         .flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,
>         .clkr.hw.init = &(struct clk_init_data){
>                 .name = "perfcl_pll",
> -               .parent_names = (const char *[]){ "xo" },
> -               .num_parents = 1,
> +               .parent_data = pll_parent,
> +               .num_parents = ARRAY_SIZE(pll_parent),
>                 .ops = &clk_alpha_pll_huayra_ops,
>         },
>  };
> @@ -190,8 +194,8 @@ static struct clk_alpha_pll pwrcl_alt_pll = {
>         .flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
>         .clkr.hw.init = &(struct clk_init_data) {
>                 .name = "pwrcl_alt_pll",
> -               .parent_names = (const char *[]){ "xo" },
> -               .num_parents = 1,
> +               .parent_data = pll_parent,
> +               .num_parents = ARRAY_SIZE(pll_parent),
>                 .ops = &clk_alpha_pll_hwfsm_ops,
>         },
>  };
> @@ -204,8 +208,8 @@ static struct clk_alpha_pll perfcl_alt_pll = {
>         .flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
>         .clkr.hw.init = &(struct clk_init_data) {
>                 .name = "perfcl_alt_pll",
> -               .parent_names = (const char *[]){ "xo" },
> -               .num_parents = 1,
> +               .parent_data = pll_parent,
> +               .num_parents = ARRAY_SIZE(pll_parent),
>                 .ops = &clk_alpha_pll_hwfsm_ops,
>         },
>  };
> @@ -252,6 +256,9 @@ static int clk_cpu_8996_pmux_set_parent(struct clk_hw *hw, u8 index)
>         u32 val;
>
>         val = index;
> +       /* We always want ACD when using the primary PLL */
> +       if (val == PLL_INDEX)
> +               val = ACD_INDEX;
>         val <<= cpuclk->shift;
>
>         return regmap_update_bits(clkr->regmap, cpuclk->reg, mask, val);
> @@ -282,17 +289,24 @@ static const struct clk_ops clk_cpu_8996_pmux_ops = {
>         .determine_rate = clk_cpu_8996_pmux_determine_rate,
>  };
>
> +static const struct clk_parent_data pwrcl_smux_parents[] = {
> +       { .fw_name = "xo" },
> +       { .hw = &pwrcl_pll_postdiv.hw },
> +};
> +
> +static const struct clk_parent_data perfcl_smux_parents[] = {
> +       { .fw_name = "xo" },
> +       { .hw = &perfcl_pll_postdiv.hw },
> +};
> +
>  static struct clk_regmap_mux pwrcl_smux = {
>         .reg = PWRCL_REG_OFFSET + MUX_OFFSET,
>         .shift = 2,
>         .width = 2,
>         .clkr.hw.init = &(struct clk_init_data) {
>                 .name = "pwrcl_smux",
> -               .parent_names = (const char *[]){
> -                       "xo",
> -                       "pwrcl_pll_postdiv",
> -               },
> -               .num_parents = 2,
> +               .parent_data = pwrcl_smux_parents,
> +               .num_parents = ARRAY_SIZE(pwrcl_smux_parents),
>                 .ops = &clk_regmap_mux_closest_ops,
>                 .flags = CLK_SET_RATE_PARENT,
>         },
> @@ -304,16 +318,27 @@ static struct clk_regmap_mux perfcl_smux = {
>         .width = 2,
>         .clkr.hw.init = &(struct clk_init_data) {
>                 .name = "perfcl_smux",
> -               .parent_names = (const char *[]){
> -                       "xo",
> -                       "perfcl_pll_postdiv",
> -               },
> -               .num_parents = 2,
> +               .parent_data = perfcl_smux_parents,
> +               .num_parents = ARRAY_SIZE(perfcl_smux_parents),
>                 .ops = &clk_regmap_mux_closest_ops,
>                 .flags = CLK_SET_RATE_PARENT,
>         },
>  };
>
> +static const struct clk_parent_data pwrcl_pmux_parents[] = {
> +       [SMUX_INDEX] = { .hw = &pwrcl_smux.clkr.hw },
> +       [PLL_INDEX] = { .hw = &pwrcl_pll.clkr.hw },
> +       [ACD_INDEX] = { .hw = &pwrcl_pll.clkr.hw },
> +       [ALT_INDEX] = { .hw = &pwrcl_alt_pll.clkr.hw },
> +};
> +
> +static const struct clk_parent_data perfcl_pmux_parents[] = {
> +       [SMUX_INDEX] = { .hw = &perfcl_smux.clkr.hw },
> +       [PLL_INDEX] = { .hw = &perfcl_pll.clkr.hw },
> +       [ACD_INDEX] = { .hw = &perfcl_pll.clkr.hw },
> +       [ALT_INDEX] = { .hw = &perfcl_alt_pll.clkr.hw },
> +};
> +
>  static struct clk_cpu_8996_pmux pwrcl_pmux = {
>         .reg = PWRCL_REG_OFFSET + MUX_OFFSET,
>         .shift = 0,
> @@ -323,13 +348,8 @@ static struct clk_cpu_8996_pmux pwrcl_pmux = {
>         .nb.notifier_call = cpu_clk_notifier_cb,
>         .clkr.hw.init = &(struct clk_init_data) {
>                 .name = "pwrcl_pmux",
> -               .parent_names = (const char *[]){
> -                       "pwrcl_smux",
> -                       "pwrcl_pll",
> -                       "pwrcl_pll_acd",
> -                       "pwrcl_alt_pll",
> -               },
> -               .num_parents = 4,
> +               .parent_data = pwrcl_pmux_parents,

Please use parent_hws here and below.

> +               .num_parents = ARRAY_SIZE(pwrcl_pmux_parents),
>                 .ops = &clk_cpu_8996_pmux_ops,
>                 /* CPU clock is critical and should never be gated */
>                 .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> @@ -345,13 +365,8 @@ static struct clk_cpu_8996_pmux perfcl_pmux = {
>         .nb.notifier_call = cpu_clk_notifier_cb,
>         .clkr.hw.init = &(struct clk_init_data) {
>                 .name = "perfcl_pmux",
> -               .parent_names = (const char *[]){
> -                       "perfcl_smux",
> -                       "perfcl_pll",
> -                       "perfcl_pll_acd",
> -                       "perfcl_alt_pll",
> -               },
> -               .num_parents = 4,
> +               .parent_data = perfcl_pmux_parents,
> +               .num_parents = ARRAY_SIZE(perfcl_pmux_parents),
>                 .ops = &clk_cpu_8996_pmux_ops,
>                 /* CPU clock is critical and should never be gated */
>                 .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> --
> 2.36.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 0/6] clk: qcom: msm8996-cpu: Cleanup and migrate to parent_data

[Bjorn Andersson]

On Tue 21 Jun 11:06 CDT 2022, Yassine Oudjana wrote:

> This series includes some cleanup of the MSM8996 CPU clock driver, as well as
> migration from parent_names to parent_data for all of its clocks. The DT schema
> is also fixed in this series to show the actual clocks consumed by the clock
> controller and pass checks.

This series looks almost ready to be merged, could you (or Dmitry?)
update the two outstanding items?

Thanks,
Bjorn

> 
> Yassine Oudjana (6):
>   clk: qcom: msm8996-cpu: Rename DIV_2_INDEX to SMUX_INDEX
>   clk: qcom: msm8996-cpu: Statically define PLL dividers
>   clk: qcom: msm8996-cpu: Unify cluster order
>   clk: qcom: msm8996-cpu: Convert secondary muxes to clk_regmap_mux
>   dt-bindings: clock: qcom,msm8996-apcc: Fix clocks
>   clk: qcom: msm8996-cpu: Use parent_data for all clocks
> 
>  .../bindings/clock/qcom,msm8996-apcc.yaml     |  15 +-
>  drivers/clk/qcom/clk-cpu-8996.c               | 235 ++++++++++--------
>  2 files changed, 140 insertions(+), 110 deletions(-)
> 
> -- 
> 2.36.1
> 

Re: [PATCH 0/6] clk: qcom: msm8996-cpu: Cleanup and migrate to parent_data

[Dmitry Baryshkov]

On 14/07/2022 00:32, Bjorn Andersson wrote:
> On Tue 21 Jun 11:06 CDT 2022, Yassine Oudjana wrote:
> 
>> This series includes some cleanup of the MSM8996 CPU clock driver, as well as
>> migration from parent_names to parent_data for all of its clocks. The DT schema
>> is also fixed in this series to show the actual clocks consumed by the clock
>> controller and pass checks.
> 
> This series looks almost ready to be merged, could you (or Dmitry?)
> update the two outstanding items?

I have acked the patch 2 and sent the slightly updated revision of 
patch6 (together with the rest of small changes).

> 
> Thanks,
> Bjorn
> 
>>
>> Yassine Oudjana (6):
>>    clk: qcom: msm8996-cpu: Rename DIV_2_INDEX to SMUX_INDEX
>>    clk: qcom: msm8996-cpu: Statically define PLL dividers
>>    clk: qcom: msm8996-cpu: Unify cluster order
>>    clk: qcom: msm8996-cpu: Convert secondary muxes to clk_regmap_mux
>>    dt-bindings: clock: qcom,msm8996-apcc: Fix clocks
>>    clk: qcom: msm8996-cpu: Use parent_data for all clocks
>>
>>   .../bindings/clock/qcom,msm8996-apcc.yaml     |  15 +-
>>   drivers/clk/qcom/clk-cpu-8996.c               | 235 ++++++++++--------
>>   2 files changed, 140 insertions(+), 110 deletions(-)
>>
>> -- 
>> 2.36.1
>>


-- 
With best wishes
Dmitry

Re: [PATCH 0/6] clk: qcom: msm8996-cpu: Cleanup and migrate to parent_data

[Dmitry Baryshkov]

On 14/07/2022 13:06, Dmitry Baryshkov wrote:
> On 14/07/2022 00:32, Bjorn Andersson wrote:
>> On Tue 21 Jun 11:06 CDT 2022, Yassine Oudjana wrote:
>>
>>> This series includes some cleanup of the MSM8996 CPU clock driver, as 
>>> well as
>>> migration from parent_names to parent_data for all of its clocks. The 
>>> DT schema
>>> is also fixed in this series to show the actual clocks consumed by 
>>> the clock
>>> controller and pass checks.
>>
>> This series looks almost ready to be merged, could you (or Dmitry?)
>> update the two outstanding items?
> 
> I have acked the patch 2 and sent the slightly updated revision of 
> patch6 (together with the rest of small changes).

Bjorn, could you please pick up patches 1-5?

> 
>>
>> Thanks,
>> Bjorn
>>
>>>
>>> Yassine Oudjana (6):
>>>    clk: qcom: msm8996-cpu: Rename DIV_2_INDEX to SMUX_INDEX
>>>    clk: qcom: msm8996-cpu: Statically define PLL dividers
>>>    clk: qcom: msm8996-cpu: Unify cluster order
>>>    clk: qcom: msm8996-cpu: Convert secondary muxes to clk_regmap_mux
>>>    dt-bindings: clock: qcom,msm8996-apcc: Fix clocks
>>>    clk: qcom: msm8996-cpu: Use parent_data for all clocks
>>>
>>>   .../bindings/clock/qcom,msm8996-apcc.yaml     |  15 +-
>>>   drivers/clk/qcom/clk-cpu-8996.c               | 235 ++++++++++--------
>>>   2 files changed, 140 insertions(+), 110 deletions(-)
>>>
>>> -- 
>>> 2.36.1
>>>
> 
> 

-- 
With best wishes
Dmitry

Re: (subset) [PATCH 0/6] clk: qcom: msm8996-cpu: Cleanup and migrate to parent_data

[Bjorn Andersson]

On Tue, 21 Jun 2022 20:06:15 +0400, Yassine Oudjana wrote:
> This series includes some cleanup of the MSM8996 CPU clock driver, as well as
> migration from parent_names to parent_data for all of its clocks. The DT schema
> is also fixed in this series to show the actual clocks consumed by the clock
> controller and pass checks.
> 
> Yassine Oudjana (6):
>   clk: qcom: msm8996-cpu: Rename DIV_2_INDEX to SMUX_INDEX
>   clk: qcom: msm8996-cpu: Statically define PLL dividers
>   clk: qcom: msm8996-cpu: Unify cluster order
>   clk: qcom: msm8996-cpu: Convert secondary muxes to clk_regmap_mux
>   dt-bindings: clock: qcom,msm8996-apcc: Fix clocks
>   clk: qcom: msm8996-cpu: Use parent_data for all clocks
> 
> [...]

Applied, thanks!

[1/6] clk: qcom: msm8996-cpu: Rename DIV_2_INDEX to SMUX_INDEX
      commit: 1ba0a3bbd5ed5a1bb8d0165912d9904b812af74b
[2/6] clk: qcom: msm8996-cpu: Statically define PLL dividers
      commit: de37e0214c28330cf0dbf4fe51db1d9d38c13c93
[3/6] clk: qcom: msm8996-cpu: Unify cluster order
      commit: 382139bfd68fe6cc9dc94ffe3b9d783b85be3b1c
[4/6] clk: qcom: msm8996-cpu: Convert secondary muxes to clk_regmap_mux
      commit: 9a9f5f9a5a0ca3f463eb28ba5920a6fd18dc9956
[5/6] dt-bindings: clock: qcom,msm8996-apcc: Fix clocks
      commit: b4feed4a3d0a6b8cef4a574a9df707c556928ec2

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>