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.0Hi 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
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
© 2016 - 2025 Red Hat, Inc.