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 1/2] OPP: pstate is only valid for genpd OPP tables
Date: Wed, 14 Jun 2023 16:07:25 +0530	[thread overview]
Message-ID: <5437756c65c79f9520886bc54321d39c022c8638.1686739018.git.viresh.kumar@linaro.org> (raw)

It is not very clear right now that the `pstate` field is only valid for
genpd OPP tables and not consumer tables. And there is no checking for
the same at various places.

Add checks in place to verify that and make it clear to the reader.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/core.c    | 18 ++++++++++++++++--
 drivers/opp/debugfs.c |  4 +++-
 drivers/opp/of.c      |  6 ++++++
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 7290168ec806..bfb012f5383c 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -227,16 +227,24 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_level);
 unsigned int dev_pm_opp_get_required_pstate(struct dev_pm_opp *opp,
 					    unsigned int index)
 {
+	struct opp_table *opp_table = opp->opp_table;
+
 	if (IS_ERR_OR_NULL(opp) || !opp->available ||
-	    index >= opp->opp_table->required_opp_count) {
+	    index >= opp_table->required_opp_count) {
 		pr_err("%s: Invalid parameters\n", __func__);
 		return 0;
 	}
 
 	/* required-opps not fully initialized yet */
-	if (lazy_linking_pending(opp->opp_table))
+	if (lazy_linking_pending(opp_table))
 		return 0;
 
+	/* The required OPP table must belong to a genpd */
+	if (unlikely(!opp_table->required_opp_tables[index]->is_genpd)) {
+		pr_err("%s: Performance state is only valid for genpds.\n", __func__);
+		return 0;
+	}
+
 	return opp->required_opps[index]->pstate;
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_required_pstate);
@@ -2686,6 +2694,12 @@ int dev_pm_opp_xlate_performance_state(struct opp_table *src_table,
 	int dest_pstate = -EINVAL;
 	int i;
 
+	/* Both OPP tables must belong to genpds */
+	if (unlikely(!src_table->is_genpd || !dst_table->is_genpd)) {
+		pr_err("%s: Performance state is only valid for genpds.\n", __func__);
+		return -EINVAL;
+	}
+
 	/*
 	 * Normally the src_table will have the "required_opps" property set to
 	 * point to one of the OPPs in the dst_table, but in some cases the
diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c
index 2c7fb683441e..0cc21e2b42ff 100644
--- a/drivers/opp/debugfs.c
+++ b/drivers/opp/debugfs.c
@@ -152,11 +152,13 @@ void opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
 	debugfs_create_bool("dynamic", S_IRUGO, d, &opp->dynamic);
 	debugfs_create_bool("turbo", S_IRUGO, d, &opp->turbo);
 	debugfs_create_bool("suspend", S_IRUGO, d, &opp->suspend);
-	debugfs_create_u32("performance_state", S_IRUGO, d, &opp->pstate);
 	debugfs_create_u32("level", S_IRUGO, d, &opp->level);
 	debugfs_create_ulong("clock_latency_ns", S_IRUGO, d,
 			     &opp->clock_latency_ns);
 
+	if (opp_table->is_genpd)
+		debugfs_create_u32("performance_state", S_IRUGO, d, &opp->pstate);
+
 	opp->of_name = of_node_full_name(opp->np);
 	debugfs_create_str("of_name", S_IRUGO, d, (char **)&opp->of_name);
 
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 943c7fb7402b..e23ce6e78eb6 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -1392,6 +1392,12 @@ int of_get_required_opp_performance_state(struct device_node *np, int index)
 		goto put_required_np;
 	}
 
+	/* 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;
+	}
+
 	opp = _find_opp_of_np(opp_table, required_np);
 	if (opp) {
 		pstate = opp->pstate;
-- 
2.31.1.272.g89b43f80a514


             reply	other threads:[~2023-06-14 10:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-14 10:37 Viresh Kumar [this message]
2023-06-14 10:37 ` [PATCH 2/2] OPP: Simplify the over-designed pstate <-> level dance Viresh Kumar
2023-06-14 12:37   ` Ulf Hansson
2023-06-16  6:20     ` Viresh Kumar
2023-06-14 13:05   ` kernel test robot
2023-06-14 14:07   ` kernel test robot
2023-06-14 12:33 ` [PATCH 1/2] OPP: pstate is only valid for genpd OPP tables Ulf Hansson
2023-06-15 23:35 ` Bjorn Andersson
2023-06-16  6:18   ` Viresh Kumar
2023-06-16 14:31     ` Bjorn Andersson

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=5437756c65c79f9520886bc54321d39c022c8638.1686739018.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.