| Message ID | 1551878302-8146-5-git-send-email-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | PM / Domains: A few clean up and minor fixes | expand |
On Wed, 6 Mar 2019 at 14:25, Aisheng Dong <aisheng.dong@nxp.com> wrote: > > Move the Subdomain check into _genpd_power_off, then the caller does > not have to check it each time. This also ensures a double check > of &genpd->sd_count before really power off domain in case it's > increased asynchronously by subdomains. This is the same behavior > as the original genpd_power_off() does. > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> Please squash this with patch 3, as to avoid an intermediate change in behavior. Otherwise both patch 3 and patch4 looks good to me. Kind regards Uffe > --- > drivers/base/power/domain.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 591f37f..61cd500 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -452,6 +452,9 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed) > if (!genpd->power_off) > return 0; > > + if (atomic_read(&genpd->sd_count) > 0) > + return -EBUSY; > + > if (!timed) > return genpd->power_off(genpd); > > @@ -547,9 +550,6 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, > if (!genpd->gov) > genpd->state_idx = 0; > > - if (atomic_read(&genpd->sd_count) > 0) > - return -EBUSY; > - > /* > * If sd_count > 0 at this point, one of the subdomains hasn't > * managed to call genpd_power_on() for the master yet after > @@ -962,8 +962,7 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock, > if (!genpd_status_on(genpd) || genpd_is_always_on(genpd)) > return; > > - if (genpd->suspended_count != genpd->device_count > - || atomic_read(&genpd->sd_count) > 0) > + if (genpd->suspended_count != genpd->device_count) > return; > > /* Choose the deepest state when suspending */ > -- > 2.7.4 >
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: Wednesday, March 6, 2019 10:27 PM > On Wed, 6 Mar 2019 at 14:25, Aisheng Dong <aisheng.dong@nxp.com> > wrote: > > > > Move the Subdomain check into _genpd_power_off, then the caller does > > not have to check it each time. This also ensures a double check of > > &genpd->sd_count before really power off domain in case it's increased > > asynchronously by subdomains. This is the same behavior as the > > original genpd_power_off() does. > > > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > Please squash this with patch 3, as to avoid an intermediate change in > behavior. > > Otherwise both patch 3 and patch4 looks good to me. > Thanks, I will take this as a reviewed-by tag. And will resend with patch3&4 squashed later. Regards Dong Aisheng > Kind regards > Uffe > > > --- > > drivers/base/power/domain.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index 591f37f..61cd500 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -452,6 +452,9 @@ static int _genpd_power_off(struct > generic_pm_domain *genpd, bool timed) > > if (!genpd->power_off) > > return 0; > > > > + if (atomic_read(&genpd->sd_count) > 0) > > + return -EBUSY; > > + > > if (!timed) > > return genpd->power_off(genpd); > > > > @@ -547,9 +550,6 @@ static int genpd_power_off(struct > generic_pm_domain *genpd, bool one_dev_on, > > if (!genpd->gov) > > genpd->state_idx = 0; > > > > - if (atomic_read(&genpd->sd_count) > 0) > > - return -EBUSY; > > - > > /* > > * If sd_count > 0 at this point, one of the subdomains hasn't > > * managed to call genpd_power_on() for the master yet after > > @@ -962,8 +962,7 @@ static void genpd_sync_power_off(struct > generic_pm_domain *genpd, bool use_lock, > > if (!genpd_status_on(genpd) || genpd_is_always_on(genpd)) > > return; > > > > - if (genpd->suspended_count != genpd->device_count > > - || atomic_read(&genpd->sd_count) > 0) > > + if (genpd->suspended_count != genpd->device_count) > > return; > > > > /* Choose the deepest state when suspending */ > > -- > > 2.7.4 > >
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 591f37f..61cd500 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -452,6 +452,9 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed) if (!genpd->power_off) return 0; + if (atomic_read(&genpd->sd_count) > 0) + return -EBUSY; + if (!timed) return genpd->power_off(genpd); @@ -547,9 +550,6 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, if (!genpd->gov) genpd->state_idx = 0; - if (atomic_read(&genpd->sd_count) > 0) - return -EBUSY; - /* * If sd_count > 0 at this point, one of the subdomains hasn't * managed to call genpd_power_on() for the master yet after @@ -962,8 +962,7 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock, if (!genpd_status_on(genpd) || genpd_is_always_on(genpd)) return; - if (genpd->suspended_count != genpd->device_count - || atomic_read(&genpd->sd_count) > 0) + if (genpd->suspended_count != genpd->device_count) return; /* Choose the deepest state when suspending */
Move the Subdomain check into _genpd_power_off, then the caller does not have to check it each time. This also ensures a double check of &genpd->sd_count before really power off domain in case it's increased asynchronously by subdomains. This is the same behavior as the original genpd_power_off() does. Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- drivers/base/power/domain.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)