| Message ID | 20240307140728.190184-9-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | clk: renesas: rzg2l: Add support for power domains | expand |
Hi Claudiu, On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > RZ/G3S supports deep sleep states that it can reach with the help of the > TF-A. > > RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while > Linux is running. These domains are initialized (and powered on) when > clock driver is probed. > > As the TF-A takes control at the very last(suspend)/first(resume) > phase of configuring the deep sleep state, it can do it's own settings on > power domains. > > Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which > powers on the always-on domains and rzg2l_cpg_complete() which activates > the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}. > > Along with it, added the suspend_check member to the RZ/G2L power domain > data structure whose purpose is to checks if a domain can be powered off > while the system is going to suspend. This is necessary for the serial > console domain which needs to be powered on if no_console_suspend is > available in bootargs. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > > Changes in v2: > - none; this patch is new Thanks for your patch! > --- a/drivers/clk/renesas/rzg2l-cpg.c > +++ b/drivers/clk/renesas/rzg2l-cpg.c > @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) > } else { > pd->genpd.power_on = rzg2l_cpg_power_on; > pd->genpd.power_off = rzg2l_cpg_power_off; > + if (flags & RZG2L_PD_F_CONSOLE) I think this should be replaced by some dynamic check, cfr. my comments on PATCH 9/10. > + pd->suspend_check = rzg2l_pd_suspend_check_console; > governor = &simple_qos_governor; > } > > @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev) > if (error) > return error; > > + dev_set_drvdata(dev, priv); > + > return 0; > } > > +static int rzg2l_cpg_resume(struct device *dev) > +{ > + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); > + const struct rzg2l_cpg_info *info = priv->info; > + > + /* Power on always ON domains. */ > + for (unsigned int i = 0; i < info->num_pm_domains; i++) { > + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { If you would check "priv-domains[i].flags & GENPD_FLAG_ALWAYS_ON" instead, I think you can make r9a08g045_pm_domains[] __initconst. You may need to make a copy of the name for pd->genpd.name, though. > + int ret = rzg2l_cpg_power_on(priv->domains[i]); I assume you are sure none of these domains are enabled by TF/A after system resume, or by the pmdomain core code? Gr{oetje,eeting}s, Geert
Hi, Geert, Sorry for replying that late to this one. On 18.03.2024 18:48, Geert Uytterhoeven wrote: > Hi Claudiu, > > On Thu, Mar 7, 2024 at 3:07 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> RZ/G3S supports deep sleep states that it can reach with the help of the >> TF-A. >> >> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while >> Linux is running. These domains are initialized (and powered on) when >> clock driver is probed. >> >> As the TF-A takes control at the very last(suspend)/first(resume) >> phase of configuring the deep sleep state, it can do it's own settings on >> power domains. >> >> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which >> powers on the always-on domains and rzg2l_cpg_complete() which activates >> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}. >> >> Along with it, added the suspend_check member to the RZ/G2L power domain >> data structure whose purpose is to checks if a domain can be powered off >> while the system is going to suspend. This is necessary for the serial >> console domain which needs to be powered on if no_console_suspend is >> available in bootargs. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> --- >> >> Changes in v2: >> - none; this patch is new > > Thanks for your patch! > >> --- a/drivers/clk/renesas/rzg2l-cpg.c >> +++ b/drivers/clk/renesas/rzg2l-cpg.c >> @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) >> } else { >> pd->genpd.power_on = rzg2l_cpg_power_on; >> pd->genpd.power_off = rzg2l_cpg_power_off; >> + if (flags & RZG2L_PD_F_CONSOLE) > > I think this should be replaced by some dynamic check, cfr. my comments > on PATCH 9/10. I agree. > >> + pd->suspend_check = rzg2l_pd_suspend_check_console; >> governor = &simple_qos_governor; >> } >> > >> @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev) >> if (error) >> return error; >> >> + dev_set_drvdata(dev, priv); >> + >> return 0; >> } >> >> +static int rzg2l_cpg_resume(struct device *dev) >> +{ >> + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); >> + const struct rzg2l_cpg_info *info = priv->info; >> + >> + /* Power on always ON domains. */ >> + for (unsigned int i = 0; i < info->num_pm_domains; i++) { >> + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { > > If you would check "priv-domains[i].flags & GENPD_FLAG_ALWAYS_ON" > instead, I think you can make r9a08g045_pm_domains[] __initconst. > You may need to make a copy of the name for pd->genpd.name, though. I wanted to avoid this copy. > >> + int ret = rzg2l_cpg_power_on(priv->domains[i]); > > I assume you are sure none of these domains are enabled by TF/A after > system resume, or by the pmdomain core code? Out of TF-A the MSTOP and PWRDN bits for these ones are set and setting CPG_PWRDN_MSTOP though rzg2l_cpg_complete() leads to system being blocked. It is the same as in booting case exlained in cover letter. "the DDR, TZCDDR, OTFDE_DDR were also added, to avoid system being blocked due to the following lines of code from patch 6/10. + /* Prepare for power down the BUSes in power down mode. */ + if (info->pm_domain_pwrdn_mstop) + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); Domain IDs were added to all SoC specific bindings. " The PM domain core code doesn't touch these domains while resuming as of my checkings. Thank you, Claudiu Beznea > > Gr{oetje,eeting}s, > > Geert >
On Thu, 7 Mar 2024 at 15:10, Claudiu <claudiu.beznea@tuxon.dev> wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > RZ/G3S supports deep sleep states that it can reach with the help of the > TF-A. > > RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while > Linux is running. These domains are initialized (and powered on) when > clock driver is probed. > > As the TF-A takes control at the very last(suspend)/first(resume) > phase of configuring the deep sleep state, it can do it's own settings on > power domains. For my understanding, can you please elaborate on this part a bit. What does the "last suspend/resume phase" mean, more exactly, here? > > Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which > powers on the always-on domains and rzg2l_cpg_complete() which activates > the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}. > > Along with it, added the suspend_check member to the RZ/G2L power domain > data structure whose purpose is to checks if a domain can be powered off > while the system is going to suspend. This is necessary for the serial > console domain which needs to be powered on if no_console_suspend is > available in bootargs. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > > Changes in v2: > - none; this patch is new > > drivers/clk/renesas/rzg2l-cpg.c | 66 ++++++++++++++++++++++++++++++--- > drivers/clk/renesas/rzg2l-cpg.h | 1 + > 2 files changed, 62 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c > index b36700f4a9f5..b18af227177e 100644 > --- a/drivers/clk/renesas/rzg2l-cpg.c > +++ b/drivers/clk/renesas/rzg2l-cpg.c > @@ -15,6 +15,7 @@ > #include <linux/clk.h> > #include <linux/clk-provider.h> > #include <linux/clk/renesas.h> > +#include <linux/console.h> > #include <linux/delay.h> > #include <linux/device.h> > #include <linux/init.h> > @@ -139,6 +140,7 @@ struct rzg2l_pll5_mux_dsi_div_param { > * @num_resets: Number of Module Resets in info->resets[] > * @last_dt_core_clk: ID of the last Core Clock exported to DT > * @info: Pointer to platform data > + * @domains: generic PM domains > * @mux_dsi_div_params: pll5 mux and dsi div parameters > */ > struct rzg2l_cpg_priv { > @@ -155,6 +157,8 @@ struct rzg2l_cpg_priv { > > const struct rzg2l_cpg_info *info; > > + struct generic_pm_domain **domains; > + > struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params; > }; > > @@ -1570,12 +1574,14 @@ struct rzg2l_cpg_pm_domains { > * struct rzg2l_cpg_pd - RZ/G2L power domain data structure > * @genpd: generic PM domain > * @priv: pointer to CPG private data structure > + * @suspend_check: check if domain could be powered off in suspend > * @conf: CPG PM domain configuration info > * @id: RZ/G2L power domain ID > */ > struct rzg2l_cpg_pd { > struct generic_pm_domain genpd; > struct rzg2l_cpg_priv *priv; > + int (*suspend_check)(void); > struct rzg2l_cpg_pm_domain_conf conf; > u16 id; > }; > @@ -1676,6 +1682,13 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain) > struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn; > struct rzg2l_cpg_priv *priv = pd->priv; > > + if (pd->suspend_check) { > + int ret = pd->suspend_check(); > + > + if (ret) > + return ret; > + } > + This should not be needed at all, I think. Instead, genpd should be able to take the correct decision during system-wide suspend and simply avoid calling the ->power_off() callback, when that is needed. If I understand correctly, GENPD_FLAG_ACTIVE_WAKEUP is set for the genpd in question. The only remaining thing would then be to let the console driver, during system suspend, check whether "console_suspend_enabled" is set and then call device_set_awake_path() for its device. In this way, genpd should then keep the corresponding PM domain powered-on. > /* Set MSTOP. */ > if (mstop.mask) > writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off); > @@ -1687,8 +1700,14 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain) > return 0; > } > > -static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) > +static int rzg2l_pd_suspend_check_console(void) > { > + return console_suspend_enabled ? 0 : -EBUSY; > +} > + > +static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, u32 flags) > +{ > + bool always_on = !!(flags & RZG2L_PD_F_ALWAYS_ON); > struct dev_power_governor *governor; > > pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP; > @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) > } else { > pd->genpd.power_on = rzg2l_cpg_power_on; > pd->genpd.power_off = rzg2l_cpg_power_off; > + if (flags & RZG2L_PD_F_CONSOLE) > + pd->suspend_check = rzg2l_pd_suspend_check_console; > governor = &simple_qos_governor; > } > > @@ -1719,7 +1740,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv) > > pd->genpd.name = np->name; > pd->priv = priv; > - ret = rzg2l_cpg_pd_setup(pd, true); > + ret = rzg2l_cpg_pd_setup(pd, RZG2L_PD_F_ALWAYS_ON); > if (ret) > return ret; > > @@ -1778,13 +1799,13 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv) > domains->onecell_data.domains = domains->domains; > domains->onecell_data.num_domains = info->num_pm_domains; > domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate; > + priv->domains = domains->domains; > > ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data); > if (ret) > return ret; > > for (unsigned int i = 0; i < info->num_pm_domains; i++) { > - bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON); > struct rzg2l_cpg_pd *pd; > > pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); > @@ -1796,11 +1817,11 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv) > pd->id = info->pm_domains[i].id; > pd->priv = priv; > > - ret = rzg2l_cpg_pd_setup(pd, always_on); > + ret = rzg2l_cpg_pd_setup(pd, info->pm_domains[i].flags); > if (ret) > return ret; > > - if (always_on) { > + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { > ret = rzg2l_cpg_power_on(&pd->genpd); > if (ret) > return ret; > @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev) > if (error) > return error; > > + dev_set_drvdata(dev, priv); > + > return 0; > } > > +static int rzg2l_cpg_resume(struct device *dev) > +{ > + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); > + const struct rzg2l_cpg_info *info = priv->info; > + > + /* Power on always ON domains. */ > + for (unsigned int i = 0; i < info->num_pm_domains; i++) { > + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { > + int ret = rzg2l_cpg_power_on(priv->domains[i]); > + > + if (ret) > + return ret; > + } > + } I don't quite understand why this is needed? Is always-on PM domains being powered-off during system wide suspend, so you need to power them on again? > + > + return 0; > +} > + > +static void rzg2l_cpg_complete(struct device *dev) > +{ > + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); > + > + /* Prepare for power down the BUSes in power down mode. */ > + if (priv->info->pm_domain_pwrdn_mstop) > + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); > +} > + > +static const struct dev_pm_ops rzg2l_cpg_pm_ops = { > + NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, rzg2l_cpg_resume) > + .complete = rzg2l_cpg_complete, > +}; > + > static const struct of_device_id rzg2l_cpg_match[] = { > #ifdef CONFIG_CLK_R9A07G043 > { > @@ -1931,6 +1986,7 @@ static struct platform_driver rzg2l_cpg_driver = { > .driver = { > .name = "rzg2l-cpg", > .of_match_table = rzg2l_cpg_match, > + .pm = pm_sleep_ptr(&rzg2l_cpg_pm_ops), > }, > }; > > diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h > index d9a7357c4873..abff85644270 100644 > --- a/drivers/clk/renesas/rzg2l-cpg.h > +++ b/drivers/clk/renesas/rzg2l-cpg.h > @@ -301,6 +301,7 @@ struct rzg2l_cpg_pm_domain_init_data { > > /* Power domain flags. */ > #define RZG2L_PD_F_ALWAYS_ON BIT(0) > +#define RZG2L_PD_F_CONSOLE BIT(1) > #define RZG2L_PD_F_NONE (0) > Kind regards Uffe
Hi, Ulf, On 16.04.2024 15:07, Ulf Hansson wrote: > On Thu, 7 Mar 2024 at 15:10, Claudiu <claudiu.beznea@tuxon.dev> wrote: >> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> RZ/G3S supports deep sleep states that it can reach with the help of the >> TF-A. >> >> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while >> Linux is running. These domains are initialized (and powered on) when >> clock driver is probed. >> >> As the TF-A takes control at the very last(suspend)/first(resume) >> phase of configuring the deep sleep state, it can do it's own settings on >> power domains. > > For my understanding, can you please elaborate on this part a bit. > What does the "last suspend/resume phase" mean, more exactly, here? The RZ/G3S SoC support a power saving mode where most of the SoC parts are turned off and the system RAM is switched to retention mode. This is done with the help of TF-A. The handshake b/w Linux and TF-A is done though the drivers/firmware/psci/psci.c driver. After Linux finishes the execution of suspend code the control is taken by TF-A. TF-A does the final settings on the system (e.g. switching the RAM to retention mode) and power off most of the SoC parts. By the last phase of the suspend I'm referring to the TF-A doing the final adjustments for the system to switch to this power saving mode. When resuming, as the TF-A is the 1st one being executed on the system (this is what I called above the 1st phase of the resume), TF-A moves the DDR out of retention mode, reconfigure basic IPs (like in boot case as most of the SoC parts were powered off) and then give the control to Linux which will execute the resume code. > >> >> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which >> powers on the always-on domains and rzg2l_cpg_complete() which activates >> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}. >> >> Along with it, added the suspend_check member to the RZ/G2L power domain >> data structure whose purpose is to checks if a domain can be powered off >> while the system is going to suspend. This is necessary for the serial >> console domain which needs to be powered on if no_console_suspend is >> available in bootargs. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> --- >> >> Changes in v2: >> - none; this patch is new >> >> drivers/clk/renesas/rzg2l-cpg.c | 66 ++++++++++++++++++++++++++++++--- >> drivers/clk/renesas/rzg2l-cpg.h | 1 + >> 2 files changed, 62 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c >> index b36700f4a9f5..b18af227177e 100644 >> --- a/drivers/clk/renesas/rzg2l-cpg.c >> +++ b/drivers/clk/renesas/rzg2l-cpg.c >> @@ -15,6 +15,7 @@ >> #include <linux/clk.h> >> #include <linux/clk-provider.h> >> #include <linux/clk/renesas.h> >> +#include <linux/console.h> >> #include <linux/delay.h> >> #include <linux/device.h> >> #include <linux/init.h> >> @@ -139,6 +140,7 @@ struct rzg2l_pll5_mux_dsi_div_param { >> * @num_resets: Number of Module Resets in info->resets[] >> * @last_dt_core_clk: ID of the last Core Clock exported to DT >> * @info: Pointer to platform data >> + * @domains: generic PM domains >> * @mux_dsi_div_params: pll5 mux and dsi div parameters >> */ >> struct rzg2l_cpg_priv { >> @@ -155,6 +157,8 @@ struct rzg2l_cpg_priv { >> >> const struct rzg2l_cpg_info *info; >> >> + struct generic_pm_domain **domains; >> + >> struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params; >> }; >> >> @@ -1570,12 +1574,14 @@ struct rzg2l_cpg_pm_domains { >> * struct rzg2l_cpg_pd - RZ/G2L power domain data structure >> * @genpd: generic PM domain >> * @priv: pointer to CPG private data structure >> + * @suspend_check: check if domain could be powered off in suspend >> * @conf: CPG PM domain configuration info >> * @id: RZ/G2L power domain ID >> */ >> struct rzg2l_cpg_pd { >> struct generic_pm_domain genpd; >> struct rzg2l_cpg_priv *priv; >> + int (*suspend_check)(void); >> struct rzg2l_cpg_pm_domain_conf conf; >> u16 id; >> }; >> @@ -1676,6 +1682,13 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain) >> struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn; >> struct rzg2l_cpg_priv *priv = pd->priv; >> >> + if (pd->suspend_check) { >> + int ret = pd->suspend_check(); >> + >> + if (ret) >> + return ret; >> + } >> + > > This should not be needed at all, I think. > > Instead, genpd should be able to take the correct decision during > system-wide suspend and simply avoid calling the ->power_off() > callback, when that is needed. > > If I understand correctly, GENPD_FLAG_ACTIVE_WAKEUP is set for the > genpd in question. The only remaining thing would then be to let the > console driver, during system suspend, check whether > "console_suspend_enabled" is set and then call device_set_awake_path() > for its device. In this way, genpd should then keep the corresponding > PM domain powered-on. You're right! I've checked it and all good w/o ->suspend_check() if device_set_wakeup_path() is called for the console driver. I'll send an update for it. > >> /* Set MSTOP. */ >> if (mstop.mask) >> writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off); >> @@ -1687,8 +1700,14 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain) >> return 0; >> } >> >> -static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) >> +static int rzg2l_pd_suspend_check_console(void) >> { >> + return console_suspend_enabled ? 0 : -EBUSY; >> +} >> + >> +static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, u32 flags) >> +{ >> + bool always_on = !!(flags & RZG2L_PD_F_ALWAYS_ON); >> struct dev_power_governor *governor; >> >> pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP; >> @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) >> } else { >> pd->genpd.power_on = rzg2l_cpg_power_on; >> pd->genpd.power_off = rzg2l_cpg_power_off; >> + if (flags & RZG2L_PD_F_CONSOLE) >> + pd->suspend_check = rzg2l_pd_suspend_check_console; >> governor = &simple_qos_governor; >> } >> >> @@ -1719,7 +1740,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv) >> >> pd->genpd.name = np->name; >> pd->priv = priv; >> - ret = rzg2l_cpg_pd_setup(pd, true); >> + ret = rzg2l_cpg_pd_setup(pd, RZG2L_PD_F_ALWAYS_ON); >> if (ret) >> return ret; >> >> @@ -1778,13 +1799,13 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv) >> domains->onecell_data.domains = domains->domains; >> domains->onecell_data.num_domains = info->num_pm_domains; >> domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate; >> + priv->domains = domains->domains; >> >> ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data); >> if (ret) >> return ret; >> >> for (unsigned int i = 0; i < info->num_pm_domains; i++) { >> - bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON); >> struct rzg2l_cpg_pd *pd; >> >> pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); >> @@ -1796,11 +1817,11 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv) >> pd->id = info->pm_domains[i].id; >> pd->priv = priv; >> >> - ret = rzg2l_cpg_pd_setup(pd, always_on); >> + ret = rzg2l_cpg_pd_setup(pd, info->pm_domains[i].flags); >> if (ret) >> return ret; >> >> - if (always_on) { >> + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { >> ret = rzg2l_cpg_power_on(&pd->genpd); >> if (ret) >> return ret; >> @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev) >> if (error) >> return error; >> >> + dev_set_drvdata(dev, priv); >> + >> return 0; >> } >> >> +static int rzg2l_cpg_resume(struct device *dev) >> +{ >> + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); >> + const struct rzg2l_cpg_info *info = priv->info; >> + >> + /* Power on always ON domains. */ >> + for (unsigned int i = 0; i < info->num_pm_domains; i++) { >> + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { >> + int ret = rzg2l_cpg_power_on(priv->domains[i]); >> + >> + if (ret) >> + return ret; >> + } >> + } > > I don't quite understand why this is needed? Is always-on PM domains > being powered-off during system wide suspend, so you need to power > them on again? Yes, as power to most of the system parts is cut off during sytem suspend (and DDR is kept in retention mode) and the resume is almost like a cold boot where the TF-A does basic re-initialization and then pass execution to Linux resume code. Thank you, Claudiu Beznea > >> + >> + return 0; >> +} >> + >> +static void rzg2l_cpg_complete(struct device *dev) >> +{ >> + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); >> + >> + /* Prepare for power down the BUSes in power down mode. */ >> + if (priv->info->pm_domain_pwrdn_mstop) >> + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); >> +} >> + >> +static const struct dev_pm_ops rzg2l_cpg_pm_ops = { >> + NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, rzg2l_cpg_resume) >> + .complete = rzg2l_cpg_complete, >> +}; >> + >> static const struct of_device_id rzg2l_cpg_match[] = { >> #ifdef CONFIG_CLK_R9A07G043 >> { >> @@ -1931,6 +1986,7 @@ static struct platform_driver rzg2l_cpg_driver = { >> .driver = { >> .name = "rzg2l-cpg", >> .of_match_table = rzg2l_cpg_match, >> + .pm = pm_sleep_ptr(&rzg2l_cpg_pm_ops), >> }, >> }; >> >> diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h >> index d9a7357c4873..abff85644270 100644 >> --- a/drivers/clk/renesas/rzg2l-cpg.h >> +++ b/drivers/clk/renesas/rzg2l-cpg.h >> @@ -301,6 +301,7 @@ struct rzg2l_cpg_pm_domain_init_data { >> >> /* Power domain flags. */ >> #define RZG2L_PD_F_ALWAYS_ON BIT(0) >> +#define RZG2L_PD_F_CONSOLE BIT(1) >> #define RZG2L_PD_F_NONE (0) >> > > Kind regards > Uffe
On Wed, 17 Apr 2024 at 10:05, claudiu beznea <claudiu.beznea@tuxon.dev> wrote: > > Hi, Ulf, > > On 16.04.2024 15:07, Ulf Hansson wrote: > > On Thu, 7 Mar 2024 at 15:10, Claudiu <claudiu.beznea@tuxon.dev> wrote: > >> > >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >> > >> RZ/G3S supports deep sleep states that it can reach with the help of the > >> TF-A. > >> > >> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while > >> Linux is running. These domains are initialized (and powered on) when > >> clock driver is probed. > >> > >> As the TF-A takes control at the very last(suspend)/first(resume) > >> phase of configuring the deep sleep state, it can do it's own settings on > >> power domains. > > > > For my understanding, can you please elaborate on this part a bit. > > What does the "last suspend/resume phase" mean, more exactly, here? > > The RZ/G3S SoC support a power saving mode where most of the SoC parts are > turned off and the system RAM is switched to retention mode. This is done > with the help of TF-A. The handshake b/w Linux and TF-A is done though the > drivers/firmware/psci/psci.c driver. > > After Linux finishes the execution of suspend code the control is taken by > TF-A. TF-A does the final settings on the system (e.g. switching the RAM to > retention mode) and power off most of the SoC parts. > > By the last phase of the suspend I'm referring to the TF-A doing the final > adjustments for the system to switch to this power saving mode. > > When resuming, as the TF-A is the 1st one being executed on the system > (this is what I called above the 1st phase of the resume), TF-A moves the > DDR out of retention mode, reconfigure basic IPs (like in boot case as most > of the SoC parts were powered off) and then give the control to Linux which > will execute the resume code. Alright, thanks for clarifying! This makes sense to me now! > > > > > >> > >> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which > >> powers on the always-on domains and rzg2l_cpg_complete() which activates > >> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}. > >> > >> Along with it, added the suspend_check member to the RZ/G2L power domain > >> data structure whose purpose is to checks if a domain can be powered off > >> while the system is going to suspend. This is necessary for the serial > >> console domain which needs to be powered on if no_console_suspend is > >> available in bootargs. > >> > >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >> --- > >> > >> Changes in v2: > >> - none; this patch is new > >> > >> drivers/clk/renesas/rzg2l-cpg.c | 66 ++++++++++++++++++++++++++++++--- > >> drivers/clk/renesas/rzg2l-cpg.h | 1 + > >> 2 files changed, 62 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c > >> index b36700f4a9f5..b18af227177e 100644 > >> --- a/drivers/clk/renesas/rzg2l-cpg.c > >> +++ b/drivers/clk/renesas/rzg2l-cpg.c > >> @@ -15,6 +15,7 @@ > >> #include <linux/clk.h> > >> #include <linux/clk-provider.h> > >> #include <linux/clk/renesas.h> > >> +#include <linux/console.h> > >> #include <linux/delay.h> > >> #include <linux/device.h> > >> #include <linux/init.h> > >> @@ -139,6 +140,7 @@ struct rzg2l_pll5_mux_dsi_div_param { > >> * @num_resets: Number of Module Resets in info->resets[] > >> * @last_dt_core_clk: ID of the last Core Clock exported to DT > >> * @info: Pointer to platform data > >> + * @domains: generic PM domains > >> * @mux_dsi_div_params: pll5 mux and dsi div parameters > >> */ > >> struct rzg2l_cpg_priv { > >> @@ -155,6 +157,8 @@ struct rzg2l_cpg_priv { > >> > >> const struct rzg2l_cpg_info *info; > >> > >> + struct generic_pm_domain **domains; > >> + > >> struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params; > >> }; > >> > >> @@ -1570,12 +1574,14 @@ struct rzg2l_cpg_pm_domains { > >> * struct rzg2l_cpg_pd - RZ/G2L power domain data structure > >> * @genpd: generic PM domain > >> * @priv: pointer to CPG private data structure > >> + * @suspend_check: check if domain could be powered off in suspend > >> * @conf: CPG PM domain configuration info > >> * @id: RZ/G2L power domain ID > >> */ > >> struct rzg2l_cpg_pd { > >> struct generic_pm_domain genpd; > >> struct rzg2l_cpg_priv *priv; > >> + int (*suspend_check)(void); > >> struct rzg2l_cpg_pm_domain_conf conf; > >> u16 id; > >> }; > >> @@ -1676,6 +1682,13 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain) > >> struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn; > >> struct rzg2l_cpg_priv *priv = pd->priv; > >> > >> + if (pd->suspend_check) { > >> + int ret = pd->suspend_check(); > >> + > >> + if (ret) > >> + return ret; > >> + } > >> + > > > > This should not be needed at all, I think. > > > > Instead, genpd should be able to take the correct decision during > > system-wide suspend and simply avoid calling the ->power_off() > > callback, when that is needed. > > > > If I understand correctly, GENPD_FLAG_ACTIVE_WAKEUP is set for the > > genpd in question. The only remaining thing would then be to let the > > console driver, during system suspend, check whether > > "console_suspend_enabled" is set and then call device_set_awake_path() > > for its device. In this way, genpd should then keep the corresponding > > PM domain powered-on. > > You're right! I've checked it and all good w/o ->suspend_check() if > device_set_wakeup_path() is called for the console driver. > > I'll send an update for it. Great! Please keep me posted on the entire series for next version. I will try to continue to help review this. > > > > >> /* Set MSTOP. */ > >> if (mstop.mask) > >> writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off); > >> @@ -1687,8 +1700,14 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain) > >> return 0; > >> } > >> > >> -static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) > >> +static int rzg2l_pd_suspend_check_console(void) > >> { > >> + return console_suspend_enabled ? 0 : -EBUSY; > >> +} > >> + > >> +static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, u32 flags) > >> +{ > >> + bool always_on = !!(flags & RZG2L_PD_F_ALWAYS_ON); > >> struct dev_power_governor *governor; > >> > >> pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP; > >> @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) > >> } else { > >> pd->genpd.power_on = rzg2l_cpg_power_on; > >> pd->genpd.power_off = rzg2l_cpg_power_off; > >> + if (flags & RZG2L_PD_F_CONSOLE) > >> + pd->suspend_check = rzg2l_pd_suspend_check_console; > >> governor = &simple_qos_governor; > >> } > >> > >> @@ -1719,7 +1740,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv) > >> > >> pd->genpd.name = np->name; > >> pd->priv = priv; > >> - ret = rzg2l_cpg_pd_setup(pd, true); > >> + ret = rzg2l_cpg_pd_setup(pd, RZG2L_PD_F_ALWAYS_ON); > >> if (ret) > >> return ret; > >> > >> @@ -1778,13 +1799,13 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv) > >> domains->onecell_data.domains = domains->domains; > >> domains->onecell_data.num_domains = info->num_pm_domains; > >> domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate; > >> + priv->domains = domains->domains; > >> > >> ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data); > >> if (ret) > >> return ret; > >> > >> for (unsigned int i = 0; i < info->num_pm_domains; i++) { > >> - bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON); > >> struct rzg2l_cpg_pd *pd; > >> > >> pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); > >> @@ -1796,11 +1817,11 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv) > >> pd->id = info->pm_domains[i].id; > >> pd->priv = priv; > >> > >> - ret = rzg2l_cpg_pd_setup(pd, always_on); > >> + ret = rzg2l_cpg_pd_setup(pd, info->pm_domains[i].flags); > >> if (ret) > >> return ret; > >> > >> - if (always_on) { > >> + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { > >> ret = rzg2l_cpg_power_on(&pd->genpd); > >> if (ret) > >> return ret; > >> @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev) > >> if (error) > >> return error; > >> > >> + dev_set_drvdata(dev, priv); > >> + > >> return 0; > >> } > >> > >> +static int rzg2l_cpg_resume(struct device *dev) > >> +{ > >> + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); > >> + const struct rzg2l_cpg_info *info = priv->info; > >> + > >> + /* Power on always ON domains. */ > >> + for (unsigned int i = 0; i < info->num_pm_domains; i++) { > >> + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { > >> + int ret = rzg2l_cpg_power_on(priv->domains[i]); > >> + > >> + if (ret) > >> + return ret; > >> + } > >> + } > > > > I don't quite understand why this is needed? Is always-on PM domains > > being powered-off during system wide suspend, so you need to power > > them on again? > > Yes, as power to most of the system parts is cut off during sytem suspend > (and DDR is kept in retention mode) and the resume is almost like a cold > boot where the TF-A does basic re-initialization and then pass execution to > Linux resume code. Hmm. If these are really always-on PM domains, why isn't the FW powering them on again then before returning to Linux after a system resume? In a way it sounds to me that they aren't really always-on PM domains, as Linux seems to be capable of turning them on/off too, right? That said, perhaps using GENPD_FLAG_RPM_ALWAYS_ON instead of GENPD_FLAG_ALWAYS_ON for some PM domains can be another way forward? In this way, the ->power_on|off() callbacks can be used to turn on/off the PM domains, but only during system suspend/resume. Would that work? [...] Kind regards Uffe
On 17.04.2024 12:39, Ulf Hansson wrote: > On Wed, 17 Apr 2024 at 10:05, claudiu beznea <claudiu.beznea@tuxon.dev> wrote: >> >> Hi, Ulf, >> >> On 16.04.2024 15:07, Ulf Hansson wrote: >>> On Thu, 7 Mar 2024 at 15:10, Claudiu <claudiu.beznea@tuxon.dev> wrote: >>>> >>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>>> >>>> RZ/G3S supports deep sleep states that it can reach with the help of the >>>> TF-A. >>>> >>>> RZ/G3S has a few power domains (e.g. GIC) that need to be always-on while >>>> Linux is running. These domains are initialized (and powered on) when >>>> clock driver is probed. >>>> >>>> As the TF-A takes control at the very last(suspend)/first(resume) >>>> phase of configuring the deep sleep state, it can do it's own settings on >>>> power domains. >>> >>> For my understanding, can you please elaborate on this part a bit. >>> What does the "last suspend/resume phase" mean, more exactly, here? >> >> The RZ/G3S SoC support a power saving mode where most of the SoC parts are >> turned off and the system RAM is switched to retention mode. This is done >> with the help of TF-A. The handshake b/w Linux and TF-A is done though the >> drivers/firmware/psci/psci.c driver. >> >> After Linux finishes the execution of suspend code the control is taken by >> TF-A. TF-A does the final settings on the system (e.g. switching the RAM to >> retention mode) and power off most of the SoC parts. >> >> By the last phase of the suspend I'm referring to the TF-A doing the final >> adjustments for the system to switch to this power saving mode. >> >> When resuming, as the TF-A is the 1st one being executed on the system >> (this is what I called above the 1st phase of the resume), TF-A moves the >> DDR out of retention mode, reconfigure basic IPs (like in boot case as most >> of the SoC parts were powered off) and then give the control to Linux which >> will execute the resume code. > > Alright, thanks for clarifying! This makes sense to me now! > >> >> >>> >>>> >>>> Thus, to restore the proper Linux state, add rzg2l_cpg_resume() which >>>> powers on the always-on domains and rzg2l_cpg_complete() which activates >>>> the power down mode for the IPs selected through CPG_PWRDN_IP{1, 2}. >>>> >>>> Along with it, added the suspend_check member to the RZ/G2L power domain >>>> data structure whose purpose is to checks if a domain can be powered off >>>> while the system is going to suspend. This is necessary for the serial >>>> console domain which needs to be powered on if no_console_suspend is >>>> available in bootargs. >>>> >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >>>> --- [ ... ] >>>> >>>> +static int rzg2l_cpg_resume(struct device *dev) >>>> +{ >>>> + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); >>>> + const struct rzg2l_cpg_info *info = priv->info; >>>> + >>>> + /* Power on always ON domains. */ >>>> + for (unsigned int i = 0; i < info->num_pm_domains; i++) { >>>> + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { >>>> + int ret = rzg2l_cpg_power_on(priv->domains[i]); >>>> + >>>> + if (ret) >>>> + return ret; >>>> + } >>>> + } >>> >>> I don't quite understand why this is needed? Is always-on PM domains >>> being powered-off during system wide suspend, so you need to power >>> them on again? >> >> Yes, as power to most of the system parts is cut off during sytem suspend >> (and DDR is kept in retention mode) and the resume is almost like a cold >> boot where the TF-A does basic re-initialization and then pass execution to >> Linux resume code. > > Hmm. If these are really always-on PM domains, why isn't the FW > powering them on again then before returning to Linux after a system > resume? I'll try to explain it better. The power domain implementation in this series tries to abstract the control of bus clock (though MSTOP registers) for individual modules available in RZ/G3S SoC. From hardware manual [1]: "The Module Standby Mode is a mode that requests the clock stop of the module specified by the master. The purpose of this mode is to reduce power consumption by stopping unnecessary functions." MSTOP is connected to individual modules as described in this picture: https://paste.pics/726c963c33a506651a4be5f327e2a46d There is also the PWRDN functionality for the modules that are part of the PD_ISOVCC domain. At the time of writing this series there was not much information in hardware manual about PWRDN functionality. The design team has been asked but there is no clear answer ATM if the sequence of using PWRDN in Linux (as proposed in this series) is good or not. I encountered no issues with it while experimenting thus I have kept it. If you prefer I can drop it and return with something afterwards, if any. As I said in the current implementation I also used the PWRDN. The PWRDN is IP specific but takes effect (as of my experiments) when this is executed: + /* Prepare for power down the BUSes in power down mode. */ + if (info->pm_domain_pwrdn_mstop) + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); As of my experiments having CPG_PWRDN_MSTOP.CPG_PWRDN_MSTOP_ENABLE set applies the PWRDN to all the IPs for which PWRDN bit has been set in CPG_PWRDN_IP1 and CPG_PWRD_IP2 registers. This settings are applied in probe after domains are powered (thus PWRDN bits are properly set up for IP supporting it by calling ->power_on()) and at the end of resume (after PWRDN bits are properly setup for IPs supporting it by calling ->power_on()) From my experiments, when returning from suspend (thus after firmware has been executed) the CPG_PWRDN_MSTOP_ENABLE bit at register CPG_PWRDN_MSTOP is zero. We power on the domains in Linux after resuming because of PWRDN functionality and CPG_PWRDN_MSTOP_ENABLE bit at register CPG_PWRDN_MSTOP. > > In a way it sounds to me that they aren't really always-on PM domains, > as Linux seems to be capable of turning them on/off too, right? Yes, Linux has the ability of controlling them by setting MSTOP and PWRDN bits. This is because the IP specific PWRDN functionality takes effect when CPG_PWRDN_MSTOP_ENABLE bit at register CPG_PWRDN_MSTOP is set (as of my experiments). > > That said, perhaps using GENPD_FLAG_RPM_ALWAYS_ON instead of > GENPD_FLAG_ALWAYS_ON for some PM domains can be another way forward? All this is becuase PWRD functionality. Explaining it to you made me consider that it would be better to just drop the PWRDN functionality at the moment until it is fully understood. With it I think we should be able to drop the rzg2l_cpg_power_on() in resume(), at least. What do you think? > In this way, the ->power_on|off() callbacks can be used to turn on/off > the PM domains, but only during system suspend/resume. Would that > work? I need to check it. Thank you for you review, Claudiu Beznea [1] https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzg3s-general-purpose-microprocessors-single-core-arm-cortex-a55-11-ghz-cpu-and-dual-core-cortex-m33-250 > > [...] > > Kind regards > Uffe
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index b36700f4a9f5..b18af227177e 100644 --- a/drivers/clk/renesas/rzg2l-cpg.c +++ b/drivers/clk/renesas/rzg2l-cpg.c @@ -15,6 +15,7 @@ #include <linux/clk.h> #include <linux/clk-provider.h> #include <linux/clk/renesas.h> +#include <linux/console.h> #include <linux/delay.h> #include <linux/device.h> #include <linux/init.h> @@ -139,6 +140,7 @@ struct rzg2l_pll5_mux_dsi_div_param { * @num_resets: Number of Module Resets in info->resets[] * @last_dt_core_clk: ID of the last Core Clock exported to DT * @info: Pointer to platform data + * @domains: generic PM domains * @mux_dsi_div_params: pll5 mux and dsi div parameters */ struct rzg2l_cpg_priv { @@ -155,6 +157,8 @@ struct rzg2l_cpg_priv { const struct rzg2l_cpg_info *info; + struct generic_pm_domain **domains; + struct rzg2l_pll5_mux_dsi_div_param mux_dsi_div_params; }; @@ -1570,12 +1574,14 @@ struct rzg2l_cpg_pm_domains { * struct rzg2l_cpg_pd - RZ/G2L power domain data structure * @genpd: generic PM domain * @priv: pointer to CPG private data structure + * @suspend_check: check if domain could be powered off in suspend * @conf: CPG PM domain configuration info * @id: RZ/G2L power domain ID */ struct rzg2l_cpg_pd { struct generic_pm_domain genpd; struct rzg2l_cpg_priv *priv; + int (*suspend_check)(void); struct rzg2l_cpg_pm_domain_conf conf; u16 id; }; @@ -1676,6 +1682,13 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain) struct rzg2l_cpg_reg_conf pwrdn = pd->conf.pwrdn; struct rzg2l_cpg_priv *priv = pd->priv; + if (pd->suspend_check) { + int ret = pd->suspend_check(); + + if (ret) + return ret; + } + /* Set MSTOP. */ if (mstop.mask) writel(mstop.mask | (mstop.mask << 16), priv->base + mstop.off); @@ -1687,8 +1700,14 @@ static int rzg2l_cpg_power_off(struct generic_pm_domain *domain) return 0; } -static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) +static int rzg2l_pd_suspend_check_console(void) { + return console_suspend_enabled ? 0 : -EBUSY; +} + +static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, u32 flags) +{ + bool always_on = !!(flags & RZG2L_PD_F_ALWAYS_ON); struct dev_power_governor *governor; pd->genpd.flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP; @@ -1700,6 +1719,8 @@ static int __init rzg2l_cpg_pd_setup(struct rzg2l_cpg_pd *pd, bool always_on) } else { pd->genpd.power_on = rzg2l_cpg_power_on; pd->genpd.power_off = rzg2l_cpg_power_off; + if (flags & RZG2L_PD_F_CONSOLE) + pd->suspend_check = rzg2l_pd_suspend_check_console; governor = &simple_qos_governor; } @@ -1719,7 +1740,7 @@ static int __init rzg2l_cpg_add_clk_domain(struct rzg2l_cpg_priv *priv) pd->genpd.name = np->name; pd->priv = priv; - ret = rzg2l_cpg_pd_setup(pd, true); + ret = rzg2l_cpg_pd_setup(pd, RZG2L_PD_F_ALWAYS_ON); if (ret) return ret; @@ -1778,13 +1799,13 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv) domains->onecell_data.domains = domains->domains; domains->onecell_data.num_domains = info->num_pm_domains; domains->onecell_data.xlate = rzg2l_cpg_pm_domain_xlate; + priv->domains = domains->domains; ret = devm_add_action_or_reset(dev, rzg2l_cpg_genpd_remove, &domains->onecell_data); if (ret) return ret; for (unsigned int i = 0; i < info->num_pm_domains; i++) { - bool always_on = !!(info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON); struct rzg2l_cpg_pd *pd; pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); @@ -1796,11 +1817,11 @@ static int __init rzg2l_cpg_add_pm_domains(struct rzg2l_cpg_priv *priv) pd->id = info->pm_domains[i].id; pd->priv = priv; - ret = rzg2l_cpg_pd_setup(pd, always_on); + ret = rzg2l_cpg_pd_setup(pd, info->pm_domains[i].flags); if (ret) return ret; - if (always_on) { + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { ret = rzg2l_cpg_power_on(&pd->genpd); if (ret) return ret; @@ -1890,9 +1911,43 @@ static int __init rzg2l_cpg_probe(struct platform_device *pdev) if (error) return error; + dev_set_drvdata(dev, priv); + return 0; } +static int rzg2l_cpg_resume(struct device *dev) +{ + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); + const struct rzg2l_cpg_info *info = priv->info; + + /* Power on always ON domains. */ + for (unsigned int i = 0; i < info->num_pm_domains; i++) { + if (info->pm_domains[i].flags & RZG2L_PD_F_ALWAYS_ON) { + int ret = rzg2l_cpg_power_on(priv->domains[i]); + + if (ret) + return ret; + } + } + + return 0; +} + +static void rzg2l_cpg_complete(struct device *dev) +{ + struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev); + + /* Prepare for power down the BUSes in power down mode. */ + if (priv->info->pm_domain_pwrdn_mstop) + writel(CPG_PWRDN_MSTOP_ENABLE, priv->base + CPG_PWRDN_MSTOP); +} + +static const struct dev_pm_ops rzg2l_cpg_pm_ops = { + NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, rzg2l_cpg_resume) + .complete = rzg2l_cpg_complete, +}; + static const struct of_device_id rzg2l_cpg_match[] = { #ifdef CONFIG_CLK_R9A07G043 { @@ -1931,6 +1986,7 @@ static struct platform_driver rzg2l_cpg_driver = { .driver = { .name = "rzg2l-cpg", .of_match_table = rzg2l_cpg_match, + .pm = pm_sleep_ptr(&rzg2l_cpg_pm_ops), }, }; diff --git a/drivers/clk/renesas/rzg2l-cpg.h b/drivers/clk/renesas/rzg2l-cpg.h index d9a7357c4873..abff85644270 100644 --- a/drivers/clk/renesas/rzg2l-cpg.h +++ b/drivers/clk/renesas/rzg2l-cpg.h @@ -301,6 +301,7 @@ struct rzg2l_cpg_pm_domain_init_data { /* Power domain flags. */ #define RZG2L_PD_F_ALWAYS_ON BIT(0) +#define RZG2L_PD_F_CONSOLE BIT(1) #define RZG2L_PD_F_NONE (0) /**