[PATCH 0/2] clk: renesas: rzv2h: Convert to clk_hw provider API

[PATCH 0/2] clk: renesas: rzv2h: Convert to clk_hw provider API

[Ovidiu Panait]

Hi,

The rzv2h-cpg driver stores its clocks as struct clk and uses the consumer
clk API internally, even though it is a clock provider.

This series converts it to use clk_hw, like rzg2l-cpg.c already does.

No functional change intended.

Thanks,
Ovidiu

John Madieu (2):
  clk: renesas: rzv2h: Drop duplicated parent lookup in
    fixed_mod_status_clk_register()
  clk: renesas: rzv2h: Convert to clk_hw based provider API

 drivers/clk/renesas/rzv2h-cpg.c | 154 ++++++++++++++------------------
 1 file changed, 68 insertions(+), 86 deletions(-)

-- 
2.34.1

[PATCH 1/2] clk: renesas: rzv2h: Drop duplicated parent lookup in fixed_mod_status_clk_register()

[Ovidiu Panait]

From: John Madieu <john.madieu.xa@bp.renesas.com>

rzv2h_cpg_fixed_mod_status_clk_register() looks up priv->clks[core->parent]
twice, with an IS_ERR() check after each lookup. The second lookup is a
leftover from an earlier refactor: the value cannot have changed between
the two reads, so the second IS_ERR() check is dead code.

Drop the duplicated lookup and check.

