From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Wed, 01 Jul 2015 11:44:20 +0200 Subject: [U-Boot] [PATCH v3 36/54] dm: pmic: Split output from function In-Reply-To: <1435095556-15924-37-git-send-email-sjg@chromium.org> References: <1435095556-15924-1-git-send-email-sjg@chromium.org> <1435095556-15924-37-git-send-email-sjg@chromium.org> Message-ID: <5593B674.1010000@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- > > 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 Acked-by: Przemyslaw Marczak Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com