[PATCH v5 1/3] dt-bindings: clock: add pcm reset for ipq806x lcc

[PATCH v5 1/3] dt-bindings: clock: add pcm reset for ipq806x lcc

[Christian Marangi]

Add pcm reset define for ipq806x lcc.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
v3:
 - Added review tag
 - Added ack tag
v2:
 - Fix Sob tag

 include/dt-bindings/clock/qcom,lcc-ipq806x.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,lcc-ipq806x.h b/include/dt-bindings/clock/qcom,lcc-ipq806x.h
index 25b92bbf0ab4..e0fb4acf4ba8 100644
--- a/include/dt-bindings/clock/qcom,lcc-ipq806x.h
+++ b/include/dt-bindings/clock/qcom,lcc-ipq806x.h
@@ -19,4 +19,6 @@
 #define SPDIF_CLK			10
 #define AHBIX_CLK			11
 
+#define LCC_PCM_RESET			0
+
 #endif
-- 
2.36.1

[PATCH v5 2/3] clk: qcom: lcc-ipq806x: add reset definition

[Christian Marangi]

Add reset definition for lcc-ipq806x.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
v3:
 - Added review tag
v2:
 - Fix Sob tag

 drivers/clk/qcom/lcc-ipq806x.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/qcom/lcc-ipq806x.c b/drivers/clk/qcom/lcc-ipq806x.c
index 1a2be4aeb31d..ba90bebba597 100644
--- a/drivers/clk/qcom/lcc-ipq806x.c
+++ b/drivers/clk/qcom/lcc-ipq806x.c
@@ -22,6 +22,7 @@
 #include "clk-branch.h"
 #include "clk-regmap-divider.h"
 #include "clk-regmap-mux.h"
