diff mbox

[U-Boot,4/5] power: regulator: add pfuze100 support

Message ID 1438094935-18213-5-git-send-email-Peng.Fan@freescale.com
State Superseded
Delegated to: Przemyslaw Marczak
Headers show

Commit Message

Peng Fan July 28, 2015, 2:48 p.m. UTC 
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>
---
 drivers/power/regulator/Kconfig    |   8 +
 drivers/power/regulator/Makefile   |   1 +
 drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
 include/power/pfuze100_pmic.h      |  24 +-
 4 files changed, 625 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/regulator/pfuze100.c

Comments

Simon Glass Aug. 2, 2015, 10:30 p.m. UTC | #1 
Hi Peng,

On 28 July 2015 at 08:48, Peng Fan <Peng.Fan@freescale.com> 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>

It looks correct but I have code style comments - see below. They all
apply globally.

> ---
>  drivers/power/regulator/Kconfig    |   8 +
>  drivers/power/regulator/Makefile   |   1 +
>  drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
>  include/power/pfuze100_pmic.h      |  24 +-
>  4 files changed, 625 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..6c513e9
> --- /dev/null
> +++ b/drivers/power/regulator/pfuze100.c
> @@ -0,0 +1,596 @@
> +#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;

blank line between declarations and rest - please fix global

> +       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 = NULL;

Can't this just be a single pointer?

> +
> +       desc = dev_get_platdata(dev);
> +
> +       switch (dev_get_driver_data(dev_get_parent(dev))) {
> +       case PFUZE100:
> +               *desc = se_desc(pfuze100_regulators,
> +                               ARRAY_SIZE(pfuze100_regulators),
> +                               dev->name);
> +               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;
> +       }
> +
> +       uc_pdata = dev_get_uclass_platdata(dev);
> +
> +       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;
> +       int ret;
> +       struct pfuze100_regulator_desc *desc =
> +               *(struct pfuze100_regulator_desc **)dev_get_platdata(dev);

You don't actually need the cast.

> +
> +       if (op == PMIC_OP_GET) {
> +               if (desc->type == REGULATOR_TYPE_BUCK) {
> +                       if (!strcmp(dev->name, "swbst")) {
> +                               ret = pmic_read(dev->parent, desc->vsel_reg,
> +                                               &val, 1);
> +                               if (ret)
> +                                       return ret;
> +
> +                               val &= SWBST_MODE_MASK;
> +                               val >>= SWBST_MODE_SHIFT;
> +                               *opmode = val;
> +
> +                               return 0;
> +                       }
> +                       ret = pmic_read(dev->parent,
> +                                       desc->vsel_reg + PFUZE100_MODE_OFFSET,
> +                                       &val, 1);

pmic_reg_read()

> +                       if (ret)
> +                               return ret;
> +
> +                       val &= SW_MODE_MASK;
> +                       val >>= SW_MODE_SHIFT;
> +                       *opmode = val;
> +
> +                       return 0;
> +
> +               } else if (desc->type == REGULATOR_TYPE_LDO) {
> +                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
> +                       if (ret)
> +                               return ret;
> +
> +                       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")) {
> +                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
> +                       if (ret)
> +                               return ret;
> +
> +                       val &= ~SWBST_MODE_MASK;
> +                       val |= *opmode << SWBST_MODE_SHIFT;
> +
> +                       ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);

pmic_reg_write

> +                       if (ret)
> +                               return ret;
> +
> +                       return 0;
> +               }
> +               ret = pmic_read(dev->parent,
> +                               desc->vsel_reg + PFUZE100_MODE_OFFSET,
> +                               &val, 1);
> +               if (ret)
> +                       return ret;
> +
> +               val &= ~SW_MODE_MASK;
> +               val |= *opmode << SW_MODE_SHIFT;
> +
> +               ret = pmic_write(dev->parent,
> +                                desc->vsel_reg + PFUZE100_MODE_OFFSET,
> +                                &val, 1);

pmic_clrsetbits() can replace all of this

