Hi, Please ignore these two emails, they are sent in error.
[PATCH v5 0/6] power: pmic: support more PMIC PATCH v5 1/6] dm: regulator: support regulator more state 张晴 瑞芯微电子股份有限公司 Rockchip Electronics Co.,Ltd 地址:福建省福州市铜盘路软件大道89号软件园A区21号楼 Add:No.21 Building, A District, No.89 Software Boulevard Fuzhou, Fujian 350003, P.R.China Tel:+86-0591-83991906-8601 邮编:350003 E-mail:elaine.zh...@rock-chips.com From: Jaehoon Chung Date: 2021-05-27 06:18 To: Elaine; Zhang; sjg; philipp.tomsich; kever.yang; lukma CC: zhangqing; u-boot; chenjh Subject: Re: [PATCH v5 1/6] dm: regulator: support regulator more state Hi, On 5/26/21 5:46 PM, ela...@denx.de wrote: > From: Joseph Chen <che...@rock-chips.com> Is it right patch? This patch had been already applied. Best Regards, Jaehoon Chung > > support parse regulator standard property: > regulator-off-in-suspend; > regulator-init-microvolt; > regulator-suspend-microvolt: > regulator_get_suspend_enable > regulator_set_suspend_enable > regulator_get_suspend_value > regulator_set_suspend_value > > Signed-off-by: Joseph Chen <che...@rock-chips.com> > Signed-off-by: Elaine Zhang <zhangq...@rock-chips.com> > Reviewed-by: Kever Yang<kever.y...@rock-chips.com> > --- > doc/device-tree-bindings/regulator/regulator.txt | 27 +++++++++ > drivers/power/regulator/regulator-uclass.c | 70 > ++++++++++++++++++++++++ > include/power/regulator.h | 64 ++++++++++++++++++++++ > test/dm/regulator.c | 57 +++++++++++++++++++ > 4 files changed, 218 insertions(+) > > diff --git a/doc/device-tree-bindings/regulator/regulator.txt > b/doc/device-tree-bindings/regulator/regulator.txt > index 4ba642b7c77f..6c9a02120fde 100644 > --- a/doc/device-tree-bindings/regulator/regulator.txt > +++ b/doc/device-tree-bindings/regulator/regulator.txt > @@ -36,6 +36,28 @@ Optional properties: > - regulator-always-on: regulator should never be disabled > - regulator-boot-on: enabled by bootloader/firmware > - regulator-ramp-delay: ramp delay for regulator (in uV/us) > +- regulator-init-microvolt: a init allowed Voltage value > +- regulator-state-(standby|mem|disk) > + type: object > + description: > + sub-nodes for regulator state in Standby, Suspend-to-RAM, and > + Suspend-to-DISK modes. Equivalent with standby, mem, and disk Linux > + sleep states. > + > + properties: > + regulator-on-in-suspend: > + description: regulator should be on in suspend state. > + type: boolean > + > + regulator-off-in-suspend: > + description: regulator should be off in suspend state. > + type: boolean > + > + regulator-suspend-microvolt: > + description: the default voltage which regulator would be set in > + suspend. This property is now deprecated, instead setting voltage > + for suspend mode via the API which regulator driver provides is > + recommended. > > Note > The "regulator-name" constraint is used for setting the device's uclass > @@ -59,7 +81,12 @@ ldo0 { > regulator-max-microvolt = <1800000>; > regulator-min-microamp = <100000>; > regulator-max-microamp = <100000>; > + regulator-init-microvolt = <1800000>; > regulator-always-on; > regulator-boot-on; > regulator-ramp-delay = <12000>; > + regulator-state-mem { > + regulator-on-in-suspend; > + regulator-suspend-microvolt = <1800000>; > + }; > }; > diff --git a/drivers/power/regulator/regulator-uclass.c > b/drivers/power/regulator/regulator-uclass.c > index 76be95bcd159..4986c87e7ba6 100644 > --- a/drivers/power/regulator/regulator-uclass.c > +++ b/drivers/power/regulator/regulator-uclass.c > @@ -77,6 +77,33 @@ int regulator_set_value(struct udevice *dev, int uV) > return ret; > } > > +int regulator_set_suspend_value(struct udevice *dev, int uV) > +{ > + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); > + struct dm_regulator_uclass_platdata *uc_pdata; > + > + uc_pdata = dev_get_uclass_platdata(dev); > + if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV) > + return -EINVAL; > + if (uc_pdata->max_uV != -ENODATA && uV > uc_pdata->max_uV) > + return -EINVAL; > + > + if (!ops->set_suspend_value) > + return -ENOSYS; > + > + return ops->set_suspend_value(dev, uV); > +} > + > +int regulator_get_suspend_value(struct udevice *dev) > +{ > + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); > + > + if (!ops->get_suspend_value) > + return -ENOSYS; > + > + return ops->get_suspend_value(dev); > +} > + > /* > * To be called with at most caution as there is no check > * before setting the actual voltage value. > @@ -170,6 +197,26 @@ int regulator_set_enable_if_allowed(struct udevice *dev, > bool enable) > return ret; > } > > +int regulator_set_suspend_enable(struct udevice *dev, bool enable) > +{ > + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); > + > + if (!ops->set_suspend_enable) > + return -ENOSYS; > + > + return ops->set_suspend_enable(dev, enable); > +} > + > +int regulator_get_suspend_enable(struct udevice *dev) > +{ > + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); > + > + if (!ops->get_suspend_enable) > + return -ENOSYS; > + > + return ops->get_suspend_enable(dev); > +} > + > int regulator_get_mode(struct udevice *dev) > { > const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); > @@ -235,6 +282,14 @@ int regulator_autoset(struct udevice *dev) > int ret = 0; > > uc_pdata = dev_get_uclass_platdata(dev); > + > + ret = regulator_set_suspend_enable(dev, uc_pdata->suspend_on); > + if (!ret && uc_pdata->suspend_on) { > + ret = regulator_set_suspend_value(dev, uc_pdata->suspend_uV); > + if (!ret) > + return ret; > + } > + > if (!uc_pdata->always_on && !uc_pdata->boot_on) > return -EMEDIUMTYPE; > > @@ -243,6 +298,8 @@ int regulator_autoset(struct udevice *dev) > > if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) > ret = regulator_set_value(dev, uc_pdata->min_uV); > + if (uc_pdata->init_uV > 0) > + ret = regulator_set_value(dev, uc_pdata->init_uV); > if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)) > ret = regulator_set_current(dev, uc_pdata->min_uA); > > @@ -363,6 +420,7 @@ static int regulator_post_bind(struct udevice *dev) > static int regulator_pre_probe(struct udevice *dev) > { > struct dm_regulator_uclass_platdata *uc_pdata; > + ofnode node; > > uc_pdata = dev_get_uclass_platdata(dev); > if (!uc_pdata) > @@ -373,6 +431,8 @@ static int regulator_pre_probe(struct udevice *dev) > -ENODATA); > uc_pdata->max_uV = dev_read_u32_default(dev, "regulator-max-microvolt", > -ENODATA); > + uc_pdata->init_uV = dev_read_u32_default(dev, "regulator-init-microvolt", > + -ENODATA); > uc_pdata->min_uA = dev_read_u32_default(dev, "regulator-min-microamp", > -ENODATA); > uc_pdata->max_uA = dev_read_u32_default(dev, "regulator-max-microamp", > @@ -382,6 +442,16 @@ static int regulator_pre_probe(struct udevice *dev) > uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay", > 0); > > + node = dev_read_subnode(dev, "regulator-state-mem"); > + if (ofnode_valid(node)) { > + uc_pdata->suspend_on = !ofnode_read_bool(node, "regulator-off-in-suspend"); > + if (ofnode_read_u32(node, "regulator-suspend-microvolt", > &uc_pdata->suspend_uV)) > + uc_pdata->suspend_uV = uc_pdata->max_uV; > + } else { > + uc_pdata->suspend_on = true; > + uc_pdata->suspend_uV = uc_pdata->max_uV; > + } > + > /* Those values are optional (-ENODATA if unset) */ > if ((uc_pdata->min_uV != -ENODATA) && > (uc_pdata->max_uV != -ENODATA) && > diff --git a/include/power/regulator.h b/include/power/regulator.h > index 6c6e2cd4f996..dd61eb4ccbde 100644 > --- a/include/power/regulator.h > +++ b/include/power/regulator.h > @@ -168,6 +168,7 @@ struct dm_regulator_uclass_platdata { > int mode_count; > int min_uV; > int max_uV; > + int init_uV; > int min_uA; > int max_uA; > unsigned int ramp_delay; > @@ -177,6 +178,8 @@ struct dm_regulator_uclass_platdata { > int flags; > u8 ctrl_reg; > u8 volt_reg; > + bool suspend_on; > + u32 suspend_uV; > }; > > /* Regulator device operations */ > @@ -194,6 +197,19 @@ struct dm_regulator_ops { > int (*set_value)(struct udevice *dev, int uV); > > /** > + * The regulator suspend output value function calls operates > + * on a micro Volts. > + * > + * get/set_suspen_value - get/set suspend mode output value > + * @dev - regulator device > + * Sets: > + * @uV - set the suspend output value [micro Volts] > + * @return output value [uV] on success or negative errno if fail. > + */ > + int (*set_suspend_value)(struct udevice *dev, int uV); > + int (*get_suspend_value)(struct udevice *dev); > + > + /** > * The regulator output current function calls operates on a micro Amps. > * > * get/set_current - get/set output current of the given output number > @@ -218,6 +234,19 @@ struct dm_regulator_ops { > int (*set_enable)(struct udevice *dev, bool enable); > > /** > + * The most basic feature of the regulator output is its enable state > + * in suspend mode. > + * > + * get/set_suspend_enable - get/set enable state of the suspend output > + * @dev - regulator device > + * Sets: > + * @enable - set true - enable or false - disable > + * @return true/false for get or -errno if fail; 0 / -errno for set. > + */ > + int (*set_suspend_enable)(struct udevice *dev, bool enable); > + int (*get_suspend_enable)(struct udevice *dev); > + > + /** > * The 'get/set_mode()' function calls should operate on a driver- > * specific mode id definitions, which should be found in: > * field 'id' of struct dm_regulator_mode. > @@ -262,6 +291,23 @@ int regulator_get_value(struct udevice *dev); > int regulator_set_value(struct udevice *dev, int uV); > > /** > + * regulator_set_suspend_value: set the suspend microvoltage value of a > given regulator. > + * > + * @dev - pointer to the regulator device > + * @uV - the output suspend value to set [micro Volts] > + * @return - 0 on success or -errno val if fails > + */ > +int regulator_set_suspend_value(struct udevice *dev, int uV); > + > +/** > + * regulator_get_suspend_value: get the suspend microvoltage value of a > given regulator. > + * > + * @dev - pointer to the regulator device > + * @return - positive output value [uV] on success or negative errno if fail. > + */ > +int regulator_get_suspend_value(struct udevice *dev); > + > +/** > * regulator_set_value_force: set the microvoltage value of a given regulator > * without any min-,max condition check > * > @@ -317,6 +363,24 @@ int regulator_set_enable(struct udevice *dev, bool > enable); > int regulator_set_enable_if_allowed(struct udevice *dev, bool enable); > > /** > + * regulator_set_suspend_enable: set regulator suspend enable state > + * > + * @dev - pointer to the regulator device > + * @enable - set true or false > + * @return - 0 on success or -errno val if fails > + */ > +int regulator_set_suspend_enable(struct udevice *dev, bool enable); > + > + > +/** > + * regulator_get_suspend_enable: get regulator suspend enable state > + * > + * @dev - pointer to the regulator device > + * @return - true/false of enable state or -errno val if fails > + */ > +int regulator_get_suspend_enable(struct udevice *dev); > + > +/** > * regulator_get_mode: get active operation mode id of a given regulator > * > * @dev - pointer to the regulator device > diff --git a/test/dm/regulator.c b/test/dm/regulator.c > index e510539542b6..b967902493dd 100644 > --- a/test/dm/regulator.c > +++ b/test/dm/regulator.c > @@ -215,6 +215,63 @@ static int dm_test_power_regulator_set_get_mode(struct > unit_test_state *uts) > } > DM_TEST(dm_test_power_regulator_set_get_mode, DM_TESTF_SCAN_FDT); > > +/* Test regulator set and get suspend Voltage method */ > +static int dm_test_power_regulator_set_get_suspend_voltage(struct > unit_test_state *uts) > +{ > + struct dm_regulator_uclass_platdata *uc_pdata; > + const struct dm_regulator_ops *ops; > + struct udevice *dev; > + const char *platname; > + int val_set, val_get; > + > + /* Set and get Voltage of BUCK1 - set to 'min' constraint */ > + platname = regulator_names[BUCK1][PLATNAME]; > + ut_assertok(regulator_get_by_platname(platname, &dev)); > + > + uc_pdata = dev_get_uclass_platdata(dev); > + ut_assert(uc_pdata); > + > + ops = dev_get_driver_ops(dev); > + > + if (ops->set_suspend_value && ops->get_suspend_value) { > + val_set = uc_pdata->suspend_uV; > + ut_assertok(regulator_set_suspend_value(dev, val_set)); > + val_get = regulator_get_suspend_value(dev); > + ut_assert(val_get >= 0); > + > + ut_asserteq(val_set, val_get); > + } > + return 0; > +} > +DM_TEST(dm_test_power_regulator_set_get_suspend_voltage, DM_TESTF_SCAN_FDT); > + > +/* Test regulator set and get suspend Enable method */ > +static int dm_test_power_regulator_set_get_suspend_enable(struct > unit_test_state *uts) > +{ > + const struct dm_regulator_ops *ops; > + const char *platname; > + struct udevice *dev; > + bool val_set = true; > + > + /* Set the Enable of LDO1 - default is disabled */ > + platname = regulator_names[LDO1][PLATNAME]; > + ut_assertok(regulator_get_by_platname(platname, &dev)); > + > + ops = dev_get_driver_ops(dev); > + > + if (ops->set_suspend_enable && ops->get_suspend_enable) { > + ut_assertok(regulator_set_suspend_enable(dev, val_set)); > + > + /* > + * Get the Enable state of LDO1 and > + * compare it with the requested one > + */ > + ut_asserteq(regulator_get_suspend_enable(dev), val_set); > + } > + return 0; > +} > +DM_TEST(dm_test_power_regulator_set_get_suspend_enable, DM_TESTF_SCAN_FDT); > + > /* Test regulator autoset method */ > static int dm_test_power_regulator_autoset(struct unit_test_state *uts) > { >