[PATCH 13/26] clk: amlogic: s4-pll: naming consistency alignment
Chuan Liu
chuan.liu at amlogic.com
Wed Jul 2 20:19:49 PDT 2025
Hi Jerome:
It's good for me. Thanks!
Reviewed-by: Chuan Liu <chuan.liu at amlogic.com>
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 at baylibre.com>
> ---
> 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 at amlogic.com>");
>
> --
> 2.47.2
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
More information about the linux-amlogic
mailing list