No functional change intended.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/clk/renesas/rzv2h-cpg.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index 7003d59ac34b..86f6f2f700a6 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -923,9 +923,6 @@ rzv2h_cpg_fixed_mod_status_clk_register(const struct cpg_core_clk *core,
 		return ERR_CAST(parent);
 
 	parent_name = __clk_get_name(parent);
-	parent = priv->clks[core->parent];
-	if (IS_ERR(parent))
-		return ERR_CAST(parent);
 
 	clk_hw_data = devm_kzalloc(priv->dev, sizeof(*clk_hw_data), GFP_KERNEL);
 	if (!clk_hw_data)
-- 
2.34.1

[PATCH 2/2] clk: renesas: rzv2h: Convert to clk_hw based provider API

[Ovidiu Panait]

From: John Madieu <john.madieu.xa@bp.renesas.com>

The driver mixes the clk consumer API with the clk_hw provider API for
its internal clock storage and OF lookup callback. Convert it to use
clk_hw consistently.

No functional change intended.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/clk/renesas/rzv2h-cpg.c | 151 ++++++++++++++------------------
 1 file changed, 68 insertions(+), 83 deletions(-)

diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index 86f6f2f700a6..9b0d691b1f5d 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -99,7 +99,7 @@ struct rzv2h_pll_dsi_info {
  * @dev: CPG device
  * @base: CPG register block base address
  * @rmw_lock: protects register accesses
- * @clks: Array containing all Core and Module Clocks
+ * @clks: Array containing all Core and Module clk_hw entries
  * @num_core_clks: Number of Core Clocks in clks[]
  * @num_mod_clks: Number of Module Clocks in clks[]
  * @resets: Array of resets
@@ -115,7 +115,7 @@ struct rzv2h_cpg_priv {
 	void __iomem *base;
 	spinlock_t rmw_lock;
 
-	struct clk **clks;
+	struct clk_hw **clks;
 	unsigned int num_core_clks;
 	unsigned int num_mod_clks;
 	struct rzv2h_reset *resets;
@@ -332,14 +332,14 @@ static const struct clk_ops rzv2h_cpg_plldsi_div_ops = {
 	.set_rate = rzv2h_cpg_plldsi_div_set_rate,
 };
 
-static struct clk * __init
+static struct clk_hw * __init
 rzv2h_cpg_plldsi_div_clk_register(const struct cpg_core_clk *core,
 				  struct rzv2h_cpg_priv *priv)
 {
 	struct rzv2h_plldsi_div_clk *clk_hw_data;
-	struct clk **clks = priv->clks;
+	struct clk_hw **clks = priv->clks;
 	struct clk_init_data init = {};
-	const struct clk *parent;
+	const struct clk_hw *parent;
 	const char *parent_name;
 	struct clk_hw *clk_hw;
 	int ret;
@@ -356,7 +356,7 @@ rzv2h_cpg_plldsi_div_clk_register(const struct cpg_core_clk *core,
 	clk_hw_data->ddiv = core->cfg.ddiv;
 	clk_hw_data->dtable = core->dtable;
 
-	parent_name = __clk_get_name(parent);
+	parent_name = clk_hw_get_name(parent);
 	init.name = core->name;
 	init.ops = &rzv2h_cpg_plldsi_div_ops;
 	init.flags = core->flag;
@@ -370,7 +370,7 @@ rzv2h_cpg_plldsi_div_clk_register(const struct cpg_core_clk *core,
 	if (ret)
 		return ERR_PTR(ret);
 
-	return clk_hw->clk;
+	return clk_hw;
 }
 
 static int rzv2h_cpg_plldsi_determine_rate(struct clk_hw *hw,
@@ -577,7 +577,7 @@ static const struct clk_ops rzv2h_cpg_plldsi_smux_ops = {
 	.set_duty_cycle = rzv2h_cpg_plldsi_smux_set_duty_cycle,
 };
 
-static struct clk * __init
+static struct clk_hw * __init
 rzv2h_cpg_plldsi_smux_clk_register(const struct cpg_core_clk *core,
 				   struct rzv2h_cpg_priv *priv)
 {
@@ -620,7 +620,7 @@ rzv2h_cpg_plldsi_smux_clk_register(const struct cpg_core_clk *core,
 	if (ret)
 		return ERR_PTR(ret);
 
-	return clk_hw->clk;
+	return clk_hw;
 }
 
 static int rzv2h_cpg_pll_clk_is_enabled(struct clk_hw *hw)
@@ -705,14 +705,14 @@ static const struct clk_ops rzv2h_cpg_pll_ops = {
 	.recalc_rate = rzv2h_cpg_pll_clk_recalc_rate,
 };
 
-static struct clk * __init
+static struct clk_hw * __init
 rzv2h_cpg_pll_clk_register(const struct cpg_core_clk *core,
 			   struct rzv2h_cpg_priv *priv,
 			   const struct clk_ops *ops)
 {
 	struct device *dev = priv->dev;
 	struct clk_init_data init = {};
-	const struct clk *parent;
+	const struct clk_hw *parent;
 	const char *parent_name;
 	struct pll_clk *pll_clk;
 	int ret;
@@ -729,7 +729,7 @@ rzv2h_cpg_pll_clk_register(const struct cpg_core_clk *core,
 		priv->pll_dsi_info[core->cfg.pll.instance].pll_dsi_limits =
 			core->cfg.pll.limits;
 
-	parent_name = __clk_get_name(parent);
+	parent_name = clk_hw_get_name(parent);
 	init.name = core->name;
 	init.ops = ops;
 	init.flags = 0;
@@ -744,7 +744,7 @@ rzv2h_cpg_pll_clk_register(const struct cpg_core_clk *core,
 	if (ret)
 		return ERR_PTR(ret);
 
-	return pll_clk->hw.clk;
+	return &pll_clk->hw;
 }
 
 static unsigned long rzv2h_ddiv_recalc_rate(struct clk_hw *hw,
@@ -820,7 +820,7 @@ static const struct clk_ops rzv2h_ddiv_clk_divider_ops = {
 	.set_rate = rzv2h_ddiv_set_rate,
 };
 
-static struct clk * __init
+static struct clk_hw * __init
 rzv2h_cpg_ddiv_clk_register(const struct cpg_core_clk *core,
 			    struct rzv2h_cpg_priv *priv)
 {
@@ -829,7 +829,7 @@ rzv2h_cpg_ddiv_clk_register(const struct cpg_core_clk *core,
 	struct device *dev = priv->dev;
 	u8 shift = cfg_ddiv.shift;
 	u8 width = cfg_ddiv.width;
-	const struct clk *parent;
+	const struct clk_hw *parent;
 	const char *parent_name;
 	struct clk_divider *div;
 	struct ddiv_clk *ddiv;
@@ -839,7 +839,7 @@ rzv2h_cpg_ddiv_clk_register(const struct cpg_core_clk *core,
 	if (IS_ERR(parent))
 		return ERR_CAST(parent);
 
-	parent_name = __clk_get_name(parent);
+	parent_name = clk_hw_get_name(parent);
 
 	if ((shift + width) > 16)
 		return ERR_PTR(-EINVAL);
@@ -872,15 +872,15 @@ rzv2h_cpg_ddiv_clk_register(const struct cpg_core_clk *core,
 	if (ret)
 		return ERR_PTR(ret);
 
-	return div->hw.clk;
+	return &div->hw;
 }
 
-static struct clk * __init
+static struct clk_hw * __init
 rzv2h_cpg_mux_clk_register(const struct cpg_core_clk *core,
 			   struct rzv2h_cpg_priv *priv)
 {
 	struct smuxed mux = core->cfg.smux;
-	const struct clk_hw *clk_hw;
+	struct clk_hw *clk_hw;
 
 	clk_hw = devm_clk_hw_register_mux(priv->dev, core->name,
 					  core->parent_names, core->num_parents,
@@ -890,7 +890,7 @@ rzv2h_cpg_mux_clk_register(const struct cpg_core_clk *core,
 	if (IS_ERR(clk_hw))
 		return ERR_CAST(clk_hw);
 
-	return clk_hw->clk;
+	return clk_hw;
 }
 
 static int
@@ -906,14 +906,14 @@ rzv2h_clk_ff_mod_status_is_enabled(struct clk_hw *hw)
 	return !!(val & bitmask);
 }
 
-static struct clk * __init
+static struct clk_hw * __init
 rzv2h_cpg_fixed_mod_status_clk_register(const struct cpg_core_clk *core,
 					struct rzv2h_cpg_priv *priv)
 {
 	struct rzv2h_ff_mod_status_clk *clk_hw_data;
 	struct clk_init_data init = { };
 	struct clk_fixed_factor *fix;
-	const struct clk *parent;
+	const struct clk_hw *parent;
 	const char *parent_name;
 	int ret;
 
@@ -922,7 +922,7 @@ rzv2h_cpg_fixed_mod_status_clk_register(const struct cpg_core_clk *core,
 	if (IS_ERR(parent))
 		return ERR_CAST(parent);
 
-	parent_name = __clk_get_name(parent);
+	parent_name = clk_hw_get_name(parent);
 
 	clk_hw_data = devm_kzalloc(priv->dev, sizeof(*clk_hw_data), GFP_KERNEL);
 	if (!clk_hw_data)
@@ -946,18 +946,18 @@ rzv2h_cpg_fixed_mod_status_clk_register(const struct cpg_core_clk *core,
 	if (ret)
 		return ERR_PTR(ret);
 
-	return clk_hw_data->fix.hw.clk;
+	return &clk_hw_data->fix.hw;
 }
 
-static struct clk
+static struct clk_hw
 *rzv2h_cpg_clk_src_twocell_get(struct of_phandle_args *clkspec,
 			       void *data)
 {
 	unsigned int clkidx = clkspec->args[1];
 	struct rzv2h_cpg_priv *priv = data;
 	struct device *dev = priv->dev;
+	struct clk_hw *clk_hw;
 	const char *type;
-	struct clk *clk;
 
 	switch (clkspec->args[0]) {
 	case CPG_CORE:
@@ -966,7 +966,7 @@ static struct clk
 			dev_err(dev, "Invalid %s clock index %u\n", type, clkidx);
 			return ERR_PTR(-EINVAL);
 		}
-		clk = priv->clks[clkidx];
+		clk_hw = priv->clks[clkidx];
 		break;
 
 	case CPG_MOD:
@@ -975,7 +975,7 @@ static struct clk
 			dev_err(dev, "Invalid %s clock index %u\n", type, clkidx);
 			return ERR_PTR(-EINVAL);
 		}
-		clk = priv->clks[priv->num_core_clks + clkidx];
+		clk_hw = priv->clks[priv->num_core_clks + clkidx];
 		break;
 
 	default:
@@ -983,25 +983,25 @@ static struct clk
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (IS_ERR(clk))
+	if (IS_ERR(clk_hw))
 		dev_err(dev, "Cannot get %s clock %u: %ld\n", type, clkidx,
-			PTR_ERR(clk));
+			PTR_ERR(clk_hw));
 	else
-		dev_dbg(dev, "clock (%u, %u) is %pC at %lu Hz\n",
-			clkspec->args[0], clkspec->args[1], clk,
-			clk_get_rate(clk));
-	return clk;
+		dev_dbg(dev, "clock (%u, %u) is %s at %lu Hz\n",
+			clkspec->args[0], clkspec->args[1],
+			clk_hw_get_name(clk_hw), clk_hw_get_rate(clk_hw));
+	return clk_hw;
 }
 
 static void __init
 rzv2h_cpg_register_core_clk(const struct cpg_core_clk *core,
 			    struct rzv2h_cpg_priv *priv)
 {
-	struct clk *clk = ERR_PTR(-EOPNOTSUPP), *parent;
+	struct clk_hw *clk_hw = ERR_PTR(-EOPNOTSUPP), *parent;
 	unsigned int id = core->id, div = core->div;
 	struct device *dev = priv->dev;
 	const char *parent_name;
-	struct clk_hw *clk_hw;
+	struct clk *clk;
 
 	WARN_DEBUG(id >= priv->num_core_clks);
 	WARN_DEBUG(PTR_ERR(priv->clks[id]) != -ENOENT);
@@ -1009,70 +1009,72 @@ rzv2h_cpg_register_core_clk(const struct cpg_core_clk *core,
 	switch (core->type) {
 	case CLK_TYPE_IN:
 		clk = of_clk_get_by_name(priv->dev->of_node, core->name);
+		if (IS_ERR(clk)) {
+			clk_hw = ERR_CAST(clk);
+			goto fail;
+		}
+		clk_hw = __clk_get_hw(clk);
 		break;
 	case CLK_TYPE_FF:
 		WARN_DEBUG(core->parent >= priv->num_core_clks);
 		parent = priv->clks[core->parent];
 		if (IS_ERR(parent)) {
-			clk = parent;
+			clk_hw = parent;
 			goto fail;
 		}
 
-		parent_name = __clk_get_name(parent);
+		parent_name = clk_hw_get_name(parent);
 		clk_hw = devm_clk_hw_register_fixed_factor(dev, core->name,
 							   parent_name, CLK_SET_RATE_PARENT,
 							   core->mult, div);
-		if (IS_ERR(clk_hw))
-			clk = ERR_CAST(clk_hw);
-		else
-			clk = clk_hw->clk;
 		break;
 	case CLK_TYPE_FF_MOD_STATUS:
 		if (!priv->ff_mod_status_ops) {
 			priv->ff_mod_status_ops =
 				devm_kzalloc(dev, sizeof(*priv->ff_mod_status_ops), GFP_KERNEL);
 			if (!priv->ff_mod_status_ops) {
-				clk = ERR_PTR(-ENOMEM);
+				clk_hw = ERR_PTR(-ENOMEM);
 				goto fail;
 			}
 			memcpy(priv->ff_mod_status_ops, &clk_fixed_factor_ops,
 			       sizeof(const struct clk_ops));
 			priv->ff_mod_status_ops->is_enabled = rzv2h_clk_ff_mod_status_is_enabled;
 		}
-		clk = rzv2h_cpg_fixed_mod_status_clk_register(core, priv);
+		clk_hw = rzv2h_cpg_fixed_mod_status_clk_register(core, priv);
 		break;
 	case CLK_TYPE_PLL:
-		clk = rzv2h_cpg_pll_clk_register(core, priv, &rzv2h_cpg_pll_ops);
+		clk_hw = rzv2h_cpg_pll_clk_register(core, priv, &rzv2h_cpg_pll_ops);
 		break;
 	case CLK_TYPE_DDIV:
-		clk = rzv2h_cpg_ddiv_clk_register(core, priv);
+		clk_hw = rzv2h_cpg_ddiv_clk_register(core, priv);
 		break;
 	case CLK_TYPE_SMUX:
-		clk = rzv2h_cpg_mux_clk_register(core, priv);
+		clk_hw = rzv2h_cpg_mux_clk_register(core, priv);
 		break;
 	case CLK_TYPE_PLLDSI:
-		clk = rzv2h_cpg_pll_clk_register(core, priv, &rzv2h_cpg_plldsi_ops);
+		clk_hw = rzv2h_cpg_pll_clk_register(core, priv, &rzv2h_cpg_plldsi_ops);
 		break;
 	case CLK_TYPE_PLLDSI_DIV:
-		clk = rzv2h_cpg_plldsi_div_clk_register(core, priv);
+		clk_hw = rzv2h_cpg_plldsi_div_clk_register(core, priv);
 		break;
 	case CLK_TYPE_PLLDSI_SMUX:
-		clk = rzv2h_cpg_plldsi_smux_clk_register(core, priv);
+		clk_hw = rzv2h_cpg_plldsi_smux_clk_register(core, priv);
 		break;
 	default:
 		goto fail;
 	}
 
-	if (IS_ERR(clk))
+	if (IS_ERR(clk_hw))
 		goto fail;
 
-	dev_dbg(dev, "Core clock %pC at %lu Hz\n", clk, clk_get_rate(clk));
-	priv->clks[id] = clk;
+	dev_dbg(dev, "Core clock %s at %lu Hz\n",
+		clk_hw_get_name(clk_hw), clk_hw_get_rate(clk_hw));
+	priv->clks[id] = clk_hw;
 	return;
 
 fail:
 	dev_err(dev, "Failed to register core clock %s: %ld\n",
-		core->name, PTR_ERR(clk));
+		core->name, PTR_ERR(clk_hw));
 }
 
 static void rzv2h_mod_clock_mstop_enable(struct rzv2h_cpg_priv *priv,
@@ -1119,15 +1121,10 @@ static void rzv2h_mod_clock_mstop_disable(struct rzv2h_cpg_priv *priv,
 
 static int rzv2h_parent_clk_mux_to_index(struct clk_hw *hw)
 {
-	struct clk_hw *parent_hw;
-	struct clk *parent_clk;
+	struct clk_hw *parent_hw = clk_hw_get_parent(hw);
 	struct clk_mux *mux;
 	u32 val;
 
-	/* This will always succeed, so no need to check for IS_ERR() */
-	parent_clk = clk_get_parent(hw->clk);
-
-	parent_hw = __clk_get_hw(parent_clk);
 	mux = to_clk_mux(parent_hw);
 
 	val = readl(mux->reg) >> mux->shift;
@@ -1227,8 +1224,8 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 	struct mod_clock *clock = NULL;
 	struct device *dev = priv->dev;
 	struct clk_init_data init = {};
-	struct clk *parent, *clk;
 	const char *parent_name;
+	struct clk_hw *parent;
 	unsigned int id;
 	int ret;
 
@@ -1239,13 +1236,13 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 
 	parent = priv->clks[mod->parent];
 	if (IS_ERR(parent)) {
-		clk = parent;
+		ret = PTR_ERR(parent);
 		goto fail;
 	}
 
 	clock = devm_kzalloc(dev, sizeof(*clock), GFP_KERNEL);
 	if (!clock) {
-		clk = ERR_PTR(-ENOMEM);
+		ret = -ENOMEM;
 		goto fail;
 	}
 
@@ -1255,7 +1252,7 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 	if (mod->critical)
 		init.flags |= CLK_IS_CRITICAL;
 
-	parent_name = __clk_get_name(parent);
+	parent_name = clk_hw_get_name(parent);
 	init.parent_names = &parent_name;
 	init.num_parents = 1;
 
@@ -1270,12 +1267,10 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 	clock->mstop_data = mod->mstop_data;
 
 	ret = devm_clk_hw_register(dev, &clock->hw);
-	if (ret) {
-		clk = ERR_PTR(ret);
+	if (ret)
 		goto fail;
-	}
 
-	priv->clks[id] = clock->hw.clk;
+	priv->clks[id] = &clock->hw;
 
 	/*
 	 * Ensure the module clocks and MSTOP bits are synchronized when they are
@@ -1314,8 +1309,8 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 	return;
 
 fail:
-	dev_err(dev, "Failed to register module clock %s: %ld\n",
-		mod->name, PTR_ERR(clk));
+	dev_err(dev, "Failed to register module clock %s: %d\n",
+		mod->name, ret);
 }
 
 static int __rzv2h_cpg_assert(struct reset_controller_dev *rcdev,
@@ -1448,7 +1443,7 @@ static bool rzv2h_cpg_is_pm_clk(struct rzv2h_cpg_pd *pd,
 		if (priv->clks[priv->num_core_clks + id] == ERR_PTR(-ENOENT))
 			return false;
 
-		clock = to_mod_clock(__clk_get_hw(priv->clks[priv->num_core_clks + id]));
+		clock = to_mod_clock(priv->clks[priv->num_core_clks + id]);
 
 		return !clock->no_pm;
 	}
@@ -1547,19 +1542,13 @@ static int __init rzv2h_cpg_add_pm_domains(struct rzv2h_cpg_priv *priv)
 	return of_genpd_add_provider_simple(np, &pd->genpd);
 }
 
-static void rzv2h_cpg_del_clk_provider(void *data)
-{
-	of_clk_del_provider(data);
-}
-
 static int __init rzv2h_cpg_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
 	const struct rzv2h_cpg_info *info;
 	struct rzv2h_cpg_priv *priv;
 	unsigned int nclks, i;
-	struct clk **clks;
+	struct clk_hw **clks;
 	int error;
 
 	info = of_device_get_match_data(dev);
@@ -1610,11 +1599,7 @@ static int __init rzv2h_cpg_probe(struct platform_device *pdev)
 	for (i = 0; i < info->num_mod_clks; i++)
 		rzv2h_cpg_register_mod_clk(&info->mod_clks[i], priv);
 
-	error = of_clk_add_provider(np, rzv2h_cpg_clk_src_twocell_get, priv);
-	if (error)
-		return error;
-
-	error = devm_add_action_or_reset(dev, rzv2h_cpg_del_clk_provider, np);
+	error = devm_of_clk_add_hw_provider(dev, rzv2h_cpg_clk_src_twocell_get, priv);
 	if (error)
 		return error;
 
-- 
2.34.1

Re: [PATCH 2/2] clk: renesas: rzv2h: Convert to clk_hw based provider API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing OF genpd provider cleanup on the probe error path leads to a Use-After-Free.
--

commit a63d8b10a46c55aa614df9d149b0a17736dc2cc5
Author: John Madieu <john.madieu.xa@bp.renesas.com>

clk: renesas: rzv2h: Convert to clk_hw based provider API

The driver mixes the clk consumer API with the clk_hw provider API for its
internal clock storage and OF lookup callback. Convert it to use clk_hw
consistently.

> diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
> index 86f6f2f700a65..9b0d691b1f5d0 100644
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
[ ... ]
> @@ -1547,19 +1542,13 @@ static int __init rzv2h_cpg_add_pm_domains(struct rzv2h_cpg_priv *priv)
>  	return of_genpd_add_provider_simple(np, &pd->genpd);

[Severity: High]
This is a pre-existing issue, but does this missing OF genpd provider cleanup
on the probe error path lead to a Use-After-Free?

If a subsequent operation in rzv2h_cpg_probe() (such as
rzv2h_cpg_reset_controller_register()) fails, the devm framework automatically
frees the struct rzv2h_cpg_pd that was allocated with devm_kzalloc().

However, the cleanup action pm_genpd_remove() returns -EBUSY and aborts
because of_genpd_add_provider_simple() sets genpd->has_provider = true.
Because of_genpd_del_provider() is never called to unset this flag and
remove the provider, the domain remains registered in the global
of_genpd_providers list, gpd_list, and the device core.

When a consumer device subsequently probes and looks up its PM domain, or
when the PM domain core walks the lists, it could dereference the freed
pd->genpd memory.

>  }
>  
> -static void rzv2h_cpg_del_clk_provider(void *data)
> -{
> -	of_clk_del_provider(data);
> -}
> -

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902104939.25522-1-ovidiu.panait.rb@renesas.com?part=2

Re: [PATCH 1/2] clk: renesas: rzv2h: Drop duplicated parent lookup in fixed_mod_status_clk_register()

[Geert Uytterhoeven]

On Wed, 2 Sept 2026 at 12:50, Ovidiu Panait
<ovidiu.panait.rb@renesas.com> wrote:
> From: John Madieu <john.madieu.xa@bp.renesas.com>
>
> rzv2h_cpg_fixed_mod_status_clk_register() looks up priv->clks[core->parent]
> twice, with an IS_ERR() check after each lookup. The second lookup is a
> leftover from an earlier refactor: the value cannot have changed between
> the two reads, so the second IS_ERR() check is dead code.
>
> Drop the duplicated lookup and check.
>
> No functional change intended.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v7.4.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] clk: renesas: rzv2h: Convert to clk_hw based provider API

[Geert Uytterhoeven]

Hi Ovidiu,

On Wed, 2 Sept 2026 at 12:50, Ovidiu Panait
<ovidiu.panait.rb@renesas.com> wrote:
> From: John Madieu <john.madieu.xa@bp.renesas.com>
>
> The driver mixes the clk consumer API with the clk_hw provider API for
> its internal clock storage and OF lookup callback. Convert it to use
> clk_hw consistently.
>
> No functional change intended.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>

Nice, thanks a lot for picking up John's legacy!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v7.4.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds