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

Przemyslaw Marczak
2015-04-03T16:09:26Z

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