From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Fri, 03 Apr 2015 18:09:26 +0200 Subject: [U-Boot] [PATCH v3 06/17] dm: regulator: add implementation of driver model regulator uclass In-Reply-To: References: <1425399883-14053-1-git-send-email-p.marczak@samsung.com> <1427229051-20170-1-git-send-email-p.marczak@samsung.com> <1427229051-20170-7-git-send-email-p.marczak@samsung.com> Message-ID: <551EBB36.7060409@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Simon, On 03/29/2015 03:07 PM, Simon Glass wrote: > Hi Przemyslaw, > > On 24 March 2015 at 14:30, Przemyslaw Marczak 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 >> --- >> 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 >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +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 >> + * >> + * 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