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