Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

regulator: core: Shorten off-on-delay-us for always-on/boot-on by time since booted

This is very close to a straight revert of commit 218320fec294
("regulator: core: Fix off-on-delay-us for always-on/boot-on
regulators"). We've identified that patch as causing a boot speed
regression on sc7180-trogdor boards. While boot speed certainly isn't
more important than making sure that power sequencing is correct,
looking closely at the original change it doesn't seem to have been
fully justified. It mentions "cycling issues" without describing
exactly what the issues were. That means it's possible that the
cycling issues were really a problem that should be fixed in a
different way.

Let's take a careful look at how we should handle regulators that have
an off-on-delay and that are boot-on or always-on. Linux currently
doesn't have any way to identify whether a GPIO regulator was already
on when the kernel booted. That means that when the kernel boots we
probe a regulator, see that it wants boot-on / always-on we, and then
turn the regulator on. We could be in one of two cases when we do
this:

a) The regulator might have been left on by the bootloader and we're
ensuring that it stays on.
b) The regulator might have been left off by the bootloader and we're
just now turning it on.

For case a) we definitely don't need any sort of delay. For case b) we
_might_ need some delay in case the bootloader turned the regulator
off _right_ before booting the kernel. To get the proper delay for
case b) then we can just assume a `last_off` of 0, which is what it
gets initialized to by default.

As per above, we can't tell whether we're in case a) or case b) so
we'll assume the longer delay (case b). This basically puts the code
to how it was before commit 218320fec294 ("regulator: core: Fix
off-on-delay-us for always-on/boot-on regulators"). However, we add
one important change: we make sure that the delay is actually honored
if `last_off` is 0. Though the original "cycling issues" cited were
vague, I'm hopeful that this important extra change will be enough to
fix the issues that the initial commit mentioned.

With this fix, I've confined that on a sc7180-trogdor board the delay
at boot goes down from 500 ms to ~250 ms. That's not as good as the 0
ms that we had prior to commit 218320fec294 ("regulator: core: Fix
off-on-delay-us for always-on/boot-on regulators"), but it's probably
safer because we don't know if the bootloader turned the regulator off
right before booting.

One note is that it's possible that we could be in a state that's not
a) or b) if there are other issues in the kernel. The only one I can
think of is related to pinctrl. If the pinctrl driver being used on a
board isn't careful about avoiding glitches when setting up a pin then
it's possible that setting up a pin could cause the regulator to "turn
off" briefly immediately before the regulator probes. If this is
indeed causing problems then the pinctrl driver should be fixed,
perhaps in a similar way to what was done in commit d21f4b7ffc22
("pinctrl: qcom: Avoid glitching lines when we first mux to output")

Fixes: 218320fec294 ("regulator: core: Fix off-on-delay-us for always-on/boot-on regulators")
Cc: Christian Kohlschütter <christian@kohlschutter.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20230313111806.1.I2eaad872be0932a805c239a7c7a102233fb0b03b@changeid
Signed-off-by: Mark Brown <broonie@kernel.org>

authored by

Douglas Anderson and committed by
Mark Brown
691c1fcd 38cc873c

+3 -4
+3 -4
drivers/regulator/core.c
··· 1583 1583 rdev->constraints->always_on = true; 1584 1584 } 1585 1585 1586 - if (rdev->desc->off_on_delay) 1587 - rdev->last_off = ktime_get_boottime(); 1588 - 1589 1586 /* If the constraints say the regulator should be on at this point 1590 1587 * and we have control then make sure it is enabled. 1591 1588 */ ··· 1616 1619 1617 1620 if (rdev->constraints->always_on) 1618 1621 rdev->use_count++; 1622 + } else if (rdev->desc->off_on_delay) { 1623 + rdev->last_off = ktime_get(); 1619 1624 } 1620 1625 1621 1626 print_constraints(rdev); ··· 2667 2668 2668 2669 trace_regulator_enable(rdev_get_name(rdev)); 2669 2670 2670 - if (rdev->desc->off_on_delay && rdev->last_off) { 2671 + if (rdev->desc->off_on_delay) { 2671 2672 /* if needed, keep a distance of off_on_delay from last time 2672 2673 * this regulator was disabled. 2673 2674 */