Hi Jonas,
On 8/31/24 12:42 AM, Jonas Karlman wrote:
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")
Shoot, I made a lot of mistakes in that driver :/
Thanks for catching those :)
Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
---
drivers/power/regulator/rk8xx.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
index 34e61511d884..3f5ec02b3824 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:
@@ -541,10 +541,10 @@ static int _buck_set_suspend_enable(struct udevice *pmic,
int buck, bool enable)
if (buck + 1 >= 9) {
reg = RK806_POWER_SLP_EN1;
- mask = BIT(buck + 1 - 3);
+ mask = BIT(buck - 2);
I like my (+ 1 - 3) here to match buck + 1 above. buck + 1 represents
the buck number in the datasheet (index starts at one), so you need to
subtract 3 to that index to find the bit index in the register.
} else {
reg = RK806_POWER_SLP_EN0;
- mask = BIT(buck + 1);
+ mask = BIT(buck);
}
ret = pmic_clrsetbits(pmic, reg, mask, enable ? mask :
0);
}
@@ -592,10 +592,10 @@ static int _buck_get_suspend_enable(struct udevice *pmic,
int buck)
if (buck + 1 >= 9) {
reg = RK806_POWER_SLP_EN1;
- mask = BIT(buck + 1 - 3);
+ mask = BIT(buck - 2);
Ditto.
Just a matter of taste though!
Reviewed-by: Quentin Schulz <quentin.sch...@cherry.de>
@Kever Since it's a bugfix, can we have this in your next MR for
v2024.10 please? Thanks!
Thanks!
Quentin