> +               if (ret)
> +                       return ret;
> +
> +               return 0;
> +       } else if (desc->type == REGULATOR_TYPE_LDO) {
> +               ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
> +               if (ret)
> +                       return ret;
> +
> +               val &= ~LDO_MODE_MASK;
> +               val |= *opmode << LDO_MODE_SHIFT;
> +
> +               ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
> +               if (ret)
> +                       return ret;
> +
> +               return 0;
> +       } else {
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static int pfuze100_regulator_enable(struct udevice *dev, int op, bool *enable)
> +{
> +       int ret, on_off;
> +       unsigned char val;
> +       struct dm_regulator_uclass_platdata *uc_pdata =
> +               dev_get_uclass_platdata(dev);
> +       if (op == PMIC_OP_GET) {
> +               if (!strcmp(dev->name, "vrefddr")) {
> +                       ret = pmic_read(dev->parent, PFUZE100_VREFDDRCON,
> +                                       &val, 1);
> +                       if (ret)
> +                               return ret;
> +                       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")) {
> +                       ret = pmic_read(dev->parent, PFUZE100_VREFDDRCON,
> +                                       &val, 1);
> +                       if (ret)
> +                               return ret;
> +
> +                       if (val & VREFDDRCON_EN)
> +                               return 0;
> +                       val |= VREFDDRCON_EN;
> +
> +                       return pmic_write(dev->parent, PFUZE100_VREFDDRCON,
> +                                         &val, 1);
> +               }
> +
> +               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, ret;
> +       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) {
> +                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
> +                       if (ret)
> +                               return ret;
> +                       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;
> +                       }
> +                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
> +                       if (ret)
> +                               return ret;
> +                       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;
> +               }
> +               ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
> +               if (ret)
> +                       return ret;
> +               val &= ~(desc->vsel_mask);
> +               val |= i;
> +               ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
> +               if (ret)
> +                       return ret;
> +       } else {
> +               if (uc_pdata->min_uV < 0) {
> +                       debug("Need to provide min_uV in dts.\n");
> +                       return -EINVAL;
> +               }
> +               ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
> +               if (ret)
> +                       return ret;
> +
> +               val &= ~(desc->vsel_mask);
> +               val |= (*uV - uc_pdata->min_uV) / desc->uV_step;
> +
> +               ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       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)
> +{
> +       bool enable = false;
> +       int ret;
> +
> +       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,
> +       .platdata_auto_alloc_size = sizeof(struct pfuze100_regulator_desc **),
> +};
> diff --git a/include/power/pfuze100_pmic.h b/include/power/pfuze100_pmic.h
> index b13a780..36cc212 100644
> --- a/include/power/pfuze100_pmic.h
> +++ b/include/power/pfuze100_pmic.h
> @@ -60,6 +60,13 @@ enum {
>         PMIC_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
>   */
> @@ -136,6 +143,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
> @@ -184,7 +194,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
>   */
> @@ -197,10 +212,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
> --
> 1.8.4
>
>

Regards,
Simon
Peng Fan Aug. 3, 2015, 12:38 a.m. UTC | #2 
Hi Simon,
On Sun, Aug 02, 2015 at 04:30:52PM -0600, Simon Glass wrote:
>Hi Peng,
>
>On 28 July 2015 at 08:48, Peng Fan <Peng.Fan@freescale.com> 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>
>
>It looks correct but I have code style comments - see below. They all
>apply globally.

Ok. Will fix them in V2.

>
>> ---
>>  drivers/power/regulator/Kconfig    |   8 +
>>  drivers/power/regulator/Makefile   |   1 +
>>  drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
>>  include/power/pfuze100_pmic.h      |  24 +-
>>  4 files changed, 625 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..6c513e9
>> --- /dev/null
>> +++ b/drivers/power/regulator/pfuze100.c
>> @@ -0,0 +1,596 @@
>> +#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;
>
>blank line between declarations and rest - please fix global

Will fix in V2.

>
>> +       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 = NULL;
>
>Can't this just be a single pointer?

hmm. My bad, will fix this.

>
>> +
>> +       desc = dev_get_platdata(dev);
>> +
>> +       switch (dev_get_driver_data(dev_get_parent(dev))) {
>> +       case PFUZE100:
>> +               *desc = se_desc(pfuze100_regulators,
>> +                               ARRAY_SIZE(pfuze100_regulators),
>> +                               dev->name);
>> +               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;
>> +       }
>> +
>> +       uc_pdata = dev_get_uclass_platdata(dev);
>> +
>> +       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;
>> +       int ret;
>> +       struct pfuze100_regulator_desc *desc =
>> +               *(struct pfuze100_regulator_desc **)dev_get_platdata(dev);
>
>You don't actually need the cast.

Will discard the cast.

>
>> +
>> +       if (op == PMIC_OP_GET) {
>> +               if (desc->type == REGULATOR_TYPE_BUCK) {
>> +                       if (!strcmp(dev->name, "swbst")) {
>> +                               ret = pmic_read(dev->parent, desc->vsel_reg,
>> +                                               &val, 1);
>> +                               if (ret)
>> +                                       return ret;
>> +
>> +                               val &= SWBST_MODE_MASK;
>> +                               val >>= SWBST_MODE_SHIFT;
>> +                               *opmode = val;
>> +
>> +                               return 0;
>> +                       }
>> +                       ret = pmic_read(dev->parent,
>> +                                       desc->vsel_reg + PFUZE100_MODE_OFFSET,
>> +                                       &val, 1);
>
>pmic_reg_read()
>
>> +                       if (ret)
>> +                               return ret;
>> +
>> +                       val &= SW_MODE_MASK;
>> +                       val >>= SW_MODE_SHIFT;
>> +                       *opmode = val;
>> +
>> +                       return 0;
>> +
>> +               } else if (desc->type == REGULATOR_TYPE_LDO) {
>> +                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
>> +                       if (ret)
>> +                               return ret;
>> +
>> +                       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")) {
>> +                       ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
>> +                       if (ret)
>> +                               return ret;
>> +
>> +                       val &= ~SWBST_MODE_MASK;
>> +                       val |= *opmode << SWBST_MODE_SHIFT;
>> +
>> +                       ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
>
>pmic_reg_write

Ok.

>
>> +                       if (ret)
>> +                               return ret;
>> +
>> +                       return 0;
>> +               }
>> +               ret = pmic_read(dev->parent,
>> +                               desc->vsel_reg + PFUZE100_MODE_OFFSET,
>> +                               &val, 1);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               val &= ~SW_MODE_MASK;
>> +               val |= *opmode << SW_MODE_SHIFT;
>> +
>> +               ret = pmic_write(dev->parent,
>> +                                desc->vsel_reg + PFUZE100_MODE_OFFSET,
>> +                                &val, 1);
>
>pmic_clrsetbits() can replace all of this

ok. Will use this new way.

[...]

Thanks,
Peng.
Peng Fan Aug. 4, 2015, 2:08 a.m. UTC | #3 
On Mon, Aug 03, 2015 at 08:38:49AM +0800, Peng Fan wrote:
>Hi Simon,
>On Sun, Aug 02, 2015 at 04:30:52PM -0600, Simon Glass wrote:
>>Hi Peng,
>>
>>On 28 July 2015 at 08:48, Peng Fan <Peng.Fan@freescale.com> 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>
>>
>>It looks correct but I have code style comments - see below. They all
>>apply globally.
>
>Ok. Will fix them in V2.
>
>>
>>> ---
>>>  drivers/power/regulator/Kconfig    |   8 +
>>>  drivers/power/regulator/Makefile   |   1 +
>>>  drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
>>>  include/power/pfuze100_pmic.h      |  24 +-
>>>  4 files changed, 625 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..6c513e9
>>> --- /dev/null
>>> +++ b/drivers/power/regulator/pfuze100.c
>>> @@ -0,0 +1,596 @@
>>> +#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;
>>
>>blank line between declarations and rest - please fix global
>
>Will fix in V2.
>
>>
>>> +       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 = NULL;
>>
>>Can't this just be a single pointer?

After a thought, a single pointer can not make it work.

struct pfuze100_regulator_desc is for per regulator.
If use a single pointer, then "desc = dev_get_platdata(dev);" can not
change the platdata for each regulator. I need to use "struct xx **" to
reflect the change into platdata for each regulator.

[...]

Regards,
Peng.
Simon Glass Aug. 4, 2015, 12:46 p.m. UTC | #4 
Hi Peng,

On 3 August 2015 at 20:08, Peng Fan <b51431@freescale.com> wrote:
> On Mon, Aug 03, 2015 at 08:38:49AM +0800, Peng Fan wrote:
>>Hi Simon,
>>On Sun, Aug 02, 2015 at 04:30:52PM -0600, Simon Glass wrote:
>>>Hi Peng,
>>>
>>>On 28 July 2015 at 08:48, Peng Fan <Peng.Fan@freescale.com> 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>
>>>
>>>It looks correct but I have code style comments - see below. They all
>>>apply globally.
>>
>>Ok. Will fix them in V2.
>>
>>>
>>>> ---
>>>>  drivers/power/regulator/Kconfig    |   8 +
>>>>  drivers/power/regulator/Makefile   |   1 +
>>>>  drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
>>>>  include/power/pfuze100_pmic.h      |  24 +-
>>>>  4 files changed, 625 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..6c513e9
>>>> --- /dev/null
>>>> +++ b/drivers/power/regulator/pfuze100.c
>>>> @@ -0,0 +1,596 @@
>>>> +#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;
>>>
>>>blank line between declarations and rest - please fix global
>>
>>Will fix in V2.
>>
>>>
>>>> +       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 = NULL;
>>>
>>>Can't this just be a single pointer?
>
> After a thought, a single pointer can not make it work.
>
> struct pfuze100_regulator_desc is for per regulator.
> If use a single pointer, then "desc = dev_get_platdata(dev);" can not
> change the platdata for each regulator. I need to use "struct xx **" to
> reflect the change into platdata for each regulator.
>
> [...]
>
>

I see that you a searching your table based the device name. Normally
we name the regulators LDO1, LDO2 and use the number (1, 2) to find
the right device. Can you please point me to your device tree
contents?

You could make the platdata be something like

struct pfuze_platdata {
   struct pfuze100_regulator_desc *desc;
};

I think the code would be clearer that way. You can assign to the desc
member instead of a double pointer indirect.

Regards,
Simon
Przemyslaw Marczak Aug. 4, 2015, 1:16 p.m. UTC | #5 
Hello Simon,

On 08/04/2015 02:46 PM, Simon Glass wrote:
> Hi Peng,
>
> On 3 August 2015 at 20:08, Peng Fan <b51431@freescale.com> wrote:
>> On Mon, Aug 03, 2015 at 08:38:49AM +0800, Peng Fan wrote:
>>> Hi Simon,
>>> On Sun, Aug 02, 2015 at 04:30:52PM -0600, Simon Glass wrote:
>>>> Hi Peng,
>>>>
>>>> On 28 July 2015 at 08:48, Peng Fan <Peng.Fan@freescale.com> 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>
>>>>
>>>> It looks correct but I have code style comments - see below. They all
>>>> apply globally.
>>>
>>> Ok. Will fix them in V2.
>>>
>>>>
>>>>> ---
>>>>>   drivers/power/regulator/Kconfig    |   8 +
>>>>>   drivers/power/regulator/Makefile   |   1 +
>>>>>   drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
>>>>>   include/power/pfuze100_pmic.h      |  24 +-
>>>>>   4 files changed, 625 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..6c513e9
>>>>> --- /dev/null
>>>>> +++ b/drivers/power/regulator/pfuze100.c
>>>>> @@ -0,0 +1,596 @@
>>>>> +#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;
>>>>
>>>> blank line between declarations and rest - please fix global
>>>
>>> Will fix in V2.
>>>
>>>>
>>>>> +       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 = NULL;
>>>>
>>>> Can't this just be a single pointer?
>>
>> After a thought, a single pointer can not make it work.
>>
>> struct pfuze100_regulator_desc is for per regulator.
>> If use a single pointer, then "desc = dev_get_platdata(dev);" can not
>> change the platdata for each regulator. I need to use "struct xx **" to
>> reflect the change into platdata for each regulator.
>>
>> [...]
>>
>>
>
> I see that you a searching your table based the device name. Normally
> we name the regulators LDO1, LDO2 and use the number (1, 2) to find
> the right device. Can you please point me to your device tree
> contents?
>
> You could make the platdata be something like
>
> struct pfuze_platdata {
>     struct pfuze100_regulator_desc *desc;
> };
>
> I think the code would be clearer that way. You can assign to the desc
> member instead of a double pointer indirect.
>
> Regards,
> Simon
>

I think, that we can use a single pointer for this, because function 
se_desc() returns a pointer to the single descriptor, for each regulator 
device. Please look at my comments to this patch.

Best regards,
Peng Fan Aug. 4, 2015, 1:16 p.m. UTC | #6 
On Tue, Aug 04, 2015 at 06:46:09AM -0600, Simon Glass wrote:
>Hi Peng,
>
>On 3 August 2015 at 20:08, Peng Fan <b51431@freescale.com> wrote:
>> On Mon, Aug 03, 2015 at 08:38:49AM +0800, Peng Fan wrote:
>>>Hi Simon,
>>>On Sun, Aug 02, 2015 at 04:30:52PM -0600, Simon Glass wrote:
>>>>Hi Peng,
>>>>
>>>>On 28 July 2015 at 08:48, Peng Fan <Peng.Fan@freescale.com> 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>
>>>>
>>>>It looks correct but I have code style comments - see below. They all
>>>>apply globally.
>>>
>>>Ok. Will fix them in V2.
>>>
>>>>
>>>>> ---
>>>>>  drivers/power/regulator/Kconfig    |   8 +
>>>>>  drivers/power/regulator/Makefile   |   1 +
>>>>>  drivers/power/regulator/pfuze100.c | 596 +++++++++++++++++++++++++++++++++++++
>>>>>  include/power/pfuze100_pmic.h      |  24 +-
>>>>>  4 files changed, 625 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..6c513e9
>>>>> --- /dev/null
>>>>> +++ b/drivers/power/regulator/pfuze100.c
>>>>> @@ -0,0 +1,596 @@
>>>>> +#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;
>>>>
>>>>blank line between declarations and rest - please fix global
>>>
>>>Will fix in V2.
>>>
>>>>
>>>>> +       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 = NULL;
>>>>
>>>>Can't this just be a single pointer?
>>
>> After a thought, a single pointer can not make it work.
>>
>> struct pfuze100_regulator_desc is for per regulator.
>> If use a single pointer, then "desc = dev_get_platdata(dev);" can not
>> change the platdata for each regulator. I need to use "struct xx **" to
>> reflect the change into platdata for each regulator.
>>
>> [...]
>>
>>
>
>I see that you a searching your table based the device name. Normally
>we name the regulators LDO1, LDO2 and use the number (1, 2) to find
>the right device. Can you please point me to your device tree
>contents?

I replied in v2 patch set. Still paste url here,
http://lxr.free-electrons.com/source/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
line from 208 to 306.
>
>You could make the platdata be something like
>
>struct pfuze_platdata {
>   struct pfuze100_regulator_desc *desc;
>};
>
>I think the code would be clearer that way. You can assign to the desc
>member instead of a double pointer indirect.

Thanks. Will consider to use this way.

Regards,
Peng.
diff mbox

Patch

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..6c513e9
--- /dev/null
+++ b/drivers/power/regulator/pfuze100.c
@@ -0,0 +1,596 @@ 
+#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 = NULL;
+
+	desc = dev_get_platdata(dev);
+
+	switch (dev_get_driver_data(dev_get_parent(dev))) {
+	case PFUZE100:
+		*desc = se_desc(pfuze100_regulators,
+				ARRAY_SIZE(pfuze100_regulators),
+				dev->name);
+		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;
+	}
+
+	uc_pdata = dev_get_uclass_platdata(dev);
+
+	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;
+	int ret;
+	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")) {
+				ret = pmic_read(dev->parent, desc->vsel_reg,
+						&val, 1);
+				if (ret)
+					return ret;
+
+				val &= SWBST_MODE_MASK;
+				val >>= SWBST_MODE_SHIFT;
+				*opmode = val;
+
+				return 0;
+			}
+			ret = pmic_read(dev->parent,
+					desc->vsel_reg + PFUZE100_MODE_OFFSET,
+					&val, 1);
+			if (ret)
+				return ret;
+
+			val &= SW_MODE_MASK;
+			val >>= SW_MODE_SHIFT;
+			*opmode = val;
+
+			return 0;
+
+		} else if (desc->type == REGULATOR_TYPE_LDO) {
+			ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+			if (ret)
+				return ret;
+
+			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")) {
+			ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+			if (ret)
+				return ret;
+
+			val &= ~SWBST_MODE_MASK;
+			val |= *opmode << SWBST_MODE_SHIFT;
+
+			ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
+			if (ret)
+				return ret;
+
+			return 0;
+		}
+		ret = pmic_read(dev->parent,
+				desc->vsel_reg + PFUZE100_MODE_OFFSET,
+				&val, 1);
+		if (ret)
+			return ret;
+
+		val &= ~SW_MODE_MASK;
+		val |= *opmode << SW_MODE_SHIFT;
+
+		ret = pmic_write(dev->parent,
+				 desc->vsel_reg + PFUZE100_MODE_OFFSET,
+				 &val, 1);
+		if (ret)
+			return ret;
+
+		return 0;
+	} else if (desc->type == REGULATOR_TYPE_LDO) {
+		ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+		if (ret)
+			return ret;
+
+		val &= ~LDO_MODE_MASK;
+		val |= *opmode << LDO_MODE_SHIFT;
+
+		ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
+		if (ret)
+			return ret;
+
+		return 0;
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int pfuze100_regulator_enable(struct udevice *dev, int op, bool *enable)
+{
+	int ret, on_off;
+	unsigned char val;
+	struct dm_regulator_uclass_platdata *uc_pdata =
+		dev_get_uclass_platdata(dev);
+	if (op == PMIC_OP_GET) {
+		if (!strcmp(dev->name, "vrefddr")) {
+			ret = pmic_read(dev->parent, PFUZE100_VREFDDRCON,
+					&val, 1);
+			if (ret)
+				return ret;
+			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")) {
+			ret = pmic_read(dev->parent, PFUZE100_VREFDDRCON,
+					&val, 1);
+			if (ret)
+				return ret;
+
+			if (val & VREFDDRCON_EN)
+				return 0;
+			val |= VREFDDRCON_EN;
+
+			return pmic_write(dev->parent, PFUZE100_VREFDDRCON,
+					  &val, 1);
+		}
+
+		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, ret;
+	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) {
+			ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+			if (ret)
+				return ret;
+			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;
+			}
+			ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+			if (ret)
+				return ret;
+			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;
+		}
+		ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+		if (ret)
+			return ret;
+		val &= ~(desc->vsel_mask);
+		val |= i;
+		ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
+		if (ret)
+			return ret;
+	} else {
+		if (uc_pdata->min_uV < 0) {
+			debug("Need to provide min_uV in dts.\n");
+			return -EINVAL;
+		}
+		ret = pmic_read(dev->parent, desc->vsel_reg, &val, 1);
+		if (ret)
+			return ret;
+
+		val &= ~(desc->vsel_mask);
+		val |= (*uV - uc_pdata->min_uV) / desc->uV_step;
+
+		ret = pmic_write(dev->parent, desc->vsel_reg, &val, 1);
+		if (ret)
+			return ret;
+	}
+
+	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)
+{
+	bool enable = false;
+	int ret;
+
+	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,
+	.platdata_auto_alloc_size = sizeof(struct pfuze100_regulator_desc **),
+};
diff --git a/include/power/pfuze100_pmic.h b/include/power/pfuze100_pmic.h
index b13a780..36cc212 100644
--- a/include/power/pfuze100_pmic.h
+++ b/include/power/pfuze100_pmic.h
@@ -60,6 +60,13 @@  enum {
 	PMIC_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
  */
@@ -136,6 +143,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
@@ -184,7 +194,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
  */
@@ -197,10 +212,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