OPP: Simplify opp_parse_supplies() by restructuring it
authorViresh Kumar <viresh.kumar@linaro.org>
Wed, 2 Nov 2022 11:17:08 +0000 (16:47 +0530)
committerViresh Kumar <viresh.kumar@linaro.org>
Fri, 4 Nov 2022 05:29:05 +0000 (10:59 +0530)
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.

Tested-by: James Calligeros <jcalligeros99@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
drivers/opp/of.c

index e010e11..08d97b9 100644 (file)
@@ -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 = 0, 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);