[v1] clk: renesas: Add XSPI clock support for RZ/V2N and RZ/V2H(P)

[PATCH 4/6] clk: renesas: rzv2h: Add fixed-factor module clocks with status reporting

Posted by Prabhakar 7 months, 2 weeks ago

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add support for fixed-factor module clocks that can report their enable
status through the module status monitor. Introduce a new clock type,
CLK_TYPE_FF_MOD_STATUS, and define the associated structure,
rzv2h_ff_mod_status_clk, to manage these clocks.

Implement the .is_enabled callback by reading the module status register
using monitor index and bit definitions. Provide a helper macro,
DEF_FIXED_MOD_STATUS, to simplify the definition of such clocks.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/renesas/rzv2h-cpg.c | 83 +++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rzv2h-cpg.h | 22 +++++++++
 2 files changed, 105 insertions(+)

diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index 11325980379a..a511f34abf18 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -150,6 +150,22 @@ struct ddiv_clk {
 
 #define to_ddiv_clock(_div) container_of(_div, struct ddiv_clk, div)
 
+/**
+ * struct rzv2h_ff_mod_status_clk - Fixed Factor Module Status Clock
+ *
+ * @priv: CPG private data
+ * @conf: fixed mod configuration
+ * @fix: fixed factor clock
+ */
+struct rzv2h_ff_mod_status_clk {
+	struct rzv2h_cpg_priv *priv;
+	struct fixed_mod_conf conf;
+	struct clk_fixed_factor fix;
+};
+
+#define to_rzv2h_ff_mod_status_clk(_hw) \
+	container_of(_hw, struct rzv2h_ff_mod_status_clk, fix.hw)
+
 static int rzv2h_cpg_pll_clk_is_enabled(struct clk_hw *hw)
 {
 	struct pll_clk *pll_clk = to_pll(hw);
@@ -421,6 +437,70 @@ rzv2h_cpg_mux_clk_register(const struct cpg_core_clk *core,
 	return clk_hw->clk;
 }
 
+static int
+rzv2h_clk_ff_mod_status_is_enabled(struct clk_hw *hw)
+{
+	struct rzv2h_ff_mod_status_clk *fix = to_rzv2h_ff_mod_status_clk(hw);
+	struct rzv2h_cpg_priv *priv = fix->priv;
+	u32 offset = GET_CLK_MON_OFFSET(fix->conf.mon_index);
+	u32 bitmask = BIT(fix->conf.mon_bit);
+	u32 val;
+
+	val = readl(priv->base + offset);
+	return !!(val & bitmask);
+}
+
+static struct clk_ops rzv2h_clk_ff_mod_status_ops;
+
+static struct clk * __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 char *parent_name;
+	int ret;
+
+	WARN_DEBUG(core->parent >= priv->num_core_clks);
+	parent = priv->clks[core->parent];
+	if (IS_ERR(parent))
+		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)
+		return ERR_PTR(-ENOMEM);
+
+	clk_hw_data->priv = priv;
+	clk_hw_data->conf = core->cfg.fixed_mod;
+
+	rzv2h_clk_ff_mod_status_ops = clk_fixed_factor_ops;
+	rzv2h_clk_ff_mod_status_ops.is_enabled = rzv2h_clk_ff_mod_status_is_enabled;
+
+	init.name = core->name;
+	init.ops = &rzv2h_clk_ff_mod_status_ops;
+	init.flags = CLK_SET_RATE_PARENT;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	fix = &clk_hw_data->fix;
+	fix->hw.init = &init;
+	fix->mult = core->mult;
+	fix->div = core->div;
+
+	ret = devm_clk_hw_register(priv->dev, &clk_hw_data->fix.hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return clk_hw_data->fix.hw.clk;
+}
+
 static struct clk
 *rzv2h_cpg_clk_src_twocell_get(struct of_phandle_args *clkspec,
 			       void *data)
@@ -499,6 +579,9 @@ rzv2h_cpg_register_core_clk(const struct cpg_core_clk *core,
 		else
 			clk = clk_hw->clk;
 		break;
+	case CLK_TYPE_FF_MOD_STATUS:
+		clk = 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);
 		break;
diff --git a/drivers/clk/renesas/rzv2h-cpg.h b/drivers/clk/renesas/rzv2h-cpg.h
index 7321b085f937..317c8ee2e2d5 100644
--- a/drivers/clk/renesas/rzv2h-cpg.h
+++ b/drivers/clk/renesas/rzv2h-cpg.h
@@ -93,6 +93,23 @@ struct smuxed {
 		.width = (_width), \
 	})
 
