[PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Ulf Hansson]

Recently genpd was extended to cope with devices belonging to CPUs. However,
attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
because of the virtual device that genpd allocates in this path.

In this series, this limitation is addressed, together with a few other related
fixes/cleanups.

Ulf Hansson (4):
  PM / Domains: Use the base device for
    driver_deferred_probe_check_state()
  PM / Domains: Drop unused in-parameter to some genpd functions
  PM / Domains: Search for the CPU device outside the genpd lock
  PM / Domains: Allow to attach a CPU via
    genpd_dev_pm_attach_by_id|name()

 drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
 include/linux/pm_domain.h   |  1 +
 2 files changed, 36 insertions(+), 38 deletions(-)

-- 
2.17.1

[PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Ulf Hansson]

Recently genpd was extended to cope with devices belonging to CPUs. However,
attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
because of the virtual device that genpd allocates in this path.

In this series, this limitation is addressed, together with a few other related
fixes/cleanups.

Ulf Hansson (4):
  PM / Domains: Use the base device for
    driver_deferred_probe_check_state()
  PM / Domains: Drop unused in-parameter to some genpd functions
  PM / Domains: Search for the CPU device outside the genpd lock
  PM / Domains: Allow to attach a CPU via
    genpd_dev_pm_attach_by_id|name()

 drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
 include/linux/pm_domain.h   |  1 +
 2 files changed, 36 insertions(+), 38 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/4] PM / Domains: Use the base device for driver_deferred_probe_check_state()

[Ulf Hansson]

When genpd fails to attach a device to one of its multiple PM domains, we
end up calling driver_deferred_probe_check_state() for the recently
allocated virtual device. This is wrong, as it's the base device that is
being probed. Fix this, by passing along the base device to
__genpd_dev_pm_attach() and use that instead.

Fixes: e01afc325025 ("PM / Domains: Stop deferring probe at the end of initcall")
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 8362dfe187f5..8aca1c9b4406 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2405,8 +2405,8 @@ static void genpd_dev_pm_sync(struct device *dev)
 	genpd_queue_power_off_work(pd);
 }
 
-static int __genpd_dev_pm_attach(struct device *dev, unsigned int index,
-				 bool power_on)
+static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
+				 unsigned int index, bool power_on)
 {
 	struct of_phandle_args pd_args;
 	struct generic_pm_domain *pd;
@@ -2424,7 +2424,7 @@ static int __genpd_dev_pm_attach(struct device *dev, unsigned int index,
 		mutex_unlock(&gpd_list_lock);
 		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
 			__func__, PTR_ERR(pd));
-		return driver_deferred_probe_check_state(dev);
+		return driver_deferred_probe_check_state(base_dev);
 	}
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
@@ -2480,7 +2480,7 @@ int genpd_dev_pm_attach(struct device *dev)
 				       "#power-domain-cells") != 1)
 		return 0;
 
-	return __genpd_dev_pm_attach(dev, 0, true);
+	return __genpd_dev_pm_attach(dev, dev, 0, true);
 }
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
@@ -2533,7 +2533,7 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev,
 	}
 
 	/* Try to attach the device to the PM domain at the specified index. */
-	ret = __genpd_dev_pm_attach(virt_dev, index, false);
+	ret = __genpd_dev_pm_attach(virt_dev, dev, index, false);
 	if (ret < 1) {
 		device_unregister(virt_dev);
 		return ret ? ERR_PTR(ret) : NULL;
-- 
2.17.1

[PATCH 1/4] PM / Domains: Use the base device for driver_deferred_probe_check_state()

[Ulf Hansson]

When genpd fails to attach a device to one of its multiple PM domains, we
end up calling driver_deferred_probe_check_state() for the recently
allocated virtual device. This is wrong, as it's the base device that is
being probed. Fix this, by passing along the base device to
__genpd_dev_pm_attach() and use that instead.

Fixes: e01afc325025 ("PM / Domains: Stop deferring probe at the end of initcall")
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 8362dfe187f5..8aca1c9b4406 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2405,8 +2405,8 @@ static void genpd_dev_pm_sync(struct device *dev)
 	genpd_queue_power_off_work(pd);
 }
 
-static int __genpd_dev_pm_attach(struct device *dev, unsigned int index,
-				 bool power_on)
+static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
+				 unsigned int index, bool power_on)
 {
 	struct of_phandle_args pd_args;
 	struct generic_pm_domain *pd;
@@ -2424,7 +2424,7 @@ static int __genpd_dev_pm_attach(struct device *dev, unsigned int index,
 		mutex_unlock(&gpd_list_lock);
 		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
 			__func__, PTR_ERR(pd));
-		return driver_deferred_probe_check_state(dev);
+		return driver_deferred_probe_check_state(base_dev);
 	}
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
@@ -2480,7 +2480,7 @@ int genpd_dev_pm_attach(struct device *dev)
 				       "#power-domain-cells") != 1)
 		return 0;
 
-	return __genpd_dev_pm_attach(dev, 0, true);
+	return __genpd_dev_pm_attach(dev, dev, 0, true);
 }
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
@@ -2533,7 +2533,7 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev,
 	}
 
 	/* Try to attach the device to the PM domain at the specified index. */
-	ret = __genpd_dev_pm_attach(virt_dev, index, false);
+	ret = __genpd_dev_pm_attach(virt_dev, dev, index, false);
 	if (ret < 1) {
 		device_unregister(virt_dev);
 		return ret ? ERR_PTR(ret) : NULL;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] PM / Domains: Drop unused in-parameter to some genpd functions

[Ulf Hansson]

Both genpd_alloc_dev_data() and genpd_add_device(), whom are internal genpd
functions, allows a struct gpd_timing_data *td to be passed as an
in-parameter. However, as NULL is always passed, let's just drop the
in-parameter altogether.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 8aca1c9b4406..93298b7db408 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1396,8 +1396,7 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
 
 #endif /* CONFIG_PM_SLEEP */
 
-static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
-					struct gpd_timing_data *td)
+static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev)
 {
 	struct generic_pm_domain_data *gpd_data;
 	int ret;
@@ -1412,9 +1411,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
 		goto err_put;
 	}
 
-	if (td)
-		gpd_data->td = *td;
-
 	gpd_data->base.dev = dev;
 	gpd_data->td.constraint_changed = true;
 	gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;
@@ -1504,8 +1500,7 @@ static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
 	genpd_update_cpumask(genpd, dev, false);
 }
 
-static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
-			    struct gpd_timing_data *td)
+static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 {
 	struct generic_pm_domain_data *gpd_data;
 	int ret;
@@ -1515,7 +1510,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	gpd_data = genpd_alloc_dev_data(dev, td);
+	gpd_data = genpd_alloc_dev_data(dev);
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
@@ -1553,7 +1548,7 @@ int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 	int ret;
 
 	mutex_lock(&gpd_list_lock);
-	ret = genpd_add_device(genpd, dev, NULL);
+	ret = genpd_add_device(genpd, dev);
 	mutex_unlock(&gpd_list_lock);
 
 	return ret;
@@ -2259,7 +2254,7 @@ int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
 		goto out;
 	}
 
-	ret = genpd_add_device(genpd, dev, NULL);
+	ret = genpd_add_device(genpd, dev);
 
 out:
 	mutex_unlock(&gpd_list_lock);
@@ -2429,7 +2424,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
 
-	ret = genpd_add_device(pd, dev, NULL);
+	ret = genpd_add_device(pd, dev);
 	mutex_unlock(&gpd_list_lock);
 
 	if (ret < 0) {
-- 
2.17.1

[PATCH 2/4] PM / Domains: Drop unused in-parameter to some genpd functions

[Ulf Hansson]

Both genpd_alloc_dev_data() and genpd_add_device(), whom are internal genpd
functions, allows a struct gpd_timing_data *td to be passed as an
in-parameter. However, as NULL is always passed, let's just drop the
in-parameter altogether.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 8aca1c9b4406..93298b7db408 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1396,8 +1396,7 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
 
 #endif /* CONFIG_PM_SLEEP */
 
-static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
-					struct gpd_timing_data *td)
+static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev)
 {
 	struct generic_pm_domain_data *gpd_data;
 	int ret;
@@ -1412,9 +1411,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
 		goto err_put;
 	}
 
-	if (td)
-		gpd_data->td = *td;
-
 	gpd_data->base.dev = dev;
 	gpd_data->td.constraint_changed = true;
 	gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;
@@ -1504,8 +1500,7 @@ static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
 	genpd_update_cpumask(genpd, dev, false);
 }
 
-static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
-			    struct gpd_timing_data *td)
+static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 {
 	struct generic_pm_domain_data *gpd_data;
 	int ret;
@@ -1515,7 +1510,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	gpd_data = genpd_alloc_dev_data(dev, td);
+	gpd_data = genpd_alloc_dev_data(dev);
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
@@ -1553,7 +1548,7 @@ int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 	int ret;
 
 	mutex_lock(&gpd_list_lock);
-	ret = genpd_add_device(genpd, dev, NULL);
+	ret = genpd_add_device(genpd, dev);
 	mutex_unlock(&gpd_list_lock);
 
 	return ret;
@@ -2259,7 +2254,7 @@ int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
 		goto out;
 	}
 
-	ret = genpd_add_device(genpd, dev, NULL);
+	ret = genpd_add_device(genpd, dev);
 
 out:
 	mutex_unlock(&gpd_list_lock);
@@ -2429,7 +2424,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
 
-	ret = genpd_add_device(pd, dev, NULL);
+	ret = genpd_add_device(pd, dev);
 	mutex_unlock(&gpd_list_lock);
 
 	if (ret < 0) {
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] PM / Domains: Search for the CPU device outside the genpd lock

[Ulf Hansson]

While attaching/detaching a device to a PM domain (genpd) that has the
GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check
whether the device corresponds to a CPU. This iteration is done while
holding the genpd's lock, which is unnecessary. Let's avoid the locking,
by restructuring the corresponding code a bit.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 52 +++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 93298b7db408..da1c99178943 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1450,8 +1450,8 @@ static void genpd_free_dev_data(struct device *dev,
 	dev_pm_put_subsys_data(dev);
 }
 
