From 17ed15183c2453ba2f03a813fd8b1594bd9baf98 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 31 Aug 2018 11:25:13 +0300 Subject: powercap: RAPL: Get rid of custom RAPL_CPU() macro Replace custom grown macro with generic INTEL_CPU_FAM6() one. No functional change intended. Signed-off-by: Andy Shevchenko Signed-off-by: Rafael J. Wysocki --- drivers/powercap/intel_rapl.c | 73 +++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c index 295d8dcba48c..bb92874b1175 100644 --- a/drivers/powercap/intel_rapl.c +++ b/drivers/powercap/intel_rapl.c @@ -1133,47 +1133,40 @@ static const struct rapl_defaults rapl_defaults_cht = { .compute_time_window = rapl_compute_time_window_atom, }; -#define RAPL_CPU(_model, _ops) { \ - .vendor = X86_VENDOR_INTEL, \ - .family = 6, \ - .model = _model, \ - .driver_data = (kernel_ulong_t)&_ops, \ - } - static const struct x86_cpu_id rapl_ids[] __initconst = { - RAPL_CPU(INTEL_FAM6_SANDYBRIDGE, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_SANDYBRIDGE_X, rapl_defaults_core), - - RAPL_CPU(INTEL_FAM6_IVYBRIDGE, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_IVYBRIDGE_X, rapl_defaults_core), - - RAPL_CPU(INTEL_FAM6_HASWELL_CORE, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_HASWELL_ULT, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_HASWELL_GT3E, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_HASWELL_X, rapl_defaults_hsw_server), - - RAPL_CPU(INTEL_FAM6_BROADWELL_CORE, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_BROADWELL_GT3E, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_BROADWELL_XEON_D, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_BROADWELL_X, rapl_defaults_hsw_server), - - RAPL_CPU(INTEL_FAM6_SKYLAKE_DESKTOP, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_SKYLAKE_MOBILE, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_SKYLAKE_X, rapl_defaults_hsw_server), - RAPL_CPU(INTEL_FAM6_KABYLAKE_MOBILE, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_KABYLAKE_DESKTOP, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_CANNONLAKE_MOBILE, rapl_defaults_core), - - RAPL_CPU(INTEL_FAM6_ATOM_SILVERMONT1, rapl_defaults_byt), - RAPL_CPU(INTEL_FAM6_ATOM_AIRMONT, rapl_defaults_cht), - RAPL_CPU(INTEL_FAM6_ATOM_MERRIFIELD, rapl_defaults_tng), - RAPL_CPU(INTEL_FAM6_ATOM_MOOREFIELD, rapl_defaults_ann), - RAPL_CPU(INTEL_FAM6_ATOM_GOLDMONT, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, rapl_defaults_core), - RAPL_CPU(INTEL_FAM6_ATOM_DENVERTON, rapl_defaults_core), - - RAPL_CPU(INTEL_FAM6_XEON_PHI_KNL, rapl_defaults_hsw_server), - RAPL_CPU(INTEL_FAM6_XEON_PHI_KNM, rapl_defaults_hsw_server), + INTEL_CPU_FAM6(SANDYBRIDGE, rapl_defaults_core), + INTEL_CPU_FAM6(SANDYBRIDGE_X, rapl_defaults_core), + + INTEL_CPU_FAM6(IVYBRIDGE, rapl_defaults_core), + INTEL_CPU_FAM6(IVYBRIDGE_X, rapl_defaults_core), + + INTEL_CPU_FAM6(HASWELL_CORE, rapl_defaults_core), + INTEL_CPU_FAM6(HASWELL_ULT, rapl_defaults_core), + INTEL_CPU_FAM6(HASWELL_GT3E, rapl_defaults_core), + INTEL_CPU_FAM6(HASWELL_X, rapl_defaults_hsw_server), + + INTEL_CPU_FAM6(BROADWELL_CORE, rapl_defaults_core), + INTEL_CPU_FAM6(BROADWELL_GT3E, rapl_defaults_core), + INTEL_CPU_FAM6(BROADWELL_XEON_D, rapl_defaults_core), + INTEL_CPU_FAM6(BROADWELL_X, rapl_defaults_hsw_server), + + INTEL_CPU_FAM6(SKYLAKE_DESKTOP, rapl_defaults_core), + INTEL_CPU_FAM6(SKYLAKE_MOBILE, rapl_defaults_core), + INTEL_CPU_FAM6(SKYLAKE_X, rapl_defaults_hsw_server), + INTEL_CPU_FAM6(KABYLAKE_MOBILE, rapl_defaults_core), + INTEL_CPU_FAM6(KABYLAKE_DESKTOP, rapl_defaults_core), + INTEL_CPU_FAM6(CANNONLAKE_MOBILE, rapl_defaults_core), + + INTEL_CPU_FAM6(ATOM_SILVERMONT1, rapl_defaults_byt), + INTEL_CPU_FAM6(ATOM_AIRMONT, rapl_defaults_cht), + INTEL_CPU_FAM6(ATOM_MERRIFIELD, rapl_defaults_tng), + INTEL_CPU_FAM6(ATOM_MOOREFIELD, rapl_defaults_ann), + INTEL_CPU_FAM6(ATOM_GOLDMONT, rapl_defaults_core), + INTEL_CPU_FAM6(ATOM_GEMINI_LAKE, rapl_defaults_core), + INTEL_CPU_FAM6(ATOM_DENVERTON, rapl_defaults_core), + + INTEL_CPU_FAM6(XEON_PHI_KNL, rapl_defaults_hsw_server), + INTEL_CPU_FAM6(XEON_PHI_KNM, rapl_defaults_hsw_server), {} }; MODULE_DEVICE_TABLE(x86cpu, rapl_ids); -- cgit v1.2.3 From 2fbb8670b4ff4454f1c0de510f788d737edc4b90 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Tue, 11 Sep 2018 11:14:34 +0530 Subject: OPP: Free OPP table properly on performance state irregularities The OPP table was freed, but not the individual OPPs which is done from _dev_pm_opp_remove_table(). Fix it by calling _dev_pm_opp_remove_table() as well. Cc: 4.18 # v4.18 Fixes: 3ba98324e81a ("PM / OPP: Get performance state using genpd helper") Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 7af0ddec936b..20988c426650 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -425,6 +425,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) dev_err(dev, "Not all nodes have performance state set (%d: %d)\n", count, pstate_count); ret = -ENOENT; + _dev_pm_opp_remove_table(opp_table, dev, false); goto put_opp_table; } -- cgit v1.2.3 From 404b1369ea26f598b96ae4d3183262a879703cfe Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 13 Sep 2018 13:09:27 +0530 Subject: OPP: Don't try to remove all OPP tables on failure dev_pm_opp_of_cpumask_add_table() creates the OPP table for all CPUs present in the cpumask and on errors it should revert all changes it has done. It actually is doing a bit more than that. On errors, it tries to free all the OPP tables, even the one it hasn't created yet. This may also end up freeing the OPP tables which were created from separate path, like dev_pm_opp_set_supported_hw(). Reported-and-tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/cpu.c | 8 ++++++-- drivers/opp/of.c | 4 ++-- drivers/opp/opp.h | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/opp/cpu.c b/drivers/opp/cpu.c index 0c0910709435..2eb5e2e7ff66 100644 --- a/drivers/opp/cpu.c +++ b/drivers/opp/cpu.c @@ -108,7 +108,8 @@ void dev_pm_opp_free_cpufreq_table(struct device *dev, EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table); #endif /* CONFIG_CPU_FREQ */ -void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of) +void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of, + int last_cpu) { struct device *cpu_dev; int cpu; @@ -116,6 +117,9 @@ void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of) WARN_ON(cpumask_empty(cpumask)); for_each_cpu(cpu, cpumask) { + if (cpu == last_cpu) + break; + cpu_dev = get_cpu_device(cpu); if (!cpu_dev) { pr_err("%s: failed to get cpu%d device\n", __func__, @@ -140,7 +144,7 @@ void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of) */ void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask) { - _dev_pm_opp_cpumask_remove_table(cpumask, false); + _dev_pm_opp_cpumask_remove_table(cpumask, false, -1); } EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_remove_table); diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 20988c426650..86222586f27b 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -592,7 +592,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_add_table_indexed); */ void dev_pm_opp_of_cpumask_remove_table(const struct cpumask *cpumask) { - _dev_pm_opp_cpumask_remove_table(cpumask, true); + _dev_pm_opp_cpumask_remove_table(cpumask, true, -1); } EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_remove_table); @@ -627,7 +627,7 @@ int dev_pm_opp_of_cpumask_add_table(const struct cpumask *cpumask) __func__, cpu, ret); /* Free all other OPPs */ - dev_pm_opp_of_cpumask_remove_table(cpumask); + _dev_pm_opp_cpumask_remove_table(cpumask, true, cpu); break; } } diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 7c540fd063b2..a9d22aa534c3 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -196,7 +196,7 @@ struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table); void _opp_free(struct dev_pm_opp *opp); int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table, bool rate_not_available); int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic); -void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of); +void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of, int last_cpu); struct opp_table *_add_opp_table(struct device *dev); #ifdef CONFIG_OF -- cgit v1.2.3 From 3d2556992a878a2210d3be498416aee39e0c32aa Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 3 Aug 2018 07:05:21 +0530 Subject: OPP: Protect dev_list with opp_table lock The dev_list needs to be protected with a lock, else we may have simultaneous access (addition/removal) to it and that would be racy. Extend scope of the opp_table lock to protect dev_list as well. Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 21 +++++++++++++++++++-- drivers/opp/cpu.c | 2 ++ drivers/opp/opp.h | 2 +- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 31ff03dbeb83..9f8aa31265fe 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -48,9 +48,14 @@ static struct opp_device *_find_opp_dev(const struct device *dev, static struct opp_table *_find_opp_table_unlocked(struct device *dev) { struct opp_table *opp_table; + bool found; list_for_each_entry(opp_table, &opp_tables, node) { - if (_find_opp_dev(dev, opp_table)) { + mutex_lock(&opp_table->lock); + found = !!_find_opp_dev(dev, opp_table); + mutex_unlock(&opp_table->lock); + + if (found) { _get_opp_table_kref(opp_table); return opp_table; @@ -766,6 +771,8 @@ struct opp_device *_add_opp_dev(const struct device *dev, /* Initialize opp-dev */ opp_dev->dev = dev; + + mutex_lock(&opp_table->lock); list_add(&opp_dev->node, &opp_table->dev_list); /* Create debugfs entries for the opp_table */ @@ -773,6 +780,7 @@ struct opp_device *_add_opp_dev(const struct device *dev, if (ret) dev_err(dev, "%s: Failed to register opp debugfs (%d)\n", __func__, ret); + mutex_unlock(&opp_table->lock); return opp_dev; } @@ -791,6 +799,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev) if (!opp_table) return NULL; + mutex_init(&opp_table->lock); INIT_LIST_HEAD(&opp_table->dev_list); opp_dev = _add_opp_dev(dev, opp_table); @@ -812,7 +821,6 @@ static struct opp_table *_allocate_opp_table(struct device *dev) BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head); INIT_LIST_HEAD(&opp_table->opp_list); - mutex_init(&opp_table->lock); kref_init(&opp_table->kref); /* Secure the device table modification */ @@ -854,6 +862,10 @@ static void _opp_table_kref_release(struct kref *kref) if (!IS_ERR(opp_table->clk)) clk_put(opp_table->clk); + /* + * No need to take opp_table->lock here as we are guaranteed that no + * references to the OPP table are taken at this point. + */ opp_dev = list_first_entry(&opp_table->dev_list, struct opp_device, node); @@ -1716,6 +1728,9 @@ void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, { struct dev_pm_opp *opp, *tmp; + /* Protect dev_list */ + mutex_lock(&opp_table->lock); + /* Find if opp_table manages a single device */ if (list_is_singular(&opp_table->dev_list)) { /* Free static OPPs */ @@ -1733,6 +1748,8 @@ void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, } else { _remove_opp_dev(_find_opp_dev(dev, opp_table), opp_table); } + + mutex_unlock(&opp_table->lock); } void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all) diff --git a/drivers/opp/cpu.c b/drivers/opp/cpu.c index 2eb5e2e7ff66..36586f66cd83 100644 --- a/drivers/opp/cpu.c +++ b/drivers/opp/cpu.c @@ -226,8 +226,10 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) cpumask_clear(cpumask); if (opp_table->shared_opp == OPP_TABLE_ACCESS_SHARED) { + mutex_lock(&opp_table->lock); list_for_each_entry(opp_dev, &opp_table->dev_list, node) cpumask_set_cpu(opp_dev->dev->id, cpumask); + mutex_unlock(&opp_table->lock); } else { cpumask_set_cpu(cpu_dev->id, cpumask); } diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index a9d22aa534c3..88e9f47aadf1 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -126,7 +126,7 @@ enum opp_table_access { * @dev_list: list of devices that share these OPPs * @opp_list: table of opps * @kref: for reference count of the table. - * @lock: mutex protecting the opp_list. + * @lock: mutex protecting the opp_list and dev_list. * @np: struct device_node pointer for opp's DT node. * @clock_latency_ns_max: Max clock latency in nanoseconds. * @shared_opp: OPP is shared between multiple devices. -- cgit v1.2.3 From eb7c8743d6cf489e30091e6656fd4d3306621e9a Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 5 Sep 2018 16:17:14 +0530 Subject: OPP: Pass index to _of_init_opp_table() This is a preparatory patch required for the next commit which will start using OPP table's node pointer in _of_init_opp_table(), which requires the index in order to read the OPP table's phandle. This commit adds the index argument in the call chains in order to get it delivered to _of_init_opp_table(). Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 19 +++++++++++++++---- drivers/opp/of.c | 12 +++++++----- drivers/opp/opp.h | 4 ++-- include/linux/pm_opp.h | 6 ++++++ 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 9f8aa31265fe..332748adc262 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -785,7 +785,7 @@ struct opp_device *_add_opp_dev(const struct device *dev, return opp_dev; } -static struct opp_table *_allocate_opp_table(struct device *dev) +static struct opp_table *_allocate_opp_table(struct device *dev, int index) { struct opp_table *opp_table; struct opp_device *opp_dev; @@ -808,7 +808,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev) return NULL; } - _of_init_opp_table(opp_table, dev); + _of_init_opp_table(opp_table, dev, index); /* Find clk for the device */ opp_table->clk = clk_get(dev, NULL); @@ -833,7 +833,7 @@ void _get_opp_table_kref(struct opp_table *opp_table) kref_get(&opp_table->kref); } -struct opp_table *dev_pm_opp_get_opp_table(struct device *dev) +static struct opp_table *_opp_get_opp_table(struct device *dev, int index) { struct opp_table *opp_table; @@ -844,15 +844,26 @@ struct opp_table *dev_pm_opp_get_opp_table(struct device *dev) if (!IS_ERR(opp_table)) goto unlock; - opp_table = _allocate_opp_table(dev); + opp_table = _allocate_opp_table(dev, index); unlock: mutex_unlock(&opp_table_lock); return opp_table; } + +struct opp_table *dev_pm_opp_get_opp_table(struct device *dev) +{ + return _opp_get_opp_table(dev, 0); +} EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_table); +struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *dev, + int index) +{ + return _opp_get_opp_table(dev, index); +} + static void _opp_table_kref_release(struct kref *kref) { struct opp_table *opp_table = container_of(kref, struct opp_table, kref); diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 86222586f27b..1a9e1242a2a7 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -52,7 +52,8 @@ static struct opp_table *_managed_opp(const struct device_node *np) return managed_table; } -void _of_init_opp_table(struct opp_table *opp_table, struct device *dev) +void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, + int index) { struct device_node *np; @@ -378,7 +379,8 @@ free_opp: } /* Initializes OPP tables based on new bindings */ -static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) +static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, + int index) { struct device_node *np; struct opp_table *opp_table; @@ -393,7 +395,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) goto put_opp_table; } - opp_table = dev_pm_opp_get_opp_table(dev); + opp_table = dev_pm_opp_get_opp_table_indexed(dev, index); if (!opp_table) return -ENOMEM; @@ -526,7 +528,7 @@ int dev_pm_opp_of_add_table(struct device *dev) return _of_add_opp_table_v1(dev); } - ret = _of_add_opp_table_v2(dev, opp_np); + ret = _of_add_opp_table_v2(dev, opp_np, 0); of_node_put(opp_np); return ret; @@ -574,7 +576,7 @@ again: return -ENODEV; } - ret = _of_add_opp_table_v2(dev, opp_np); + ret = _of_add_opp_table_v2(dev, opp_np, index); of_node_put(opp_np); return ret; diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 88e9f47aadf1..b235e76fc8cc 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -200,9 +200,9 @@ void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of, in struct opp_table *_add_opp_table(struct device *dev); #ifdef CONFIG_OF -void _of_init_opp_table(struct opp_table *opp_table, struct device *dev); +void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index); #else -static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev) {} +static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {} #endif #ifdef CONFIG_DEBUG_FS diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 099b31960dec..5d399eeef172 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -79,6 +79,7 @@ struct dev_pm_set_opp_data { #if defined(CONFIG_PM_OPP) struct opp_table *dev_pm_opp_get_opp_table(struct device *dev); +struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *dev, int index); void dev_pm_opp_put_opp_table(struct opp_table *opp_table); unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp); @@ -136,6 +137,11 @@ static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev) return ERR_PTR(-ENOTSUPP); } +static inline struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *dev, int index) +{ + return ERR_PTR(-ENOTSUPP); +} + static inline void dev_pm_opp_put_opp_table(struct opp_table *opp_table) {} static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp) -- cgit v1.2.3 From f06ed90e7051a3a50c2172001c86ff9645e5c2ba Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 14 Jun 2018 12:04:43 +0530 Subject: OPP: Parse OPP table's DT properties from _of_init_opp_table() Parse the DT properties present in the OPP table from _of_init_opp_table(), which is a dedicated routine for DT parsing. Minor relocation of helpers is required for this. It is possible now for _managed_opp() to return a partially initialized OPP table if the OPP table is created via the helpers like dev_pm_opp_set_supported_hw() and we need another flag to indicate if the static OPP are already parsed or not to make sure we don't incorrectly skip initializing the static OPPs. Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 79 +++++++++++++++++++++++++++++++++---------------------- drivers/opp/opp.h | 2 ++ 2 files changed, 50 insertions(+), 31 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 1a9e1242a2a7..4a19f76880d3 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -23,6 +23,24 @@ #include "opp.h" +/* + * Returns opp descriptor node for a device node, caller must + * do of_node_put(). + */ +static struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, + int index) +{ + /* "operating-points-v2" can be an array for power domain providers */ + return of_parse_phandle(np, "operating-points-v2", index); +} + +/* Returns opp descriptor node for a device, caller must do of_node_put() */ +struct device_node *dev_pm_opp_of_get_opp_desc_node(struct device *dev) +{ + return _opp_of_get_opp_desc_node(dev->of_node, 0); +} +EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_opp_desc_node); + static struct opp_table *_managed_opp(const struct device_node *np) { struct opp_table *opp_table, *managed_table = NULL; @@ -55,22 +73,37 @@ static struct opp_table *_managed_opp(const struct device_node *np) void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) { - struct device_node *np; + struct device_node *np, *opp_np; + u32 val; /* * Only required for backward compatibility with v1 bindings, but isn't * harmful for other cases. And so we do it unconditionally. */ np = of_node_get(dev->of_node); - if (np) { - u32 val; - - if (!of_property_read_u32(np, "clock-latency", &val)) - opp_table->clock_latency_ns_max = val; - of_property_read_u32(np, "voltage-tolerance", - &opp_table->voltage_tolerance_v1); - of_node_put(np); - } + if (!np) + return; + + if (!of_property_read_u32(np, "clock-latency", &val)) + opp_table->clock_latency_ns_max = val; + of_property_read_u32(np, "voltage-tolerance", + &opp_table->voltage_tolerance_v1); + + /* Get OPP table node */ + opp_np = _opp_of_get_opp_desc_node(np, index); + of_node_put(np); + + if (!opp_np) + return; + + if (of_property_read_bool(opp_np, "opp-shared")) + opp_table->shared_opp = OPP_TABLE_ACCESS_SHARED; + else + opp_table->shared_opp = OPP_TABLE_ACCESS_EXCLUSIVE; + + opp_table->np = opp_np; + + of_node_put(opp_np); } static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table, @@ -250,22 +283,6 @@ void dev_pm_opp_of_remove_table(struct device *dev) } EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table); -/* Returns opp descriptor node for a device node, caller must - * do of_node_put() */ -static struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, - int index) -{ - /* "operating-points-v2" can be an array for power domain providers */ - return of_parse_phandle(np, "operating-points-v2", index); -} - -/* Returns opp descriptor node for a device, caller must do of_node_put() */ -struct device_node *dev_pm_opp_of_get_opp_desc_node(struct device *dev) -{ - return _opp_of_get_opp_desc_node(dev->of_node, 0); -} -EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_opp_desc_node); - /** * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) * @opp_table: OPP table @@ -392,6 +409,9 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, /* OPPs are already managed */ if (!_add_opp_dev(dev, opp_table)) ret = -ENOMEM; + else if (!opp_table->parsed_static_opps) + goto initialize_static_opps; + goto put_opp_table; } @@ -399,6 +419,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, if (!opp_table) return -ENOMEM; +initialize_static_opps: /* We have opp-table node now, iterate over it and add OPPs */ for_each_available_child_of_node(opp_np, np) { count++; @@ -434,11 +455,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, if (pstate_count) opp_table->genpd_performance_state = true; - opp_table->np = opp_np; - if (of_property_read_bool(opp_np, "opp-shared")) - opp_table->shared_opp = OPP_TABLE_ACCESS_SHARED; - else - opp_table->shared_opp = OPP_TABLE_ACCESS_EXCLUSIVE; + opp_table->parsed_static_opps = true; put_opp_table: dev_pm_opp_put_opp_table(opp_table); diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index b235e76fc8cc..b04c2b511c4d 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -129,6 +129,7 @@ enum opp_table_access { * @lock: mutex protecting the opp_list and dev_list. * @np: struct device_node pointer for opp's DT node. * @clock_latency_ns_max: Max clock latency in nanoseconds. + * @parsed_static_opps: True if OPPs are initialized from DT. * @shared_opp: OPP is shared between multiple devices. * @suspend_opp: Pointer to OPP to be used during device suspend. * @supported_hw: Array of version number to support. @@ -164,6 +165,7 @@ struct opp_table { /* For backward compatibility with v1 bindings */ unsigned int voltage_tolerance_v1; + bool parsed_static_opps; enum opp_table_access shared_opp; struct dev_pm_opp *suspend_opp; -- cgit v1.2.3 From 0ad8c623907c27f4b8572d36c4ba73ea103e3108 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 2 Aug 2018 14:23:09 +0530 Subject: OPP: Don't take OPP table's kref for static OPPs The reference count is only required to be incremented for every call that may lead to adding the OPP table. For static OPPs the same should be done from the parent routine which adds all static OPPs together and so only one refcount for all static OPPs. Update code to reflect that. The refcount is incremented every time a dynamic OPP is created (as that can lead to creating the OPP table) and the same is dropped when the OPP is removed. Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 332748adc262..2a6976265580 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -919,7 +919,6 @@ static void _opp_kref_release(struct kref *kref) kfree(opp); mutex_unlock(&opp_table->lock); - dev_pm_opp_put_opp_table(opp_table); } void dev_pm_opp_get(struct dev_pm_opp *opp) @@ -963,11 +962,15 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) if (found) { dev_pm_opp_put(opp); + + /* Drop the reference taken by dev_pm_opp_add() */ + dev_pm_opp_put_opp_table(opp_table); } else { dev_warn(dev, "%s: Couldn't find OPP with freq: %lu\n", __func__, freq); } + /* Drop the reference taken by _find_opp_table() */ dev_pm_opp_put_opp_table(opp_table); } EXPORT_SYMBOL_GPL(dev_pm_opp_remove); @@ -1085,9 +1088,6 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, new_opp->opp_table = opp_table; kref_init(&new_opp->kref); - /* Get a reference to the OPP table */ - _get_opp_table_kref(opp_table); - ret = opp_debug_create_one(new_opp, opp_table); if (ret) dev_err(dev, "%s: Failed to register opp to debugfs (%d)\n", @@ -1566,8 +1566,9 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt) return -ENOMEM; ret = _opp_add_v1(opp_table, dev, freq, u_volt, true); + if (ret) + dev_pm_opp_put_opp_table(opp_table); - dev_pm_opp_put_opp_table(opp_table); return ret; } EXPORT_SYMBOL_GPL(dev_pm_opp_add); -- cgit v1.2.3 From d0e8ae6c26da7b9436775dfd9768d7a821ed47b7 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Tue, 11 Sep 2018 11:14:11 +0530 Subject: OPP: Create separate kref for static OPPs list The static OPPs don't always get freed with the OPP table, it can happen before that as well. For example, if the OPP table is first created using helpers like dev_pm_opp_set_supported_hw() and the OPPs are created at a later point. Now when the OPPs are removed, the OPP table stays until the time dev_pm_opp_put_supported_hw() is called. Later patches will streamline the freeing of OPP table and that requires the static OPPs to get freed with help of a separate kernel reference. This patch prepares for that by creating a separate kref for static OPPs list. Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 33 ++++++++++++++++++++++++++++++++- drivers/opp/of.c | 7 +++++++ drivers/opp/opp.h | 3 +++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 2a6976265580..b555121b878b 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -892,6 +892,33 @@ static void _opp_table_kref_release(struct kref *kref) mutex_unlock(&opp_table_lock); } +void _opp_remove_all_static(struct opp_table *opp_table) +{ + struct dev_pm_opp *opp, *tmp; + + list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) { + if (!opp->dynamic) + dev_pm_opp_put(opp); + } + + opp_table->parsed_static_opps = false; +} + +static void _opp_table_list_kref_release(struct kref *kref) +{ + struct opp_table *opp_table = container_of(kref, struct opp_table, + list_kref); + + _opp_remove_all_static(opp_table); + mutex_unlock(&opp_table_lock); +} + +void _put_opp_list_kref(struct opp_table *opp_table) +{ + kref_put_mutex(&opp_table->list_kref, _opp_table_list_kref_release, + &opp_table_lock); +} + void dev_pm_opp_put_opp_table(struct opp_table *opp_table) { kref_put_mutex(&opp_table->kref, _opp_table_kref_release, @@ -1746,8 +1773,11 @@ void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, /* Find if opp_table manages a single device */ if (list_is_singular(&opp_table->dev_list)) { /* Free static OPPs */ + _put_opp_list_kref(opp_table); + + /* Free dynamic OPPs */ list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) { - if (remove_all || !opp->dynamic) + if (remove_all) dev_pm_opp_put(opp); } @@ -1758,6 +1788,7 @@ void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, if (opp_table->genpd_performance_state) dev_pm_genpd_set_performance_state(dev, 0); } else { + _put_opp_list_kref(opp_table); _remove_opp_dev(_find_opp_dev(dev, opp_table), opp_table); } diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 4a19f76880d3..aaa4bab69846 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -411,6 +411,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, ret = -ENOMEM; else if (!opp_table->parsed_static_opps) goto initialize_static_opps; + else + kref_get(&opp_table->list_kref); goto put_opp_table; } @@ -420,6 +422,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, return -ENOMEM; initialize_static_opps: + kref_init(&opp_table->list_kref); + /* We have opp-table node now, iterate over it and add OPPs */ for_each_available_child_of_node(opp_np, np) { count++; @@ -437,6 +441,7 @@ initialize_static_opps: /* There should be one of more OPP defined */ if (WARN_ON(!count)) { ret = -ENOENT; + _put_opp_list_kref(opp_table); goto put_opp_table; } @@ -491,6 +496,8 @@ static int _of_add_opp_table_v1(struct device *dev) if (!opp_table) return -ENOMEM; + kref_init(&opp_table->list_kref); + val = prop->value; while (nr) { unsigned long freq = be32_to_cpup(val++) * 1000; diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index b04c2b511c4d..9274116c90e4 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -126,6 +126,7 @@ enum opp_table_access { * @dev_list: list of devices that share these OPPs * @opp_list: table of opps * @kref: for reference count of the table. + * @list_kref: for reference count of the OPP list. * @lock: mutex protecting the opp_list and dev_list. * @np: struct device_node pointer for opp's DT node. * @clock_latency_ns_max: Max clock latency in nanoseconds. @@ -157,6 +158,7 @@ struct opp_table { struct list_head dev_list; struct list_head opp_list; struct kref kref; + struct kref list_kref; struct mutex lock; struct device_node *np; @@ -200,6 +202,7 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *o int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic); void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of, int last_cpu); struct opp_table *_add_opp_table(struct device *dev); +void _put_opp_list_kref(struct opp_table *opp_table); #ifdef CONFIG_OF void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index); -- cgit v1.2.3 From 883071c4bd9a6e45bf0b5e45575d110eaeb2e6d0 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 12 Sep 2018 12:25:22 +0530 Subject: cpufreq: mvebu: Remove OPPs using dev_pm_opp_remove() dev_pm_opp_cpumask_remove_table() is going to change in the next commit and will not remove dynamic OPPs automatically. They must be removed with a call to dev_pm_opp_remove(). Reviewed-by: Gregory CLEMENT Signed-off-by: Viresh Kumar --- drivers/cpufreq/mvebu-cpufreq.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/cpufreq/mvebu-cpufreq.c b/drivers/cpufreq/mvebu-cpufreq.c index 31513bd42705..6d33a639f902 100644 --- a/drivers/cpufreq/mvebu-cpufreq.c +++ b/drivers/cpufreq/mvebu-cpufreq.c @@ -84,9 +84,10 @@ static int __init armada_xp_pmsu_cpufreq_init(void) ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk) / 2, 0); if (ret) { + dev_pm_opp_remove(cpu_dev, clk_get_rate(clk)); clk_put(clk); dev_err(cpu_dev, "Failed to register OPPs\n"); - goto opp_register_failed; + return ret; } ret = dev_pm_opp_set_sharing_cpus(cpu_dev, @@ -99,11 +100,5 @@ static int __init armada_xp_pmsu_cpufreq_init(void) platform_device_register_simple("cpufreq-dt", -1, NULL, 0); return 0; - -opp_register_failed: - /* As registering has failed remove all the opp for all cpus */ - dev_pm_opp_cpumask_remove_table(cpu_possible_mask); - - return ret; } device_initcall(armada_xp_pmsu_cpufreq_init); -- cgit v1.2.3 From 2a4eb7358aba6beff7fa23f028c733310756e525 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 13 Sep 2018 13:14:36 +0530 Subject: OPP: Don't remove dynamic OPPs from _dev_pm_opp_remove_table() Only one platform was depending on this feature and it is already updated now. Stop removing dynamic OPPs from _dev_pm_opp_remove_table(). This simplifies lot of paths and removes unnecessary parameters. Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 20 +++++--------------- drivers/opp/cpu.c | 9 +++------ drivers/opp/of.c | 12 ++++++------ drivers/opp/opp.h | 6 +++--- 4 files changed, 17 insertions(+), 30 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index b555121b878b..2319ad4a0177 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1759,14 +1759,10 @@ int dev_pm_opp_unregister_notifier(struct device *dev, EXPORT_SYMBOL(dev_pm_opp_unregister_notifier); /* - * Free OPPs either created using static entries present in DT or even the - * dynamically added entries based on remove_all param. + * Free OPPs either created using static entries present in DT. */ -void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, - bool remove_all) +void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev) { - struct dev_pm_opp *opp, *tmp; - /* Protect dev_list */ mutex_lock(&opp_table->lock); @@ -1775,12 +1771,6 @@ void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, /* Free static OPPs */ _put_opp_list_kref(opp_table); - /* Free dynamic OPPs */ - list_for_each_entry_safe(opp, tmp, &opp_table->opp_list, node) { - if (remove_all) - dev_pm_opp_put(opp); - } - /* * The OPP table is getting removed, drop the performance state * constraints. @@ -1795,7 +1785,7 @@ void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, mutex_unlock(&opp_table->lock); } -void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all) +void _dev_pm_opp_find_and_remove_table(struct device *dev) { struct opp_table *opp_table; @@ -1812,7 +1802,7 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all) return; } - _dev_pm_opp_remove_table(opp_table, dev, remove_all); + _dev_pm_opp_remove_table(opp_table, dev); dev_pm_opp_put_opp_table(opp_table); } @@ -1826,6 +1816,6 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all) */ void dev_pm_opp_remove_table(struct device *dev) { - _dev_pm_opp_find_and_remove_table(dev, true); + _dev_pm_opp_find_and_remove_table(dev); } EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table); diff --git a/drivers/opp/cpu.c b/drivers/opp/cpu.c index 36586f66cd83..ab6d07e78945 100644 --- a/drivers/opp/cpu.c +++ b/drivers/opp/cpu.c @@ -108,7 +108,7 @@ void dev_pm_opp_free_cpufreq_table(struct device *dev, EXPORT_SYMBOL_GPL(dev_pm_opp_free_cpufreq_table); #endif /* CONFIG_CPU_FREQ */ -void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of, +void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, int last_cpu) { struct device *cpu_dev; @@ -127,10 +127,7 @@ void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of, continue; } - if (of) - dev_pm_opp_of_remove_table(cpu_dev); - else - dev_pm_opp_remove_table(cpu_dev); + _dev_pm_opp_find_and_remove_table(cpu_dev); } } @@ -144,7 +141,7 @@ void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of, */ void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask) { - _dev_pm_opp_cpumask_remove_table(cpumask, false, -1); + _dev_pm_opp_cpumask_remove_table(cpumask, -1); } EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_remove_table); diff --git a/drivers/opp/of.c b/drivers/opp/of.c index aaa4bab69846..861cc75de329 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -279,7 +279,7 @@ free_microvolt: */ void dev_pm_opp_of_remove_table(struct device *dev) { - _dev_pm_opp_find_and_remove_table(dev, false); + _dev_pm_opp_find_and_remove_table(dev); } EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table); @@ -432,7 +432,7 @@ initialize_static_opps: if (ret) { dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, ret); - _dev_pm_opp_remove_table(opp_table, dev, false); + _dev_pm_opp_remove_table(opp_table, dev); of_node_put(np); goto put_opp_table; } @@ -453,7 +453,7 @@ initialize_static_opps: dev_err(dev, "Not all nodes have performance state set (%d: %d)\n", count, pstate_count); ret = -ENOENT; - _dev_pm_opp_remove_table(opp_table, dev, false); + _dev_pm_opp_remove_table(opp_table, dev); goto put_opp_table; } @@ -507,7 +507,7 @@ static int _of_add_opp_table_v1(struct device *dev) if (ret) { dev_err(dev, "%s: Failed to add OPP %ld (%d)\n", __func__, freq, ret); - _dev_pm_opp_remove_table(opp_table, dev, false); + _dev_pm_opp_remove_table(opp_table, dev); break; } nr -= 2; @@ -618,7 +618,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_add_table_indexed); */ void dev_pm_opp_of_cpumask_remove_table(const struct cpumask *cpumask) { - _dev_pm_opp_cpumask_remove_table(cpumask, true, -1); + _dev_pm_opp_cpumask_remove_table(cpumask, -1); } EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_remove_table); @@ -653,7 +653,7 @@ int dev_pm_opp_of_cpumask_add_table(const struct cpumask *cpumask) __func__, cpu, ret); /* Free all other OPPs */ - _dev_pm_opp_cpumask_remove_table(cpumask, true, cpu); + _dev_pm_opp_cpumask_remove_table(cpumask, cpu); break; } } diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 9274116c90e4..98dd7d39e1ad 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -194,13 +194,13 @@ void _get_opp_table_kref(struct opp_table *opp_table); int _get_opp_count(struct opp_table *opp_table); struct opp_table *_find_opp_table(struct device *dev); struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table); -void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev, bool remove_all); -void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all); +void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev); +void _dev_pm_opp_find_and_remove_table(struct device *dev); struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table); void _opp_free(struct dev_pm_opp *opp); int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table, bool rate_not_available); int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic); -void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, bool of, int last_cpu); +void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, int last_cpu); struct opp_table *_add_opp_table(struct device *dev); void _put_opp_list_kref(struct opp_table *opp_table); -- cgit v1.2.3 From cdd6ed90cdb6c2fd982909501f0a109274147fb4 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 12 Sep 2018 12:35:19 +0530 Subject: OPP: Use a single mechanism to free the OPP table Currently there are two separate ways to free the OPP table based on how it is created in the first place. We call _dev_pm_opp_remove_table() to free the static and/or dynamic OPP, OPP list devices, etc. This is done for the case where the OPP table is added while initializing the OPPs, like via the path dev_pm_opp_of_add_table(). We also call dev_pm_opp_put_opp_table() in some cases which eventually frees the OPP table structure once the reference count reaches 0. This is used by the first case as well as other cases like dev_pm_opp_set_regulators() where the OPPs aren't necessarily initialized at this point. This whole thing is a bit unclear and messy and obstruct any further cleanup/fixup of OPP core. This patch tries to streamline this by keeping a single path for OPP table destruction, i.e. dev_pm_opp_put_opp_table(). All the cleanup happens in _opp_table_kref_release() now after the reference count reaches 0. _dev_pm_opp_remove_table() is removed as it isn't required anymore. We don't drop the reference to the OPP table after creating it from _of_add_opp_table_v{1|2}() anymore and the same is dropped only when we try to remove them. Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 54 ++++++++++++++++-------------------------------------- drivers/opp/of.c | 32 ++++++++++++++++++-------------- drivers/opp/opp.h | 2 +- 3 files changed, 35 insertions(+), 53 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 2319ad4a0177..d3e33fd32694 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -867,23 +867,24 @@ struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device *dev, static void _opp_table_kref_release(struct kref *kref) { struct opp_table *opp_table = container_of(kref, struct opp_table, kref); - struct opp_device *opp_dev; + struct opp_device *opp_dev, *temp; /* Release clk */ if (!IS_ERR(opp_table->clk)) clk_put(opp_table->clk); - /* - * No need to take opp_table->lock here as we are guaranteed that no - * references to the OPP table are taken at this point. - */ - opp_dev = list_first_entry(&opp_table->dev_list, struct opp_device, - node); + WARN_ON(!list_empty(&opp_table->opp_list)); - _remove_opp_dev(opp_dev, opp_table); + list_for_each_entry_safe(opp_dev, temp, &opp_table->dev_list, node) { + /* + * The OPP table is getting removed, drop the performance state + * constraints. + */ + if (opp_table->genpd_performance_state) + dev_pm_genpd_set_performance_state((struct device *)(opp_dev->dev), 0); - /* dev_list must be empty now */ - WARN_ON(!list_empty(&opp_table->dev_list)); + _remove_opp_dev(opp_dev, opp_table); + } mutex_destroy(&opp_table->lock); list_del(&opp_table->node); @@ -1758,33 +1759,6 @@ int dev_pm_opp_unregister_notifier(struct device *dev, } EXPORT_SYMBOL(dev_pm_opp_unregister_notifier); -/* - * Free OPPs either created using static entries present in DT. - */ -void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev) -{ - /* Protect dev_list */ - mutex_lock(&opp_table->lock); - - /* Find if opp_table manages a single device */ - if (list_is_singular(&opp_table->dev_list)) { - /* Free static OPPs */ - _put_opp_list_kref(opp_table); - - /* - * The OPP table is getting removed, drop the performance state - * constraints. - */ - if (opp_table->genpd_performance_state) - dev_pm_genpd_set_performance_state(dev, 0); - } else { - _put_opp_list_kref(opp_table); - _remove_opp_dev(_find_opp_dev(dev, opp_table), opp_table); - } - - mutex_unlock(&opp_table->lock); -} - void _dev_pm_opp_find_and_remove_table(struct device *dev) { struct opp_table *opp_table; @@ -1802,8 +1776,12 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev) return; } - _dev_pm_opp_remove_table(opp_table, dev); + _put_opp_list_kref(opp_table); + + /* Drop reference taken by _find_opp_table() */ + dev_pm_opp_put_opp_table(opp_table); + /* Drop reference taken while the OPP table was added */ dev_pm_opp_put_opp_table(opp_table); } diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 861cc75de329..ae0436eaa911 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -407,14 +407,17 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, opp_table = _managed_opp(opp_np); if (opp_table) { /* OPPs are already managed */ - if (!_add_opp_dev(dev, opp_table)) + if (!_add_opp_dev(dev, opp_table)) { ret = -ENOMEM; - else if (!opp_table->parsed_static_opps) - goto initialize_static_opps; - else + goto put_opp_table; + } + + if (opp_table->parsed_static_opps) { kref_get(&opp_table->list_kref); + return 0; + } - goto put_opp_table; + goto initialize_static_opps; } opp_table = dev_pm_opp_get_opp_table_indexed(dev, index); @@ -432,17 +435,15 @@ initialize_static_opps: if (ret) { dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, ret); - _dev_pm_opp_remove_table(opp_table, dev); of_node_put(np); - goto put_opp_table; + goto put_list_kref; } } /* There should be one of more OPP defined */ if (WARN_ON(!count)) { ret = -ENOENT; - _put_opp_list_kref(opp_table); - goto put_opp_table; + goto put_list_kref; } list_for_each_entry(opp, &opp_table->opp_list, node) @@ -453,8 +454,7 @@ initialize_static_opps: dev_err(dev, "Not all nodes have performance state set (%d: %d)\n", count, pstate_count); ret = -ENOENT; - _dev_pm_opp_remove_table(opp_table, dev); - goto put_opp_table; + goto put_list_kref; } if (pstate_count) @@ -462,6 +462,10 @@ initialize_static_opps: opp_table->parsed_static_opps = true; + return 0; + +put_list_kref: + _put_opp_list_kref(opp_table); put_opp_table: dev_pm_opp_put_opp_table(opp_table); @@ -507,13 +511,13 @@ static int _of_add_opp_table_v1(struct device *dev) if (ret) { dev_err(dev, "%s: Failed to add OPP %ld (%d)\n", __func__, freq, ret); - _dev_pm_opp_remove_table(opp_table, dev); - break; + _put_opp_list_kref(opp_table); + dev_pm_opp_put_opp_table(opp_table); + return ret; } nr -= 2; } - dev_pm_opp_put_opp_table(opp_table); return ret; } diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 98dd7d39e1ad..f9fbb7553fc4 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -190,11 +190,11 @@ struct opp_table { /* Routines internal to opp core */ void dev_pm_opp_get(struct dev_pm_opp *opp); +void _opp_remove_all_static(struct opp_table *opp_table); void _get_opp_table_kref(struct opp_table *opp_table); int _get_opp_count(struct opp_table *opp_table); struct opp_table *_find_opp_table(struct device *dev); struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_table); -void _dev_pm_opp_remove_table(struct opp_table *opp_table, struct device *dev); void _dev_pm_opp_find_and_remove_table(struct device *dev); struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table); void _opp_free(struct dev_pm_opp *opp); -- cgit v1.2.3 From 283d55e68d8a0f302057f57dcbd4d2e000c2ac85 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 7 Sep 2018 09:01:54 +0530 Subject: OPP: Prevent creating multiple OPP tables for devices sharing OPP nodes When two or more devices are sharing their clock and voltage rails, they share the same OPP table. But there are some corner cases where the OPP core incorrectly creates separate OPP tables for them. For example, CPU 0 and 1 share clock/voltage rails. The platform specific code calls dev_pm_opp_set_regulators() for CPU0 and the OPP core creates an OPP table for it (the individual OPPs aren't initialized as of now). The same is repeated for CPU1 then. Because _opp_get_opp_table() doesn't compare DT node pointers currently, it fails to find the link between CPU0 and CPU1 and so creates a new OPP table. Fix this by calling _managed_opp() from _opp_get_opp_table(). _managed_opp() gain an additional argument (index) to get the right node pointer. This resulted in simplifying code in _of_add_opp_table_v2() as well. Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 25 ++++++++++++++++++++++--- drivers/opp/of.c | 35 +++++++++++++---------------------- drivers/opp/opp.h | 2 ++ 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index d3e33fd32694..cdf918aaac34 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -759,8 +759,8 @@ static void _remove_opp_dev(struct opp_device *opp_dev, kfree(opp_dev); } -struct opp_device *_add_opp_dev(const struct device *dev, - struct opp_table *opp_table) +static struct opp_device *_add_opp_dev_unlocked(const struct device *dev, + struct opp_table *opp_table) { struct opp_device *opp_dev; int ret; @@ -772,7 +772,6 @@ struct opp_device *_add_opp_dev(const struct device *dev, /* Initialize opp-dev */ opp_dev->dev = dev; - mutex_lock(&opp_table->lock); list_add(&opp_dev->node, &opp_table->dev_list); /* Create debugfs entries for the opp_table */ @@ -780,6 +779,17 @@ struct opp_device *_add_opp_dev(const struct device *dev, if (ret) dev_err(dev, "%s: Failed to register opp debugfs (%d)\n", __func__, ret); + + return opp_dev; +} + +struct opp_device *_add_opp_dev(const struct device *dev, + struct opp_table *opp_table) +{ + struct opp_device *opp_dev; + + mutex_lock(&opp_table->lock); + opp_dev = _add_opp_dev_unlocked(dev, opp_table); mutex_unlock(&opp_table->lock); return opp_dev; @@ -844,6 +854,15 @@ static struct opp_table *_opp_get_opp_table(struct device *dev, int index) if (!IS_ERR(opp_table)) goto unlock; + opp_table = _managed_opp(dev, index); + if (opp_table) { + if (!_add_opp_dev_unlocked(dev, opp_table)) { + dev_pm_opp_put_opp_table(opp_table); + opp_table = NULL; + } + goto unlock; + } + opp_table = _allocate_opp_table(dev, index); unlock: diff --git a/drivers/opp/of.c b/drivers/opp/of.c index ae0436eaa911..1ddef52c27fd 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -41,11 +41,14 @@ struct device_node *dev_pm_opp_of_get_opp_desc_node(struct device *dev) } EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_opp_desc_node); -static struct opp_table *_managed_opp(const struct device_node *np) +struct opp_table *_managed_opp(struct device *dev, int index) { struct opp_table *opp_table, *managed_table = NULL; + struct device_node *np; - mutex_lock(&opp_table_lock); + np = _opp_of_get_opp_desc_node(dev->of_node, index); + if (!np) + return NULL; list_for_each_entry(opp_table, &opp_tables, node) { if (opp_table->np == np) { @@ -65,7 +68,7 @@ static struct opp_table *_managed_opp(const struct device_node *np) } } - mutex_unlock(&opp_table_lock); + of_node_put(np); return managed_table; } @@ -401,30 +404,19 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, { struct device_node *np; struct opp_table *opp_table; - int ret = 0, count = 0, pstate_count = 0; + int ret, count = 0, pstate_count = 0; struct dev_pm_opp *opp; - opp_table = _managed_opp(opp_np); - if (opp_table) { - /* OPPs are already managed */ - if (!_add_opp_dev(dev, opp_table)) { - ret = -ENOMEM; - goto put_opp_table; - } - - if (opp_table->parsed_static_opps) { - kref_get(&opp_table->list_kref); - return 0; - } - - goto initialize_static_opps; - } - opp_table = dev_pm_opp_get_opp_table_indexed(dev, index); if (!opp_table) return -ENOMEM; -initialize_static_opps: + /* OPP table is already initialized for the device */ + if (opp_table->parsed_static_opps) { + kref_get(&opp_table->list_kref); + return 0; + } + kref_init(&opp_table->list_kref); /* We have opp-table node now, iterate over it and add OPPs */ @@ -466,7 +458,6 @@ initialize_static_opps: put_list_kref: _put_opp_list_kref(opp_table); -put_opp_table: dev_pm_opp_put_opp_table(opp_table); return ret; diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index f9fbb7553fc4..9c6544b4f4f9 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -206,8 +206,10 @@ void _put_opp_list_kref(struct opp_table *opp_table); #ifdef CONFIG_OF void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index); +struct opp_table *_managed_opp(struct device *dev, int index); #else static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {} +static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; } #endif #ifdef CONFIG_DEBUG_FS -- cgit v1.2.3 From 5ed4cecd75e90232a19afa502cf477925854561e Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 12 Sep 2018 11:21:17 +0530 Subject: OPP: Pass OPP table to _of_add_opp_table_v{1|2}() Both _of_add_opp_table_v1() and _of_add_opp_table_v2() contain similar code to get the OPP table and their parent routine also parses the DT to find the OPP table's node pointer. This can be simplified by getting the OPP table in advance and then passing it as argument to these routines. Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 68 +++++++++++++++++++++++--------------------------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 1ddef52c27fd..a71ff3acca0f 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -399,18 +399,12 @@ free_opp: } /* Initializes OPP tables based on new bindings */ -static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, - int index) +static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table) { struct device_node *np; - struct opp_table *opp_table; int ret, count = 0, pstate_count = 0; struct dev_pm_opp *opp; - opp_table = dev_pm_opp_get_opp_table_indexed(dev, index); - if (!opp_table) - return -ENOMEM; - /* OPP table is already initialized for the device */ if (opp_table->parsed_static_opps) { kref_get(&opp_table->list_kref); @@ -420,7 +414,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, kref_init(&opp_table->list_kref); /* We have opp-table node now, iterate over it and add OPPs */ - for_each_available_child_of_node(opp_np, np) { + for_each_available_child_of_node(opp_table->np, np) { count++; ret = _opp_add_static_v2(opp_table, dev, np); @@ -458,15 +452,13 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np, put_list_kref: _put_opp_list_kref(opp_table); - dev_pm_opp_put_opp_table(opp_table); return ret; } /* Initializes OPP tables based on old-deprecated bindings */ -static int _of_add_opp_table_v1(struct device *dev) +static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table) { - struct opp_table *opp_table; const struct property *prop; const __be32 *val; int nr, ret = 0; @@ -487,10 +479,6 @@ static int _of_add_opp_table_v1(struct device *dev) return -EINVAL; } - opp_table = dev_pm_opp_get_opp_table(dev); - if (!opp_table) - return -ENOMEM; - kref_init(&opp_table->list_kref); val = prop->value; @@ -503,7 +491,6 @@ static int _of_add_opp_table_v1(struct device *dev) dev_err(dev, "%s: Failed to add OPP %ld (%d)\n", __func__, freq, ret); _put_opp_list_kref(opp_table); - dev_pm_opp_put_opp_table(opp_table); return ret; } nr -= 2; @@ -531,24 +518,24 @@ static int _of_add_opp_table_v1(struct device *dev) */ int dev_pm_opp_of_add_table(struct device *dev) { - struct device_node *opp_np; + struct opp_table *opp_table; int ret; + opp_table = dev_pm_opp_get_opp_table_indexed(dev, 0); + if (!opp_table) + return -ENOMEM; + /* - * OPPs have two version of bindings now. The older one is deprecated, - * try for the new binding first. + * OPPs have two version of bindings now. Also try the old (v1) + * bindings for backward compatibility with older dtbs. */ - opp_np = dev_pm_opp_of_get_opp_desc_node(dev); - if (!opp_np) { - /* - * Try old-deprecated bindings for backward compatibility with - * older dtbs. - */ - return _of_add_opp_table_v1(dev); - } + if (opp_table->np) + ret = _of_add_opp_table_v2(dev, opp_table); + else + ret = _of_add_opp_table_v1(dev, opp_table); - ret = _of_add_opp_table_v2(dev, opp_np, 0); - of_node_put(opp_np); + if (ret) + dev_pm_opp_put_opp_table(opp_table); return ret; } @@ -575,28 +562,29 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_add_table); */ int dev_pm_opp_of_add_table_indexed(struct device *dev, int index) { - struct device_node *opp_np; + struct opp_table *opp_table; int ret, count; -again: - opp_np = _opp_of_get_opp_desc_node(dev->of_node, index); - if (!opp_np) { + if (index) { /* * If only one phandle is present, then the same OPP table * applies for all index requests. */ count = of_count_phandle_with_args(dev->of_node, "operating-points-v2", NULL); - if (count == 1 && index) { - index = 0; - goto again; - } + if (count != 1) + return -ENODEV; - return -ENODEV; + index = 0; } - ret = _of_add_opp_table_v2(dev, opp_np, index); - of_node_put(opp_np); + opp_table = dev_pm_opp_get_opp_table_indexed(dev, index); + if (!opp_table) + return -ENOMEM; + + ret = _of_add_opp_table_v2(dev, opp_table); + if (ret) + dev_pm_opp_put_opp_table(opp_table); return ret; } -- cgit v1.2.3 From 50b6b87c8484da8bb5dcce00f84ec80aee8fc2bd Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 3 Oct 2018 15:12:06 +0530 Subject: OPP: Improve error handling in dev_pm_opp_of_cpumask_add_table() The error handling wasn't appropriate in dev_pm_opp_of_cpumask_add_table(). For example it returns 0 on success and also for the case where cpumask is empty or cpu_device wasn't found for any of the CPUs. It should really return error on such cases, so that the callers can be aware of the outcome. Fix it. Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index a71ff3acca0f..67a384c8ead2 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -614,16 +614,18 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_remove_table); int dev_pm_opp_of_cpumask_add_table(const struct cpumask *cpumask) { struct device *cpu_dev; - int cpu, ret = 0; + int cpu, ret; - WARN_ON(cpumask_empty(cpumask)); + if (WARN_ON(cpumask_empty(cpumask))) + return -ENODEV; for_each_cpu(cpu, cpumask) { cpu_dev = get_cpu_device(cpu); if (!cpu_dev) { pr_err("%s: failed to get cpu%d device\n", __func__, cpu); - continue; + ret = -ENODEV; + goto remove_table; } ret = dev_pm_opp_of_add_table(cpu_dev); @@ -635,12 +637,16 @@ int dev_pm_opp_of_cpumask_add_table(const struct cpumask *cpumask) pr_debug("%s: couldn't find opp table for cpu:%d, %d\n", __func__, cpu, ret); - /* Free all other OPPs */ - _dev_pm_opp_cpumask_remove_table(cpumask, cpu); - break; + goto remove_table; } } + return 0; + +remove_table: + /* Free all other OPPs */ + _dev_pm_opp_cpumask_remove_table(cpumask, cpu); + return ret; } EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_add_table); -- cgit v1.2.3 From 09f662f95306f3e3d47ab6842bc4b0bb868a80ad Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 3 Oct 2018 15:22:03 +0530 Subject: OPP: Return error on error from dev_pm_opp_get_opp_count() Return error number instead of 0 on failures. Fixes: a1e8c13600bf ("PM / OPP: "opp-hz" is optional for power domains") Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index cdf918aaac34..2c2df4e4fc14 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -318,7 +318,7 @@ int dev_pm_opp_get_opp_count(struct device *dev) count = PTR_ERR(opp_table); dev_dbg(dev, "%s: OPP table not found (%d)\n", __func__, count); - return 0; + return count; } count = _get_opp_count(opp_table); -- cgit v1.2.3 From 51c99dd2c06b234575661fa1e0a1dea6c3ef566f Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 3 Oct 2018 15:35:21 +0530 Subject: cpufreq: dt: Try freeing static OPPs only if we have added them We can not call dev_pm_opp_of_cpumask_remove_table() freely anymore since the latest OPP core updates as that uses reference counting to free resources. There are cases where no static OPPs are added (using DT) for a platform and trying to remove the OPP table may end up decrementing refcount which is already zero and hence generating warnings. Lets track if we were able to add static OPPs or not and then only remove the table based on that. Some reshuffling of code is also done to do that. Reported-by: Niklas Cassel Tested-by: Niklas Cassel Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq-dt.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c index 0a9ebf00be46..e58bfcb1169e 100644 --- a/drivers/cpufreq/cpufreq-dt.c +++ b/drivers/cpufreq/cpufreq-dt.c @@ -32,6 +32,7 @@ struct private_data { struct device *cpu_dev; struct thermal_cooling_device *cdev; const char *reg_name; + bool have_static_opps; }; static struct freq_attr *cpufreq_dt_attr[] = { @@ -204,6 +205,15 @@ static int cpufreq_init(struct cpufreq_policy *policy) } } + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) { + ret = -ENOMEM; + goto out_put_regulator; + } + + priv->reg_name = name; + priv->opp_table = opp_table; + /* * Initialize OPP tables for all policy->cpus. They will be shared by * all CPUs which have marked their CPUs shared with OPP bindings. @@ -214,7 +224,8 @@ static int cpufreq_init(struct cpufreq_policy *policy) * * OPPs might be populated at runtime, don't check for error here */ - dev_pm_opp_of_cpumask_add_table(policy->cpus); + if (!dev_pm_opp_of_cpumask_add_table(policy->cpus)) + priv->have_static_opps = true; /* * But we need OPP table to function so if it is not there let's @@ -240,19 +251,10 @@ static int cpufreq_init(struct cpufreq_policy *policy) __func__, ret); } - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (!priv) { - ret = -ENOMEM; - goto out_free_opp; - } - - priv->reg_name = name; - priv->opp_table = opp_table; - ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); if (ret) { dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret); - goto out_free_priv; + goto out_free_opp; } priv->cpu_dev = cpu_dev; @@ -282,10 +284,11 @@ static int cpufreq_init(struct cpufreq_policy *policy) out_free_cpufreq_table: dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); -out_free_priv: - kfree(priv); out_free_opp: - dev_pm_opp_of_cpumask_remove_table(policy->cpus); + if (priv->have_static_opps) + dev_pm_opp_of_cpumask_remove_table(policy->cpus); + kfree(priv); +out_put_regulator: if (name) dev_pm_opp_put_regulators(opp_table); out_put_clk: @@ -300,7 +303,8 @@ static int cpufreq_exit(struct cpufreq_policy *policy) cpufreq_cooling_unregister(priv->cdev); dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table); - dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); + if (priv->have_static_opps) + dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); if (priv->reg_name) dev_pm_opp_put_regulators(priv->opp_table); -- cgit v1.2.3 From deac8703da5faafce96be3f91c668dcd5da931a5 Mon Sep 17 00:00:00 2001 From: Dave Gerlach Date: Wed, 3 Oct 2018 16:13:15 +0530 Subject: PM / OPP: _of_add_opp_table_v2(): increment count only if OPP is added Currently the _of_add_opp_table_v2 call loops through the OPP nodes in the operating-points-v2 table in the device tree and calls _opp_add_static_v2 for each to add them to the table. It counts each iteration through this loop as an added OPP, however there are cases where _opp_add_static_v2() returns 0 but no new OPP is added to the list. This can happen while adding duplicate OPP or if the OPP isn't supported by hardware. Because of this the count variable will contain the number of OPP nodes in the table in device tree but not necessarily the ones that are actually added. As this count value is what is checked to determine if there are any valid OPPs, if a platform has an operating-points-v2 table with all OPP nodes containing opp-supported-hw values that are not currently supported, then _of_add_opp_table_v2 will fail to abort as it should due to an empty table. Additionally, since commit 3ba98324e81a ("PM / OPP: Get performance state using genpd helper"), the same count variable is compared against the number of OPPs containing performance states and requires that either all or none have pstates set, however in the case of any opp table that has any entries that do not get added by _opp_add_static_v2 due to incompatible opp-supported-hw fields, these numbers will not match and _of_add_opp_table_v2 will incorrectly fail. We need to clearly identify all the three cases (success, failure, unsupported/duplicate OPPs) and then increment count only on success case. Change return type of _opp_add_static_v2() to return the pointer to the newly added OPP instead of an integer. This routine now returns a valid pointer if the OPP is really added, NULL for unsupported or duplicate OPPs, and error value cased as a pointer on errors. Ideally the fixes tag in this commit should point back to the commit that introduced OPP v2 initially, as that's where we started incorrectly accounting for duplicate OPPs: commit 274659029c9d ("PM / OPP: Add support to parse "operating-points-v2" bindings") But it wasn't a real problem until recently as the count was only used to check if any OPPs are added or not. And so this commit points to a rather recent commit where we added more code that depends on the value of "count". Fixes: 3ba98324e81a ("PM / OPP: Get performance state using genpd helper") Reported-by: Dave Gerlach Reported-by: Niklas Cassel Tested-by: Niklas Cassel Signed-off-by: Dave Gerlach Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 67a384c8ead2..5a4b47958073 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -297,15 +297,21 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table); * removed by dev_pm_opp_remove. * * Return: - * 0 On success OR + * Valid OPP pointer: + * On success + * NULL: * Duplicate OPPs (both freq and volt are same) and opp->available - * -EEXIST Freq are same and volt are different OR + * OR if the OPP is not supported by hardware. + * ERR_PTR(-EEXIST): + * Freq are same and volt are different OR * Duplicate OPPs (both freq and volt are same) and !opp->available - * -ENOMEM Memory allocation failure - * -EINVAL Failed parsing the OPP node + * ERR_PTR(-ENOMEM): + * Memory allocation failure + * ERR_PTR(-EINVAL): + * Failed parsing the OPP node */ -static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev, - struct device_node *np) +static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, + struct device *dev, struct device_node *np) { struct dev_pm_opp *new_opp; u64 rate = 0; @@ -315,7 +321,7 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev, new_opp = _opp_allocate(opp_table); if (!new_opp) - return -ENOMEM; + return ERR_PTR(-ENOMEM); ret = of_property_read_u64(np, "opp-hz", &rate); if (ret < 0) { @@ -390,12 +396,12 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev, * frequency/voltage list. */ blocking_notifier_call_chain(&opp_table->head, OPP_EVENT_ADD, new_opp); - return 0; + return new_opp; free_opp: _opp_free(new_opp); - return ret; + return ERR_PTR(ret); } /* Initializes OPP tables based on new bindings */ @@ -415,14 +421,15 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table) /* We have opp-table node now, iterate over it and add OPPs */ for_each_available_child_of_node(opp_table->np, np) { - count++; - - ret = _opp_add_static_v2(opp_table, dev, np); - if (ret) { + opp = _opp_add_static_v2(opp_table, dev, np); + if (IS_ERR(opp)) { + ret = PTR_ERR(opp); dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, ret); of_node_put(np); goto put_list_kref; + } else if (opp) { + count++; } } -- cgit v1.2.3