Re: [PATCH] regulator: core: Skip balancing of the enabled regulators in regulator_enable()

[Mark Brown <broonie@kernel.org>]

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

On Tue, Oct 08, 2019 at 12:17:09PM +0200, Marek Szyprowski wrote:
> Commit f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators
> locking"), regardless of the subject, added additional call to
> regulator_balance_voltage() during regulator_enable(). This is basically
> a good idea, however it causes some issue for the regulators which are
> already enabled at boot and are critical for system operation (for example
> provides supply to the CPU).

If regulators are essential to system operation they should be marked as
always-on...

> CPUfreq or other drivers typically call regulator_enable() on such
> regulators during their probe, although the regulators are already enabled
> by bootloader. The mentioned patch however added a call to
> regulator_balance_voltage(), what in case of system boot, where no
> additional requirements are set yet, typically causes to limit the voltage
> to the minimal value defined at regulator constraints. This causes a crash
> of the system when voltage on the CPU regulator is set to the lowest
> possible value without adjusting the operation frequency. Fix this by
> adding a check if regulator is already enabled - if so, then skip the
> balancing procedure. The voltage will be balanced later anyway once the
> required voltage value is requested.

This then means that for users that might legitimately enable and
disable regulators that need to be constrained are forced to change the
voltage when they enable the regualtors in order to have their
constraints take effect which seems bad.  I'd rather change the the
cpufreq consumers to either not do the enable (since there really should
be an always-on constraint this should be redundant, we might need to
fix the core to take account of their settings though I think we lost
that) or to set the voltage to whatever they need prior to doing their
first enable, that seems more robust.

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