-static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
-				   int cpu, bool set, unsigned int depth)
+static void genpd_update_cpumask(struct generic_pm_domain *genpd,
+				 int cpu, bool set, unsigned int depth)
 {
 	struct gpd_link *link;
 
@@ -1462,7 +1462,7 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
 		struct generic_pm_domain *master = link->master;
 
 		genpd_lock_nested(master, depth + 1);
-		__genpd_update_cpumask(master, cpu, set, depth + 1);
+		genpd_update_cpumask(master, cpu, set, depth + 1);
 		genpd_unlock(master);
 	}
 
@@ -1472,38 +1472,37 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
 		cpumask_clear_cpu(cpu, genpd->cpus);
 }
 
-static void genpd_update_cpumask(struct generic_pm_domain *genpd,
-				 struct device *dev, bool set)
+static void genpd_set_cpumask(struct generic_pm_domain *genpd, int cpu)
+{
+	if (cpu >= 0)
+		genpd_update_cpumask(genpd, cpu, true, 0);
+}
+
+static void genpd_clear_cpumask(struct generic_pm_domain *genpd, int cpu)
+{
+	if (cpu >= 0)
+		genpd_update_cpumask(genpd, cpu, false, 0);
+}
+
+static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
 {
 	int cpu;
 
 	if (!genpd_is_cpu_domain(genpd))
-		return;
+		return -1;
 
 	for_each_possible_cpu(cpu) {
-		if (get_cpu_device(cpu) == dev) {
-			__genpd_update_cpumask(genpd, cpu, set, 0);
-			return;
-		}
+		if (get_cpu_device(cpu) == dev)
+			return cpu;
 	}
-}
 
-static void genpd_set_cpumask(struct generic_pm_domain *genpd,
-			      struct device *dev)
-{
-	genpd_update_cpumask(genpd, dev, true);
-}
-
-static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
-				struct device *dev)
-{
-	genpd_update_cpumask(genpd, dev, false);
+	return -1;
 }
 
 static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 {
 	struct generic_pm_domain_data *gpd_data;
-	int ret;
+	int ret, cpu;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -1514,13 +1513,15 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
+	cpu = genpd_get_cpu(genpd, dev);
+
 	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
 	if (ret)
 		goto out;
 
 	genpd_lock(genpd);
 
-	genpd_set_cpumask(genpd, dev);
+	genpd_set_cpumask(genpd, cpu);
 	dev_pm_domain_set(dev, &genpd->domain);
 
 	genpd->device_count++;
@@ -1560,13 +1561,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 {
 	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
-	int ret = 0;
+	int cpu, ret = 0;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
 	pdd = dev->power.subsys_data->domain_data;
 	gpd_data = to_gpd_data(pdd);
 	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
+	cpu = genpd_get_cpu(genpd, dev);
 
 	genpd_lock(genpd);
 
@@ -1578,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 	genpd->device_count--;
 	genpd->max_off_time_changed = true;
 
-	genpd_clear_cpumask(genpd, dev);
+	genpd_clear_cpumask(genpd, cpu);
 	dev_pm_domain_set(dev, NULL);
 
 	list_del_init(&pdd->list_node);
-- 
2.17.1

[PATCH 3/4] PM / Domains: Search for the CPU device outside the genpd lock

[Ulf Hansson]

While attaching/detaching a device to a PM domain (genpd) that has the
GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check
whether the device corresponds to a CPU. This iteration is done while
holding the genpd's lock, which is unnecessary. Let's avoid the locking,
by restructuring the corresponding code a bit.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 52 +++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 93298b7db408..da1c99178943 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1450,8 +1450,8 @@ static void genpd_free_dev_data(struct device *dev,
 	dev_pm_put_subsys_data(dev);
 }
 
-static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
-				   int cpu, bool set, unsigned int depth)
+static void genpd_update_cpumask(struct generic_pm_domain *genpd,
+				 int cpu, bool set, unsigned int depth)
 {
 	struct gpd_link *link;
 
@@ -1462,7 +1462,7 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
 		struct generic_pm_domain *master = link->master;
 
 		genpd_lock_nested(master, depth + 1);
-		__genpd_update_cpumask(master, cpu, set, depth + 1);
+		genpd_update_cpumask(master, cpu, set, depth + 1);
 		genpd_unlock(master);
 	}
 
@@ -1472,38 +1472,37 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
 		cpumask_clear_cpu(cpu, genpd->cpus);
 }
 
-static void genpd_update_cpumask(struct generic_pm_domain *genpd,
-				 struct device *dev, bool set)
+static void genpd_set_cpumask(struct generic_pm_domain *genpd, int cpu)
+{
+	if (cpu >= 0)
+		genpd_update_cpumask(genpd, cpu, true, 0);
+}
+
+static void genpd_clear_cpumask(struct generic_pm_domain *genpd, int cpu)
+{
+	if (cpu >= 0)
+		genpd_update_cpumask(genpd, cpu, false, 0);
+}
+
+static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
 {
 	int cpu;
 
 	if (!genpd_is_cpu_domain(genpd))
-		return;
+		return -1;
 
 	for_each_possible_cpu(cpu) {
-		if (get_cpu_device(cpu) == dev) {
-			__genpd_update_cpumask(genpd, cpu, set, 0);
-			return;
-		}
+		if (get_cpu_device(cpu) == dev)
+			return cpu;
 	}
-}
 
-static void genpd_set_cpumask(struct generic_pm_domain *genpd,
-			      struct device *dev)
-{
-	genpd_update_cpumask(genpd, dev, true);
-}
-
-static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
-				struct device *dev)
-{
-	genpd_update_cpumask(genpd, dev, false);
+	return -1;
 }
 
 static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 {
 	struct generic_pm_domain_data *gpd_data;
-	int ret;
+	int ret, cpu;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -1514,13 +1513,15 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
+	cpu = genpd_get_cpu(genpd, dev);
+
 	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
 	if (ret)
 		goto out;
 
 	genpd_lock(genpd);
 
-	genpd_set_cpumask(genpd, dev);
+	genpd_set_cpumask(genpd, cpu);
 	dev_pm_domain_set(dev, &genpd->domain);
 
 	genpd->device_count++;
@@ -1560,13 +1561,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 {
 	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
-	int ret = 0;
+	int cpu, ret = 0;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
 	pdd = dev->power.subsys_data->domain_data;
 	gpd_data = to_gpd_data(pdd);
 	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
+	cpu = genpd_get_cpu(genpd, dev);
 
 	genpd_lock(genpd);
 
@@ -1578,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 	genpd->device_count--;
 	genpd->max_off_time_changed = true;
 
-	genpd_clear_cpumask(genpd, dev);
+	genpd_clear_cpumask(genpd, cpu);
 	dev_pm_domain_set(dev, NULL);
 
 	list_del_init(&pdd->list_node);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] PM / Domains: Allow to attach a CPU via genpd_dev_pm_attach_by_id|name()

[Ulf Hansson]

Attaching a device via genpd_dev_pm_attach_by_id|name() makes genpd to
allocate a virtual device that it attaches instead. This leads to a problem
in case the base device belongs to a CPU. More precisely, it means
genpd_get_cpu() compares against the virtual device, thus it fails to find
a matching CPU device.

Address this limitation, by passing the base device to genpd_get_cpu()
rather than the virtual device. Moreover, to deal with detach correctly
from genpd_remove_device(), let's store the CPU number in the struct
generic_pm_domain_data, as to be able to clear the corresponding bit in the
cpumask for the genpd.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 20 ++++++++++----------
 include/linux/pm_domain.h   |  1 +
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index da1c99178943..3d899e8abd58 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1499,10 +1499,11 @@ static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
 	return -1;
 }
 
-static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
+static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			    struct device *base_dev)
 {
 	struct generic_pm_domain_data *gpd_data;
-	int ret, cpu;
+	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -1513,7 +1514,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
-	cpu = genpd_get_cpu(genpd, dev);
+	gpd_data->cpu = genpd_get_cpu(genpd, base_dev);
 
 	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
 	if (ret)
@@ -1521,7 +1522,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 
 	genpd_lock(genpd);
 
-	genpd_set_cpumask(genpd, cpu);
+	genpd_set_cpumask(genpd, gpd_data->cpu);
 	dev_pm_domain_set(dev, &genpd->domain);
 
 	genpd->device_count++;
@@ -1549,7 +1550,7 @@ int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 	int ret;
 
 	mutex_lock(&gpd_list_lock);
-	ret = genpd_add_device(genpd, dev);
+	ret = genpd_add_device(genpd, dev, dev);
 	mutex_unlock(&gpd_list_lock);
 
 	return ret;
@@ -1561,14 +1562,13 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 {
 	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
-	int cpu, ret = 0;
+	int ret = 0;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
 	pdd = dev->power.subsys_data->domain_data;
 	gpd_data = to_gpd_data(pdd);
 	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
-	cpu = genpd_get_cpu(genpd, dev);
 
 	genpd_lock(genpd);
 
@@ -1580,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 	genpd->device_count--;
 	genpd->max_off_time_changed = true;
 
-	genpd_clear_cpumask(genpd, cpu);
+	genpd_clear_cpumask(genpd, gpd_data->cpu);
 	dev_pm_domain_set(dev, NULL);
 
 	list_del_init(&pdd->list_node);
@@ -2256,7 +2256,7 @@ int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
 		goto out;
 	}
 
-	ret = genpd_add_device(genpd, dev);
+	ret = genpd_add_device(genpd, dev, dev);
 
 out:
 	mutex_unlock(&gpd_list_lock);
@@ -2426,7 +2426,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
 
-	ret = genpd_add_device(pd, dev);
+	ret = genpd_add_device(pd, dev, base_dev);
 	mutex_unlock(&gpd_list_lock);
 
 	if (ret < 0) {
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index bc82e74560ee..0e8e356bed6a 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -175,6 +175,7 @@ struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_timing_data td;
 	struct notifier_block nb;
+	int cpu;
 	unsigned int performance_state;
 	void *data;
 };
-- 
2.17.1

[PATCH 4/4] PM / Domains: Allow to attach a CPU via genpd_dev_pm_attach_by_id|name()

[Ulf Hansson]

Attaching a device via genpd_dev_pm_attach_by_id|name() makes genpd to
allocate a virtual device that it attaches instead. This leads to a problem
in case the base device belongs to a CPU. More precisely, it means
genpd_get_cpu() compares against the virtual device, thus it fails to find
a matching CPU device.

Address this limitation, by passing the base device to genpd_get_cpu()
rather than the virtual device. Moreover, to deal with detach correctly
from genpd_remove_device(), let's store the CPU number in the struct
generic_pm_domain_data, as to be able to clear the corresponding bit in the
cpumask for the genpd.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 20 ++++++++++----------
 include/linux/pm_domain.h   |  1 +
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index da1c99178943..3d899e8abd58 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1499,10 +1499,11 @@ static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
 	return -1;
 }
 
