Re: [PATCH] regulator: rk808: Add support setting suspend voltage

[Doug Anderson]

Chris,

On Tue, Oct 7, 2014 at 1:43 AM, Chris Zhong <zyw@xxxxxxxxxxxxxx> wrote:
> support setting suspend voltage and disable regulator in suspend.
>
> Signed-off-by: Chris Zhong <zyw@xxxxxxxxxxxxxx>
>
> ---
>
>  drivers/regulator/rk808-regulator.c |   37 +++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
> index e305416..6687e6e 100644
> --- a/drivers/regulator/rk808-regulator.c
> +++ b/drivers/regulator/rk808-regulator.c
> @@ -36,6 +36,12 @@
>  #define RK808_RAMP_RATE_6MV_PER_US     (2 << RK808_RAMP_RATE_OFFSET)
>  #define RK808_RAMP_RATE_10MV_PER_US    (3 << RK808_RAMP_RATE_OFFSET)
>
> +/* Offset from XXX_ON_VSEL to XXX_SLP_VSEL */
> +#define RK808_SLP_REG_OFFSET 1
> +
> +/* Offset from XXX_EN_REG to SLEEP_SET_OFF_XXX */
> +#define RK808_SLP_SET_OFF_REG_OFFSET 2
> +
>  static const int rk808_buck_config_regs[] = {
>         RK808_BUCK1_CONFIG_REG,
>         RK808_BUCK2_CONFIG_REG,
> @@ -91,6 +97,32 @@ static int rk808_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
>                                   RK808_RAMP_RATE_MASK, ramp_value);
>  }
>
> +int rk808_set_suspend_voltage(struct regulator_dev *rdev, int uv)
> +{
> +       unsigned int reg;
> +       int sel = regulator_map_voltage_linear_range(rdev, uv, uv);
> +
> +       if (sel < 0)
> +               return -EINVAL;
> +
> +       reg = rdev->desc->vsel_reg + RK808_SLP_REG_OFFSET;
> +
> +       return regmap_update_bits(rdev->regmap, reg,
> +                                 rdev->desc->vsel_mask,
> +                                 sel);
> +}
> +
> +int rk808_set_suspend_disable(struct regulator_dev *rdev)
> +{
> +       unsigned int reg;
> +
> +       reg = rdev->desc->enable_reg + RK808_SLP_SET_OFF_REG_OFFSET;
> +
> +       return regmap_update_bits(rdev->regmap, reg,
> +                                 rdev->desc->enable_mask,
> +                                 rdev->desc->enable_mask);
> +}
> +
>  static struct regulator_ops rk808_buck1_2_ops = {
>         .list_voltage           = regulator_list_voltage_linear_range,
>         .map_voltage            = regulator_map_voltage_linear_range,
> @@ -100,6 +132,8 @@ static struct regulator_ops rk808_buck1_2_ops = {
>         .disable                = regulator_disable_regmap,
>         .is_enabled             = regulator_is_enabled_regmap,
>         .set_ramp_delay         = rk808_set_ramp_delay,
> +       .set_suspend_voltage    = rk808_set_suspend_voltage,
> +       .set_suspend_disable    = rk808_set_suspend_disable,
>  };
>
>  static struct regulator_ops rk808_reg_ops = {
> @@ -110,12 +144,15 @@ static struct regulator_ops rk808_reg_ops = {
>         .enable                 = regulator_enable_regmap,
>         .disable                = regulator_disable_regmap,
>         .is_enabled             = regulator_is_enabled_regmap,
> +       .set_suspend_voltage    = rk808_set_suspend_voltage,
> +       .set_suspend_disable    = rk808_set_suspend_disable,
>  };
>
>  static struct regulator_ops rk808_switch_ops = {
>         .enable = regulator_enable_regmap,
>         .disable = regulator_disable_regmap,
>         .is_enabled = regulator_is_enabled_regmap,
> +       .set_suspend_disable    = rk808_set_suspend_disable,
>  };
>
>  static const struct regulator_desc rk808_reg[] = {

Your patch looks right to me.

Reviewed-by: Doug Anderson <dianders@xxxxxxxxxxxx>

I don't have all the right patches to test this right now.  Hopefully
you can point me at what we're using right now.  I'd expect that this
will need the patches that Chanwoo and Javier are working on, so I've
added them to this.

One point of curiosity (maybe this is a question for Chanwoo and
Javier): I'd expect that if someone didn't explicitly setup a "suspend
voltage" that their voltage would just be left alone at suspend time.
I believe that won't be the case for your driver.  The rk808 will (I
think) automatically transition to the "suspend voltage" settings for
ALL regulators at suspend time.  If you didn't explicitly set the
suspend voltage then you'll move to whatever the default voltage is,
right?

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/