Re: [PATCH v2 2/5] regulator: rk8xx: Return correct voltage for buck converters

Tue, 26 Sep 2023 19:49:50 -0700 


On 2023/8/22 06:30, Jonas Karlman wrote:

From: Joseph Chen <che...@rock-chips.com>

Information from the first range group is always used to calculate the
voltage returned for buck converters. This may result in wrong voltage
reported back to the regulator_get_value caller.

Traverse all the possible BUCK ranges to fix this issue.

Fixes: addd062beacc ("power: pmic: rk816: support rk816 pmic")
Fixes: b62280745e55 ("power: pmic: rk805: support rk805 pmic")
Fixes: b4a35574b38d ("power: pmic: rk817: support rk817 pmic")
Fixes: ee30068fa574 ("power: pmic: rk809: support rk809 pmic")
Signed-off-by: Joseph Chen <che...@rock-chips.com>
[jo...@kwiboo.se: fix checkpatch error, simplify buck get_value, update commit 
message]
Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
Reviewed-by: Simon Glass <s...@chromium.org>

Reviewed-by: Kever Yang <kever.y...@rock-chips.com>

Thanks,
- Kever

---
v2:
- Simplify locating correct rk8xx_reg_info
- Update max_sel to include range up to and including vsel_mask
- Drop range_num
- Collect r-b tag

  drivers/power/regulator/rk8xx.c | 76 +++++++++++++++++----------------
  1 file changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
