dm: pmic: Split output from function

The regulator_autoset() function mixes printf() output and PMIC adjustment
code. It provides a boolean to control the output. It is better to avoid
missing logic and output, and this permits a smaller SPL code size. So
split the output into a separate function.

Also rename the function to have a by_name() suffix, since we would like
to be able to pass a device when we know it, and thus avoid the name
search.

Signed-off-by: Simon Glass <sjg@chromium.org>
Tested-by: Przemyslaw Marczak <p.marczak@samsung.com>
Acked-by: Przemyslaw Marczak <p.marczak@samsung.com>

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 0f1ca774021f98896136385daa2e4a93f195b8e1..687d3b1e31b6574a8099a675b801113335c00671 100644 (file)
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -138,87 +138,57 @@ int regulator_get_by_devname(const char *devname, struct udevice **devp)
        return uclass_get_device_by_name(UCLASS_REGULATOR, devname, devp);
 }
 
-static int failed(int ret, bool verbose, const char *fmt, ...)
+int regulator_autoset(struct udevice *dev)
 {
-       va_list args;
-       char buf[64];
-
-       if (verbose == false)
-               return ret;
+       struct dm_regulator_uclass_platdata *uc_pdata;
+       int ret = 0;
 
-       va_start(args, fmt);
-       vscnprintf(buf, sizeof(buf), fmt, args);
-       va_end(args);
+       uc_pdata = dev_get_uclass_platdata(dev);
+       if (!uc_pdata->always_on && !uc_pdata->boot_on)
+               return -EMEDIUMTYPE;
 
-       printf(buf);
+       if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV)
+               ret = regulator_set_value(dev, uc_pdata->min_uV);
+       if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA))
+               ret = regulator_set_current(dev, uc_pdata->min_uA);
 
        if (!ret)
-               return 0;
-
-       printf(" (ret: %d)", ret);
+               ret = regulator_set_enable(dev, true);
 
        return ret;
 }
 
-int regulator_autoset(const char *platname,
-                     struct udevice **devp,
-                     bool verbose)
+static void regulator_show(struct udevice *dev, int ret)
 {
        struct dm_regulator_uclass_platdata *uc_pdata;
-       struct udevice *dev;
-       int ret;
-
-       if (devp)
-               *devp = NULL;
-
-       ret = regulator_get_by_platname(platname, &dev);
-       if (ret) {
-               error("Can get the regulator: %s!", platname);
-               return ret;
-       }
 
        uc_pdata = dev_get_uclass_platdata(dev);
-       if (!uc_pdata) {
-               error("Can get the regulator %s uclass platdata!", platname);
-               return -ENXIO;
-       }
-
-       if (!uc_pdata->always_on && !uc_pdata->boot_on)
-               goto retdev;
 
-       if (verbose)
-               printf("%s@%s: ", dev->name, uc_pdata->name);
-
-       /* Those values are optional (-ENODATA if unset) */
-       if ((uc_pdata->min_uV != -ENODATA) &&
-           (uc_pdata->max_uV != -ENODATA) &&
-           (uc_pdata->min_uV == uc_pdata->max_uV)) {
-               ret = regulator_set_value(dev, uc_pdata->min_uV);
-               if (failed(ret, verbose, "set %d uV", uc_pdata->min_uV))
-                       goto exit;
-       }
-
-       /* Those values are optional (-ENODATA if unset) */
-       if ((uc_pdata->min_uA != -ENODATA) &&
-           (uc_pdata->max_uA != -ENODATA) &&
-           (uc_pdata->min_uA == uc_pdata->max_uA)) {
-               ret = regulator_set_current(dev, uc_pdata->min_uA);
-               if (failed(ret, verbose, "; set %d uA", uc_pdata->min_uA))
-                       goto exit;
-       }
+       printf("%s@%s: ", dev->name, uc_pdata->name);
+       if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV)
+               printf("set %d uV", uc_pdata->min_uV);
+       if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)
+               printf("; set %d uA", uc_pdata->min_uA);
+       printf("; enabling");
+       if (ret)
+               printf(" (ret: %d)\n", ret);
+       printf("\n");
+}
 
