[U-Boot] [PATCH V2 5/6] power: regulator: add pfuze100 support

Peng Fan
2015-08-06T04:52:35Z

On Tue, Aug 04, 2015 at 09:14:17PM +0800, Peng Fan wrote:
>On Tue, Aug 04, 2015 at 07:21:05AM -0600, Simon Glass wrote:
>>Hi,
>>
>>On 4 August 2015 at 07:10, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>>> Hello Peng,
>>>
>>>
>>> On 08/04/2015 07:32 AM, Peng Fan wrote:
>>>>
>>>> 1. Add new regulator driver pfuze100.
>>>> * Introduce struct pfuze100_regulator_desc for mataining info for
>>>> regulator.
>>>> 2. Add new Kconfig entry DM_REGULATOR_PFUZE100 for pfuze100.
>>>> 3. This driver intends to support PF100, PF200 and PF3000.
>>>> 4. Add related macro definition in pfuze header file.
>>>>
>>>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>>>> Cc: Przemyslaw Marczak <p.marczak@samsung.com>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>
>>>> Changes v2:
>>>> Addressed Simon's comments.
>>>> 1. Use pmic_reg_read/pmic_reg_write/pmic_clrsetbits
>>>> 2. blank line between declarations and rest
>>
>>Would still like to see you device tree file somewhere.
>
>See: http://lxr.free-electrons.com/source/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
>line from 208 to 306.
>

Simon,

Do you have further comments about this patch regarding the device tree?
I plan to work out V3 patch.

>>
>>>>
>>>> Hi Simon,
>>>>
>>>> type case is still kept there:
>>>> " struct pfuze100_regulator_desc *desc =
>>>> *(struct pfuze100_regulator_desc **)dev_get_platdata(dev); "

Przemyslaw,

I use the way Simon suggested to me and discard the double pointer.
struct pfuze_platdata {
struct pfuze100_regulator_desc *desc;
};