index e95640a39b0a..9444daa85c19 100644
--- a/drivers/power/regulator/rk8xx.c
+++ b/drivers/power/regulator/rk8xx.c
@@ -88,62 +88,63 @@ struct rk8xx_reg_info {
        u8 config_reg;
        u8 vsel_mask;
        u8 min_sel;
+       u8 max_sel;
  };

  
  static const struct rk8xx_reg_info rk808_buck[] = {

-       { 712500,   12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK808_BUCK_VSEL_MASK, },
-       { 712500,   12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK808_BUCK_VSEL_MASK, },
-       { 712500,   12500, NA,                NA,                 
REG_BUCK3_CONFIG, RK808_BUCK_VSEL_MASK, },
-       { 1800000, 100000, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK808_BUCK4_VSEL_MASK, },
+       {  712500,  12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK808_BUCK_VSEL_MASK, 0x00, 0x3f },
+       {  712500,  12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK808_BUCK_VSEL_MASK, 0x00, 0x3f },
+       {      NA,     NA,                NA,                 NA, 
REG_BUCK3_CONFIG,                    NA,   NA,   NA },
+       { 1800000, 100000, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK808_BUCK4_VSEL_MASK, 0x00, 0x0f },
  };

  
  static const struct rk8xx_reg_info rk816_buck[] = {

        /* buck 1 */
-       {  712500,  12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x00, },
-       { 1800000, 200000, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x3c, },
-       { 2300000,      0, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x3f, },
+       {  712500,  12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK818_BUCK_VSEL_MASK, 0x00, 0x3b },
+       { 1800000, 200000, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK818_BUCK_VSEL_MASK, 0x3c, 0x3e },
+       { 2300000,      0, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK818_BUCK_VSEL_MASK, 0x3f, 0x3f },
        /* buck 2 */
-       {  712500,  12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x00, },
-       { 1800000, 200000, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x3c, },
-       { 2300000,      0, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x3f, },
+       {  712500,  12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK818_BUCK_VSEL_MASK, 0x00, 0x3b },
+       { 1800000, 200000, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK818_BUCK_VSEL_MASK, 0x3c, 0x3e },
+       { 2300000,      0, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK818_BUCK_VSEL_MASK, 0x3f, 0x3f },
        /* buck 3 */
-       { 712500,   12500, NA,                NA,                 
REG_BUCK3_CONFIG, RK818_BUCK_VSEL_MASK, },
+       {      NA,     NA,                NA,                 NA, 
REG_BUCK3_CONFIG,                    NA,   NA,   NA },
        /* buck 4 */
-       {  800000, 100000, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, },
+       {  800000, 100000, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, 0x00, 0x1f },
  };

  
  static const struct rk8xx_reg_info rk809_buck5[] = {

        /* buck 5 */
-       { 1500000,      0, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x00, },
-       { 1800000, 200000, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x01, },
-       { 2800000, 200000, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x04, },
-       { 3300000, 300000, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x06, },
+       { 1500000,      0, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x00, 0x00 },
+       { 1800000, 200000, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x01, 0x03 },
+       { 2800000, 200000, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x04, 0x05 },
+       { 3300000, 300000, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, 
RK809_BUCK5_VSEL_MASK, 0x06, 0x07 },
  };

  
  static const struct rk8xx_reg_info rk817_buck[] = {

        /* buck 1 */
-       {  500000,  12500, RK817_BUCK_ON_VSEL(1), RK817_BUCK_SLP_VSEL(1), 
RK817_BUCK_CONFIG(1), RK817_BUCK_VSEL_MASK, 0x00, },
-       { 1500000, 100000, RK817_BUCK_ON_VSEL(1), RK817_BUCK_SLP_VSEL(1), 
RK817_BUCK_CONFIG(1), RK817_BUCK_VSEL_MASK, 0x50, },
-       { 2400000,      0, RK817_BUCK_ON_VSEL(1), RK817_BUCK_SLP_VSEL(1), 
RK817_BUCK_CONFIG(1), RK817_BUCK_VSEL_MASK, 0x59, },
+       {  500000,  12500, RK817_BUCK_ON_VSEL(1), RK817_BUCK_SLP_VSEL(1), 
RK817_BUCK_CONFIG(1), RK817_BUCK_VSEL_MASK, 0x00, 0x4f },
+       { 1500000, 100000, RK817_BUCK_ON_VSEL(1), RK817_BUCK_SLP_VSEL(1), 
RK817_BUCK_CONFIG(1), RK817_BUCK_VSEL_MASK, 0x50, 0x58 },
+       { 2400000,      0, RK817_BUCK_ON_VSEL(1), RK817_BUCK_SLP_VSEL(1), 
RK817_BUCK_CONFIG(1), RK817_BUCK_VSEL_MASK, 0x59, 0x7f },
        /* buck 2 */
-       {  500000,  12500, RK817_BUCK_ON_VSEL(2), RK817_BUCK_SLP_VSEL(2), 
RK817_BUCK_CONFIG(2), RK817_BUCK_VSEL_MASK, 0x00, },
-       { 1500000, 100000, RK817_BUCK_ON_VSEL(2), RK817_BUCK_SLP_VSEL(2), 
RK817_BUCK_CONFIG(2), RK817_BUCK_VSEL_MASK, 0x50, },
-       { 2400000,      0, RK817_BUCK_ON_VSEL(2), RK817_BUCK_SLP_VSEL(2), 
RK817_BUCK_CONFIG(2), RK817_BUCK_VSEL_MASK, 0x59, },
+       {  500000,  12500, RK817_BUCK_ON_VSEL(2), RK817_BUCK_SLP_VSEL(2), 
RK817_BUCK_CONFIG(2), RK817_BUCK_VSEL_MASK, 0x00, 0x4f },
+       { 1500000, 100000, RK817_BUCK_ON_VSEL(2), RK817_BUCK_SLP_VSEL(2), 
RK817_BUCK_CONFIG(2), RK817_BUCK_VSEL_MASK, 0x50, 0x58 },
+       { 2400000,      0, RK817_BUCK_ON_VSEL(2), RK817_BUCK_SLP_VSEL(2), 
RK817_BUCK_CONFIG(2), RK817_BUCK_VSEL_MASK, 0x59, 0x7f },
        /* buck 3 */
-       {  500000,  12500, RK817_BUCK_ON_VSEL(3), RK817_BUCK_SLP_VSEL(3), 
RK817_BUCK_CONFIG(3), RK817_BUCK_VSEL_MASK, 0x00, },
-       { 1500000, 100000, RK817_BUCK_ON_VSEL(3), RK817_BUCK_SLP_VSEL(3), 
RK817_BUCK_CONFIG(3), RK817_BUCK_VSEL_MASK, 0x50, },
-       { 2400000,      0, RK817_BUCK_ON_VSEL(3), RK817_BUCK_SLP_VSEL(3), 
RK817_BUCK_CONFIG(3), RK817_BUCK_VSEL_MASK, 0x59, },
+       {  500000,  12500, RK817_BUCK_ON_VSEL(3), RK817_BUCK_SLP_VSEL(3), 
RK817_BUCK_CONFIG(3), RK817_BUCK_VSEL_MASK, 0x00, 0x4f },
+       { 1500000, 100000, RK817_BUCK_ON_VSEL(3), RK817_BUCK_SLP_VSEL(3), 
RK817_BUCK_CONFIG(3), RK817_BUCK_VSEL_MASK, 0x50, 0x58 },
+       { 2400000,      0, RK817_BUCK_ON_VSEL(3), RK817_BUCK_SLP_VSEL(3), 
RK817_BUCK_CONFIG(3), RK817_BUCK_VSEL_MASK, 0x59, 0x7f },
        /* buck 4 */
-       {  500000,  12500, RK817_BUCK_ON_VSEL(4), RK817_BUCK_SLP_VSEL(4), 
RK817_BUCK_CONFIG(4), RK817_BUCK_VSEL_MASK, 0x00, },
-       { 1500000, 100000, RK817_BUCK_ON_VSEL(4), RK817_BUCK_SLP_VSEL(4), 
RK817_BUCK_CONFIG(4), RK817_BUCK_VSEL_MASK, 0x50, },
-       { 3400000,      0, RK817_BUCK_ON_VSEL(4), RK817_BUCK_SLP_VSEL(4), 
RK817_BUCK_CONFIG(4), RK817_BUCK_VSEL_MASK, 0x63, },
+       {  500000,  12500, RK817_BUCK_ON_VSEL(4), RK817_BUCK_SLP_VSEL(4), 
RK817_BUCK_CONFIG(4), RK817_BUCK_VSEL_MASK, 0x00, 0x4f },
+       { 1500000, 100000, RK817_BUCK_ON_VSEL(4), RK817_BUCK_SLP_VSEL(4), 
RK817_BUCK_CONFIG(4), RK817_BUCK_VSEL_MASK, 0x50, 0x62 },
+       { 3400000,      0, RK817_BUCK_ON_VSEL(4), RK817_BUCK_SLP_VSEL(4), 
RK817_BUCK_CONFIG(4), RK817_BUCK_VSEL_MASK, 0x63, 0x7f },
  };

  
  static const struct rk8xx_reg_info rk818_buck[] = {

-       { 712500,   12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, },
-       { 712500,   12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, },
-       { 712500,   12500, NA,                NA,                 
REG_BUCK3_CONFIG, RK818_BUCK_VSEL_MASK, },
-       { 1800000, 100000, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, },
+       { 712500,   12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, 
REG_BUCK1_CONFIG,  RK818_BUCK_VSEL_MASK, 0x00, 0x3f },
+       { 712500,   12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, 
REG_BUCK2_CONFIG,  RK818_BUCK_VSEL_MASK, 0x00, 0x3f },
+       {     NA,      NA,                NA,                 NA, 
REG_BUCK3_CONFIG,                    NA,   NA,   NA },
+       { 1800000, 100000, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, 
REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, 0x00, 0x1f },
  };

  
  #ifdef ENABLE_DRIVER

