All of lore.kernel.org
 help / color / mirror / Atom feed
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
	Stephen Boyd <sboyd@kernel.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
	linux-pm@vger.kernel.org,
	Vincent Guittot <vincent.guittot@linaro.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 4/5] OPP: Remove genpd_virt_dev_lock
Date: Fri, 13 Oct 2023 14:18:40 +0530	[thread overview]
Message-ID: <afec94cb6515e79c3b336f74be8aa929a303b26f.1697186772.git.viresh.kumar@linaro.org> (raw)
In-Reply-To: <cover.1697186772.git.viresh.kumar@linaro.org>

All the config operations for OPP tables share common code paths now and
none of the other ones have such protection in place. Either all should
have it or none.

The understanding here is that user won't clear the OPP configs while
still using them and so such a case won't happen. We can always come
back and use a wider lock for all resource types if required.

Also fix the error on failing to allocate memory.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/core.c | 33 +++------------------------------
 drivers/opp/opp.h  |  2 --
 2 files changed, 3 insertions(+), 32 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 3516e79cf743..befe46036ad5 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1089,12 +1089,6 @@ static int _opp_set_required_opps_genpd(struct device *dev,
 		delta = -1;
 	}
 
-	/*
-	 * Acquire genpd_virt_dev_lock to make sure we don't use a genpd_dev
-	 * after it is freed from another thread.
-	 */
-	mutex_lock(&opp_table->genpd_virt_dev_lock);
-
 	while (index != target) {
 		ret = _set_performance_state(dev, genpd_virt_devs[index], opp, index);
 		if (ret)
@@ -1103,8 +1097,6 @@ static int _opp_set_required_opps_genpd(struct device *dev,
 		index += delta;
 	}
 
-	mutex_unlock(&opp_table->genpd_virt_dev_lock);
-
 	return 0;
 }
 
@@ -1474,7 +1466,6 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
 		return ERR_PTR(-ENOMEM);
 
 	mutex_init(&opp_table->lock);
-	mutex_init(&opp_table->genpd_virt_dev_lock);
 	INIT_LIST_HEAD(&opp_table->dev_list);
 	INIT_LIST_HEAD(&opp_table->lazy);
 
@@ -1510,7 +1501,6 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
 remove_opp_dev:
 	_of_clear_opp_table(opp_table);
 	_remove_opp_dev(opp_dev, opp_table);
-	mutex_destroy(&opp_table->genpd_virt_dev_lock);
 	mutex_destroy(&opp_table->lock);
 err:
 	kfree(opp_table);
@@ -1678,7 +1668,6 @@ static void _opp_table_kref_release(struct kref *kref)
 	list_for_each_entry_safe(opp_dev, temp, &opp_table->dev_list, node)
 		_remove_opp_dev(opp_dev, opp_table);
 
-	mutex_destroy(&opp_table->genpd_virt_dev_lock);
 	mutex_destroy(&opp_table->lock);
 	kfree(opp_table);
 }
@@ -2395,7 +2384,7 @@ static void _opp_put_config_regulators_helper(struct opp_table *opp_table)
 		opp_table->config_regulators = NULL;
 }
 
-static void _detach_genpd(struct opp_table *opp_table)
+static void _opp_detach_genpd(struct opp_table *opp_table)
 {
 	int index;
 
@@ -2449,13 +2438,11 @@ static int _opp_attach_genpd(struct opp_table *opp_table, struct device *dev,
 	if (!opp_table->required_opp_count)
 		return -EPROBE_DEFER;
 
-	mutex_lock(&opp_table->genpd_virt_dev_lock);
-
 	opp_table->genpd_virt_devs = kcalloc(opp_table->required_opp_count,
 					     sizeof(*opp_table->genpd_virt_devs),
 					     GFP_KERNEL);
 	if (!opp_table->genpd_virt_devs)
-		goto unlock;
+		return -ENOMEM;
 
 	while (*name) {
 		if (index >= opp_table->required_opp_count) {
@@ -2478,29 +2465,15 @@ static int _opp_attach_genpd(struct opp_table *opp_table, struct device *dev,
 
 	if (virt_devs)
 		*virt_devs = opp_table->genpd_virt_devs;
-	mutex_unlock(&opp_table->genpd_virt_dev_lock);
 
 	return 0;
 
 err:
-	_detach_genpd(opp_table);
-unlock:
-	mutex_unlock(&opp_table->genpd_virt_dev_lock);
+	_opp_detach_genpd(opp_table);
 	return ret;
 
 }
 
-static void _opp_detach_genpd(struct opp_table *opp_table)
-{
-	/*
-	 * Acquire genpd_virt_dev_lock to make sure virt_dev isn't getting
-	 * used in parallel.
-	 */
-	mutex_lock(&opp_table->genpd_virt_dev_lock);
-	_detach_genpd(opp_table);
-	mutex_unlock(&opp_table->genpd_virt_dev_lock);
-}
-
 static void _opp_clear_config(struct opp_config_data *data)
 {
 	if (data->flags & OPP_CONFIG_GENPD)
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index fefdf9845692..08366f90f16b 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -160,7 +160,6 @@ enum opp_table_access {
  * @rate_clk_single: Currently configured frequency for single clk.
  * @current_opp: Currently configured OPP for the table.
  * @suspend_opp: Pointer to OPP to be used during device suspend.
- * @genpd_virt_dev_lock: Mutex protecting the genpd virtual device pointers.
  * @genpd_virt_devs: List of virtual devices for multiple genpd support.
  * @required_opp_tables: List of device OPP tables that are required by OPPs in
  *		this table.
@@ -212,7 +211,6 @@ struct opp_table {
 	struct dev_pm_opp *current_opp;
 	struct dev_pm_opp *suspend_opp;
 
-	struct mutex genpd_virt_dev_lock;
 	struct device **genpd_virt_devs;
 	struct opp_table **required_opp_tables;
 	unsigned int required_opp_count;
-- 
2.31.1.272.g89b43f80a514


  parent reply	other threads:[~2023-10-13  8:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-13  8:48 [PATCH 0/5] OPP: Minor cleanups Viresh Kumar
2023-10-13  8:48 ` [PATCH 1/5] OPP: Fix formatting of if/else block Viresh Kumar
2023-10-16 10:11   ` Ulf Hansson
2023-10-13  8:48 ` [PATCH 2/5] OPP: Add _link_required_opps() to avoid code duplication Viresh Kumar
2023-10-16 10:11   ` Ulf Hansson
2023-10-13  8:48 ` [PATCH 3/5] OPP: Reorder code in _opp_set_required_opps_genpd() Viresh Kumar
2023-10-16 10:11   ` Ulf Hansson
2023-10-16 10:38     ` Viresh Kumar
2023-10-16 14:50       ` Ulf Hansson
2023-10-13  8:48 ` Viresh Kumar [this message]
2023-10-16 10:11   ` [PATCH 4/5] OPP: Remove genpd_virt_dev_lock Ulf Hansson
2023-10-13  8:48 ` [PATCH 5/5] OPP: No need to defer probe from _opp_attach_genpd() Viresh Kumar
2023-10-16 10:11   ` Ulf Hansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afec94cb6515e79c3b336f74be8aa929a303b26f.1697186772.git.viresh.kumar@linaro.org \
    --to=viresh.kumar@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=rafael@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=ulf.hansson@linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vireshk@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.