[PATCH] power: ab8500_chargalg: Use CLOCK_MONOTONIC

[PATCH] power: ab8500_chargalg: Use CLOCK_MONOTONIC

[Linus Walleij]

The HRTimer in the AB8500 charging code is using CLOCK_REALTIME
to set an alarm some hours forward in time +/- 5 min for a safety
timer.

I have observed that this will sometimes fire sporadically
early when charging a battery with the result that
charging stops.

As CLOCK_REALTIME can be subject to adjustments of time from
sources such as NTP, this cannot be trusted and will likely
for example fire events if the clock is set forward some hours
by say NTP.

Use CLOCK_MONOTONIC as indicated in other instances and the
problem goes away. Also initialize the timer to REL mode
as this is what will be used later.

Fixes: 257107ae6b9b ("ab8500-chargalg: Use hrtimer")
Cc: Lee Jones <lee.jones@linaro.org>
Suggested-by: Matti Vaittinen <mazziesaccount@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/power/supply/ab8500_chargalg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/ab8500_chargalg.c b/drivers/power/supply/ab8500_chargalg.c
index 20d6b29cc9e2..c345d5e99118 100644
--- a/drivers/power/supply/ab8500_chargalg.c
+++ b/drivers/power/supply/ab8500_chargalg.c
@@ -1780,11 +1780,11 @@ static int ab8500_chargalg_probe(struct platform_device *pdev)
 	psy_cfg.drv_data = di;
 
 	/* Initilialize safety timer */
-	hrtimer_init(&di->safety_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
+	hrtimer_init(&di->safety_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	di->safety_timer.function = ab8500_chargalg_safety_timer_expired;
 
 	/* Initilialize maintenance timer */
-	hrtimer_init(&di->maintenance_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
+	hrtimer_init(&di->maintenance_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	di->maintenance_timer.function =
 		ab8500_chargalg_maintenance_timer_expired;
 
-- 
2.35.1

Re: [PATCH] power: ab8500_chargalg: Use CLOCK_MONOTONIC

[Vaittinen, Matti]

On 3/8/22 17:44, Linus Walleij wrote:
> The HRTimer in the AB8500 charging code is using CLOCK_REALTIME
> to set an alarm some hours forward in time +/- 5 min for a safety
> timer.
> 
> I have observed that this will sometimes fire sporadically
> early when charging a battery with the result that
> charging stops.
> 
> As CLOCK_REALTIME can be subject to adjustments of time from
> sources such as NTP, this cannot be trusted and will likely
> for example fire events if the clock is set forward some hours
> by say NTP.
> 
> Use CLOCK_MONOTONIC as indicated in other instances and the
> problem goes away. Also initialize the timer to REL mode
> as this is what will be used later.
> 
> Fixes: 257107ae6b9b ("ab8500-chargalg: Use hrtimer")
> Cc: Lee Jones <lee.jones@linaro.org>
> Suggested-by: Matti Vaittinen <mazziesaccount@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

FWIW:
Reviewed-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>


-- 
The Linux Kernel guy at ROHM Semiconductors

Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~ this year is the year of a signature writers block ~~

Re: [PATCH] power: ab8500_chargalg: Use CLOCK_MONOTONIC

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

Hi,

On Wed, Mar 09, 2022 at 06:45:59AM +0000, Vaittinen, Matti wrote:
> On 3/8/22 17:44, Linus Walleij wrote:
> > The HRTimer in the AB8500 charging code is using CLOCK_REALTIME
> > to set an alarm some hours forward in time +/- 5 min for a safety
> > timer.
> > 
> > I have observed that this will sometimes fire sporadically
> > early when charging a battery with the result that
> > charging stops.
> > 
> > As CLOCK_REALTIME can be subject to adjustments of time from
> > sources such as NTP, this cannot be trusted and will likely
> > for example fire events if the clock is set forward some hours
> > by say NTP.
> > 
> > Use CLOCK_MONOTONIC as indicated in other instances and the
> > problem goes away. Also initialize the timer to REL mode
> > as this is what will be used later.
> > 
> > Fixes: 257107ae6b9b ("ab8500-chargalg: Use hrtimer")
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Suggested-by: Matti Vaittinen <mazziesaccount@gmail.com>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> FWIW:
> Reviewed-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Thanks, queued.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]