[PATCH 4/5] OPP: Simplify opp_parse_supplies() by restructuring it

From: Viresh Kumar
Date: Thu Nov 03 2022 - 07:02:01 EST


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@xxxxxxxxxx>
---
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