@@ -706,7 +707,6 @@ static int _ldo_get_suspend_enable(struct udevice *pmic, 
int ldo)
  static int buck_get_value(struct udevice *dev)
  {
        int buck = dev->driver_data - 1;
-       /* We assume level-1 voltage is enough for usage in U-Boot */
        const struct rk8xx_reg_info *info = get_buck_reg(dev->parent, buck, 0);
        int mask = info->vsel_mask;
        int ret, val;
@@ -717,9 +717,12 @@ static int buck_get_value(struct udevice *dev)
        ret = pmic_reg_read(dev->parent, info->vsel_reg);
        if (ret < 0)
                return ret;
+
        val = ret & mask;
+       while (val > info->max_sel)
+               info++;

  
-	return info->min_uv + val * info->step_uv;

+       return info->min_uv + (val - info->min_sel) * info->step_uv;
  }

  
  static int buck_set_value(struct udevice *dev, int uvolt)

@@ -732,7 +735,6 @@ static int buck_set_value(struct udevice *dev, int uvolt)
  static int buck_get_suspend_value(struct udevice *dev)
  {
        int buck = dev->driver_data - 1;
-       /* We assume level-1 voltage is enough for usage in U-Boot */
        const struct rk8xx_reg_info *info = get_buck_reg(dev->parent, buck, 0);
        int mask = info->vsel_mask;
        int ret, val;
@@ -745,8 +747,10 @@ static int buck_get_suspend_value(struct udevice *dev)
                return ret;

  
  	val = ret & mask;

+       while (val > info->max_sel)
+               info++;

  
-	return info->min_uv + val * info->step_uv;

+       return info->min_uv + (val - info->min_sel) * info->step_uv;
  }

  
  static int buck_set_suspend_value(struct udevice *dev, int uvolt)














    











					Reply via email to