[U-Boot] [PATCH v4 05/16] dm: regulator: add implementation of driver model regulator uclass
Przemyslaw Marczak
p.marczak at samsung.com
Thu Apr 23 13:33:20 CEST 2015
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 at 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 at 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 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 <p.marczak at 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 at 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 at 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,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
More information about the U-Boot
mailing list