mfd: tps65910: Correct power-off programming sequence
PATCH v2 1/1 mfd: tps65910: Correct power-off programming sequence
This patch fixes system shutdown on a devices that use TPS65910 as a system's power controller. In accordance to the TPS65910 datasheet, the PMIC's state-machine transitions into the OFF state only when DEV_OFF bit of DEVCTRL_REG is set. The ON / SLEEP states also should be cleared, otherwise PMIC won't get into a proper state on shutdown. Devices like Nexus 7 tablet and Ouya game console are now shutting down properly.
---
Changelog:
v2: - Now using a single tps65910_reg_update_bits() instead of set+clear.
Thanks to Michał Mirosław for the suggestion.
drivers/mfd/tps65910.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/mfd/tps65910.c +7 -2
@@ -440,8 +440,13 @@ static void tps65910_power_off(void) DEVCTRL_PWR_OFF_MASK) < 0)return ; -tps65910_reg_clear_bits (tps65910,TPS65910_DEVCTRL , -DEVCTRL_DEV_ON_MASK ); +if (tps65910_reg_clear_bits (tps65910,TPS65910_DEVCTRL , +DEVCTRL_DEV_SLP_MASK ) <0 ) +return ; + +tps65910_reg_update_bits (tps65910,TPS65910_DEVCTRL , +DEVCTRL_DEV_OFF_MASK |DEVCTRL_DEV_ON_MASK , +DEVCTRL_DEV_OFF_MASK ); } static int tps65910_i2c_probe(struct i2c_client *i2c,
Signature · 1 line
2.26.0
Quoted 7 lines · Dmitry Osipenko
This patch fixes system shutdown on a devices that use TPS65910 as a system's power controller. In accordance to the TPS65910 datasheet, the PMIC's state-machine transitions into the OFF state only when
DEV_OFFbit ofDEVCTRL_REGis set. The ON / SLEEP states also should be cleared, otherwise PMIC won't get into a proper state on shutdown. Devices like Nexus 7 tablet and Ouya game console are now shutting down properly.
Quoted 8 lines
Signed-off-by Dmitry Osipenko <[email protected]>---
Changelog:
v2: - Now using a single
tps65910_reg_update_bits()instead of set+clear.
Thanks to Michał Mirosław for the suggestion.
Michał should review.
Quoted 23 lines
drivers/mfd/tps65910.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/mfd/tps65910.c +7
-2@@ -440,8 +440,13 @@ static void tps65910_power_off(void) DEVCTRL_PWR_OFF_MASK) < 0)return ; -tps65910_reg_clear_bits (tps65910,TPS65910_DEVCTRL , -DEVCTRL_DEV_ON_MASK ); +if (tps65910_reg_clear_bits (tps65910,TPS65910_DEVCTRL , +DEVCTRL_DEV_SLP_MASK ) <0 ) +return ; + +tps65910_reg_update_bits (tps65910,TPS65910_DEVCTRL , +DEVCTRL_DEV_OFF_MASK |DEVCTRL_DEV_ON_MASK , +DEVCTRL_DEV_OFF_MASK ); } static int tps65910_i2c_probe(struct i2c_client *i2c,
Signature · 4 lines
Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Quoted 6 lines · Dmitry Osipenko
This patch fixes system shutdown on a devices that use TPS65910 as a system's power controller. In accordance to the TPS65910 datasheet, the PMIC's state-machine transitions into the OFF state only when
DEV_OFFbit ofDEVCTRL_REGis set. The ON / SLEEP states also should be cleared, otherwise PMIC won't get into a proper state on shutdown. Devices like Nexus 7 tablet and Ouya game console are now shutting down properly.
The datasheets of 65910 and 65911 say that ON and SLP bits are cleared during OFF state. But I guess the hardware might work differently.
[...]
Quoted 6 lines
drivers/mfd/tps65910.c +0
-0@@ -440,8 +440,13 @@ static void tps65910_power_off(void) DEVCTRL_PWR_OFF_MASK) < 0)return ;
drivers/mfd/tps65910.c static void tps65910_power_off(void) · line 450
Quoted 10 lines
drivers/mfd/tps65910.c +7
-2-tps65910_reg_clear_bits (tps65910,TPS65910_DEVCTRL , -DEVCTRL_DEV_ON_MASK ); +if (tps65910_reg_clear_bits (tps65910,TPS65910_DEVCTRL , +DEVCTRL_DEV_SLP_MASK ) <0 ) +return ; + +tps65910_reg_update_bits (tps65910,TPS65910_DEVCTRL , +DEVCTRL_DEV_OFF_MASK |DEVCTRL_DEV_ON_MASK , +DEVCTRL_DEV_OFF_MASK ); }
There is tps65910_reg_set_bits() at the start of function. I guess it doesn't work if your changes are needed. Maybe you can remove it?
I would also include your observations about the chip's behaviour in the commit message so it doesn't get "fixed" later.
Best Regards,
Michał Mirosław
26.05.2020 18:01, Michał Mirosław пишет:
Quoted 10 lines · Dmitry Osipenko
Quoted 6 lines
This patch fixes system shutdown on a devices that use TPS65910 as a system's power controller. In accordance to the TPS65910 datasheet, the PMIC's state-machine transitions into the OFF state only when
DEV_OFFbit ofDEVCTRL_REGis set. The ON / SLEEP states also should be cleared, otherwise PMIC won't get into a proper state on shutdown. Devices like Nexus 7 tablet and Ouya game console are now shutting down properly.The datasheets of 65910 and 65911 say that ON and SLP bits are cleared during OFF state. But I guess the hardware might work differently.
Indeed, sounds like we can remove the SLP bit-clearing safely. IIUC, both tps65910 and tps65911 are nearly the same in regards to the power-off programming, tps65911 only supports an additional (sequential) power-off mode.
I'm not sure whether we've tried to remove the SLP bit-clearing before, will be interesting to try.
Quoted 7 lines
[...]
Quoted 6 lines
drivers/mfd/tps65910.c +0
-0@@ -440,8 +440,13 @@ static void tps65910_power_off(void) DEVCTRL_PWR_OFF_MASK) < 0)return ;
Quoted 13 lines
drivers/mfd/tps65910.c static void tps65910_power_off(void) · line 450
Quoted 10 lines
drivers/mfd/tps65910.c +7
-2-tps65910_reg_clear_bits (tps65910,TPS65910_DEVCTRL , -DEVCTRL_DEV_ON_MASK ); +if (tps65910_reg_clear_bits (tps65910,TPS65910_DEVCTRL , +DEVCTRL_DEV_SLP_MASK ) <0 ) +return ; + +tps65910_reg_update_bits (tps65910,TPS65910_DEVCTRL , +DEVCTRL_DEV_OFF_MASK |DEVCTRL_DEV_ON_MASK , +DEVCTRL_DEV_OFF_MASK ); }There is
tps65910_reg_set_bits()at the start of function. I guess it doesn't work if your changes are needed. Maybe you can remove it?
It enables the "sequential power-off, reverse of power-on sequence", like datasheet says. I think it works and we actually need that PWR_OFF bit to be set separately, before setting the DEV_OFF bit.
Quoted 2 lines
I would also include your observations about the chip's behaviour in the commit message so it doesn't get "fixed" later.
I'll add a clarifying comment about it in v3, thank you for the suggestions.