+#include "reset.h"
 
 static struct clk_pll pll4 = {
 	.l_reg = 0x4,
@@ -405,6 +406,10 @@ static struct clk_regmap *lcc_ipq806x_clks[] = {
 	[AHBIX_CLK] = &ahbix_clk.clkr,
 };
 
+static const struct qcom_reset_map lcc_ipq806x_resets[] = {
+	[LCC_PCM_RESET] = { 0x54, 13 },
+};
+
 static const struct regmap_config lcc_ipq806x_regmap_config = {
 	.reg_bits	= 32,
 	.reg_stride	= 4,
@@ -417,6 +422,8 @@ static const struct qcom_cc_desc lcc_ipq806x_desc = {
 	.config = &lcc_ipq806x_regmap_config,
 	.clks = lcc_ipq806x_clks,
 	.num_clks = ARRAY_SIZE(lcc_ipq806x_clks),
+	.resets = lcc_ipq806x_resets,
+	.num_resets = ARRAY_SIZE(lcc_ipq806x_resets),
 };
 
 static const struct of_device_id lcc_ipq806x_match_table[] = {
-- 
2.36.1

[PATCH v5 3/3] clk: qcom: lcc-ipq806x: convert to parent data

[Christian Marangi]

Convert lcc-ipq806x driver to parent_data API.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
v5:
- Fix the same compilation error (don't know what the hell happen
  to my buildroot)
v4:
- Fix compilation error
v3:
 - Inline pxo pll4 parent
 - Change .name from pxo to pxo_board

 drivers/clk/qcom/lcc-ipq806x.c | 77 ++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/qcom/lcc-ipq806x.c b/drivers/clk/qcom/lcc-ipq806x.c
index ba90bebba597..72d6aea5be30 100644
--- a/drivers/clk/qcom/lcc-ipq806x.c
+++ b/drivers/clk/qcom/lcc-ipq806x.c
@@ -34,7 +34,9 @@ static struct clk_pll pll4 = {
 	.status_bit = 16,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "pll4",
-		.parent_names = (const char *[]){ "pxo" },
+		.parent_data = &(const struct clk_parent_data) {
+			.fw_name = "pxo", .name = "pxo_board",
+		},
 		.num_parents = 1,
 		.ops = &clk_pll_ops,
 	},
@@ -64,9 +66,9 @@ static const struct parent_map lcc_pxo_pll4_map[] = {
 	{ P_PLL4, 2 }
 };
 
-static const char * const lcc_pxo_pll4[] = {
-	"pxo",
-	"pll4_vote",
+static const struct clk_parent_data lcc_pxo_pll4[] = {
+	{ .fw_name = "pxo", .name = "pxo" },
+	{ .fw_name = "pll4_vote", .name = "pll4_vote" },
 };
 
 static struct freq_tbl clk_tbl_aif_mi2s[] = {
@@ -131,18 +133,14 @@ static struct clk_rcg mi2s_osr_src = {
 		.enable_mask = BIT(9),
 		.hw.init = &(struct clk_init_data){
 			.name = "mi2s_osr_src",
-			.parent_names = lcc_pxo_pll4,
-			.num_parents = 2,
+			.parent_data = lcc_pxo_pll4,
+			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
 			.ops = &clk_rcg_ops,
 			.flags = CLK_SET_RATE_GATE,
 		},
 	},
 };
 
-static const char * const lcc_mi2s_parents[] = {
-	"mi2s_osr_src",
-};
-
 static struct clk_branch mi2s_osr_clk = {
 	.halt_reg = 0x50,
 	.halt_bit = 1,
@@ -152,7 +150,9 @@ static struct clk_branch mi2s_osr_clk = {
 		.enable_mask = BIT(17),
 		.hw.init = &(struct clk_init_data){
 			.name = "mi2s_osr_clk",
-			.parent_names = lcc_mi2s_parents,
+			.parent_hws = (const struct clk_hw*[]){
+				&mi2s_osr_src.clkr.hw,
+			},
 			.num_parents = 1,
 			.ops = &clk_branch_ops,
 			.flags = CLK_SET_RATE_PARENT,
@@ -167,7 +167,9 @@ static struct clk_regmap_div mi2s_div_clk = {
 	.clkr = {
 		.hw.init = &(struct clk_init_data){
 			.name = "mi2s_div_clk",
-			.parent_names = lcc_mi2s_parents,
+			.parent_hws = (const struct clk_hw*[]){
+				&mi2s_osr_src.clkr.hw,
+			},
 			.num_parents = 1,
 			.ops = &clk_regmap_div_ops,
 		},
@@ -183,7 +185,9 @@ static struct clk_branch mi2s_bit_div_clk = {
 		.enable_mask = BIT(15),
 		.hw.init = &(struct clk_init_data){
 			.name = "mi2s_bit_div_clk",
-			.parent_names = (const char *[]){ "mi2s_div_clk" },
+			.parent_hws = (const struct clk_hw*[]){
+				&mi2s_div_clk.clkr.hw,
+			},
 			.num_parents = 1,
 			.ops = &clk_branch_ops,
 			.flags = CLK_SET_RATE_PARENT,
@@ -191,6 +195,10 @@ static struct clk_branch mi2s_bit_div_clk = {
 	},
 };
 
+static const struct clk_parent_data lcc_mi2s_bit_div_codec_clk[] = {
+	{ .hw = &mi2s_bit_div_clk.clkr.hw, },
+	{ .fw_name = "mi2s_codec_clk", .name = "mi2s_codec_clk" },
+};
 
 static struct clk_regmap_mux mi2s_bit_clk = {
 	.reg = 0x48,
@@ -199,11 +207,8 @@ static struct clk_regmap_mux mi2s_bit_clk = {
 	.clkr = {
 		.hw.init = &(struct clk_init_data){
 			.name = "mi2s_bit_clk",
-			.parent_names = (const char *[]){
-				"mi2s_bit_div_clk",
-				"mi2s_codec_clk",
-			},
-			.num_parents = 2,
+			.parent_data = lcc_mi2s_bit_div_codec_clk,
+			.num_parents = ARRAY_SIZE(lcc_mi2s_bit_div_codec_clk),
 			.ops = &clk_regmap_mux_closest_ops,
 			.flags = CLK_SET_RATE_PARENT,
 		},
@@ -245,8 +250,8 @@ static struct clk_rcg pcm_src = {
 		.enable_mask = BIT(9),
 		.hw.init = &(struct clk_init_data){
 			.name = "pcm_src",
-			.parent_names = lcc_pxo_pll4,
-			.num_parents = 2,
+			.parent_data = lcc_pxo_pll4,
+			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
 			.ops = &clk_rcg_ops,
 			.flags = CLK_SET_RATE_GATE,
 		},
@@ -262,7 +267,9 @@ static struct clk_branch pcm_clk_out = {
 		.enable_mask = BIT(11),
 		.hw.init = &(struct clk_init_data){
 			.name = "pcm_clk_out",
-			.parent_names = (const char *[]){ "pcm_src" },
+			.parent_hws = (const struct clk_hw*[]){
+				&pcm_src.clkr.hw,
+			},
 			.num_parents = 1,
 			.ops = &clk_branch_ops,
 			.flags = CLK_SET_RATE_PARENT,
@@ -270,6 +277,11 @@ static struct clk_branch pcm_clk_out = {
 	},
 };
 
+static const struct clk_parent_data lcc_pcm_clk_out_codec_clk[] = {
+	{ .hw = &pcm_clk_out.clkr.hw, },
+	{ .fw_name = "pcm_codec_clk", .name = "pcm_codec_clk" },
+};
+
 static struct clk_regmap_mux pcm_clk = {
 	.reg = 0x54,
 	.shift = 10,
@@ -277,11 +289,8 @@ static struct clk_regmap_mux pcm_clk = {
 	.clkr = {
 		.hw.init = &(struct clk_init_data){
 			.name = "pcm_clk",
-			.parent_names = (const char *[]){
-				"pcm_clk_out",
-				"pcm_codec_clk",
-			},
-			.num_parents = 2,
+			.parent_data = lcc_pcm_clk_out_codec_clk,
+			.num_parents = ARRAY_SIZE(lcc_pcm_clk_out_codec_clk),
 			.ops = &clk_regmap_mux_closest_ops,
 			.flags = CLK_SET_RATE_PARENT,
 		},
@@ -325,18 +334,14 @@ static struct clk_rcg spdif_src = {
 		.enable_mask = BIT(9),
 		.hw.init = &(struct clk_init_data){
 			.name = "spdif_src",
-			.parent_names = lcc_pxo_pll4,
-			.num_parents = 2,
+			.parent_data = lcc_pxo_pll4,
+			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
 			.ops = &clk_rcg_ops,
 			.flags = CLK_SET_RATE_GATE,
 		},
 	},
 };
 
-static const char * const lcc_spdif_parents[] = {
-	"spdif_src",
-};
-
 static struct clk_branch spdif_clk = {
 	.halt_reg = 0xd4,
 	.halt_bit = 1,
@@ -346,7 +351,9 @@ static struct clk_branch spdif_clk = {
 		.enable_mask = BIT(12),
 		.hw.init = &(struct clk_init_data){
 			.name = "spdif_clk",
-			.parent_names = lcc_spdif_parents,
+			.parent_hws = (const struct clk_hw*[]){
+				&spdif_src.clkr.hw,
+			},
 			.num_parents = 1,
 			.ops = &clk_branch_ops,
 			.flags = CLK_SET_RATE_PARENT,
@@ -384,8 +391,8 @@ static struct clk_rcg ahbix_clk = {
 		.enable_mask = BIT(11),
 		.hw.init = &(struct clk_init_data){
 			.name = "ahbix",
-			.parent_names = lcc_pxo_pll4,
-			.num_parents = 2,
+			.parent_data = lcc_pxo_pll4,
+			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
 			.ops = &clk_rcg_lcc_ops,
 		},
 	},
-- 
2.36.1

Re: [PATCH v5 3/3] clk: qcom: lcc-ipq806x: convert to parent data

[Bjorn Andersson]

On Thu 07 Jul 19:03 CDT 2022, Christian Marangi wrote:

> Convert lcc-ipq806x driver to parent_data API.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> v5:
> - Fix the same compilation error (don't know what the hell happen
>   to my buildroot)
> v4:
> - Fix compilation error
> v3:
>  - Inline pxo pll4 parent
>  - Change .name from pxo to pxo_board
> 
>  drivers/clk/qcom/lcc-ipq806x.c | 77 ++++++++++++++++++----------------
>  1 file changed, 42 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/clk/qcom/lcc-ipq806x.c b/drivers/clk/qcom/lcc-ipq806x.c
> index ba90bebba597..72d6aea5be30 100644
> --- a/drivers/clk/qcom/lcc-ipq806x.c
> +++ b/drivers/clk/qcom/lcc-ipq806x.c
> @@ -34,7 +34,9 @@ static struct clk_pll pll4 = {
>  	.status_bit = 16,
>  	.clkr.hw.init = &(struct clk_init_data){
>  		.name = "pll4",
> -		.parent_names = (const char *[]){ "pxo" },
> +		.parent_data = &(const struct clk_parent_data) {
> +			.fw_name = "pxo", .name = "pxo_board",

This changes the behavior from looking for the globally named "pxo" to
look for the globally named "pxo_board", in the event that no
clock-names of "pxo" was found (based on the .fw_name).

So you probably want to keep this as .fw_name = "pxo", .name = "pxo".

> +		},
>  		.num_parents = 1,
>  		.ops = &clk_pll_ops,
>  	},
> @@ -64,9 +66,9 @@ static const struct parent_map lcc_pxo_pll4_map[] = {
>  	{ P_PLL4, 2 }
>  };
>  
> -static const char * const lcc_pxo_pll4[] = {
> -	"pxo",
> -	"pll4_vote",
> +static const struct clk_parent_data lcc_pxo_pll4[] = {
> +	{ .fw_name = "pxo", .name = "pxo" },
> +	{ .fw_name = "pll4_vote", .name = "pll4_vote" },

This is a reference to a clock defined in this same driver, so you can
use { .hw = &pll4_vote.clkr.hw } to avoid the lookup all together.

>  };
>  
>  static struct freq_tbl clk_tbl_aif_mi2s[] = {
> @@ -131,18 +133,14 @@ static struct clk_rcg mi2s_osr_src = {
>  		.enable_mask = BIT(9),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "mi2s_osr_src",
> -			.parent_names = lcc_pxo_pll4,
> -			.num_parents = 2,
> +			.parent_data = lcc_pxo_pll4,
> +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
>  			.ops = &clk_rcg_ops,
>  			.flags = CLK_SET_RATE_GATE,
>  		},
>  	},
>  };
>  
> -static const char * const lcc_mi2s_parents[] = {
> -	"mi2s_osr_src",
> -};
> -
>  static struct clk_branch mi2s_osr_clk = {
>  	.halt_reg = 0x50,
>  	.halt_bit = 1,
> @@ -152,7 +150,9 @@ static struct clk_branch mi2s_osr_clk = {
>  		.enable_mask = BIT(17),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "mi2s_osr_clk",
> -			.parent_names = lcc_mi2s_parents,
> +			.parent_hws = (const struct clk_hw*[]){
> +				&mi2s_osr_src.clkr.hw,
> +			},
>  			.num_parents = 1,
>  			.ops = &clk_branch_ops,
>  			.flags = CLK_SET_RATE_PARENT,
> @@ -167,7 +167,9 @@ static struct clk_regmap_div mi2s_div_clk = {
>  	.clkr = {
>  		.hw.init = &(struct clk_init_data){
>  			.name = "mi2s_div_clk",
> -			.parent_names = lcc_mi2s_parents,
> +			.parent_hws = (const struct clk_hw*[]){

It would be wonderful if you could keep a space between ) and { in
these.

> +				&mi2s_osr_src.clkr.hw,
> +			},
>  			.num_parents = 1,
>  			.ops = &clk_regmap_div_ops,
>  		},
> @@ -183,7 +185,9 @@ static struct clk_branch mi2s_bit_div_clk = {
>  		.enable_mask = BIT(15),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "mi2s_bit_div_clk",
> -			.parent_names = (const char *[]){ "mi2s_div_clk" },
> +			.parent_hws = (const struct clk_hw*[]){
> +				&mi2s_div_clk.clkr.hw,
> +			},
>  			.num_parents = 1,
>  			.ops = &clk_branch_ops,
>  			.flags = CLK_SET_RATE_PARENT,
> @@ -191,6 +195,10 @@ static struct clk_branch mi2s_bit_div_clk = {
>  	},
>  };
>  
> +static const struct clk_parent_data lcc_mi2s_bit_div_codec_clk[] = {
> +	{ .hw = &mi2s_bit_div_clk.clkr.hw, },
> +	{ .fw_name = "mi2s_codec_clk", .name = "mi2s_codec_clk" },

Is mi2s_codec_clk and external clock? I don't see it documented in the
DT binding. And if we're introducing new clock-names, perhaps we could
skip the _clk suffix - because obviously it's a clock :)

Regards,
Bjorn

> +};
>  
>  static struct clk_regmap_mux mi2s_bit_clk = {
>  	.reg = 0x48,
> @@ -199,11 +207,8 @@ static struct clk_regmap_mux mi2s_bit_clk = {
>  	.clkr = {
>  		.hw.init = &(struct clk_init_data){
>  			.name = "mi2s_bit_clk",
> -			.parent_names = (const char *[]){
> -				"mi2s_bit_div_clk",
> -				"mi2s_codec_clk",
> -			},
> -			.num_parents = 2,
> +			.parent_data = lcc_mi2s_bit_div_codec_clk,
> +			.num_parents = ARRAY_SIZE(lcc_mi2s_bit_div_codec_clk),
>  			.ops = &clk_regmap_mux_closest_ops,
>  			.flags = CLK_SET_RATE_PARENT,
>  		},
> @@ -245,8 +250,8 @@ static struct clk_rcg pcm_src = {
>  		.enable_mask = BIT(9),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "pcm_src",
> -			.parent_names = lcc_pxo_pll4,
> -			.num_parents = 2,
> +			.parent_data = lcc_pxo_pll4,
> +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
>  			.ops = &clk_rcg_ops,
>  			.flags = CLK_SET_RATE_GATE,
>  		},
> @@ -262,7 +267,9 @@ static struct clk_branch pcm_clk_out = {
>  		.enable_mask = BIT(11),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "pcm_clk_out",
> -			.parent_names = (const char *[]){ "pcm_src" },
> +			.parent_hws = (const struct clk_hw*[]){
> +				&pcm_src.clkr.hw,
> +			},
>  			.num_parents = 1,
>  			.ops = &clk_branch_ops,
>  			.flags = CLK_SET_RATE_PARENT,
> @@ -270,6 +277,11 @@ static struct clk_branch pcm_clk_out = {
>  	},
>  };
>  
> +static const struct clk_parent_data lcc_pcm_clk_out_codec_clk[] = {
> +	{ .hw = &pcm_clk_out.clkr.hw, },
> +	{ .fw_name = "pcm_codec_clk", .name = "pcm_codec_clk" },
> +};
> +
>  static struct clk_regmap_mux pcm_clk = {
>  	.reg = 0x54,
>  	.shift = 10,
> @@ -277,11 +289,8 @@ static struct clk_regmap_mux pcm_clk = {
>  	.clkr = {
>  		.hw.init = &(struct clk_init_data){
>  			.name = "pcm_clk",
> -			.parent_names = (const char *[]){
> -				"pcm_clk_out",
> -				"pcm_codec_clk",
> -			},
> -			.num_parents = 2,
> +			.parent_data = lcc_pcm_clk_out_codec_clk,
> +			.num_parents = ARRAY_SIZE(lcc_pcm_clk_out_codec_clk),
>  			.ops = &clk_regmap_mux_closest_ops,
>  			.flags = CLK_SET_RATE_PARENT,
>  		},
> @@ -325,18 +334,14 @@ static struct clk_rcg spdif_src = {
>  		.enable_mask = BIT(9),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "spdif_src",
> -			.parent_names = lcc_pxo_pll4,
> -			.num_parents = 2,
> +			.parent_data = lcc_pxo_pll4,
> +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
>  			.ops = &clk_rcg_ops,
>  			.flags = CLK_SET_RATE_GATE,
>  		},
>  	},
>  };
>  
> -static const char * const lcc_spdif_parents[] = {
> -	"spdif_src",
> -};
> -
>  static struct clk_branch spdif_clk = {
>  	.halt_reg = 0xd4,
>  	.halt_bit = 1,
> @@ -346,7 +351,9 @@ static struct clk_branch spdif_clk = {
>  		.enable_mask = BIT(12),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "spdif_clk",
> -			.parent_names = lcc_spdif_parents,
> +			.parent_hws = (const struct clk_hw*[]){
> +				&spdif_src.clkr.hw,
> +			},
>  			.num_parents = 1,
>  			.ops = &clk_branch_ops,
>  			.flags = CLK_SET_RATE_PARENT,
> @@ -384,8 +391,8 @@ static struct clk_rcg ahbix_clk = {
>  		.enable_mask = BIT(11),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "ahbix",
> -			.parent_names = lcc_pxo_pll4,
> -			.num_parents = 2,
> +			.parent_data = lcc_pxo_pll4,
> +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
>  			.ops = &clk_rcg_lcc_ops,
>  		},
>  	},
> -- 
> 2.36.1
> 

Re: [PATCH v5 3/3] clk: qcom: lcc-ipq806x: convert to parent data

[Christian Marangi]

On Mon, Jul 18, 2022 at 11:42:29PM -0500, Bjorn Andersson wrote:
> On Thu 07 Jul 19:03 CDT 2022, Christian Marangi wrote:
> 
> > Convert lcc-ipq806x driver to parent_data API.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> > v5:
> > - Fix the same compilation error (don't know what the hell happen
> >   to my buildroot)
> > v4:
> > - Fix compilation error
> > v3:
> >  - Inline pxo pll4 parent
> >  - Change .name from pxo to pxo_board
> > 
> >  drivers/clk/qcom/lcc-ipq806x.c | 77 ++++++++++++++++++----------------
> >  1 file changed, 42 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/clk/qcom/lcc-ipq806x.c b/drivers/clk/qcom/lcc-ipq806x.c
> > index ba90bebba597..72d6aea5be30 100644
> > --- a/drivers/clk/qcom/lcc-ipq806x.c
> > +++ b/drivers/clk/qcom/lcc-ipq806x.c
> > @@ -34,7 +34,9 @@ static struct clk_pll pll4 = {
> >  	.status_bit = 16,
> >  	.clkr.hw.init = &(struct clk_init_data){
> >  		.name = "pll4",
> > -		.parent_names = (const char *[]){ "pxo" },
> > +		.parent_data = &(const struct clk_parent_data) {
> > +			.fw_name = "pxo", .name = "pxo_board",
> 
> This changes the behavior from looking for the globally named "pxo" to
> look for the globally named "pxo_board", in the event that no
> clock-names of "pxo" was found (based on the .fw_name).
> 
> So you probably want to keep this as .fw_name = "pxo", .name = "pxo".
>

Hi,
I will make this change but just for reference, I could be wrong by
Dimitry pointed out that the pattern is .fw_name pxo .name pxo_board.
The original patch had both set to pxo and it was asked to be changed.

> > +		},
> >  		.num_parents = 1,
> >  		.ops = &clk_pll_ops,
> >  	},
> > @@ -64,9 +66,9 @@ static const struct parent_map lcc_pxo_pll4_map[] = {
> >  	{ P_PLL4, 2 }
> >  };
> >  
> > -static const char * const lcc_pxo_pll4[] = {
> > -	"pxo",
> > -	"pll4_vote",
> > +static const struct clk_parent_data lcc_pxo_pll4[] = {
> > +	{ .fw_name = "pxo", .name = "pxo" },
> > +	{ .fw_name = "pll4_vote", .name = "pll4_vote" },
> 
> This is a reference to a clock defined in this same driver, so you can
> use { .hw = &pll4_vote.clkr.hw } to avoid the lookup all together.
> 

Eh... pll4_vote is defined in gcc (for some reason) the one we have here
is pll4.

I asked if this could be fixed in some way but it was said that it's
better to not complicate things too much.

> >  };
> >  
> >  static struct freq_tbl clk_tbl_aif_mi2s[] = {
> > @@ -131,18 +133,14 @@ static struct clk_rcg mi2s_osr_src = {
> >  		.enable_mask = BIT(9),
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "mi2s_osr_src",
> > -			.parent_names = lcc_pxo_pll4,
> > -			.num_parents = 2,
> > +			.parent_data = lcc_pxo_pll4,
> > +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
> >  			.ops = &clk_rcg_ops,
> >  			.flags = CLK_SET_RATE_GATE,
> >  		},
> >  	},
> >  };
> >  
> > -static const char * const lcc_mi2s_parents[] = {
> > -	"mi2s_osr_src",
> > -};
> > -
> >  static struct clk_branch mi2s_osr_clk = {
> >  	.halt_reg = 0x50,
> >  	.halt_bit = 1,
> > @@ -152,7 +150,9 @@ static struct clk_branch mi2s_osr_clk = {
> >  		.enable_mask = BIT(17),
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "mi2s_osr_clk",
> > -			.parent_names = lcc_mi2s_parents,
> > +			.parent_hws = (const struct clk_hw*[]){
> > +				&mi2s_osr_src.clkr.hw,
> > +			},
> >  			.num_parents = 1,
> >  			.ops = &clk_branch_ops,
> >  			.flags = CLK_SET_RATE_PARENT,
> > @@ -167,7 +167,9 @@ static struct clk_regmap_div mi2s_div_clk = {
> >  	.clkr = {
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "mi2s_div_clk",
> > -			.parent_names = lcc_mi2s_parents,
> > +			.parent_hws = (const struct clk_hw*[]){
> 
> It would be wonderful if you could keep a space between ) and { in
> these.
> 

You mean only here or in the entire patch? I assume the latter.

> > +				&mi2s_osr_src.clkr.hw,
> > +			},
> >  			.num_parents = 1,
> >  			.ops = &clk_regmap_div_ops,
> >  		},
> > @@ -183,7 +185,9 @@ static struct clk_branch mi2s_bit_div_clk = {
> >  		.enable_mask = BIT(15),
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "mi2s_bit_div_clk",
> > -			.parent_names = (const char *[]){ "mi2s_div_clk" },
> > +			.parent_hws = (const struct clk_hw*[]){
> > +				&mi2s_div_clk.clkr.hw,
> > +			},
> >  			.num_parents = 1,
> >  			.ops = &clk_branch_ops,
> >  			.flags = CLK_SET_RATE_PARENT,
> > @@ -191,6 +195,10 @@ static struct clk_branch mi2s_bit_div_clk = {
> >  	},
> >  };
> >  
> > +static const struct clk_parent_data lcc_mi2s_bit_div_codec_clk[] = {
> > +	{ .hw = &mi2s_bit_div_clk.clkr.hw, },
> > +	{ .fw_name = "mi2s_codec_clk", .name = "mi2s_codec_clk" },
> 
> Is mi2s_codec_clk and external clock? I don't see it documented in the
> DT binding. And if we're introducing new clock-names, perhaps we could
> skip the _clk suffix - because obviously it's a clock :)
> 
> Regards,
> Bjorn
> 

I also didn't find where is mi2s_codec_clk... but yes I will change the
fw_name with the clock with _clk stripped.

Will send v6 with the other question clarified.

> > +};
> >  
> >  static struct clk_regmap_mux mi2s_bit_clk = {
> >  	.reg = 0x48,
> > @@ -199,11 +207,8 @@ static struct clk_regmap_mux mi2s_bit_clk = {
> >  	.clkr = {
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "mi2s_bit_clk",
> > -			.parent_names = (const char *[]){
> > -				"mi2s_bit_div_clk",
> > -				"mi2s_codec_clk",
> > -			},
> > -			.num_parents = 2,
> > +			.parent_data = lcc_mi2s_bit_div_codec_clk,
> > +			.num_parents = ARRAY_SIZE(lcc_mi2s_bit_div_codec_clk),
> >  			.ops = &clk_regmap_mux_closest_ops,
> >  			.flags = CLK_SET_RATE_PARENT,
> >  		},
> > @@ -245,8 +250,8 @@ static struct clk_rcg pcm_src = {
> >  		.enable_mask = BIT(9),
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "pcm_src",
> > -			.parent_names = lcc_pxo_pll4,
> > -			.num_parents = 2,
> > +			.parent_data = lcc_pxo_pll4,
> > +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
> >  			.ops = &clk_rcg_ops,
> >  			.flags = CLK_SET_RATE_GATE,
> >  		},
> > @@ -262,7 +267,9 @@ static struct clk_branch pcm_clk_out = {
> >  		.enable_mask = BIT(11),
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "pcm_clk_out",
> > -			.parent_names = (const char *[]){ "pcm_src" },
> > +			.parent_hws = (const struct clk_hw*[]){
> > +				&pcm_src.clkr.hw,
> > +			},
> >  			.num_parents = 1,
> >  			.ops = &clk_branch_ops,
> >  			.flags = CLK_SET_RATE_PARENT,
> > @@ -270,6 +277,11 @@ static struct clk_branch pcm_clk_out = {
> >  	},
> >  };
> >  
> > +static const struct clk_parent_data lcc_pcm_clk_out_codec_clk[] = {
> > +	{ .hw = &pcm_clk_out.clkr.hw, },
> > +	{ .fw_name = "pcm_codec_clk", .name = "pcm_codec_clk" },
> > +};
> > +
> >  static struct clk_regmap_mux pcm_clk = {
> >  	.reg = 0x54,
> >  	.shift = 10,
> > @@ -277,11 +289,8 @@ static struct clk_regmap_mux pcm_clk = {
> >  	.clkr = {
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "pcm_clk",
> > -			.parent_names = (const char *[]){
> > -				"pcm_clk_out",
> > -				"pcm_codec_clk",
> > -			},
> > -			.num_parents = 2,
> > +			.parent_data = lcc_pcm_clk_out_codec_clk,
> > +			.num_parents = ARRAY_SIZE(lcc_pcm_clk_out_codec_clk),
> >  			.ops = &clk_regmap_mux_closest_ops,
> >  			.flags = CLK_SET_RATE_PARENT,
> >  		},
> > @@ -325,18 +334,14 @@ static struct clk_rcg spdif_src = {
> >  		.enable_mask = BIT(9),
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "spdif_src",
> > -			.parent_names = lcc_pxo_pll4,
> > -			.num_parents = 2,
> > +			.parent_data = lcc_pxo_pll4,
> > +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
> >  			.ops = &clk_rcg_ops,
> >  			.flags = CLK_SET_RATE_GATE,
> >  		},
> >  	},
> >  };
> >  
> > -static const char * const lcc_spdif_parents[] = {
> > -	"spdif_src",
> > -};
> > -
> >  static struct clk_branch spdif_clk = {
> >  	.halt_reg = 0xd4,
> >  	.halt_bit = 1,
> > @@ -346,7 +351,9 @@ static struct clk_branch spdif_clk = {
> >  		.enable_mask = BIT(12),
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "spdif_clk",
> > -			.parent_names = lcc_spdif_parents,
> > +			.parent_hws = (const struct clk_hw*[]){
> > +				&spdif_src.clkr.hw,
> > +			},
> >  			.num_parents = 1,
> >  			.ops = &clk_branch_ops,
> >  			.flags = CLK_SET_RATE_PARENT,
> > @@ -384,8 +391,8 @@ static struct clk_rcg ahbix_clk = {
> >  		.enable_mask = BIT(11),
> >  		.hw.init = &(struct clk_init_data){
> >  			.name = "ahbix",
> > -			.parent_names = lcc_pxo_pll4,
> > -			.num_parents = 2,
> > +			.parent_data = lcc_pxo_pll4,
> > +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
> >  			.ops = &clk_rcg_lcc_ops,
> >  		},
> >  	},
> > -- 
> > 2.36.1
> > 

-- 
	Ansuel

Re: [PATCH v5 3/3] clk: qcom: lcc-ipq806x: convert to parent data

[Marijn Suijten]

On 2022-07-08 02:03:38, Christian Marangi wrote:
> Convert lcc-ipq806x driver to parent_data API.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> v5:
> - Fix the same compilation error (don't know what the hell happen
>   to my buildroot)
> v4:
> - Fix compilation error
> v3:
>  - Inline pxo pll4 parent
>  - Change .name from pxo to pxo_board
> 
>  drivers/clk/qcom/lcc-ipq806x.c | 77 ++++++++++++++++++----------------
>  1 file changed, 42 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/clk/qcom/lcc-ipq806x.c b/drivers/clk/qcom/lcc-ipq806x.c
> index ba90bebba597..72d6aea5be30 100644
> --- a/drivers/clk/qcom/lcc-ipq806x.c
> +++ b/drivers/clk/qcom/lcc-ipq806x.c
> @@ -34,7 +34,9 @@ static struct clk_pll pll4 = {
>  	.status_bit = 16,
>  	.clkr.hw.init = &(struct clk_init_data){
>  		.name = "pll4",
> -		.parent_names = (const char *[]){ "pxo" },
> +		.parent_data = &(const struct clk_parent_data) {
> +			.fw_name = "pxo", .name = "pxo_board",
> +		},
>  		.num_parents = 1,
>  		.ops = &clk_pll_ops,
>  	},
> @@ -64,9 +66,9 @@ static const struct parent_map lcc_pxo_pll4_map[] = {
>  	{ P_PLL4, 2 }
>  };
>  
> -static const char * const lcc_pxo_pll4[] = {
> -	"pxo",
> -	"pll4_vote",
> +static const struct clk_parent_data lcc_pxo_pll4[] = {
> +	{ .fw_name = "pxo", .name = "pxo" },
> +	{ .fw_name = "pll4_vote", .name = "pll4_vote" },
>  };
>  
>  static struct freq_tbl clk_tbl_aif_mi2s[] = {
> @@ -131,18 +133,14 @@ static struct clk_rcg mi2s_osr_src = {
>  		.enable_mask = BIT(9),
>  		.hw.init = &(struct clk_init_data){
>  			.name = "mi2s_osr_src",
> -			.parent_names = lcc_pxo_pll4,
> -			.num_parents = 2,
> +			.parent_data = lcc_pxo_pll4,
> +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),

We've typically done the ARRAY_SIZE conversion in a separate patch, as
they're not related to "parent_data API", strictly speaking.
(Except in the lcc_mi2s_bit_div_codec_clk etc case below, which was
written inline previously).

- Marijn

[.. snip ..]

Re: [PATCH v5 3/3] clk: qcom: lcc-ipq806x: convert to parent data

[Dmitry Baryshkov]

On 19/07/2022 15:23, Christian Marangi wrote:
> On Mon, Jul 18, 2022 at 11:42:29PM -0500, Bjorn Andersson wrote:
>> On Thu 07 Jul 19:03 CDT 2022, Christian Marangi wrote:
>>
>>> Convert lcc-ipq806x driver to parent_data API.
>>>
>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>> ---
>>> v5:
>>> - Fix the same compilation error (don't know what the hell happen
>>>    to my buildroot)
>>> v4:
>>> - Fix compilation error
>>> v3:
>>>   - Inline pxo pll4 parent
>>>   - Change .name from pxo to pxo_board
>>>
>>>   drivers/clk/qcom/lcc-ipq806x.c | 77 ++++++++++++++++++----------------
>>>   1 file changed, 42 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/lcc-ipq806x.c b/drivers/clk/qcom/lcc-ipq806x.c
>>> index ba90bebba597..72d6aea5be30 100644
>>> --- a/drivers/clk/qcom/lcc-ipq806x.c
>>> +++ b/drivers/clk/qcom/lcc-ipq806x.c
>>> @@ -34,7 +34,9 @@ static struct clk_pll pll4 = {
>>>   	.status_bit = 16,
>>>   	.clkr.hw.init = &(struct clk_init_data){
>>>   		.name = "pll4",
>>> -		.parent_names = (const char *[]){ "pxo" },
>>> +		.parent_data = &(const struct clk_parent_data) {
>>> +			.fw_name = "pxo", .name = "pxo_board",
>>
>> This changes the behavior from looking for the globally named "pxo" to
>> look for the globally named "pxo_board", in the event that no
>> clock-names of "pxo" was found (based on the .fw_name).
>>
>> So you probably want to keep this as .fw_name = "pxo", .name = "pxo".
>>
> 
> Hi,
> I will make this change but just for reference, I could be wrong by
> Dimitry pointed out that the pattern is .fw_name pxo .name pxo_board.
> The original patch had both set to pxo and it was asked to be changed.

We are generally trying to get rid of manually registered 'pxo' clock, 
thus all parent_names = pxo/cxo/xo entries are converted to .fw_name = 
"pxo/cxo/xo", .name = "pxo_board/cxo_board/xo_board" clocks. This has 
been done previously for all converted drivers w/o any questions. May be 
it's worth it mentioning pxo_board in the commit message.

> 
>>> +		},
>>>   		.num_parents = 1,
>>>   		.ops = &clk_pll_ops,
>>>   	},
>>> @@ -64,9 +66,9 @@ static const struct parent_map lcc_pxo_pll4_map[] = {
>>>   	{ P_PLL4, 2 }
>>>   };
>>>   
>>> -static const char * const lcc_pxo_pll4[] = {
>>> -	"pxo",
>>> -	"pll4_vote",
>>> +static const struct clk_parent_data lcc_pxo_pll4[] = {
>>> +	{ .fw_name = "pxo", .name = "pxo" },
>>> +	{ .fw_name = "pll4_vote", .name = "pll4_vote" },
>>
>> This is a reference to a clock defined in this same driver, so you can
>> use { .hw = &pll4_vote.clkr.hw } to avoid the lookup all together.
>>
> 
> Eh... pll4_vote is defined in gcc (for some reason) the one we have here
> is pll4.
> 
> I asked if this could be fixed in some way but it was said that it's
> better to not complicate things too much.

The chain is:
pxo -> pll4 @ lcc -> pll4_vote @ gcc -> i2s clocks @ lcc.


> 
>>>   };
>>>   
>>>   static struct freq_tbl clk_tbl_aif_mi2s[] = {
>>> @@ -131,18 +133,14 @@ static struct clk_rcg mi2s_osr_src = {
>>>   		.enable_mask = BIT(9),
>>>   		.hw.init = &(struct clk_init_data){
>>>   			.name = "mi2s_osr_src",
>>> -			.parent_names = lcc_pxo_pll4,
>>> -			.num_parents = 2,
>>> +			.parent_data = lcc_pxo_pll4,
>>> +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
>>>   			.ops = &clk_rcg_ops,
>>>   			.flags = CLK_SET_RATE_GATE,
>>>   		},
>>>   	},
>>>   };
>>>   
>>> -static const char * const lcc_mi2s_parents[] = {
>>> -	"mi2s_osr_src",
>>> -};
>>> -
>>>   static struct clk_branch mi2s_osr_clk = {
>>>   	.halt_reg = 0x50,
>>>   	.halt_bit = 1,
>>> @@ -152,7 +150,9 @@ static struct clk_branch mi2s_osr_clk = {
>>>   		.enable_mask = BIT(17),
>>>   		.hw.init = &(struct clk_init_data){
>>>   			.name = "mi2s_osr_clk",
>>> -			.parent_names = lcc_mi2s_parents,
>>> +			.parent_hws = (const struct clk_hw*[]){
>>> +				&mi2s_osr_src.clkr.hw,
>>> +			},
>>>   			.num_parents = 1,
>>>   			.ops = &clk_branch_ops,
>>>   			.flags = CLK_SET_RATE_PARENT,
>>> @@ -167,7 +167,9 @@ static struct clk_regmap_div mi2s_div_clk = {
>>>   	.clkr = {
>>>   		.hw.init = &(struct clk_init_data){
>>>   			.name = "mi2s_div_clk",
>>> -			.parent_names = lcc_mi2s_parents,
>>> +			.parent_hws = (const struct clk_hw*[]){
>>
>> It would be wonderful if you could keep a space between ) and { in
>> these.
>>
> 
> You mean only here or in the entire patch? I assume the latter.
> 
>>> +				&mi2s_osr_src.clkr.hw,
>>> +			},
>>>   			.num_parents = 1,
>>>   			.ops = &clk_regmap_div_ops,
>>>   		},
>>> @@ -183,7 +185,9 @@ static struct clk_branch mi2s_bit_div_clk = {
>>>   		.enable_mask = BIT(15),
>>>   		.hw.init = &(struct clk_init_data){
>>>   			.name = "mi2s_bit_div_clk",
>>> -			.parent_names = (const char *[]){ "mi2s_div_clk" },
>>> +			.parent_hws = (const struct clk_hw*[]){
>>> +				&mi2s_div_clk.clkr.hw,
>>> +			},
>>>   			.num_parents = 1,
>>>   			.ops = &clk_branch_ops,
>>>   			.flags = CLK_SET_RATE_PARENT,
>>> @@ -191,6 +195,10 @@ static struct clk_branch mi2s_bit_div_clk = {
>>>   	},
>>>   };
>>>   
>>> +static const struct clk_parent_data lcc_mi2s_bit_div_codec_clk[] = {
>>> +	{ .hw = &mi2s_bit_div_clk.clkr.hw, },
>>> +	{ .fw_name = "mi2s_codec_clk", .name = "mi2s_codec_clk" },
>>
>> Is mi2s_codec_clk and external clock? I don't see it documented in the
>> DT binding. And if we're introducing new clock-names, perhaps we could
>> skip the _clk suffix - because obviously it's a clock :)
>>
>> Regards,
>> Bjorn
>>
> 
> I also didn't find where is mi2s_codec_clk... but yes I will change the
> fw_name with the clock with _clk stripped.

Downstream seems not to use _codec_clk, it just always uses the 
bit_div_clk as the codec's bit_clk. Maybe Srini knows additional 
details, as APQ8064 has more or less the same structure of clocks.

> 
> Will send v6 with the other question clarified.
> 


-- 
With best wishes
Dmitry

Re: [PATCH v5 3/3] clk: qcom: lcc-ipq806x: convert to parent data

[Bjorn Andersson]

On Tue 19 Jul 07:23 CDT 2022, Christian Marangi wrote:

> On Mon, Jul 18, 2022 at 11:42:29PM -0500, Bjorn Andersson wrote:
> > On Thu 07 Jul 19:03 CDT 2022, Christian Marangi wrote:
> > 
> > > Convert lcc-ipq806x driver to parent_data API.
> > > 
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > > v5:
> > > - Fix the same compilation error (don't know what the hell happen
> > >   to my buildroot)
> > > v4:
> > > - Fix compilation error
> > > v3:
> > >  - Inline pxo pll4 parent
> > >  - Change .name from pxo to pxo_board
> > > 
> > >  drivers/clk/qcom/lcc-ipq806x.c | 77 ++++++++++++++++++----------------
> > >  1 file changed, 42 insertions(+), 35 deletions(-)
> > > 
> > > diff --git a/drivers/clk/qcom/lcc-ipq806x.c b/drivers/clk/qcom/lcc-ipq806x.c
> > > index ba90bebba597..72d6aea5be30 100644
> > > --- a/drivers/clk/qcom/lcc-ipq806x.c
> > > +++ b/drivers/clk/qcom/lcc-ipq806x.c
> > > @@ -34,7 +34,9 @@ static struct clk_pll pll4 = {
> > >  	.status_bit = 16,
> > >  	.clkr.hw.init = &(struct clk_init_data){
> > >  		.name = "pll4",
> > > -		.parent_names = (const char *[]){ "pxo" },
> > > +		.parent_data = &(const struct clk_parent_data) {
> > > +			.fw_name = "pxo", .name = "pxo_board",
> > 
> > This changes the behavior from looking for the globally named "pxo" to
> > look for the globally named "pxo_board", in the event that no
> > clock-names of "pxo" was found (based on the .fw_name).
> > 
> > So you probably want to keep this as .fw_name = "pxo", .name = "pxo".
> >
> 
> Hi,
> I will make this change but just for reference, I could be wrong by
> Dimitry pointed out that the pattern is .fw_name pxo .name pxo_board.
> The original patch had both set to pxo and it was asked to be changed.
> 

I see, so we currently have gcc registering a fixed clock to "create"
"pxo" out of "pxo_board" and with this change we'd hook onto pxo
directly. Which would then allow us to drop the creation of "pxo" in
gcc.

I'm in favor of this plan, but this makes the change not only a
transition from parent_names to parent_data, so could you please
document that you're changing this parent.

> > > +		},
> > >  		.num_parents = 1,
> > >  		.ops = &clk_pll_ops,
> > >  	},
> > > @@ -64,9 +66,9 @@ static const struct parent_map lcc_pxo_pll4_map[] = {
> > >  	{ P_PLL4, 2 }
> > >  };
> > >  
> > > -static const char * const lcc_pxo_pll4[] = {
> > > -	"pxo",
> > > -	"pll4_vote",
> > > +static const struct clk_parent_data lcc_pxo_pll4[] = {
> > > +	{ .fw_name = "pxo", .name = "pxo" },
> > > +	{ .fw_name = "pll4_vote", .name = "pll4_vote" },
> > 
> > This is a reference to a clock defined in this same driver, so you can
> > use { .hw = &pll4_vote.clkr.hw } to avoid the lookup all together.
> > 
> 
> Eh... pll4_vote is defined in gcc (for some reason) the one we have here
> is pll4.
> 
> I asked if this could be fixed in some way but it was said that it's
> better to not complicate things too much.
> 

Sorry for the noise, I didn't pay attention to the gcc/lcc split.
This seems reasonable.

Presumably though, you should make the pxo -> pxo_board transition for
.name here as well then?

Regards,
Bjorn

> > >  };
> > >  
> > >  static struct freq_tbl clk_tbl_aif_mi2s[] = {
> > > @@ -131,18 +133,14 @@ static struct clk_rcg mi2s_osr_src = {
> > >  		.enable_mask = BIT(9),
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "mi2s_osr_src",
> > > -			.parent_names = lcc_pxo_pll4,
> > > -			.num_parents = 2,
> > > +			.parent_data = lcc_pxo_pll4,
> > > +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
> > >  			.ops = &clk_rcg_ops,
> > >  			.flags = CLK_SET_RATE_GATE,
> > >  		},
> > >  	},
> > >  };
> > >  
> > > -static const char * const lcc_mi2s_parents[] = {
> > > -	"mi2s_osr_src",
> > > -};
> > > -
> > >  static struct clk_branch mi2s_osr_clk = {
> > >  	.halt_reg = 0x50,
> > >  	.halt_bit = 1,
> > > @@ -152,7 +150,9 @@ static struct clk_branch mi2s_osr_clk = {
> > >  		.enable_mask = BIT(17),
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "mi2s_osr_clk",
> > > -			.parent_names = lcc_mi2s_parents,
> > > +			.parent_hws = (const struct clk_hw*[]){
> > > +				&mi2s_osr_src.clkr.hw,
> > > +			},
> > >  			.num_parents = 1,
> > >  			.ops = &clk_branch_ops,
> > >  			.flags = CLK_SET_RATE_PARENT,
> > > @@ -167,7 +167,9 @@ static struct clk_regmap_div mi2s_div_clk = {
> > >  	.clkr = {
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "mi2s_div_clk",
> > > -			.parent_names = lcc_mi2s_parents,
> > > +			.parent_hws = (const struct clk_hw*[]){
> > 
> > It would be wonderful if you could keep a space between ) and { in
> > these.
> > 
> 
> You mean only here or in the entire patch? I assume the latter.
> 
> > > +				&mi2s_osr_src.clkr.hw,
> > > +			},
> > >  			.num_parents = 1,
> > >  			.ops = &clk_regmap_div_ops,
> > >  		},
> > > @@ -183,7 +185,9 @@ static struct clk_branch mi2s_bit_div_clk = {
> > >  		.enable_mask = BIT(15),
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "mi2s_bit_div_clk",
> > > -			.parent_names = (const char *[]){ "mi2s_div_clk" },
> > > +			.parent_hws = (const struct clk_hw*[]){
> > > +				&mi2s_div_clk.clkr.hw,
> > > +			},
> > >  			.num_parents = 1,
> > >  			.ops = &clk_branch_ops,
> > >  			.flags = CLK_SET_RATE_PARENT,
> > > @@ -191,6 +195,10 @@ static struct clk_branch mi2s_bit_div_clk = {
> > >  	},
> > >  };
> > >  
> > > +static const struct clk_parent_data lcc_mi2s_bit_div_codec_clk[] = {
> > > +	{ .hw = &mi2s_bit_div_clk.clkr.hw, },
> > > +	{ .fw_name = "mi2s_codec_clk", .name = "mi2s_codec_clk" },
> > 
> > Is mi2s_codec_clk and external clock? I don't see it documented in the
> > DT binding. And if we're introducing new clock-names, perhaps we could
> > skip the _clk suffix - because obviously it's a clock :)
> > 
> > Regards,
> > Bjorn
> > 
> 
> I also didn't find where is mi2s_codec_clk... but yes I will change the
> fw_name with the clock with _clk stripped.
> 
> Will send v6 with the other question clarified.
> 
> > > +};
> > >  
> > >  static struct clk_regmap_mux mi2s_bit_clk = {
> > >  	.reg = 0x48,
> > > @@ -199,11 +207,8 @@ static struct clk_regmap_mux mi2s_bit_clk = {
> > >  	.clkr = {
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "mi2s_bit_clk",
> > > -			.parent_names = (const char *[]){
> > > -				"mi2s_bit_div_clk",
> > > -				"mi2s_codec_clk",
> > > -			},
> > > -			.num_parents = 2,
> > > +			.parent_data = lcc_mi2s_bit_div_codec_clk,
> > > +			.num_parents = ARRAY_SIZE(lcc_mi2s_bit_div_codec_clk),
> > >  			.ops = &clk_regmap_mux_closest_ops,
> > >  			.flags = CLK_SET_RATE_PARENT,
> > >  		},
> > > @@ -245,8 +250,8 @@ static struct clk_rcg pcm_src = {
> > >  		.enable_mask = BIT(9),
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "pcm_src",
> > > -			.parent_names = lcc_pxo_pll4,
> > > -			.num_parents = 2,
> > > +			.parent_data = lcc_pxo_pll4,
> > > +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
> > >  			.ops = &clk_rcg_ops,
> > >  			.flags = CLK_SET_RATE_GATE,
> > >  		},
> > > @@ -262,7 +267,9 @@ static struct clk_branch pcm_clk_out = {
> > >  		.enable_mask = BIT(11),
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "pcm_clk_out",
> > > -			.parent_names = (const char *[]){ "pcm_src" },
> > > +			.parent_hws = (const struct clk_hw*[]){
> > > +				&pcm_src.clkr.hw,
> > > +			},
> > >  			.num_parents = 1,
> > >  			.ops = &clk_branch_ops,
> > >  			.flags = CLK_SET_RATE_PARENT,
> > > @@ -270,6 +277,11 @@ static struct clk_branch pcm_clk_out = {
> > >  	},
> > >  };
> > >  
> > > +static const struct clk_parent_data lcc_pcm_clk_out_codec_clk[] = {
> > > +	{ .hw = &pcm_clk_out.clkr.hw, },
> > > +	{ .fw_name = "pcm_codec_clk", .name = "pcm_codec_clk" },
> > > +};
> > > +
> > >  static struct clk_regmap_mux pcm_clk = {
> > >  	.reg = 0x54,
> > >  	.shift = 10,
> > > @@ -277,11 +289,8 @@ static struct clk_regmap_mux pcm_clk = {
> > >  	.clkr = {
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "pcm_clk",
> > > -			.parent_names = (const char *[]){
> > > -				"pcm_clk_out",
> > > -				"pcm_codec_clk",
> > > -			},
> > > -			.num_parents = 2,
> > > +			.parent_data = lcc_pcm_clk_out_codec_clk,
> > > +			.num_parents = ARRAY_SIZE(lcc_pcm_clk_out_codec_clk),
> > >  			.ops = &clk_regmap_mux_closest_ops,
> > >  			.flags = CLK_SET_RATE_PARENT,
> > >  		},
> > > @@ -325,18 +334,14 @@ static struct clk_rcg spdif_src = {
> > >  		.enable_mask = BIT(9),
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "spdif_src",
> > > -			.parent_names = lcc_pxo_pll4,
> > > -			.num_parents = 2,
> > > +			.parent_data = lcc_pxo_pll4,
> > > +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
> > >  			.ops = &clk_rcg_ops,
> > >  			.flags = CLK_SET_RATE_GATE,
> > >  		},
> > >  	},
> > >  };
> > >  
> > > -static const char * const lcc_spdif_parents[] = {
> > > -	"spdif_src",
> > > -};
> > > -
> > >  static struct clk_branch spdif_clk = {
> > >  	.halt_reg = 0xd4,
> > >  	.halt_bit = 1,
> > > @@ -346,7 +351,9 @@ static struct clk_branch spdif_clk = {
> > >  		.enable_mask = BIT(12),
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "spdif_clk",
> > > -			.parent_names = lcc_spdif_parents,
> > > +			.parent_hws = (const struct clk_hw*[]){
> > > +				&spdif_src.clkr.hw,
> > > +			},
> > >  			.num_parents = 1,
> > >  			.ops = &clk_branch_ops,
> > >  			.flags = CLK_SET_RATE_PARENT,
> > > @@ -384,8 +391,8 @@ static struct clk_rcg ahbix_clk = {
> > >  		.enable_mask = BIT(11),
> > >  		.hw.init = &(struct clk_init_data){
> > >  			.name = "ahbix",
> > > -			.parent_names = lcc_pxo_pll4,
> > > -			.num_parents = 2,
> > > +			.parent_data = lcc_pxo_pll4,
> > > +			.num_parents = ARRAY_SIZE(lcc_pxo_pll4),
> > >  			.ops = &clk_rcg_lcc_ops,
> > >  		},
> > >  	},
> > > -- 
> > > 2.36.1
> > > 
> 
> -- 
> 	Ansuel