[PATCH v2 1/2] regulator: rk8xx: Fix buck get and set enabled state on RK806

Wrong POWER_EN reg is used to get and set enabled state for the RK806 buck 4 and 8 regulators, also wrong POWER_SLP_EN0 bit is used for suspend state for the RK806 buck 1-8 regulators.
Fix this by not adding one to the zero based buck variable.
Fixes: f172575d92cd ("power: rk8xx: add support for RK806") Signed-off-by: Jonas Karlman jonas@kwiboo.se Reviewed-by: Kever Yang kever.yang@rock-chips.com Reviewed-by: Quentin Schulz quentin.schulz@cherry.de --- v2: - Move buck + 1 related changes into a separate patch - Collect r-b tags --- drivers/power/regulator/rk8xx.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c index 34e61511d884..b910943c2c46 100644 --- a/drivers/power/regulator/rk8xx.c +++ b/drivers/power/regulator/rk8xx.c @@ -415,7 +415,7 @@ static int _buck_set_enable(struct udevice *pmic, int buck, bool enable) break; case RK806_ID: value = RK806_POWER_EN_CLRSETBITS(buck % 4, enable); - en_reg = RK806_POWER_EN((buck + 1) / 4); + en_reg = RK806_POWER_EN(buck / 4); ret = pmic_reg_write(pmic, en_reg, value); break; case RK808_ID: @@ -494,7 +494,7 @@ static int _buck_get_enable(struct udevice *pmic, int buck) break; case RK806_ID: mask = BIT(buck % 4); - ret = pmic_reg_read(pmic, RK806_POWER_EN((buck + 1) / 4)); + ret = pmic_reg_read(pmic, RK806_POWER_EN(buck / 4)); break; case RK808_ID: case RK818_ID: @@ -544,7 +544,7 @@ static int _buck_set_suspend_enable(struct udevice *pmic, int buck, bool enable) mask = BIT(buck + 1 - 3); } else { reg = RK806_POWER_SLP_EN0; - mask = BIT(buck + 1); + mask = BIT(buck); } ret = pmic_clrsetbits(pmic, reg, mask, enable ? mask : 0); } @@ -595,7 +595,7 @@ static int _buck_get_suspend_enable(struct udevice *pmic, int buck) mask = BIT(buck + 1 - 3); } else { reg = RK806_POWER_SLP_EN0; - mask = BIT(buck + 1); + mask = BIT(buck); } val = pmic_reg_read(pmic, reg); }

The buck variable is zero based, i.e. buck=0 match BUCK1 in datasheet.
Remove any buck + 1 calculation to be more consistent in usage of the buck variable across the different RK8xx variants in the driver.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - Move buck + 1 related changes into a separate patch - Remove all buck + 1 use to be fully consistent --- drivers/power/regulator/rk8xx.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c index b910943c2c46..375d06e3207f 100644 --- a/drivers/power/regulator/rk8xx.c +++ b/drivers/power/regulator/rk8xx.c @@ -381,7 +381,7 @@ static int _buck_set_value(struct udevice *pmic, int buck, int uvolt) val = ((uvolt - info->min_uv) / info->step_uv) + info->min_sel;
debug("%s: volt=%d, buck=%d, reg=0x%x, mask=0x%x, val=0x%x\n", - __func__, uvolt, buck + 1, info->vsel_reg, mask, val); + __func__, uvolt, buck, info->vsel_reg, mask, val);
if (priv->variant == RK816_ID) { pmic_clrsetbits(pmic, info->vsel_reg, mask, val); @@ -470,7 +470,7 @@ static int _buck_set_suspend_value(struct udevice *pmic, int buck, int uvolt) val = ((uvolt - info->min_uv) / info->step_uv) + info->min_sel;
debug("%s: volt=%d, buck=%d, reg=0x%x, mask=0x%x, val=0x%x\n", - __func__, uvolt, buck + 1, info->vsel_sleep_reg, mask, val); + __func__, uvolt, buck, info->vsel_sleep_reg, mask, val);
return pmic_clrsetbits(pmic, info->vsel_sleep_reg, mask, val); } @@ -539,9 +539,10 @@ static int _buck_set_suspend_enable(struct udevice *pmic, int buck, bool enable) { u8 reg;
- if (buck + 1 >= 9) { + if (buck >= 8) { + /* BUCK9 and BUCK10 */ reg = RK806_POWER_SLP_EN1; - mask = BIT(buck + 1 - 3); + mask = BIT(buck - 2); } else { reg = RK806_POWER_SLP_EN0; mask = BIT(buck); @@ -590,9 +591,10 @@ static int _buck_get_suspend_enable(struct udevice *pmic, int buck) { u8 reg;
- if (buck + 1 >= 9) { + if (buck >= 8) { + /* BUCK9 and BUCK10 */ reg = RK806_POWER_SLP_EN1; - mask = BIT(buck + 1 - 3); + mask = BIT(buck - 2); } else { reg = RK806_POWER_SLP_EN0; mask = BIT(buck);

On 2024/9/18 04:59, Jonas Karlman wrote:
The buck variable is zero based, i.e. buck=0 match BUCK1 in datasheet.
Remove any buck + 1 calculation to be more consistent in usage of the buck variable across the different RK8xx variants in the driver.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
- Move buck + 1 related changes into a separate patch
- Remove all buck + 1 use to be fully consistent
drivers/power/regulator/rk8xx.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c index b910943c2c46..375d06e3207f 100644 --- a/drivers/power/regulator/rk8xx.c +++ b/drivers/power/regulator/rk8xx.c @@ -381,7 +381,7 @@ static int _buck_set_value(struct udevice *pmic, int buck, int uvolt) val = ((uvolt - info->min_uv) / info->step_uv) + info->min_sel;
debug("%s: volt=%d, buck=%d, reg=0x%x, mask=0x%x, val=0x%x\n",
__func__, uvolt, buck + 1, info->vsel_reg, mask, val);
__func__, uvolt, buck, info->vsel_reg, mask, val);
if (priv->variant == RK816_ID) { pmic_clrsetbits(pmic, info->vsel_reg, mask, val);
@@ -470,7 +470,7 @@ static int _buck_set_suspend_value(struct udevice *pmic, int buck, int uvolt) val = ((uvolt - info->min_uv) / info->step_uv) + info->min_sel;
debug("%s: volt=%d, buck=%d, reg=0x%x, mask=0x%x, val=0x%x\n",
__func__, uvolt, buck + 1, info->vsel_sleep_reg, mask, val);
__func__, uvolt, buck, info->vsel_sleep_reg, mask, val);
return pmic_clrsetbits(pmic, info->vsel_sleep_reg, mask, val); }
@@ -539,9 +539,10 @@ static int _buck_set_suspend_enable(struct udevice *pmic, int buck, bool enable) { u8 reg;
if (buck + 1 >= 9) {
if (buck >= 8) {
/* BUCK9 and BUCK10 */ reg = RK806_POWER_SLP_EN1;
mask = BIT(buck + 1 - 3);
mask = BIT(buck - 2); } else { reg = RK806_POWER_SLP_EN0; mask = BIT(buck);
@@ -590,9 +591,10 @@ static int _buck_get_suspend_enable(struct udevice *pmic, int buck) { u8 reg;
if (buck + 1 >= 9) {
if (buck >= 8) {
/* BUCK9 and BUCK10 */ reg = RK806_POWER_SLP_EN1;
mask = BIT(buck + 1 - 3);
mask = BIT(buck - 2); } else { reg = RK806_POWER_SLP_EN0; mask = BIT(buck);
participants (2)
-
Jonas Karlman
-
Kever Yang