+/**
+ * struct fixed_mod_conf - Structure for fixed module configuration
+ *
+ * @mon_index: monitor index
+ * @mon_bit: monitor bit
+ */
+struct fixed_mod_conf {
+	u8 mon_index;
+	u8 mon_bit;
+};
+
+#define FIXED_MOD_CONF_PACK(_index, _bit) \
+	((struct fixed_mod_conf){ \
+		.mon_index = (_index), \
+		.mon_bit = (_bit), \
+	})
+
 #define CPG_SSEL0		(0x300)
 #define CPG_SSEL1		(0x304)
 #define CPG_CDDIV0		(0x400)
@@ -151,6 +168,7 @@ struct cpg_core_clk {
 		struct ddiv ddiv;
 		struct pll pll;
 		struct smuxed smux;
+		struct fixed_mod_conf fixed_mod;
 	} cfg;
 	const struct clk_div_table *dtable;
 	const char * const *parent_names;
@@ -163,6 +181,7 @@ enum clk_types {
 	/* Generic */
 	CLK_TYPE_IN,		/* External Clock Input */
 	CLK_TYPE_FF,		/* Fixed Factor Clock */
+	CLK_TYPE_FF_MOD_STATUS,	/* Fixed Factor Clock which can report the status of module clock */
 	CLK_TYPE_PLL,
 	CLK_TYPE_DDIV,		/* Dynamic Switching Divider */
 	CLK_TYPE_SMUX,		/* Static Mux */
@@ -178,6 +197,9 @@ enum clk_types {
 	DEF_TYPE(_name, _id, CLK_TYPE_IN)
 #define DEF_FIXED(_name, _id, _parent, _mult, _div) \
 	DEF_BASE(_name, _id, CLK_TYPE_FF, _parent, .div = _div, .mult = _mult)
+#define DEF_FIXED_MOD_STATUS(_name, _id, _parent, _mult, _div, _gate) \
+	DEF_BASE(_name, _id, CLK_TYPE_FF_MOD_STATUS, _parent, .div = _div, \
+		 .mult = _mult, .cfg.fixed_mod = _gate)
 #define DEF_DDIV(_name, _id, _parent, _ddiv_packed, _dtable) \
 	DEF_TYPE(_name, _id, CLK_TYPE_DDIV, \
 		.cfg.ddiv = _ddiv_packed, \
-- 
2.49.0

Re: [PATCH 4/6] clk: renesas: rzv2h: Add fixed-factor module clocks with status reporting

Posted by Geert Uytterhoeven 7 months, 2 weeks ago

Hi Prabhakar,

On Tue, 24 Jun 2025 at 19:30, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add support for fixed-factor module clocks that can report their enable
> status through the module status monitor. Introduce a new clock type,
> CLK_TYPE_FF_MOD_STATUS, and define the associated structure,
> rzv2h_ff_mod_status_clk, to manage these clocks.
>
> Implement the .is_enabled callback by reading the module status register
> using monitor index and bit definitions. Provide a helper macro,
> DEF_FIXED_MOD_STATUS, to simplify the definition of such clocks.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

One early review comment below...

> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c

> +static struct clk_ops rzv2h_clk_ff_mod_status_ops;

This is an empty block of 200 bytes, consuming memory even when running
on a different platform.

> +static struct clk * __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 char *parent_name;
> +       int ret;
> +
> +       WARN_DEBUG(core->parent >= priv->num_core_clks);
> +       parent = priv->clks[core->parent];
> +       if (IS_ERR(parent))
> +               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)
> +               return ERR_PTR(-ENOMEM);
> +
> +       clk_hw_data->priv = priv;
> +       clk_hw_data->conf = core->cfg.fixed_mod;
> +
> +       rzv2h_clk_ff_mod_status_ops = clk_fixed_factor_ops;

This overwrites rzv2h_clk_ff_mod_status_ops in every call (currently
there is only one).

> +       rzv2h_clk_ff_mod_status_ops.is_enabled = rzv2h_clk_ff_mod_status_is_enabled;

If there would be multiple calls, there is a short time window where
rzv2h_clk_ff_mod_status_ops.is_enabled is NULL, possibly affecting
already-registered clocks of the same type.

Hence I think you better store rzv2h_clk_ff_mod_status_ops inside
rzv2h_cpg_priv (so it is allocated dynamically), and initialize it from
rzv2h_cpg_probe (so it is initialized once).

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 4/6] clk: renesas: rzv2h: Add fixed-factor module clocks with status reporting

Posted by Lad, Prabhakar 7 months, 2 weeks ago

Hi Geert,

Thank you for the review.

On Thu, Jun 26, 2025 at 2:22 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, 24 Jun 2025 at 19:30, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add support for fixed-factor module clocks that can report their enable
> > status through the module status monitor. Introduce a new clock type,
> > CLK_TYPE_FF_MOD_STATUS, and define the associated structure,
> > rzv2h_ff_mod_status_clk, to manage these clocks.
> >
> > Implement the .is_enabled callback by reading the module status register
> > using monitor index and bit definitions. Provide a helper macro,
> > DEF_FIXED_MOD_STATUS, to simplify the definition of such clocks.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> One early review comment below...
>
> > --- a/drivers/clk/renesas/rzv2h-cpg.c
> > +++ b/drivers/clk/renesas/rzv2h-cpg.c
>
> > +static struct clk_ops rzv2h_clk_ff_mod_status_ops;
>
> This is an empty block of 200 bytes, consuming memory even when running
> on a different platform.
>
Agreed.

> > +static struct clk * __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 char *parent_name;
> > +       int ret;
> > +
> > +       WARN_DEBUG(core->parent >= priv->num_core_clks);
> > +       parent = priv->clks[core->parent];
> > +       if (IS_ERR(parent))
> > +               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)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       clk_hw_data->priv = priv;
> > +       clk_hw_data->conf = core->cfg.fixed_mod;
> > +
> > +       rzv2h_clk_ff_mod_status_ops = clk_fixed_factor_ops;
>
> This overwrites rzv2h_clk_ff_mod_status_ops in every call (currently
> there is only one).
>
Good point.

> > +       rzv2h_clk_ff_mod_status_ops.is_enabled = rzv2h_clk_ff_mod_status_is_enabled;
>
> If there would be multiple calls, there is a short time window where
> rzv2h_clk_ff_mod_status_ops.is_enabled is NULL, possibly affecting
> already-registered clocks of the same type.
>
Yes agreed.

> Hence I think you better store rzv2h_clk_ff_mod_status_ops inside
> rzv2h_cpg_priv (so it is allocated dynamically), and initialize it from
> rzv2h_cpg_probe (so it is initialized once).
>
Sure, I'll update as above, that is allocate only when needed (and only once).

Cheers,
Prabhakar

[PATCH 1/6] dt-bindings: clock: renesas,r9a09g056/57-cpg: Add XSPI core clock
[PATCH 2/6] clk: renesas: r9a09g056: Add support for xspi mux and divider
[PATCH 3/6] clk: renesas: r9a09g057: Add support for xspi mux and divider
[PATCH 4/6] clk: renesas: rzv2h: Add fixed-factor module clocks with status reporting
[PATCH 5/6] clk: renesas: r9a09g056: Add XSPI clock/reset
[PATCH 6/6] clk: renesas: r9a09g057: Add XSPI clock/reset