Re: Re: [PATCH v5 1/6] dm: regulator: support regulator more state

["zhangqing@rock-chips.com" <zhangqing@rock-chips.com>]

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.zhang@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, Elaine@denx.de wrote:
> From: Joseph Chen <chenjh@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 <chenjh@rock-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> Reviewed-by: Kever Yang<kever.yang@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)
>  {
>