From patchwork Fri May 8 16:20:31 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Przemyslaw Marczak X-Patchwork-Id: 470092 X-Patchwork-Delegate: sjg@chromium.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id A9BE0140281 for ; Sat, 9 May 2015 02:22:18 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 528CA4B652; Fri, 8 May 2015 18:22:03 +0200 (CEST) Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YVbw8DZ5HgFi; Fri, 8 May 2015 18:22:03 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 363A34B686; Fri, 8 May 2015 18:21:48 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 4ABA04B61A for ; Fri, 8 May 2015 18:21:35 +0200 (CEST) Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eOjtTQx6x4AE for ; Fri, 8 May 2015 18:21:35 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by theia.denx.de (Postfix) with ESMTPS id 165B44B616 for ; Fri, 8 May 2015 18:21:33 +0200 (CEST) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout2.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NO100A0LHFSCL40@mailout2.w1.samsung.com> for u-boot@lists.denx.de; Fri, 08 May 2015 17:21:28 +0100 (BST) X-AuditID: cbfec7f4-f79c56d0000012ee-28-554ce2871f25 Received: from eusync3.samsung.com ( [203.254.199.213]) by eucpsbgm1.samsung.com (EUCPMTA) with SMTP id 0A.2A.04846.782EC455; Fri, 8 May 2015 17:21:27 +0100 (BST) Received: from AMDC1186.digital.local ([106.116.147.185]) by eusync3.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0NO100K4NHFMYL40@eusync3.samsung.com>; Fri, 08 May 2015 17:21:27 +0100 (BST) From: Przemyslaw Marczak To: u-boot@lists.denx.de Date: Fri, 08 May 2015 18:20:31 +0200 Message-id: <1431102040-745-4-git-send-email-p.marczak@samsung.com> X-Mailer: git-send-email 1.9.1 In-reply-to: <1431102040-745-1-git-send-email-p.marczak@samsung.com> References: <1431102040-745-1-git-send-email-p.marczak@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupgluLIzCtJLcpLzFFi42I5/e/4Vd32Rz6hBiveW1m8ebiZ0WLH5Rss Ft+2bGO0eLu3k91i773PjA6sHrMbLrJ4nL2zg9Gjb8sqRo8T07+zBLBEcdmkpOZklqUW6dsl cGVc7GphLHjax1hx8PEB5gbGhsIuRk4OCQETidYPF5kgbDGJC/fWs3UxcnEICSxllFg7Yy8j hNPMJPF7zR42kCo2AQOJPZfOMIPYIgISEr/6r4IVMQusZZR4+vooO0hCWMBB4sZeiCIWAVWJ tz96weK8As4S3WduQq2Tkzh5bDIriM0p4CLxef1BRhBbCKjm8e+zbBMYeRcwMqxiFE0tTS4o TkrPNdQrTswtLs1L10vOz93ECAmfLzsYFx+zOsQowMGoxMO7YY9PqBBrYllxZe4hRgkOZiUR 3snngUK8KYmVValF+fFFpTmpxYcYpTlYlMR55+56HyIkkJ5YkpqdmlqQWgSTZeLglGpgZG2Y tzzXpsc48cr6d0HmoU6nKlYxSUhdXxx9dPuRVrHsyGf2ogrzTc7Ok+ANmvN7XnCAxYWWW0uv ne+psP4vGMobWX+i6UuNZ2bkh80T8wS/flbjW1px88GLR2FXFgUdO6HDvGgD80X2v/F/Jq8P 37fW/b/iTfNF95VVbMIfvmCW+rb+xJtds5RYijMSDbWYi4oTAZjnFmIbAgAA Cc: Przemyslaw Marczak Subject: [U-Boot] [PATCH 03/12] dm: regulator: uclass driver code cleanup X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.15 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" This cleanup includes: - remove of the preprocessor macros which pointed to long name functions - update of the names of some regulator uclass driver functions - cleanup of the function regulator_autoset() - reword of some comments of regulator uclass header file - regulator_get_by_platname: check error for uclass_find_* function calls - add function: regulator_name_is_unique - regulator post_bind(): check regulator name uniqueness - fix mistakes in: regulator/Kconfig - regulator.h: update comments Signed-off-by: Przemyslaw Marczak Acked-by: Simon Glass Tested-by: Simon Glass --- drivers/power/regulator/Kconfig | 2 +- drivers/power/regulator/regulator-uclass.c | 104 +++++++++++++++++--------- include/power/regulator.h | 116 +++++++++++++++-------------- 3 files changed, 128 insertions(+), 94 deletions(-) diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig index 54ce188..fd3cf35 100644 --- a/drivers/power/regulator/Kconfig +++ b/drivers/power/regulator/Kconfig @@ -14,7 +14,7 @@ config DM_REGULATOR It's important to call the device_bind() with the proper node offset, when binding the regulator devices. The pmic_bind_childs() can be used for this purpose if PMIC I/O driver is implemented or dm_scan_fdt_node() - otherwise. Detailed informations can be found in the header file. + otherwise. Detailed information can be found in the header file. config DM_REGULATOR_MAX77686 bool "Enable Driver Model for REGULATOR MAX77686" diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 07ce286..31ffd44 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -108,16 +108,19 @@ int regulator_set_mode(struct udevice *dev, int mode) return ops->set_mode(dev, mode); } -int regulator_by_platname(const char *plat_name, struct udevice **devp) +int regulator_get_by_platname(const char *plat_name, struct udevice **devp) { struct dm_regulator_uclass_platdata *uc_pdata; struct udevice *dev; + int ret; *devp = NULL; - for (uclass_find_first_device(UCLASS_REGULATOR, &dev); - dev; - uclass_find_next_device(&dev)) { + for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev; + ret = uclass_find_next_device(&dev)) { + if (ret) + continue; + uc_pdata = dev_get_uclass_platdata(dev); if (!uc_pdata || strcmp(plat_name, uc_pdata->name)) continue; @@ -130,12 +133,12 @@ int regulator_by_platname(const char *plat_name, struct udevice **devp) return -ENODEV; } -int regulator_by_devname(const char *devname, struct udevice **devp) +int regulator_get_by_devname(const char *devname, struct udevice **devp) { return uclass_get_device_by_name(UCLASS_REGULATOR, devname, devp); } -static int setting_failed(int ret, bool verbose, const char *fmt, ...) +static int failed(int ret, bool verbose, const char *fmt, ...) { va_list args; char buf[64]; @@ -157,19 +160,18 @@ static int setting_failed(int ret, bool verbose, const char *fmt, ...) return ret; } -int regulator_by_platname_autoset_and_enable(const char *platname, - struct udevice **devp, - bool verbose) +int regulator_autoset(const char *platname, + struct udevice **devp, + bool verbose) { struct dm_regulator_uclass_platdata *uc_pdata; struct udevice *dev; - bool v = verbose; int ret; if (devp) *devp = NULL; - ret = regulator_by_platname(platname, &dev); + ret = regulator_get_by_platname(platname, &dev); if (ret) { error("Can get the regulator: %s!", platname); return ret; @@ -181,7 +183,10 @@ int regulator_by_platname_autoset_and_enable(const char *platname, return -ENXIO; } - if (v) + 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) */ @@ -189,7 +194,7 @@ int regulator_by_platname_autoset_and_enable(const char *platname, (uc_pdata->max_uV != -ENODATA) && (uc_pdata->min_uV == uc_pdata->max_uV)) { ret = regulator_set_value(dev, uc_pdata->min_uV); - if (setting_failed(ret, v, "set %d uV", uc_pdata->min_uV)) + if (failed(ret, verbose, "set %d uV", uc_pdata->min_uV)) goto exit; } @@ -198,49 +203,69 @@ int regulator_by_platname_autoset_and_enable(const char *platname, (uc_pdata->max_uA != -ENODATA) && (uc_pdata->min_uA == uc_pdata->max_uA)) { ret = regulator_set_current(dev, uc_pdata->min_uA); - if (setting_failed(ret, v, "; set %d uA", uc_pdata->min_uA)) + if (failed(ret, verbose, "; set %d uA", uc_pdata->min_uA)) goto exit; } - if (!uc_pdata->always_on && !uc_pdata->boot_on) - goto retdev; - ret = regulator_set_enable(dev, true); - if (setting_failed(ret, v, "; enabling", uc_pdata->min_uA)) + if (failed(ret, verbose, "; enabling", uc_pdata->min_uA)) goto exit; retdev: if (devp) *devp = dev; exit: - if (v) + if (verbose) printf("\n"); + return ret; } -int regulator_by_platname_list_autoset_and_enable(const char *list_platname[], - int list_entries, - struct udevice *list_devp[], - bool verbose) +int regulator_list_autoset(const char *list_platname[], + struct udevice *list_devp[], + bool verbose) { struct udevice *dev; - int i, ret, success = 0; + int error = 0, i = 0, ret; - for (i = 0; i < list_entries; i++) { + while (list_platname[i]) { ret = regulator_autoset(list_platname[i], &dev, verbose); - if (!ret) - success++; + if (ret & !error) + error = ret; + + if (list_devp) + list_devp[i] = dev; + + i++; + } + + return error; +} + +static bool regulator_name_is_unique(struct udevice *check_dev, + const char *check_name) +{ + struct dm_regulator_uclass_platdata *uc_pdata; + struct udevice *dev; + int check_len = strlen(check_name); + int ret; + int len; - if (!list_devp) + for (ret = uclass_find_first_device(UCLASS_REGULATOR, &dev); dev; + ret = uclass_find_next_device(&dev)) { + if (ret || dev == check_dev) continue; - if (ret) - list_devp[i] = NULL; - else - list_devp[i] = dev; + uc_pdata = dev_get_uclass_platdata(dev); + len = strlen(uc_pdata->name); + if (len != check_len) + continue; + + if (!strcmp(uc_pdata->name, check_name)) + return false; } - return (success != list_entries); + return true; } static int regulator_post_bind(struct udevice *dev) @@ -248,20 +273,27 @@ static int regulator_post_bind(struct udevice *dev) struct dm_regulator_uclass_platdata *uc_pdata; int offset = dev->of_offset; const void *blob = gd->fdt_blob; + const char *property = "regulator-name"; uc_pdata = dev_get_uclass_platdata(dev); if (!uc_pdata) return -ENXIO; /* Regulator's mandatory constraint */ - uc_pdata->name = fdt_getprop(blob, offset, "regulator-name", NULL); + uc_pdata->name = fdt_getprop(blob, offset, property, NULL); if (!uc_pdata->name) { debug("%s: dev: %s has no property 'regulator-name'\n", __func__, dev->name); - return -ENXIO; + return -EINVAL; } - return 0; + if (regulator_name_is_unique(dev, uc_pdata->name)) + return 0; + + error("\"%s\" of dev: \"%s\", has nonunique value: \"%s\"", + property, dev->name, uc_pdata->name); + + return -EINVAL; } static int regulator_pre_probe(struct udevice *dev) diff --git a/include/power/regulator.h b/include/power/regulator.h index 6916660..03a2cef 100644 --- a/include/power/regulator.h +++ b/include/power/regulator.h @@ -34,7 +34,7 @@ * regulator constraints, like in the example below: * * ldo1 { - * regulator-name = "VDD_MMC_1.8V"; (mandatory for bind) + * regulator-name = "VDD_MMC_1.8V"; (must be unique for proper bind) * regulator-min-microvolt = <1000000>; (optional) * regulator-max-microvolt = <1000000>; (optional) * regulator-min-microamp = <1000>; (optional) @@ -43,19 +43,22 @@ * regulator-boot-on; (optional) * }; * - * Please take a notice, that for the proper operation at least name constraint - * is needed, e.g. for call the device_by_platname(...). + * Note: For the proper operation, at least name constraint is needed, since + * it can be used when calling regulator_get_by_platname(). And the mandatory + * rule for this name is, that it must be globally unique for the single dts. * * Regulator bind: * For each regulator device, the device_bind() should be called with passed * device tree offset. This is required for this uclass's '.post_bind' method, - * which do the scan on the device node, for the 'regulator-name' constraint. + * which does the scan on the device node, for the 'regulator-name' constraint. * If the parent is not a PMIC device, and the child is not bind by function: * 'pmic_bind_childs()', then it's recommended to bind the device by call to * dm_scan_fdt_node() - this is usually done automatically for bus devices, * as a post bind method. + * + * Regulator get: * Having the device's name constraint, we can call regulator_by_platname(), - * to find interesting regulator. Before return, the regulator is probed, + * to find the required regulator. Before return, the regulator is probed, * and the rest of its constraints are put into the device's uclass platform * data, by the uclass regulator '.pre_probe' method. * @@ -201,8 +204,8 @@ struct dm_regulator_ops { /** * The 'get/set_mode()' function calls should operate on a driver- - * specific mode definitions, which should be found in: - * field 'mode' of struct mode_desc. + * specific mode id definitions, which should be found in: + * field 'id' of struct dm_regulator_mode. * * get/set_mode - get/set operation mode of the given output number * @dev - regulator device @@ -211,7 +214,7 @@ struct dm_regulator_ops { * @return id/0 for get/set on success or negative errno if fail. * Note: * The field 'id' of struct type 'dm_regulator_mode', should be always - * positive number, since the negative is reserved for the error. + * a positive number, since the negative is reserved for the error. */ int (*get_mode)(struct udevice *dev); int (*set_mode)(struct udevice *dev, int mode_id); @@ -278,107 +281,106 @@ bool regulator_get_enable(struct udevice *dev); int regulator_set_enable(struct udevice *dev, bool enable); /** - * regulator_get_mode: get mode of a given device regulator + * regulator_get_mode: get active operation mode id of a given regulator * * @dev - pointer to the regulator device - * @return - positive mode number on success or -errno val if fails + * @return - positive mode 'id' number on success or -errno val if fails * Note: - * The regulator driver should return one of defined, mode number rather, than - * the raw register value. The struct type 'mode_desc' provides a field 'mode' - * for this purpose and register_value for a raw register value. + * The device can provide an array of operating modes, which is type of struct + * dm_regulator_mode. Each mode has it's own 'id', which should be unique inside + * that array. By calling this function, the driver should return an active mode + * id of the given regulator device. */ int regulator_get_mode(struct udevice *dev); /** - * regulator_set_mode: set given regulator mode + * regulator_set_mode: set the given regulator's, active mode id * - * @dev - pointer to the regulator device - * @mode - mode type (field 'mode' of struct mode_desc) - * @return - 0 on success or -errno value if fails + * @dev - pointer to the regulator device + * @mode_id - mode id to set ('id' field of struct type dm_regulator_mode) + * @return - 0 on success or -errno value if fails * Note: - * The regulator driver should take one of defined, mode number rather - * than a raw register value. The struct type 'regulator_mode_desc' has - * a mode field for this purpose and register_value for a raw register value. + * The device can provide an array of operating modes, which is type of struct + * dm_regulator_mode. Each mode has it's own 'id', which should be unique inside + * that array. By calling this function, the driver should set the active mode + * of a given regulator to given by "mode_id" argument. */ -int regulator_set_mode(struct udevice *dev, int mode); +int regulator_set_mode(struct udevice *dev, int mode_id); /** - * regulator_by_platname_autoset_and_enable: setup the regulator given by - * its uclass's platform data '.name'. The setup depends on constraints found - * in device's uclass's platform data (struct dm_regulator_uclass_platdata): + * 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): + * - 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 - * - Enable - will set - if any of: '.always_on' or '.boot_on', is set to true * * The function returns on first encountered error. * * @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 + * @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_* - * For shorter call name, the below macro regulator_autoset() can be used. */ -int regulator_by_platname_autoset_and_enable(const char *platname, - struct udevice **devp, - bool verbose); - -#define regulator_autoset(platname, devp, verbose) \ - regulator_by_platname_autoset_and_enable(platname, devp, verbose) +int regulator_autoset(const char *platname, + struct udevice **devp, + bool verbose); /** - * regulator_by_platname_list_autoset_and_enable: setup the regulators given by - * list of its uclass's platform data '.name'. The setup depends on constraints - * found in device's uclass's platform data. The function loops with calls to: - * regulator_by_platname_autoset_and_enable() for each name of list. + * 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. * * @list_platname - an array of expected strings for .name field of each * regulator's uclass platdata - * @list_entries - number of regulator's name list entries * @list_devp - an array of returned pointers to the successfully setup * regulator devices if non-NULL passed * @verbose - (true/false) print each regulator setup info, or be quiet - * @return 0 on successfully setup of all list entries or 1 otwerwise. + * @return 0 on successfully setup of all list entries, otherwise first error. * * The returned 'regulator' devices can be used with: * - regulator_get/set_* - * For shorter call name, the below macro regulator_list_autoset() can be used. + * + * Note: The list must ends with NULL entry, like in the "platname" list below: + * char *my_regulators[] = { + * "VCC_3.3V", + * "VCC_1.8V", + * NULL, + * }; */ -int regulator_by_platname_list_autoset_and_enable(const char *list_platname[], - int list_entries, - struct udevice *list_devp[], - bool verbose); - -#define regulator_list_autoset(namelist, entries, devlist, verbose) \ - regulator_by_platname_list_autoset_and_enable(namelist, entries, \ - devlist, verbose) +int regulator_list_autoset(const char *list_platname[], + struct udevice *list_devp[], + bool verbose); /** - * regulator_by_devname: returns the pointer to the pmic regulator device. - * Search by name, found in regulator device's name. + * regulator_get_by_devname: returns the pointer to the pmic regulator device. + * Search by name, found in regulator device's name. * * @devname - expected string for 'dev->name' of regulator device * @devp - returned pointer to the regulator device * @return 0 on success or negative value of errno. * - * The returned 'regulator' device can be used with: + * The returned 'regulator' device is probed and can be used with: * - regulator_get/set_* */ -int regulator_by_devname(const char *devname, struct udevice **devp); +int regulator_get_by_devname(const char *devname, struct udevice **devp); /** - * regulator_by_platname: returns the pointer to the pmic regulator device. - * Search by name, found in regulator uclass platdata. + * regulator_get_by_platname: returns the pointer to the pmic regulator device. + * 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 * @return 0 on success or negative value of errno. * - * The returned 'regulator' device can be used with: + * The returned 'regulator' device is probed and can be used with: * - regulator_get/set_* */ -int regulator_by_platname(const char *platname, struct udevice **devp); +int regulator_get_by_platname(const char *platname, struct udevice **devp); #endif /* _INCLUDE_REGULATOR_H_ */