Messages in this thread | | | From | Ulf Hansson <> | Date | Wed, 7 Jul 2021 11:50:39 +0200 | Subject | Re: [PATCH] PM: domains: not update genpd status when power on fail |
| |
On Tue, 6 Jul 2021 at 11:52, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > > Hi Ulf, > > > Subject: Re: [PATCH] PM: domains: not update genpd status when power on > > fail > > > > On Tue, 6 Jul 2021 at 11:31, <peng.fan@oss.nxp.com> wrote: > > > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > When _genpd_power_on fail, the generic power domain status should not > > > be changed to GENPD_STATE_ON. > > > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > > --- > > > drivers/base/power/domain.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > > index ab0b740cc0f1..754a5d384479 100644 > > > --- a/drivers/base/power/domain.c > > > +++ b/drivers/base/power/domain.c > > > @@ -1101,6 +1101,7 @@ static void genpd_sync_power_on(struct > > generic_pm_domain *genpd, bool use_lock, > > > unsigned int depth) { > > > struct gpd_link *link; > > > + int ret; > > > > > > if (genpd_status_on(genpd)) > > > return; > > > @@ -1117,8 +1118,9 @@ static void genpd_sync_power_on(struct > > generic_pm_domain *genpd, bool use_lock, > > > genpd_unlock(link->parent); > > > } > > > > > > - _genpd_power_on(genpd, false); > > > - genpd->status = GENPD_STATE_ON; > > > + ret = _genpd_power_on(genpd, false); > > > + if (!ret) > > > > To support this error path, a lot more code needs to be added. Just > > have a look at the callers of genpd_sync_power_on() and compare it > > with genpd_power_on(). > > > > In my opinion, supporting this error path isn't really worth it, at > > least until someone comes with a valid use case. Do you have one - or > > did you send the $subject patch based solely on code inspection? > > This is two patchset: > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210604111005.6804-1-peng.fan@oss.nxp.com/ > https://patchwork.kernel.org/project/linux-arm-kernel/cover/20210629072941.7980-1-peng.fan@oss.nxp.com/ > > I collected in github with a few fixes: > https://github.com/MrVan/linux/tree/linux-next-master-628-blk-ctl-test
I looked at some of the changes there - and I especially don't quite understand why you enable runtime PM for the genpd->dev. That looks wrong to me. I am referring to: "soc: imx: blk-ctl: fix to v8 patchset".
> > > In my test, when suspend/resume, I see > pgc_vpu_h1/g1/g2 has status on, but actually they are not on in hardware.
That sounds like a bug somewhere. Did genpd fail to power off the PM domain or what happened, more exactly?
> Because vpumix_blk_ctl bus domain is not on, pgc_vpu_h1/g1/g2 runtime > power off will fail.
What is pgc_vpu_h1/g1/g2?
I am happy to help/guide you forward, but it sounds a bit that more in-depth debugging is needed at your side. At least I need some stack trace or kernel logs with debug messages enabled.
[...]
Kind regards Uffe
| |