[U-Boot] [PATCH v3 06/17] dm: regulator: add implementation of driver model regulator uclass

[Przemyslaw Marczak <p.marczak@samsung.com>]

Hello Simon,

On 03/29/2015 03:07 PM, Simon Glass wrote:
> Hi Przemyslaw,
>
> On 24 March 2015 at 14:30, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>> This is the implementation of driver model regulator uclass api.
>> To use it, the CONFIG_DM_PMIC is required with driver implementation,
>> since it provides pmic devices basic I/O API.
>>
>> To get the regulator device:
>> - regulator_get()             - get the regulator device
>>
>> The regulator framework is based on a 'struct dm_regulator_ops'.
>> It provides a common function calls, for it's basic features:
>> - regulator_info()            - get the regulator info structure
>> - regulator_mode()            - get the regulator mode info structure
>> - regulator_get/set_value()   - get/set the regulator output voltage
>> - regulator_get/set_current() - get/set the regulator output current
>> - regulator_get/set_enable()  - get/set the regulator output enable state
>> - regulator_get/set_mode()    - get/set the regulator output operation mode
>>
>> An optional and useful regulator framework features are two descriptors:
>> - struct dm_regulator_info- describes the regulator name and output value limits
>>
>> - struct dm_regulator_mode - (array) describes the regulators operation modes
>>
>> The regulator framework features are described in file:
>> - include/power/regulator.h
>>
>> Main files:
>> - drivers/power/regulator-uclass.c - provides regulator common functions api
>> - include/power/regulator.h - define all structures required by the regulator
>>
>> Changes:
>> - new uclass-id: UCLASS_PMIC_REGULATOR
>> - new config: CONFIG_DM_REGULATOR
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> ---
>> Changes V2:
>> - new operations for regulator uclass:
>> -- get/set output state - for output on/off setting
>> --- add enum: REGULATOR_OFF, REGULATOR_ON
>>
>> - regulator uclass code rework and cleanup:
>> -- change name of:
>> --- enum 'regulator_desc_type' to 'regulator_type'
>> --- add type DVS
>> --- struct 'regulator_desc' to 'regulator_value_desc'
>>
>> -- regulator ops function calls:
>> --- remove 'ldo/buck' from naming
>> --- add new argument 'type' for define regulator type
>>
>> -- regulator.h - update comments
>>
>> Changes V3:
>> - regulator-uclass.c and regulator.h:
>>    -- api cleanup
>>    -- new function regulator_ofdata_to_platdata()
>>    -- update of comments
>>    -- add Kconfig
>> ---
>>   drivers/power/Kconfig            |  33 ++++-
>>   drivers/power/Makefile           |   1 +
>>   drivers/power/regulator-uclass.c | 219 +++++++++++++++++++++++++++++++++
>>   include/dm/uclass-id.h           |   1 +
>>   include/power/regulator.h        | 259 +++++++++++++++++++++++++++++++++++++++
>>   5 files changed, 512 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/power/regulator-uclass.c
>>   create mode 100644 include/power/regulator.h
>>
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index 3513b46..1e73c7a 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -66,7 +66,38 @@ config DM_PMIC
>>          So the call will looks like below:
>>          'pmic_write(regulator->parent, addr, value, len);'
>>
>> -config AXP221_POWER
>> +config DM_REGULATOR
>
> Can you move this to drivers/power/regulator?
>

Ok, will do this.

>> +       bool "Enable Driver Model for REGULATOR drivers (UCLASS_REGULATOR)"
>> +       depends on DM
>> +       ---help---
>> +       This config enables the driver-model regulator uclass support, which
>> +       provides implementation of driver model regulator uclass api.
>> +
>> +       Regulator uclass API calls:
>> +       To get the regulator device:
>> +       - regulator_get()             - get the regulator device
>> +
>> +       The regulator framework is based on a 'struct dm_regulator_ops'.
>> +       It provides a common function calls, for it's basic features:
>> +       - regulator_info()            - get the regulator info structure
>> +       - regulator_mode()            - get the regulator mode info structure
>> +       - regulator_get/set_value()   - operate on output voltage value
>> +       - regulator_get/set_current() - operate on output current value
>> +       - regulator_get/set_enable()  - operate on output enable state
>> +       - regulator_get/set_mode()    - operate on output operation mode
>> +
>> +       An optional and useful regulator framework features are two descriptors:
>> +       - struct dm_regulator_info - describes the regulator name and output limits
>> +       - struct dm_regulator_mode - describes the regulators operation mode
>> +
>> +       The regulator framework features are described in file:
>> +       - include/power/regulator.h
>> +
>> +       Main files:
>> +       - drivers/power/regulator-uclass.c - provides regulator common functions api
>> +       - include/power/regulator.h - define all structures required by the regulato
>> +
>> +       config AXP221_POWER
>
> I don't think this should be indented.
>

