All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Calligeros <jcalligeros99@gmail.com>
To: vireshk@kernel.org, nm@ti.com, sboyd@kernel.org,
	linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, asahi@vger.kernel.org,
	James Calligeros <jcalligeros99@gmail.com>
Subject: [PATCH] OPP: decouple dt properties in opp_parse_supplies()
Date: Sun, 30 Oct 2022 13:44:15 +1000	[thread overview]
Message-ID: <20221030034414.24672-1-jcalligeros99@gmail.com> (raw)

The opp-microwatt property was added with the intention of providing
platforms a way to specify a precise value for the power consumption
of a device at a given OPP to enable better energy-aware scheduling
decisions by informing the kernel of the total static and dynamic
power of a device at a given OPP, removing the reliance on the EM
subsystem's often flawed estimations. This property is parsed by
opp_parse_supplies(), which creates a hard dependency on the
opp-microvolt property.

Some platforms, such as Apple Silicon, do not describe their devices'
voltage regulators in the DT as they cannot be controlled by the kernel
and/or rely on opaque firmware algorithms to control their voltage and
current characteristics at runtime. We can, however, experimentally
determine the power consumption of a given device at a given OPP, taking
advantage of opp-microwatt to provide EAS on such devices as was initially
intended.

Allow platforms to specify and consume any subset of opp-microvolt,
opp-microamp, or opp-microwatt without a hard dependency on opp-microvolt
to enable this functionality on such platforms.

Fixes: 4f9a7a1dc2a2 ("OPP: Add "opp-microwatt" supporting code")
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 drivers/opp/of.c | 198 +++++++++++++++++++++++++----------------------
 1 file changed, 104 insertions(+), 94 deletions(-)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 605d68673f92..0fa25c3a959e 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -581,166 +581,176 @@ static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
 static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
 			      struct opp_table *opp_table)
 {
-	u32 *microvolt, *microamp = NULL, *microwatt = NULL;
+	u32 *microvolt = NULL, *microamp = NULL, *microwatt = NULL;
 	int supplies = opp_table->regulator_count;
 	int vcount, icount, pcount, ret, i, j;
-	struct property *prop = NULL;
+	struct property *prop_mv = NULL, *prop_ma = NULL, *prop_mw = NULL;
 	char name[NAME_MAX];
 
 	/* Search for "opp-microvolt-<name>" */
 	if (opp_table->prop_name) {
 		snprintf(name, sizeof(name), "opp-microvolt-%s",
 			 opp_table->prop_name);
-		prop = of_find_property(opp->np, name, NULL);
+		prop_mv = of_find_property(opp->np, name, NULL);
 	}
 
-	if (!prop) {
+	if (!prop_mv) {
 		/* Search for "opp-microvolt" */
 		sprintf(name, "opp-microvolt");
-		prop = of_find_property(opp->np, name, NULL);
-
-		/* Missing property isn't a problem, but an invalid entry is */
-		if (!prop) {
-			if (unlikely(supplies == -1)) {
-				/* Initialize regulator_count */
-				opp_table->regulator_count = 0;
-				return 0;
-			}
+		prop_mv = of_find_property(opp->np, name, NULL);
 
-			if (!supplies)
-				return 0;
-
-			dev_err(dev, "%s: opp-microvolt missing although OPP managing regulators\n",
-				__func__);
-			return -EINVAL;
-		}
 	}
 
-	if (unlikely(supplies == -1)) {
-		/* Initialize regulator_count */
-		supplies = opp_table->regulator_count = 1;
-	} else if (unlikely(!supplies)) {
-		dev_err(dev, "%s: opp-microvolt wasn't expected\n", __func__);
-		return -EINVAL;
+	if (prop_mv) {
+		vcount = of_property_count_u32_elems(opp->np, name);
+		if (unlikely(supplies == -1))
+			supplies = opp_table->regulator_count = vcount;
+	} else {
+		prop_mv = NULL;
+		vcount = 0;
 	}
 
-	vcount = of_property_count_u32_elems(opp->np, name);
 	if (vcount < 0) {
 		dev_err(dev, "%s: Invalid %s property (%d)\n",
 			__func__, name, vcount);
 		return vcount;
 	}
 
-	/* There can be one or three elements per supply */
-	if (vcount != supplies && vcount != supplies * 3) {
-		dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
-			__func__, name, vcount, supplies);
-		return -EINVAL;
-	}
+	if (vcount) {
+		/* There can be one or three elements per supply */
+		if (vcount != supplies && vcount != supplies * 3) {
+			dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
+				__func__, name, vcount, supplies);
+			return -EINVAL;
+		}
 
-	microvolt = kmalloc_array(vcount, sizeof(*microvolt), GFP_KERNEL);
-	if (!microvolt)
-		return -ENOMEM;
+		microvolt = kmalloc_array(vcount, sizeof(*microvolt), GFP_KERNEL);
+		if (!microvolt)
+			return -ENOMEM;
 
-	ret = of_property_read_u32_array(opp->np, name, microvolt, vcount);
-	if (ret) {
-		dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret);
-		ret = -EINVAL;
-		goto free_microvolt;
+		ret = of_property_read_u32_array(opp->np, name, microvolt, vcount);
+		if (ret) {
+			dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret);
+			ret = -EINVAL;
+			goto free_microvolt;
+		}
 	}
 
 	/* Search for "opp-microamp-<name>" */
