Re: [PATCH 16/26] clk: amlogic: add probe helper for mmio based controllers

[Chuan Liu <chuan.liu@amlogic.com>]

Hi Jerome:


On 7/2/2025 11:26 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> Add a 2nd probe function helper for mmio based controllers, which
> are getting the memory region from a resource instead of a syscon.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>   drivers/clk/meson/meson-clkc-utils.c | 65 ++++++++++++++++++++++++++++--------
>   drivers/clk/meson/meson-clkc-utils.h |  1 +
>   2 files changed, 53 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/meson/meson-clkc-utils.c b/drivers/clk/meson/meson-clkc-utils.c
> index 49f562d0f203b9a7d15b5119100216564c10cb21..272b2dd8c95e73e6c021cbf8852dd64733fa00e0 100644
> --- a/drivers/clk/meson/meson-clkc-utils.c
> +++ b/drivers/clk/meson/meson-clkc-utils.c
> @@ -26,12 +26,9 @@ struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_da
>   }
>   EXPORT_SYMBOL_NS_GPL(meson_clk_hw_get, "CLK_MESON");
>
> -int meson_clkc_syscon_probe(struct platform_device *pdev)
> +static int meson_clkc_init(struct device *dev, struct regmap *map)
>   {
>          const struct meson_clkc_data *data;
> -       struct device *dev = &pdev->dev;
> -       struct device_node *np;
> -       struct regmap *map;
>          struct clk_hw *hw;
>          int ret, i;
>
> @@ -39,15 +36,6 @@ int meson_clkc_syscon_probe(struct platform_device *pdev)
>          if (!data)
>                  return -EINVAL;
>
> -       np = of_get_parent(dev->of_node);
> -       map = syscon_node_to_regmap(np);
> -       of_node_put(np);
> -       if (IS_ERR(map)) {
> -               dev_err(dev,
> -                       "failed to get parent syscon regmap\n");
> -               return PTR_ERR(map);
> -       }
> -
>          if (data->init_count)
>                  regmap_multi_reg_write(map, data->init_regs, data->init_count);
>
> @@ -68,8 +56,59 @@ int meson_clkc_syscon_probe(struct platform_device *pdev)
>
>          return devm_of_clk_add_hw_provider(dev, meson_clk_hw_get, (void *)&data->hw_clks);
>   }
> +
> +int meson_clkc_syscon_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np;
> +       struct regmap *map;
> +
> +       np = of_get_parent(dev->of_node);
> +       map = syscon_node_to_regmap(np);
> +       of_node_put(np);
> +       if (IS_ERR(map)) {
> +               dev_err(dev, "failed to get parent syscon regmap\n");
> +               return PTR_ERR(map);
> +       }
> +
> +       return meson_clkc_init(dev, map);
> +}
>   EXPORT_SYMBOL_NS_GPL(meson_clkc_syscon_probe, "CLK_MESON");
>
> +static const struct regmap_config base_clkc_regmap_cfg = {
> +       .reg_bits       = 32,
> +       .val_bits       = 32,
> +       .reg_stride     = 4,
> +};
> +


Since 'base_clkc_regmap_cfg' is only referenced within 
'meson_clkc_mmio_probe()',
we should move it as a local variable inside the function. This would be 
more
logical and may optimize code size (During compiler optimization, only 
critical
data needs to be preserved rather than the entire structure?)


> +int meson_clkc_mmio_probe(struct platform_device *pdev)
> +{
> +       const struct meson_clkc_data *data;
> +       struct device *dev = &pdev->dev;
> +       struct regmap_config regmap_cfg;
> +       struct resource *res;
> +       void __iomem *base;
> +       struct regmap *map;
> +
> +       data = of_device_get_match_data(dev);
> +       if (!data)
> +               return -EINVAL;
> +
> +       base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
> +
> +       memcpy(&regmap_cfg, &base_clkc_regmap_cfg, sizeof(regmap_cfg));
> +       regmap_cfg.max_register = resource_size(res) - 4;
> +
> +       map = devm_regmap_init_mmio(dev, base, &regmap_cfg);
> +       if (IS_ERR(map))
> +               return PTR_ERR(map);
> +
> +       return meson_clkc_init(dev, map);
> +}
> +EXPORT_SYMBOL_NS_GPL(meson_clkc_mmio_probe, "CLK_MESON");
> +
>   MODULE_DESCRIPTION("Amlogic Clock Controller Utilities");
>   MODULE_LICENSE("GPL");
>   MODULE_IMPORT_NS("CLK_MESON");
> diff --git a/drivers/clk/meson/meson-clkc-utils.h b/drivers/clk/meson/meson-clkc-utils.h
> index 26cd47544302b28ca1a342e178956559a84b152a..b45f85f630d7190fb6509b088f05f17ca91fa1c8 100644
> --- a/drivers/clk/meson/meson-clkc-utils.h
> +++ b/drivers/clk/meson/meson-clkc-utils.h
> @@ -25,5 +25,6 @@ struct meson_clkc_data {
>   };
>
>   int meson_clkc_syscon_probe(struct platform_device *pdev);
> +int meson_clkc_mmio_probe(struct platform_device *pdev);
>
>   #endif
>
> --
> 2.47.2
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic