Re: [U-Boot] [PATCH v4 05/16] dm: regulator: add implementation of driver model regulator uclass

Hello Simon,

On 04/22/2015 06:30 PM, Simon Glass wrote:

Hi Przemyslaw,

On 20 April 2015 at 12:07, Przemyslaw Marczak p.marczak@samsung.com 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 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

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@1", "LDO1", "LDOREG@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 p.marczak@samsung.com +# +# 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 p.marczak@samsung.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <dm.h> +#include <dm/uclass-internal.h> +#include <power/pmic.h> +#include <power/regulator.h>

+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 p.marczak@samsung.com
• • 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,