Patchwork [4/6] PM / Domains: Move the Subdomain check into _genpd_power_off

login
register
mail settings
Submitter Dong Aisheng
Date March 6, 2019, 1:25 p.m.
Message ID <1551878302-8146-5-git-send-email-aisheng.dong@nxp.com>
Download mbox | patch
Permalink /patch/742533/
State New
Headers show

Comments

Dong Aisheng - March 6, 2019, 1:25 p.m.
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(-)
Ulf Hansson - March 6, 2019, 2:26 p.m.
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
>
Dong Aisheng - March 6, 2019, 2:47 p.m.
> 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

> >

Patch

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 */