[1/2] PM / Domains: Simplify genpd_lookup_dev()

Message ID 20190829144805.634-1-ulf.hansson@linaro.org
State Accepted
Commit b3ad17c09899d491cf9815a6db44d3f9b3f244e7
Headers show
Series
  • [1/2] PM / Domains: Simplify genpd_lookup_dev()
Related show

Commit Message

Ulf Hansson Aug. 29, 2019, 2:48 p.m.
genpd_lookup_dev(), is a bit unnecessary heavy, as it walks the gpd_list to
try to find a valid PM domain corresponding to the device's attached genpd.

Instead of walking the gpd_list, let's use the fact that a genpd always has
the ->runtime_suspend() callback assigned to the genpd_runtime_suspend()
function.

While changing this, let's take the opportunity to also rename
genpd_lookup_dev(), into dev_to_genpd_safe() to better reflect its purpose.

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

---
 drivers/base/power/domain.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

-- 
2.17.1

Comments

Ulf Hansson Aug. 29, 2019, 3:01 p.m. | #1
On Thu, 29 Aug 2019 at 16:48, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>

> genpd_lookup_dev(), is a bit unnecessary heavy, as it walks the gpd_list to

> try to find a valid PM domain corresponding to the device's attached genpd.

>

> Instead of walking the gpd_list, let's use the fact that a genpd always has

> the ->runtime_suspend() callback assigned to the genpd_runtime_suspend()

> function.

>

> While changing this, let's take the opportunity to also rename

> genpd_lookup_dev(), into dev_to_genpd_safe() to better reflect its purpose.

>

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

> ---

>  drivers/base/power/domain.c | 23 +++++++++--------------

>  1 file changed, 9 insertions(+), 14 deletions(-)

>

> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c

> index b063bc41b0a9..27592b73061d 100644

> --- a/drivers/base/power/domain.c

> +++ b/drivers/base/power/domain.c

> @@ -149,29 +149,24 @@ static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,

>         return ret;

>  }

>

> +static int genpd_runtime_suspend(struct device *dev);

> +

>  /*

>   * Get the generic PM domain for a particular struct device.

>   * This validates the struct device pointer, the PM domain pointer,

> - * and checks that the PM domain pointer is a real generic PM domain.

> + * and checks that the PM domain pointer is real generic PM domain.


Rafael, I thought I got rid of this line change. I can send a new
version - or if you manually just drop this line of change when
applying?

Kind regards
Uffe

>   * Any failure results in NULL being returned.

>   */

> -static struct generic_pm_domain *genpd_lookup_dev(struct device *dev)

> +static struct generic_pm_domain *dev_to_genpd_safe(struct device *dev)

>  {

> -       struct generic_pm_domain *genpd = NULL, *gpd;

> -

>         if (IS_ERR_OR_NULL(dev) || IS_ERR_OR_NULL(dev->pm_domain))

>                 return NULL;

>

> -       mutex_lock(&gpd_list_lock);

> -       list_for_each_entry(gpd, &gpd_list, gpd_list_node) {

> -               if (&gpd->domain == dev->pm_domain) {

> -                       genpd = gpd;

> -                       break;

> -               }

> -       }

> -       mutex_unlock(&gpd_list_lock);

> +       /* A genpd's always have its ->runtime_suspend() callback assigned. */

> +       if (dev->pm_domain->ops.runtime_suspend == genpd_runtime_suspend)

> +               return pd_to_genpd(dev->pm_domain);

>

> -       return genpd;

> +       return NULL;

>  }

>

>  /*

> @@ -1610,7 +1605,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,

>   */

>  int pm_genpd_remove_device(struct device *dev)

>  {

> -       struct generic_pm_domain *genpd = genpd_lookup_dev(dev);

> +       struct generic_pm_domain *genpd = dev_to_genpd_safe(dev);

>

>         if (!genpd)

>                 return -EINVAL;

> --

> 2.17.1

>

Patch

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index b063bc41b0a9..27592b73061d 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -149,29 +149,24 @@  static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
 	return ret;
 }
 
+static int genpd_runtime_suspend(struct device *dev);
+
 /*
  * Get the generic PM domain for a particular struct device.
  * This validates the struct device pointer, the PM domain pointer,
- * and checks that the PM domain pointer is a real generic PM domain.
+ * and checks that the PM domain pointer is real generic PM domain.
  * Any failure results in NULL being returned.
  */
-static struct generic_pm_domain *genpd_lookup_dev(struct device *dev)
+static struct generic_pm_domain *dev_to_genpd_safe(struct device *dev)
 {
-	struct generic_pm_domain *genpd = NULL, *gpd;
-
 	if (IS_ERR_OR_NULL(dev) || IS_ERR_OR_NULL(dev->pm_domain))
 		return NULL;
 
-	mutex_lock(&gpd_list_lock);
-	list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
-		if (&gpd->domain == dev->pm_domain) {
-			genpd = gpd;
-			break;
-		}
-	}
-	mutex_unlock(&gpd_list_lock);
+	/* A genpd's always have its ->runtime_suspend() callback assigned. */
+	if (dev->pm_domain->ops.runtime_suspend == genpd_runtime_suspend)
+		return pd_to_genpd(dev->pm_domain);
 
-	return genpd;
+	return NULL;
 }
 
 /*
@@ -1610,7 +1605,7 @@  static int genpd_remove_device(struct generic_pm_domain *genpd,
  */
 int pm_genpd_remove_device(struct device *dev)
 {
-	struct generic_pm_domain *genpd = genpd_lookup_dev(dev);
+	struct generic_pm_domain *genpd = dev_to_genpd_safe(dev);
 
 	if (!genpd)
 		return -EINVAL;