[PATCH] regulator: core: Allow dummy regulators for supplies

[PATCH] regulator: core: Allow dummy regulators for supplies

[Mark Brown]

Rather than just not resolving the supply when there is explicitly no
supply mapping fall through and allow a dummy supply to be substituted.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 82205cc5daa7..1e7e6d361238 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1533,14 +1533,6 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	if (IS_ERR(r)) {
 		ret = PTR_ERR(r);
 
-		if (ret == -ENODEV) {
-			/*
-			 * No supply was specified for this regulator and
-			 * there will never be one.
-			 */
-			return 0;
-		}
-
 		/* Did the lookup explicitly defer for us? */
 		if (ret == -EPROBE_DEFER)
 			return ret;
-- 
2.11.0

Re: [PATCH] regulator: core: Allow dummy regulators for supplies

[Mark Brown]

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

On Tue, Apr 11, 2017 at 09:34:37PM +0100, Mark Brown wrote:
> Rather than just not resolving the supply when there is explicitly no
> supply mapping fall through and allow a dummy supply to be substituted.

I should mention that this is completely untested, sorry.

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

RE: [PATCH] regulator: core: Allow dummy regulators for supplies

[A.S. Dong]

Hi Mark,

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Wednesday, April 12, 2017 4:37 AM
> To: A.S. Dong
> Cc: Liam Girdwood; linux-kernel@vger.kernel.org; shawnguo@kernel.org;
> Robin Gong
> Subject: Re: [PATCH] regulator: core: Allow dummy regulators for supplies
> 
> On Tue, Apr 11, 2017 at 09:34:37PM +0100, Mark Brown wrote:
> > Rather than just not resolving the supply when there is explicitly no
> > supply mapping fall through and allow a dummy supply to be substituted.
> 
> I should mention that this is completely untested, sorry.

It did break the MX6Q cpufreq as follows:
[    3.950097] coda 2040000.vpu: Direct firmware load for vpu/vpu_fw_imx6q.bin failed with error -2
[    3.950111] coda 2040000.vpu: firmware request failed
[    4.074044] cpu cpu0: failed to scale vddarm down: -22
[    4.079262] cpu cpu0: failed to scale vddsoc down: -22
[    4.084809] cpu cpu0: failed to scale vddpu down: -22

It seems the regulator supply behavior is changed a bit after apply this patch.

Before this patch, the regulator core will not report an error if can't resolve
A non exist supply. (It's exactly what this patch removes)

Now we allow a supply to be a dummy regulator. But due to 
fc42112c0eaa ("regulator: core: Propagate voltage changes to supply regulators")
which supports propagate voltage change to supply regulator,
then it will certainly fail if we want to configure a dummy supply.

I tried a quick fix as follows and test seemed ok.
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 43dafac..a1c952db 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2935,7 +2935,8 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
        if (ret < 0)
                goto out2;
 
-       if (rdev->supply && (rdev->desc->min_dropout_uV ||
+       if (rdev->supply && rdev->supply->rdev != dummy_regulator_rdev &&
+                               (rdev->desc->min_dropout_uV ||

Not sure if it's a sufficient fix.
Please help check it.

Regards
Dong Aisheng

Re: [PATCH] regulator: core: Allow dummy regulators for supplies

[Mark Brown]

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

On Wed, Apr 12, 2017 at 03:33:14PM +0000, A.S. Dong wrote:

> Now we allow a supply to be a dummy regulator. But due to 
> fc42112c0eaa ("regulator: core: Propagate voltage changes to supply regulators")
> which supports propagate voltage change to supply regulator,
> then it will certainly fail if we want to configure a dummy supply.
> 
> I tried a quick fix as follows and test seemed ok.

> -       if (rdev->supply && (rdev->desc->min_dropout_uV ||
> +       if (rdev->supply && rdev->supply->rdev != dummy_regulator_rdev &&
> +                               (rdev->desc->min_dropout_uV ||

> Not sure if it's a sufficient fix.
> Please help check it.

Ah, of course.  That'll work but I don't think it's the best solution -
there are some other regulators that also won't support setting voltages
but might get used as supplies so we should do something based on the
properties of the regulator rather than based specifically on the dummy
regulator.  Let me see...

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

Re: [PATCH] regulator: core: Allow dummy regulators for supplies

[Dong Aisheng]

Hi Mark,

On Thu, Apr 13, 2017 at 8:20 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Apr 12, 2017 at 03:33:14PM +0000, A.S. Dong wrote:
>
>> Now we allow a supply to be a dummy regulator. But due to
>> fc42112c0eaa ("regulator: core: Propagate voltage changes to supply regulators")
>> which supports propagate voltage change to supply regulator,
>> then it will certainly fail if we want to configure a dummy supply.
>>
>> I tried a quick fix as follows and test seemed ok.
>
>> -       if (rdev->supply && (rdev->desc->min_dropout_uV ||
>> +       if (rdev->supply && rdev->supply->rdev != dummy_regulator_rdev &&
>> +                               (rdev->desc->min_dropout_uV ||
>
>> Not sure if it's a sufficient fix.
>> Please help check it.
>
> Ah, of course.  That'll work but I don't think it's the best solution -
> there are some other regulators that also won't support setting voltages
> but might get used as supplies so we should do something based on the
> properties of the regulator rather than based specifically on the dummy
> regulator.  Let me see..

Yes, i was also wondering that..
Let's waiting for you...

Thanks

Regards
Dong Aisheng

Applied "regulator: core: Allow dummy regulators for supplies" to the regulator tree

[Mark Brown]

The patch

   regulator: core: Allow dummy regulators for supplies

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

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

>From c93609ab3924cc974fc90001fb6aa250a8900a3c Mon Sep 17 00:00:00 2001
From: Mark Brown <broonie@kernel.org>
Date: Tue, 11 Apr 2017 21:31:40 +0100
Subject: [PATCH] regulator: core: Allow dummy regulators for supplies

Rather than just not resolving the supply when there is explicitly no
supply mapping fall through and allow a dummy supply to be substituted.
This fixes issues with constant retries reported by Dong Aisheng.

Signed-off-by: Mark Brown <broonie@kernel.org>
Tested-by: Dong Aisheng <aisheng.dong@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/regulator/core.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 3f424ec4fc56..462e6e679ce1 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1532,14 +1532,6 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	if (IS_ERR(r)) {
 		ret = PTR_ERR(r);
 
-		if (ret == -ENODEV) {
-			/*
-			 * No supply was specified for this regulator and
-			 * there will never be one.
-			 */
-			return 0;
-		}
-
 		/* Did the lookup explicitly defer for us? */
 		if (ret == -EPROBE_DEFER)
 			return ret;
-- 
2.11.0