Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1621600 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] regulator: core: Allow dummy regulators for supplies Mark Brown <broonie@kernel.org> - 2017-04-11 22:40 +0200
    RE: [PATCH] regulator: core: Allow dummy regulators for supplies "A.S. Dong" <aisheng.dong@nxp.com> - 2017-04-12 17:40 +0200
      Re: [PATCH] regulator: core: Allow dummy regulators for supplies Mark Brown <broonie@kernel.org> - 2017-04-13 14:30 +0200
        Re: [PATCH] regulator: core: Allow dummy regulators for supplies Dong Aisheng <dongas86@gmail.com> - 2017-04-13 15:50 +0200

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

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [next] | [standalone]

#1622251

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

[toc] | [prev] | [next] | [standalone]

#1622984

[Multipart message — attachments visible in raw view] — view raw

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...

[toc] | [prev] | [next] | [standalone]

#1623034

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web