From: Lad Prabhakar <[email protected]>
Hi All,
This patch series adds support for indicating MOD clocks as no PM (if any).
Patch#1 adds DEF_NO_PM() macro to flagup no PM clock and patch #2 switches
sysclk and vclk clocks to no PM.
Sending it as an RFC as there wasn't any way we could obtain the priv data
due to which I had to create a static global var for rzg2l_cpg_priv.
RFC->v1
* Added no_pm_mod_clks and num_no_pm_mod_clks members as part of
struct rzg2l_cpg_priv so that we dont loop the entire mod clocks
array for each device probe.
* Patch#2 unchanged
RFC: https://patchwork.kernel.org/project/linux-renesas-soc/cover/[email protected]/
Cheers,
Prabhakar
Lad Prabhakar (2):
clk: renesas: rzg2l: Don't assume all CPG_MOD clocks support PM
clk: renesas: r9a07g044: Mark CRU_SYSCLK and CRU_VCLK as no PM
drivers/clk/renesas/r9a07g044-cpg.c | 4 +--
drivers/clk/renesas/rzg2l-cpg.c | 45 ++++++++++++++++++++++++-----
drivers/clk/renesas/rzg2l-cpg.h | 12 ++++++--
3 files changed, 49 insertions(+), 12 deletions(-)
--
2.25.1
From: Lad Prabhakar <[email protected]>
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 <[email protected]>
---
RFC->v1
* Added no_pm_mod_clks and num_no_pm_mod_clks members as part of
struct rzg2l_cpg_priv
---
drivers/clk/renesas/rzg2l-cpg.c | 45 ++++++++++++++++++++++++++++-----
drivers/clk/renesas/rzg2l-cpg.h | 12 ++++++---
2 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index 3ff6ecd61756..431697a37692 100644
--- 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;
+
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;
+ 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)
@@ -1330,7 +1342,7 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
struct device_node *np = dev->of_node;
const struct rzg2l_cpg_info *info;
struct rzg2l_cpg_priv *priv;
- unsigned int nclks, i;
+ unsigned int nclks, i, j;
struct clk **clks;
int error;
@@ -1348,6 +1360,7 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev)
if (IS_ERR(priv->base))
return PTR_ERR(priv->base);
+ rzg2l_cpg_priv = priv;
nclks = info->num_total_core_clks + info->num_hw_mod_clks;
clks = devm_kmalloc_array(dev, nclks, sizeof(*clks), GFP_KERNEL);
if (!clks)
@@ -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);
+ }
error = of_clk_add_provider(np, rzg2l_cpg_clk_src_twocell_get, priv);
if (error)
diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h
index cecbdf5e4f93..1d68d3838392 100644
--- a/drivers/clk/renesas/rzg2l-cpg.h
+++ b/drivers/clk/renesas/rzg2l-cpg.h
@@ -176,6 +176,7 @@ enum clk_types {
* @off: register offset
* @bit: ON/MON bit
* @is_coupled: flag to indicate coupled clock
+ * @no_pm: flag to indicate if clock doesn't support PM
*/
struct rzg2l_mod_clk {
const char *name;
@@ -184,9 +185,10 @@ struct rzg2l_mod_clk {
u16 off;
u8 bit;
bool is_coupled;
+ bool no_pm;
};
-#define DEF_MOD_BASE(_name, _id, _parent, _off, _bit, _is_coupled) \
+#define DEF_MOD_BASE(_name, _id, _parent, _off, _bit, _is_coupled, _no_pm) \
{ \
.name = _name, \
.id = MOD_CLK_BASE + (_id), \
@@ -194,13 +196,17 @@ struct rzg2l_mod_clk {
.off = (_off), \
.bit = (_bit), \
.is_coupled = (_is_coupled), \
+ .no_pm = (_no_pm), \
}
#define DEF_MOD(_name, _id, _parent, _off, _bit) \
- DEF_MOD_BASE(_name, _id, _parent, _off, _bit, false)
+ DEF_MOD_BASE(_name, _id, _parent, _off, _bit, false, false)
#define DEF_COUPLED(_name, _id, _parent, _off, _bit) \
- DEF_MOD_BASE(_name, _id, _parent, _off, _bit, true)
+ DEF_MOD_BASE(_name, _id, _parent, _off, _bit, true, false)
+
+#define DEF_NO_PM(_name, _id, _parent, _off, _bit) \
+ DEF_MOD_BASE(_name, _id, _parent, _off, _bit, false, true)
/**
* struct rzg2l_reset - Reset definitions
--
2.25.1
From: Lad Prabhakar <[email protected]>
CRU_SYSCLK and CRU_VCLK clocks need to be turned ON/OFF in particular
sequence for the CRU block hence use DEF_NO_PM() to set the no_pm flag.
Signed-off-by: Lad Prabhakar <[email protected]>
---
RFC->v1
* No change
---
drivers/clk/renesas/r9a07g044-cpg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index 6935441f7504..31b1437c57e4 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -252,9 +252,9 @@ static const struct {
0x558, 1),
DEF_MOD("gpu_ace_clk", R9A07G044_GPU_ACE_CLK, R9A07G044_CLK_P1,
0x558, 2),
- DEF_MOD("cru_sysclk", R9A07G044_CRU_SYSCLK, CLK_M2_DIV2,
+ DEF_NO_PM("cru_sysclk", R9A07G044_CRU_SYSCLK, CLK_M2_DIV2,
0x564, 0),
- DEF_MOD("cru_vclk", R9A07G044_CRU_VCLK, R9A07G044_CLK_M2,
+ DEF_NO_PM("cru_vclk", R9A07G044_CRU_VCLK, R9A07G044_CLK_M2,
0x564, 1),
DEF_MOD("cru_pclk", R9A07G044_CRU_PCLK, R9A07G044_CLK_ZT,
0x564, 2),
--
2.25.1
Hi Prabhakar,
On Thu, Sep 29, 2022 at 8:51 PM Prabhakar <[email protected]> wrote:
> From: Lad Prabhakar <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 -- [email protected]
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 Tue, Oct 25, 2022 at 9:56 AM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Thu, Sep 29, 2022 at 8:51 PM Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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