Hi Changhuang,
> The StarFive JHB100 SoC has several clock and reset generators (CRGs)
> which share the same probe flow: map the registers, register all the
> clocks described by a table, add the clock provider and register the
> auxiliary reset device.
>
> Add jh71x0_crg_probe() together with struct jh71x0_crg_domain_info so
> that each CRG driver only needs to provide its clock table, external
> clock names and reset auxiliary device name. The reset auxiliary device
> is registered with jh71x0_reset_controller_register().
>
> The clock index space of these CRGs is sparse, so also make
> jh71x0_clk_get() reject indexes that were never registered when the
> domain info is available.
>
> Signed-off-by: Changhuang Liang <[email protected]>
>
> diff --git a/drivers/clk/starfive/clk-starfive-jh71x0.c
> b/drivers/clk/starfive/clk-starfive-jh71x0.c
> index 2cc71f49ddee..bb7706ede1a6 100644
> --- a/drivers/clk/starfive/clk-starfive-jh71x0.c
> +++ b/drivers/clk/starfive/clk-starfive-jh71x0.c
> @@ -10,6 +10,7 @@
> #include <linux/debugfs.h>
> #include <linux/device.h>
> #include <linux/io.h>
> +#include <linux/pm_runtime.h>
Add #include <linux/property.h> for device_get_match_data()
> #include <linux/slab.h>
> #include <soc/starfive/reset-starfive-jh71x0.h>
>
> @@ -334,10 +335,14 @@ struct clk_hw *jh71x0_clk_get(struct of_phandle_args
> *clkspec, void *data)
> struct jh71x0_clk_priv *priv = data;
> unsigned int idx = clkspec->args[0];
>
> - if (idx < priv->num_reg)
> - return &priv->reg[idx].hw;
> + if (idx >= priv->num_reg)
> + return ERR_PTR(-EINVAL);
>
> - return ERR_PTR(-EINVAL);
> + /* Index space is sparse: reject holes that were never registered. */
> + if (priv->info && !priv->info->clk_data[idx].name)
> + return ERR_PTR(-ENOENT);
> +
> + return &priv->reg[idx].hw;
> }
> EXPORT_SYMBOL_GPL(jh71x0_clk_get);
>
> @@ -391,3 +396,87 @@ int jh71x0_reset_controller_register(struct
> jh71x0_clk_priv *priv,
> jh71x0_reset_unregister_adev, adev);
> }
> EXPORT_SYMBOL_GPL(jh71x0_reset_controller_register);
> +
> +int jh71x0_crg_probe(struct platform_device *pdev)
> +{
> + const struct jh71x0_crg_domain_info *info;
> + struct jh71x0_clk_priv *priv;
> + unsigned int idx;
> + int ret;
> +
> + info = device_get_match_data(&pdev->dev);
> + if (!info)
> + return -ENODEV;
> +
> + priv = devm_kzalloc(&pdev->dev, struct_size(priv, reg, info->num_clk),
> + GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + spin_lock_init(&priv->rmw_lock);
> + priv->info = info;
> + priv->num_reg = info->num_clk;
> + priv->dev = &pdev->dev;
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(priv->base))
> + return PTR_ERR(priv->base);
> +
> + if (info->power_domain) {
> + ret = devm_pm_runtime_enable(priv->dev);
> + if (ret)
> + return dev_err_probe(priv->dev, ret,
> + "failed to enable runtime PM\n");
> + }
> +
> + for (idx = 0; idx < info->num_clk; idx++) {
> + u32 max = info->clk_data[idx].max;
> + struct clk_parent_data parents[4] = {};
> + struct clk_init_data init = {
> + .name = info->clk_data[idx].name,
> + .ops = starfive_jh71x0_clk_ops(max),
> + .parent_data = parents,
> + .num_parents =
> + ((max & JH71X0_CLK_MUX_MASK) >>
> JH71X0_CLK_MUX_SHIFT) + 1,
> + .flags = info->clk_data[idx].flags,
> + };
> + struct jh71x0_clk *clk = &priv->reg[idx];
> + unsigned int i;
> +
> + if (!init.name)
> + continue;
> +
> + if (init.num_parents > ARRAY_SIZE(parents))
> + return dev_err_probe(priv->dev, -EINVAL,
> + "clock %s: too many parents (%u >
> %zu)\n",
> + init.name, init.num_parents,
> ARRAY_SIZE(parents));
> +
> + for (i = 0; i < init.num_parents; i++) {
> + unsigned int pidx = info->clk_data[idx].parents[i];
> +
> + if (pidx < info->num_clk) {
> + parents[i].hw = &priv->reg[pidx].hw;
> + } else {
> + if (pidx - info->num_clk >= info->num_ext_clk)
> + return -EINVAL;
What do you think about adding a dev_err_probe() here as well to make it
easier to troubleshoot any failures here?
Brian
--
Brian Masney <[email protected]>