-	prop = NULL;
 	if (opp_table->prop_name) {
 		snprintf(name, sizeof(name), "opp-microamp-%s",
 			 opp_table->prop_name);
-		prop = of_find_property(opp->np, name, NULL);
+		prop_ma = of_find_property(opp->np, name, NULL);
 	}
 
-	if (!prop) {
+	if (!prop_ma) {
 		/* Search for "opp-microamp" */
 		sprintf(name, "opp-microamp");
-		prop = of_find_property(opp->np, name, NULL);
+		prop_ma = of_find_property(opp->np, name, NULL);
+
 	}
 
-	if (prop) {
+	if (prop_ma) {
 		icount = of_property_count_u32_elems(opp->np, name);
-		if (icount < 0) {
-			dev_err(dev, "%s: Invalid %s property (%d)\n", __func__,
-				name, icount);
-			ret = icount;
-			goto free_microvolt;
-		}
+		if (unlikely(supplies == -1))
+			supplies = opp_table->regulator_count = icount;
+	} else {
+		prop_ma = NULL;
+		icount = 0;
+	}
 
-		if (icount != supplies) {
+	if (icount < 0) {
+		dev_err(dev, "%s: Invalid %s property (%d)\n",
+			__func__, name, icount);
+		return icount;
+	}
+
+	if (icount) {
+		/* There can be one or three elements per supply */
+		if (icount != supplies && icount != supplies * 3) {
 			dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
 				__func__, name, icount, supplies);
-			ret = -EINVAL;
-			goto free_microvolt;
+			return -EINVAL;
 		}
 
 		microamp = kmalloc_array(icount, sizeof(*microamp), GFP_KERNEL);
-		if (!microamp) {
-			ret = -EINVAL;
-			goto free_microvolt;
-		}
+		if (!microamp)
+			return -ENOMEM;
 
-		ret = of_property_read_u32_array(opp->np, name, microamp,
-						 icount);
+		ret = of_property_read_u32_array(opp->np, name, microamp, icount);
 		if (ret) {
-			dev_err(dev, "%s: error parsing %s: %d\n", __func__,
-				name, ret);
+			dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret);
 			ret = -EINVAL;
 			goto free_microamp;
 		}
 	}
 
-	/* Search for "opp-microwatt" */
-	sprintf(name, "opp-microwatt");
-	prop = of_find_property(opp->np, name, NULL);
+	/* Search for "opp-microwatt-<name>" */
+	if (opp_table->prop_name) {
+		snprintf(name, sizeof(name), "opp-microwatt-%s",
+			 opp_table->prop_name);
+		prop_mw = of_find_property(opp->np, name, NULL);
+	}
+
+	if (!prop_mw) {
+		/* Search for "opp-microwatt" */
+		sprintf(name, "opp-microwatt");
+		prop_mw = of_find_property(opp->np, name, NULL);
 
-	if (prop) {
+	}
+
+	if (prop_mw) {
 		pcount = of_property_count_u32_elems(opp->np, name);
-		if (pcount < 0) {
-			dev_err(dev, "%s: Invalid %s property (%d)\n", __func__,
-				name, pcount);
-			ret = pcount;
-			goto free_microamp;
-		}
+		if (unlikely(supplies == -1))
+			supplies = opp_table->regulator_count = pcount;
+	} else {
+		prop_mw = NULL;
+		pcount = 0;
+	}
+
+	if (pcount < 0) {
+		dev_err(dev, "%s: Invalid %s property (%d)\n",
+			__func__, name, pcount);
+		return pcount;
+	}
 
-		if (pcount != supplies) {
+	if (pcount) {
+		/* There can be one or three elements per supply */
+		if (pcount != supplies && pcount != supplies * 3) {
 			dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
 				__func__, name, pcount, supplies);
-			ret = -EINVAL;
-			goto free_microamp;
+			return -EINVAL;
 		}
 
-		microwatt = kmalloc_array(pcount, sizeof(*microwatt),
-					  GFP_KERNEL);
-		if (!microwatt) {
-			ret = -EINVAL;
-			goto free_microamp;
-		}
+		microwatt = kmalloc_array(pcount, sizeof(*microwatt), GFP_KERNEL);
+		if (!microwatt)
+			return -ENOMEM;
 
-		ret = of_property_read_u32_array(opp->np, name, microwatt,
-						 pcount);
+		ret = of_property_read_u32_array(opp->np, name, microwatt, pcount);
 		if (ret) {
-			dev_err(dev, "%s: error parsing %s: %d\n", __func__,
-				name, ret);
+			dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret);
 			ret = -EINVAL;
 			goto free_microwatt;
 		}
 	}
 
-	for (i = 0, j = 0; i < supplies; i++) {
-		opp->supplies[i].u_volt = microvolt[j++];
+	/* No supplies associated with the OPP */
+	if (unlikely(supplies == -1)) {
+		opp->regulator_count = 0;
+		return 0;
+	}
 
-		if (vcount == supplies) {
-			opp->supplies[i].u_volt_min = opp->supplies[i].u_volt;
-			opp->supplies[i].u_volt_max = opp->supplies[i].u_volt;
-		} else {
-			opp->supplies[i].u_volt_min = microvolt[j++];
-			opp->supplies[i].u_volt_max = microvolt[j++];
+	for (i = 0, j = 0; i < supplies; i++) {
+		if (microvolt) {
+			opp->supplies[i].u_volt = microvolt[j++];
+
+			if (vcount == supplies) {
+				opp->supplies[i].u_volt_min = opp->supplies[i].u_volt;
+				opp->supplies[i].u_volt_max = opp->supplies[i].u_volt;
+			} else {
+				opp->supplies[i].u_volt_min = microvolt[j++];
+				opp->supplies[i].u_volt_max = microvolt[j++];
+			}
 		}
 
 		if (microamp)
-- 
2.38.0


             reply	other threads:[~2022-10-30  3:45 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-30  3:44 James Calligeros [this message]
2022-10-30  5:24 ` [PATCH] OPP: decouple dt properties in opp_parse_supplies() kernel test robot
2022-10-30  5:34 ` kernel test robot

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=20221030034414.24672-1-jcalligeros99@gmail.com \
    --to=jcalligeros99@gmail.com \
    --cc=asahi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=sboyd@kernel.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.