Re: [PATCH 13/26] clk: amlogic: s4-pll: naming consistency alignment
From: Chuan Liu
Date: Wed Jul 02 2025 - 23:20:26 EST
Hi Jerome:
It's good for me. Thanks!
Reviewed-by: Chuan Liu <chuan.liu@xxxxxxxxxxx>
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-pll.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/clk/meson/s4-pll.c b/drivers/clk/meson/s4-pll.c
index 3d689d2f003e21658016b84918bf1b7b1954c05a..6a266bcafd6257937c1de50cbc5606dcc6f8207b 100644
--- a/drivers/clk/meson/s4-pll.c
+++ b/drivers/clk/meson/s4-pll.c
@@ -281,7 +281,7 @@ static const struct pll_mult_range s4_gp0_pll_mult_range = {
/*
* Internal gp0 pll emulation configuration parameters
*/
-static const struct reg_sequence s4_gp0_init_regs[] = {
+static const struct reg_sequence s4_gp0_pll_init_regs[] = {
{ .reg = ANACTRL_GP0PLL_CTRL1, .def = 0x00000000 },
{ .reg = ANACTRL_GP0PLL_CTRL2, .def = 0x00000000 },
{ .reg = ANACTRL_GP0PLL_CTRL3, .def = 0x48681c00 },
@@ -318,8 +318,8 @@ static struct clk_regmap s4_gp0_pll_dco = {
.width = 1,
},
.range = &s4_gp0_pll_mult_range,
- .init_regs = s4_gp0_init_regs,
- .init_count = ARRAY_SIZE(s4_gp0_init_regs),
+ .init_regs = s4_gp0_pll_init_regs,
+ .init_count = ARRAY_SIZE(s4_gp0_pll_init_regs),
},
.hw.init = &(struct clk_init_data){
.name = "gp0_pll_dco",
@@ -353,7 +353,7 @@ static struct clk_regmap s4_gp0_pll = {
/*
* Internal hifi pll emulation configuration parameters
*/
-static const struct reg_sequence s4_hifi_init_regs[] = {
+static const struct reg_sequence s4_hifi_pll_init_regs[] = {
{ .reg = ANACTRL_HIFIPLL_CTRL2, .def = 0x00000000 },
{ .reg = ANACTRL_HIFIPLL_CTRL3, .def = 0x6a285c00 },
{ .reg = ANACTRL_HIFIPLL_CTRL4, .def = 0x65771290 },
@@ -394,8 +394,8 @@ static struct clk_regmap s4_hifi_pll_dco = {
.width = 1,
},
.range = &s4_gp0_pll_mult_range,
- .init_regs = s4_hifi_init_regs,
- .init_count = ARRAY_SIZE(s4_hifi_init_regs),
+ .init_regs = s4_hifi_pll_init_regs,
+ .init_count = ARRAY_SIZE(s4_hifi_pll_init_regs),
.frac_max = 100000,
.flags = CLK_MESON_PLL_ROUND_CLOSEST,
},
@@ -794,11 +794,11 @@ static struct clk_hw *s4_pll_hw_clks[] = {
[CLKID_MPLL3] = &s4_mpll3.hw,
};
-static const struct reg_sequence s4_init_regs[] = {
+static const struct reg_sequence s4_pll_init_regs[] = {
{ .reg = ANACTRL_MPLL_CTRL0, .def = 0x00000543 },
};
-static const struct regmap_config clkc_regmap_config = {
+static const struct regmap_config s4_pll_clkc_regmap_cfg = {
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
@@ -810,7 +810,7 @@ static struct meson_clk_hw_data s4_pll_clks = {
.num = ARRAY_SIZE(s4_pll_hw_clks),
};
-static int meson_s4_pll_probe(struct platform_device *pdev)
+static int s4_pll_clkc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct regmap *regmap;
@@ -822,12 +822,12 @@ static int meson_s4_pll_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(base),
"can't ioremap resource\n");
- regmap = devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
+ regmap = devm_regmap_init_mmio(dev, base, &s4_pll_clkc_regmap_cfg);
if (IS_ERR(regmap))
return dev_err_probe(dev, PTR_ERR(regmap),
"can't init regmap mmio region\n");
- ret = regmap_multi_reg_write(regmap, s4_init_regs, ARRAY_SIZE(s4_init_regs));
+ ret = regmap_multi_reg_write(regmap, s4_pll_init_regs, ARRAY_SIZE(s4_pll_init_regs));
if (ret)
return dev_err_probe(dev, ret,
"Failed to init registers\n");
@@ -848,22 +848,22 @@ static int meson_s4_pll_probe(struct platform_device *pdev)
&s4_pll_clks);
}
-static const struct of_device_id clkc_match_table[] = {
+static const struct of_device_id s4_pll_clkc_match_table[] = {
{
.compatible = "amlogic,s4-pll-clkc",
},
{}
};
-MODULE_DEVICE_TABLE(of, clkc_match_table);
+MODULE_DEVICE_TABLE(of, s4_pll_clkc_match_table);
-static struct platform_driver s4_driver = {
- .probe = meson_s4_pll_probe,
+static struct platform_driver s4_pll_clkc_driver = {
+ .probe = s4_pll_clkc_probe,
.driver = {
.name = "s4-pll-clkc",
- .of_match_table = clkc_match_table,
+ .of_match_table = s4_pll_clkc_match_table,
},
};
-module_platform_driver(s4_driver);
+module_platform_driver(s4_pll_clkc_driver);
MODULE_DESCRIPTION("Amlogic S4 PLL 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