[U-Boot] [PATCH v3 06/17] dm: regulator: add implementation of driver model regulator uclass
Simon Glass
sjg at chromium.org
Sun Mar 29 15:07:47 CEST 2015
Hi Przemyslaw,
On 24 March 2015 at 14:30, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
> This is the implementation of driver model regulator uclass api.
> To use it, the CONFIG_DM_PMIC is required with driver implementation,
> since it provides pmic devices basic I/O API.
>
> To get the regulator device:
> - regulator_get() - get the regulator device
>
> The regulator framework is based on a 'struct dm_regulator_ops'.
> It provides a common function calls, for it's basic features:
> - regulator_info() - get the regulator info structure
> - regulator_mode() - get the regulator mode info structure
> - regulator_get/set_value() - get/set the regulator output voltage
> - regulator_get/set_current() - get/set the regulator output current
> - regulator_get/set_enable() - get/set the regulator output enable state
> - regulator_get/set_mode() - get/set the regulator output operation mode
>
> An optional and useful regulator framework features are two descriptors:
> - struct dm_regulator_info- describes the regulator name and output value limits
>
> - struct dm_regulator_mode - (array) describes the regulators operation modes
>
> The regulator framework features are described in file:
> - include/power/regulator.h
>
> Main files:
> - drivers/power/regulator-uclass.c - provides regulator common functions api
> - include/power/regulator.h - define all structures required by the regulator
>
> Changes:
> - new uclass-id: UCLASS_PMIC_REGULATOR
> - new config: CONFIG_DM_REGULATOR
>
> Signed-off-by: Przemyslaw Marczak <p.marczak 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
> ---
> drivers/power/Kconfig | 33 ++++-
> drivers/power/Makefile | 1 +
> drivers/power/regulator-uclass.c | 219 +++++++++++++++++++++++++++++++++
> include/dm/uclass-id.h | 1 +
> include/power/regulator.h | 259 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 512 insertions(+), 1 deletion(-)
> create mode 100644 drivers/power/regulator-uclass.c
> create mode 100644 include/power/regulator.h
>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 3513b46..1e73c7a 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -66,7 +66,38 @@ config DM_PMIC
> So the call will looks like below:
> 'pmic_write(regulator->parent, addr, value, len);'
>
> -config AXP221_POWER
> +config DM_REGULATOR
Can you move this to drivers/power/regulator?
> + bool "Enable Driver Model for REGULATOR drivers (UCLASS_REGULATOR)"
> + depends on DM
> + ---help---
> + This config enables the driver-model regulator uclass support, which
> + provides implementation of driver model regulator uclass api.
> +
> + Regulator uclass API calls:
> + To get the regulator device:
> + - regulator_get() - get the regulator device
> +
> + The regulator framework is based on a 'struct dm_regulator_ops'.
> + It provides a common function calls, for it's basic features:
> + - regulator_info() - get the regulator info structure
> + - regulator_mode() - get the regulator mode info structure
> + - regulator_get/set_value() - operate on output voltage value
> + - regulator_get/set_current() - operate on output current value
> + - regulator_get/set_enable() - operate on output enable state
> + - regulator_get/set_mode() - operate on output operation mode
> +
> + An optional and useful regulator framework features are two descriptors:
> + - struct dm_regulator_info - describes the regulator name and output limits
> + - struct dm_regulator_mode - describes the regulators operation mode
> +
> + The regulator framework features are described in file:
> + - include/power/regulator.h
> +
> + Main files:
> + - drivers/power/regulator-uclass.c - provides regulator common functions api
> + - include/power/regulator.h - define all structures required by the regulato
> +
> + config AXP221_POWER
I don't think this should be indented.
> boolean "axp221 / axp223 pmic support"
> depends on MACH_SUN6I || MACH_SUN8I
> default y
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 5c9a189..a6b7012 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_POWER_FSL) += power_fsl.o
> obj-$(CONFIG_POWER_I2C) += power_i2c.o
> obj-$(CONFIG_POWER_SPI) += power_spi.o
> obj-$(CONFIG_DM_PMIC) += pmic-uclass.o
> +obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
> diff --git a/drivers/power/regulator-uclass.c b/drivers/power/regulator-uclass.c
> new file mode 100644
> index 0000000..b6a00c6
> --- /dev/null
> +++ b/drivers/power/regulator-uclass.c
> @@ -0,0 +1,219 @@
> +/*
> + * Copyright (C) 2014-2015 Samsung Electronics
> + * Przemyslaw Marczak <p.marczak at samsung.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +#include <common.h>
> +#include <linux/types.h>
> +#include <fdtdec.h>
> +#include <dm.h>
> +#include <power/pmic.h>
> +#include <power/regulator.h>
> +#include <compiler.h>
> +#include <dm/device.h>
> +#include <dm/lists.h>
> +#include <dm/device-internal.h>
> +#include <errno.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int regulator_info(struct udevice *dev, struct dm_regulator_info **infop)
> +{
> + if (!dev || !dev->uclass_priv)
> + return -ENODEV;
assert() would be OK here, but if you prefer this, OK.
> +
> + *infop = dev->uclass_priv;
> +
> + return 0;
> +}
> +
> +int regulator_mode(struct udevice *dev, struct dm_regulator_mode **modep)
> +{
> + struct dm_regulator_info *info;
> + int ret;
> +
> + ret = regulator_info(dev, &info);
> + if (ret)
> + return ret;
> +
> + *modep = info->mode;
> + return info->mode_count;
> +}
> +
> +int regulator_get_value(struct udevice *dev)
> +{
> + const struct dm_regulator_ops *ops;
> +
> + ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
Can you instead define regulator_get_uclass_ops()?
> + if (!ops)
> + return -ENODEV;
-ENOSYS
We normally assume that operations existing, so assert() is OK. But if
you are worried about this, then this is OK.
> +
> + if (!ops->get_value)
> + return -EPERM;
-ENOSYS
i.e.
if (!ops || !ops->get_value)
return -ENOSYS;
Please fix below also.
> +
> + return ops->get_value(dev);
> +}
> +
> +int regulator_set_value(struct udevice *dev, int uV)
> +{
> + const struct dm_regulator_ops *ops;
> +
> + ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
> + if (!ops)
> + return -ENODEV;
> +
> + if (!ops->set_value)
> + return -EPERM;
> +
> + return ops->set_value(dev, uV);
> +}
> +
> +int regulator_get_current(struct udevice *dev)
> +{
> + const struct dm_regulator_ops *ops;
> +
> + ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
> + if (!ops)
> + return -ENODEV;
> +
> + if (!ops->get_current)
> + return -EPERM;
> +
> + return ops->get_current(dev);
> +}
> +
> +int regulator_set_current(struct udevice *dev, int uA)
> +{
> + const struct dm_regulator_ops *ops;
> +
> + ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
> + if (!ops)
> + return -ENODEV;
> +
> + if (!ops->set_current)
> + return -EPERM;
> +
> + return ops->set_current(dev, uA);
> +}
> +
> +bool regulator_get_enable(struct udevice *dev)
> +{
> + const struct dm_regulator_ops *ops;
> +
> + ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
> + if (!ops)
> + return -ENODEV;
> +
> + if (!ops->get_enable)
> + return -EPERM;
> +
> + return ops->get_enable(dev);
> +}
> +
> +int regulator_set_enable(struct udevice *dev, bool enable)
> +{
> + const struct dm_regulator_ops *ops;
> +
> + ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
> + if (!ops)
> + return -ENODEV;
> +
> + if (!ops->set_enable)
> + return -EPERM;
> +
> + return ops->set_enable(dev, enable);
> +}
> +
> +int regulator_get_mode(struct udevice *dev)
> +{
> + const struct dm_regulator_ops *ops;
> +
> + ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
> + if (!ops)
> + return -ENODEV;
> +
> + if (!ops->get_mode)
> + return -EPERM;
> +
> + return ops->get_mode(dev);
> +}
> +
> +int regulator_set_mode(struct udevice *dev, int mode)
> +{
> + const struct dm_regulator_ops *ops;
> +
> + ops = pmic_get_uclass_ops(dev, UCLASS_REGULATOR);
> + if (!ops)
> + return -ENODEV;
> +
> + if (!ops->set_mode)
> + return -EPERM;
> +
> + return ops->set_mode(dev, mode);
> +}
> +
> +int regulator_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct dm_regulator_info *info = dev->uclass_priv;
> + int offset = dev->of_offset;
> + int len;
> +
> + /* Mandatory constraints */
> + info->name = strdup(fdt_getprop(gd->fdt_blob, offset,
> + "regulator-name", &len));
> + if (!info->name)
> + return -ENXIO;
-ENOMEM
But there is no need to strdup() this - the device tree does not move.
> +
> + info->min_uV = fdtdec_get_int(gd->fdt_blob, offset,
> + "regulator-min-microvolt", -1);
> + if (info->min_uV < 0)
> + return -ENXIO;
> +
> + info->max_uV = fdtdec_get_int(gd->fdt_blob, offset,
> + "regulator-max-microvolt", -1);
> + if (info->max_uV < 0)
> + return -ENXIO;
> +
> + /* Optional constraints */
> + info->min_uA = fdtdec_get_int(gd->fdt_blob, offset,
> + "regulator-min-microamp", -1);
> + info->max_uA = fdtdec_get_int(gd->fdt_blob, offset,
> + "regulator-max-microamp", -1);
> + info->always_on = fdtdec_get_bool(gd->fdt_blob, offset,
> + "regulator-always-on");
> + info->boot_on = fdtdec_get_bool(gd->fdt_blob, offset,
> + "regulator-boot-on");
> +
> + return 0;
> +}
> +
> +int regulator_get(char *name, struct udevice **devp)
> +{
> + struct dm_regulator_info *info;
> + struct udevice *dev;
> + int ret;
> +
> + *devp = NULL;
> +
> + for (ret = uclass_first_device(UCLASS_REGULATOR, &dev);
> + dev;
> + ret = uclass_next_device(&dev)) {
This will probe all devices. See my suggestion about creating
uclass_find_first_device()/uclass_find_next_device() in the next
patch.
As before, I think this could use a function like uclass_get_device_by_name().
> + info = dev->uclass_priv;
> + if (!info)
> + continue;
> +
> + if (!strcmp(name, info->name)) {
> + *devp = dev;
> + return ret;
> + }
> + }
> +
> + return -ENODEV;
> +}
> +
> +UCLASS_DRIVER(regulator) = {
> + .id = UCLASS_REGULATOR,
> + .name = "regulator",
> + .per_device_auto_alloc_size = sizeof(struct dm_regulator_info),
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 3ecfa23..23356f3 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -37,6 +37,7 @@ enum uclass_id {
>
> /* PMIC uclass and PMIC-related uclass types */
> UCLASS_PMIC,
> + UCLASS_REGULATOR,
OK I see what the comment means now. I think it would be nice to have
a comment here 'Voltage regulator'
>
> UCLASS_COUNT,
> UCLASS_INVALID = -1,
> diff --git a/include/power/regulator.h b/include/power/regulator.h
> new file mode 100644
> index 0000000..cf083c5
> --- /dev/null
> +++ b/include/power/regulator.h
> @@ -0,0 +1,259 @@
> +/*
> + * Copyright (C) 2014-2015 Samsung Electronics
> + * Przemyslaw Marczak <p.marczak at samsung.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef _INCLUDE_REGULATOR_H_
> +#define _INCLUDE_REGULATOR_H_
> +
> +/* enum regulator_type - used for regulator_*() variant calls */
> +enum regulator_type {
> + REGULATOR_TYPE_LDO = 0,
> + REGULATOR_TYPE_BUCK,
> + REGULATOR_TYPE_DVS,
> + REGULATOR_TYPE_FIXED,
> + REGULATOR_TYPE_OTHER,
> +};
> +
> +/**
> + * struct dm_regulator_mode - this structure holds an information about
> + * each regulator operation mode. Probably in most cases - an array.
> + * This will be probably a driver-static data, since it is device-specific.
> + *
> + * @id - a driver-specific mode id
> + * @register_value - a driver-specific value for its mode id
> + * @name - the name of mode - used for regulator command
> + * Note:
> + * The field 'id', should be always a positive number, since the negative values
> + * are reserved for the errno numbers when returns the mode id.
> + */
> +struct dm_regulator_mode {
> + int id; /* Set only as >= 0 (negative value is reserved for errno) */
> + int register_value;
> + const char *name;
> +};
> +
> +/**
> + * struct dm_regulator_info - this structure holds an information about
> + * each regulator constraints and supported operation modes. There is no "step"
> + * voltage value - so driver should take care of this.
> + *
> + * @type - one of 'enum regulator_type'
> + * @mode - pointer to the regulator mode (array if more than one)
> + * @mode_count - number of '.mode' entries
> + * @min_uV* - minimum voltage (micro Volts)
> + * @max_uV* - maximum voltage (micro Volts)
> + * @min_uA* - minimum amperage (micro Amps)
> + * @max_uA* - maximum amperage (micro Amps)
> + * @always_on* - bool type, true or false
> + * @boot_on* - bool type, true or false
> + * @name* - fdt regulator name - should be taken from the device tree
> + *
> + * Note: for attributes signed with '*'
> + * These platform-specific constraints can be taken by regulator api function,
> + * which is 'regulator_ofdata_to_platdata()'. Please read the description, which
> + * can be found near the declaration of the mentioned function.
> +*/
> +struct dm_regulator_info {
> + enum regulator_type type;
> + struct dm_regulator_mode *mode;
> + int mode_count;
> + int min_uV;
> + int max_uV;
> + int min_uA;
> + int max_uA;
> + bool always_on;
> + bool boot_on;
> + const char *name;
> +};
> +
> +/* PMIC regulator device operations */
> +struct dm_regulator_ops {
> + /**
> + * The regulator output value function calls operates on a micro Volts.
> + *
> + * get/set_value - get/set output value of the given output number
> + * @dev - regulator device
> + * Sets:
> + * @uV - set the output value [micro Volts]
> + * Returns: output value [uV] on success or negative errno if fail.
> + */
> + int (*get_value)(struct udevice *dev);
> + int (*set_value)(struct udevice *dev, int uV);
> +
> + /**
> + * The regulator output current function calls operates on a micro Amps.
> + *
> + * get/set_current - get/set output current of the given output number
> + * @dev - regulator device
> + * Sets:
> + * @uA - set the output current [micro Amps]
> + * Returns: output value [uA] on success or negative errno if fail.
> + */
> + int (*get_current)(struct udevice *dev);
> + int (*set_current)(struct udevice *dev, int uA);
> +
> + /**
> + * The most basic feature of the regulator output is its enable state.
> + *
> + * get/set_enable - get/set enable state of the given output number
> + * @dev - regulator device
> + * Sets:
> + * @enable - set true - enable or false - disable
> + * Returns: true/false for get; or 0 / -errno for set.
> + */
> + bool (*get_enable)(struct udevice *dev);
> + int (*set_enable)(struct udevice *dev, bool enable);
> +
> + /**
> + * The 'get/set_mode()' function calls should operate on a driver
driver-
> + * specific mode definitions, which should be found in:
> + * field 'mode' of struct mode_desc.
> + *
> + * get/set_mode - get/set operation mode of the given output number
> + * @dev - regulator device
> + * Sets
> + * @mode_id - set output mode id (struct dm_regulator_mode->id)
> + * Returns: id/0 for get/set on success or negative errno if fail.
> + * Note:
> + * The field 'id' of struct type 'dm_regulator_mode', should be always
> + * positive number, since the negative is reserved for the error.
> + */
> + int (*get_mode)(struct udevice *dev);
> + int (*set_mode)(struct udevice *dev, int mode_id);
> +};
> +
> +/**
> + * regulator_info: returns a pointer to the devices regulator info structure
> + *
> + * @dev - pointer to the regulator device
> + * @infop - pointer to the returned regulator info
> + * Returns - 0 on success or negative value of errno.
@return 0 on success...
> + */
> +int regulator_info(struct udevice *dev, struct dm_regulator_info **infop);
> +
> +/**
> + * regulator_mode: returns a pointer to the array of regulator mode info
> + *
> + * @dev - pointer to the regulator device
> + * @modep - pointer to the returned mode info array
> + * Returns - count of modep entries on success or negative errno if fail.
> + */
> +int regulator_mode(struct udevice *dev, struct dm_regulator_mode **modep);
> +
> +/**
> + * regulator_get_value: get microvoltage voltage value of a given regulator
> + *
> + * @dev - pointer to the regulator device
> + * Returns - positive output value [uV] on success or negative errno if fail.
> + */
> +int regulator_get_value(struct udevice *dev);
> +
> +/**
> + * regulator_set_value: set the microvoltage value of a given regulator.
> + *
> + * @dev - pointer to the regulator device
> + * @uV - the output value to set [micro Volts]
> + * Returns - 0 on success or -errno val if fails
> + */
> +int regulator_set_value(struct udevice *dev, int uV);
> +
> +/**
> + * regulator_get_current: get microampere value of a given regulator
> + *
> + * @dev - pointer to the regulator device
> + * Returns - positive output current [uA] on success or negative errno if fail.
> + */
> +int regulator_get_current(struct udevice *dev);
> +
> +/**
> + * regulator_set_current: set the microampere value of a given regulator.
> + *
> + * @dev - pointer to the regulator device
> + * @uA - set the output current [micro Amps]
> + * Returns - 0 on success or -errno val if fails
> + */
> +int regulator_set_current(struct udevice *dev, int uA);
> +
> +/**
> + * regulator_get_enable: get regulator device enable state.
> + *
> + * @dev - pointer to the regulator device
> + * Returns - true/false of enable state
> + */
> +bool regulator_get_enable(struct udevice *dev);
> +
> +/**
> + * regulator_set_enable: set regulator enable state
> + *
> + * @dev - pointer to the regulator device
> + * @enable - set true or false
> + * Returns - 0 on success or -errno val if fails
> + */
> +int regulator_set_enable(struct udevice *dev, bool enable);
> +
> +/**
> + * regulator_get_mode: get mode of a given device regulator
> + *
> + * @dev - pointer to the regulator device
> + * Returns - positive mode number on success or -errno val if fails
> + * Note:
> + * The regulator driver should return one of defined, mode number rather, than
> + * the raw register value. The struct type 'mode_desc' provides a field 'mode'
> + * for this purpose and register_value for a raw register value.
> + */
> +int regulator_get_mode(struct udevice *dev);
> +
> +/**
> + * regulator_set_mode: set given regulator mode
> + *
> + * @dev - pointer to the regulator device
> + * @mode - mode type (field 'mode' of struct mode_desc)
> + * Returns - 0 on success or -errno value if fails
> + * Note:
> + * The regulator driver should take one of defined, mode number rather
> + * than a raw register value. The struct type 'regulator_mode_desc' has
> + * a mode field for this purpose and register_value for a raw register value.
> + */
> +int regulator_set_mode(struct udevice *dev, int mode);
> +
> +/**
> + * regulator_ofdata_to_platdata: get the regulator constraints from its fdt node
> + * and put it into the regulator 'dm_regulator_info' (dev->uclass_priv).
> + *
> + * An example of required regulator fdt node constraints:
> + * ldo1 {
> + * regulator-compatible = "LDO1"; (not used here, but required for bind)
> + * regulator-name = "VDD_MMC_1.8V"; (mandatory)
> + * regulator-min-microvolt = <1000000>; (mandatory)
> + * regulator-max-microvolt = <1000000>; (mandatory)
> + * regulator-min-microamp = <1000>; (optional)
> + * regulator-max-microamp = <1000>; (optional)
> + * regulator-always-on; (optional)
> + * regulator-boot-on; (optional)
> + * };
> + *
> + * @dev - pointer to the regulator device
> + * Returns - 0 on success or -errno value if fails
> + * Note2:
> + * This function can be called at stage in which 'dev->uclass_priv' is non-NULL.
> + * It is possible at two driver call stages: '.ofdata_to_platdata' or '.probe'.
> + */
> +int regulator_ofdata_to_platdata(struct udevice *dev);
> +
> +/**
> + * regulator_get: returns the pointer to the pmic regulator device based on
> + * regulator fdt node data
> + *
> + * @fdt_name - property 'regulator-name' value of regulator fdt node
> + * @devp - returned pointer to the regulator device
> + * Returns: 0 on success or negative value of errno.
> + *
> + * The returned 'regulator' device can be used with:
> + * - regulator_get/set_*
> + */
> +int regulator_get(char *fdt_name, struct udevice **devp);
Can we s/fdt_name/name/ since by now it is a device name, not a device
tree name.
> +
> +#endif /* _INCLUDE_REGULATOR_H_ */
> --
> 1.9.1
>
Regards,
Simon
More information about the U-Boot
mailing list