From 4cb1383f9522a3d2f5e26b70688417187c7d48e4 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 24 Apr 2025 14:21:40 +0530 Subject: [PATCH 1/8] OPP: Remove _get_opp_table_kref() Use dev_pm_opp_get_opp_table_ref() directly instead. No intentional functional impact. Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 9 ++------- drivers/opp/of.c | 6 +++--- drivers/opp/opp.h | 1 - 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 73e9a3b2f29b..e63a9b009df1 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -59,7 +59,7 @@ static struct opp_table *_find_opp_table_unlocked(struct device *dev) list_for_each_entry(opp_table, &opp_tables, node) { if (_find_opp_dev(dev, opp_table)) { - _get_opp_table_kref(opp_table); + dev_pm_opp_get_opp_table_ref(opp_table); return opp_table; } } @@ -1688,14 +1688,9 @@ static void _opp_table_kref_release(struct kref *kref) kfree(opp_table); } -void _get_opp_table_kref(struct opp_table *opp_table) -{ - kref_get(&opp_table->kref); -} - void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table) { - _get_opp_table_kref(opp_table); + kref_get(&opp_table->kref); } EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_table_ref); diff --git a/drivers/opp/of.c b/drivers/opp/of.c index a24f76f5fd01..87cb6aeb49ed 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -61,7 +61,7 @@ struct opp_table *_managed_opp(struct device *dev, int index) * OPP table contains a "opp-shared" property. */ if (opp_table->shared_opp == OPP_TABLE_ACCESS_SHARED) { - _get_opp_table_kref(opp_table); + dev_pm_opp_get_opp_table_ref(opp_table); managed_table = opp_table; } @@ -117,7 +117,7 @@ static struct opp_table *_find_table_of_opp_np(struct device_node *opp_np) mutex_lock(&opp_table_lock); list_for_each_entry(opp_table, &opp_tables, node) { if (opp_table_np == opp_table->np) { - _get_opp_table_kref(opp_table); + dev_pm_opp_get_opp_table_ref(opp_table); mutex_unlock(&opp_table_lock); return opp_table; } @@ -406,7 +406,7 @@ static void lazy_link_required_opp_table(struct opp_table *new_table) } required_opp_tables[i] = new_table; - _get_opp_table_kref(new_table); + dev_pm_opp_get_opp_table_ref(new_table); /* Link OPPs now */ ret = lazy_link_required_opps(opp_table, new_table, i); diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 5c7c81190e41..9eba63e01a9e 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -251,7 +251,6 @@ struct opp_table { /* Routines internal to opp core */ bool _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); From 7a3be00771aa9786c7bb4cdb0ee36fee45f67d69 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 23 Apr 2025 15:48:40 +0530 Subject: [PATCH 2/8] OPP: Return opp from dev_pm_opp_get() For convenience of users, return back the pointer to the opp from dev_pm_opp_get(), so they can do: opp = dev_pm_opp_get(tmp_opp); No intentional functional impact. Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 13 ++++++------- include/linux/pm_opp.h | 7 +++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index e63a9b009df1..150439a18b87 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1188,8 +1188,8 @@ static void _find_current_opp(struct device *dev, struct opp_table *opp_table) */ if (IS_ERR(opp)) { mutex_lock(&opp_table->lock); - opp = list_first_entry(&opp_table->opp_list, struct dev_pm_opp, node); - dev_pm_opp_get(opp); + opp = dev_pm_opp_get(list_first_entry(&opp_table->opp_list, + struct dev_pm_opp, node)); mutex_unlock(&opp_table->lock); } @@ -1329,8 +1329,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, dev_pm_opp_put(old_opp); /* Make sure current_opp doesn't get freed */ - dev_pm_opp_get(opp); - opp_table->current_opp = opp; + opp_table->current_opp = dev_pm_opp_get(opp); return ret; } @@ -1724,9 +1723,10 @@ static void _opp_kref_release(struct kref *kref) kfree(opp); } -void dev_pm_opp_get(struct dev_pm_opp *opp) +struct dev_pm_opp *dev_pm_opp_get(struct dev_pm_opp *opp) { kref_get(&opp->kref); + return opp; } EXPORT_SYMBOL_GPL(dev_pm_opp_get); @@ -2706,8 +2706,7 @@ struct dev_pm_opp *dev_pm_opp_xlate_required_opp(struct opp_table *src_table, list_for_each_entry(opp, &src_table->opp_list, node) { if (opp == src_opp) { - dest_opp = opp->required_opps[i]; - dev_pm_opp_get(dest_opp); + dest_opp = dev_pm_opp_get(opp->required_opps[i]); break; } } diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index c247317aae38..5e4c3428b139 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -161,7 +161,7 @@ struct dev_pm_opp *dev_pm_opp_find_bw_ceil(struct device *dev, struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev, unsigned int *bw, int index); -void dev_pm_opp_get(struct dev_pm_opp *opp); +struct dev_pm_opp *dev_pm_opp_get(struct dev_pm_opp *opp); void dev_pm_opp_put(struct dev_pm_opp *opp); int dev_pm_opp_add_dynamic(struct device *dev, struct dev_pm_opp_data *opp); @@ -345,7 +345,10 @@ static inline struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev, return ERR_PTR(-EOPNOTSUPP); } -static inline void dev_pm_opp_get(struct dev_pm_opp *opp) {} +static inline struct dev_pm_opp *dev_pm_opp_get(struct dev_pm_opp *opp) +{ + return opp; +} static inline void dev_pm_opp_put(struct dev_pm_opp *opp) {} From ead694941686345bfd3f95100d889191cb9e3cda Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 23 Apr 2025 15:48:40 +0530 Subject: [PATCH 3/8] OPP: Return opp_table from dev_pm_opp_get_opp_table_ref() For convenience of users, return back the pointer to the opp_table from dev_pm_opp_get_opp_table_ref(), so they can do: opp_table = dev_pm_opp_get_opp_table_ref(tmp_table); No intentional functional impact. Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 9 ++++----- drivers/opp/of.c | 9 +++------ include/linux/pm_opp.h | 7 +++++-- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 150439a18b87..14fb0f43cc77 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -58,10 +58,8 @@ static struct opp_table *_find_opp_table_unlocked(struct device *dev) struct opp_table *opp_table; list_for_each_entry(opp_table, &opp_tables, node) { - if (_find_opp_dev(dev, opp_table)) { - dev_pm_opp_get_opp_table_ref(opp_table); - return opp_table; - } + if (_find_opp_dev(dev, opp_table)) + return dev_pm_opp_get_opp_table_ref(opp_table); } return ERR_PTR(-ENODEV); @@ -1687,9 +1685,10 @@ static void _opp_table_kref_release(struct kref *kref) kfree(opp_table); } -void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table) +struct opp_table *dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table) { kref_get(&opp_table->kref); + return opp_table; } EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_table_ref); diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 87cb6aeb49ed..c240acc81a8d 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -60,10 +60,8 @@ struct opp_table *_managed_opp(struct device *dev, int index) * But the OPPs will be considered as shared only if the * OPP table contains a "opp-shared" property. */ - if (opp_table->shared_opp == OPP_TABLE_ACCESS_SHARED) { - dev_pm_opp_get_opp_table_ref(opp_table); - managed_table = opp_table; - } + if (opp_table->shared_opp == OPP_TABLE_ACCESS_SHARED) + managed_table = dev_pm_opp_get_opp_table_ref(opp_table); break; } @@ -405,8 +403,7 @@ static void lazy_link_required_opp_table(struct opp_table *new_table) continue; } - required_opp_tables[i] = new_table; - dev_pm_opp_get_opp_table_ref(new_table); + required_opp_tables[i] = dev_pm_opp_get_opp_table_ref(new_table); /* Link OPPs now */ ret = lazy_link_required_opps(opp_table, new_table, i); diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 5e4c3428b139..0deddfa91aca 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -100,7 +100,7 @@ struct dev_pm_opp_data { #if defined(CONFIG_PM_OPP) struct opp_table *dev_pm_opp_get_opp_table(struct device *dev); -void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table); +struct opp_table *dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table); void dev_pm_opp_put_opp_table(struct opp_table *opp_table); unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, bool peak, int index); @@ -207,7 +207,10 @@ static inline struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device * return ERR_PTR(-EOPNOTSUPP); } -static inline void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table) {} +static inline struct opp_table *dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table) +{ + return opp_table; +} static inline void dev_pm_opp_put_opp_table(struct opp_table *opp_table) {} From f95c855306fe47e809ca7ad6b4f330535f8b34fb Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 23 Apr 2025 15:51:06 +0530 Subject: [PATCH 4/8] OPP: Use scope-based OF cleanup helpers Use the OF scope-based cleanup helpers for the OPP core. No intentional functional impact. Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 111 +++++++++++++++++------------------------------ 1 file changed, 40 insertions(+), 71 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index c240acc81a8d..aa43fbfa3e50 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -45,7 +45,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_opp_desc_node); struct opp_table *_managed_opp(struct device *dev, int index) { struct opp_table *opp_table, *managed_table = NULL; - struct device_node *np; + struct device_node *np __free(device_node); np = _opp_of_get_opp_desc_node(dev->of_node, index); if (!np) @@ -67,8 +67,6 @@ struct opp_table *_managed_opp(struct device *dev, int index) } } - of_node_put(np); - return managed_table; } @@ -102,16 +100,13 @@ static struct device_node *of_parse_required_opp(struct device_node *np, /* The caller must call dev_pm_opp_put_opp_table() after the table is used */ static struct opp_table *_find_table_of_opp_np(struct device_node *opp_np) { + struct device_node *opp_table_np __free(device_node); struct opp_table *opp_table; - struct device_node *opp_table_np; opp_table_np = of_get_parent(opp_np); if (!opp_table_np) goto err; - /* It is safe to put the node now as all we need now is its address */ - of_node_put(opp_table_np); - mutex_lock(&opp_table_lock); list_for_each_entry(opp_table, &opp_tables, node) { if (opp_table_np == opp_table->np) { @@ -161,7 +156,7 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table, struct device_node *opp_np) { struct opp_table **required_opp_tables; - struct device_node *required_np, *np; + struct device_node *np __free(device_node); bool lazy = false; int count, i, size; @@ -169,30 +164,32 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table, np = of_get_next_available_child(opp_np, NULL); if (!np) { dev_warn(dev, "Empty OPP table\n"); - return; } count = of_count_phandle_with_args(np, "required-opps", NULL); if (count <= 0) - goto put_np; + return; size = sizeof(*required_opp_tables) + sizeof(*opp_table->required_devs); required_opp_tables = kcalloc(count, size, GFP_KERNEL); if (!required_opp_tables) - goto put_np; + return; opp_table->required_opp_tables = required_opp_tables; opp_table->required_devs = (void *)(required_opp_tables + count); opp_table->required_opp_count = count; for (i = 0; i < count; i++) { + struct device_node *required_np __free(device_node); + required_np = of_parse_required_opp(np, i); - if (!required_np) - goto free_required_tables; + if (!required_np) { + _opp_table_free_required_tables(opp_table); + return; + } required_opp_tables[i] = _find_table_of_opp_np(required_np); - of_node_put(required_np); if (IS_ERR(required_opp_tables[i])) lazy = true; @@ -208,19 +205,12 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table, list_add(&opp_table->lazy, &lazy_opp_tables); mutex_unlock(&opp_table_lock); } - - goto put_np; - -free_required_tables: - _opp_table_free_required_tables(opp_table); -put_np: - of_node_put(np); } void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) { - struct device_node *np, *opp_np; + struct device_node *np __free(device_node), *opp_np; u32 val; /* @@ -241,8 +231,6 @@ void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, /* Get OPP table node */ opp_np = _opp_of_get_opp_desc_node(np, index); - of_node_put(np); - if (!opp_np) return; @@ -296,15 +284,13 @@ void _of_clear_opp(struct opp_table *opp_table, struct dev_pm_opp *opp) static int _link_required_opps(struct dev_pm_opp *opp, struct opp_table *required_table, int index) { - struct device_node *np; + struct device_node *np __free(device_node); np = of_parse_required_opp(opp->np, index); if (unlikely(!np)) return -ENODEV; opp->required_opps[index] = _find_opp_of_np(required_table, np); - of_node_put(np); - if (!opp->required_opps[index]) { pr_err("%s: Unable to find required OPP node: %pOF (%d)\n", __func__, opp->np, index); @@ -368,19 +354,22 @@ static int lazy_link_required_opps(struct opp_table *opp_table, static void lazy_link_required_opp_table(struct opp_table *new_table) { struct opp_table *opp_table, *temp, **required_opp_tables; - struct device_node *required_np, *opp_np, *required_table_np; struct dev_pm_opp *opp; int i, ret; mutex_lock(&opp_table_lock); list_for_each_entry_safe(opp_table, temp, &lazy_opp_tables, lazy) { + struct device_node *opp_np __free(device_node); bool lazy = false; /* opp_np can't be invalid here */ opp_np = of_get_next_available_child(opp_table->np, NULL); for (i = 0; i < opp_table->required_opp_count; i++) { + struct device_node *required_np __free(device_node) = NULL; + struct device_node *required_table_np __free(device_node) = NULL; + required_opp_tables = opp_table->required_opp_tables; /* Required opp-table is already parsed */ @@ -391,9 +380,6 @@ static void lazy_link_required_opp_table(struct opp_table *new_table) required_np = of_parse_required_opp(opp_np, i); required_table_np = of_get_parent(required_np); - of_node_put(required_table_np); - of_node_put(required_np); - /* * Newly added table isn't the required opp-table for * opp_table. @@ -414,8 +400,6 @@ static void lazy_link_required_opp_table(struct opp_table *new_table) } } - of_node_put(opp_np); - /* All required opp-tables found, remove from lazy list */ if (!lazy) { list_del_init(&opp_table->lazy); @@ -430,16 +414,18 @@ static void lazy_link_required_opp_table(struct opp_table *new_table) static int _bandwidth_supported(struct device *dev, struct opp_table *opp_table) { - struct device_node *np, *opp_np; + struct device_node *opp_np __free(device_node) = NULL; + struct device_node *np __free(device_node) = NULL; struct property *prop; if (!opp_table) { + struct device_node *np __free(device_node); + np = of_node_get(dev->of_node); if (!np) return -ENODEV; opp_np = _opp_of_get_opp_desc_node(np, 0); - of_node_put(np); } else { opp_np = of_node_get(opp_table->np); } @@ -450,15 +436,12 @@ static int _bandwidth_supported(struct device *dev, struct opp_table *opp_table) /* Checking only first OPP is sufficient */ np = of_get_next_available_child(opp_np, NULL); - of_node_put(opp_np); if (!np) { dev_err(dev, "OPP table empty\n"); return -EINVAL; } prop = of_find_property(np, "opp-peak-kBps", NULL); - of_node_put(np); - if (!prop || !prop->length) return 0; @@ -468,7 +451,7 @@ static int _bandwidth_supported(struct device *dev, struct opp_table *opp_table) int dev_pm_opp_of_find_icc_paths(struct device *dev, struct opp_table *opp_table) { - struct device_node *np; + struct device_node *np __free(device_node) = of_node_get(dev->of_node); int ret, i, count, num_paths; struct icc_path **paths; @@ -478,15 +461,13 @@ int dev_pm_opp_of_find_icc_paths(struct device *dev, else if (ret <= 0) return ret; - ret = 0; - - np = of_node_get(dev->of_node); if (!np) return 0; + ret = 0; + count = of_count_phandle_with_args(np, "interconnects", "#interconnect-cells"); - of_node_put(np); if (count < 0) return 0; @@ -1303,8 +1284,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_cpumask_add_table); int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) { - struct device_node *np, *tmp_np, *cpu_np; - int cpu, ret = 0; + struct device_node *np __free(device_node); + int cpu; /* Get OPP descriptor node */ np = dev_pm_opp_of_get_opp_desc_node(cpu_dev); @@ -1317,9 +1298,12 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, /* OPPs are shared ? */ if (!of_property_read_bool(np, "opp-shared")) - goto put_cpu_node; + return 0; for_each_possible_cpu(cpu) { + struct device_node *cpu_np __free(device_node) = NULL; + struct device_node *tmp_np __free(device_node) = NULL; + if (cpu == cpu_dev->id) continue; @@ -1327,29 +1311,22 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, if (!cpu_np) { dev_err(cpu_dev, "%s: failed to get cpu%d node\n", __func__, cpu); - ret = -ENOENT; - goto put_cpu_node; + return -ENOENT; } /* Get OPP descriptor node */ tmp_np = _opp_of_get_opp_desc_node(cpu_np, 0); - of_node_put(cpu_np); if (!tmp_np) { pr_err("%pOF: Couldn't find opp node\n", cpu_np); - ret = -ENOENT; - goto put_cpu_node; + return -ENOENT; } /* CPUs are sharing opp node */ if (np == tmp_np) cpumask_set_cpu(cpu, cpumask); - - of_node_put(tmp_np); } -put_cpu_node: - of_node_put(np); - return ret; + return 0; } EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_sharing_cpus); @@ -1366,9 +1343,9 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_sharing_cpus); */ int of_get_required_opp_performance_state(struct device_node *np, int index) { - struct dev_pm_opp *opp; - struct device_node *required_np; + struct device_node *required_np __free(device_node); struct opp_table *opp_table; + struct dev_pm_opp *opp; int pstate = -EINVAL; required_np = of_parse_required_opp(np, index); @@ -1379,13 +1356,13 @@ int of_get_required_opp_performance_state(struct device_node *np, int index) if (IS_ERR(opp_table)) { pr_err("%s: Failed to find required OPP table %pOF: %ld\n", __func__, np, PTR_ERR(opp_table)); - goto put_required_np; + return PTR_ERR(opp_table); } /* The OPP tables must belong to a genpd */ if (unlikely(!opp_table->is_genpd)) { pr_err("%s: Performance state is only valid for genpds.\n", __func__); - goto put_required_np; + return -EINVAL; } opp = _find_opp_of_np(opp_table, required_np); @@ -1401,10 +1378,6 @@ int of_get_required_opp_performance_state(struct device_node *np, int index) } dev_pm_opp_put_opp_table(opp_table); - -put_required_np: - of_node_put(required_np); - return pstate; } EXPORT_SYMBOL_GPL(of_get_required_opp_performance_state); @@ -1421,7 +1394,7 @@ EXPORT_SYMBOL_GPL(of_get_required_opp_performance_state); */ bool dev_pm_opp_of_has_required_opp(struct device *dev) { - struct device_node *opp_np, *np; + struct device_node *np __free(device_node) = NULL, *opp_np __free(device_node); int count; opp_np = _opp_of_get_opp_desc_node(dev->of_node, 0); @@ -1429,14 +1402,12 @@ bool dev_pm_opp_of_has_required_opp(struct device *dev) return false; np = of_get_next_available_child(opp_np, NULL); - of_node_put(opp_np); if (!np) { dev_warn(dev, "Empty OPP table\n"); return false; } count = of_count_phandle_with_args(np, "required-opps", NULL); - of_node_put(np); return count > 0; } @@ -1513,8 +1484,8 @@ _get_dt_power(struct device *dev, unsigned long *uW, unsigned long *kHz) int dev_pm_opp_calc_power(struct device *dev, unsigned long *uW, unsigned long *kHz) { + struct device_node *np __free(device_node); struct dev_pm_opp *opp; - struct device_node *np; unsigned long mV, Hz; u32 cap; u64 tmp; @@ -1525,7 +1496,6 @@ int dev_pm_opp_calc_power(struct device *dev, unsigned long *uW, return -EINVAL; ret = of_property_read_u32(np, "dynamic-power-coefficient", &cap); - of_node_put(np); if (ret) return -EINVAL; @@ -1581,8 +1551,8 @@ static bool _of_has_opp_microwatt_property(struct device *dev) */ int dev_pm_opp_of_register_em(struct device *dev, struct cpumask *cpus) { + struct device_node *np __free(device_node) = NULL; struct em_data_callback em_cb; - struct device_node *np; int ret, nr_opp; u32 cap; @@ -1617,7 +1587,6 @@ int dev_pm_opp_of_register_em(struct device *dev, struct cpumask *cpus) * user about the inconsistent configuration. */ ret = of_property_read_u32(np, "dynamic-power-coefficient", &cap); - of_node_put(np); if (ret || !cap) { dev_dbg(dev, "Couldn't find proper 'dynamic-power-coefficient' in DT\n"); ret = -EINVAL; From 8c5d8c0b9e8184a86baede72ca392f90d867d22e Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 23 Apr 2025 14:21:28 +0530 Subject: [PATCH 5/8] OPP: Define and use scope-based cleanup helpers Define and use scope-based cleanup helpers for `struct opp` and `struct opp_table`. No intentional functional impact. Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 234 ++++++++++++++--------------------------- drivers/opp/cpu.c | 27 ++--- drivers/opp/of.c | 24 ++--- include/linux/pm_opp.h | 7 ++ 4 files changed, 102 insertions(+), 190 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 14fb0f43cc77..31d5b749afc7 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -317,18 +317,13 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_is_turbo); */ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev) { - struct opp_table *opp_table; - unsigned long clock_latency_ns; + struct opp_table *opp_table __free(put_opp_table); opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) return 0; - clock_latency_ns = opp_table->clock_latency_ns_max; - - dev_pm_opp_put_opp_table(opp_table); - - return clock_latency_ns; + return opp_table->clock_latency_ns_max; } EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency); @@ -340,7 +335,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency); */ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) { - struct opp_table *opp_table; + struct opp_table *opp_table __free(put_opp_table); struct dev_pm_opp *opp; struct regulator *reg; unsigned long latency_ns = 0; @@ -356,13 +351,13 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) /* Regulator may not be required for the device */ if (!opp_table->regulators) - goto put_opp_table; + return 0; count = opp_table->regulator_count; uV = kmalloc_array(count, sizeof(*uV), GFP_KERNEL); if (!uV) - goto put_opp_table; + return 0; mutex_lock(&opp_table->lock); @@ -395,8 +390,6 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) } kfree(uV); -put_opp_table: - dev_pm_opp_put_opp_table(opp_table); return latency_ns; } @@ -426,7 +419,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_transition_latency); */ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) { - struct opp_table *opp_table; + struct opp_table *opp_table __free(put_opp_table); unsigned long freq = 0; opp_table = _find_opp_table(dev); @@ -436,8 +429,6 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) if (opp_table->suspend_opp && opp_table->suspend_opp->available) freq = dev_pm_opp_get_freq(opp_table->suspend_opp); - dev_pm_opp_put_opp_table(opp_table); - return freq; } EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq); @@ -468,21 +459,16 @@ int _get_opp_count(struct opp_table *opp_table) */ int dev_pm_opp_get_opp_count(struct device *dev) { - struct opp_table *opp_table; - int count; + struct opp_table *opp_table __free(put_opp_table); opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { - count = PTR_ERR(opp_table); - dev_dbg(dev, "%s: OPP table not found (%d)\n", - __func__, count); - return count; + dev_dbg(dev, "%s: OPP table not found (%ld)\n", + __func__, PTR_ERR(opp_table)); + return PTR_ERR(opp_table); } - count = _get_opp_count(opp_table); - dev_pm_opp_put_opp_table(opp_table); - - return count; + return _get_opp_count(opp_table); } EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count); @@ -576,8 +562,7 @@ _find_key(struct device *dev, unsigned long *key, int index, bool available, unsigned long opp_key, unsigned long key), bool (*assert)(struct opp_table *opp_table, unsigned int index)) { - struct opp_table *opp_table; - struct dev_pm_opp *opp; + struct opp_table *opp_table __free(put_opp_table); opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { @@ -586,12 +571,8 @@ _find_key(struct device *dev, unsigned long *key, int index, bool available, return ERR_CAST(opp_table); } - opp = _opp_table_find_key(opp_table, key, index, available, read, - compare, assert); - - dev_pm_opp_put_opp_table(opp_table); - - return opp; + return _opp_table_find_key(opp_table, key, index, available, read, + compare, assert); } static struct dev_pm_opp *_find_key_exact(struct device *dev, @@ -1345,11 +1326,10 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, */ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) { - struct opp_table *opp_table; + struct opp_table *opp_table __free(put_opp_table); + struct dev_pm_opp *opp __free(put_opp) = NULL; unsigned long freq = 0, temp_freq; - struct dev_pm_opp *opp = NULL; bool forced = false; - int ret; opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { @@ -1366,9 +1346,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) * equivalent to a clk_set_rate() */ if (!_get_opp_count(opp_table)) { - ret = opp_table->config_clks(dev, opp_table, NULL, - &target_freq, false); - goto put_opp_table; + return opp_table->config_clks(dev, opp_table, NULL, + &target_freq, false); } freq = clk_round_rate(opp_table->clk, target_freq); @@ -1383,10 +1362,9 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) temp_freq = freq; opp = _find_freq_ceil(opp_table, &temp_freq); if (IS_ERR(opp)) { - ret = PTR_ERR(opp); - dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n", - __func__, freq, ret); - goto put_opp_table; + dev_err(dev, "%s: failed to find OPP for freq %lu (%ld)\n", + __func__, freq, PTR_ERR(opp)); + return PTR_ERR(opp); } /* @@ -1399,14 +1377,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) forced = opp_table->current_rate_single_clk != freq; } - ret = _set_opp(dev, opp_table, opp, &freq, forced); - - if (freq) - dev_pm_opp_put(opp); - -put_opp_table: - dev_pm_opp_put_opp_table(opp_table); - return ret; + return _set_opp(dev, opp_table, opp, &freq, forced); } EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate); @@ -1422,8 +1393,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate); */ int dev_pm_opp_set_opp(struct device *dev, struct dev_pm_opp *opp) { - struct opp_table *opp_table; - int ret; + struct opp_table *opp_table __free(put_opp_table); opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { @@ -1431,10 +1401,7 @@ int dev_pm_opp_set_opp(struct device *dev, struct dev_pm_opp *opp) return PTR_ERR(opp_table); } - ret = _set_opp(dev, opp_table, opp, NULL, false); - dev_pm_opp_put_opp_table(opp_table); - - return ret; + return _set_opp(dev, opp_table, opp, NULL, false); } EXPORT_SYMBOL_GPL(dev_pm_opp_set_opp); @@ -1744,15 +1711,15 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_put); */ void dev_pm_opp_remove(struct device *dev, unsigned long freq) { + struct opp_table *opp_table __free(put_opp_table); struct dev_pm_opp *opp = NULL, *iter; - struct opp_table *opp_table; opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) return; if (!assert_single_clk(opp_table, 0)) - goto put_table; + return; mutex_lock(&opp_table->lock); @@ -1774,10 +1741,6 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) dev_warn(dev, "%s: Couldn't find OPP with freq: %lu\n", __func__, freq); } - -put_table: - /* Drop the reference taken by _find_opp_table() */ - dev_pm_opp_put_opp_table(opp_table); } EXPORT_SYMBOL_GPL(dev_pm_opp_remove); @@ -1849,16 +1812,13 @@ bool _opp_remove_all_static(struct opp_table *opp_table) */ void dev_pm_opp_remove_all_dynamic(struct device *dev) { - struct opp_table *opp_table; + struct opp_table *opp_table __free(put_opp_table); opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) return; _opp_remove_all(opp_table, true); - - /* Drop the reference taken by _find_opp_table() */ - dev_pm_opp_put_opp_table(opp_table); } EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic); @@ -2846,47 +2806,43 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_add_dynamic); static int _opp_set_availability(struct device *dev, unsigned long freq, bool availability_req) { - struct opp_table *opp_table; - struct dev_pm_opp *tmp_opp, *opp = ERR_PTR(-ENODEV); - int r = 0; + struct dev_pm_opp *opp __free(put_opp) = ERR_PTR(-ENODEV), *tmp_opp; + struct opp_table *opp_table __free(put_opp_table); /* Find the opp_table */ opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { - r = PTR_ERR(opp_table); - dev_warn(dev, "%s: Device OPP not found (%d)\n", __func__, r); - return r; + dev_warn(dev, "%s: Device OPP not found (%ld)\n", __func__, + PTR_ERR(opp_table)); + return PTR_ERR(opp_table); } - if (!assert_single_clk(opp_table, 0)) { - r = -EINVAL; - goto put_table; - } + if (!assert_single_clk(opp_table, 0)) + return -EINVAL; mutex_lock(&opp_table->lock); /* Do we have the frequency? */ list_for_each_entry(tmp_opp, &opp_table->opp_list, node) { if (tmp_opp->rates[0] == freq) { - opp = tmp_opp; + opp = dev_pm_opp_get(tmp_opp); + + /* Is update really needed? */ + if (opp->available == availability_req) { + mutex_unlock(&opp_table->lock); + return 0; + } + + opp->available = availability_req; break; } } - if (IS_ERR(opp)) { - r = PTR_ERR(opp); - goto unlock; - } - - /* Is update really needed? */ - if (opp->available == availability_req) - goto unlock; - - opp->available = availability_req; - - dev_pm_opp_get(opp); mutex_unlock(&opp_table->lock); + if (IS_ERR(opp)) + return PTR_ERR(opp); + /* Notify the change of the OPP availability */ if (availability_req) blocking_notifier_call_chain(&opp_table->head, OPP_EVENT_ENABLE, @@ -2895,14 +2851,7 @@ static int _opp_set_availability(struct device *dev, unsigned long freq, blocking_notifier_call_chain(&opp_table->head, OPP_EVENT_DISABLE, opp); - dev_pm_opp_put(opp); - goto put_table; - -unlock: - mutex_unlock(&opp_table->lock); -put_table: - dev_pm_opp_put_opp_table(opp_table); - return r; + return 0; } /** @@ -2922,9 +2871,9 @@ int dev_pm_opp_adjust_voltage(struct device *dev, unsigned long freq, unsigned long u_volt_max) { - struct opp_table *opp_table; - struct dev_pm_opp *tmp_opp, *opp = ERR_PTR(-ENODEV); - int r = 0; + struct dev_pm_opp *opp __free(put_opp) = ERR_PTR(-ENODEV), *tmp_opp; + struct opp_table *opp_table __free(put_opp_table); + int r; /* Find the opp_table */ opp_table = _find_opp_table(dev); @@ -2934,49 +2883,40 @@ int dev_pm_opp_adjust_voltage(struct device *dev, unsigned long freq, return r; } - if (!assert_single_clk(opp_table, 0)) { - r = -EINVAL; - goto put_table; - } + if (!assert_single_clk(opp_table, 0)) + return -EINVAL; mutex_lock(&opp_table->lock); /* Do we have the frequency? */ list_for_each_entry(tmp_opp, &opp_table->opp_list, node) { if (tmp_opp->rates[0] == freq) { - opp = tmp_opp; + opp = dev_pm_opp_get(tmp_opp); + + /* Is update really needed? */ + if (opp->supplies->u_volt == u_volt) { + mutex_unlock(&opp_table->lock); + return 0; + } + + opp->supplies->u_volt = u_volt; + opp->supplies->u_volt_min = u_volt_min; + opp->supplies->u_volt_max = u_volt_max; + break; } } - if (IS_ERR(opp)) { - r = PTR_ERR(opp); - goto adjust_unlock; - } - - /* Is update really needed? */ - if (opp->supplies->u_volt == u_volt) - goto adjust_unlock; - - opp->supplies->u_volt = u_volt; - opp->supplies->u_volt_min = u_volt_min; - opp->supplies->u_volt_max = u_volt_max; - - dev_pm_opp_get(opp); mutex_unlock(&opp_table->lock); + if (IS_ERR(opp)) + return PTR_ERR(opp); + /* Notify the voltage change of the OPP */ blocking_notifier_call_chain(&opp_table->head, OPP_EVENT_ADJUST_VOLTAGE, opp); - dev_pm_opp_put(opp); - goto put_table; - -adjust_unlock: - mutex_unlock(&opp_table->lock); -put_table: - dev_pm_opp_put_opp_table(opp_table); - return r; + return 0; } EXPORT_SYMBOL_GPL(dev_pm_opp_adjust_voltage); @@ -2990,9 +2930,9 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_adjust_voltage); */ int dev_pm_opp_sync_regulators(struct device *dev) { - struct opp_table *opp_table; + struct opp_table *opp_table __free(put_opp_table); struct regulator *reg; - int i, ret = 0; + int ret, i; /* Device may not have OPP table */ opp_table = _find_opp_table(dev); @@ -3001,23 +2941,20 @@ int dev_pm_opp_sync_regulators(struct device *dev) /* Regulator may not be required for the device */ if (unlikely(!opp_table->regulators)) - goto put_table; + return 0; /* Nothing to sync if voltage wasn't changed */ if (!opp_table->enabled) - goto put_table; + return 0; for (i = 0; i < opp_table->regulator_count; i++) { reg = opp_table->regulators[i]; ret = regulator_sync_voltage(reg); if (ret) - break; + return ret; } -put_table: - /* Drop reference taken by _find_opp_table() */ - dev_pm_opp_put_opp_table(opp_table); - return ret; + return 0; } EXPORT_SYMBOL_GPL(dev_pm_opp_sync_regulators); @@ -3069,18 +3006,13 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_disable); */ int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb) { - struct opp_table *opp_table; - int ret; + struct opp_table *opp_table __free(put_opp_table); opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) return PTR_ERR(opp_table); - ret = blocking_notifier_chain_register(&opp_table->head, nb); - - dev_pm_opp_put_opp_table(opp_table); - - return ret; + return blocking_notifier_chain_register(&opp_table->head, nb); } EXPORT_SYMBOL(dev_pm_opp_register_notifier); @@ -3094,18 +3026,13 @@ EXPORT_SYMBOL(dev_pm_opp_register_notifier); int dev_pm_opp_unregister_notifier(struct device *dev, struct notifier_block *nb) { - struct opp_table *opp_table; - int ret; + struct opp_table *opp_table __free(put_opp_table); opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) return PTR_ERR(opp_table); - ret = blocking_notifier_chain_unregister(&opp_table->head, nb); - - dev_pm_opp_put_opp_table(opp_table); - - return ret; + return blocking_notifier_chain_unregister(&opp_table->head, nb); } EXPORT_SYMBOL(dev_pm_opp_unregister_notifier); @@ -3118,7 +3045,7 @@ EXPORT_SYMBOL(dev_pm_opp_unregister_notifier); */ void dev_pm_opp_remove_table(struct device *dev) { - struct opp_table *opp_table; + struct opp_table *opp_table __free(put_opp_table); /* Check for existing table for 'dev' */ opp_table = _find_opp_table(dev); @@ -3139,8 +3066,5 @@ void dev_pm_opp_remove_table(struct device *dev) **/ if (_opp_remove_all_static(opp_table)) dev_pm_opp_put_opp_table(opp_table); - - /* Drop reference taken by _find_opp_table() */ - dev_pm_opp_put_opp_table(opp_table); } EXPORT_SYMBOL_GPL(dev_pm_opp_remove_table); diff --git a/drivers/opp/cpu.c b/drivers/opp/cpu.c index 12c429b407ca..330a1753fb22 100644 --- a/drivers/opp/cpu.c +++ b/drivers/opp/cpu.c @@ -43,7 +43,6 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev, struct cpufreq_frequency_table **opp_table) { - struct dev_pm_opp *opp; struct cpufreq_frequency_table *freq_table = NULL; int i, max_opps, ret = 0; unsigned long rate; @@ -57,6 +56,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev, return -ENOMEM; for (i = 0, rate = 0; i < max_opps; i++, rate++) { + struct dev_pm_opp *opp __free(put_opp); + /* find next rate */ opp = dev_pm_opp_find_freq_ceil(dev, &rate); if (IS_ERR(opp)) { @@ -69,8 +70,6 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev, /* Is Boost/turbo opp ? */ if (dev_pm_opp_is_turbo(opp)) freq_table[i].flags = CPUFREQ_BOOST_FREQ; - - dev_pm_opp_put(opp); } freq_table[i].driver_data = i; @@ -155,10 +154,10 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_cpumask_remove_table); int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask) { + struct opp_table *opp_table __free(put_opp_table); struct opp_device *opp_dev; - struct opp_table *opp_table; struct device *dev; - int cpu, ret = 0; + int cpu; opp_table = _find_opp_table(cpu_dev); if (IS_ERR(opp_table)) @@ -186,9 +185,7 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, opp_table->shared_opp = OPP_TABLE_ACCESS_SHARED; } - dev_pm_opp_put_opp_table(opp_table); - - return ret; + return 0; } EXPORT_SYMBOL_GPL(dev_pm_opp_set_sharing_cpus); @@ -204,18 +201,15 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_set_sharing_cpus); */ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) { + struct opp_table *opp_table __free(put_opp_table); struct opp_device *opp_dev; - struct opp_table *opp_table; - int ret = 0; opp_table = _find_opp_table(cpu_dev); if (IS_ERR(opp_table)) return PTR_ERR(opp_table); - if (opp_table->shared_opp == OPP_TABLE_ACCESS_UNKNOWN) { - ret = -EINVAL; - goto put_opp_table; - } + if (opp_table->shared_opp == OPP_TABLE_ACCESS_UNKNOWN) + return -EINVAL; cpumask_clear(cpumask); @@ -228,9 +222,6 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) cpumask_set_cpu(cpu_dev->id, cpumask); } -put_opp_table: - dev_pm_opp_put_opp_table(opp_table); - - return ret; + return 0; } EXPORT_SYMBOL_GPL(dev_pm_opp_get_sharing_cpus); diff --git a/drivers/opp/of.c b/drivers/opp/of.c index aa43fbfa3e50..54109e813d4f 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -1344,8 +1344,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_get_sharing_cpus); int of_get_required_opp_performance_state(struct device_node *np, int index) { struct device_node *required_np __free(device_node); - struct opp_table *opp_table; - struct dev_pm_opp *opp; + struct opp_table *opp_table __free(put_opp_table) = NULL; + struct dev_pm_opp *opp __free(put_opp) = NULL; int pstate = -EINVAL; required_np = of_parse_required_opp(np, index); @@ -1373,11 +1373,8 @@ int of_get_required_opp_performance_state(struct device_node *np, int index) } else { pstate = opp->level; } - dev_pm_opp_put(opp); - } - dev_pm_opp_put_opp_table(opp_table); return pstate; } EXPORT_SYMBOL_GPL(of_get_required_opp_performance_state); @@ -1443,7 +1440,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_of_node); static int __maybe_unused _get_dt_power(struct device *dev, unsigned long *uW, unsigned long *kHz) { - struct dev_pm_opp *opp; + struct dev_pm_opp *opp __free(put_opp); unsigned long opp_freq, opp_power; /* Find the right frequency and related OPP */ @@ -1453,7 +1450,6 @@ _get_dt_power(struct device *dev, unsigned long *uW, unsigned long *kHz) return -EINVAL; opp_power = dev_pm_opp_get_power(opp); - dev_pm_opp_put(opp); if (!opp_power) return -EINVAL; @@ -1484,8 +1480,8 @@ _get_dt_power(struct device *dev, unsigned long *uW, unsigned long *kHz) int dev_pm_opp_calc_power(struct device *dev, unsigned long *uW, unsigned long *kHz) { + struct dev_pm_opp *opp __free(put_opp) = NULL; struct device_node *np __free(device_node); - struct dev_pm_opp *opp; unsigned long mV, Hz; u32 cap; u64 tmp; @@ -1505,7 +1501,6 @@ int dev_pm_opp_calc_power(struct device *dev, unsigned long *uW, return -EINVAL; mV = dev_pm_opp_get_voltage(opp) / 1000; - dev_pm_opp_put(opp); if (!mV) return -EINVAL; @@ -1522,20 +1517,15 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_calc_power); static bool _of_has_opp_microwatt_property(struct device *dev) { - unsigned long power, freq = 0; - struct dev_pm_opp *opp; + struct dev_pm_opp *opp __free(put_opp); + unsigned long freq = 0; /* Check if at least one OPP has needed property */ opp = dev_pm_opp_find_freq_ceil(dev, &freq); if (IS_ERR(opp)) return false; - power = dev_pm_opp_get_power(opp); - dev_pm_opp_put(opp); - if (!power) - return false; - - return true; + return !!dev_pm_opp_get_power(opp); } /** diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 0deddfa91aca..8313ed981535 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -11,6 +11,7 @@ #ifndef __LINUX_OPP_H__ #define __LINUX_OPP_H__ +#include #include #include #include @@ -579,6 +580,12 @@ static inline int dev_pm_opp_of_find_icc_paths(struct device *dev, struct opp_ta } #endif +/* Scope based cleanup macro for OPP reference counting */ +DEFINE_FREE(put_opp, struct dev_pm_opp *, if (!IS_ERR_OR_NULL(_T)) dev_pm_opp_put(_T)) + +/* Scope based cleanup macro for OPP table reference counting */ +DEFINE_FREE(put_opp_table, struct opp_table *, if (!IS_ERR_OR_NULL(_T)) dev_pm_opp_put_opp_table(_T)) + /* OPP Configuration helpers */ static inline int dev_pm_opp_add(struct device *dev, unsigned long freq, From ff9c512041f2b405536640374ae3a3fe10efaf8b Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 24 Apr 2025 14:15:53 +0530 Subject: [PATCH 6/8] OPP: Use mutex locking guards Use mutex locking guard in the OPP core. No intentional functional impact. Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 201 ++++++++++++++++++--------------------------- drivers/opp/cpu.c | 3 +- drivers/opp/of.c | 65 ++++++--------- 3 files changed, 105 insertions(+), 164 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 31d5b749afc7..72fbb6cadc23 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -40,17 +40,14 @@ static DEFINE_XARRAY_ALLOC1(opp_configs); static bool _find_opp_dev(const struct device *dev, struct opp_table *opp_table) { struct opp_device *opp_dev; - bool found = false; - mutex_lock(&opp_table->lock); + guard(mutex)(&opp_table->lock); + list_for_each_entry(opp_dev, &opp_table->dev_list, node) - if (opp_dev->dev == dev) { - found = true; - break; - } + if (opp_dev->dev == dev) + return true; - mutex_unlock(&opp_table->lock); - return found; + return false; } static struct opp_table *_find_opp_table_unlocked(struct device *dev) @@ -78,18 +75,13 @@ static struct opp_table *_find_opp_table_unlocked(struct device *dev) */ struct opp_table *_find_opp_table(struct device *dev) { - struct opp_table *opp_table; - if (IS_ERR_OR_NULL(dev)) { pr_err("%s: Invalid parameters\n", __func__); return ERR_PTR(-EINVAL); } - mutex_lock(&opp_table_lock); - opp_table = _find_opp_table_unlocked(dev); - mutex_unlock(&opp_table_lock); - - return opp_table; + guard(mutex)(&opp_table_lock); + return _find_opp_table_unlocked(dev); } /* @@ -359,25 +351,23 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) if (!uV) return 0; - mutex_lock(&opp_table->lock); + scoped_guard(mutex, &opp_table->lock) { + for (i = 0; i < count; i++) { + uV[i].min = ~0; + uV[i].max = 0; - for (i = 0; i < count; i++) { - uV[i].min = ~0; - uV[i].max = 0; + list_for_each_entry(opp, &opp_table->opp_list, node) { + if (!opp->available) + continue; - list_for_each_entry(opp, &opp_table->opp_list, node) { - if (!opp->available) - continue; - - if (opp->supplies[i].u_volt_min < uV[i].min) - uV[i].min = opp->supplies[i].u_volt_min; - if (opp->supplies[i].u_volt_max > uV[i].max) - uV[i].max = opp->supplies[i].u_volt_max; + if (opp->supplies[i].u_volt_min < uV[i].min) + uV[i].min = opp->supplies[i].u_volt_min; + if (opp->supplies[i].u_volt_max > uV[i].max) + uV[i].max = opp->supplies[i].u_volt_max; + } } } - mutex_unlock(&opp_table->lock); - /* * The caller needs to ensure that opp_table (and hence the regulator) * isn't freed, while we are executing this routine. @@ -438,15 +428,13 @@ int _get_opp_count(struct opp_table *opp_table) struct dev_pm_opp *opp; int count = 0; - mutex_lock(&opp_table->lock); + guard(mutex)(&opp_table->lock); list_for_each_entry(opp, &opp_table->opp_list, node) { if (opp->available) count++; } - mutex_unlock(&opp_table->lock); - return count; } @@ -535,7 +523,7 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table, if (assert && !assert(opp_table, index)) return ERR_PTR(-EINVAL); - mutex_lock(&opp_table->lock); + guard(mutex)(&opp_table->lock); list_for_each_entry(temp_opp, &opp_table->opp_list, node) { if (temp_opp->available == available) { @@ -550,8 +538,6 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table, dev_pm_opp_get(opp); } - mutex_unlock(&opp_table->lock); - return opp; } @@ -1166,10 +1152,9 @@ static void _find_current_opp(struct device *dev, struct opp_table *opp_table) * make special checks to validate current_opp. */ if (IS_ERR(opp)) { - mutex_lock(&opp_table->lock); + guard(mutex)(&opp_table->lock); opp = dev_pm_opp_get(list_first_entry(&opp_table->opp_list, struct dev_pm_opp, node)); - mutex_unlock(&opp_table->lock); } opp_table->current_opp = opp; @@ -1426,9 +1411,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); - mutex_unlock(&opp_table->lock); + scoped_guard(mutex, &opp_table->lock) + list_add(&opp_dev->node, &opp_table->dev_list); /* Create debugfs entries for the opp_table */ opp_debug_register(opp_dev, opp_table); @@ -1721,17 +1705,15 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) if (!assert_single_clk(opp_table, 0)) return; - mutex_lock(&opp_table->lock); - - list_for_each_entry(iter, &opp_table->opp_list, node) { - if (iter->rates[0] == freq) { - opp = iter; - break; + scoped_guard(mutex, &opp_table->lock) { + list_for_each_entry(iter, &opp_table->opp_list, node) { + if (iter->rates[0] == freq) { + opp = iter; + break; + } } } - mutex_unlock(&opp_table->lock); - if (opp) { dev_pm_opp_put(opp); @@ -1747,22 +1729,20 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove); static struct dev_pm_opp *_opp_get_next(struct opp_table *opp_table, bool dynamic) { - struct dev_pm_opp *opp = NULL, *temp; + struct dev_pm_opp *opp; - mutex_lock(&opp_table->lock); - list_for_each_entry(temp, &opp_table->opp_list, node) { + guard(mutex)(&opp_table->lock); + + list_for_each_entry(opp, &opp_table->opp_list, node) { /* * Refcount must be dropped only once for each OPP by OPP core, * do that with help of "removed" flag. */ - if (!temp->removed && dynamic == temp->dynamic) { - opp = temp; - break; - } + if (!opp->removed && dynamic == opp->dynamic) + return opp; } - mutex_unlock(&opp_table->lock); - return opp; + return NULL; } /* @@ -1786,20 +1766,14 @@ static void _opp_remove_all(struct opp_table *opp_table, bool dynamic) bool _opp_remove_all_static(struct opp_table *opp_table) { - mutex_lock(&opp_table->lock); + scoped_guard(mutex, &opp_table->lock) { + if (!opp_table->parsed_static_opps) + return false; - if (!opp_table->parsed_static_opps) { - mutex_unlock(&opp_table->lock); - return false; + if (--opp_table->parsed_static_opps) + return true; } - if (--opp_table->parsed_static_opps) { - mutex_unlock(&opp_table->lock); - return true; - } - - mutex_unlock(&opp_table->lock); - _opp_remove_all(opp_table, false); return true; } @@ -2003,18 +1977,16 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct list_head *head; int ret; - mutex_lock(&opp_table->lock); - head = &opp_table->opp_list; + scoped_guard(mutex, &opp_table->lock) { + head = &opp_table->opp_list; - ret = _opp_is_duplicate(dev, new_opp, opp_table, &head); - if (ret) { - mutex_unlock(&opp_table->lock); - return ret; + ret = _opp_is_duplicate(dev, new_opp, opp_table, &head); + if (ret) + return ret; + + list_add(&new_opp->node, head); } - list_add(&new_opp->node, head); - mutex_unlock(&opp_table->lock); - new_opp->opp_table = opp_table; kref_init(&new_opp->kref); @@ -2660,17 +2632,16 @@ struct dev_pm_opp *dev_pm_opp_xlate_required_opp(struct opp_table *src_table, return ERR_PTR(-EBUSY); for (i = 0; i < src_table->required_opp_count; i++) { - if (src_table->required_opp_tables[i] == dst_table) { - mutex_lock(&src_table->lock); + if (src_table->required_opp_tables[i] != dst_table) + continue; + scoped_guard(mutex, &src_table->lock) { list_for_each_entry(opp, &src_table->opp_list, node) { if (opp == src_opp) { dest_opp = dev_pm_opp_get(opp->required_opps[i]); break; } } - - mutex_unlock(&src_table->lock); break; } } @@ -2702,7 +2673,6 @@ int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, unsigned int pstate) { struct dev_pm_opp *opp; - int dest_pstate = -EINVAL; int i; /* @@ -2736,22 +2706,17 @@ int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, return -EINVAL; } - mutex_lock(&src_table->lock); + guard(mutex)(&src_table->lock); list_for_each_entry(opp, &src_table->opp_list, node) { - if (opp->level == pstate) { - dest_pstate = opp->required_opps[i]->level; - goto unlock; - } + if (opp->level == pstate) + return opp->required_opps[i]->level; } pr_err("%s: Couldn't find matching OPP (%p: %p)\n", __func__, src_table, dst_table); -unlock: - mutex_unlock(&src_table->lock); - - return dest_pstate; + return -EINVAL; } /** @@ -2820,26 +2785,22 @@ static int _opp_set_availability(struct device *dev, unsigned long freq, if (!assert_single_clk(opp_table, 0)) return -EINVAL; - mutex_lock(&opp_table->lock); + scoped_guard(mutex, &opp_table->lock) { + /* Do we have the frequency? */ + list_for_each_entry(tmp_opp, &opp_table->opp_list, node) { + if (tmp_opp->rates[0] == freq) { + opp = dev_pm_opp_get(tmp_opp); - /* Do we have the frequency? */ - list_for_each_entry(tmp_opp, &opp_table->opp_list, node) { - if (tmp_opp->rates[0] == freq) { - opp = dev_pm_opp_get(tmp_opp); + /* Is update really needed? */ + if (opp->available == availability_req) + return 0; - /* Is update really needed? */ - if (opp->available == availability_req) { - mutex_unlock(&opp_table->lock); - return 0; + opp->available = availability_req; + break; } - - opp->available = availability_req; - break; } } - mutex_unlock(&opp_table->lock); - if (IS_ERR(opp)) return PTR_ERR(opp); @@ -2886,29 +2847,25 @@ int dev_pm_opp_adjust_voltage(struct device *dev, unsigned long freq, if (!assert_single_clk(opp_table, 0)) return -EINVAL; - mutex_lock(&opp_table->lock); + scoped_guard(mutex, &opp_table->lock) { + /* Do we have the frequency? */ + list_for_each_entry(tmp_opp, &opp_table->opp_list, node) { + if (tmp_opp->rates[0] == freq) { + opp = dev_pm_opp_get(tmp_opp); - /* Do we have the frequency? */ - list_for_each_entry(tmp_opp, &opp_table->opp_list, node) { - if (tmp_opp->rates[0] == freq) { - opp = dev_pm_opp_get(tmp_opp); + /* Is update really needed? */ + if (opp->supplies->u_volt == u_volt) + return 0; - /* Is update really needed? */ - if (opp->supplies->u_volt == u_volt) { - mutex_unlock(&opp_table->lock); - return 0; + opp->supplies->u_volt = u_volt; + opp->supplies->u_volt_min = u_volt_min; + opp->supplies->u_volt_max = u_volt_max; + + break; } - - opp->supplies->u_volt = u_volt; - opp->supplies->u_volt_min = u_volt_min; - opp->supplies->u_volt_max = u_volt_max; - - break; } } - mutex_unlock(&opp_table->lock); - if (IS_ERR(opp)) return PTR_ERR(opp); diff --git a/drivers/opp/cpu.c b/drivers/opp/cpu.c index 330a1753fb22..97989d4fe336 100644 --- a/drivers/opp/cpu.c +++ b/drivers/opp/cpu.c @@ -214,10 +214,9 @@ 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); + guard(mutex)(&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/of.c b/drivers/opp/of.c index 54109e813d4f..505d79821584 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -76,18 +76,13 @@ static struct dev_pm_opp *_find_opp_of_np(struct opp_table *opp_table, { struct dev_pm_opp *opp; - mutex_lock(&opp_table->lock); + guard(mutex)(&opp_table->lock); list_for_each_entry(opp, &opp_table->opp_list, node) { - if (opp->np == opp_np) { - dev_pm_opp_get(opp); - mutex_unlock(&opp_table->lock); - return opp; - } + if (opp->np == opp_np) + return dev_pm_opp_get(opp); } - mutex_unlock(&opp_table->lock); - return NULL; } @@ -105,19 +100,15 @@ static struct opp_table *_find_table_of_opp_np(struct device_node *opp_np) opp_table_np = of_get_parent(opp_np); if (!opp_table_np) - goto err; + return ERR_PTR(-ENODEV); + + guard(mutex)(&opp_table_lock); - mutex_lock(&opp_table_lock); list_for_each_entry(opp_table, &opp_tables, node) { - if (opp_table_np == opp_table->np) { - dev_pm_opp_get_opp_table_ref(opp_table); - mutex_unlock(&opp_table_lock); - return opp_table; - } + if (opp_table_np == opp_table->np) + return dev_pm_opp_get_opp_table_ref(opp_table); } - mutex_unlock(&opp_table_lock); -err: return ERR_PTR(-ENODEV); } @@ -142,9 +133,8 @@ static void _opp_table_free_required_tables(struct opp_table *opp_table) opp_table->required_opp_count = 0; opp_table->required_opp_tables = NULL; - mutex_lock(&opp_table_lock); + guard(mutex)(&opp_table_lock); list_del(&opp_table->lazy); - mutex_unlock(&opp_table_lock); } /* @@ -201,9 +191,8 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table, * The OPP table is not held while allocating the table, take it * now to avoid corruption to the lazy_opp_tables list. */ - mutex_lock(&opp_table_lock); + guard(mutex)(&opp_table_lock); list_add(&opp_table->lazy, &lazy_opp_tables); - mutex_unlock(&opp_table_lock); } } @@ -357,7 +346,7 @@ static void lazy_link_required_opp_table(struct opp_table *new_table) struct dev_pm_opp *opp; int i, ret; - mutex_lock(&opp_table_lock); + guard(mutex)(&opp_table_lock); list_for_each_entry_safe(opp_table, temp, &lazy_opp_tables, lazy) { struct device_node *opp_np __free(device_node); @@ -408,8 +397,6 @@ static void lazy_link_required_opp_table(struct opp_table *new_table) _required_opps_available(opp, opp_table->required_opp_count); } } - - mutex_unlock(&opp_table_lock); } static int _bandwidth_supported(struct device *dev, struct opp_table *opp_table) @@ -970,15 +957,14 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table) struct dev_pm_opp *opp; /* OPP table is already initialized for the device */ - mutex_lock(&opp_table->lock); - if (opp_table->parsed_static_opps) { - opp_table->parsed_static_opps++; - mutex_unlock(&opp_table->lock); - return 0; - } + scoped_guard(mutex, &opp_table->lock) { + if (opp_table->parsed_static_opps) { + opp_table->parsed_static_opps++; + return 0; + } - opp_table->parsed_static_opps = 1; - mutex_unlock(&opp_table->lock); + opp_table->parsed_static_opps = 1; + } /* We have opp-table node now, iterate over it and add OPPs */ for_each_available_child_of_node(opp_table->np, np) { @@ -1018,15 +1004,14 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table) const __be32 *val; int nr, ret = 0; - mutex_lock(&opp_table->lock); - if (opp_table->parsed_static_opps) { - opp_table->parsed_static_opps++; - mutex_unlock(&opp_table->lock); - return 0; - } + scoped_guard(mutex, &opp_table->lock) { + if (opp_table->parsed_static_opps) { + opp_table->parsed_static_opps++; + return 0; + } - opp_table->parsed_static_opps = 1; - mutex_unlock(&opp_table->lock); + opp_table->parsed_static_opps = 1; + } prop = of_find_property(dev->of_node, "operating-points", NULL); if (!prop) { From ee3de3cf7035aaf289085111fd0cfaddc93f0409 Mon Sep 17 00:00:00 2001 From: Praveen Talari Date: Fri, 2 May 2025 10:58:22 +0530 Subject: [PATCH 7/8] OPP: Add dev_pm_opp_set_level() To configure a device to a specific performance level, consumer drivers currently need to determine the OPP based on the exact level and then set it, resulting in code duplication across drivers. The new helper API, dev_pm_opp_set_level(), addresses this issue by providing a streamlined method for consumer drivers to find and set the OPP based on the desired performance level, thereby eliminating redundancy. Signed-off-by: Praveen Talari [ Viresh: Lot of fixes in the code, and rebased over latest changes. Fixed commit log too. ] Signed-off-by: Viresh Kumar --- include/linux/pm_opp.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 8313ed981535..cf477beae4bb 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -197,6 +197,7 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) void dev_pm_opp_remove_table(struct device *dev); void dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask); int dev_pm_opp_sync_regulators(struct device *dev); + #else static inline struct opp_table *dev_pm_opp_get_opp_table(struct device *dev) { @@ -717,4 +718,14 @@ static inline unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp) return dev_pm_opp_get_freq_indexed(opp, 0); } +static inline int dev_pm_opp_set_level(struct device *dev, unsigned int level) +{ + struct dev_pm_opp *opp __free(put_opp) = dev_pm_opp_find_level_exact(dev, level); + + if (IS_ERR(opp)) + return PTR_ERR(opp); + + return dev_pm_opp_set_opp(dev, opp); +} + #endif /* __LINUX_OPP_H__ */ From 03eadcbd981b4c9b10ec89a046deeccb115c98b3 Mon Sep 17 00:00:00 2001 From: Zhang Enpei Date: Thu, 15 May 2025 20:13:12 +0800 Subject: [PATCH 8/8] OPP: switch to use kmemdup_array() Use kmemdup_array() to avoid multiplication and possible overflows. Signed-off-by: Zhang Enpei Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 72fbb6cadc23..edbd60501cf0 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -2087,8 +2087,8 @@ static int _opp_set_supported_hw(struct opp_table *opp_table, if (opp_table->supported_hw) return 0; - opp_table->supported_hw = kmemdup(versions, count * sizeof(*versions), - GFP_KERNEL); + opp_table->supported_hw = kmemdup_array(versions, count, + sizeof(*versions), GFP_KERNEL); if (!opp_table->supported_hw) return -ENOMEM;