-static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
+static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			    struct device *base_dev)
 {
 	struct generic_pm_domain_data *gpd_data;
-	int ret, cpu;
+	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -1513,7 +1514,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
-	cpu = genpd_get_cpu(genpd, dev);
+	gpd_data->cpu = genpd_get_cpu(genpd, base_dev);
 
 	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
 	if (ret)
@@ -1521,7 +1522,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 
 	genpd_lock(genpd);
 
-	genpd_set_cpumask(genpd, cpu);
+	genpd_set_cpumask(genpd, gpd_data->cpu);
 	dev_pm_domain_set(dev, &genpd->domain);
 
 	genpd->device_count++;
@@ -1549,7 +1550,7 @@ int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 	int ret;
 
 	mutex_lock(&gpd_list_lock);
-	ret = genpd_add_device(genpd, dev);
+	ret = genpd_add_device(genpd, dev, dev);
 	mutex_unlock(&gpd_list_lock);
 
 	return ret;
@@ -1561,14 +1562,13 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 {
 	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
-	int cpu, ret = 0;
+	int ret = 0;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
 	pdd = dev->power.subsys_data->domain_data;
 	gpd_data = to_gpd_data(pdd);
 	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
-	cpu = genpd_get_cpu(genpd, dev);
 
 	genpd_lock(genpd);
 
@@ -1580,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 	genpd->device_count--;
 	genpd->max_off_time_changed = true;
 
-	genpd_clear_cpumask(genpd, cpu);
+	genpd_clear_cpumask(genpd, gpd_data->cpu);
 	dev_pm_domain_set(dev, NULL);
 
 	list_del_init(&pdd->list_node);
@@ -2256,7 +2256,7 @@ int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
 		goto out;
 	}
 
-	ret = genpd_add_device(genpd, dev);
+	ret = genpd_add_device(genpd, dev, dev);
 
 out:
 	mutex_unlock(&gpd_list_lock);
@@ -2426,7 +2426,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
 
 	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
 
-	ret = genpd_add_device(pd, dev);
+	ret = genpd_add_device(pd, dev, base_dev);
 	mutex_unlock(&gpd_list_lock);
 
 	if (ret < 0) {
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index bc82e74560ee..0e8e356bed6a 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -175,6 +175,7 @@ struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_timing_data td;
 	struct notifier_block nb;
+	int cpu;
 	unsigned int performance_state;
 	void *data;
 };
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] PM / Domains: Use the base device for driver_deferred_probe_check_state()

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> When genpd fails to attach a device to one of its multiple PM domains, we
> end up calling driver_deferred_probe_check_state() for the recently
> allocated virtual device. This is wrong, as it's the base device that is
> being probed. Fix this, by passing along the base device to
> __genpd_dev_pm_attach() and use that instead.
> 
> Fixes: e01afc325025 ("PM / Domains: Stop deferring probe at the end of initcall")
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 8362dfe187f5..8aca1c9b4406 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2405,8 +2405,8 @@ static void genpd_dev_pm_sync(struct device *dev)
>  	genpd_queue_power_off_work(pd);
>  }
>  
> -static int __genpd_dev_pm_attach(struct device *dev, unsigned int index,
> -				 bool power_on)
> +static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
> +				 unsigned int index, bool power_on)
>  {
>  	struct of_phandle_args pd_args;
>  	struct generic_pm_domain *pd;
> @@ -2424,7 +2424,7 @@ static int __genpd_dev_pm_attach(struct device *dev, unsigned int index,
>  		mutex_unlock(&gpd_list_lock);
>  		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
>  			__func__, PTR_ERR(pd));
> -		return driver_deferred_probe_check_state(dev);
> +		return driver_deferred_probe_check_state(base_dev);
>  	}
>  
>  	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
> @@ -2480,7 +2480,7 @@ int genpd_dev_pm_attach(struct device *dev)
>  				       "#power-domain-cells") != 1)
>  		return 0;
>  
> -	return __genpd_dev_pm_attach(dev, 0, true);
> +	return __genpd_dev_pm_attach(dev, dev, 0, true);
>  }
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>  
> @@ -2533,7 +2533,7 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev,
>  	}
>  
>  	/* Try to attach the device to the PM domain at the specified index. */
> -	ret = __genpd_dev_pm_attach(virt_dev, index, false);
> +	ret = __genpd_dev_pm_attach(virt_dev, dev, index, false);
>  	if (ret < 1) {
>  		device_unregister(virt_dev);
>  		return ret ? ERR_PTR(ret) : NULL;

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 1/4] PM / Domains: Use the base device for driver_deferred_probe_check_state()

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> When genpd fails to attach a device to one of its multiple PM domains, we
> end up calling driver_deferred_probe_check_state() for the recently
> allocated virtual device. This is wrong, as it's the base device that is
> being probed. Fix this, by passing along the base device to
> __genpd_dev_pm_attach() and use that instead.
> 
> Fixes: e01afc325025 ("PM / Domains: Stop deferring probe at the end of initcall")
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 8362dfe187f5..8aca1c9b4406 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2405,8 +2405,8 @@ static void genpd_dev_pm_sync(struct device *dev)
>  	genpd_queue_power_off_work(pd);
>  }
>  
> -static int __genpd_dev_pm_attach(struct device *dev, unsigned int index,
> -				 bool power_on)
> +static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
> +				 unsigned int index, bool power_on)
>  {
>  	struct of_phandle_args pd_args;
>  	struct generic_pm_domain *pd;
> @@ -2424,7 +2424,7 @@ static int __genpd_dev_pm_attach(struct device *dev, unsigned int index,
>  		mutex_unlock(&gpd_list_lock);
>  		dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
>  			__func__, PTR_ERR(pd));
> -		return driver_deferred_probe_check_state(dev);
> +		return driver_deferred_probe_check_state(base_dev);
>  	}
>  
>  	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
> @@ -2480,7 +2480,7 @@ int genpd_dev_pm_attach(struct device *dev)
>  				       "#power-domain-cells") != 1)
>  		return 0;
>  
> -	return __genpd_dev_pm_attach(dev, 0, true);
> +	return __genpd_dev_pm_attach(dev, dev, 0, true);
>  }
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>  
> @@ -2533,7 +2533,7 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev,
>  	}
>  
>  	/* Try to attach the device to the PM domain at the specified index. */
> -	ret = __genpd_dev_pm_attach(virt_dev, index, false);
> +	ret = __genpd_dev_pm_attach(virt_dev, dev, index, false);
>  	if (ret < 1) {
>  		device_unregister(virt_dev);
>  		return ret ? ERR_PTR(ret) : NULL;

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] PM / Domains: Drop unused in-parameter to some genpd functions

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> Both genpd_alloc_dev_data() and genpd_add_device(), whom are internal genpd
> functions, allows a struct gpd_timing_data *td to be passed as an
> in-parameter. However, as NULL is always passed, let's just drop the
> in-parameter altogether.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 8aca1c9b4406..93298b7db408 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1396,8 +1396,7 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
>  
>  #endif /* CONFIG_PM_SLEEP */
>  
> -static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
> -					struct gpd_timing_data *td)
> +static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	int ret;
> @@ -1412,9 +1411,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
>  		goto err_put;
>  	}
>  
> -	if (td)
> -		gpd_data->td = *td;
> -
>  	gpd_data->base.dev = dev;
>  	gpd_data->td.constraint_changed = true;
>  	gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;
> @@ -1504,8 +1500,7 @@ static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
>  	genpd_update_cpumask(genpd, dev, false);
>  }
>  
> -static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
> -			    struct gpd_timing_data *td)
> +static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	int ret;
> @@ -1515,7 +1510,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
>  	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
>  		return -EINVAL;
>  
> -	gpd_data = genpd_alloc_dev_data(dev, td);
> +	gpd_data = genpd_alloc_dev_data(dev);
>  	if (IS_ERR(gpd_data))
>  		return PTR_ERR(gpd_data);
>  
> @@ -1553,7 +1548,7 @@ int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	int ret;
>  
>  	mutex_lock(&gpd_list_lock);
> -	ret = genpd_add_device(genpd, dev, NULL);
> +	ret = genpd_add_device(genpd, dev);
>  	mutex_unlock(&gpd_list_lock);
>  
>  	return ret;
> @@ -2259,7 +2254,7 @@ int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
>  		goto out;
>  	}
>  
> -	ret = genpd_add_device(genpd, dev, NULL);
> +	ret = genpd_add_device(genpd, dev);
>  
>  out:
>  	mutex_unlock(&gpd_list_lock);
> @@ -2429,7 +2424,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>  
>  	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
>  
> -	ret = genpd_add_device(pd, dev, NULL);
> +	ret = genpd_add_device(pd, dev);
>  	mutex_unlock(&gpd_list_lock);
>  
>  	if (ret < 0) {

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 2/4] PM / Domains: Drop unused in-parameter to some genpd functions

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> Both genpd_alloc_dev_data() and genpd_add_device(), whom are internal genpd
> functions, allows a struct gpd_timing_data *td to be passed as an
> in-parameter. However, as NULL is always passed, let's just drop the
> in-parameter altogether.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 8aca1c9b4406..93298b7db408 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1396,8 +1396,7 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
>  
>  #endif /* CONFIG_PM_SLEEP */
>  
> -static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
> -					struct gpd_timing_data *td)
> +static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	int ret;
> @@ -1412,9 +1411,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
>  		goto err_put;
>  	}
>  
> -	if (td)
> -		gpd_data->td = *td;
> -
>  	gpd_data->base.dev = dev;
>  	gpd_data->td.constraint_changed = true;
>  	gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS;
> @@ -1504,8 +1500,7 @@ static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
>  	genpd_update_cpumask(genpd, dev, false);
>  }
>  
> -static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
> -			    struct gpd_timing_data *td)
> +static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	int ret;
> @@ -1515,7 +1510,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
>  	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
>  		return -EINVAL;
>  
> -	gpd_data = genpd_alloc_dev_data(dev, td);
> +	gpd_data = genpd_alloc_dev_data(dev);
>  	if (IS_ERR(gpd_data))
>  		return PTR_ERR(gpd_data);
>  
> @@ -1553,7 +1548,7 @@ int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	int ret;
>  
>  	mutex_lock(&gpd_list_lock);
> -	ret = genpd_add_device(genpd, dev, NULL);
> +	ret = genpd_add_device(genpd, dev);
>  	mutex_unlock(&gpd_list_lock);
>  
>  	return ret;
> @@ -2259,7 +2254,7 @@ int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
>  		goto out;
>  	}
>  
> -	ret = genpd_add_device(genpd, dev, NULL);
> +	ret = genpd_add_device(genpd, dev);
>  
>  out:
>  	mutex_unlock(&gpd_list_lock);
> @@ -2429,7 +2424,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>  
>  	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
>  
> -	ret = genpd_add_device(pd, dev, NULL);
> +	ret = genpd_add_device(pd, dev);
>  	mutex_unlock(&gpd_list_lock);
>  
>  	if (ret < 0) {

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] PM / Domains: Search for the CPU device outside the genpd lock

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> While attaching/detaching a device to a PM domain (genpd) that has the
> GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check
> whether the device corresponds to a CPU. This iteration is done while
> holding the genpd's lock, which is unnecessary. Let's avoid the locking,
> by restructuring the corresponding code a bit.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 52 +++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 93298b7db408..da1c99178943 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1450,8 +1450,8 @@ static void genpd_free_dev_data(struct device *dev,
>  	dev_pm_put_subsys_data(dev);
>  }
>  
> -static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> -				   int cpu, bool set, unsigned int depth)
> +static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> +				 int cpu, bool set, unsigned int depth)
>  {
>  	struct gpd_link *link;
>  
> @@ -1462,7 +1462,7 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
>  		struct generic_pm_domain *master = link->master;
>  
>  		genpd_lock_nested(master, depth + 1);
> -		__genpd_update_cpumask(master, cpu, set, depth + 1);
> +		genpd_update_cpumask(master, cpu, set, depth + 1);
>  		genpd_unlock(master);
>  	}
>  
> @@ -1472,38 +1472,37 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
>  		cpumask_clear_cpu(cpu, genpd->cpus);
>  }
>  
> -static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> -				 struct device *dev, bool set)
> +static void genpd_set_cpumask(struct generic_pm_domain *genpd, int cpu)
> +{
> +	if (cpu >= 0)
> +		genpd_update_cpumask(genpd, cpu, true, 0);
> +}
> +
> +static void genpd_clear_cpumask(struct generic_pm_domain *genpd, int cpu)
> +{
> +	if (cpu >= 0)
> +		genpd_update_cpumask(genpd, cpu, false, 0);
> +}
> +
> +static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	int cpu;
>  
>  	if (!genpd_is_cpu_domain(genpd))
> -		return;
> +		return -1;
>  
>  	for_each_possible_cpu(cpu) {
> -		if (get_cpu_device(cpu) == dev) {
> -			__genpd_update_cpumask(genpd, cpu, set, 0);
> -			return;
> -		}
> +		if (get_cpu_device(cpu) == dev)
> +			return cpu;
>  	}
> -}
>  
> -static void genpd_set_cpumask(struct generic_pm_domain *genpd,
> -			      struct device *dev)
> -{
> -	genpd_update_cpumask(genpd, dev, true);
> -}
> -
> -static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
> -				struct device *dev)
> -{
> -	genpd_update_cpumask(genpd, dev, false);
> +	return -1;
>  }
>  
>  static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
> -	int ret;
> +	int ret, cpu;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
> @@ -1514,13 +1513,15 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	if (IS_ERR(gpd_data))
>  		return PTR_ERR(gpd_data);
>  
> +	cpu = genpd_get_cpu(genpd, dev);
> +
>  	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
>  	if (ret)
>  		goto out;
>  
>  	genpd_lock(genpd);
>  
> -	genpd_set_cpumask(genpd, dev);
> +	genpd_set_cpumask(genpd, cpu);

