diff mbox series

[v2] regulator: core: Use ktime_get_boottime() to determine how long a regulator was off

Message ID 20230223003301.v2.1.I9719661b8eb0a73b8c416f9c26cf5bd8c0563f99@changeid
State New
Headers
Series [v2] regulator: core: Use ktime_get_boottime() to determine how long a regulator was off |

Commit Message

Matthias Kaehlcke Feb. 23, 2023, 12:33 a.m. UTC 
  For regulators with 'off-on-delay-us' the regulator framework currently
uses ktime_get() to determine how long the regulator has been off
before re-enabling it (after a delay if needed). A problem with using
ktime_get() is that it doesn't account for the time the system is
suspended. As a result a regulator with a longer 'off-on-delay' (e.g.
500ms) that was switched off during suspend might still incurr in a
delay on resume before it is re-enabled, even though the regulator
might have been off for hours. ktime_get_boottime() accounts for
suspend time, use it instead of ktime_get().

Fixes: a8ce7bd89689 ("regulator: core: Fix off_on_delay handling")
Cc: stable@vger.kernel.org    # 5.13+
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
The issue already existed before the commit in the 'Fixes' tag, but
it's probably not worth backporting this to older kernels that
use jiffies instead of ktime.

Changes in v2:
- added 'Fixes' and Cc: stable tags
- added 'Reviewed-by' tag from Stephen

 drivers/regulator/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Mark Brown Feb. 23, 2023, 11:22 a.m. UTC | #1 
On Thu, Feb 23, 2023 at 12:33:30AM +0000, Matthias Kaehlcke wrote:

> delay on resume before it is re-enabled, even though the regulator
> might have been off for hours. ktime_get_boottime() accounts for
> suspend time, use it instead of ktime_get().

This API seems like landmines...
Mark Brown Feb. 23, 2023, 12:55 p.m. UTC | #2 
On Thu, 23 Feb 2023 00:33:30 +0000, Matthias Kaehlcke wrote:
> For regulators with 'off-on-delay-us' the regulator framework currently
> uses ktime_get() to determine how long the regulator has been off
> before re-enabling it (after a delay if needed). A problem with using
> ktime_get() is that it doesn't account for the time the system is
> suspended. As a result a regulator with a longer 'off-on-delay' (e.g.
> 500ms) that was switched off during suspend might still incurr in a
> delay on resume before it is re-enabled, even though the regulator
> might have been off for hours. ktime_get_boottime() accounts for
> suspend time, use it instead of ktime_get().
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: core: Use ktime_get_boottime() to determine how long a regulator was off
      commit: 80d2c29e09e663761c2778167a625b25ffe01b6f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ae69e493913d..4fcd36055b02 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1584,7 +1584,7 @@  static int set_machine_constraints(struct regulator_dev *rdev)
 	}
 
 	if (rdev->desc->off_on_delay)
-		rdev->last_off = ktime_get();
+		rdev->last_off = ktime_get_boottime();
 
 	/* If the constraints say the regulator should be on at this point
 	 * and we have control then make sure it is enabled.
@@ -2673,7 +2673,7 @@  static int _regulator_do_enable(struct regulator_dev *rdev)
 		 * this regulator was disabled.
 		 */
 		ktime_t end = ktime_add_us(rdev->last_off, rdev->desc->off_on_delay);
-		s64 remaining = ktime_us_delta(end, ktime_get());
+		s64 remaining = ktime_us_delta(end, ktime_get_boottime());
 
 		if (remaining > 0)
 			_regulator_delay_helper(remaining);
@@ -2912,7 +2912,7 @@  static int _regulator_do_disable(struct regulator_dev *rdev)
 	}
 
 	if (rdev->desc->off_on_delay)
-		rdev->last_off = ktime_get();
+		rdev->last_off = ktime_get_boottime();
 
 	trace_regulator_disable_complete(rdev_get_name(rdev));