[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <bf7e6e072ed166c11c687200df53aec8d7446182.1667473008.git.viresh.kumar@linaro.org>
Date: Thu, 3 Nov 2022 16:31:07 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: James Calligeros <jcalligeros99@...il.com>,
Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
Stephen Boyd <sboyd@...nel.org>
Cc: Viresh Kumar <viresh.kumar@...aro.org>, linux-pm@...r.kernel.org,
Vincent Guittot <vincent.guittot@...aro.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
linux-kernel@...r.kernel.org
Subject: [PATCH 4/5] OPP: Simplify opp_parse_supplies() by restructuring it
opp_parse_supplies() has grown into too big of a routine (~190 lines)
and it is not straight-forward to understand it anymore.
Break it into smaller routines and reduce code redundancy a bit by using
the same code to parse properties.
This shouldn't result in any logical changes.
Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
---
drivers/opp/of.c | 216 ++++++++++++++++++-----------------------------
1 file changed, 81 insertions(+), 135 deletions(-)
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index e010e119c42b..e51c43495e21 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -578,179 +578,126 @@ static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
return false;
}
-static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
- struct opp_table *opp_table)
+static u32 *_parse_named_prop(struct dev_pm_opp *opp, struct device *dev,
+ struct opp_table *opp_table,
+ const char *prop_type, bool *triplet)
{
- u32 *microvolt, *microamp = NULL, *microwatt = NULL;
- int supplies = opp_table->regulator_count;
- int vcount, icount, pcount, ret, i, j;
struct property *prop = NULL;
char name[NAME_MAX];
+ int count, ret;
+ u32 *out;
- /* Search for "opp-microvolt-<name>" */
+ /* Search for "opp-<prop_type>-<name>" */
if (opp_table->prop_name) {
- snprintf(name, sizeof(name), "opp-microvolt-%s",
+ snprintf(name, sizeof(name), "opp-%s-%s", prop_type,
opp_table->prop_name);
prop = of_find_property(opp->np, name, NULL);
}
if (!prop) {
- /* Search for "opp-microvolt" */
- sprintf(name, "opp-microvolt");
+ /* Search for "opp-<prop_type>" */
+ snprintf(name, sizeof(name), "opp-%s", prop_type);
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;
- }
-
- 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)
+ return NULL;
}
- 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;
+ count = of_property_count_u32_elems(opp->np, name);
+ if (count < 0) {
+ dev_err(dev, "%s: Invalid %s property (%d)\n", __func__, name,
+ count);
+ return ERR_PTR(count);
}
- /* 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;
+ /*
+ * Initialize regulator_count, if regulator information isn't provided
+ * by the platform. Now that one of the properties is available, fix the
+ * regulator_count to 1.
+ */
+ if (unlikely(opp_table->regulator_count == -1))
+ opp_table->regulator_count = 1;
+
+ if (count != opp_table->regulator_count &&
+ (!triplet || count != opp_table->regulator_count * 3)) {
+ dev_err(dev, "%s: Invalid number of elements in %s property (%u) with supplies (%d)\n",
+ __func__, prop_type, count, opp_table->regulator_count);
+ return ERR_PTR(-EINVAL);
}
- microvolt = kmalloc_array(vcount, sizeof(*microvolt), GFP_KERNEL);
- if (!microvolt)
- return -ENOMEM;
+ out = kmalloc_array(count, sizeof(*out), GFP_KERNEL);
+ if (!out)
+ return ERR_PTR(-EINVAL);
- ret = of_property_read_u32_array(opp->np, name, microvolt, vcount);
+ ret = of_property_read_u32_array(opp->np, name, out, count);
if (ret) {
dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret);
- ret = -EINVAL;
- goto free_microvolt;
+ kfree(out);
+ return ERR_PTR(-EINVAL);
}
- /* 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);
- }
+ if (triplet)
+ *triplet = count != opp_table->regulator_count;
- if (!prop) {
- /* Search for "opp-microamp" */
- sprintf(name, "opp-microamp");
- prop = of_find_property(opp->np, name, NULL);
- }
-
- if (prop) {
- 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;
- }
+ return out;
+}
- if (icount != supplies) {
- 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;
- }
+static u32 *opp_parse_microvolt(struct dev_pm_opp *opp, struct device *dev,
+ struct opp_table *opp_table, bool *triplet)
+{
+ u32 *microvolt;
- microamp = kmalloc_array(icount, sizeof(*microamp), GFP_KERNEL);
- if (!microamp) {
- ret = -EINVAL;
- goto free_microvolt;
- }
+ microvolt = _parse_named_prop(opp, dev, opp_table, "microvolt", triplet);
+ if (IS_ERR(microvolt))
+ return microvolt;
- 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);
- ret = -EINVAL;
- goto free_microamp;
+ if (!microvolt) {
+ /*
+ * Missing property isn't a problem, but an invalid
+ * entry is. This property isn't optional if regulator
+ * information is provided.
+ */
+ if (opp_table->regulator_count > 0) {
+ dev_err(dev, "%s: opp-microvolt missing although OPP managing regulators\n",
+ __func__);
+ return ERR_PTR(-EINVAL);
}
}
- /* Search for "opp-microwatt-<name>" */
- prop = NULL;
- if (opp_table->prop_name) {
- snprintf(name, sizeof(name), "opp-microwatt-%s",
- opp_table->prop_name);
- prop = of_find_property(opp->np, name, NULL);
- }
-
- if (!prop) {
- /* Search for "opp-microwatt" */
- sprintf(name, "opp-microwatt");
- prop = of_find_property(opp->np, name, NULL);
- }
+ return microvolt;
+}
- if (prop) {
- 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;
- }
+static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
+ struct opp_table *opp_table)
+{
+ u32 *microvolt, *microamp, *microwatt;
+ int ret, i, j;
+ bool triplet;
- if (pcount != supplies) {
- 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;
- }
+ microvolt = opp_parse_microvolt(opp, dev, opp_table, &triplet);
+ if (IS_ERR_OR_NULL(microvolt))
+ return PTR_ERR(microvolt);
- microwatt = kmalloc_array(pcount, sizeof(*microwatt),
- GFP_KERNEL);
- if (!microwatt) {
- ret = -EINVAL;
- goto free_microamp;
- }
+ microamp = _parse_named_prop(opp, dev, opp_table, "microamp", NULL);
+ if (IS_ERR(microamp)) {
+ ret = PTR_ERR(microamp);
+ goto free_microvolt;
+ }
- 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);
- ret = -EINVAL;
- goto free_microwatt;
- }
+ microwatt = _parse_named_prop(opp, dev, opp_table, "microwatt", NULL);
+ if (IS_ERR(microwatt)) {
+ ret = PTR_ERR(microwatt);
+ goto free_microamp;
}
- for (i = 0, j = 0; i < supplies; i++) {
+ for (i = 0, j = 0; i < opp_table->regulator_count; i++) {
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 {
+ if (triplet) {
opp->supplies[i].u_volt_min = microvolt[j++];
opp->supplies[i].u_volt_max = microvolt[j++];
+ } else {
+ opp->supplies[i].u_volt_min = opp->supplies[i].u_volt;
+ opp->supplies[i].u_volt_max = opp->supplies[i].u_volt;
}
if (microamp)
@@ -760,7 +707,6 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
opp->supplies[i].u_watt = microwatt[i];
}
-free_microwatt:
kfree(microwatt);
free_microamp:
kfree(microamp);
--
2.31.1.272.g89b43f80a514
Powered by blists - more mailing lists