Should we check if "cpu" is valid here ? As that was done earlier.

>  	dev_pm_domain_set(dev, &genpd->domain);
>  
>  	genpd->device_count++;
> @@ -1560,13 +1561,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	struct pm_domain_data *pdd;
> -	int ret = 0;
> +	int cpu, ret = 0;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
>  	pdd = dev->power.subsys_data->domain_data;
>  	gpd_data = to_gpd_data(pdd);
>  	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> +	cpu = genpd_get_cpu(genpd, dev);
>  
>  	genpd_lock(genpd);
>  
> @@ -1578,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  	genpd->device_count--;
>  	genpd->max_off_time_changed = true;
>  
> -	genpd_clear_cpumask(genpd, dev);
> +	genpd_clear_cpumask(genpd, cpu);

Same here.

>  	dev_pm_domain_set(dev, NULL);
>  
>  	list_del_init(&pdd->list_node);
> -- 
> 2.17.1

-- 
viresh

Re: [PATCH 3/4] PM / Domains: Search for the CPU device outside the genpd lock

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> While attaching/detaching a device to a PM domain (genpd) that has the
> GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check
> whether the device corresponds to a CPU. This iteration is done while
> holding the genpd's lock, which is unnecessary. Let's avoid the locking,
> by restructuring the corresponding code a bit.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 52 +++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 93298b7db408..da1c99178943 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1450,8 +1450,8 @@ static void genpd_free_dev_data(struct device *dev,
>  	dev_pm_put_subsys_data(dev);
>  }
>  
> -static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> -				   int cpu, bool set, unsigned int depth)
> +static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> +				 int cpu, bool set, unsigned int depth)
>  {
>  	struct gpd_link *link;
>  
> @@ -1462,7 +1462,7 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
>  		struct generic_pm_domain *master = link->master;
>  
>  		genpd_lock_nested(master, depth + 1);
> -		__genpd_update_cpumask(master, cpu, set, depth + 1);
> +		genpd_update_cpumask(master, cpu, set, depth + 1);
>  		genpd_unlock(master);
>  	}
>  
> @@ -1472,38 +1472,37 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
>  		cpumask_clear_cpu(cpu, genpd->cpus);
>  }
>  
> -static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> -				 struct device *dev, bool set)
> +static void genpd_set_cpumask(struct generic_pm_domain *genpd, int cpu)
> +{
> +	if (cpu >= 0)
> +		genpd_update_cpumask(genpd, cpu, true, 0);
> +}
> +
> +static void genpd_clear_cpumask(struct generic_pm_domain *genpd, int cpu)
> +{
> +	if (cpu >= 0)
> +		genpd_update_cpumask(genpd, cpu, false, 0);
> +}
> +
> +static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	int cpu;
>  
>  	if (!genpd_is_cpu_domain(genpd))
> -		return;
> +		return -1;
>  
>  	for_each_possible_cpu(cpu) {
> -		if (get_cpu_device(cpu) == dev) {
> -			__genpd_update_cpumask(genpd, cpu, set, 0);
> -			return;
> -		}
> +		if (get_cpu_device(cpu) == dev)
> +			return cpu;
>  	}
> -}
>  
> -static void genpd_set_cpumask(struct generic_pm_domain *genpd,
> -			      struct device *dev)
> -{
> -	genpd_update_cpumask(genpd, dev, true);
> -}
> -
> -static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
> -				struct device *dev)
> -{
> -	genpd_update_cpumask(genpd, dev, false);
> +	return -1;
>  }
>  
>  static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
> -	int ret;
> +	int ret, cpu;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
> @@ -1514,13 +1513,15 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	if (IS_ERR(gpd_data))
>  		return PTR_ERR(gpd_data);
>  
> +	cpu = genpd_get_cpu(genpd, dev);
> +
>  	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
>  	if (ret)
>  		goto out;
>  
>  	genpd_lock(genpd);
>  
> -	genpd_set_cpumask(genpd, dev);
> +	genpd_set_cpumask(genpd, cpu);

Should we check if "cpu" is valid here ? As that was done earlier.

>  	dev_pm_domain_set(dev, &genpd->domain);
>  
>  	genpd->device_count++;
> @@ -1560,13 +1561,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	struct pm_domain_data *pdd;
> -	int ret = 0;
> +	int cpu, ret = 0;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
>  	pdd = dev->power.subsys_data->domain_data;
>  	gpd_data = to_gpd_data(pdd);
>  	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> +	cpu = genpd_get_cpu(genpd, dev);
>  
>  	genpd_lock(genpd);
>  
> @@ -1578,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  	genpd->device_count--;
>  	genpd->max_off_time_changed = true;
>  
> -	genpd_clear_cpumask(genpd, dev);
> +	genpd_clear_cpumask(genpd, cpu);

Same here.

>  	dev_pm_domain_set(dev, NULL);
>  
>  	list_del_init(&pdd->list_node);
> -- 
> 2.17.1

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] PM / Domains: Allow to attach a CPU via genpd_dev_pm_attach_by_id|name()

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> Attaching a device via genpd_dev_pm_attach_by_id|name() makes genpd to
> allocate a virtual device that it attaches instead. This leads to a problem
> in case the base device belongs to a CPU. More precisely, it means
> genpd_get_cpu() compares against the virtual device, thus it fails to find
> a matching CPU device.
> 
> Address this limitation, by passing the base device to genpd_get_cpu()
> rather than the virtual device. Moreover, to deal with detach correctly
> from genpd_remove_device(), let's store the CPU number in the struct
> generic_pm_domain_data, as to be able to clear the corresponding bit in the
> cpumask for the genpd.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 20 ++++++++++----------
>  include/linux/pm_domain.h   |  1 +
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index da1c99178943..3d899e8abd58 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1499,10 +1499,11 @@ static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
>  	return -1;
>  }
>  
> -static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
> +static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
> +			    struct device *base_dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
> -	int ret, cpu;
> +	int ret;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
> @@ -1513,7 +1514,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	if (IS_ERR(gpd_data))
>  		return PTR_ERR(gpd_data);
>  
> -	cpu = genpd_get_cpu(genpd, dev);
> +	gpd_data->cpu = genpd_get_cpu(genpd, base_dev);
>  
>  	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
>  	if (ret)
> @@ -1521,7 +1522,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  
>  	genpd_lock(genpd);
>  
> -	genpd_set_cpumask(genpd, cpu);
> +	genpd_set_cpumask(genpd, gpd_data->cpu);
>  	dev_pm_domain_set(dev, &genpd->domain);
>  
>  	genpd->device_count++;
> @@ -1549,7 +1550,7 @@ int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	int ret;
>  
>  	mutex_lock(&gpd_list_lock);
> -	ret = genpd_add_device(genpd, dev);
> +	ret = genpd_add_device(genpd, dev, dev);
>  	mutex_unlock(&gpd_list_lock);
>  
>  	return ret;
> @@ -1561,14 +1562,13 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	struct pm_domain_data *pdd;
> -	int cpu, ret = 0;
> +	int ret = 0;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
>  	pdd = dev->power.subsys_data->domain_data;
>  	gpd_data = to_gpd_data(pdd);
>  	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> -	cpu = genpd_get_cpu(genpd, dev);
>  
>  	genpd_lock(genpd);
>  
> @@ -1580,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  	genpd->device_count--;
>  	genpd->max_off_time_changed = true;
>  
> -	genpd_clear_cpumask(genpd, cpu);
> +	genpd_clear_cpumask(genpd, gpd_data->cpu);
>  	dev_pm_domain_set(dev, NULL);
>  
>  	list_del_init(&pdd->list_node);
> @@ -2256,7 +2256,7 @@ int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
>  		goto out;
>  	}
>  
> -	ret = genpd_add_device(genpd, dev);
> +	ret = genpd_add_device(genpd, dev, dev);
>  
>  out:
>  	mutex_unlock(&gpd_list_lock);
> @@ -2426,7 +2426,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>  
>  	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
>  
> -	ret = genpd_add_device(pd, dev);
> +	ret = genpd_add_device(pd, dev, base_dev);
>  	mutex_unlock(&gpd_list_lock);
>  
>  	if (ret < 0) {
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index bc82e74560ee..0e8e356bed6a 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -175,6 +175,7 @@ struct generic_pm_domain_data {
>  	struct pm_domain_data base;
>  	struct gpd_timing_data td;
>  	struct notifier_block nb;
> +	int cpu;
>  	unsigned int performance_state;
>  	void *data;
>  };

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 4/4] PM / Domains: Allow to attach a CPU via genpd_dev_pm_attach_by_id|name()

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> Attaching a device via genpd_dev_pm_attach_by_id|name() makes genpd to
> allocate a virtual device that it attaches instead. This leads to a problem
> in case the base device belongs to a CPU. More precisely, it means
> genpd_get_cpu() compares against the virtual device, thus it fails to find
> a matching CPU device.
> 
> Address this limitation, by passing the base device to genpd_get_cpu()
> rather than the virtual device. Moreover, to deal with detach correctly
> from genpd_remove_device(), let's store the CPU number in the struct
> generic_pm_domain_data, as to be able to clear the corresponding bit in the
> cpumask for the genpd.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 20 ++++++++++----------
>  include/linux/pm_domain.h   |  1 +
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index da1c99178943..3d899e8abd58 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1499,10 +1499,11 @@ static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
>  	return -1;
>  }
>  
> -static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
> +static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
> +			    struct device *base_dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
> -	int ret, cpu;
> +	int ret;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
> @@ -1513,7 +1514,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	if (IS_ERR(gpd_data))
>  		return PTR_ERR(gpd_data);
>  
> -	cpu = genpd_get_cpu(genpd, dev);
> +	gpd_data->cpu = genpd_get_cpu(genpd, base_dev);
>  
>  	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
>  	if (ret)
> @@ -1521,7 +1522,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  
>  	genpd_lock(genpd);
>  
> -	genpd_set_cpumask(genpd, cpu);
> +	genpd_set_cpumask(genpd, gpd_data->cpu);
>  	dev_pm_domain_set(dev, &genpd->domain);
>  
>  	genpd->device_count++;
> @@ -1549,7 +1550,7 @@ int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	int ret;
>  
>  	mutex_lock(&gpd_list_lock);
> -	ret = genpd_add_device(genpd, dev);
> +	ret = genpd_add_device(genpd, dev, dev);
>  	mutex_unlock(&gpd_list_lock);
>  
>  	return ret;
> @@ -1561,14 +1562,13 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	struct pm_domain_data *pdd;
> -	int cpu, ret = 0;
> +	int ret = 0;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
>  	pdd = dev->power.subsys_data->domain_data;
>  	gpd_data = to_gpd_data(pdd);
>  	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> -	cpu = genpd_get_cpu(genpd, dev);
>  
>  	genpd_lock(genpd);
>  
> @@ -1580,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  	genpd->device_count--;
>  	genpd->max_off_time_changed = true;
>  
> -	genpd_clear_cpumask(genpd, cpu);
> +	genpd_clear_cpumask(genpd, gpd_data->cpu);
>  	dev_pm_domain_set(dev, NULL);
>  
>  	list_del_init(&pdd->list_node);
> @@ -2256,7 +2256,7 @@ int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
>  		goto out;
>  	}
>  
> -	ret = genpd_add_device(genpd, dev);
> +	ret = genpd_add_device(genpd, dev, dev);
>  
>  out:
>  	mutex_unlock(&gpd_list_lock);
> @@ -2426,7 +2426,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>  
>  	dev_dbg(dev, "adding to PM domain %s\n", pd->name);
>  
> -	ret = genpd_add_device(pd, dev);
> +	ret = genpd_add_device(pd, dev, base_dev);
>  	mutex_unlock(&gpd_list_lock);
>  
>  	if (ret < 0) {
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index bc82e74560ee..0e8e356bed6a 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -175,6 +175,7 @@ struct generic_pm_domain_data {
>  	struct pm_domain_data base;
>  	struct gpd_timing_data td;
>  	struct notifier_block nb;
> +	int cpu;
>  	unsigned int performance_state;
>  	void *data;
>  };

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Rafael J. Wysocki]

On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Recently genpd was extended to cope with devices belonging to CPUs. However,
> attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> because of the virtual device that genpd allocates in this path.
>
> In this series, this limitation is addressed, together with a few other related
> fixes/cleanups.
>
> Ulf Hansson (4):
>   PM / Domains: Use the base device for
>     driver_deferred_probe_check_state()
>   PM / Domains: Drop unused in-parameter to some genpd functions
>   PM / Domains: Search for the CPU device outside the genpd lock
>   PM / Domains: Allow to attach a CPU via
>     genpd_dev_pm_attach_by_id|name()
>
>  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
>  include/linux/pm_domain.h   |  1 +
>  2 files changed, 36 insertions(+), 38 deletions(-)

Are there any dependencies between this and the series you've recently posted?

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Rafael J. Wysocki]

On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Recently genpd was extended to cope with devices belonging to CPUs. However,
> attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> because of the virtual device that genpd allocates in this path.
>
> In this series, this limitation is addressed, together with a few other related
> fixes/cleanups.
>
> Ulf Hansson (4):
>   PM / Domains: Use the base device for
>     driver_deferred_probe_check_state()
>   PM / Domains: Drop unused in-parameter to some genpd functions
>   PM / Domains: Search for the CPU device outside the genpd lock
>   PM / Domains: Allow to attach a CPU via
>     genpd_dev_pm_attach_by_id|name()
>
>  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
>  include/linux/pm_domain.h   |  1 +
>  2 files changed, 36 insertions(+), 38 deletions(-)

Are there any dependencies between this and the series you've recently posted?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Ulf Hansson]

On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > because of the virtual device that genpd allocates in this path.
> >
> > In this series, this limitation is addressed, together with a few other related
> > fixes/cleanups.
> >
> > Ulf Hansson (4):
> >   PM / Domains: Use the base device for
> >     driver_deferred_probe_check_state()
> >   PM / Domains: Drop unused in-parameter to some genpd functions
> >   PM / Domains: Search for the CPU device outside the genpd lock
> >   PM / Domains: Allow to attach a CPU via
> >     genpd_dev_pm_attach_by_id|name()
> >
> >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> >  include/linux/pm_domain.h   |  1 +
> >  2 files changed, 36 insertions(+), 38 deletions(-)
>
> Are there any dependencies between this and the series you've recently posted?

Yep. I should have stated that, sorry. This should be applied on top.

Kind regards
Uffe

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Ulf Hansson]

On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > because of the virtual device that genpd allocates in this path.
> >
> > In this series, this limitation is addressed, together with a few other related
> > fixes/cleanups.
> >
> > Ulf Hansson (4):
> >   PM / Domains: Use the base device for
> >     driver_deferred_probe_check_state()
> >   PM / Domains: Drop unused in-parameter to some genpd functions
> >   PM / Domains: Search for the CPU device outside the genpd lock
> >   PM / Domains: Allow to attach a CPU via
> >     genpd_dev_pm_attach_by_id|name()
> >
> >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> >  include/linux/pm_domain.h   |  1 +
> >  2 files changed, 36 insertions(+), 38 deletions(-)
>
> Are there any dependencies between this and the series you've recently posted?

Yep. I should have stated that, sorry. This should be applied on top.

Kind regards
Uffe

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Ulf Hansson]

On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > because of the virtual device that genpd allocates in this path.
> >
> > In this series, this limitation is addressed, together with a few other related
> > fixes/cleanups.
> >
> > Ulf Hansson (4):
> >   PM / Domains: Use the base device for
> >     driver_deferred_probe_check_state()
> >   PM / Domains: Drop unused in-parameter to some genpd functions
> >   PM / Domains: Search for the CPU device outside the genpd lock
> >   PM / Domains: Allow to attach a CPU via
> >     genpd_dev_pm_attach_by_id|name()
> >
> >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> >  include/linux/pm_domain.h   |  1 +
> >  2 files changed, 36 insertions(+), 38 deletions(-)
>
> Are there any dependencies between this and the series you've recently posted?

Yep. I should have stated that, sorry. This should be applied on top.

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Rafael J. Wysocki]

On Thu, Apr 25, 2019 at 12:11 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > > because of the virtual device that genpd allocates in this path.
> > >
> > > In this series, this limitation is addressed, together with a few other related
> > > fixes/cleanups.
> > >
> > > Ulf Hansson (4):
> > >   PM / Domains: Use the base device for
> > >     driver_deferred_probe_check_state()
> > >   PM / Domains: Drop unused in-parameter to some genpd functions
> > >   PM / Domains: Search for the CPU device outside the genpd lock
> > >   PM / Domains: Allow to attach a CPU via
> > >     genpd_dev_pm_attach_by_id|name()
> > >
> > >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> > >  include/linux/pm_domain.h   |  1 +
> > >  2 files changed, 36 insertions(+), 38 deletions(-)
> >
> > Are there any dependencies between this and the series you've recently posted?
>
> Yep. I should have stated that, sorry. This should be applied on top.

On top of which series?

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Rafael J. Wysocki]

On Thu, Apr 25, 2019 at 12:11 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > > because of the virtual device that genpd allocates in this path.
> > >
> > > In this series, this limitation is addressed, together with a few other related
> > > fixes/cleanups.
> > >
> > > Ulf Hansson (4):
> > >   PM / Domains: Use the base device for
> > >     driver_deferred_probe_check_state()
> > >   PM / Domains: Drop unused in-parameter to some genpd functions
> > >   PM / Domains: Search for the CPU device outside the genpd lock
> > >   PM / Domains: Allow to attach a CPU via
> > >     genpd_dev_pm_attach_by_id|name()
> > >
> > >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> > >  include/linux/pm_domain.h   |  1 +
> > >  2 files changed, 36 insertions(+), 38 deletions(-)
> >
> > Are there any dependencies between this and the series you've recently posted?
>
> Yep. I should have stated that, sorry. This should be applied on top.

On top of which series?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] PM / Domains: Search for the CPU device outside the genpd lock

[Ulf Hansson]