>>>> Also use pointer points to pointer type, "struct type **", see this
>>>> line:
>>>> " .platdata_auto_alloc_size = sizeof(struct pfuze100_regulator_desc **),
>>>> "
>>>>
>>>> Each regulator has its own pfuze_regulator_desc, single pointer can not
>>>> do
>>>> the work well. For example:
>>>> .platdata_auto_alloc_size = sizeof(struct pfuze100_regulator_desc *);
>>>>
>>>> struct pfuze100_regulator_desc *desc = dev_get_platdata(dev);
>>>>
>>>> desc = se_desc(pfuze100_regulators, ARRAY_SIZE(pfuze100_regulators),
>>>> dev->name);
>>>> what we changed here is only local variable desc, but not change the
>>>> pointer
>>>> platdata. What I need here to set the pointer platdata to point to
>>>> correct regulator descriptor. Single pointer can not handle this, so
>>>> use "struct type **".
>>>>
>>>>
>>>> drivers/power/regulator/Kconfig | 8 +
>>>> drivers/power/regulator/Makefile | 1 +
>>>> drivers/power/regulator/pfuze100.c | 554
>>>> +++++++++++++++++++++++++++++++++++++
>>>> include/power/pfuze100_pmic.h | 24 +-
>>>> 4 files changed, 583 insertions(+), 4 deletions(-)
>>>> create mode 100644 drivers/power/regulator/pfuze100.c
>>>>
>>>> diff --git a/drivers/power/regulator/Kconfig
>>>> b/drivers/power/regulator/Kconfig
>>>> index 6289b83..b854773 100644
>>>> --- a/drivers/power/regulator/Kconfig
>>>> +++ b/drivers/power/regulator/Kconfig
>>>> @@ -16,6 +16,14 @@ config DM_REGULATOR
>>>> for this purpose if PMIC I/O driver is implemented or
>>>> dm_scan_fdt_node()
>>>> otherwise. Detailed information can be found in the header file.
>>>>
>>>> +config DM_REGULATOR_PFUZE100
>>>> + bool "Enable Driver Model for REGULATOR PFUZE100"
>>>> + depends on DM_REGULATOR && DM_PMIC_PFUZE100
>>>> + ---help---
>>>> + This config enables implementation of driver-model regulator
>>>> uclass
>>>> + features for REGULATOR PFUZE100. The driver implements get/set api
>>>> for:
>>>> + value, enable and mode.
>>>> +
>>>> config DM_REGULATOR_MAX77686
>>>> bool "Enable Driver Model for REGULATOR MAX77686"
>>>> depends on DM_REGULATOR && DM_PMIC_MAX77686
>>>> diff --git a/drivers/power/regulator/Makefile
>>>> b/drivers/power/regulator/Makefile
>>>> index 96aa624..9f8f17b 100644
>>>> --- a/drivers/power/regulator/Makefile
>>>> +++ b/drivers/power/regulator/Makefile
>>>> @@ -7,5 +7,6 @@
>>>>
>>>> obj-$(CONFIG_DM_REGULATOR) += regulator-uclass.o
>>>> obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
>>>> +obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
>>>> obj-$(CONFIG_DM_REGULATOR_FIXED) += fixed.o
>>>> obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o
>>>> diff --git a/drivers/power/regulator/pfuze100.c
>>>> b/drivers/power/regulator/pfuze100.c
>>>> new file mode 100644
>>>> index 0000000..300aee8
>>>> --- /dev/null
>>>> +++ b/drivers/power/regulator/pfuze100.c
>>>> @@ -0,0 +1,554 @@
>>>> +#include <common.h>
>>>> +#include <fdtdec.h>
>>>> +#include <errno.h>
>>>> +#include <dm.h>
>>>> +#include <i2c.h>
>>>> +#include <power/pmic.h>
>>>> +#include <power/regulator.h>
>>>> +#include <power/pfuze100_pmic.h>
>>>> +
>>>> +/**
>>>> + * struct pfuze100_regulator_desc - regulator descriptor
>>>> + *
>>>> + * @name: Identify name for the regulator.
>>>> + * @type: Indicates the regulator type.
>>>> + * @uV_step: Voltage increase for each selector.
>>>> + * @vsel_reg: Register for adjust regulator voltage for normal.
>>>> + * @vsel_mask: Mask bit for setting regulator voltage for normal.
>>>> + * @stby_reg: Register for adjust regulator voltage for standby.
>>>> + * @stby_mask: Mask bit for setting regulator voltage for standby.
>>>> + * @volt_table: Voltage mapping table (if table based mapping).
>>>> + * @voltage: Current voltage for REGULATOR_TYPE_FIXED type regulator.
>>>> + */
>>>> +struct pfuze100_regulator_desc {
>>>> + char *name;
>>>> + enum regulator_type type;
>>>> + unsigned int uV_step;
>>>> + unsigned int vsel_reg;
>>>> + unsigned int vsel_mask;
>>>> + unsigned int stby_reg;
>>>> + unsigned int stby_mask;
>>>> + unsigned int *volt_table;
>>>> + unsigned int voltage;
>>>> +};
>>>> +
>>>> +#define PFUZE100_FIXED_REG(_name, base, vol) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_FIXED, \
>>>> + .voltage = (vol), \
>>>> + }
>>>> +
>>>> +#define PFUZE100_SW_REG(_name, base, step) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_BUCK, \
>>>> + .uV_step = (step), \
>>>> + .vsel_reg = (base) + PFUZE100_VOL_OFFSET, \
>>>> + .vsel_mask = 0x3F, \
>>>> + .stby_reg = (base) + PFUZE100_STBY_OFFSET, \
>>>> + .stby_mask = 0x3F, \
>>>> + }
>>>> +
>>>> +#define PFUZE100_SWB_REG(_name, base, mask, step, voltages) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_BUCK, \
>>>> + .uV_step = (step), \
>>>> + .vsel_reg = (base), \
>>>> + .vsel_mask = (mask), \
>>>> + .volt_table = (voltages), \
>>>> + }
>>>> +
>>>> +#define PFUZE100_SNVS_REG(_name, base, mask, voltages) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_OTHER, \
>>>> + .vsel_reg = (base), \
>>>> + .vsel_mask = (mask), \
>>>> + .volt_table = (voltages), \
>>>> + }
>>>> +
>>>> +#define PFUZE100_VGEN_REG(_name, base, step) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_LDO, \
>>>> + .uV_step = (step), \
>>>> + .vsel_reg = (base), \
>>>> + .vsel_mask = 0xF, \
>>>> + .stby_reg = (base), \
>>>> + .stby_mask = 0x20, \
>>>> + }
>>>> +
>>>> +#define PFUZE3000_VCC_REG(_name, base, step) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_LDO, \
>>>> + .uV_step = (step), \
>>>> + .vsel_reg = (base), \
>>>> + .vsel_mask = 0x3, \
>>>> + .stby_reg = (base), \
>>>> + .stby_mask = 0x20, \
>>>> +}
>>>> +
>>>> +#define PFUZE3000_SW1_REG(_name, base, step) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_BUCK, \
>>>> + .uV_step = (step), \
>>>> + .vsel_reg = (base) + PFUZE100_VOL_OFFSET, \
>>>> + .vsel_mask = 0x1F, \
>>>> + .stby_reg = (base) + PFUZE100_STBY_OFFSET, \
>>>> + .stby_mask = 0x1F, \
>>>> + }
>>>> +
>>>> +#define PFUZE3000_SW2_REG(_name, base, step) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_BUCK, \
>>>> + .uV_step = (step), \
>>>> + .vsel_reg = (base) + PFUZE100_VOL_OFFSET, \
>>>> + .vsel_mask = 0x7, \
>>>> + .stby_reg = (base) + PFUZE100_STBY_OFFSET, \
>>>> + .stby_mask = 0x7, \
>>>> + }
>>>> +
>>>> +#define PFUZE3000_SW3_REG(_name, base, step) \
>>>> + { \
>>>> + .name = #_name, \
>>>> + .type = REGULATOR_TYPE_BUCK, \
>>>> + .uV_step = (step), \
>>>> + .vsel_reg = (base) + PFUZE100_VOL_OFFSET, \
>>>> + .vsel_mask = 0xF, \
>>>> + .stby_reg = (base) + PFUZE100_STBY_OFFSET, \
>>>> + .stby_mask = 0xF, \
>>>> + }
>>>> +
>>>> +static unsigned int pfuze100_swbst[] = {
>>>> + 5000000, 5050000, 5100000, 5150000
>>>> +};
>>>> +
>>>> +static unsigned int pfuze100_vsnvs[] = {
>>>> + 1000000, 1100000, 1200000, 1300000, 1500000, 1800000, 3000000, -1
>>>> +};
>>>> +
>>>> +static unsigned int pfuze3000_vsnvs[] = {
>>>> + -1, -1, -1, -1, -1, -1, 3000000, -1
>>>> +};
>>>> +
>>>> +static unsigned int pfuze3000_sw2lo[] = {
>>>> + 1500000, 1550000, 1600000, 1650000, 1700000, 1750000, 1800000,
>>>> 1850000
>>>> +};
>>>> +
>>>> +/* PFUZE100 */
>>>> +static struct pfuze100_regulator_desc pfuze100_regulators[] = {
>>>> + PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
>>>> + PFUZE100_SW_REG(sw1c, PFUZE100_SW1CVOL, 25000),
>>>> + PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
>>>> + PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
>>>> + PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
>>>> + PFUZE100_SW_REG(sw4, PFUZE100_SW4VOL, 25000),
>>>> + PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000,
>>>> pfuze100_swbst),
>>>> + PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
>>>> + PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
>>>> + PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
>>>> + PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
>>>> + PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
>>>> + PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
>>>> + PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
>>>> + PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
>>>> +};
>>>> +
>>>> +/* PFUZE200 */
>>>> +static struct pfuze100_regulator_desc pfuze200_regulators[] = {
>>>> + PFUZE100_SW_REG(sw1ab, PFUZE100_SW1ABVOL, 25000),
>>>> + PFUZE100_SW_REG(sw2, PFUZE100_SW2VOL, 25000),
>>>> + PFUZE100_SW_REG(sw3a, PFUZE100_SW3AVOL, 25000),
>>>> + PFUZE100_SW_REG(sw3b, PFUZE100_SW3BVOL, 25000),
>>>> + PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000,
>>>> pfuze100_swbst),
>>>> + PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze100_vsnvs),
>>>> + PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
>>>> + PFUZE100_VGEN_REG(vgen1, PFUZE100_VGEN1VOL, 50000),
>>>> + PFUZE100_VGEN_REG(vgen2, PFUZE100_VGEN2VOL, 50000),
>>>> + PFUZE100_VGEN_REG(vgen3, PFUZE100_VGEN3VOL, 100000),
>>>> + PFUZE100_VGEN_REG(vgen4, PFUZE100_VGEN4VOL, 100000),
>>>> + PFUZE100_VGEN_REG(vgen5, PFUZE100_VGEN5VOL, 100000),
>>>> + PFUZE100_VGEN_REG(vgen6, PFUZE100_VGEN6VOL, 100000),
>>>> +};
>>>> +
>>>> +/* PFUZE3000 */
>>>> +static struct pfuze100_regulator_desc pfuze3000_regulators[] = {
>>>> + PFUZE3000_SW1_REG(sw1a, PFUZE100_SW1ABVOL, 25000),
>>>> + PFUZE3000_SW1_REG(sw1b, PFUZE100_SW1CVOL, 25000),
>>>> + PFUZE100_SWB_REG(sw2, PFUZE100_SW2VOL, 0x7, 50000,
>>>> pfuze3000_sw2lo),
>>>> + PFUZE3000_SW3_REG(sw3, PFUZE100_SW3AVOL, 50000),
>>>> + PFUZE100_SWB_REG(swbst, PFUZE100_SWBSTCON1, 0x3, 50000,
>>>> pfuze100_swbst),
>>>> + PFUZE100_SNVS_REG(vsnvs, PFUZE100_VSNVSVOL, 0x7, pfuze3000_vsnvs),
>>>> + PFUZE100_FIXED_REG(vrefddr, PFUZE100_VREFDDRCON, 750000),
>>>> + PFUZE100_VGEN_REG(vldo1, PFUZE100_VGEN1VOL, 100000),
>>>> + PFUZE100_VGEN_REG(vldo2, PFUZE100_VGEN2VOL, 50000),
>>>> + PFUZE3000_VCC_REG(vccsd, PFUZE100_VGEN3VOL, 150000),
>>>> + PFUZE3000_VCC_REG(v33, PFUZE100_VGEN4VOL, 150000),
>>>> + PFUZE100_VGEN_REG(vldo3, PFUZE100_VGEN5VOL, 100000),
>>>> + PFUZE100_VGEN_REG(vldo4, PFUZE100_VGEN6VOL, 100000),
>>>> +};
>>>> +
>>>> +#define MODE(_id, _val, _name) { \
>>>> + .id = _id, \
>>>> + .register_value = _val, \
>>>> + .name = _name, \
>>>> +}
>>>> +
>>>> +/* SWx Buck regulator mode */
>>>> +static struct dm_regulator_mode pfuze_sw_modes[] = {
>>>> + MODE(OFF_OFF, OFF_OFF, "OFF_OFF"),
>>>> + MODE(PWM_OFF, PWM_OFF, "PWM_OFF"),
>>>> + MODE(PFM_OFF, PFM_OFF, "PFM_OFF"),
>>>> + MODE(APS_OFF, APS_OFF, "APS_OFF"),
>>>> + MODE(PWM_PWM, PWM_PWM, "PWM_PWM"),
>>>> + MODE(PWM_APS, PWM_APS, "PWM_APS"),
>>>> + MODE(APS_APS, APS_APS, "APS_APS"),
>>>> + MODE(APS_PFM, APS_PFM, "APS_PFM"),
>>>> + MODE(PWM_PFM, PWM_PFM, "PWM_PFM"),
>>>> +};
>>>> +
>>>> +/* Boost Buck regulator mode for normal operation */
>>>> +static struct dm_regulator_mode pfuze_swbst_modes[] = {
>>>> + MODE(SWBST_MODE_OFF, SWBST_MODE_OFF , "SWBST_MODE_OFF"),
>>>> + MODE(SWBST_MODE_PFM, SWBST_MODE_PFM, "SWBST_MODE_PFM"),
>>>> + MODE(SWBST_MODE_AUTO, SWBST_MODE_AUTO, "SWBST_MODE_AUTO"),
>>>> + MODE(SWBST_MODE_APS, SWBST_MODE_APS, "SWBST_MODE_APS"),
>>>> +};
>>>> +
>>>> +/* VGENx LDO regulator mode for normal operation */
>>>> +static struct dm_regulator_mode pfuze_ldo_modes[] = {
>>>> + MODE(LDO_MODE_OFF, LDO_MODE_OFF, "LDO_MODE_OFF"),
>>>> + MODE(LDO_MODE_ON, LDO_MODE_ON, "LDO_MODE_ON"),
>>>> +};
>>>> +
>>>> +static struct pfuze100_regulator_desc *se_desc(struct
>>>> pfuze100_regulator_desc *desc,
>>>> + int size,
>>>> + const char *name)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < size; desc++) {
>>>> + if (!strcmp(desc->name, name))
>>>> + return desc;
>>>> + continue;
>>>> + }
>>>> +
>>>> + return NULL;
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_probe(struct udevice *dev)
>>>> +{
>>>> + struct dm_regulator_uclass_platdata *uc_pdata;
>>>> + struct pfuze100_regulator_desc **desc = dev_get_platdata(dev);
>>>
>>>
>>> Try this:
>>> struct pfuze100_regulator_desc *desc;
>>>
>>>> +
>>>> + switch (dev_get_driver_data(dev_get_parent(dev))) {
>>>> + case PFUZE100:
>>>> + *desc = se_desc(pfuze100_regulators,
>>>> + ARRAY_SIZE(pfuze100_regulators),
>>>> + dev->name);
>>>
>>>
>>> Try this:
>>> desc = se_desc(..)
>>>
>>>> + break;
>>>> + case PFUZE200:
>>>> + *desc = se_desc(pfuze200_regulators,
>>>> + ARRAY_SIZE(pfuze200_regulators),
>>>> + dev->name);
>>>> + break;
>>>> + case PFUZE3000:
>>>> + *desc = se_desc(pfuze3000_regulators,
>>>> + ARRAY_SIZE(pfuze3000_regulators),
>>>> + dev->name);
>>>> + break;
>>>> + }
>>>> + if (!*desc) {
>>>> + debug("Do not support regulator %s\n", dev->name);
>>>> + return -EINVAL;
>>>> + }
>>>
>>>
>>> Try this:
>>> dev->platdata = desc;
>>>
>>>> +
>>>> + uc_pdata = dev_get_uclass_platdata(dev);
>>>> +
>>>
>>>
>>> Try this:
>>> uc_pdata->type = desc->type;
>>>
>>>> + uc_pdata->type = (*desc)->type;
>>>> + if (uc_pdata->type == REGULATOR_TYPE_BUCK) {
>>>> + if (!strcmp(dev->name, "swbst")) {
>>>> + uc_pdata->mode = pfuze_swbst_modes;
>>>> + uc_pdata->mode_count =
>>>> ARRAY_SIZE(pfuze_swbst_modes);
>>>> + } else {
>>>> + uc_pdata->mode = pfuze_sw_modes;
>>>> + uc_pdata->mode_count = ARRAY_SIZE(pfuze_sw_modes);
>>>> + }
>>>> + } else if (uc_pdata->type == REGULATOR_TYPE_LDO) {
>>>> + uc_pdata->mode = pfuze_ldo_modes;
>>>> + uc_pdata->mode_count = ARRAY_SIZE(pfuze_ldo_modes);
>>>> + } else {
>>>> + uc_pdata->mode = NULL;
>>>> + uc_pdata->mode_count = 0;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_mode(struct udevice *dev, int op, int
>>>> *opmode)
>>>> +{
>>>> + unsigned char val;
>>>
>>>
>>> Try this:
>>> struct pfuze100_regulator_desc *desc = dev_get_platdata(dev);
>>>
>>>> + struct pfuze100_regulator_desc *desc =
>>>> + *(struct pfuze100_regulator_desc **)dev_get_platdata(dev);
>>>> +
>>>> + if (op == PMIC_OP_GET) {
>>>> + if (desc->type == REGULATOR_TYPE_BUCK) {
>>>> + if (!strcmp(dev->name, "swbst")) {
>>>> + val = pmic_reg_read(dev->parent,
>>>> + desc->vsel_reg);
>>>> + if (val < 0)
>>>> + return val;
>>>> +
>>>> + val &= SWBST_MODE_MASK;
>>>> + val >>= SWBST_MODE_SHIFT;
>>>> + *opmode = val;
>>>> +
>>>> + return 0;
>>>> + }
>>>> + val = pmic_reg_read(dev->parent,
>>>> + desc->vsel_reg +
>>>> + PFUZE100_MODE_OFFSET);
>>>> + if (val < 0)
>>>> + return val;
>>>> +
>>>> + val &= SW_MODE_MASK;
>>>> + val >>= SW_MODE_SHIFT;
>>>> + *opmode = val;
>>>> +
>>>> + return 0;
>>>> +
>>>> + } else if (desc->type == REGULATOR_TYPE_LDO) {
>>>> + val = pmic_reg_read(dev->parent, desc->vsel_reg);
>>>> + if (val < 0)
>>>> + return val;
>>>> +
>>>> + val &= LDO_MODE_MASK;
>>>> + val >>= LDO_MODE_SHIFT;
>>>> + *opmode = val;
>>>> +
>>>> + return 0;
>>>> + } else {
>>>> + return -EINVAL;
>>>> + }
>>>> + }
>>>> +
>>>> + if (desc->type == REGULATOR_TYPE_BUCK) {
>>>> + if (!strcmp(dev->name, "swbst"))
>>>> + return pmic_clrsetbits(dev->parent,
>>>> desc->vsel_reg,
>>>> + SWBST_MODE_MASK,
>>>> + *opmode <<
>>>> SWBST_MODE_SHIFT);
>>>> +
>>>> + val = pmic_clrsetbits(dev->parent,
>>>> + desc->vsel_reg +
>>>> PFUZE100_MODE_OFFSET,
>>>> + SW_MODE_MASK,
>>>> + *opmode << SW_MODE_SHIFT);
>>>> +
>>>> + } else if (desc->type == REGULATOR_TYPE_LDO) {
>>>> + val = pmic_clrsetbits(dev->parent, desc->vsel_reg,
>>>> + LDO_MODE_MASK,
>>>> + *opmode << LDO_MODE_SHIFT);
>>>> + return val;
>>>> + } else {
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_enable(struct udevice *dev, int op, bool
>>>> *enable)
>>>> +{
>>>> + unsigned char val;
>>>> + int ret, on_off;
>>>> + struct dm_regulator_uclass_platdata *uc_pdata =
>>>> + dev_get_uclass_platdata(dev);
>>>> +
>>>> + if (op == PMIC_OP_GET) {
>>>> + if (!strcmp(dev->name, "vrefddr")) {
>>>> + val = pmic_reg_read(dev->parent,
>>>> PFUZE100_VREFDDRCON);
>>>> + if (val < 0)
>>>> + return val;
>>>> +
>>>> + if (val & VREFDDRCON_EN)
>>>> + *enable = true;
>>>> + else
>>>> + *enable = false;
>>>> + return 0;
>>>> + }
>>>> + ret = pfuze100_regulator_mode(dev, op, &on_off);
>>>> + if (ret)
>>>> + return ret;
>>>> + switch (on_off) {
>>>> + /* OFF_OFF, SWBST_MODE_OFF, LDO_MODE_OFF have same value
>>>> */
>>>> + case OFF_OFF:
>>>> + *enable = false;
>>>> + break;
>>>> + default:
>>>> + *enable = true;
>>>> + break;
>>>> + }
>>>> + } else if (op == PMIC_OP_SET) {
>>>> + if (!strcmp(dev->name, "vrefddr")) {
>>>> + val = pmic_reg_read(dev->parent,
>>>> PFUZE100_VREFDDRCON);
>>>> + if (val < 0)
>>>> + return val;
>>>> +
>>>> + if (val & VREFDDRCON_EN)
>>>> + return 0;
>>>> + val |= VREFDDRCON_EN;
>>>> +
>>>> + return pmic_reg_write(dev->parent,
>>>> PFUZE100_VREFDDRCON,
>>>> + val);
>>>> + }
>>>> +
>>>> + if (uc_pdata->type == REGULATOR_TYPE_LDO) {
>>>> + on_off = *enable ? LDO_MODE_ON : LDO_MODE_OFF;
>>>> + } else if (uc_pdata->type == REGULATOR_TYPE_BUCK) {
>>>> + if (!strcmp(dev->name, "swbst"))
>>>> + on_off = *enable ? SWBST_MODE_AUTO :
>>>> + SWBST_MODE_OFF;
>>>> + else
>>>> + on_off = *enable ? APS_PFM : OFF_OFF;
>>>> + } else {
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + return pfuze100_regulator_mode(dev, op, &on_off);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_val(struct udevice *dev, int op, int *uV)
>>>> +{
>>>> + unsigned char val;
>>>> + int i;
>>>
>>>
>>> Try this:
>>> struct pfuze100_regulator_desc *desc = dev_get_platdata(dev);
>>>
>>>> + struct pfuze100_regulator_desc *desc =
>>>> + *(struct pfuze100_regulator_desc **)dev_get_platdata(dev);
>>>> + struct dm_regulator_uclass_platdata *uc_pdata =
>>>> + dev_get_uclass_platdata(dev);
>>>> +
>>>> + if (op == PMIC_OP_GET) {
>>>> + *uV = 0;
>>>> + if (uc_pdata->type == REGULATOR_TYPE_FIXED) {
>>>> + *uV = desc->voltage;
>>>> + } else if (desc->volt_table) {
>>>> + val = pmic_reg_read(dev->parent, desc->vsel_reg);
>>>> + if (val < 0)
>>>> + return val;
>>>> + val &= desc->vsel_mask;
>>>> + *uV = desc->volt_table[val];
>>>> + } else {
>>>> + if (uc_pdata->min_uV < 0) {
>>>> + debug("Need to provide min_uV in dts.\n");
>>>> + return -EINVAL;
>>>> + }
>>>> + val = pmic_reg_read(dev->parent, desc->vsel_reg);
>>>> + if (val < 0)
>>>> + return val;
>>>> + val &= desc->vsel_mask;
>>>> + *uV = uc_pdata->min_uV + (int)val * desc->uV_step;
>>>> + }
>>>> +
>>>> + return 0;
>>>> + }
>>>> +
>>>> + if (uc_pdata->type == REGULATOR_TYPE_FIXED) {
>>>> + debug("Set voltage for REGULATOR_TYPE_FIXED regulator\n");
>>>> + return -EINVAL;
>>>> + } else if (desc->volt_table) {
>>>> + for (i = 0; i < desc->vsel_mask; i++) {
>>>> + if (*uV == desc->volt_table[i])
>>>> + break;
>>>> + }
>>>> + if (i == desc->vsel_mask) {
>>>> + debug("Unsupported voltage %u\n", *uV);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + return pmic_clrsetbits(dev->parent, desc->vsel_reg,
>>>> + desc->vsel_mask, i);
>>>> + } else {
>>>> + if (uc_pdata->min_uV < 0) {
>>>> + debug("Need to provide min_uV in dts.\n");
>>>> + return -EINVAL;
>>>> + }
>>>> + return pmic_clrsetbits(dev->parent, desc->vsel_reg,
>>>> + desc->vsel_mask,
>>>> + (*uV - uc_pdata->min_uV) /
>>>> desc->uV_step);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_get_value(struct udevice *dev)
>>>> +{
>>>> + int uV;
>>>> + int ret;
>>>> +
>>>> + ret = pfuze100_regulator_val(dev, PMIC_OP_GET, &uV);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return uV;
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_set_value(struct udevice *dev, int uV)
>>>> +{
>>>> + return pfuze100_regulator_val(dev, PMIC_OP_SET, &uV);
>>>> +}
>>>> +
>>>> +static bool pfuze100_regulator_get_enable(struct udevice *dev)
>>>> +{
>>>> + int ret;
>>>> + bool enable = false;
>>>> +
>>>> + ret = pfuze100_regulator_enable(dev, PMIC_OP_GET, &enable);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return enable;
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_set_enable(struct udevice *dev, bool
>>>> enable)
>>>> +{
>>>> + return pfuze100_regulator_enable(dev, PMIC_OP_SET, &enable);
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_get_mode(struct udevice *dev)
>>>> +{
>>>> + int mode;
>>>> + int ret;
>>>> +
>>>> + ret = pfuze100_regulator_mode(dev, PMIC_OP_GET, &mode);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return mode;
>>>> +}
>>>> +
>>>> +static int pfuze100_regulator_set_mode(struct udevice *dev, int mode)
>>>> +{
>>>> + return pfuze100_regulator_mode(dev, PMIC_OP_SET, &mode);
>>>> +}
>>>> +
>>>> +static const struct dm_regulator_ops pfuze100_regulator_ops = {
>>>> + .get_value = pfuze100_regulator_get_value,
>>>> + .set_value = pfuze100_regulator_set_value,
>>>> + .get_enable = pfuze100_regulator_get_enable,
>>>> + .set_enable = pfuze100_regulator_set_enable,
>>>> + .get_mode = pfuze100_regulator_get_mode,
>>>> + .set_mode = pfuze100_regulator_set_mode,
>>>> +};
>>>> +
>>>> +U_BOOT_DRIVER(pfuze100_regulator) = {
>>>> + .name = "pfuze100_regulator",
>>>> + .id = UCLASS_REGULATOR,
>>>> + .ops = &pfuze100_regulator_ops,
>>>> + .probe = pfuze100_regulator_probe,
>>>
>>>
>>> You can use: sizeof(struct pfuze100_regulator_desc *)
>>> It is only size of the pointer not an array.
>>>
>>>
>>>> + .platdata_auto_alloc_size = sizeof(struct pfuze100_regulator_desc
>>
>>BTW if you go with assigning the platdata pointer as above (which I
>>think is a good idea) then you can drop the above line. The allocation
>>is not needed if you assign the pointer to something else.
>>
>>>> **),
>>>> +};
>>>> diff --git a/include/power/pfuze100_pmic.h b/include/power/pfuze100_pmic.h
>>>> index c40a976..41cb710 100644
>>>> --- a/include/power/pfuze100_pmic.h
>>>> +++ b/include/power/pfuze100_pmic.h
>>>> @@ -62,6 +62,13 @@ enum {
>>>> PFUZE100_NUM_OF_REGS = 0x7f,
>>>> };
>>>>
>>>> +/* Registor offset based on VOLT register */
>>>> +#define PFUZE100_VOL_OFFSET 0
>>>> +#define PFUZE100_STBY_OFFSET 1
>>>> +#define PFUZE100_OFF_OFFSET 2
>>>> +#define PFUZE100_MODE_OFFSET 3
>>>> +#define PFUZE100_CONF_OFFSET 4
>>>> +
>>>> /*
>>>> * Buck Regulators
>>>> */
>>>> @@ -138,6 +145,9 @@ enum {
>>>> #define SW1x_STBY_MASK 0x3f
>>>> #define SW1x_OFF_MASK 0x3f
>>>>
>>>> +#define SW_MODE_MASK 0xf
>>>> +#define SW_MODE_SHIFT 0
>>>> +
>>>> #define SW1xCONF_DVSSPEED_MASK 0xc0
>>>> #define SW1xCONF_DVSSPEED_2US 0x00
>>>> #define SW1xCONF_DVSSPEED_4US 0x40
>>>> @@ -186,7 +196,12 @@ enum {
>>>>
>>>> #define LDO_VOL_MASK 0xf
>>>> #define LDO_EN (1 << 4)
>>>> +#define LDO_MODE_SHIFT 4
>>>> +#define LDO_MODE_MASK (1 << 4)
>>>> +#define LDO_MODE_OFF 0
>>>> +#define LDO_MODE_ON 1
>>>>
>>>> +#define VREFDDRCON_EN (1 << 4)
>>>> /*
>>>> * Boost Regulator
>>>> */
>>>> @@ -199,10 +214,11 @@ enum {
>>>>
>>>> #define SWBST_VOL_MASK 0x3
>>>> #define SWBST_MODE_MASK 0xC
>>>> -#define SWBST_MODE_OFF (0 << 2)
>>>> -#define SWBST_MODE_PFM (1 << 2)
>>>> -#define SWBST_MODE_AUTO (2 << 2)
>>>> -#define SWBST_MODE_APS (3 << 2)
>>>> +#define SWBST_MODE_SHIFT 0x2
>>>> +#define SWBST_MODE_OFF 0
>>>> +#define SWBST_MODE_PFM 1
>>>> +#define SWBST_MODE_AUTO 2
>>>> +#define SWBST_MODE_APS 3
>>>>
>>>> /*
>>>> * Regulator Mode Control
>>>>
>>>
>>> Best regards,
>>> --
>>> Przemyslaw Marczak
>>> Samsung R&D Institute Poland
>>> Samsung Electronics
>>> p.marczak at samsung.com
>>
>>Regards,
>>Simon
>
>--
Thanks,
Peng.
--