Hello Simon,
On 06/23/2015 11:38 PM, Simon Glass wrote:
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 <s...@chromium.org>
---
Changes in v3: None
Changes in v2: None
drivers/power/regulator/regulator-uclass.c | 98 +++++++++++-------------------
include/power/regulator.h | 34 ++++++++---
include/power/sandbox_pmic.h | 4 +-
test/dm/regulator.c | 2 +-
4 files changed, 63 insertions(+), 75 deletions(-)
diff --git a/drivers/power/regulator/regulator-uclass.c
b/drivers/power/regulator/regulator-uclass.c
index 0f1ca77..687d3b1 100644
--- 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 79ce0a4..86e9c3b 100644
--- 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 the voltage/current on a regulator
duplicated "the"
+ *
+ * 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 ae14292..8547674 100644
--- 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 d279c04..3d0056f 100644
--- 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));
Tested on:
- Odroid U3 (odroid_defconfig)
- Sandbox - ut pmic/regulator
Tested-by: Przemyslaw Marczak <p.marc...@samsung.com>
Acked-by: Przemyslaw Marczak <p.marc...@samsung.com>
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
