Re: [PATCH 12/26] clk: amlogic: s4-peripherals: naming consistency alignment
From: Chuan Liu
Date: Wed Jul 02 2025 - 23:18:45 EST
Hi Jerome:
On 7/2/2025 11:26 PM, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]
Amlogic clock controller drivers are all doing the same thing, more or
less. Yet, over the years, tiny (and often pointless) differences have
emerged.
This makes reviews more difficult, allows some errors to slip through and
make it more difficult to exploit SoC commonalities, leading to code
duplication.
This change enforce, wherever possible, a consistent and predictable scheme
when it comes to code organisation and naming, The scheme chosen is what
was used the most already, to try and minimise the size of the ugly
resulting diff. Here are some of the rules applied:
- Aligning clock names, variable names and IDs.
- ID cannot change (used in DT)
- Variable names w/ SoC name prefixes
- Clock names w/o SoC name prefixes, except pclks for historic reasons
- Composite clock systematic naming : mux: X_sel, div:X_div, gate:X
- Parent table systematically named with the same name as the clock and
a '_parents' suffix
- Group various tables next to the related clock
- etc ...
Doing so removes what would otherwise show up as unrelated diff in
following changes. It will allow to introduce common definitions for
peripheral clocks, probe helpers, composite clocks, etc ... making further
review and maintenance easier.
Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
---
drivers/clk/meson/s4-peripherals.c | 746 ++++++++++++++++++-------------------
1 file changed, 370 insertions(+), 376 deletions(-)
diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
index c9400cf54c84c3dc7c63d0636933951b0cac230c..9bcd35f12836de5e318fd1ad9c9ae15a2bfc3dd7 100644
--- a/drivers/clk/meson/s4-peripherals.c
+++ b/drivers/clk/meson/s4-peripherals.c
[...]
@@ -1320,7 +1320,7 @@ static struct clk_regmap s4_ts_clk_gate = {
* mux because it does top-to-bottom updates the each clock tree and
* switches to the "inactive" one when CLK_SET_RATE_GATE is set.
*/
-static const struct clk_parent_data s4_mali_0_1_parent_data[] = {
+static const struct clk_parent_data s4_mali_parents[] = {
{ .fw_name = "xtal", },
{ .fw_name = "gp0_pll", },
{ .fw_name = "hifi_pll", },
@@ -1340,8 +1340,8 @@ static struct clk_regmap s4_mali_0_sel = {
.hw.init = &(struct clk_init_data){
.name = "mali_0_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_mali_0_1_parent_data,
- .num_parents = ARRAY_SIZE(s4_mali_0_1_parent_data),
+ .parent_data = s4_mali_parents,
+ .num_parents = ARRAY_SIZE(s4_mali_parents),
/*
* Don't request the parent to change the rate because
* all GPU frequencies can be derived from the fclk_*
@@ -1394,8 +1394,8 @@ static struct clk_regmap s4_mali_1_sel = {
.hw.init = &(struct clk_init_data){
.name = "mali_1_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_mali_0_1_parent_data,
- .num_parents = ARRAY_SIZE(s4_mali_0_1_parent_data),
+ .parent_data = s4_mali_parents,
+ .num_parents = ARRAY_SIZE(s4_mali_parents),
.flags = 0,
},
};
@@ -1433,28 +1433,26 @@ static struct clk_regmap s4_mali_1 = {
},
};
-static const struct clk_hw *s4_mali_parent_hws[] = {
- &s4_mali_0.hw,
- &s4_mali_1.hw
-};
-
-static struct clk_regmap s4_mali_mux = {
+static struct clk_regmap s4_mali_sel = {
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_MALI_CLK_CTRL,
.mask = 1,
.shift = 31,
},
.hw.init = &(struct clk_init_data){
- .name = "mali",
+ .name = "mali_sel",
.ops = &clk_regmap_mux_ops,
- .parent_hws = s4_mali_parent_hws,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_mali_0.hw,
+ &s4_mali_1.hw,
+ },
.num_parents = 2,
.flags = CLK_SET_RATE_PARENT,
},
};
/* VDEC clocks */
-static const struct clk_parent_data s4_dec_parent_data[] = {
+static const struct clk_parent_data s4_dec_parents[] = {
{ .fw_name = "fclk_div2p5", },
{ .fw_name = "fclk_div3", },
{ .fw_name = "fclk_div4", },
@@ -1465,7 +1463,7 @@ static const struct clk_parent_data s4_dec_parent_data[] = {
{ .fw_name = "xtal", }
};
-static struct clk_regmap s4_vdec_p0_mux = {
+static struct clk_regmap s4_vdec_p0_sel = {
Since both vdec_clk and mali_clk are 'no glitch clock', should we also unify
the naming from 's4_vdec_p0'/'s4_vdec_p1' to 's4_vdec_0'/'s4_vdec_1'?
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VDEC_CLK_CTRL,
.mask = 0x7,
@@ -1473,10 +1471,10 @@ static struct clk_regmap s4_vdec_p0_mux = {
.flags = CLK_MUX_ROUND_CLOSEST,
},
.hw.init = &(struct clk_init_data) {
- .name = "vdec_p0_mux",
+ .name = "vdec_p0_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_dec_parent_data,
- .num_parents = ARRAY_SIZE(s4_dec_parent_data),
+ .parent_data = s4_dec_parents,
+ .num_parents = ARRAY_SIZE(s4_dec_parents),
.flags = 0,
},
};
@@ -1492,7 +1490,7 @@ static struct clk_regmap s4_vdec_p0_div = {
.name = "vdec_p0_div",
.ops = &clk_regmap_divider_ops,
.parent_hws = (const struct clk_hw *[]) {
- &s4_vdec_p0_mux.hw
+ &s4_vdec_p0_sel.hw
},
.num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
@@ -1515,7 +1513,7 @@ static struct clk_regmap s4_vdec_p0 = {
},
};
-static struct clk_regmap s4_vdec_p1_mux = {
+static struct clk_regmap s4_vdec_p1_sel = {
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VDEC3_CLK_CTRL,
.mask = 0x7,
@@ -1523,10 +1521,10 @@ static struct clk_regmap s4_vdec_p1_mux = {
.flags = CLK_MUX_ROUND_CLOSEST,
},
.hw.init = &(struct clk_init_data) {
- .name = "vdec_p1_mux",
+ .name = "vdec_p1_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_dec_parent_data,
- .num_parents = ARRAY_SIZE(s4_dec_parent_data),
+ .parent_data = s4_dec_parents,
+ .num_parents = ARRAY_SIZE(s4_dec_parents),
.flags = 0,
},
};
@@ -1542,7 +1540,7 @@ static struct clk_regmap s4_vdec_p1_div = {
.name = "vdec_p1_div",
.ops = &clk_regmap_divider_ops,
.parent_hws = (const struct clk_hw *[]) {
- &s4_vdec_p1_mux.hw
+ &s4_vdec_p1_sel.hw
},
.num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
@@ -1565,27 +1563,25 @@ static struct clk_regmap s4_vdec_p1 = {
},
};
-static const struct clk_hw *s4_vdec_mux_parent_hws[] = {
- &s4_vdec_p0.hw,
- &s4_vdec_p1.hw
-};
-
-static struct clk_regmap s4_vdec_mux = {
+static struct clk_regmap s4_vdec_sel = {
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VDEC3_CLK_CTRL,
.mask = 0x1,
.shift = 15,
},
.hw.init = &(struct clk_init_data) {
- .name = "vdec_mux",
+ .name = "vdec_sel",
.ops = &clk_regmap_mux_ops,
- .parent_hws = s4_vdec_mux_parent_hws,
- .num_parents = ARRAY_SIZE(s4_vdec_mux_parent_hws),
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_vdec_p0.hw,
+ &s4_vdec_p1.hw,
+ },
+ .num_parents = 2,
.flags = CLK_SET_RATE_PARENT,
},
};
-static struct clk_regmap s4_hevcf_p0_mux = {
+static struct clk_regmap s4_hevcf_p0_sel = {
+static struct clk_regmap s4_hevcf_0_sel
+static struct clk_regmap s4_hevcf_0_div
.
.
.
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VDEC2_CLK_CTRL,
.mask = 0x7,
@@ -1593,10 +1589,10 @@ static struct clk_regmap s4_hevcf_p0_mux = {
.flags = CLK_MUX_ROUND_CLOSEST,
},
.hw.init = &(struct clk_init_data) {
- .name = "hevcf_p0_mux",
+ .name = "hevcf_p0_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_dec_parent_data,
- .num_parents = ARRAY_SIZE(s4_dec_parent_data),
+ .parent_data = s4_dec_parents,
+ .num_parents = ARRAY_SIZE(s4_dec_parents),
.flags = 0,
},
};
@@ -1612,7 +1608,7 @@ static struct clk_regmap s4_hevcf_p0_div = {
.name = "hevcf_p0_div",
.ops = &clk_regmap_divider_ops,
.parent_hws = (const struct clk_hw *[]) {
- &s4_hevcf_p0_mux.hw
+ &s4_hevcf_p0_sel.hw
},
.num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
@@ -1625,7 +1621,7 @@ static struct clk_regmap s4_hevcf_p0 = {
.bit_idx = 8,
},
.hw.init = &(struct clk_init_data){
- .name = "hevcf_p0_gate",
+ .name = "hevcf_p0",
.ops = &clk_regmap_gate_ops,
.parent_hws = (const struct clk_hw *[]) {
&s4_hevcf_p0_div.hw
@@ -1635,7 +1631,7 @@ static struct clk_regmap s4_hevcf_p0 = {
},
};
-static struct clk_regmap s4_hevcf_p1_mux = {
+static struct clk_regmap s4_hevcf_p1_sel = {
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VDEC4_CLK_CTRL,
.mask = 0x7,
@@ -1643,10 +1639,10 @@ static struct clk_regmap s4_hevcf_p1_mux = {
.flags = CLK_MUX_ROUND_CLOSEST,
},
.hw.init = &(struct clk_init_data) {
- .name = "hevcf_p1_mux",
+ .name = "hevcf_p1_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_dec_parent_data,
- .num_parents = ARRAY_SIZE(s4_dec_parent_data),
+ .parent_data = s4_dec_parents,
+ .num_parents = ARRAY_SIZE(s4_dec_parents),
.flags = 0,
},
};
@@ -1662,7 +1658,7 @@ static struct clk_regmap s4_hevcf_p1_div = {
.name = "hevcf_p1_div",
.ops = &clk_regmap_divider_ops,
.parent_hws = (const struct clk_hw *[]) {
- &s4_hevcf_p1_mux.hw
+ &s4_hevcf_p1_sel.hw
},
.num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
@@ -1685,28 +1681,26 @@ static struct clk_regmap s4_hevcf_p1 = {
},
};
-static const struct clk_hw *s4_hevcf_mux_parent_hws[] = {
- &s4_hevcf_p0.hw,
- &s4_hevcf_p1.hw
-};
-
-static struct clk_regmap s4_hevcf_mux = {
+static struct clk_regmap s4_hevcf_sel = {
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VDEC4_CLK_CTRL,
.mask = 0x1,
.shift = 15,
},
.hw.init = &(struct clk_init_data) {
- .name = "hevcf",
+ .name = "hevcf_sel",
.ops = &clk_regmap_mux_ops,
- .parent_hws = s4_hevcf_mux_parent_hws,
- .num_parents = ARRAY_SIZE(s4_hevcf_mux_parent_hws),
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_hevcf_p0.hw,
+ &s4_hevcf_p1.hw,
+ },
+ .num_parents = 2,
.flags = CLK_SET_RATE_PARENT,
},
};
/* VPU Clock */
-static const struct clk_parent_data s4_vpu_parent_data[] = {
+static const struct clk_parent_data s4_vpu_parents[] = {
{ .fw_name = "fclk_div3", },
{ .fw_name = "fclk_div4", },
{ .fw_name = "fclk_div5", },
@@ -1726,8 +1720,8 @@ static struct clk_regmap s4_vpu_0_sel = {
.hw.init = &(struct clk_init_data){
.name = "vpu_0_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_vpu_parent_data,
- .num_parents = ARRAY_SIZE(s4_vpu_parent_data),
+ .parent_data = s4_vpu_parents,
+ .num_parents = ARRAY_SIZE(s4_vpu_parents),
.flags = 0,
},
};
@@ -1770,8 +1764,8 @@ static struct clk_regmap s4_vpu_1_sel = {
.hw.init = &(struct clk_init_data){
.name = "vpu_1_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_vpu_parent_data,
- .num_parents = ARRAY_SIZE(s4_vpu_parent_data),
+ .parent_data = s4_vpu_parents,
+ .num_parents = ARRAY_SIZE(s4_vpu_parents),
.flags = 0,
},
};
@@ -1823,24 +1817,24 @@ static struct clk_regmap s4_vpu = {
},
};
-static const struct clk_parent_data vpu_clkb_tmp_parent_data[] = {
+static const struct clk_parent_data vpu_clkb_tmp_parents[] = {
{ .hw = &s4_vpu.hw },
{ .fw_name = "fclk_div4", },
{ .fw_name = "fclk_div5", },
{ .fw_name = "fclk_div7", }
};
-static struct clk_regmap s4_vpu_clkb_tmp_mux = {
+static struct clk_regmap s4_vpu_clkb_tmp_sel = {
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VPU_CLKB_CTRL,
.mask = 0x3,
.shift = 20,
},
.hw.init = &(struct clk_init_data) {
- .name = "vpu_clkb_tmp_mux",
+ .name = "vpu_clkb_tmp_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = vpu_clkb_tmp_parent_data,
- .num_parents = ARRAY_SIZE(vpu_clkb_tmp_parent_data),
+ .parent_data = vpu_clkb_tmp_parents,
+ .num_parents = ARRAY_SIZE(vpu_clkb_tmp_parents),
.flags = CLK_SET_RATE_PARENT,
},
};
@@ -1855,7 +1849,7 @@ static struct clk_regmap s4_vpu_clkb_tmp_div = {
.name = "vpu_clkb_tmp_div",
.ops = &clk_regmap_divider_ops,
.parent_hws = (const struct clk_hw *[]) {
- &s4_vpu_clkb_tmp_mux.hw
+ &s4_vpu_clkb_tmp_sel.hw
},
.num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
@@ -1911,7 +1905,7 @@ static struct clk_regmap s4_vpu_clkb = {
},
};
-static const struct clk_parent_data s4_vpu_clkc_parent_data[] = {
+static const struct clk_parent_data s4_vpu_clkc_parents[] = {
{ .fw_name = "fclk_div4", },
{ .fw_name = "fclk_div3", },
{ .fw_name = "fclk_div5", },
@@ -1922,17 +1916,17 @@ static const struct clk_parent_data s4_vpu_clkc_parent_data[] = {
{ .fw_name = "gp0_pll", },
};
-static struct clk_regmap s4_vpu_clkc_p0_mux = {
+static struct clk_regmap s4_vpu_clkc_p0_sel = {
+static struct clk_regmap s4_vpu_clkc_0_sel
+static struct clk_regmap s4_vpu_clkc_0_div
.
.
.
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VPU_CLKC_CTRL,
.mask = 0x7,
.shift = 9,
},
.hw.init = &(struct clk_init_data) {
- .name = "vpu_clkc_p0_mux",
+ .name = "vpu_clkc_p0_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_vpu_clkc_parent_data,
- .num_parents = ARRAY_SIZE(s4_vpu_clkc_parent_data),
+ .parent_data = s4_vpu_clkc_parents,
+ .num_parents = ARRAY_SIZE(s4_vpu_clkc_parents),
.flags = 0,
},
};
@@ -1947,7 +1941,7 @@ static struct clk_regmap s4_vpu_clkc_p0_div = {
.name = "vpu_clkc_p0_div",
.ops = &clk_regmap_divider_ops,
.parent_hws = (const struct clk_hw *[]) {
- &s4_vpu_clkc_p0_mux.hw
+ &s4_vpu_clkc_p0_sel.hw
},
.num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
@@ -1970,17 +1964,17 @@ static struct clk_regmap s4_vpu_clkc_p0 = {
},
};
-static struct clk_regmap s4_vpu_clkc_p1_mux = {
+static struct clk_regmap s4_vpu_clkc_p1_sel = {
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VPU_CLKC_CTRL,
.mask = 0x7,
.shift = 25,
},
.hw.init = &(struct clk_init_data) {
- .name = "vpu_clkc_p1_mux",
+ .name = "vpu_clkc_p1_sel",
.ops = &clk_regmap_mux_ops,
- .parent_data = s4_vpu_clkc_parent_data,
- .num_parents = ARRAY_SIZE(s4_vpu_clkc_parent_data),
+ .parent_data = s4_vpu_clkc_parents,
+ .num_parents = ARRAY_SIZE(s4_vpu_clkc_parents),
.flags = 0,
},
};
@@ -1995,7 +1989,7 @@ static struct clk_regmap s4_vpu_clkc_p1_div = {
.name = "vpu_clkc_p1_div",
.ops = &clk_regmap_divider_ops,
.parent_hws = (const struct clk_hw *[]) {
- &s4_vpu_clkc_p1_mux.hw
+ &s4_vpu_clkc_p1_sel.hw
},
.num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
@@ -2018,28 +2012,26 @@ static struct clk_regmap s4_vpu_clkc_p1 = {
},
};
-static const struct clk_hw *s4_vpu_mux_parent_hws[] = {
- &s4_vpu_clkc_p0.hw,
- &s4_vpu_clkc_p1.hw
-};
-
-static struct clk_regmap s4_vpu_clkc_mux = {
+static struct clk_regmap s4_vpu_clkc_sel = {
.data = &(struct clk_regmap_mux_data){
.offset = CLKCTRL_VPU_CLKC_CTRL,
.mask = 0x1,
.shift = 31,
},
.hw.init = &(struct clk_init_data) {
- .name = "vpu_clkc_mux",
+ .name = "vpu_clkc_sel",
.ops = &clk_regmap_mux_ops,
- .parent_hws = s4_vpu_mux_parent_hws,
- .num_parents = ARRAY_SIZE(s4_vpu_mux_parent_hws),
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_vpu_clkc_p0.hw,
+ &s4_vpu_clkc_p1.hw,
+ },
+ .num_parents = 2,
.flags = CLK_SET_RATE_PARENT,
},
};
[...]
MODULE_DESCRIPTION("Amlogic S4 Peripherals Clock Controller driver");
MODULE_AUTHOR("Yu Tu <yu.tu@xxxxxxxxxxx>");
--
2.47.2
_______________________________________________
linux-amlogic mailing list
linux-amlogic@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-amlogic