| Message ID | 1435095556-15924-37-git-send-email-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
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 <sjg@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.marczak@samsung.com> Acked-by: Przemyslaw Marczak <p.marczak@samsung.com> Best regards,
On 1 July 2015 at 03:44, Przemyslaw Marczak <p.marczak@samsung.com> wrote: > 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 <sjg@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.marczak@samsung.com> > Acked-by: Przemyslaw Marczak <p.marczak@samsung.com> > > Best regards, > -- > Przemyslaw Marczak > Samsung R&D Institute Poland > Samsung Electronics > p.marczak@samsung.com Applied to u-boot-dm.
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 + * + * 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));
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> --- 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(-)