Hi Prabhakar,
Thanks for your patch!
s/rzv2h-cpg/rzv2h/
On Tue, Nov 5, 2024 at 12:24 AM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Update `rzv2h_cpg_attach_dev` to prevent external clocks not tied to the
> power domain from being managed by Runtime PM. This ensures that only
> clocks originating from the domain are controlled, thereby avoiding
> unintended handling of external clocks.
>
> Additionally, introduce a `no_pm` flag in `mod_clock` and `rzv2h_mod_clk`
> structures to exclude specific clocks from Runtime PM when needed. Some
> clocks, such as those in the CRU block, require unique enable/disable
> sequences that are incompatible with standard Runtime PM. For example,
> the CSI-2 D-PHY clock initialization requires toggling individual clocks,
> making Runtime PM unsuitable.
>
> The helper function `rzv2h_cpg_is_pm_clk()` checks whether a clock should
> be managed by Runtime PM based on this `no_pm` flag. New macros, such as
> `DEF_MOD_NO_PM`, allow straightforward declaration of clocks that bypass
> PM.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> ---
> v1->v2
> - Updated code to skip external clocks to be controlled from runtime PM
> - Updated id range check
> - Updated commit message
Thanks for the update!
> --- a/drivers/clk/renesas/rzv2h-cpg.c
> +++ b/drivers/clk/renesas/rzv2h-cpg.c
> @@ -668,8 +671,38 @@ struct rzv2h_cpg_pd {
> struct generic_pm_domain genpd;
> };
>
> +static bool rzv2h_cpg_is_pm_clk(struct rzv2h_cpg_pd *pd,
> + const struct of_phandle_args *clkspec)
> +{
> + if (clkspec->np != pd->genpd.dev.of_node || clkspec->args_count != 2)
> + return false;
> +
> + switch (clkspec->args[0]) {
> + case CPG_MOD: {
> + struct rzv2h_cpg_priv *priv = pd->priv;
> + unsigned int id = clkspec->args[1];
> + struct mod_clock *clock;
> +
> + if (id >= priv->num_mod_clks)
> + return true;
> +
> + if (priv->clks[priv->num_core_clks + id] == ERR_PTR(-ENOENT))
> + return true;
Shouldn't this return false for the two invalid cases above?
> +
> + clock = to_mod_clock(__clk_get_hw(priv->clks[priv->num_core_clks + id]));
> +
> + return !clock->no_pm;
> + }
> +
> + case CPG_CORE:
> + default:
> + return true;
False? (I know the code treated it is a PM clock before)
> + }
> +}
> +
> static int rzv2h_cpg_attach_dev(struct generic_pm_domain *domain, struct device *dev)
> {
> + struct rzv2h_cpg_pd *pd = container_of(domain, struct rzv2h_cpg_pd, genpd);
> struct device_node *np = dev->of_node;
> struct of_phandle_args clkspec;
> bool once = true;
> @@ -679,6 +712,12 @@ static int rzv2h_cpg_attach_dev(struct generic_pm_domain *domain, struct device
>
> while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
> &clkspec)) {
> + if (!rzv2h_cpg_is_pm_clk(pd, &clkspec)) {
> + of_node_put(clkspec.np);
> + i++;
> + continue;
This loop may start to loop nicer using
"for (i = 0; !of_parse_phandle_with_args(...); i++)".
> + }
> +
> if (once) {
> once = false;
> error = pm_clk_create(dev);
> diff --git a/drivers/clk/renesas/rzv2h-cpg.h b/drivers/clk/renesas/rzv2h-cpg.h
> index 819029c81904..0723df4c1134 100644
> --- a/drivers/clk/renesas/rzv2h-cpg.h
> +++ b/drivers/clk/renesas/rzv2h-cpg.h
> @@ -100,6 +100,7 @@ enum clk_types {
> * @name: handle between common and hardware-specific interfaces
> * @parent: id of parent clock
> * @critical: flag to indicate the clock is critical
> + * @no_pm: flag to indicate PM is not supported
> * @on_index: control register index
> * @on_bit: ON bit
> * @mon_index: monitor register index
> @@ -109,17 +110,19 @@ struct rzv2h_mod_clk {
> const char *name;
> u16 parent;
> bool critical;
> + bool no_pm;
> u8 on_index;
> u8 on_bit;
> s8 mon_index;
> u8 mon_bit;
> };
>
> -#define DEF_MOD_BASE(_name, _parent, _critical, _onindex, _onbit, _monindex, _monbit) \
> +#define DEF_MOD_BASE(_name, _parent, _critical, _no_pm, _onindex, _onbit, _monindex, _monbit) \
Note that this series conflicts with "[PATCH 00/12] Add support for
Renesas RZ/G3E SoC and SMARC-EVK platform", which you are probably
already aware of.
[1] https://lore.kernel.org/all/20241122124558.149827-1-biju.das.jz@xxxxxxxxxxxxxx/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