On Thu, 25 Apr 2019 at 11:45, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 25-04-19, 11:04, Ulf Hansson wrote:
> > While attaching/detaching a device to a PM domain (genpd) that has the
> > GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check
> > whether the device corresponds to a CPU. This iteration is done while
> > holding the genpd's lock, which is unnecessary. Let's avoid the locking,
> > by restructuring the corresponding code a bit.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >  drivers/base/power/domain.c | 52 +++++++++++++++++++------------------
> >  1 file changed, 27 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > index 93298b7db408..da1c99178943 100644
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -1450,8 +1450,8 @@ static void genpd_free_dev_data(struct device *dev,
> >       dev_pm_put_subsys_data(dev);
> >  }
> >
> > -static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> > -                                int cpu, bool set, unsigned int depth)
> > +static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> > +                              int cpu, bool set, unsigned int depth)
> >  {
> >       struct gpd_link *link;
> >
> > @@ -1462,7 +1462,7 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> >               struct generic_pm_domain *master = link->master;
> >
> >               genpd_lock_nested(master, depth + 1);
> > -             __genpd_update_cpumask(master, cpu, set, depth + 1);
> > +             genpd_update_cpumask(master, cpu, set, depth + 1);
> >               genpd_unlock(master);
> >       }
> >
> > @@ -1472,38 +1472,37 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> >               cpumask_clear_cpu(cpu, genpd->cpus);
> >  }
> >
> > -static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> > -                              struct device *dev, bool set)
> > +static void genpd_set_cpumask(struct generic_pm_domain *genpd, int cpu)
> > +{
> > +     if (cpu >= 0)
> > +             genpd_update_cpumask(genpd, cpu, true, 0);
> > +}
> > +
> > +static void genpd_clear_cpumask(struct generic_pm_domain *genpd, int cpu)
> > +{
> > +     if (cpu >= 0)
> > +             genpd_update_cpumask(genpd, cpu, false, 0);
> > +}
> > +
> > +static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
> >  {
> >       int cpu;
> >
> >       if (!genpd_is_cpu_domain(genpd))
> > -             return;
> > +             return -1;
> >
> >       for_each_possible_cpu(cpu) {
> > -             if (get_cpu_device(cpu) == dev) {
> > -                     __genpd_update_cpumask(genpd, cpu, set, 0);
> > -                     return;
> > -             }
> > +             if (get_cpu_device(cpu) == dev)
> > +                     return cpu;
> >       }
> > -}
> >
> > -static void genpd_set_cpumask(struct generic_pm_domain *genpd,
> > -                           struct device *dev)
> > -{
> > -     genpd_update_cpumask(genpd, dev, true);
> > -}
> > -
> > -static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
> > -                             struct device *dev)
> > -{
> > -     genpd_update_cpumask(genpd, dev, false);
> > +     return -1;
> >  }
> >
> >  static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
> >  {
> >       struct generic_pm_domain_data *gpd_data;
> > -     int ret;
> > +     int ret, cpu;
> >
> >       dev_dbg(dev, "%s()\n", __func__);
> >
> > @@ -1514,13 +1513,15 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
> >       if (IS_ERR(gpd_data))
> >               return PTR_ERR(gpd_data);
> >
> > +     cpu = genpd_get_cpu(genpd, dev);
> > +
> >       ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
> >       if (ret)
> >               goto out;
> >
> >       genpd_lock(genpd);
> >
> > -     genpd_set_cpumask(genpd, dev);
> > +     genpd_set_cpumask(genpd, cpu);
>
> Should we check if "cpu" is valid here ? As that was done earlier.

This is being done in the new version of genpd_set|clear_cpumask(). Like below.

"if (cpu >= 0)"...

>
> >       dev_pm_domain_set(dev, &genpd->domain);
> >
> >       genpd->device_count++;
> > @@ -1560,13 +1561,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
> >  {
> >       struct generic_pm_domain_data *gpd_data;
> >       struct pm_domain_data *pdd;
> > -     int ret = 0;
> > +     int cpu, ret = 0;
> >
> >       dev_dbg(dev, "%s()\n", __func__);
> >
> >       pdd = dev->power.subsys_data->domain_data;
> >       gpd_data = to_gpd_data(pdd);
> >       dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> > +     cpu = genpd_get_cpu(genpd, dev);
> >
> >       genpd_lock(genpd);
> >
> > @@ -1578,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
> >       genpd->device_count--;
> >       genpd->max_off_time_changed = true;
> >
> > -     genpd_clear_cpumask(genpd, dev);
> > +     genpd_clear_cpumask(genpd, cpu);
>
> Same here.

See above.

>
> >       dev_pm_domain_set(dev, NULL);
> >
> >       list_del_init(&pdd->list_node);
> > --
> > 2.17.1
>
> --
> viresh

Thanks for reviewing!

Kind regards
Uffe

Re: [PATCH 3/4] PM / Domains: Search for the CPU device outside the genpd lock

[Ulf Hansson]

On Thu, 25 Apr 2019 at 11:45, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 25-04-19, 11:04, Ulf Hansson wrote:
> > While attaching/detaching a device to a PM domain (genpd) that has the
> > GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check
> > whether the device corresponds to a CPU. This iteration is done while
> > holding the genpd's lock, which is unnecessary. Let's avoid the locking,
> > by restructuring the corresponding code a bit.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >  drivers/base/power/domain.c | 52 +++++++++++++++++++------------------
> >  1 file changed, 27 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > index 93298b7db408..da1c99178943 100644
> > --- a/drivers/base/power/domain.c
> > +++ b/drivers/base/power/domain.c
> > @@ -1450,8 +1450,8 @@ static void genpd_free_dev_data(struct device *dev,
> >       dev_pm_put_subsys_data(dev);
> >  }
> >
> > -static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> > -                                int cpu, bool set, unsigned int depth)
> > +static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> > +                              int cpu, bool set, unsigned int depth)
> >  {
> >       struct gpd_link *link;
> >
> > @@ -1462,7 +1462,7 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> >               struct generic_pm_domain *master = link->master;
> >
> >               genpd_lock_nested(master, depth + 1);
> > -             __genpd_update_cpumask(master, cpu, set, depth + 1);
> > +             genpd_update_cpumask(master, cpu, set, depth + 1);
> >               genpd_unlock(master);
> >       }
> >
> > @@ -1472,38 +1472,37 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> >               cpumask_clear_cpu(cpu, genpd->cpus);
> >  }
> >
> > -static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> > -                              struct device *dev, bool set)
> > +static void genpd_set_cpumask(struct generic_pm_domain *genpd, int cpu)
> > +{
> > +     if (cpu >= 0)
> > +             genpd_update_cpumask(genpd, cpu, true, 0);
> > +}
> > +
> > +static void genpd_clear_cpumask(struct generic_pm_domain *genpd, int cpu)
> > +{
> > +     if (cpu >= 0)
> > +             genpd_update_cpumask(genpd, cpu, false, 0);
> > +}
> > +
> > +static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
> >  {
> >       int cpu;
> >
> >       if (!genpd_is_cpu_domain(genpd))
> > -             return;
> > +             return -1;
> >
> >       for_each_possible_cpu(cpu) {
> > -             if (get_cpu_device(cpu) == dev) {
> > -                     __genpd_update_cpumask(genpd, cpu, set, 0);
> > -                     return;
> > -             }
> > +             if (get_cpu_device(cpu) == dev)
> > +                     return cpu;
> >       }
> > -}
> >
> > -static void genpd_set_cpumask(struct generic_pm_domain *genpd,
> > -                           struct device *dev)
> > -{
> > -     genpd_update_cpumask(genpd, dev, true);
> > -}
> > -
> > -static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
> > -                             struct device *dev)
> > -{
> > -     genpd_update_cpumask(genpd, dev, false);
> > +     return -1;
> >  }
> >
> >  static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
> >  {
> >       struct generic_pm_domain_data *gpd_data;
> > -     int ret;
> > +     int ret, cpu;
> >
> >       dev_dbg(dev, "%s()\n", __func__);
> >
> > @@ -1514,13 +1513,15 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
> >       if (IS_ERR(gpd_data))
> >               return PTR_ERR(gpd_data);
> >
> > +     cpu = genpd_get_cpu(genpd, dev);
> > +
> >       ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
> >       if (ret)
> >               goto out;
> >
> >       genpd_lock(genpd);
> >
> > -     genpd_set_cpumask(genpd, dev);
> > +     genpd_set_cpumask(genpd, cpu);
>
> Should we check if "cpu" is valid here ? As that was done earlier.

This is being done in the new version of genpd_set|clear_cpumask(). Like below.

"if (cpu >= 0)"...

>
> >       dev_pm_domain_set(dev, &genpd->domain);
> >
> >       genpd->device_count++;
> > @@ -1560,13 +1561,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
> >  {
> >       struct generic_pm_domain_data *gpd_data;
> >       struct pm_domain_data *pdd;
> > -     int ret = 0;
> > +     int cpu, ret = 0;
> >
> >       dev_dbg(dev, "%s()\n", __func__);
> >
> >       pdd = dev->power.subsys_data->domain_data;
> >       gpd_data = to_gpd_data(pdd);
> >       dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> > +     cpu = genpd_get_cpu(genpd, dev);
> >
> >       genpd_lock(genpd);
> >
> > @@ -1578,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
> >       genpd->device_count--;
> >       genpd->max_off_time_changed = true;
> >
> > -     genpd_clear_cpumask(genpd, dev);
> > +     genpd_clear_cpumask(genpd, cpu);
>
> Same here.

See above.

>
> >       dev_pm_domain_set(dev, NULL);
> >
> >       list_del_init(&pdd->list_node);
> > --
> > 2.17.1
>
> --
> viresh

Thanks for reviewing!

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] PM / Domains: Search for the CPU device outside the genpd lock

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> While attaching/detaching a device to a PM domain (genpd) that has the
> GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check
> whether the device corresponds to a CPU. This iteration is done while
> holding the genpd's lock, which is unnecessary. Let's avoid the locking,
> by restructuring the corresponding code a bit.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 52 +++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 93298b7db408..da1c99178943 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1450,8 +1450,8 @@ static void genpd_free_dev_data(struct device *dev,
>  	dev_pm_put_subsys_data(dev);
>  }
>  
> -static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> -				   int cpu, bool set, unsigned int depth)
> +static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> +				 int cpu, bool set, unsigned int depth)
>  {
>  	struct gpd_link *link;
>  
> @@ -1462,7 +1462,7 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
>  		struct generic_pm_domain *master = link->master;
>  
>  		genpd_lock_nested(master, depth + 1);
> -		__genpd_update_cpumask(master, cpu, set, depth + 1);
> +		genpd_update_cpumask(master, cpu, set, depth + 1);
>  		genpd_unlock(master);
>  	}
>  
> @@ -1472,38 +1472,37 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
>  		cpumask_clear_cpu(cpu, genpd->cpus);
>  }
>  
> -static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> -				 struct device *dev, bool set)
> +static void genpd_set_cpumask(struct generic_pm_domain *genpd, int cpu)
> +{
> +	if (cpu >= 0)
> +		genpd_update_cpumask(genpd, cpu, true, 0);
> +}
> +
> +static void genpd_clear_cpumask(struct generic_pm_domain *genpd, int cpu)
> +{
> +	if (cpu >= 0)
> +		genpd_update_cpumask(genpd, cpu, false, 0);
> +}
> +
> +static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	int cpu;
>  
>  	if (!genpd_is_cpu_domain(genpd))
> -		return;
> +		return -1;
>  
>  	for_each_possible_cpu(cpu) {
> -		if (get_cpu_device(cpu) == dev) {
> -			__genpd_update_cpumask(genpd, cpu, set, 0);
> -			return;
> -		}
> +		if (get_cpu_device(cpu) == dev)
> +			return cpu;
>  	}
> -}
>  
> -static void genpd_set_cpumask(struct generic_pm_domain *genpd,
> -			      struct device *dev)
> -{
> -	genpd_update_cpumask(genpd, dev, true);
> -}
> -
> -static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
> -				struct device *dev)
> -{
> -	genpd_update_cpumask(genpd, dev, false);
> +	return -1;
>  }
>  
>  static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
> -	int ret;
> +	int ret, cpu;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
> @@ -1514,13 +1513,15 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	if (IS_ERR(gpd_data))
>  		return PTR_ERR(gpd_data);
>  
> +	cpu = genpd_get_cpu(genpd, dev);
> +
>  	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
>  	if (ret)
>  		goto out;
>  
>  	genpd_lock(genpd);
>  
> -	genpd_set_cpumask(genpd, dev);
> +	genpd_set_cpumask(genpd, cpu);
>  	dev_pm_domain_set(dev, &genpd->domain);
>  
>  	genpd->device_count++;
> @@ -1560,13 +1561,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	struct pm_domain_data *pdd;
> -	int ret = 0;
> +	int cpu, ret = 0;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
>  	pdd = dev->power.subsys_data->domain_data;
>  	gpd_data = to_gpd_data(pdd);
>  	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> +	cpu = genpd_get_cpu(genpd, dev);
>  
>  	genpd_lock(genpd);
>  
> @@ -1578,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  	genpd->device_count--;
>  	genpd->max_off_time_changed = true;
>  
> -	genpd_clear_cpumask(genpd, dev);
> +	genpd_clear_cpumask(genpd, cpu);
>  	dev_pm_domain_set(dev, NULL);
>  
>  	list_del_init(&pdd->list_node);

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Ulf Hansson]

On Thu, 25 Apr 2019 at 12:14, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Apr 25, 2019 at 12:11 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > >
> > > > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > > > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > > > because of the virtual device that genpd allocates in this path.
> > > >
> > > > In this series, this limitation is addressed, together with a few other related
> > > > fixes/cleanups.
> > > >
> > > > Ulf Hansson (4):
> > > >   PM / Domains: Use the base device for
> > > >     driver_deferred_probe_check_state()
> > > >   PM / Domains: Drop unused in-parameter to some genpd functions
> > > >   PM / Domains: Search for the CPU device outside the genpd lock
> > > >   PM / Domains: Allow to attach a CPU via
> > > >     genpd_dev_pm_attach_by_id|name()
> > > >
> > > >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> > > >  include/linux/pm_domain.h   |  1 +
> > > >  2 files changed, 36 insertions(+), 38 deletions(-)
> > >
> > > Are there any dependencies between this and the series you've recently posted?
> >
> > Yep. I should have stated that, sorry. This should be applied on top.
>
> On top of which series?

[PATCH 0/3] PM / Domains: Improve support for multi PM domains

and

[PATCH v13 0/4] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM)

Kind regards
Uffe

Re: [PATCH 3/4] PM / Domains: Search for the CPU device outside the genpd lock

[Viresh Kumar]

