Hi Przemyslaw, On 24 March 2015 at 14:30, Przemyslaw Marczak <p.marc...@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.marc...@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.marc...@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.marc...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot