[PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators

[PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators

[Dmitry Baryshkov]

For the boot-on/always-on regulators the set_machine_constrainst() is
called before resolving rdev->supply. Thus the code would try to enable
rdev before enabling supplying regulator. Enforce resolving supply
regulator before enabling rdev.

Fixes: aea6cb99703e ("regulator: resolve supply after creating regulator")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/regulator/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f192bf19492e..e20e77e4c159 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1425,6 +1425,12 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 	 * and we have control then make sure it is enabled.
 	 */
 	if (rdev->constraints->always_on || rdev->constraints->boot_on) {
+		/* If we want to enable this regulator, make sure that we know
+		 * the supplying regulator.
+		 */
+		if (rdev->supply_name && !rdev->supply)
+			return -EPROBE_DEFER;
+
 		if (rdev->supply) {
 			ret = regulator_enable(rdev->supply);
 			if (ret < 0) {
-- 
2.30.2

[PATCH 2/2] regulator: core: always use enable_delay when enabling regulators

[Dmitry Baryshkov]

Some regulators (e.g. fixed) do not have .enable callback per se, but
use supply regulator and enable_delay. Do not return early from
_regulator_do_enable in such cases, so that enable_delay is properly
handled.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/regulator/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e20e77e4c159..66c465bd00ca 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2516,7 +2516,8 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
 		ret = rdev->desc->ops->enable(rdev);
 		if (ret < 0)
 			return ret;
-	} else {
+	} else if (!rdev->supply) {
+		/* Still handle delay if the regulator was just turned on using the supply */
 		return -EINVAL;
 	}
 
-- 
2.30.2

Re: [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators

[Mark Brown]

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

On Thu, May 20, 2021 at 01:12:24AM +0300, Dmitry Baryshkov wrote:
> Some regulators (e.g. fixed) do not have .enable callback per se, but
> use supply regulator and enable_delay. Do not return early from
> _regulator_do_enable in such cases, so that enable_delay is properly
> handled.

This doesn't seem like the right fix - if we didn't actually do anything
then we don't need to add a delay.  We should only be doing this if some
parent regulator changed state.

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

Re: [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators

[Dmitry Baryshkov]

On Thu, 20 May 2021 at 14:58, Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, May 20, 2021 at 01:12:24AM +0300, Dmitry Baryshkov wrote:
> > Some regulators (e.g. fixed) do not have .enable callback per se, but
> > use supply regulator and enable_delay. Do not return early from
> > _regulator_do_enable in such cases, so that enable_delay is properly
> > handled.
>
> This doesn't seem like the right fix - if we didn't actually do anything
> then we don't need to add a delay.  We should only be doing this if some
> parent regulator changed state.

I have implemented this, but then it becomes too fragile. If the
parent gets enabled for whatever reason just few us ago, the whole
delay would be skipped.

-- 
With best wishes
Dmitry

Re: [PATCH 2/2] regulator: core: always use enable_delay when enabling regulators

[Mark Brown]

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

On Wed, Jun 02, 2021 at 04:14:35PM +0300, Dmitry Baryshkov wrote:
> On Thu, 20 May 2021 at 14:58, Mark Brown <broonie@kernel.org> wrote:

> > On Thu, May 20, 2021 at 01:12:24AM +0300, Dmitry Baryshkov wrote:
> > > Some regulators (e.g. fixed) do not have .enable callback per se, but
> > > use supply regulator and enable_delay. Do not return early from
> > > _regulator_do_enable in such cases, so that enable_delay is properly
> > > handled.

> > This doesn't seem like the right fix - if we didn't actually do anything
> > then we don't need to add a delay.  We should only be doing this if some
> > parent regulator changed state.

> I have implemented this, but then it becomes too fragile. If the
> parent gets enabled for whatever reason just few us ago, the whole
> delay would be skipped.

This seems like a solvable problem, we can track what parents are doing
or teach parents about their children for example.

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

Re: (subset) [PATCH 1/2] regulator: core: resolve supply for boot-on/always-on regulators

[Mark Brown]

On Thu, 20 May 2021 01:12:23 +0300, Dmitry Baryshkov wrote:
> For the boot-on/always-on regulators the set_machine_constrainst() is
> called before resolving rdev->supply. Thus the code would try to enable
> rdev before enabling supplying regulator. Enforce resolving supply
> regulator before enabling rdev.

Applied to

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

Thanks!

[1/2] regulator: core: resolve supply for boot-on/always-on regulators
      commit: 98e48cd9283dbac0e1445ee780889f10b3d1db6a

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