On 25-04-19, 11:04, Ulf Hansson wrote:
> While attaching/detaching a device to a PM domain (genpd) that has the
> GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check
> whether the device corresponds to a CPU. This iteration is done while
> holding the genpd's lock, which is unnecessary. Let's avoid the locking,
> by restructuring the corresponding code a bit.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 52 +++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 93298b7db408..da1c99178943 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1450,8 +1450,8 @@ static void genpd_free_dev_data(struct device *dev,
>  	dev_pm_put_subsys_data(dev);
>  }
>  
> -static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
> -				   int cpu, bool set, unsigned int depth)
> +static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> +				 int cpu, bool set, unsigned int depth)
>  {
>  	struct gpd_link *link;
>  
> @@ -1462,7 +1462,7 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
>  		struct generic_pm_domain *master = link->master;
>  
>  		genpd_lock_nested(master, depth + 1);
> -		__genpd_update_cpumask(master, cpu, set, depth + 1);
> +		genpd_update_cpumask(master, cpu, set, depth + 1);
>  		genpd_unlock(master);
>  	}
>  
> @@ -1472,38 +1472,37 @@ static void __genpd_update_cpumask(struct generic_pm_domain *genpd,
>  		cpumask_clear_cpu(cpu, genpd->cpus);
>  }
>  
> -static void genpd_update_cpumask(struct generic_pm_domain *genpd,
> -				 struct device *dev, bool set)
> +static void genpd_set_cpumask(struct generic_pm_domain *genpd, int cpu)
> +{
> +	if (cpu >= 0)
> +		genpd_update_cpumask(genpd, cpu, true, 0);
> +}
> +
> +static void genpd_clear_cpumask(struct generic_pm_domain *genpd, int cpu)
> +{
> +	if (cpu >= 0)
> +		genpd_update_cpumask(genpd, cpu, false, 0);
> +}
> +
> +static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	int cpu;
>  
>  	if (!genpd_is_cpu_domain(genpd))
> -		return;
> +		return -1;
>  
>  	for_each_possible_cpu(cpu) {
> -		if (get_cpu_device(cpu) == dev) {
> -			__genpd_update_cpumask(genpd, cpu, set, 0);
> -			return;
> -		}
> +		if (get_cpu_device(cpu) == dev)
> +			return cpu;
>  	}
> -}
>  
> -static void genpd_set_cpumask(struct generic_pm_domain *genpd,
> -			      struct device *dev)
> -{
> -	genpd_update_cpumask(genpd, dev, true);
> -}
> -
> -static void genpd_clear_cpumask(struct generic_pm_domain *genpd,
> -				struct device *dev)
> -{
> -	genpd_update_cpumask(genpd, dev, false);
> +	return -1;
>  }
>  
>  static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  {
>  	struct generic_pm_domain_data *gpd_data;
> -	int ret;
> +	int ret, cpu;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
> @@ -1514,13 +1513,15 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
>  	if (IS_ERR(gpd_data))
>  		return PTR_ERR(gpd_data);
>  
> +	cpu = genpd_get_cpu(genpd, dev);
> +
>  	ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
>  	if (ret)
>  		goto out;
>  
>  	genpd_lock(genpd);
>  
> -	genpd_set_cpumask(genpd, dev);
> +	genpd_set_cpumask(genpd, cpu);
>  	dev_pm_domain_set(dev, &genpd->domain);
>  
>  	genpd->device_count++;
> @@ -1560,13 +1561,14 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  {
>  	struct generic_pm_domain_data *gpd_data;
>  	struct pm_domain_data *pdd;
> -	int ret = 0;
> +	int cpu, ret = 0;
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
>  	pdd = dev->power.subsys_data->domain_data;
>  	gpd_data = to_gpd_data(pdd);
>  	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
> +	cpu = genpd_get_cpu(genpd, dev);
>  
>  	genpd_lock(genpd);
>  
> @@ -1578,7 +1580,7 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
>  	genpd->device_count--;
>  	genpd->max_off_time_changed = true;
>  
> -	genpd_clear_cpumask(genpd, dev);
> +	genpd_clear_cpumask(genpd, cpu);
>  	dev_pm_domain_set(dev, NULL);
>  
>  	list_del_init(&pdd->list_node);

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Ulf Hansson]

On Thu, 25 Apr 2019 at 12:14, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Apr 25, 2019 at 12:11 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > >
> > > > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > > > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > > > because of the virtual device that genpd allocates in this path.
> > > >
> > > > In this series, this limitation is addressed, together with a few other related
> > > > fixes/cleanups.
> > > >
> > > > Ulf Hansson (4):
> > > >   PM / Domains: Use the base device for
> > > >     driver_deferred_probe_check_state()
> > > >   PM / Domains: Drop unused in-parameter to some genpd functions
> > > >   PM / Domains: Search for the CPU device outside the genpd lock
> > > >   PM / Domains: Allow to attach a CPU via
> > > >     genpd_dev_pm_attach_by_id|name()
> > > >
> > > >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> > > >  include/linux/pm_domain.h   |  1 +
> > > >  2 files changed, 36 insertions(+), 38 deletions(-)
> > >
> > > Are there any dependencies between this and the series you've recently posted?
> >
> > Yep. I should have stated that, sorry. This should be applied on top.
>
> On top of which series?

[PATCH 0/3] PM / Domains: Improve support for multi PM domains

and

[PATCH v13 0/4] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM)

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Rafael J. Wysocki]

On Thu, Apr 25, 2019 at 12:17 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 25 Apr 2019 at 12:14, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Apr 25, 2019 at 12:11 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > > >
> > > > On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > >
> > > > > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > > > > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > > > > because of the virtual device that genpd allocates in this path.
> > > > >
> > > > > In this series, this limitation is addressed, together with a few other related
> > > > > fixes/cleanups.
> > > > >
> > > > > Ulf Hansson (4):
> > > > >   PM / Domains: Use the base device for
> > > > >     driver_deferred_probe_check_state()
> > > > >   PM / Domains: Drop unused in-parameter to some genpd functions
> > > > >   PM / Domains: Search for the CPU device outside the genpd lock
> > > > >   PM / Domains: Allow to attach a CPU via
> > > > >     genpd_dev_pm_attach_by_id|name()
> > > > >
> > > > >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> > > > >  include/linux/pm_domain.h   |  1 +
> > > > >  2 files changed, 36 insertions(+), 38 deletions(-)
> > > >
> > > > Are there any dependencies between this and the series you've recently posted?
> > >
> > > Yep. I should have stated that, sorry. This should be applied on top.
> >
> > On top of which series?
>
> [PATCH 0/3] PM / Domains: Improve support for multi PM domains
>
> and
>
> [PATCH v13 0/4] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM)

OK, thanks!

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Rafael J. Wysocki]

On Thu, Apr 25, 2019 at 12:17 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 25 Apr 2019 at 12:14, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Apr 25, 2019 at 12:11 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > > >
> > > > On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > >
> > > > > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > > > > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > > > > because of the virtual device that genpd allocates in this path.
> > > > >
> > > > > In this series, this limitation is addressed, together with a few other related
> > > > > fixes/cleanups.
> > > > >
> > > > > Ulf Hansson (4):
> > > > >   PM / Domains: Use the base device for
> > > > >     driver_deferred_probe_check_state()
> > > > >   PM / Domains: Drop unused in-parameter to some genpd functions
> > > > >   PM / Domains: Search for the CPU device outside the genpd lock
> > > > >   PM / Domains: Allow to attach a CPU via
> > > > >     genpd_dev_pm_attach_by_id|name()
> > > > >
> > > > >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> > > > >  include/linux/pm_domain.h   |  1 +
> > > > >  2 files changed, 36 insertions(+), 38 deletions(-)
> > > >
> > > > Are there any dependencies between this and the series you've recently posted?
> > >
> > > Yep. I should have stated that, sorry. This should be applied on top.
> >
> > On top of which series?
>
> [PATCH 0/3] PM / Domains: Improve support for multi PM domains
>
> and
>
> [PATCH v13 0/4] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM)

OK, thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Rafael J. Wysocki]

On Thursday, April 25, 2019 12:32:24 PM CEST Rafael J. Wysocki wrote:
> On Thu, Apr 25, 2019 at 12:17 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Thu, 25 Apr 2019 at 12:14, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Thu, Apr 25, 2019 at 12:11 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > >
> > > > On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > > > >
> > > > > On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > > >
> > > > > > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > > > > > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > > > > > because of the virtual device that genpd allocates in this path.
> > > > > >
> > > > > > In this series, this limitation is addressed, together with a few other related
> > > > > > fixes/cleanups.
> > > > > >
> > > > > > Ulf Hansson (4):
> > > > > >   PM / Domains: Use the base device for
> > > > > >     driver_deferred_probe_check_state()
> > > > > >   PM / Domains: Drop unused in-parameter to some genpd functions
> > > > > >   PM / Domains: Search for the CPU device outside the genpd lock
> > > > > >   PM / Domains: Allow to attach a CPU via
> > > > > >     genpd_dev_pm_attach_by_id|name()
> > > > > >
> > > > > >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> > > > > >  include/linux/pm_domain.h   |  1 +
> > > > > >  2 files changed, 36 insertions(+), 38 deletions(-)
> > > > >
> > > > > Are there any dependencies between this and the series you've recently posted?
> > > >
> > > > Yep. I should have stated that, sorry. This should be applied on top.
> > >
> > > On top of which series?
> >
> > [PATCH 0/3] PM / Domains: Improve support for multi PM domains
> >
> > and
> >
> > [PATCH v13 0/4] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM)
> 
> OK, thanks!
> 

This series has been applied, thanks!

Re: [PATCH 0/4] PM / Domains: Improve support for CPUs in genpd

[Rafael J. Wysocki]

On Thursday, April 25, 2019 12:32:24 PM CEST Rafael J. Wysocki wrote:
> On Thu, Apr 25, 2019 at 12:17 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Thu, 25 Apr 2019 at 12:14, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Thu, Apr 25, 2019 at 12:11 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > >
> > > > On Thu, 25 Apr 2019 at 11:56, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > > > >
> > > > > On Thu, Apr 25, 2019 at 11:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > > > >
> > > > > > Recently genpd was extended to cope with devices belonging to CPUs. However,
> > > > > > attaching CPU devices via genpd_dev_pm_attach_by_id|name() doesn't work,
> > > > > > because of the virtual device that genpd allocates in this path.
> > > > > >
> > > > > > In this series, this limitation is addressed, together with a few other related
> > > > > > fixes/cleanups.
> > > > > >
> > > > > > Ulf Hansson (4):
> > > > > >   PM / Domains: Use the base device for
> > > > > >     driver_deferred_probe_check_state()
> > > > > >   PM / Domains: Drop unused in-parameter to some genpd functions
> > > > > >   PM / Domains: Search for the CPU device outside the genpd lock
> > > > > >   PM / Domains: Allow to attach a CPU via
> > > > > >     genpd_dev_pm_attach_by_id|name()
> > > > > >
> > > > > >  drivers/base/power/domain.c | 73 ++++++++++++++++++-------------------
> > > > > >  include/linux/pm_domain.h   |  1 +
> > > > > >  2 files changed, 36 insertions(+), 38 deletions(-)
> > > > >
> > > > > Are there any dependencies between this and the series you've recently posted?
> > > >
> > > > Yep. I should have stated that, sorry. This should be applied on top.
> > >
> > > On top of which series?
> >
> > [PATCH 0/3] PM / Domains: Improve support for multi PM domains
> >
> > and
> >
> > [PATCH v13 0/4] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM)
> 
> OK, thanks!
> 

This series has been applied, thanks!





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel