LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names
@ 2021-08-24 15:06 Bjorn Andersson
  2021-08-24 20:46 ` Marijn Suijten
  0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Andersson @ 2021-08-24 15:06 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-arm-msm, linux-clk, linux-kernel, Konrad Dybcio,
	angelogioacchino.delregno

Using parent_data and parent_hws, instead of parent_names, does protect
against some cases of incompletely defined clock trees. While it turns
out that the bug being chased this time was totally unrelated, this
patch converts the SDM660 GCC driver to avoid such issues.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/clk/qcom/gcc-sdm660.c | 532 +++++++++++++++++-----------------
 1 file changed, 268 insertions(+), 264 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sdm660.c b/drivers/clk/qcom/gcc-sdm660.c
index 6394257ca8c0..60e41f92d616 100644
--- a/drivers/clk/qcom/gcc-sdm660.c
+++ b/drivers/clk/qcom/gcc-sdm660.c
@@ -27,129 +27,6 @@
 
 #define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
 
-enum {
-	P_XO,
-	P_SLEEP_CLK,
-	P_GPLL0,
-	P_GPLL1,
-	P_GPLL4,
-	P_GPLL0_EARLY_DIV,
-	P_GPLL1_EARLY_DIV,
-};
-
-static const struct parent_map gcc_parent_map_xo_gpll0_gpll0_early_div[] = {
-	{ P_XO, 0 },
-	{ P_GPLL0, 1 },
-	{ P_GPLL0_EARLY_DIV, 6 },
-};
-
-static const char * const gcc_parent_names_xo_gpll0_gpll0_early_div[] = {
-	"xo",
-	"gpll0",
-	"gpll0_early_div",
-};
-
-static const struct parent_map gcc_parent_map_xo_gpll0[] = {
-	{ P_XO, 0 },
-	{ P_GPLL0, 1 },
-};
-
-static const char * const gcc_parent_names_xo_gpll0[] = {
-	"xo",
-	"gpll0",
-};
-
-static const struct parent_map gcc_parent_map_xo_gpll0_sleep_clk_gpll0_early_div[] = {
-	{ P_XO, 0 },
-	{ P_GPLL0, 1 },
-	{ P_SLEEP_CLK, 5 },
-	{ P_GPLL0_EARLY_DIV, 6 },
-};
-
-static const char * const gcc_parent_names_xo_gpll0_sleep_clk_gpll0_early_div[] = {
-	"xo",
-	"gpll0",
-	"sleep_clk",
-	"gpll0_early_div",
-};
-
-static const struct parent_map gcc_parent_map_xo_sleep_clk[] = {
-	{ P_XO, 0 },
-	{ P_SLEEP_CLK, 5 },
-};
-
-static const char * const gcc_parent_names_xo_sleep_clk[] = {
-	"xo",
-	"sleep_clk",
-};
-
-static const struct parent_map gcc_parent_map_xo_gpll4[] = {
-	{ P_XO, 0 },
-	{ P_GPLL4, 5 },
-};
-
-static const char * const gcc_parent_names_xo_gpll4[] = {
-	"xo",
-	"gpll4",
-};
-
-static const struct parent_map gcc_parent_map_xo_gpll0_gpll0_early_div_gpll1_gpll4_gpll1_early_div[] = {
-	{ P_XO, 0 },
-	{ P_GPLL0, 1 },
-	{ P_GPLL0_EARLY_DIV, 3 },
-	{ P_GPLL1, 4 },
-	{ P_GPLL4, 5 },
-	{ P_GPLL1_EARLY_DIV, 6 },
-};
-
-static const char * const gcc_parent_names_xo_gpll0_gpll0_early_div_gpll1_gpll4_gpll1_early_div[] = {
-	"xo",
-	"gpll0",
-	"gpll0_early_div",
-	"gpll1",
-	"gpll4",
-	"gpll1_early_div",
-};
-
-static const struct parent_map gcc_parent_map_xo_gpll0_gpll4_gpll0_early_div[] = {
-	{ P_XO, 0 },
-	{ P_GPLL0, 1 },
-	{ P_GPLL4, 5 },
-	{ P_GPLL0_EARLY_DIV, 6 },
-};
-
-static const char * const gcc_parent_names_xo_gpll0_gpll4_gpll0_early_div[] = {
-	"xo",
-	"gpll0",
-	"gpll4",
-	"gpll0_early_div",
-};
-
-static const struct parent_map gcc_parent_map_xo_gpll0_gpll0_early_div_gpll4[] = {
-	{ P_XO, 0 },
-	{ P_GPLL0, 1 },
-	{ P_GPLL0_EARLY_DIV, 2 },
-	{ P_GPLL4, 5 },
-};
-
-static const char * const gcc_parent_names_xo_gpll0_gpll0_early_div_gpll4[] = {
-	"xo",
-	"gpll0",
-	"gpll0_early_div",
-	"gpll4",
-};
-
-static struct clk_fixed_factor xo = {
-	.mult = 1,
-	.div = 1,
-	.hw.init = &(struct clk_init_data){
-		.name = "xo",
-		.parent_names = (const char *[]){ "xo_board" },
-		.num_parents = 1,
-		.ops = &clk_fixed_factor_ops,
-	},
-};
-
 static struct clk_alpha_pll gpll0_early = {
 	.offset = 0x0,
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
@@ -158,7 +35,9 @@ static struct clk_alpha_pll gpll0_early = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gpll0_early",
-			.parent_names = (const char *[]){ "xo" },
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "xo",
+			},
 			.num_parents = 1,
 			.ops = &clk_alpha_pll_ops,
 		},
@@ -170,7 +49,9 @@ static struct clk_fixed_factor gpll0_early_div = {
 	.div = 2,
 	.hw.init = &(struct clk_init_data){
 		.name = "gpll0_early_div",
-		.parent_names = (const char *[]){ "gpll0_early" },
+		.parent_hws = (const struct clk_hw*[]){
+			&gpll0_early.clkr.hw,
+		},
 		.num_parents = 1,
 		.ops = &clk_fixed_factor_ops,
 	},
@@ -181,7 +62,9 @@ static struct clk_alpha_pll_postdiv gpll0 = {
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll0",
-		.parent_names = (const char *[]){ "gpll0_early" },
+		.parent_hws = (const struct clk_hw*[]){
+			&gpll0_early.clkr.hw,
+		},
 		.num_parents = 1,
 		.ops = &clk_alpha_pll_postdiv_ops,
 	},
@@ -195,7 +78,9 @@ static struct clk_alpha_pll gpll1_early = {
 		.enable_mask = BIT(1),
 		.hw.init = &(struct clk_init_data){
 			.name = "gpll1_early",
-			.parent_names = (const char *[]){ "xo" },
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "xo",
+			},
 			.num_parents = 1,
 			.ops = &clk_alpha_pll_ops,
 		},
@@ -207,7 +92,9 @@ static struct clk_fixed_factor gpll1_early_div = {
 	.div = 2,
 	.hw.init = &(struct clk_init_data){
 		.name = "gpll1_early_div",
-		.parent_names = (const char *[]){ "gpll1_early" },
+		.parent_hws = (const struct clk_hw*[]){
+			&gpll1_early.clkr.hw,
+		},
 		.num_parents = 1,
 		.ops = &clk_fixed_factor_ops,
 	},
@@ -218,7 +105,9 @@ static struct clk_alpha_pll_postdiv gpll1 = {
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gpll1",
-		.parent_names = (const char *[]){ "gpll1_early" },
+		.parent_hws = (const struct clk_hw*[]){
+			&gpll1_early.clkr.hw,
+		},
 		.num_parents = 1,
 		.ops = &clk_alpha_pll_postdiv_ops,
 	},
@@ -232,7 +121,9 @@ static struct clk_alpha_pll gpll4_early = {
 		.enable_mask = BIT(4),
 		.hw.init = &(struct clk_init_data){
 			.name = "gpll4_early",
-			.parent_names = (const char *[]){ "xo" },
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "xo",
+			},
 			.num_parents = 1,
 			.ops = &clk_alpha_pll_ops,
 		},
@@ -245,12 +136,126 @@ static struct clk_alpha_pll_postdiv gpll4 = {
 	.clkr.hw.init = &(struct clk_init_data)
 	{
 		.name = "gpll4",
-		.parent_names = (const char *[]) { "gpll4_early" },
+		.parent_hws = (const struct clk_hw*[]){
+			&gpll4_early.clkr.hw,
+		},
 		.num_parents = 1,
 		.ops = &clk_alpha_pll_postdiv_ops,
 	},
 };
 
+enum {
+	P_XO,
+	P_SLEEP_CLK,
+	P_GPLL0,
+	P_GPLL1,
+	P_GPLL4,
+	P_GPLL0_EARLY_DIV,
+	P_GPLL1_EARLY_DIV,
+};
+
+static const struct parent_map gcc_parent_map_xo_gpll0_gpll0_early_div[] = {
+	{ P_XO, 0 },
+	{ P_GPLL0, 1 },
+	{ P_GPLL0_EARLY_DIV, 6 },
+};
+
+static const struct clk_parent_data gcc_parent_data_xo_gpll0_gpll0_early_div[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &gpll0.clkr.hw },
+	{ .hw = &gpll0_early_div.hw },
+};
+
+static const struct parent_map gcc_parent_map_xo_gpll0[] = {
+	{ P_XO, 0 },
+	{ P_GPLL0, 1 },
+};
+
+static const struct clk_parent_data gcc_parent_data_xo_gpll0[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &gpll0.clkr.hw },
+};
+
+static const struct parent_map gcc_parent_map_xo_gpll0_sleep_clk_gpll0_early_div[] = {
+	{ P_XO, 0 },
+	{ P_GPLL0, 1 },
+	{ P_SLEEP_CLK, 5 },
+	{ P_GPLL0_EARLY_DIV, 6 },
+};
+
+static const struct clk_parent_data gcc_parent_data_xo_gpll0_sleep_clk_gpll0_early_div[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &gpll0.clkr.hw },
+	{ .fw_name = "sleep_clk" },
+	{ .hw = &gpll0_early_div.hw },
+};
+
+static const struct parent_map gcc_parent_map_xo_sleep_clk[] = {
+	{ P_XO, 0 },
+	{ P_SLEEP_CLK, 5 },
+};
+
+static const struct clk_parent_data gcc_parent_data_xo_sleep_clk[] = {
+	{ .fw_name = "xo" },
+	{ .fw_name = "sleep_clk" },
+};
+
+static const struct parent_map gcc_parent_map_xo_gpll4[] = {
+	{ P_XO, 0 },
+	{ P_GPLL4, 5 },
+};
+
+static const struct clk_parent_data gcc_parent_data_xo_gpll4[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &gpll4.clkr.hw },
+};
+
+static const struct parent_map gcc_parent_map_xo_gpll0_gpll0_early_div_gpll1_gpll4_gpll1_early_div[] = {
+	{ P_XO, 0 },
+	{ P_GPLL0, 1 },
+	{ P_GPLL0_EARLY_DIV, 3 },
+	{ P_GPLL1, 4 },
+	{ P_GPLL4, 5 },
+	{ P_GPLL1_EARLY_DIV, 6 },
+};
+
+static const struct clk_parent_data gcc_parent_data_xo_gpll0_gpll0_early_div_gpll1_gpll4_gpll1_early_div[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &gpll0.clkr.hw },
+	{ .hw = &gpll0_early_div.hw },
+	{ .hw = &gpll1.clkr.hw },
+	{ .hw = &gpll4.clkr.hw },
+	{ .hw = &gpll1_early_div.hw },
+};
+
+static const struct parent_map gcc_parent_map_xo_gpll0_gpll4_gpll0_early_div[] = {
+	{ P_XO, 0 },
+	{ P_GPLL0, 1 },
+	{ P_GPLL4, 5 },
+	{ P_GPLL0_EARLY_DIV, 6 },
+};
+
+static const struct clk_parent_data gcc_parent_data_xo_gpll0_gpll4_gpll0_early_div[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &gpll0.clkr.hw },
+	{ .hw = &gpll4.clkr.hw },
+	{ .hw = &gpll0_early_div.hw },
+};
+
+static const struct parent_map gcc_parent_map_xo_gpll0_gpll0_early_div_gpll4[] = {
+	{ P_XO, 0 },
+	{ P_GPLL0, 1 },
+	{ P_GPLL0_EARLY_DIV, 2 },
+	{ P_GPLL4, 5 },
+};
+
+static const struct clk_parent_data gcc_parent_data_xo_gpll0_gpll0_early_div_gpll4[] = {
+	{ .fw_name = "xo" },
+	{ .hw = &gpll0.clkr.hw },
+	{ .hw = &gpll0_early_div.hw },
+	{ .hw = &gpll4.clkr.hw },
+};
+
 static const struct freq_tbl ftbl_blsp1_qup1_i2c_apps_clk_src[] = {
 	F(19200000, P_XO, 1, 0, 0),
 	F(50000000, P_GPLL0, 12, 0, 0),
@@ -265,7 +270,7 @@ static struct clk_rcg2 blsp1_qup1_i2c_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_qup1_i2c_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -290,7 +295,7 @@ static struct clk_rcg2 blsp1_qup1_spi_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_spi_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_qup1_spi_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -304,7 +309,7 @@ static struct clk_rcg2 blsp1_qup2_i2c_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_qup2_i2c_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -318,7 +323,7 @@ static struct clk_rcg2 blsp1_qup2_spi_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_spi_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_qup2_spi_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -332,7 +337,7 @@ static struct clk_rcg2 blsp1_qup3_i2c_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_qup3_i2c_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -346,7 +351,7 @@ static struct clk_rcg2 blsp1_qup3_spi_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_spi_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_qup3_spi_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -360,7 +365,7 @@ static struct clk_rcg2 blsp1_qup4_i2c_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_qup4_i2c_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -374,7 +379,7 @@ static struct clk_rcg2 blsp1_qup4_spi_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_spi_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_qup4_spi_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -407,7 +412,7 @@ static struct clk_rcg2 blsp1_uart1_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_uart1_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_uart1_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -421,7 +426,7 @@ static struct clk_rcg2 blsp1_uart2_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_uart1_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp1_uart2_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -435,7 +440,7 @@ static struct clk_rcg2 blsp2_qup1_i2c_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_qup1_i2c_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -449,7 +454,7 @@ static struct clk_rcg2 blsp2_qup1_spi_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_spi_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_qup1_spi_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -463,7 +468,7 @@ static struct clk_rcg2 blsp2_qup2_i2c_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_qup2_i2c_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -477,7 +482,7 @@ static struct clk_rcg2 blsp2_qup2_spi_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_spi_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_qup2_spi_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -491,7 +496,7 @@ static struct clk_rcg2 blsp2_qup3_i2c_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_qup3_i2c_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -505,7 +510,7 @@ static struct clk_rcg2 blsp2_qup3_spi_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_spi_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_qup3_spi_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -519,7 +524,7 @@ static struct clk_rcg2 blsp2_qup4_i2c_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_qup4_i2c_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -533,7 +538,7 @@ static struct clk_rcg2 blsp2_qup4_spi_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_qup1_spi_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_qup4_spi_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -547,7 +552,7 @@ static struct clk_rcg2 blsp2_uart1_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_uart1_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_uart1_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -561,7 +566,7 @@ static struct clk_rcg2 blsp2_uart2_apps_clk_src = {
 	.freq_tbl = ftbl_blsp1_uart1_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "blsp2_uart2_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -582,7 +587,7 @@ static struct clk_rcg2 gp1_clk_src = {
 	.freq_tbl = ftbl_gp1_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gp1_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_sleep_clk_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_sleep_clk_gpll0_early_div,
 		.num_parents = 4,
 		.ops = &clk_rcg2_ops,
 	},
@@ -596,7 +601,7 @@ static struct clk_rcg2 gp2_clk_src = {
 	.freq_tbl = ftbl_gp1_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gp2_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_sleep_clk_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_sleep_clk_gpll0_early_div,
 		.num_parents = 4,
 		.ops = &clk_rcg2_ops,
 	},
@@ -610,7 +615,7 @@ static struct clk_rcg2 gp3_clk_src = {
 	.freq_tbl = ftbl_gp1_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "gp3_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_sleep_clk_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_sleep_clk_gpll0_early_div,
 		.num_parents = 4,
 		.ops = &clk_rcg2_ops,
 	},
@@ -630,7 +635,7 @@ static struct clk_rcg2 hmss_gpll0_clk_src = {
 	.freq_tbl = ftbl_hmss_gpll0_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "hmss_gpll0_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -651,7 +656,7 @@ static struct clk_rcg2 hmss_gpll4_clk_src = {
 	.freq_tbl = ftbl_hmss_gpll4_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "hmss_gpll4_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll4,
+		.parent_data = gcc_parent_data_xo_gpll4,
 		.num_parents = 2,
 		.ops = &clk_rcg2_ops,
 	},
@@ -670,7 +675,7 @@ static struct clk_rcg2 hmss_rbcpr_clk_src = {
 	.freq_tbl = ftbl_hmss_rbcpr_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "hmss_rbcpr_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0,
+		.parent_data = gcc_parent_data_xo_gpll0,
 		.num_parents = 2,
 		.ops = &clk_rcg2_ops,
 	},
@@ -689,7 +694,7 @@ static struct clk_rcg2 pdm2_clk_src = {
 	.freq_tbl = ftbl_pdm2_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "pdm2_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -711,7 +716,7 @@ static struct clk_rcg2 qspi_ser_clk_src = {
 	.freq_tbl = ftbl_qspi_ser_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "qspi_ser_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div_gpll1_gpll4_gpll1_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div_gpll1_gpll4_gpll1_early_div,
 		.num_parents = 6,
 		.ops = &clk_rcg2_ops,
 	},
@@ -737,7 +742,7 @@ static struct clk_rcg2 sdcc1_apps_clk_src = {
 	.freq_tbl = ftbl_sdcc1_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "sdcc1_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll4_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll4_gpll0_early_div,
 		.num_parents = 4,
 		.ops = &clk_rcg2_ops,
 	},
@@ -759,7 +764,7 @@ static struct clk_rcg2 sdcc1_ice_core_clk_src = {
 	.freq_tbl = ftbl_sdcc1_ice_core_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "sdcc1_ice_core_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -785,7 +790,7 @@ static struct clk_rcg2 sdcc2_apps_clk_src = {
 	.freq_tbl = ftbl_sdcc2_apps_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "sdcc2_apps_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div_gpll4,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div_gpll4,
 		.num_parents = 4,
 		.ops = &clk_rcg2_floor_ops,
 	},
@@ -808,7 +813,7 @@ static struct clk_rcg2 ufs_axi_clk_src = {
 	.freq_tbl = ftbl_ufs_axi_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "ufs_axi_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -829,7 +834,7 @@ static struct clk_rcg2 ufs_ice_core_clk_src = {
 	.freq_tbl = ftbl_ufs_ice_core_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "ufs_ice_core_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -843,7 +848,7 @@ static struct clk_rcg2 ufs_phy_aux_clk_src = {
 	.freq_tbl = ftbl_hmss_rbcpr_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "ufs_phy_aux_clk_src",
-		.parent_names = gcc_parent_names_xo_sleep_clk,
+		.parent_data = gcc_parent_data_xo_sleep_clk,
 		.num_parents = 2,
 		.ops = &clk_rcg2_ops,
 	},
@@ -864,7 +869,7 @@ static struct clk_rcg2 ufs_unipro_core_clk_src = {
 	.freq_tbl = ftbl_ufs_unipro_core_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "ufs_unipro_core_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -885,7 +890,7 @@ static struct clk_rcg2 usb20_master_clk_src = {
 	.freq_tbl = ftbl_usb20_master_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "usb20_master_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -905,7 +910,7 @@ static struct clk_rcg2 usb20_mock_utmi_clk_src = {
 	.freq_tbl = ftbl_usb20_mock_utmi_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "usb20_mock_utmi_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -930,7 +935,7 @@ static struct clk_rcg2 usb30_master_clk_src = {
 	.freq_tbl = ftbl_usb30_master_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "usb30_master_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -951,7 +956,7 @@ static struct clk_rcg2 usb30_mock_utmi_clk_src = {
 	.freq_tbl = ftbl_usb30_mock_utmi_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "usb30_mock_utmi_clk_src",
-		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
+		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
 		.num_parents = 3,
 		.ops = &clk_rcg2_ops,
 	},
@@ -971,7 +976,7 @@ static struct clk_rcg2 usb3_phy_aux_clk_src = {
 	.freq_tbl = ftbl_usb3_phy_aux_clk_src,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "usb3_phy_aux_clk_src",
-		.parent_names = gcc_parent_names_xo_sleep_clk,
+		.parent_data = gcc_parent_data_xo_sleep_clk,
 		.num_parents = 2,
 		.ops = &clk_rcg2_ops,
 	},
@@ -985,8 +990,8 @@ static struct clk_branch gcc_aggre2_ufs_axi_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_aggre2_ufs_axi_clk",
-			.parent_names = (const char *[]){
-				"ufs_axi_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&ufs_axi_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.ops = &clk_branch2_ops,
@@ -1002,8 +1007,8 @@ static struct clk_branch gcc_aggre2_usb3_axi_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_aggre2_usb3_axi_clk",
-			.parent_names = (const char *[]){
-				"usb30_master_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&usb30_master_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.ops = &clk_branch2_ops,
@@ -1071,8 +1076,8 @@ static struct clk_branch gcc_blsp1_qup1_i2c_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_qup1_i2c_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_qup1_i2c_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_qup1_i2c_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1089,8 +1094,8 @@ static struct clk_branch gcc_blsp1_qup1_spi_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_qup1_spi_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_qup1_spi_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_qup1_spi_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1107,8 +1112,8 @@ static struct clk_branch gcc_blsp1_qup2_i2c_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_qup2_i2c_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_qup2_i2c_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_qup2_i2c_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1125,8 +1130,8 @@ static struct clk_branch gcc_blsp1_qup2_spi_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_qup2_spi_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_qup2_spi_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_qup2_spi_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1143,8 +1148,8 @@ static struct clk_branch gcc_blsp1_qup3_i2c_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_qup3_i2c_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_qup3_i2c_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_qup3_i2c_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1161,8 +1166,8 @@ static struct clk_branch gcc_blsp1_qup3_spi_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_qup3_spi_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_qup3_spi_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_qup3_spi_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1179,8 +1184,8 @@ static struct clk_branch gcc_blsp1_qup4_i2c_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_qup4_i2c_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_qup4_i2c_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_qup4_i2c_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1197,8 +1202,8 @@ static struct clk_branch gcc_blsp1_qup4_spi_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_qup4_spi_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_qup4_spi_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_qup4_spi_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1215,8 +1220,8 @@ static struct clk_branch gcc_blsp1_uart1_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_uart1_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_uart1_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_uart1_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1233,8 +1238,8 @@ static struct clk_branch gcc_blsp1_uart2_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp1_uart2_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp1_uart2_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp1_uart2_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1264,8 +1269,8 @@ static struct clk_branch gcc_blsp2_qup1_i2c_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_qup1_i2c_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_qup1_i2c_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_qup1_i2c_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1282,8 +1287,8 @@ static struct clk_branch gcc_blsp2_qup1_spi_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_qup1_spi_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_qup1_spi_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_qup1_spi_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1300,8 +1305,8 @@ static struct clk_branch gcc_blsp2_qup2_i2c_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_qup2_i2c_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_qup2_i2c_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_qup2_i2c_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1318,8 +1323,8 @@ static struct clk_branch gcc_blsp2_qup2_spi_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_qup2_spi_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_qup2_spi_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_qup2_spi_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1336,8 +1341,8 @@ static struct clk_branch gcc_blsp2_qup3_i2c_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_qup3_i2c_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_qup3_i2c_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_qup3_i2c_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1354,8 +1359,8 @@ static struct clk_branch gcc_blsp2_qup3_spi_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_qup3_spi_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_qup3_spi_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_qup3_spi_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1372,8 +1377,8 @@ static struct clk_branch gcc_blsp2_qup4_i2c_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_qup4_i2c_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_qup4_i2c_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_qup4_i2c_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1390,8 +1395,8 @@ static struct clk_branch gcc_blsp2_qup4_spi_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_qup4_spi_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_qup4_spi_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_qup4_spi_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1408,8 +1413,8 @@ static struct clk_branch gcc_blsp2_uart1_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_uart1_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_uart1_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_uart1_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1426,8 +1431,8 @@ static struct clk_branch gcc_blsp2_uart2_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_blsp2_uart2_apps_clk",
-			.parent_names = (const char *[]){
-				"blsp2_uart2_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&blsp2_uart2_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1457,8 +1462,8 @@ static struct clk_branch gcc_cfg_noc_usb2_axi_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_cfg_noc_usb2_axi_clk",
-			.parent_names = (const char *[]){
-				"usb20_master_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&usb20_master_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.ops = &clk_branch2_ops,
@@ -1474,8 +1479,8 @@ static struct clk_branch gcc_cfg_noc_usb3_axi_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_cfg_noc_usb3_axi_clk",
-			.parent_names = (const char *[]){
-				"usb30_master_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&usb30_master_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.ops = &clk_branch2_ops,
@@ -1503,8 +1508,8 @@ static struct clk_branch gcc_gp1_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_gp1_clk",
-			.parent_names = (const char *[]){
-				"gp1_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gp1_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1521,8 +1526,8 @@ static struct clk_branch gcc_gp2_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_gp2_clk",
-			.parent_names = (const char *[]){
-				"gp2_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gp2_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1539,8 +1544,8 @@ static struct clk_branch gcc_gp3_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_gp3_clk",
-			.parent_names = (const char *[]){
-				"gp3_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gp3_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1584,8 +1589,8 @@ static struct clk_branch gcc_gpu_gpll0_clk = {
 		.enable_mask = BIT(4),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_gpu_gpll0_clk",
-			.parent_names = (const char *[]){
-				"gpll0",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gpll0.clkr.hw,
 			},
 			.num_parents = 1,
 			.ops = &clk_branch2_ops,
@@ -1601,8 +1606,8 @@ static struct clk_branch gcc_gpu_gpll0_div_clk = {
 		.enable_mask = BIT(3),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_gpu_gpll0_div_clk",
-			.parent_names = (const char *[]){
-				"gpll0_early_div",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gpll0_early_div.hw,
 			},
 			.num_parents = 1,
 			.ops = &clk_branch2_ops,
@@ -1632,8 +1637,8 @@ static struct clk_branch gcc_hmss_rbcpr_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_hmss_rbcpr_clk",
-			.parent_names = (const char *[]){
-				"hmss_rbcpr_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&hmss_rbcpr_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1650,8 +1655,8 @@ static struct clk_branch gcc_mmss_gpll0_clk = {
 		.enable_mask = BIT(1),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_mmss_gpll0_clk",
-			.parent_names = (const char *[]){
-				"gpll0",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gpll0.clkr.hw,
 			},
 			.num_parents = 1,
 			.ops = &clk_branch2_ops,
@@ -1667,8 +1672,8 @@ static struct clk_branch gcc_mmss_gpll0_div_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_mmss_gpll0_div_clk",
-			.parent_names = (const char *[]){
-				"gpll0_early_div",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gpll0_early_div.hw,
 			},
 			.num_parents = 1,
 			.ops = &clk_branch2_ops,
@@ -1767,8 +1772,8 @@ static struct clk_branch gcc_pdm2_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_pdm2_clk",
-			.parent_names = (const char *[]){
-				"pdm2_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&pdm2_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1824,8 +1829,8 @@ static struct clk_branch gcc_qspi_ser_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_qspi_ser_clk",
-			.parent_names = (const char *[]){
-				"qspi_ser_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&qspi_ser_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1881,8 +1886,8 @@ static struct clk_branch gcc_sdcc1_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_sdcc1_apps_clk",
-			.parent_names = (const char *[]){
-				"sdcc1_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&sdcc1_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1899,8 +1904,8 @@ static struct clk_branch gcc_sdcc1_ice_core_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_sdcc1_ice_core_clk",
-			.parent_names = (const char *[]){
-				"sdcc1_ice_core_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&sdcc1_ice_core_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1930,8 +1935,8 @@ static struct clk_branch gcc_sdcc2_apps_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_sdcc2_apps_clk",
-			.parent_names = (const char *[]){
-				"sdcc2_apps_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&sdcc2_apps_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1961,8 +1966,8 @@ static struct clk_branch gcc_ufs_axi_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_ufs_axi_clk",
-			.parent_names = (const char *[]){
-				"ufs_axi_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&ufs_axi_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -1992,8 +1997,8 @@ static struct clk_branch gcc_ufs_ice_core_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_ufs_ice_core_clk",
-			.parent_names = (const char *[]){
-				"ufs_ice_core_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&ufs_ice_core_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -2010,8 +2015,8 @@ static struct clk_branch gcc_ufs_phy_aux_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_ufs_phy_aux_clk",
-			.parent_names = (const char *[]){
-				"ufs_phy_aux_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&ufs_phy_aux_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -2067,8 +2072,8 @@ static struct clk_branch gcc_ufs_unipro_core_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_ufs_unipro_core_clk",
-			.parent_names = (const char *[]){
-				"ufs_unipro_core_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&ufs_unipro_core_clk_src.clkr.hw,
 			},
 			.flags = CLK_SET_RATE_PARENT,
 			.num_parents = 1,
@@ -2085,8 +2090,8 @@ static struct clk_branch gcc_usb20_master_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_usb20_master_clk",
-			.parent_names = (const char *[]){
-				"usb20_master_clk_src"
+			.parent_hws = (const struct clk_hw*[]) {
+				&usb20_master_clk_src.clkr.hw,
 			},
 			.flags = CLK_SET_RATE_PARENT,
 			.num_parents = 1,
@@ -2103,8 +2108,8 @@ static struct clk_branch gcc_usb20_mock_utmi_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_usb20_mock_utmi_clk",
-			.parent_names = (const char *[]){
-				"usb20_mock_utmi_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&usb20_mock_utmi_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -2134,8 +2139,8 @@ static struct clk_branch gcc_usb30_master_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_usb30_master_clk",
-			.parent_names = (const char *[]){
-				"usb30_master_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&usb30_master_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -2152,8 +2157,8 @@ static struct clk_branch gcc_usb30_mock_utmi_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_usb30_mock_utmi_clk",
-			.parent_names = (const char *[]){
-				"usb30_mock_utmi_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&usb30_mock_utmi_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -2196,8 +2201,8 @@ static struct clk_branch gcc_usb3_phy_aux_clk = {
 		.enable_mask = BIT(0),
 		.hw.init = &(struct clk_init_data){
 			.name = "gcc_usb3_phy_aux_clk",
-			.parent_names = (const char *[]){
-				"usb3_phy_aux_clk_src",
+			.parent_hws = (const struct clk_hw*[]) {
+				&usb3_phy_aux_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -2263,7 +2268,6 @@ static struct gdsc pcie_0_gdsc = {
 };
 
 static struct clk_hw *gcc_sdm660_hws[] = {
-	&xo.hw,
 	&gpll0_early_div.hw,
 	&gpll1_early_div.hw,
 };
-- 
2.29.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names
  2021-08-24 15:06 [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names Bjorn Andersson
@ 2021-08-24 20:46 ` Marijn Suijten
  2021-08-24 22:38   ` Bjorn Andersson
  0 siblings, 1 reply; 8+ messages in thread
From: Marijn Suijten @ 2021-08-24 20:46 UTC (permalink / raw)
  To: Bjorn Andersson, Michael Turquette, Stephen Boyd
  Cc: linux-arm-msm, linux-clk, linux-kernel, Konrad Dybcio,
	angelogioacchino.delregno

Hi Bjorn,

Thanks for this cleanup, that's needed and much appreciated!

On 8/24/21 5:06 PM, Bjorn Andersson wrote:
> Using parent_data and parent_hws, instead of parent_names, does protect
> against some cases of incompletely defined clock trees. While it turns
> out that the bug being chased this time was totally unrelated, this
> patch converts the SDM660 GCC driver to avoid such issues.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>


Tested-by: Marijn Suijten <marijn.suijten@somainline.org>

On the Sony Xperia XA2 Ultra, bar the necessary change in the 14NM DSI 
PHY driver commented below.

> [..]
> -
> -static struct clk_fixed_factor xo = {
> -	.mult = 1,
> -	.div = 1,
> -	.hw.init = &(struct clk_init_data){
> -		.name = "xo",
> -		.parent_names = (const char *[]){ "xo_board" },
> -		.num_parents = 1,
> -		.ops = &clk_fixed_factor_ops,
> -	},
> -};


Removing the global "xo" clock makes it so that our 14nm DSI PHY does 
not have a parent clock anymore, as the clock is called "xo_board" 
nowadays ("xo" in the position of fw_name is, as you know, only local to 
this driver because it is named that way in the clock-names property). 
We (SoMainline) suffer the same DSI PHY hardcoding issue on many other 
boards and are at this point investigating whether to provide &xo_board 
in DT like any other sane driver.  Do you happen to know if work is 
already underway to tackle this?

>   static struct clk_alpha_pll gpll0_early = {
>   	.offset = 0x0,
>   	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> @@ -158,7 +35,9 @@ static struct clk_alpha_pll gpll0_early = {
>   		.enable_mask = BIT(0),
>   		.hw.init = &(struct clk_init_data){
>   			.name = "gpll0_early",
> -			.parent_names = (const char *[]){ "xo" },
> +			.parent_data = &(const struct clk_parent_data){
> +				.fw_name = "xo",
> +			},


I wish we could use .parent_names for a list of .fw_name's too
> [..]
> @@ -265,7 +270,7 @@ static struct clk_rcg2 blsp1_qup1_i2c_apps_clk_src = {
>   	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
>   	.clkr.hw.init = &(struct clk_init_data){
>   		.name = "blsp1_qup1_i2c_apps_clk_src",
> -		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
> +		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
>   		.num_parents = 3,


How about using ARRAY_SIZE(gcc_parent_data_xo_gpll0_gpll0_early_div) 
now?  Same for every other occurrence of this pattern.

- Marijn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names
  2021-08-24 20:46 ` Marijn Suijten