-       ret = regulator_set_enable(dev, true);
-       if (failed(ret, verbose, "; enabling", uc_pdata->min_uA))
-               goto exit;
+int regulator_autoset_by_name(const char *platname, struct udevice **devp)
+{
+       struct udevice *dev;
+       int ret;
 
-retdev:
+       ret = regulator_get_by_platname(platname, &dev);
        if (devp)
                *devp = dev;
-exit:
-       if (verbose)
-               printf("\n");
+       if (ret) {
+               debug("Can get the regulator: %s!", platname);
+               return ret;
+       }
 
-       return ret;
+       return regulator_autoset(dev);
 }
 
 int regulator_list_autoset(const char *list_platname[],
@@ -229,7 +199,9 @@ int regulator_list_autoset(const char *list_platname[],
        int error = 0, i = 0, ret;
 
        while (list_platname[i]) {
-               ret = regulator_autoset(list_platname[i], &dev, verbose);
+               ret = regulator_autoset_by_name(list_platname[i], &dev);
+               if (ret != -EMEDIUMTYPE && verbose)
+                       regulator_show(dev, ret);
                if (ret & !error)
                        error = ret;
 

diff --git a/include/power/regulator.h b/include/power/regulator.h
index 79ce0a4183255af23a9b1ad26a5c04214cabd9dc..dd043711405c4ad24dad3c7ce7180364b512e720 100644 (file)
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -316,9 +316,28 @@ int regulator_get_mode(struct udevice *dev);
 int regulator_set_mode(struct udevice *dev, int mode_id);
 
 /**
- * regulator_autoset: setup the regulator given by its uclass's platform data
- * name field. The setup depends on constraints found in device's uclass's
- * platform data (struct dm_regulator_uclass_platdata):
+ * regulator_autoset: setup the voltage/current on a regulator
+ *
+ * The setup depends on constraints found in device's uclass's platform data
+ * (struct dm_regulator_uclass_platdata):
+ *
+ * - Enable - will set - if any of: 'always_on' or 'boot_on' is set to true,
+ *   or if both are unset, then the function returns
+ * - Voltage value - will set - if '.min_uV' and '.max_uV' values are equal
+ * - Current limit - will set - if '.min_uA' and '.max_uA' values are equal
+ *
+ * The function returns on the first-encountered error.
+ *
+ * @platname - expected string for dm_regulator_uclass_platdata .name field
+ * @devp     - returned pointer to the regulator device - if non-NULL passed
+ * @return: 0 on success or negative value of errno.
+ */
+int regulator_autoset(struct udevice *dev);
+
+/**
+ * regulator_autoset_by_name: setup the regulator given by its uclass's
+ * platform data name field. The setup depends on constraints found in device's
+ * uclass's platform data (struct dm_regulator_uclass_platdata):
  * - Enable - will set - if any of: 'always_on' or 'boot_on' is set to true,
  *   or if both are unset, then the function returns
  * - Voltage value - will set - if '.min_uV' and '.max_uV' values are equal
@@ -328,21 +347,18 @@ int regulator_set_mode(struct udevice *dev, int mode_id);
  *
  * @platname - expected string for dm_regulator_uclass_platdata .name field
  * @devp     - returned pointer to the regulator device - if non-NULL passed
- * @verbose  - (true/false) print regulator setup info, or be quiet
  * @return: 0 on success or negative value of errno.
  *
  * The returned 'regulator' device can be used with:
  * - regulator_get/set_*
  */
-int regulator_autoset(const char *platname,
-                     struct udevice **devp,
-                     bool verbose);
+int regulator_autoset_by_name(const char *platname, struct udevice **devp);
 
 /**
  * regulator_list_autoset: setup the regulators given by list of their uclass's
  * platform data name field. The setup depends on constraints found in device's
  * uclass's platform data. The function loops with calls to:
- * regulator_autoset() for each name from the list.
+ * regulator_autoset_by_name() for each name from the list.
  *
  * @list_platname - an array of expected strings for .name field of each
  *                  regulator's uclass platdata
@@ -383,7 +399,7 @@ int regulator_get_by_devname(const char *devname, struct udevice **devp);
  * Search by name, found in regulator uclass platdata.
  *
  * @platname - expected string for uc_pdata->name of regulator uclass platdata
- * @devp     - returned pointer to the regulator device
+ * @devp     - returns pointer to the regulator device or NULL on error
  * @return 0 on success or negative value of errno.
  *
  * The returned 'regulator' device is probed and can be used with:

diff --git a/include/power/sandbox_pmic.h b/include/power/sandbox_pmic.h
index ae142921e53b86a6f5866f3d98a18a0e934a5702..854767497145f4a1a1670a807e4e4b5ac5c99030 100644 (file)
--- a/include/power/sandbox_pmic.h
+++ b/include/power/sandbox_pmic.h
@@ -117,11 +117,11 @@ enum {
 
 /*
  * Expected regulators setup after call of:
- * - regulator_autoset()
+ * - regulator_autoset_by_name()
  * - regulator_list_autoset()
  */
 
-/* BUCK1: for testing regulator_autoset() */
+/* BUCK1: for testing regulator_autoset_by_name() */
 #define SANDBOX_BUCK1_AUTOSET_EXPECTED_UV      1200000
 #define SANDBOX_BUCK1_AUTOSET_EXPECTED_UA      200000
 #define SANDBOX_BUCK1_AUTOSET_EXPECTED_ENABLE  true

diff --git a/test/dm/regulator.c b/test/dm/regulator.c
index d279c04c840c415352716c96c4cf02c69c0f4233..3d0056f2dc7bf13049d4d2c13e711bedf9aa8f77 100644 (file)
--- a/test/dm/regulator.c
+++ b/test/dm/regulator.c
@@ -210,7 +210,7 @@ static int dm_test_power_regulator_autoset(struct unit_test_state *uts)
         * Expected output state: uV=1200000; uA=200000; output enabled
         */
        platname = regulator_names[BUCK1][PLATNAME];
-       ut_assertok(regulator_autoset(platname, &dev_autoset, false));
+       ut_assertok(regulator_autoset_by_name(platname, &dev_autoset));
 
        /* Check, that the returned device is proper */
        ut_assertok(regulator_get_by_platname(platname, &dev));