From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
linux-renesas-soc@vger.kernel.org, linux-clk@vger.kernel.org,
linux-kernel@vger.kernel.org,
Biju Das <biju.das.jz@bp.renesas.com>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [PATCH 1/2] clk: renesas: rzg2l: Don't assume all CPG_MOD clocks support PM
Date: Wed, 26 Oct 2022 01:33:24 +0100 [thread overview]
Message-ID: <CA+V-a8ujJR-Ca4QRgiosPfaEmscFKERHySUL9xpzr794x5EzmQ@mail.gmail.com> (raw)
In-Reply-To: <CAMuHMdXR+Nsyd0EkNeBvUOpm+qPNUDMZedfY0wErESi5x2Z9JA@mail.gmail.com>
Hi Geert,
Thank you for the review.
On Tue, Oct 25, 2022 at 9:56 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Sep 29, 2022 at 8:51 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > 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@bp.renesas.com>
> > ---
> > 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().
>
Agreed.
> 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?
>
With the below change suggested info will be used more than once
(3-times), so I'll keep it.
> > + 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.
>
Agreed.
Cheers,
Prabhakar
next prev parent reply other threads:[~2022-10-26 0:33 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-29 18:51 [PATCH 0/2] clk: renesas: RZ/G2L: Add support for no PM clocks Prabhakar
2022-09-29 18:51 ` [PATCH 1/2] clk: renesas: rzg2l: Don't assume all CPG_MOD clocks support PM Prabhakar
2022-10-25 8:55 ` Geert Uytterhoeven
2022-10-26 0:33 ` Lad, Prabhakar [this message]
2022-09-29 18:51 ` [PATCH 2/2] clk: renesas: r9a07g044: Mark CRU_SYSCLK and CRU_VCLK as no PM Prabhakar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+V-a8ujJR-Ca4QRgiosPfaEmscFKERHySUL9xpzr794x5EzmQ@mail.gmail.com \
--to=prabhakar.csengg@gmail.com \
--cc=biju.das.jz@bp.renesas.com \
--cc=geert@linux-m68k.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).