Messages in this thread | | | From | Peng Fan <> | Subject | RE: [PATCH V3 4/8] genpd: imx: scu-pd: do not power off console if no_console_suspend | Date | Mon, 14 Aug 2023 10:26:26 +0000 |
| |
> Subject: Re: [PATCH V3 4/8] genpd: imx: scu-pd: do not power off console if > no_console_suspend > > On Mon, 31 Jul 2023 at 08:43, Peng Fan (OSS) <peng.fan@oss.nxp.com> > wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > Do not power off console if no_console_suspend > > Perhaps extend this a bit to let the reader understand this is about leaving > the serial device's corresponding PM domain on.
ok
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > A comment below - that doesn't need to stop this from being applied > though. > > > --- > > drivers/genpd/imx/scu-pd.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c > > index 08583a10ac62..d69da79d3130 100644 > > --- a/drivers/genpd/imx/scu-pd.c > > +++ b/drivers/genpd/imx/scu-pd.c > > @@ -52,6 +52,7 @@ > > */ > > > > #include <dt-bindings/firmware/imx/rsrc.h> > > +#include <linux/console.h> > > #include <linux/firmware/imx/sci.h> > > #include <linux/firmware/imx/svc/rm.h> #include <linux/io.h> @@ > > -324,6 +325,10 @@ static int imx_sc_pd_power(struct > generic_pm_domain *domain, bool power_on) > > msg.resource = pd->rsrc; > > msg.mode = power_on ? IMX_SC_PM_PW_MODE_ON : > > IMX_SC_PM_PW_MODE_LP; > > > > + /* keep uart console power on for no_console_suspend */ > > + if (imx_con_rsrc == pd->rsrc && !console_suspend_enabled && > > + !power_on) > > As I indicated above, I don't mind this, but I also think this is a rather generic > problem that you are trying to solve here. > > In principle, I think it should be the serial driver's responsibility to check the > console_suspend_enabled flag. Based upon that, it should inform upper > layers (genpd) that its device may need to stay powered on during system > suspend. Quite similar to how we deal with system wakeups. To make this > work we could do the following instead of $subject patch. > > 1. The serial driver should call device_set_wakeup_path() (the name of that > function is a bit confusing in this regard, but let's discuss that separately) in > its ->suspend() callback. > 2. Set the GENPD_FLAG_ACTIVE_WAKEUP (again the name is a bit confusing > in this regard) for the corresponding genpd provider.
Thanks for your suggestion, I will try it. For the current v3 4/8, I will keep it in V4. For your suggested method, I will post in a separate patch after I finish and verified.
Thanks, Peng.
> > In this way, genpd will keep the PM domain powered on during system > suspend. > > > + return -EBUSY; > > + > > ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true); > > if (ret) > > dev_err(&domain->dev, "failed to power %s resource %d > > ret %d\n", > > -- > > 2.37.1 > > > > Kind regards > Uffe
| |