Right, will fix.

>>          boolean "axp221 / axp223 pmic support"
>>          depends on MACH_SUN6I || MACH_SUN8I
>>          default y
>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>> index 5c9a189..a6b7012 100644
>> --- a/drivers/power/Makefile
>> +++ b/drivers/power/Makefile
>> @@ -22,3 +22,4 @@ obj-$(CONFIG_POWER_FSL) += power_fsl.o
>>   obj-$(CONFIG_POWER_I2C) += power_i2c.o
>>   obj-$(CONFIG_POWER_SPI) += power_spi.o
>>   obj-$(CONFIG_DM_PMIC) += pmic-uclass.o
>> +obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
>> diff --git a/drivers/power/regulator-uclass.c b/drivers/power/regulator-uclass.c
>> new file mode 100644
>> index 0000000..b6a00c6
>> --- /dev/null
>> +++ b/drivers/power/regulator-uclass.c
>> @@ -0,0 +1,219 @@
>> +/*
>> + * Copyright (C) 2014-2015 Samsung Electronics
>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +#include <common.h>
>> +#include <linux/types.h>
>> +#include <fdtdec.h>
>> +#include <dm.h>
>> +#include <power/pmic.h>
>> +#include <power/regulator.h>
>> +#include <compiler.h>
>> +#include <dm/device.h>
>> +#include <dm/lists.h>
>> +#include <dm/device-internal.h>
>> +#include <errno.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +int regulator_info(struct udevice *dev, struct dm_regulator_info **infop)
>> +{
>> +       if (!dev || !dev->uclass_priv)
>> +               return -ENODEV;
>
> assert() would be OK here, but if you prefer this, OK.
>

Sorry, I should add this in this version - will fix.

>> +
>> +       *infop = dev->uclass_priv;
>> +
>> +       return 0;
>> +}
>> +
>> +int regulator_mode(struct udevice *dev, struct dm_regulator_mode **modep)
>> +{
>> +       struct dm_regulator_info *info;
>> +       int ret;
>> +
>> +       ret = regulator_info(dev, &info);
>> +       if (ret)
>> +               return ret;
>> +
>> +       *modep = info->mode;
>> +       return info->mode_count;
>> +}
>> +
>> +int regulator_get_value(struct udevice *dev)
>> +{
>> +       const struct dm_regulator_ops *ops;
>> +
>> +       ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
>
> Can you instead define regulator_get_uclass_ops()?
>

So maybe I will add this to uclass.c ?

>> +       if (!ops)
>> +               return -ENODEV;
>
> -ENOSYS
>
> We normally assume that operations existing, so assert() is OK. But if
> you are worried about this, then this is OK.
>

Yes, but the pmic_get_uclass_ops, can return NULL, so I would prefer to 
leave this null-checking here and also add the assert.

>> +
>> +       if (!ops->get_value)
>> +               return -EPERM;
>
> -ENOSYS
>
> i.e.
>
> if (!ops || !ops->get_value)
>     return -ENOSYS;
>
> Please fix below also.
>

Ok.

>> +
>> +       return ops->get_value(dev);
>> +}
>> +
>> +int regulator_set_value(struct udevice *dev, int uV)
>> +{
>> +       const struct dm_regulator_ops *ops;
>> +
>> +       ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
>> +       if (!ops)
>> +               return -ENODEV;
>> +
>> +       if (!ops->set_value)
>> +               return -EPERM;
>> +
>> +       return ops->set_value(dev, uV);
>> +}
>> +
>> +int regulator_get_current(struct udevice *dev)
>> +{
>> +       const struct dm_regulator_ops *ops;
>> +
>> +       ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
>> +       if (!ops)
>> +               return -ENODEV;
>> +
>> +       if (!ops->get_current)
>> +               return -EPERM;
>> +
>> +       return ops->get_current(dev);
>> +}
>> +
>> +int regulator_set_current(struct udevice *dev, int uA)
>> +{
>> +       const struct dm_regulator_ops *ops;
>> +
>> +       ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
>> +       if (!ops)
>> +               return -ENODEV;
>> +
>> +       if (!ops->set_current)
>> +               return -EPERM;
>> +
>> +       return ops->set_current(dev, uA);
>> +}
>> +
>> +bool regulator_get_enable(struct udevice *dev)
>> +{
>> +       const struct dm_regulator_ops *ops;
>> +
>> +       ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
>> +       if (!ops)
>> +               return -ENODEV;
>> +
>> +       if (!ops->get_enable)
>> +               return -EPERM;
>> +
>> +       return ops->get_enable(dev);
>> +}
>> +
>> +int regulator_set_enable(struct udevice *dev, bool enable)
>> +{
>> +       const struct dm_regulator_ops *ops;
>> +
>> +       ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
>> +       if (!ops)
>> +               return -ENODEV;
>> +
>> +       if (!ops->set_enable)
>> +               return -EPERM;
>> +
>> +       return ops->set_enable(dev, enable);
>> +}
>> +
>> +int regulator_get_mode(struct udevice *dev)
>> +{
>> +       const struct dm_regulator_ops *ops;
>> +
>> +       ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
>> +       if (!ops)
>> +               return -ENODEV;
>> +
>> +       if (!ops->get_mode)
>> +               return -EPERM;
>> +
>> +       return ops->get_mode(dev);
>> +}
>> +
>> +int regulator_set_mode(struct udevice *dev, int mode)
>> +{
>> +       const struct dm_regulator_ops *ops;
>> +
>> +       ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
>> +       if (!ops)
>> +               return -ENODEV;
>> +
>> +       if (!ops->set_mode)
>> +               return -EPERM;
>> +
>> +       return ops->set_mode(dev, mode);
>> +}
>> +
>> +int regulator_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct dm_regulator_info *info = dev->uclass_priv;
>> +       int offset = dev->of_offset;
>> +       int len;
>> +
>> +       /* Mandatory constraints */
>> +       info->name = strdup(fdt_getprop(gd->fdt_blob, offset,
>> +                                       "regulator-name", &len));
>> +       if (!info->name)
>> +               return -ENXIO;
>
> -ENOMEM
>
> But there is no need to strdup() this - the device tree does not move.
>

