dm: regulator: uclass driver code cleanup
authorPrzemyslaw Marczak <p.marczak@samsung.com>
Wed, 13 May 2015 11:38:27 +0000 (13:38 +0200)
committerSimon Glass <sjg@chromium.org>
Fri, 15 May 2015 01:58:34 +0000 (19:58 -0600)
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
- odroid u3: cleanup the regulator calls

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Acked-by: Simon Glass <sjg@chromium.org>
Tested on sandbox:
Tested-by: Simon Glass <sjg@chromium.org>
board/samsung/odroid/odroid.c
drivers/power/regulator/Kconfig
drivers/power/regulator/regulator-uclass.c
include/power/regulator.h

index 29de325aff03e752c73c741ab2a7bf3ea6d50feb..32155f11844ac495921d0785f96eeccebd3695ab 100644 (file)
@@ -37,6 +37,7 @@ static const char *mmc_regulators[] = {
        "VDDQ_EMMC_1.8V",
        "VDDQ_EMMC_2.8V",
        "TFLASH_2.8V",
+       NULL,
 };
 
 void set_board_type(void)
@@ -427,9 +428,7 @@ int exynos_init(void)
 
 int exynos_power_init(void)
 {
-       int list_count = ARRAY_SIZE(mmc_regulators);
-
-       if (regulator_list_autoset(mmc_regulators, list_count, NULL, true))
+       if (regulator_list_autoset(mmc_regulators, NULL, true))
                error("Unable to init all mmc regulators");
 
        return 0;
@@ -441,7 +440,7 @@ static int s5pc210_phy_control(int on)
        struct udevice *dev;
        int ret;
 
-       ret = regulator_by_platname("VDD_UOTG_3.0V", &dev);
+       ret = regulator_get_by_platname("VDD_UOTG_3.0V", &dev);
        if (ret) {
                error("Regulator get error: %d", ret);
                return ret;
@@ -487,7 +486,7 @@ int board_usb_init(int index, enum usb_init_type init)
        /* Power off and on BUCK8 for LAN9730 */
        debug("LAN9730 - Turning power buck 8 OFF and ON.\n");
 
-       ret = regulator_by_platname("VCC_P3V3_2.85V", &dev);
+       ret = regulator_get_by_platname("VCC_P3V3_2.85V", &dev);
        if (ret) {
                error("Regulator get error: %d", ret);
                return ret;
index 54ce188743c80765bbc1c8f217199b5dc95387a2..fd3cf351b1b86c2fc245aee2834edb389d6fec85 100644 (file)
@@ -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"
index 07ce2869e6f4603f7c74b718ec667aaa84881f60..31ffd44454cf30e5d0b51b3c57799b4c21a8e802 100644 (file)
@@ -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)
index 6916660255d38b2ee141ae0c508b4bade222b8a0..03a2cefcd61bc4a56d7070d9756c41623b6244a3 100644 (file)
@@ -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)
  *      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.
+        * 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_ */