From e757e7fa3a93afa54a1bf31953f458b0005e0910 Mon Sep 17 00:00:00 2001 From: Yangtao Li Date: Tue, 16 Apr 2019 12:23:05 -0400 Subject: PM / Domains: remove unnecessary unlikely() WARN_ON() already contains an unlikely(), so it's not necessary to use unlikely. Signed-off-by: Yangtao Li Reviewed-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 96a6dc9d305c..598a4e02aee1 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -391,11 +391,9 @@ int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state) if (unlikely(!genpd->set_performance_state)) return -EINVAL; - if (unlikely(!dev->power.subsys_data || - !dev->power.subsys_data->domain_data)) { - WARN_ON(1); + if (WARN_ON(!dev->power.subsys_data || + !dev->power.subsys_data->domain_data)) return -EINVAL; - } genpd_lock(genpd); -- cgit v1.2.3 From 71b77697af9ef06b559875e4bd8dc3d141807c93 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 18 Apr 2019 12:27:55 +0200 Subject: PM / Domains: Don't kfree() the virtual device in the error path It's not correct to call kfree(dev) when device_register(dev) has failed. Fix this by calling put_device(dev) instead. Fixes: 3c095f32a92b ("PM / Domains: Add support for multi PM domains per device to genpd") Signed-off-by: Ulf Hansson Acked-by: Viresh Kumar Acked-by: Niklas Cassel Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 598a4e02aee1..5422fc01dca3 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2455,7 +2455,7 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev, ret = device_register(virt_dev); if (ret) { - kfree(virt_dev); + put_device(virt_dev); return ERR_PTR(ret); } -- cgit v1.2.3 From e8b04de9da71b56dbbc5fa443d4ab52b617977bb Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 18 Apr 2019 12:27:56 +0200 Subject: PM / Domains: Allow OF lookup for multi PM domain case from ->attach_dev() A genpd provider that uses the ->attach_dev() callback to look up resources for a device fails to do so when the device has multiple PM domains attached. That is because when genpd invokes the ->attach_dev() callback, it passes the allocated virtual device as the in-parameter. To address this problem, simply assign the dev->of_node for the virtual device, based upon the original device's OF node. Signed-off-by: Ulf Hansson Acked-by: Viresh Kumar Acked-by: Niklas Cassel Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 5422fc01dca3..a0b021d53084 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2272,6 +2272,7 @@ EXPORT_SYMBOL_GPL(of_genpd_remove_last); static void genpd_release_dev(struct device *dev) { + of_node_put(dev->of_node); kfree(dev); } @@ -2333,14 +2334,14 @@ static void genpd_dev_pm_sync(struct device *dev) genpd_queue_power_off_work(pd); } -static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np, - unsigned int index, bool power_on) +static int __genpd_dev_pm_attach(struct device *dev, unsigned int index, + bool power_on) { struct of_phandle_args pd_args; struct generic_pm_domain *pd; int ret; - ret = of_parse_phandle_with_args(np, "power-domains", + ret = of_parse_phandle_with_args(dev->of_node, "power-domains", "#power-domain-cells", index, &pd_args); if (ret < 0) return ret; @@ -2408,7 +2409,7 @@ int genpd_dev_pm_attach(struct device *dev) "#power-domain-cells") != 1) return 0; - return __genpd_dev_pm_attach(dev, dev->of_node, 0, true); + return __genpd_dev_pm_attach(dev, 0, true); } EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); @@ -2452,6 +2453,7 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev, dev_set_name(virt_dev, "genpd:%u:%s", index, dev_name(dev)); virt_dev->bus = &genpd_bus_type; virt_dev->release = genpd_release_dev; + virt_dev->of_node = of_node_get(dev->of_node); ret = device_register(virt_dev); if (ret) { @@ -2460,7 +2462,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, dev->of_node, index, false); + ret = __genpd_dev_pm_attach(virt_dev, index, false); if (ret < 1) { device_unregister(virt_dev); return ret ? ERR_PTR(ret) : NULL; -- cgit v1.2.3 From 3ccf3f0cd1971e007680114ff732e8a717aafbf8 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 18 Apr 2019 12:27:57 +0200 Subject: PM / Domains: Enable genpd_dev_pm_attach_by_id|name() for single PM domain If a call to dev_pm_domain_attach() succeeds to attach a device to its single PM domain, the important point is to prevent subsequent dev_pm_domain_attach_by_name|id() calls from failing. That is done by checking the dev->pm_domain pointer and then returning -EEXIST, rather than continuing to call genpd_dev_pm_attach_by_id|name(). For this reason, enable genpd_dev_pm_attach_by_id|name() to be used for single PM domains too. This simplifies future users, so they only need to use dev_pm_domain_attach_by_id|name() instead of having to combine it with dev_pm_domain_attach(). Signed-off-by: Ulf Hansson Acked-by: Viresh Kumar Acked-by: Niklas Cassel Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index a0b021d53084..d97bcf6918fb 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2439,10 +2439,10 @@ struct device *genpd_dev_pm_attach_by_id(struct device *dev, if (!dev->of_node) return NULL; - /* Deal only with devices using multiple PM domains. */ + /* Verify that the index is within a valid range. */ num_domains = of_count_phandle_with_args(dev->of_node, "power-domains", "#power-domain-cells"); - if (num_domains < 2 || index >= num_domains) + if (index >= num_domains) return NULL; /* Allocate and register device on the genpd bus. */ -- cgit v1.2.3 From 51dcf7482f4b3166ef477d44181648110d501d25 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 25 Apr 2019 11:04:10 +0200 Subject: PM / Domains: Use the base device for driver_deferred_probe_check_state() 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 incorrect, 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") Signed-off-by: Ulf Hansson Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/base') 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; -- cgit v1.2.3 From a174920d69db9be80332f535db4a8ca2e3bccf24 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 25 Apr 2019 11:04:11 +0200 Subject: PM / Domains: Drop unused in-parameter to some genpd functions Both genpd_alloc_dev_data() and genpd_add_device(), that are internal genpd functions, allow 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 Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'drivers/base') 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) { -- cgit v1.2.3 From b24e196586fecafed1c3cff9b2f87c1a64138ade Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 25 Apr 2019 11:04:12 +0200 Subject: PM / Domains: Search for the CPU device outside the genpd lock While attaching/detaching a device to a PM domain (genpd) with GENPD_FLAG_CPU_DOMAIN set, genpd iterates the cpu_possible_mask to check whether or not the device corresponds to a CPU. This iteration is done while holding the genpd's lock, which is unnecessary. Avoid the locking by restructuring the corresponding code a bit. Signed-off-by: Ulf Hansson Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 52 +++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 25 deletions(-) (limited to 'drivers/base') 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); -- cgit v1.2.3 From f9ccd7c3a1d87cea3a6f9ed6c946dee9e7456b2e Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 25 Apr 2019 11:04:13 +0200 Subject: PM / Domains: Allow to attach a CPU via genpd_dev_pm_attach_by_id|name() Attaching a device via genpd_dev_pm_attach_by_id|name() makes genpd allocate a virtual device that it attaches instead. This leads to a problem in case when 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(), store the CPU number in struct generic_pm_domain_data, so as to be able to clear the corresponding bit in the cpumask for the genpd. Signed-off-by: Ulf Hansson Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'drivers/base') 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) { -- cgit v1.2.3