@ 2021-08-24 22:38   ` Bjorn Andersson
  2021-08-25 10:39     ` AngeloGioacchino Del Regno
  2021-08-25 16:00     ` Marijn Suijten
  0 siblings, 2 replies; 8+ messages in thread
From: Bjorn Andersson @ 2021-08-24 22:38 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk,
	linux-kernel, Konrad Dybcio, angelogioacchino.delregno

On Tue 24 Aug 13:46 PDT 2021, Marijn Suijten wrote:

> Hi Bjorn,
> 
> Thanks for this cleanup, that's needed and much appreciated!
> 
> On 8/24/21 5:06 PM, Bjorn Andersson wrote:
> > Using parent_data and parent_hws, instead of parent_names, does protect
> > against some cases of incompletely defined clock trees. While it turns
> > out that the bug being chased this time was totally unrelated, this
> > patch converts the SDM660 GCC driver to avoid such issues.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> 
> Tested-by: Marijn Suijten <marijn.suijten@somainline.org>
> 
> On the Sony Xperia XA2 Ultra, bar the necessary change in the 14NM DSI PHY
> driver commented below.
> 
> > [..]
> > -
> > -static struct clk_fixed_factor xo = {
> > -	.mult = 1,
> > -	.div = 1,
> > -	.hw.init = &(struct clk_init_data){
> > -		.name = "xo",
> > -		.parent_names = (const char *[]){ "xo_board" },
> > -		.num_parents = 1,
> > -		.ops = &clk_fixed_factor_ops,
> > -	},
> > -};
> 
> 
> Removing the global "xo" clock makes it so that our 14nm DSI PHY does not
> have a parent clock anymore, as the clock is called "xo_board" nowadays
> ("xo" in the position of fw_name is, as you know, only local to this driver
> because it is named that way in the clock-names property). We (SoMainline)
> suffer the same DSI PHY hardcoding issue on many other boards and are at
> this point investigating whether to provide &xo_board in DT like any other
> sane driver.  Do you happen to know if work is already underway to tackle
> this?
> 

As far as I can tell most other platforms doesn't define "xo" either.
E.g. according to debugfs dsi0vco_clk doesn't have a parent on sdm845...

Sounds like we should update the dsi phys to specify a fw_name and
update binding and dts to provide this...


Does this cause a noticeable regression or it's just that we have a
dangling clock?

> >   static struct clk_alpha_pll gpll0_early = {
> >   	.offset = 0x0,
> >   	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> > @@ -158,7 +35,9 @@ static struct clk_alpha_pll gpll0_early = {
> >   		.enable_mask = BIT(0),
> >   		.hw.init = &(struct clk_init_data){
> >   			.name = "gpll0_early",
> > -			.parent_names = (const char *[]){ "xo" },
> > +			.parent_data = &(const struct clk_parent_data){
> > +				.fw_name = "xo",
> > +			},
> 
> 
> I wish we could use .parent_names for a list of .fw_name's too

Afaict specifying "name" in struct clk_parent_data is the same as using
parent_names. But I'm not up to speed on the details of how to migrate
the dsi phys.

> > [..]
> > @@ -265,7 +270,7 @@ static struct clk_rcg2 blsp1_qup1_i2c_apps_clk_src = {
> >   	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
> >   	.clkr.hw.init = &(struct clk_init_data){
> >   		.name = "blsp1_qup1_i2c_apps_clk_src",
> > -		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
> > +		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
> >   		.num_parents = 3,
> 
> 
> How about using ARRAY_SIZE(gcc_parent_data_xo_gpll0_gpll0_early_div) now?
> Same for every other occurrence of this pattern.
> 

I omitted that because it felt unrelated to the change I was doing, but
it could certainly be done.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names
  2021-08-24 22:38   ` Bjorn Andersson
@ 2021-08-25 10:39     ` AngeloGioacchino Del Regno
  2021-08-25 17:17       ` Bjorn Andersson
  2021-08-25 16:00     ` Marijn Suijten
  1 sibling, 1 reply; 8+ messages in thread
From: AngeloGioacchino Del Regno @ 2021-08-25 10:39 UTC (permalink / raw)
  To: Bjorn Andersson, Marijn Suijten
  Cc: Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk,
	linux-kernel, Konrad Dybcio

Il 25/08/21 00:38, Bjorn Andersson ha scritto:
> On Tue 24 Aug 13:46 PDT 2021, Marijn Suijten wrote:
> 
>> Hi Bjorn,
>>
>> Thanks for this cleanup, that's needed and much appreciated!
>>
>> On 8/24/21 5:06 PM, Bjorn Andersson wrote:
>>> Using parent_data and parent_hws, instead of parent_names, does protect
>>> against some cases of incompletely defined clock trees. While it turns
>>> out that the bug being chased this time was totally unrelated, this
>>> patch converts the SDM660 GCC driver to avoid such issues.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>
>>
>> Tested-by: Marijn Suijten <marijn.suijten@somainline.org>
>>
>> On the Sony Xperia XA2 Ultra, bar the necessary change in the 14NM DSI PHY
>> driver commented below.
>>
>>> [..]
>>> -
>>> -static struct clk_fixed_factor xo = {
>>> -	.mult = 1,
>>> -	.div = 1,
>>> -	.hw.init = &(struct clk_init_data){
>>> -		.name = "xo",
>>> -		.parent_names = (const char *[]){ "xo_board" },
>>> -		.num_parents = 1,
>>> -		.ops = &clk_fixed_factor_ops,
>>> -	},
>>> -};
>>
>>
>> Removing the global "xo" clock makes it so that our 14nm DSI PHY does not
>> have a parent clock anymore, as the clock is called "xo_board" nowadays
>> ("xo" in the position of fw_name is, as you know, only local to this driver
>> because it is named that way in the clock-names property). We (SoMainline)
>> suffer the same DSI PHY hardcoding issue on many other boards and are at
>> this point investigating whether to provide &xo_board in DT like any other
>> sane driver.  Do you happen to know if work is already underway to tackle
>> this?
>>
> 
> As far as I can tell most other platforms doesn't define "xo" either.
> E.g. according to debugfs dsi0vco_clk doesn't have a parent on sdm845...
> 
> Sounds like we should update the dsi phys to specify a fw_name and
> update binding and dts to provide this...
> 
> 
> Does this cause a noticeable regression or it's just that we have a
> dangling clock?
> 

Both, actually... but sincerely I would be more for updating the DSI PHY
drivers instead of keeping a "mock" crystal clock in there (since we do
always specify one in DT), also because, as Marijn pointed out and as I
can also confirm, we're seeing the same situation on multiple platforms.

That would allow us to solve the issue simply with DT, and would make us
able to switch platforms one by one to the RPM/RPMh XO in a perfect future
where we will be able to perform XO shutdown on selected platforms.

>>>    static struct clk_alpha_pll gpll0_early = {
>>>    	.offset = 0x0,
>>>    	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
>>> @@ -158,7 +35,9 @@ static struct clk_alpha_pll gpll0_early = {
>>>    		.enable_mask = BIT(0),
>>>    		.hw.init = &(struct clk_init_data){
>>>    			.name = "gpll0_early",
>>> -			.parent_names = (const char *[]){ "xo" },
>>> +			.parent_data = &(const struct clk_parent_data){
>>> +				.fw_name = "xo",
>>> +			},
>>
>>
>> I wish we could use .parent_names for a list of .fw_name's too
> 
> Afaict specifying "name" in struct clk_parent_data is the same as using
> parent_names. But I'm not up to speed on the details of how to migrate
> the dsi phys.
> 
>>> [..]
>>> @@ -265,7 +270,7 @@ static struct clk_rcg2 blsp1_qup1_i2c_apps_clk_src = {
>>>    	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
>>>    	.clkr.hw.init = &(struct clk_init_data){
>>>    		.name = "blsp1_qup1_i2c_apps_clk_src",
>>> -		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
>>> +		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
>>>    		.num_parents = 3,
>>
>>
>> How about using ARRAY_SIZE(gcc_parent_data_xo_gpll0_gpll0_early_div) now?
>> Same for every other occurrence of this pattern.
>>
> 
> I omitted that because it felt unrelated to the change I was doing, but
> it could certainly be done.
> 

Totally fair and I totally agree.

By the way,
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>

> Regards,
> Bjorn
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names
  2021-08-24 22:38   ` Bjorn Andersson
  2021-08-25 10:39     ` AngeloGioacchino Del Regno
@ 2021-08-25 16:00     ` Marijn Suijten
  2021-08-25 17:20       ` Bjorn Andersson
  1 sibling, 1 reply; 8+ messages in thread
From: Marijn Suijten @ 2021-08-25 16:00 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk,
	linux-kernel, Konrad Dybcio, angelogioacchino.delregno

Hi Bjorn,

On 8/25/21 12:38 AM, Bjorn Andersson wrote:
> On Tue 24 Aug 13:46 PDT 2021, Marijn Suijten wrote:
> 
>> Hi Bjorn,
>>
>> Thanks for this cleanup, that's needed and much appreciated!
>>
>> On 8/24/21 5:06 PM, Bjorn Andersson wrote:
>>> Using parent_data and parent_hws, instead of parent_names, does protect
>>> against some cases of incompletely defined clock trees. While it turns
>>> out that the bug being chased this time was totally unrelated, this
>>> patch converts the SDM660 GCC driver to avoid such issues.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>
>>
>> Tested-by: Marijn Suijten <marijn.suijten@somainline.org>
>>
>> On the Sony Xperia XA2 Ultra, bar the necessary change in the 14NM DSI PHY
>> driver commented below.
>>
>>> [..]
>>> -
>>> -static struct clk_fixed_factor xo = {
>>> -	.mult = 1,
>>> -	.div = 1,
>>> -	.hw.init = &(struct clk_init_data){
>>> -		.name = "xo",
>>> -		.parent_names = (const char *[]){ "xo_board" },
>>> -		.num_parents = 1,
>>> -		.ops = &clk_fixed_factor_ops,
>>> -	},
>>> -};
>>
>>
>> Removing the global "xo" clock makes it so that our 14nm DSI PHY does not
>> have a parent clock anymore, as the clock is called "xo_board" nowadays
>> ("xo" in the position of fw_name is, as you know, only local to this driver
>> because it is named that way in the clock-names property). We (SoMainline)
>> suffer the same DSI PHY hardcoding issue on many other boards and are at
>> this point investigating whether to provide &xo_board in DT like any other
>> sane driver.  Do you happen to know if work is already underway to tackle
>> this?
>>
> 
> As far as I can tell most other platforms doesn't define "xo" either.
> E.g. according to debugfs dsi0vco_clk doesn't have a parent on sdm845...
> 
> Sounds like we should update the dsi phys to specify a fw_name and
> update binding and dts to provide this...


I'm all for using .fw_name there, and I hope we all agree that clock 
dependencies based on global names should become a thing of the past; 
every such inter-driver dependency should be clearly visible in the DT. 
  We (SoMainline) can tackle this DSI side if no-one else is working on 
it yet.

> Does this cause a noticeable regression or it's just that we have a
> dangling clock?


Unfortunately this regresses yes, starting with:

     dsi0n1_postdiv_clk: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set

And proceeding with more such errors on different clocks, clocks getting 
stuck or failing to update, and the panel never showing anything at all.

Should we fix DSI PHYs first and let this patch sit for a while, or keep 
the implicit global "xo" clock just a little while longer until that's 
over with?

Either way, feel free to attach my:

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

After that.

>>>    static struct clk_alpha_pll gpll0_early = {
>>>    	.offset = 0x0,
>>>    	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
>>> @@ -158,7 +35,9 @@ static struct clk_alpha_pll gpll0_early = {
>>>    		.enable_mask = BIT(0),
>>>    		.hw.init = &(struct clk_init_data){
>>>    			.name = "gpll0_early",
>>> -			.parent_names = (const char *[]){ "xo" },
>>> +			.parent_data = &(const struct clk_parent_data){
>>> +				.fw_name = "xo",
>>> +			},
>>
>>
>> I wish we could use .parent_names for a list of .fw_name's too
> 
> Afaict specifying "name" in struct clk_parent_data is the same as using
> parent_names. But I'm not up to speed on the details of how to migrate
> the dsi phys.


Yes it is, both do _not_ look at clocks specified in DT before "falling 
back" to global names (that only happens when both .name and .fw_name 
are specified).  I'm sort of expressing the desire for .parent_fw_names 
here in hopes of phasing out global clock names on DT platforms 
altogether.  We definitely shouldn't rework .parent_names to support 
both, that only causes confusion and an implicit fallback to global 
clocks when the DT is under-specifying the required clocks is exactly 
what we're trying to avoid.

>>> [..]
>>> @@ -265,7 +270,7 @@ static struct clk_rcg2 blsp1_qup1_i2c_apps_clk_src = {
>>>    	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
>>>    	.clkr.hw.init = &(struct clk_init_data){
>>>    		.name = "blsp1_qup1_i2c_apps_clk_src",
>>> -		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
>>> +		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
>>>    		.num_parents = 3,
>>
>>
>> How about using ARRAY_SIZE(gcc_parent_data_xo_gpll0_gpll0_early_div) now?
>> Same for every other occurrence of this pattern.
>>
> 
> I omitted that because it felt unrelated to the change I was doing, but
> it could certainly be done.


Fair, if done at all it should end up in a separate (2/2) patch or I'll 
take care of this in a followup.

> Regards,
> Bjorn
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names
  2021-08-25 10:39     ` AngeloGioacchino Del Regno
@ 2021-08-25 17:17       ` Bjorn Andersson
  0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2021-08-25 17:17 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: Marijn Suijten, Michael Turquette, Stephen Boyd, linux-arm-msm,
	linux-clk, linux-kernel, Konrad Dybcio

On Wed 25 Aug 05:39 CDT 2021, AngeloGioacchino Del Regno wrote:

> Il 25/08/21 00:38, Bjorn Andersson ha scritto:
> > On Tue 24 Aug 13:46 PDT 2021, Marijn Suijten wrote:
> > 
> > > Hi Bjorn,
> > > 
> > > Thanks for this cleanup, that's needed and much appreciated!
> > > 
> > > On 8/24/21 5:06 PM, Bjorn Andersson wrote:
> > > > Using parent_data and parent_hws, instead of parent_names, does protect
> > > > against some cases of incompletely defined clock trees. While it turns
> > > > out that the bug being chased this time was totally unrelated, this
> > > > patch converts the SDM660 GCC driver to avoid such issues.
> > > > 
> > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > 
> > > 
> > > Tested-by: Marijn Suijten <marijn.suijten@somainline.org>
> > > 
> > > On the Sony Xperia XA2 Ultra, bar the necessary change in the 14NM DSI PHY
> > > driver commented below.
> > > 
> > > > [..]
> > > > -
> > > > -static struct clk_fixed_factor xo = {
> > > > -	.mult = 1,
> > > > -	.div = 1,
> > > > -	.hw.init = &(struct clk_init_data){
> > > > -		.name = "xo",
> > > > -		.parent_names = (const char *[]){ "xo_board" },
> > > > -		.num_parents = 1,
> > > > -		.ops = &clk_fixed_factor_ops,
> > > > -	},
> > > > -};
> > > 
> > > 
> > > Removing the global "xo" clock makes it so that our 14nm DSI PHY does not
> > > have a parent clock anymore, as the clock is called "xo_board" nowadays
> > > ("xo" in the position of fw_name is, as you know, only local to this driver
> > > because it is named that way in the clock-names property). We (SoMainline)
> > > suffer the same DSI PHY hardcoding issue on many other boards and are at
> > > this point investigating whether to provide &xo_board in DT like any other
> > > sane driver.  Do you happen to know if work is already underway to tackle
> > > this?
> > > 
> > 
> > As far as I can tell most other platforms doesn't define "xo" either.
> > E.g. according to debugfs dsi0vco_clk doesn't have a parent on sdm845...
> > 
> > Sounds like we should update the dsi phys to specify a fw_name and
> > update binding and dts to provide this...
> > 
> > 
> > Does this cause a noticeable regression or it's just that we have a
> > dangling clock?
> > 
> 
> Both, actually... but sincerely I would be more for updating the DSI PHY
> drivers instead of keeping a "mock" crystal clock in there (since we do
> always specify one in DT), also because, as Marijn pointed out and as I
> can also confirm, we're seeing the same situation on multiple platforms.
> 
> That would allow us to solve the issue simply with DT, and would make us
> able to switch platforms one by one to the RPM/RPMh XO in a perfect future
> where we will be able to perform XO shutdown on selected platforms.
> 

Fixing the DSI PHY to properly acquire the reference clock using
.fw_name is the right solution in the end. But afaict sdm845, sm8150 and
sm8250 doesn't have a "xo" clock.

So that's why I'm wondering if there's a functional regression caused by
this...and hence if I need to respin this patch with the clock
remaining.

> > > >    static struct clk_alpha_pll gpll0_early = {
> > > >    	.offset = 0x0,
> > > >    	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> > > > @@ -158,7 +35,9 @@ static struct clk_alpha_pll gpll0_early = {
> > > >    		.enable_mask = BIT(0),
> > > >    		.hw.init = &(struct clk_init_data){
> > > >    			.name = "gpll0_early",
> > > > -			.parent_names = (const char *[]){ "xo" },
> > > > +			.parent_data = &(const struct clk_parent_data){
> > > > +				.fw_name = "xo",
> > > > +			},
> > > 
> > > 
> > > I wish we could use .parent_names for a list of .fw_name's too
> > 
> > Afaict specifying "name" in struct clk_parent_data is the same as using
> > parent_names. But I'm not up to speed on the details of how to migrate
> > the dsi phys.
> > 
> > > > [..]
> > > > @@ -265,7 +270,7 @@ static struct clk_rcg2 blsp1_qup1_i2c_apps_clk_src = {
> > > >    	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
> > > >    	.clkr.hw.init = &(struct clk_init_data){
> > > >    		.name = "blsp1_qup1_i2c_apps_clk_src",
> > > > -		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
> > > > +		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
> > > >    		.num_parents = 3,
> > > 
> > > 
> > > How about using ARRAY_SIZE(gcc_parent_data_xo_gpll0_gpll0_early_div) now?
> > > Same for every other occurrence of this pattern.
> > > 
> > 
> > I omitted that because it felt unrelated to the change I was doing, but
> > it could certainly be done.
> > 
> 
> Totally fair and I totally agree.
> 
> By the way,
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> 

Thanks,
Bjorn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names
  2021-08-25 16:00     ` Marijn Suijten
