diff mbox series

[v3,1/4] PM: domains: Drop genpd status manipulation for hibernate restore

Message ID 20221102142104.2006554-2-shawn.guo@linaro.org
State New
Headers
Series Manage domain power state for hibernate |

Commit Message

Shawn Guo Nov. 2, 2022, 2:21 p.m. UTC 
  The genpd status manipulation for hibernate restore has really never
worked as intended.  For example, if the genpd->status was GENPD_STATE_ON,
the parent domain's `sd_count` must have been increased, so it needs to
be adjusted too.  So drop this status manipulation.

Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/base/power/domain.c | 13 -------------
 1 file changed, 13 deletions(-)

Comments

Ulf Hansson Nov. 4, 2022, 3:32 p.m. UTC | #1 
On Wed, 2 Nov 2022 at 15:21, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> The genpd status manipulation for hibernate restore has really never
> worked as intended.  For example, if the genpd->status was GENPD_STATE_ON,
> the parent domain's `sd_count` must have been increased, so it needs to
> be adjusted too.  So drop this status manipulation.
>
> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Note that, I was trying to understand a little more about the
background to the below code, although it's not entirely easy to
browse the git history around this.

Unless Rafael thinks there are reasons to keep the code as is, I
wouldn't mind seeing it go away. So:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/base/power/domain.c | 13 -------------
>  1 file changed, 13 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 6471b559230e..97deae1d4e77 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1374,20 +1374,7 @@ static int genpd_restore_noirq(struct device *dev)
>         if (IS_ERR(genpd))
>                 return -EINVAL;
>
> -       /*
> -        * At this point suspended_count == 0 means we are being run for the
> -        * first time for the given domain in the present cycle.
> -        */
>         genpd_lock(genpd);
> -       if (genpd->suspended_count++ == 0) {
> -               /*
> -                * The boot kernel might put the domain into arbitrary state,
> -                * so make it appear as powered off to genpd_sync_power_on(),
> -                * so that it tries to power it on in case it was really off.
> -                */
> -               genpd->status = GENPD_STATE_OFF;
> -       }
> -
>         genpd_sync_power_on(genpd, true, 0);
>         genpd_unlock(genpd);
>
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 6471b559230e..97deae1d4e77 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1374,20 +1374,7 @@  static int genpd_restore_noirq(struct device *dev)
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	/*
-	 * At this point suspended_count == 0 means we are being run for the
-	 * first time for the given domain in the present cycle.
-	 */
 	genpd_lock(genpd);
-	if (genpd->suspended_count++ == 0) {
-		/*
-		 * The boot kernel might put the domain into arbitrary state,
-		 * so make it appear as powered off to genpd_sync_power_on(),
-		 * so that it tries to power it on in case it was really off.
-		 */
-		genpd->status = GENPD_STATE_OFF;
-	}
-
 	genpd_sync_power_on(genpd, true, 0);
 	genpd_unlock(genpd);