[PATCH 0/6] OPP: Scope based cleanup helpers and lock guards

[PATCH 0/6] OPP: Scope based cleanup helpers and lock guards

[Viresh Kumar]

Hello,

This series performs various cleanups / optimizations for the OPP core.
- Introduce and use scope based cleanup headers
- Use mutex locking guards

No intentional functional impact.

--
Viresh

Viresh Kumar (6):
  OPP: Remove _get_opp_table_kref()
  OPP: Return opp from dev_pm_opp_get()
  OPP: Return opp_table from dev_pm_opp_get_opp_table_ref()
  OPP: Use scope-based OF cleanup helpers
  OPP: Define and use scope-based cleanup helpers
  OPP: Use mutex locking guards

 drivers/opp/core.c     | 426 ++++++++++++++---------------------------
 drivers/opp/cpu.c      |  30 +--
 drivers/opp/of.c       | 205 +++++++-------------
 drivers/opp/opp.h      |   1 -
 include/linux/pm_opp.h |  21 +-
 5 files changed, 249 insertions(+), 434 deletions(-)

-- 
2.31.1.272.g89b43f80a514

[PATCH 1/6] OPP: Remove _get_opp_table_kref()

[Viresh Kumar]

Use dev_pm_opp_get_opp_table_ref() directly instead.

No intentional functional impact.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 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);
-- 
2.31.1.272.g89b43f80a514

[PATCH 2/6] OPP: Return opp from dev_pm_opp_get()

[Viresh Kumar]

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 <viresh.kumar@linaro.org>
---
 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) {}
 
-- 
2.31.1.272.g89b43f80a514

[PATCH 3/6] OPP: Return opp_table from dev_pm_opp_get_opp_table_ref()

[Viresh Kumar]

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 <viresh.kumar@linaro.org>
---
 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) {}
 
-- 
2.31.1.272.g89b43f80a514

[PATCH 4/6] OPP: Use scope-based OF cleanup helpers

[Viresh Kumar]

Use the OF scope-based cleanup helpers for the OPP core.

No intentional functional impact.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 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;
-- 
2.31.1.272.g89b43f80a514

[PATCH 5/6] OPP: Define and use scope-based cleanup helpers

[Viresh Kumar]

Define and use scope-based cleanup helpers for `struct opp` and `struct
opp_table`.

No intentional functional impact.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/core.c     | 236 ++++++++++++++---------------------------
 drivers/opp/cpu.c      |  27 ++---
 drivers/opp/of.c       |  24 ++---
 include/linux/pm_opp.h |   7 ++
 4 files changed, 102 insertions(+), 192 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 14fb0f43cc77..87d27132cd87 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;
-			break;
-		}
-	}
+			opp = dev_pm_opp_get(tmp_opp);
 
-	if (IS_ERR(opp)) {
-		r = PTR_ERR(opp);
-		goto adjust_unlock;
-	}
+			/* Is update really needed? */
+			if (opp->supplies->u_volt == u_volt) {
+				mutex_unlock(&opp_table->lock);
+				return 0;
+			}
 
-	/* 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;
 
-	opp->supplies->u_volt = u_volt;
-	opp->supplies->u_volt_min = u_volt_min;
-	opp->supplies->u_volt_max = u_volt_max;
+			break;
+		}
+	}
 
-	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 i;
 
 	/* Device may not have OPP table */
 	opp_table = _find_opp_table(dev);
@@ -3001,23 +2941,18 @@ 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 regulator_sync_voltage(reg);
 	}
-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 +3004,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 +3024,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 +3043,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 +3064,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..e7b5c602c92f 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 <linux/cleanup.h>
 #include <linux/energy_model.h>
 #include <linux/err.h>
 #include <linux/notifier.h>
@@ -710,4 +711,10 @@ static inline unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp)
 	return dev_pm_opp_get_freq_indexed(opp, 0);
 }
 
+/* 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))
+
 #endif		/* __LINUX_OPP_H__ */
-- 
2.31.1.272.g89b43f80a514

[PATCH 6/6] OPP: Use mutex locking guards

[Viresh Kumar]

Use mutex locking guard in the OPP core.

No intentional functional impact.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 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 87d27132cd87..fc9874946453 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);
-
-	for (i = 0; i < count; i++) {
-		uV[i].min = ~0;
-		uV[i].max = 0;
+	scoped_guard(mutex, &opp_table->lock) {
+		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);
-
-	if (!opp_table->parsed_static_opps) {
-		mutex_unlock(&opp_table->lock);
-		return false;
-	}
+	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 true;
+		if (--opp_table->parsed_static_opps)
+			return true;
 	}
 
-	mutex_unlock(&opp_table->lock);
-
 	_opp_remove_all(opp_table, false);
 	return true;
 }
@@ -2003,17 +1977,15 @@ 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);
-	mutex_unlock(&opp_table->lock);
+		list_add(&new_opp->node, head);
+	}
 
 	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);
-
-	/* 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);
+	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);
 
-			/* Is update really needed? */
-			if (opp->supplies->u_volt == u_volt) {
-				mutex_unlock(&opp_table->lock);
-				return 0;
-			}
+				/* Is update really needed? */
+				if (opp->supplies->u_volt == u_volt)
+					return 0;
 
-			opp->supplies->u_volt = u_volt;
-			opp->supplies->u_volt_min = u_volt_min;
-			opp->supplies->u_volt_max = u_volt_max;
+				opp->supplies->u_volt = u_volt;
+				opp->supplies->u_volt_min = u_volt_min;
+				opp->supplies->u_volt_max = u_volt_max;
 
-			break;
+				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) {
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH 6/6] OPP: Use mutex locking guards

[David Lechner]

On 4/24/25 5:36 AM, Viresh Kumar wrote:
> Use mutex locking guard in the OPP core.
> 
> No intentional functional impact.

There is an unintentional functional change here.

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

...

> @@ -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;
>  		}
>  	}

scoped_guard() is implemented as a for loop. So now this break statement
breaks out out the scoped_guard() and not out of the outer for loop. Now
the outer loop always iterates to completion.

Assuming each item in src_table->required_opp_tables is unique, this doesn't
look like a bug. Rather it is just doing unnecessary extra iterations.

To preserve the logic, the break should be moved out of the scoped_guard().