@ 2021-08-25 17:20       ` Bjorn Andersson
  2021-08-25 17:55         ` Marijn Suijten
  0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Andersson @ 2021-08-25 17:20 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk,
	linux-kernel, Konrad Dybcio, angelogioacchino.delregno

On Wed 25 Aug 11:00 CDT 2021, Marijn Suijten wrote:

> Hi Bjorn,
> 
> On 8/25/21 12:38 AM, Bjorn Andersson wrote:
> > On Tue 24 Aug 13:46 PDT 2021, Marijn Suijten wrote:
> > 
> > > Hi Bjorn,
> > > 
> > > Thanks for this cleanup, that's needed and much appreciated!
> > > 
> > > On 8/24/21 5:06 PM, Bjorn Andersson wrote:
> > > > Using parent_data and parent_hws, instead of parent_names, does protect
> > > > against some cases of incompletely defined clock trees. While it turns
> > > > out that the bug being chased this time was totally unrelated, this
> > > > patch converts the SDM660 GCC driver to avoid such issues.
> > > > 
> > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > 
> > > 
> > > Tested-by: Marijn Suijten <marijn.suijten@somainline.org>
> > > 
> > > On the Sony Xperia XA2 Ultra, bar the necessary change in the 14NM DSI PHY
> > > driver commented below.
> > > 
> > > > [..]
> > > > -
> > > > -static struct clk_fixed_factor xo = {
> > > > -	.mult = 1,
> > > > -	.div = 1,
> > > > -	.hw.init = &(struct clk_init_data){
> > > > -		.name = "xo",
> > > > -		.parent_names = (const char *[]){ "xo_board" },
> > > > -		.num_parents = 1,
> > > > -		.ops = &clk_fixed_factor_ops,
> > > > -	},
> > > > -};
> > > 
> > > 
> > > Removing the global "xo" clock makes it so that our 14nm DSI PHY does not
> > > have a parent clock anymore, as the clock is called "xo_board" nowadays
> > > ("xo" in the position of fw_name is, as you know, only local to this driver
> > > because it is named that way in the clock-names property). We (SoMainline)
> > > suffer the same DSI PHY hardcoding issue on many other boards and are at
> > > this point investigating whether to provide &xo_board in DT like any other
> > > sane driver.  Do you happen to know if work is already underway to tackle
> > > this?
> > > 
> > 
> > As far as I can tell most other platforms doesn't define "xo" either.
> > E.g. according to debugfs dsi0vco_clk doesn't have a parent on sdm845...
> > 
> > Sounds like we should update the dsi phys to specify a fw_name and
> > update binding and dts to provide this...
> 
> 
> I'm all for using .fw_name there, and I hope we all agree that clock
> dependencies based on global names should become a thing of the past; every
> such inter-driver dependency should be clearly visible in the DT.  We
> (SoMainline) can tackle this DSI side if no-one else is working on it yet.
> 
> > Does this cause a noticeable regression or it's just that we have a
> > dangling clock?
> 
> 
> Unfortunately this regresses yes, starting with:
> 
>     dsi0n1_postdiv_clk: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set
> 
> And proceeding with more such errors on different clocks, clocks getting
> stuck or failing to update, and the panel never showing anything at all.
> 
> Should we fix DSI PHYs first and let this patch sit for a while, or keep the
> implicit global "xo" clock just a little while longer until that's over
> with?
> 

Thanks, should have read your email as well before replying to the
other.

I will respin this with "xo" intact, then once we've fixed up the DSI
code we can drop it.


But I still don't understand why we don't have this problem on e.g.
sdm845. I must be missing something...

> Either way, feel free to attach my:
> 
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> 
> After that.
> 
> > > >    static struct clk_alpha_pll gpll0_early = {
> > > >    	.offset = 0x0,
> > > >    	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> > > > @@ -158,7 +35,9 @@ static struct clk_alpha_pll gpll0_early = {
> > > >    		.enable_mask = BIT(0),
> > > >    		.hw.init = &(struct clk_init_data){
> > > >    			.name = "gpll0_early",
> > > > -			.parent_names = (const char *[]){ "xo" },
> > > > +			.parent_data = &(const struct clk_parent_data){
> > > > +				.fw_name = "xo",
> > > > +			},
> > > 
> > > 
> > > I wish we could use .parent_names for a list of .fw_name's too
> > 
> > Afaict specifying "name" in struct clk_parent_data is the same as using
> > parent_names. But I'm not up to speed on the details of how to migrate
> > the dsi phys.
> 
> 
> Yes it is, both do _not_ look at clocks specified in DT before "falling
> back" to global names (that only happens when both .name and .fw_name are
> specified).  I'm sort of expressing the desire for .parent_fw_names here in
> hopes of phasing out global clock names on DT platforms altogether.  We
> definitely shouldn't rework .parent_names to support both, that only causes
> confusion and an implicit fallback to global clocks when the DT is
> under-specifying the required clocks is exactly what we're trying to avoid.
> 
> > > > [..]
> > > > @@ -265,7 +270,7 @@ static struct clk_rcg2 blsp1_qup1_i2c_apps_clk_src = {
> > > >    	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
> > > >    	.clkr.hw.init = &(struct clk_init_data){
> > > >    		.name = "blsp1_qup1_i2c_apps_clk_src",
> > > > -		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
> > > > +		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
> > > >    		.num_parents = 3,
> > > 
> > > 
> > > How about using ARRAY_SIZE(gcc_parent_data_xo_gpll0_gpll0_early_div) now?
> > > Same for every other occurrence of this pattern.
> > > 
> > 
> > I omitted that because it felt unrelated to the change I was doing, but
> > it could certainly be done.
> 
> 
> Fair, if done at all it should end up in a separate (2/2) patch or I'll take
> care of this in a followup.
> 

Sounds good, I'd be happy to give you a review on that patch :)

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names
  2021-08-25 17:20       ` Bjorn Andersson
@ 2021-08-25 17:55         ` Marijn Suijten
  0 siblings, 0 replies; 8+ messages in thread
From: Marijn Suijten @ 2021-08-25 17:55 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Michael Turquette, Stephen Boyd, linux-arm-msm, linux-clk,
	linux-kernel, Konrad Dybcio, AngeloGioacchino Del Regno

Hi Bjorn,

On 8/25/21 7:20 PM, Bjorn Andersson wrote:
> On Wed 25 Aug 11:00 CDT 2021, Marijn Suijten wrote:
> 
>> Hi Bjorn,
>>
>> On 8/25/21 12:38 AM, Bjorn Andersson wrote:
>>> On Tue 24 Aug 13:46 PDT 2021, Marijn Suijten wrote:
>>>
>>>> Hi Bjorn,
>>>>
>>>> Thanks for this cleanup, that's needed and much appreciated!
>>>>
>>>> On 8/24/21 5:06 PM, Bjorn Andersson wrote:
>>>>> Using parent_data and parent_hws, instead of parent_names, does protect
>>>>> against some cases of incompletely defined clock trees. While it turns
>>>>> out that the bug being chased this time was totally unrelated, this
>>>>> patch converts the SDM660 GCC driver to avoid such issues.
>>>>>
>>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>>
>>>>
>>>> Tested-by: Marijn Suijten <marijn.suijten@somainline.org>
>>>>
>>>> On the Sony Xperia XA2 Ultra, bar the necessary change in the 14NM DSI PHY
>>>> driver commented below.
>>>>
>>>>> [..]
>>>>> -
>>>>> -static struct clk_fixed_factor xo = {
>>>>> -	.mult = 1,
>>>>> -	.div = 1,
>>>>> -	.hw.init = &(struct clk_init_data){
>>>>> -		.name = "xo",
>>>>> -		.parent_names = (const char *[]){ "xo_board" },
>>>>> -		.num_parents = 1,
>>>>> -		.ops = &clk_fixed_factor_ops,
>>>>> -	},
>>>>> -};
>>>>
>>>>
>>>> Removing the global "xo" clock makes it so that our 14nm DSI PHY does not
>>>> have a parent clock anymore, as the clock is called "xo_board" nowadays
>>>> ("xo" in the position of fw_name is, as you know, only local to this driver
>>>> because it is named that way in the clock-names property). We (SoMainline)
>>>> suffer the same DSI PHY hardcoding issue on many other boards and are at
>>>> this point investigating whether to provide &xo_board in DT like any other
>>>> sane driver.  Do you happen to know if work is already underway to tackle
>>>> this?
>>>>
>>>
>>> As far as I can tell most other platforms doesn't define "xo" either.
>>> E.g. according to debugfs dsi0vco_clk doesn't have a parent on sdm845...
>>>
>>> Sounds like we should update the dsi phys to specify a fw_name and
>>> update binding and dts to provide this...
>>
>>
>> I'm all for using .fw_name there, and I hope we all agree that clock
>> dependencies based on global names should become a thing of the past; every
>> such inter-driver dependency should be clearly visible in the DT.  We
>> (SoMainline) can tackle this DSI side if no-one else is working on it yet.
>>
>>> Does this cause a noticeable regression or it's just that we have a
>>> dangling clock?
>>
>>
>> Unfortunately this regresses yes, starting with:
>>
>>      dsi0n1_postdiv_clk: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set
>>
>> And proceeding with more such errors on different clocks, clocks getting
>> stuck or failing to update, and the panel never showing anything at all.
>>
>> Should we fix DSI PHYs first and let this patch sit for a while, or keep the
>> implicit global "xo" clock just a little while longer until that's over
>> with?
>>
> 
> Thanks, should have read your email as well before replying to the
> other.


No biggie, Angelo was first to reply after all :)

> I will respin this with "xo" intact, then once we've fixed up the DSI
> code we can drop it.


Sounds good, thanks!

> But I still don't understand why we don't have this problem on e.g.
> sdm845. I must be missing something...


It is strange indeed.  On MSM89[57]6 (Sony Xperia Loire, 28nm DSI 
PHY/PLL) we don't have this parent hooked up either because of the same 
issue, and it's not a problem there either.  I bet it all comes down to 
slight differences between the various DSI PHY/PLL implementations.

>> Either way, feel free to attach my:
>>
>> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>>
>> After that.
>>
>>>>>     static struct clk_alpha_pll gpll0_early = {
>>>>>     	.offset = 0x0,
>>>>>     	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
>>>>> @@ -158,7 +35,9 @@ static struct clk_alpha_pll gpll0_early = {
>>>>>     		.enable_mask = BIT(0),
>>>>>     		.hw.init = &(struct clk_init_data){
>>>>>     			.name = "gpll0_early",
>>>>> -			.parent_names = (const char *[]){ "xo" },
>>>>> +			.parent_data = &(const struct clk_parent_data){
>>>>> +				.fw_name = "xo",
>>>>> +			},
>>>>
>>>>
>>>> I wish we could use .parent_names for a list of .fw_name's too
>>>
>>> Afaict specifying "name" in struct clk_parent_data is the same as using
>>> parent_names. But I'm not up to speed on the details of how to migrate
>>> the dsi phys.
>>
>>
>> Yes it is, both do _not_ look at clocks specified in DT before "falling
>> back" to global names (that only happens when both .name and .fw_name are
>> specified).  I'm sort of expressing the desire for .parent_fw_names here in
>> hopes of phasing out global clock names on DT platforms altogether.  We
>> definitely shouldn't rework .parent_names to support both, that only causes
>> confusion and an implicit fallback to global clocks when the DT is
>> under-specifying the required clocks is exactly what we're trying to avoid.
>>
>>>>> [..]
>>>>> @@ -265,7 +270,7 @@ static struct clk_rcg2 blsp1_qup1_i2c_apps_clk_src = {
>>>>>     	.freq_tbl = ftbl_blsp1_qup1_i2c_apps_clk_src,
>>>>>     	.clkr.hw.init = &(struct clk_init_data){
>>>>>     		.name = "blsp1_qup1_i2c_apps_clk_src",
>>>>> -		.parent_names = gcc_parent_names_xo_gpll0_gpll0_early_div,
>>>>> +		.parent_data = gcc_parent_data_xo_gpll0_gpll0_early_div,
>>>>>     		.num_parents = 3,
>>>>
>>>>
>>>> How about using ARRAY_SIZE(gcc_parent_data_xo_gpll0_gpll0_early_div) now?
>>>> Same for every other occurrence of this pattern.
>>>>
>>>
>>> I omitted that because it felt unrelated to the change I was doing, but
>>> it could certainly be done.
>>
>>
>> Fair, if done at all it should end up in a separate (2/2) patch or I'll take
>> care of this in a followup.
>>
> 
> Sounds good, I'd be happy to give you a review on that patch :)


I'll spin that patch as soon as v2 is merged (preventing 
ordering/dependency issues?), feel free to CC me.

> Regards,
> Bjorn
> 

- Marijn

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-08-25 17:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 15:06 [PATCH] clk: qcom: gcc-sdm660: Replace usage of parent_names Bjorn Andersson
2021-08-24 20:46 ` Marijn Suijten
2021-08-24 22:38   ` Bjorn Andersson
2021-08-25 10:39     ` AngeloGioacchino Del Regno
2021-08-25 17:17       ` Bjorn Andersson
2021-08-25 16:00     ` Marijn Suijten
2021-08-25 17:20       ` Bjorn Andersson
2021-08-25 17:55         ` Marijn Suijten

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).