Ok, will fix.

>> +
>> +       info->min_uV = fdtdec_get_int(gd->fdt_blob, offset,
>> +                                     "regulator-min-microvolt", -1);
>> +       if (info->min_uV < 0)
>> +               return -ENXIO;
>> +
>> +       info->max_uV = fdtdec_get_int(gd->fdt_blob, offset,
>> +                                     "regulator-max-microvolt", -1);
>> +       if (info->max_uV < 0)
>> +               return -ENXIO;
>> +
>> +       /* Optional constraints */
>> +       info->min_uA = fdtdec_get_int(gd->fdt_blob, offset,
>> +                                     "regulator-min-microamp", -1);
>> +       info->max_uA = fdtdec_get_int(gd->fdt_blob, offset,
>> +                                     "regulator-max-microamp", -1);
>> +       info->always_on = fdtdec_get_bool(gd->fdt_blob, offset,
>> +                                        "regulator-always-on");
>> +       info->boot_on = fdtdec_get_bool(gd->fdt_blob, offset,
>> +                                        "regulator-boot-on");
>> +
>> +       return 0;
>> +}
>> +
>> +int regulator_get(char *name, struct udevice **devp)
>> +{
>> +       struct dm_regulator_info *info;
>> +       struct udevice *dev;
>> +       int ret;
>> +
>> +       *devp = NULL;
>> +
>> +       for (ret = uclass_first_device(UCLASS_REGULATOR, &dev);
>> +            dev;
>> +            ret = uclass_next_device(&dev)) {
>
> This will probe all devices. See my suggestion about creating
> uclass_find_first_device()/uclass_find_next_device() in the next
> patch.
>
> As before, I think this could use a function like uclass_get_device_by_name().
>

Yes, this is the same. But in this case, there is one more  issue, which 
is the regulator device name.
Usually after bind -> the dev->name is the same as node name. This is 
good, since it's natural that regulator IC, provides e.g "ldo1", or some 
other device-output name.
But we would like check the "regulator-name" property. For this 
patch-set, the name is fixed at device probe stage, when 
dev->ofdata_to_platdata() is called, and the regulator constraints, the 
same as it's name goes to struct dm_regulator_info.

I put the dm_regulator_info into uclass priv, because it it 
uclass-specific, the same as struct dm_i2c_bus is specific for i2c buses.

But, the ucalss priv is allocated at device probe stage.

I can't use the .per_child_platdata_auto_alloc_size, since the parent is 
a pmic, and its uclass type is different.

Actually I could, move the dm_regulator_info only to device platdata, 
but then, the drivers should take care of this uclass-specific structure 
allocation.
Is it acceptable?

But then, also ambiguous seem to be filling platdata (struct 
dm_regulator_info) in uclass post_bind() method.

So then, maybe reasonable is:
- move dm_regulator_info from dev->uclass_priv to dev->platdata - is 
allocated after device bind

- add .post_bind() method to regulator uclass, and get the 
"regulator-name" in it - only

- fill all platdata constraints on call to dev->ofdata_to_platdata()

Then, I could avoid probing every device, when checking the regulator 
name. But, still I can't use the uclass.c functions, since I'm checking 
the regulator info at dev->platdata.

So I update the function as below:

+ uclass_foreach_dev(dev, uc) {
+	if (!dev->platdata)
+		continue;
+
+	info = dev->platdata;
+	if (!strcmp(name, info->name)) {
+		ret = device_probe(dev);
+		if (ret)
+                              ....
+			*regulator = dev;
+			return ret;
+		}
+	}


>> +               info = dev->uclass_priv;
>> +               if (!info)
>> +                       continue;
>> +
>> +               if (!strcmp(name, info->name)) {
>> +                       *devp = dev;
>> +                       return ret;
>> +               }
>> +       }
>> +
>> +       return -ENODEV;
>> +}
>> +
>> +UCLASS_DRIVER(regulator) = {
>> +       .id             = UCLASS_REGULATOR,
>> +       .name           = "regulator",
>> +       .per_device_auto_alloc_size = sizeof(struct dm_regulator_info),
>> +};
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 3ecfa23..23356f3 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -37,6 +37,7 @@ enum uclass_id {
>>
>>          /* PMIC uclass and PMIC-related uclass types */
>>          UCLASS_PMIC,
>> +       UCLASS_REGULATOR,
>
> OK I see what the comment means now. I think it would be nice to have
> a comment here 'Voltage regulator'
>

Yes, will fix it.

>>
>>          UCLASS_COUNT,
>>          UCLASS_INVALID = -1,
>> diff --git a/include/power/regulator.h b/include/power/regulator.h
>> new file mode 100644
>> index 0000000..cf083c5
>> --- /dev/null
>> +++ b/include/power/regulator.h
>> @@ -0,0 +1,259 @@
>> +/*
>> + *  Copyright (C) 2014-2015 Samsung Electronics
>> + *  Przemyslaw Marczak <p.marczak@samsung.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef _INCLUDE_REGULATOR_H_
>> +#define _INCLUDE_REGULATOR_H_
>> +
>> +/* enum regulator_type - used for regulator_*() variant calls */
>> +enum regulator_type {
>> +       REGULATOR_TYPE_LDO = 0,
>> +       REGULATOR_TYPE_BUCK,
>> +       REGULATOR_TYPE_DVS,
>> +       REGULATOR_TYPE_FIXED,
>> +       REGULATOR_TYPE_OTHER,
>> +};
>> +
>> +/**
>> + * struct dm_regulator_mode - this structure holds an information about
>> + * each regulator operation mode. Probably in most cases - an array.
>> + * This will be probably a driver-static data, since it is device-specific.
>> + *
>> + * @id             - a driver-specific mode id
>> + * @register_value - a driver-specific value for its mode id
>> + * @name           - the name of mode - used for regulator command
>> + * Note:
>> + * The field 'id', should be always a positive number, since the negative values
>> + * are reserved for the errno numbers when returns the mode id.
>> + */
>> +struct dm_regulator_mode {
>> +       int id; /* Set only as >= 0 (negative value is reserved for errno) */
>> +       int register_value;
>> +       const char *name;
>> +};
>> +
>> +/**
>> + * struct dm_regulator_info - this structure holds an information about
>> + * each regulator constraints and supported operation modes. There is no "step"
>> + * voltage value - so driver should take care of this.
>> + *
>> + * @type       - one of 'enum regulator_type'
>> + * @mode       - pointer to the regulator mode (array if more than one)
>> + * @mode_count - number of '.mode' entries
>> + * @min_uV*    - minimum voltage (micro Volts)
>> + * @max_uV*    - maximum voltage (micro Volts)
>> + * @min_uA*    - minimum amperage (micro Amps)
>> + * @max_uA*    - maximum amperage (micro Amps)
>> + * @always_on* - bool type, true or false
>> + * @boot_on*   - bool type, true or false
>> + * @name*      - fdt regulator name - should be taken from the device tree
>> + *
>> + * Note: for attributes signed with '*'
>> + * These platform-specific constraints can be taken by regulator api function,
>> + * which is 'regulator_ofdata_to_platdata()'. Please read the description, which
>> + * can be found near the declaration of the mentioned function.
>> +*/
>> +struct dm_regulator_info {
>> +       enum regulator_type type;
>> +       struct dm_regulator_mode *mode;
>> +       int mode_count;
>> +       int min_uV;
>> +       int max_uV;
>> +       int min_uA;
>> +       int max_uA;
>> +       bool always_on;
>> +       bool boot_on;
>> +       const char *name;
>> +};
>> +
>> +/* PMIC regulator device operations */
>> +struct dm_regulator_ops {
>> +       /**
>> +        * The regulator output value function calls operates on a micro Volts.
>> +        *
>> +        * get/set_value - get/set output value of the given output number
>> +        * @dev          - regulator device
>> +        * Sets:
>> +        * @uV           - set the output value [micro Volts]
>> +        * Returns: output value [uV] on success or negative errno if fail.
>> +        */
>> +       int (*get_value)(struct udevice *dev);
>> +       int (*set_value)(struct udevice *dev, int uV);
>> +
>> +       /**
>> +        * The regulator output current function calls operates on a micro Amps.
>> +        *
>> +        * get/set_current - get/set output current of the given output number
>> +        * @dev            - regulator device
>> +        * Sets:
>> +        * @uA           - set the output current [micro Amps]
>> +        * Returns: output value [uA] on success or negative errno if fail.
>> +        */
>> +       int (*get_current)(struct udevice *dev);
>> +       int (*set_current)(struct udevice *dev, int uA);
>> +
>> +       /**
>> +        * The most basic feature of the regulator output is its enable state.
>> +        *
>> +        * get/set_enable - get/set enable state of the given output number
>> +        * @dev           - regulator device
>> +        * Sets:
>> +        * @enable         - set true - enable or false - disable
>> +        * Returns: true/false for get; or 0 / -errno for set.
>> +        */
>> +       bool (*get_enable)(struct udevice *dev);
>> +       int (*set_enable)(struct udevice *dev, bool enable);
>> +
>> +       /**
>> +        * The 'get/set_mode()' function calls should operate on a driver
>
> driver-
>
>> +        * specific mode definitions, which should be found in:
>> +        * field 'mode' of struct mode_desc.
>> +        *
>> +        * get/set_mode - get/set operation mode of the given output number
>> +        * @dev         - regulator device
>> +        * Sets
>> +        * @mode_id     - set output mode id (struct dm_regulator_mode->id)
>> +        * Returns: 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.
>> +        */
>> +       int (*get_mode)(struct udevice *dev);
>> +       int (*set_mode)(struct udevice *dev, int mode_id);
>> +};
>> +
>> +/**
>> + * regulator_info: returns a pointer to the devices regulator info structure
>> + *
>> + * @dev    - pointer to the regulator device
>> + * @infop  - pointer to the returned regulator info
>> + * Returns - 0 on success or negative value of errno.
>
> @return 0 on success...
>
>> + */
>> +int regulator_info(struct udevice *dev, struct dm_regulator_info **infop);
>> +
>> +/**
>> + * regulator_mode: returns a pointer to the array of regulator mode info
>> + *
>> + * @dev        - pointer to the regulator device
>> + * @modep      - pointer to the returned mode info array
>> + * Returns     - count of modep entries on success or negative errno if fail.
>> + */
>> +int regulator_mode(struct udevice *dev, struct dm_regulator_mode **modep);
>> +
>> +/**
>> + * regulator_get_value: get microvoltage voltage value of a given regulator
>> + *
>> + * @dev    - pointer to the regulator device
>> + * Returns - positive output value [uV] on success or negative errno if fail.
>> + */
>> +int regulator_get_value(struct udevice *dev);
>> +
>> +/**
>> + * regulator_set_value: set the microvoltage value of a given regulator.
>> + *
>> + * @dev    - pointer to the regulator device
>> + * @uV     - the output value to set [micro Volts]
>> + * Returns - 0 on success or -errno val if fails
>> + */
>> +int regulator_set_value(struct udevice *dev, int uV);
>> +
>> +/**
>> + * regulator_get_current: get microampere value of a given regulator
>> + *
>> + * @dev    - pointer to the regulator device
>> + * Returns - positive output current [uA] on success or negative errno if fail.
>> + */
>> +int regulator_get_current(struct udevice *dev);
>> +
>> +/**
>> + * regulator_set_current: set the microampere value of a given regulator.
>> + *
>> + * @dev    - pointer to the regulator device
>> + * @uA     - set the output current [micro Amps]
>> + * Returns - 0 on success or -errno val if fails
>> + */
>> +int regulator_set_current(struct udevice *dev, int uA);
>> +
>> +/**
>> + * regulator_get_enable: get regulator device enable state.
>> + *
>> + * @dev    - pointer to the regulator device
>> + * Returns - true/false of enable state
>> + */
>> +bool regulator_get_enable(struct udevice *dev);
>> +
>> +/**
>> + * regulator_set_enable: set regulator enable state
>> + *
>> + * @dev    - pointer to the regulator device
>> + * @enable - set true or false
>> + * Returns - 0 on success or -errno val if fails
>> + */
>> +int regulator_set_enable(struct udevice *dev, bool enable);
>> +
>> +/**
>> + * regulator_get_mode: get mode of a given device regulator
>> + *
>> + * @dev    - pointer to the regulator device
>> + * Returns - positive  mode 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.
>> + */
>> +int regulator_get_mode(struct udevice *dev);
>> +
>> +/**
>> + * regulator_set_mode: set given regulator mode
>> + *
>> + * @dev    - pointer to the regulator device
>> + * @mode   - mode type (field 'mode' of struct mode_desc)
>> + * Returns - 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.
>> + */
>> +int regulator_set_mode(struct udevice *dev, int mode);
>> +
>> +/**
>> + * regulator_ofdata_to_platdata: get the regulator constraints from its fdt node
>> + * and put it into the regulator 'dm_regulator_info' (dev->uclass_priv).
>> + *
>> + * An example of required regulator fdt node constraints:
>> + * ldo1 {
>> + *      regulator-compatible = "LDO1"; (not used here, but required for bind)
>> + *      regulator-name = "VDD_MMC_1.8V"; (mandatory)
>> + *      regulator-min-microvolt = <1000000>; (mandatory)
>> + *      regulator-max-microvolt = <1000000>; (mandatory)
>> + *      regulator-min-microamp = <1000>; (optional)
>> + *      regulator-max-microamp = <1000>; (optional)
>> + *      regulator-always-on; (optional)
>> + *      regulator-boot-on; (optional)
>> + * };
>> + *
>> + * @dev    - pointer to the regulator device
>> + * Returns - 0 on success or -errno value if fails
>> + * Note2:
>> + * This function can be called at stage in which 'dev->uclass_priv' is non-NULL.
>> + * It is possible at two driver call stages: '.ofdata_to_platdata' or '.probe'.
>> + */
>> +int regulator_ofdata_to_platdata(struct udevice *dev);
>> +
>> +/**
>> + * regulator_get: returns the pointer to the pmic regulator device based on
>> + * regulator fdt node data
>> + *
>> + * @fdt_name - property 'regulator-name' value of regulator fdt node
>> + * @devp     - returned pointer to the regulator device
>> + * Returns: 0 on success or negative value of errno.
>> + *
>> + * The returned 'regulator' device can be used with:
>> + * - regulator_get/set_*
>> + */
>> +int regulator_get(char *fdt_name, struct udevice **devp);
>
> Can we s/fdt_name/name/ since by now it is a device name, not a device
> tree name.

Right, thanks for catching this all.

>> +
>> +#endif /* _INCLUDE_REGULATOR_H_ */
>> --
>> 1.9.1
>>
>
> Regards,
> Simon
>

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com