From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Thu, 23 Apr 2015 13:33:20 +0200 Subject: [U-Boot] [PATCH v4 05/16] dm: regulator: add implementation of driver model regulator uclass In-Reply-To: References: <1427229051-20170-1-git-send-email-p.marczak@samsung.com> <1429553273-6453-1-git-send-email-p.marczak@samsung.com> <1429553273-6453-6-git-send-email-p.marczak@samsung.com> Message-ID: <5538D880.1060604@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 04/22/2015 06:30 PM, Simon Glass wrote: > Hi Przemyslaw, > > On 20 April 2015 at 12:07, Przemyslaw Marczak wrote: >> This commit introduces the implementation of dm regulator API. >> Device tree support allows for auto binding. And by the basic >> uclass operations, it allows to driving the devices in a common >> way. For detailed informations, please look into the header file. >> >> Core 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_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 >> >> Changes V4: >> - move file drivers/power/regulator-uclass.c to >> drivers/power/regulator/regulator-uclass.c >> - move DM_REGULATOR Kconfig entry from: drivers/power/Kconfig to >> drivers/power/regulator/Kconfig >> - drivers/power/Kconfig: include regulator Kconfig path >> - Kconfig: provide only general informations >> - regulator-uclass.c: cleanup >> - regulator-uclass.c: allow init regulator with name only >> - regulator-uclass.c: remove pmic_get_uclass_ops and use dev_get_driver_ops >> - regulator-uclass.c: add use of uclass_get_device_by_name() >> - regulator.h: add 'struct dm_regulator_uclass_platdata' >> - regulator.h: API documentation cleanup >> - regulator - add binding info >> >> --- >> Makefile | 3 +- >> doc/device-tree-bindings/regulator/regulator.txt | 55 ++++ >> drivers/power/Kconfig | 2 + >> drivers/power/regulator/Kconfig | 17 + >> drivers/power/regulator/Makefile | 8 + >> drivers/power/regulator/regulator-uclass.c | 300 ++++++++++++++++++ >> include/dm/uclass-id.h | 1 + >> include/power/regulator.h | 384 +++++++++++++++++++++++ >> 8 files changed, 769 insertions(+), 1 deletion(-) >> create mode 100644 doc/device-tree-bindings/regulator/regulator.txt >> create mode 100644 drivers/power/regulator/Kconfig >> create mode 100644 drivers/power/regulator/Makefile >> create mode 100644 drivers/power/regulator/regulator-uclass.c >> create mode 100644 include/power/regulator.h > > A few more nits for later. > >> >> diff --git a/Makefile b/Makefile >> index dc25f70..dfb0d56 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -645,7 +645,8 @@ libs-y += drivers/power/ \ >> drivers/power/fuel_gauge/ \ >> drivers/power/mfd/ \ >> drivers/power/pmic/ \ >> - drivers/power/battery/ >> + drivers/power/battery/ \ >> + drivers/power/regulator/ >> libs-y += drivers/spi/ >> libs-$(CONFIG_FMAN_ENET) += drivers/net/fm/ >> libs-$(CONFIG_SYS_FSL_DDR) += drivers/ddr/fsl/ >> diff --git a/doc/device-tree-bindings/regulator/regulator.txt b/doc/device-tree-bindings/regulator/regulator.txt >> new file mode 100644 >> index 0000000..249e0f5 >> --- /dev/null >> +++ b/doc/device-tree-bindings/regulator/regulator.txt >> @@ -0,0 +1,55 @@ >> +Voltage/Current regulator >> + >> +Binding: >> +The regulator devices don't use the "compatible" property. The binding is done >> +by the prefix of regulator node's name. Usually the pmic I/O driver will provide >> +the array of 'struct pmic_child_info' with the prefixes and compatible drivers. >> +The bind is done by calling function: pmic_bind_childs(). >> +Example drivers: >> +pmic: drivers/power/pmic/max77686.c >> +regulator: drivers/power/regulator/max77686.c >> + >> +For the node name e.g.: "prefix[:alpha:]num { ... }": >> +- the driver prefix should be: "prefix" or "PREFIX" - case insensitive >> +- the node name's "num" is set as "dev->driver_data" on bind >> + >> +Example the prefix "ldo" will pass for: "ldo1", "ldo at 1", "LDO1", "LDOREG at 1"... >> + >> +Required properties: >> +- regulator-name: a string, required by the regulator uclass >> + >> +Note >> +The "regulator-name" constraint is used for setting the device's uclass >> +platform data '.name' field. And the regulator device name is set from >> +it's node name. >> + >> +Optional properties: >> +- regulator-min-microvolt: a minimum allowed Voltage value >> +- regulator-max-microvolt: a maximum allowed Voltage value >> +- regulator-min-microamp: a minimum allowed Current value >> +- regulator-max-microamp: a maximum allowed Current value >> +- regulator-always-on: regulator should never be disabled >> +- regulator-boot-on: enabled by bootloader/firmware >> + >> +Other kernel-style properties, are currently not used. >> + >> +Note: >> +For the regulator autoset from constraints, the framework expects that: >> +- regulator-min-microvolt is equal to regulator-max-microvolt >> +- regulator-min-microamp is equal to regulator-max-microamp >> +- regulator-always-on or regulator-boot-on is set >> + >> +Example: >> +ldo0 { >> + /* Mandatory */ >> + regulator-name = "VDDQ_EMMC_1.8V"; >> + >> + /* Optional */ >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + regulator-min-microamp = <100000>; >> + regulator-max-microamp = <100000>; >> + regulator-always-on; >> + regulator-boot-on; >> +}; >> + >> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig >> index d03626e..23cdd71 100644 >> --- a/drivers/power/Kconfig >> +++ b/drivers/power/Kconfig >> @@ -2,6 +2,8 @@ menu "Power" >> >> source "drivers/power/pmic/Kconfig" >> >> +source "drivers/power/regulator/Kconfig" >> + >> config AXP221_POWER >> boolean "axp221 / axp223 pmic support" >> depends on MACH_SUN6I || MACH_SUN8I >> diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig >> new file mode 100644 >> index 0000000..cb15162 >> --- /dev/null >> +++ b/drivers/power/regulator/Kconfig >> @@ -0,0 +1,17 @@ >> +config DM_REGULATOR >> + bool "Enable Driver Model for REGULATOR drivers (UCLASS_REGULATOR)" >> + depends on DM >> + ---help--- >> + This config enables the driver model regulator support. >> + UCLASS_REGULATOR - designed to provide a common API for basic regulator's >> + functions, like get/set Voltage or Current value, enable state, etc... >> + Note: >> + When enabling this, please read the description, found in the files: >> + - 'include/power/pmic.h' >> + - 'include/power/regulator.h' >> + - 'drivers/power/pmic/pmic-uclass.c' >> + - 'drivers/power/pmic/regulator-uclass.c' >> + 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. >> diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile >> new file mode 100644 >> index 0000000..27c9006 >> --- /dev/null >> +++ b/drivers/power/regulator/Makefile >> @@ -0,0 +1,8 @@ >> +# >> +# Copyright (C) 2015 Samsung Electronics >> +# Przemyslaw Marczak >> +# >> +# SPDX-License-Identifier: GPL-2.0+ >> +# >> + >> +obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o >> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c >> new file mode 100644 >> index 0000000..07ce286 >> --- /dev/null >> +++ b/drivers/power/regulator/regulator-uclass.c >> @@ -0,0 +1,300 @@ >> +/* >> + * Copyright (C) 2014-2015 Samsung Electronics >> + * Przemyslaw Marczak >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +DECLARE_GLOBAL_DATA_PTR; >> + >> +int regulator_mode(struct udevice *dev, struct dm_regulator_mode **modep) >> +{ >> + struct dm_regulator_uclass_platdata *uc_pdata; >> + >> + *modep = NULL; >> + >> + uc_pdata = dev_get_uclass_platdata(dev); >> + if (!uc_pdata) >> + return -ENXIO; >> + >> + *modep = uc_pdata->mode; >> + return uc_pdata->mode_count; >> +} >> + >> +int regulator_get_value(struct udevice *dev) >> +{ >> + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); >> + >> + if (!ops || !ops->get_value) >> + return -ENOSYS; >> + >> + return ops->get_value(dev); >> +} >> + >> +int regulator_set_value(struct udevice *dev, int uV) >> +{ >> + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); >> + >> + if (!ops || !ops->set_value) >> + return -ENOSYS; >> + >> + return ops->set_value(dev, uV); >> +} >> + >> +int regulator_get_current(struct udevice *dev) >> +{ >> + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); >> + >> + if (!ops || !ops->get_current) >> + return -ENOSYS; >> + >> + return ops->get_current(dev); >> +} >> + >> +int regulator_set_current(struct udevice *dev, int uA) >> +{ >> + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); >> + >> + if (!ops || !ops->set_current) >> + return -ENOSYS; >> + >> + return ops->set_current(dev, uA); >> +} >> + >> +bool regulator_get_enable(struct udevice *dev) >> +{ >> + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); >> + >> + if (!ops || !ops->get_enable) >> + return -ENOSYS; >> + >> + return ops->get_enable(dev); >> +} >> + >> +int regulator_set_enable(struct udevice *dev, bool enable) >> +{ >> + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); >> + >> + if (!ops || !ops->set_enable) >> + return -ENOSYS; >> + >> + return ops->set_enable(dev, enable); >> +} >> + >> +int regulator_get_mode(struct udevice *dev) >> +{ >> + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); >> + >> + if (!ops || !ops->get_mode) >> + return -ENOSYS; >> + >> + return ops->get_mode(dev); >> +} >> + >> +int regulator_set_mode(struct udevice *dev, int mode) >> +{ >> + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); >> + >> + if (!ops || !ops->set_mode) >> + return -ENOSYS; >> + >> + return ops->set_mode(dev, mode); >> +} >> + >> +int regulator_by_platname(const char *plat_name, struct udevice **devp) > > regulator_get_by_platname() > > since it does probe the device > Ok. >> +{ >> + struct dm_regulator_uclass_platdata *uc_pdata; >> + struct udevice *dev; >> + >> + *devp = NULL; >> + >> + for (uclass_find_first_device(UCLASS_REGULATOR, &dev); >> + dev; >> + uclass_find_next_device(&dev)) { >> + uc_pdata = dev_get_uclass_platdata(dev); >> + if (!uc_pdata || strcmp(plat_name, uc_pdata->name)) >> + continue; >> + >> + return uclass_get_device_tail(dev, 0, devp); >> + } >> + >> + debug("%s: can't find: %s\n", __func__, plat_name); >> + >> + return -ENODEV; >> +} >> + >> +int regulator_by_devname(const char *devname, struct udevice **devp) > > regulator_get_by_devname > Ok. >> +{ >> + return uclass_get_device_by_name(UCLASS_REGULATOR, devname, devp); >> +} >> + >> +static int setting_failed(int ret, bool verbose, const char *fmt, ...) >> +{ >> + va_list args; >> + char buf[64]; >> + >> + if (verbose == false) >> + return ret; >> + >> + va_start(args, fmt); >> + vscnprintf(buf, sizeof(buf), fmt, args); >> + va_end(args); >> + >> + printf(buf); > > I wonder if we should have a vprintf() in U-Boot? > Yes, it could be useful. >> + >> + if (!ret) >> + return 0; >> + >> + printf(" (ret: %d)", ret); >> + >> + return ret; >> +} >> + >> +int regulator_by_platname_autoset_and_enable(const char *platname, >> + struct udevice **devp, >> + bool verbose) >> +{ >> + struct dm_regulator_uclass_platdata *uc_pdata; >> + struct udevice *dev; >> + bool v = verbose; > > Can we drop this local var and just use 'verbose' > I added this to avoid breaking the line of call to setting_failed(). So I will use just failed(), and then can put the verbose with no line breaking inside 'if' brackets. >> + int ret; >> + >> + if (devp) >> + *devp = NULL; >> + >> + ret = regulator_by_platname(platname, &dev); >> + if (ret) { >> + error("Can get the regulator: %s!", platname); >> + return ret; >> + } >> + >> + uc_pdata = dev_get_uclass_platdata(dev); >> + if (!uc_pdata) { >> + error("Can get the regulator %s uclass platdata!", platname); >> + return -ENXIO; >> + } >> + >> + if (v) >> + printf("%s@%s: ", dev->name, uc_pdata->name); >> + >> + /* Those values are optional (-ENODATA if unset) */ >> + if ((uc_pdata->min_uV != -ENODATA) && >> + (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)) >> + goto exit; >> + } >> + >> + /* Those values are optional (-ENODATA if unset) */ >> + if ((uc_pdata->min_uA != -ENODATA) && >> + (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)) >> + 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)) >> + goto exit; >> + >> +retdev: >> + if (devp) >> + *devp = dev; >> +exit: >> + if (v) >> + 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) > > I wonder if you could shorten this (e.g. to regulator_list_autoset())? > I introduced macro: "regulator_list_autoset()" in include/power/regulator.h. Is it good? >> +{ >> + struct udevice *dev; >> + int i, ret, success = 0; >> + >> + for (i = 0; i < list_entries; i++) { >> + ret = regulator_autoset(list_platname[i], &dev, verbose); >> + if (!ret) >> + success++; >> + >> + if (!list_devp) >> + continue; >> + >> + if (ret) >> + list_devp[i] = NULL; >> + else >> + list_devp[i] = dev; > > Shouldn't dev be NULL if ret is non-zero anyway? > Right, it always be if given. Will fix. >> + } >> + >> + return (success != list_entries); > > return success == list_entries ? 0 : -EIO > > or something. Better would be to record the first error you get and return that. > Ok, I will tune it. >> +} >> + >> +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; >> + >> + 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); >> + if (!uc_pdata->name) { >> + debug("%s: dev: %s has no property 'regulator-name'\n", >> + __func__, dev->name); >> + return -ENXIO; > > -EINVAL as it indicates invalid device tree data. > Ok. >> + } >> + >> + return 0; >> +} >> + >> +static int regulator_pre_probe(struct udevice *dev) >> +{ >> + struct dm_regulator_uclass_platdata *uc_pdata; >> + int offset = dev->of_offset; >> + >> + uc_pdata = dev_get_uclass_platdata(dev); >> + if (!uc_pdata) >> + return -ENXIO; >> + >> + /* Regulator's optional constraints */ >> + uc_pdata->min_uV = fdtdec_get_int(gd->fdt_blob, offset, >> + "regulator-min-microvolt", -ENODATA); >> + uc_pdata->max_uV = fdtdec_get_int(gd->fdt_blob, offset, >> + "regulator-max-microvolt", -ENODATA); >> + uc_pdata->min_uA = fdtdec_get_int(gd->fdt_blob, offset, >> + "regulator-min-microamp", -ENODATA); >> + uc_pdata->max_uA = fdtdec_get_int(gd->fdt_blob, offset, >> + "regulator-max-microamp", -ENODATA); >> + uc_pdata->always_on = fdtdec_get_bool(gd->fdt_blob, offset, >> + "regulator-always-on"); >> + uc_pdata->boot_on = fdtdec_get_bool(gd->fdt_blob, offset, >> + "regulator-boot-on"); >> + >> + return 0; >> +} >> + >> +UCLASS_DRIVER(regulator) = { >> + .id = UCLASS_REGULATOR, >> + .name = "regulator", >> + .post_bind = regulator_post_bind, >> + .pre_probe = regulator_pre_probe, >> + .per_device_platdata_auto_alloc_size = >> + sizeof(struct dm_regulator_uclass_platdata), >> +}; >> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h >> index 23b3eb9..3c572d7 100644 >> --- a/include/dm/uclass-id.h >> +++ b/include/dm/uclass-id.h >> @@ -48,6 +48,7 @@ enum uclass_id { >> >> /* Power Management */ >> UCLASS_PMIC, /* PMIC I/O device */ >> + UCLASS_REGULATOR, /* REGULATOR device */ >> >> UCLASS_COUNT, >> UCLASS_INVALID = -1, >> diff --git a/include/power/regulator.h b/include/power/regulator.h >> new file mode 100644 >> index 0000000..0302c1d >> --- /dev/null >> +++ b/include/power/regulator.h >> @@ -0,0 +1,384 @@ >> +/* >> + * Copyright (C) 2014-2015 Samsung Electronics >> + * Przemyslaw Marczak >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#ifndef _INCLUDE_REGULATOR_H_ >> +#define _INCLUDE_REGULATOR_H_ >> + >> +/** >> + * U-Boot Voltage/Current Regulator >> + * ================================ >> + * >> + * The regulator API is based on a driver model, with the device tree support. >> + * And this header describes the functions and data types for the uclass id: >> + * 'UCLASS_REGULATOR' and the regulator driver API. >> + * >> + * The regulator uclass - is based on uclass platform data which is allocated, >> + * automatically for each regulator device on bind and 'dev->uclass_platdata' >> + * points to it. The data type is: 'struct dm_regulator_uclass_platdata'. >> + * The uclass file: 'drivers/power/regulator/regulator-uclass.c' >> + * >> + * The regulator device - is based on driver's model 'struct udevice'. >> + * The API can use regulator name in two meanings: >> + * - devname - the regulator device's name: 'dev->name' >> + * - platname - the device's platdata's name. So in the code it looks like: >> + * 'uc_pdata = dev->uclass_platdata'; 'name = uc_pdata->name'. >> + * >> + * The regulator device driver - provide an implementation of uclass operations >> + * pointed by 'dev->driver->ops' as a struct of type 'struct dm_regulator_ops'. >> + * >> + * To proper bind the regulator device, the device tree node should provide >> + * regulator constraints, like in the example below: >> + * >> + * ldo1 { >> + * regulator-name = "VDD_MMC_1.8V"; (mandatory for bind) >> + * regulator-min-microvolt = <1000000>; (optional) >> + * regulator-max-microvolt = <1000000>; (optional) >> + * regulator-min-microamp = <1000>; (optional) >> + * regulator-max-microamp = <1000>; (optional) >> + * regulator-always-on; (optional) >> + * regulator-boot-on; (optional) >> + * }; >> + * >> + * Please take a notice, that for the proper operation at least name constraint > > Please note that for the proper > > or > > Note: For the proper > Ok. >> + * is needed, e.g. for call the device_by_platname(...). >> + * >> + * 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. > > s/do/does/ > Ok. >> + * 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. >> + * Having the device's name constraint, we can call regulator_by_platname(), >> + * to find interesting regulator. Before return, the regulator is probed, > > How about: > > s/to find interesting regulator/to find the required regulator/ > Ok. >> + * and the rest of its constraints are put into the device's uclass platform >> + * data, by the uclass regulator '.pre_probe' method. >> + * >> + * For more info about PMIC bind, please refer to file: 'include/power/pmic.h' >> + * >> + * Note: >> + * Please do not use the device_bind_by_name() function, since it pass '-1' as >> + * device node offset - and the bind will fail on uclass .post_bind method, >> + * because of missing 'regulator-name' constraint. > > Indeed. BTW I think i'm going to add a similar function which allows > the node to be passed. > That's great, would be usefull. >> + * >> + * >> + * Fixed Voltage/Current Regulator >> + * =============================== >> + * >> + * When fixed voltage regulator is needed, then enable the config: >> + * - CONFIG_DM_REGULATOR_FIXED >> + * >> + * The driver file: 'drivers/power/regulator/fixed.c', provides basic support >> + * for control the GPIO, and return the device tree constraint values. >> + * >> + * To bind the fixed voltage regulator device, we usually use a 'simple-bus' >> + * node as a parent. And 'regulator-fixed' for the driver compatible. This is >> + * the same as in the kernel. The example node of fixed regulator: >> + * >> + * simple-bus { >> + * compatible = "simple-bus"; >> + * #address-cells = <1>; >> + * #size-cells = <0>; >> + * >> + * blue_led { >> + * compatible = "regulator-fixed"; >> + * regulator-name = "VDD_LED_3.3V"; >> + * regulator-min-microvolt = <3300000>; >> + * regulator-max-microvolt = <3300000>; >> + * gpio = <&gpc1 0 GPIO_ACTIVE_LOW>; >> + * }; >> + * }; >> + * >> + * The fixed regulator devices also provide regulator uclass platform data. And >> + * devices bound from such node, can use the regulator drivers API. >> +*/ >> + >> +/* 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_uclass_platdata - pointed by dev->uclass_platdata, and >> + * allocated on each regulator bind. This structure holds an information >> + * about each regulator's 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: >> + * * - set automatically on device probe by the uclass's '.pre_probe' method. >> + * ** - set automatically on device bind by the uclass's '.post_bind' method. >> + * The constraints: type, mode, mode_count, can be set by device driver, e.g. >> + * by the driver '.probe' method. >> + */ >> +struct dm_regulator_uclass_platdata { >> + 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; >> +}; >> + >> +/* 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 >> + * 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_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. > > Can you please reword the first sentence? I don't understand what it > is trying to say. Similar below. > Ok, will fix this. >> + */ >> +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_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): >> + * - 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 '.always_on' or '.boot_on' are 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 >> + * Returns: 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) >> + > > Can we just use the shorter name for the function and avoid this #ifdef? > Ok. >> +/** >> + * 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. >> + * >> + * @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 >> + * Returns: 0 on successfully setup of all list entries or 1 otwerwise. > > otherwise > >> + * >> + * The returned 'regulator' devices can be used with: >> + * - regulator_get/set_* >> + * For shorter call name, the below macro regulator_list_autoset() can be used. >> + */ >> +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) > > As above. With #defines it changes the name in the map output or > debugger, which confuses people. An inline function is better, but in > this case it may be better to just rename the function. > Okay, will fix both. >> + >> +/** >> + * regulator_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 >> + * Returns: 0 on success or negative value of errno. >> + * >> + * The returned 'regulator' device can be used with: >> + * - regulator_get/set_* >> + */ >> +int regulator_by_devname(const char *devname, struct udevice **devp); > > regulator_get_by_devname > >> + >> +/** >> + * regulator_by_platname: returns the pointer to the pmic regulator device. >> + * Search by name, found in regulator uclass platdata. >> + * >> + * @platname - expected string for dm_regulator_uclass_platdata .name field >> + * @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_by_platname(const char *platname, struct udevice **devp); > > regulator_get_by_platname > Also, will fix both. >> + >> +#endif /* _INCLUDE_REGULATOR_H_ */ >> -- >> 1.9.1 >> > > Regards, > Simon > Thank you again, will resend ASAP. Best regards, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com