Hi Prabhakar,
On Thu, Sep 29, 2022 at 8:51 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> There are cases where not all CPG_MOD clocks should be assumed to support
> PM. For example on the CRU block there is a particular sequence that needs
> to be followed to initialize the CSI-2 D-PHY in which individual clocks
> need to be turned ON/OFF, due to which Runtime PM support wasn't used by
> the CRU CSI-2 driver.
>
> This patch adds support to allow indicating if PM is supported by the
> CPG_MOD clocks. A new macro is DEF_NO_PM() is added which sets the no_pm
> flag in struct rzg2l_mod_clk and when the driver uses Runtime PM support
> no_pm flag is checked to see if the clk needs to included as part of
> Runtime PM.
>
> CPG_MOD clocks with no_pm flag set need to be individually turned ON/OFF
> depending on the requirement of the driver.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> ---
> RFC->v1
> * Added no_pm_mod_clks and num_no_pm_mod_clks members as part of
> struct rzg2l_cpg_priv
Thanks for your patch!
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -108,12 +108,16 @@ struct rzg2l_cpg_priv {
> unsigned int num_mod_clks;
> unsigned int num_resets;
> unsigned int last_dt_core_clk;
> + int *no_pm_mod_clks;
> + unsigned int num_no_pm_mod_clks;
>
> const struct rzg2l_cpg_info *info;
>
> struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params;
> };
>
> +static struct rzg2l_cpg_priv *rzg2l_cpg_priv;
I think you can do without this, by moving the currently separately
allocated struct generic_pm_domain into struct rzg2l_cpg_priv,
and using container_of() on the currently unused pointer passed to
rzg2l_cpg_attach_dev().
Note to self: get rid of the cpg_mssr_clk_domain static pointer in
the CPG/MSSR driver.
> +
> static void rzg2l_cpg_del_clk_provider(void *data)
> {
> of_clk_del_provider(data);
> @@ -1225,16 +1229,24 @@ static int rzg2l_cpg_reset_controller_register(struct rzg2l_cpg_priv *priv)
>
> static bool rzg2l_cpg_is_pm_clk(const struct of_phandle_args *clkspec)
> {
> + struct rzg2l_cpg_priv *priv = rzg2l_cpg_priv;
> + const struct rzg2l_cpg_info *info = priv->info;
info is used only once, expand user below?
> + unsigned int id;
> + unsigned int i;
> +
> if (clkspec->args_count != 2)
> return false;
>
> - switch (clkspec->args[0]) {
> - case CPG_MOD:
> - return true;
> -
> - default:
> + if (clkspec->args[0] != CPG_MOD)
> return false;
> +
> + id = clkspec->args[1] + info->num_total_core_clks;
> + for (i = 0; i < priv->num_no_pm_mod_clks; i++) {
> + if (priv->no_pm_mod_clks[i] == id)
> + return false;
> }
> +
> + return true;
> }
>
> static int rzg2l_cpg_attach_dev(struct generic_pm_domain *unused, struct device *dev)
> @@ -1366,8 +1379,26 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
> for (i = 0; i < info->num_core_clks; i++)
> rzg2l_cpg_register_core_clk(&info->core_clks[i], info, priv);
>
> - for (i = 0; i < info->num_mod_clks; i++)
> + priv->num_no_pm_mod_clks = 0;
> + for (i = 0; i < info->num_mod_clks; i++) {
> + if (info->mod_clks[i].no_pm)
> + priv->num_no_pm_mod_clks++;
> + }
> +
> + if (priv->num_no_pm_mod_clks && info->num_mod_clks) {
> + priv->no_pm_mod_clks =
> + devm_kmalloc_array(dev, priv->num_no_pm_mod_clks,
> + sizeof(info->mod_clks[0].id),
> + GFP_KERNEL);
> + if (!priv->no_pm_mod_clks)
> + return -ENOMEM;
> + }
> +
> + for (i = 0, j = 0; i < info->num_mod_clks; i++) {
> + if (info->mod_clks[i].no_pm)
> + priv->no_pm_mod_clks[j++] = info->mod_clks[i].id;
> rzg2l_cpg_register_mod_clk(&info->mod_clks[i], info, priv);
> + }
Alternatively, you could have a separate rzg2l_cpg_info.no_pm_clks[]
array, like .crit_mod_clks[], and do the counting etc. at compile-time.
>
> error = of_clk_add_provider(np, rzg2l_cpg_clk_src_twocell_get, priv);
> if (error)
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
