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>

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;
+	struct dm_regulator_uclass_platdata *uc_pdata;
-	char buf[64];
+	int ret = 0;
 
-	if (verbose == false)
+	uc_pdata = dev_get_uclass_platdata(dev);
-		return ret;
+	if (!uc_pdata->always_on && !uc_pdata->boot_on)
+		return -EMEDIUMTYPE;
 
-	va_start(args, fmt);
+	if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV)
-	vscnprintf(buf, sizeof(buf), fmt, args);
+		ret = regulator_set_value(dev, uc_pdata->min_uV);
-	va_end(args);
+	if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA))
-
+		ret = regulator_set_current(dev, uc_pdata->min_uA);
-	printf(buf);
 
 	if (!ret)
-		return 0;
+		ret = regulator_set_enable(dev, true);
-
-	printf(" (ret: %d)", ret);
 
 	return ret;
 }
 
-int regulator_autoset(const char *platname,
+static void regulator_show(struct udevice *dev, int ret)
-		      struct udevice **devp,
-		      bool verbose)
 {
 	struct dm_regulator_uclass_platdata *uc_pdata;
+
+	uc_pdata = dev_get_uclass_platdata(dev);
+
+	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");
+}
+
+int regulator_autoset_by_name(const char *platname, struct udevice **devp)
+{
 	struct udevice *dev;
 	int ret;
 
-	if (devp)
-		*devp = NULL;
-
 	ret = regulator_get_by_platname(platname, &dev);
+	if (devp)
+		*devp = dev;
 	if (ret) {
-		error("Can get the regulator: %s!", platname);
+		debug("Can get the regulator: %s!", platname);
 		return ret;
 	}
 
-	uc_pdata = dev_get_uclass_platdata(dev);
+	return regulator_autoset(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;
-	}
-
-	ret = regulator_set_enable(dev, true);
-	if (failed(ret, verbose, "; enabling", uc_pdata->min_uA))
-		goto exit;
-
-retdev:
-	if (devp)
-		*devp = dev;
-exit:
-	if (verbose)
-		printf("\n");
-
-	return ret;
 }
 
 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;
 

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
+ * regulator_autoset: setup the voltage/current on a regulator
- * name field. The setup depends on constraints found in device's uclass's
+ *
- * platform data (struct dm_regulator_uclass_platdata):
+ * 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,
+int regulator_autoset_by_name(const char *platname, struct udevice **devp);
-		      struct udevice **devp,
-		      bool verbose);
 
 /**
  * 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:

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

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));