All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD
@ 2018-12-12 17:13 Loic Poulain
  2018-12-12 17:23 ` Jeffrey Hugo
  2018-12-12 19:46 ` Bjorn Andersson
  0 siblings, 2 replies; 8+ messages in thread
From: Loic Poulain @ 2018-12-12 17:13 UTC (permalink / raw)
  To: andy.gross, david.brown, robh+dt
  Cc: linux-arm-msm, bjorn.andersson, devicetree, Loic Poulain

In the same way as for msm8974-hammerhead, l21 load, used for SDCARD
VMMC, needs to be increased in order to prevent any voltage drop issues
(due to limited current) happening with some SDCARDS or during specific
operations (e.g. write).

Fixes: 660a9763c6a9 (arm64: dts: qcom: db820c: Add pm8994 regulator node)
Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
index 104cad9..c15e2c0 100644
--- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
@@ -634,6 +634,8 @@
 				l21 {
 					regulator-min-microvolt = <2950000>;
 					regulator-max-microvolt = <2950000>;
+					regulator-allow-set-load;
+					regulator-system-load = <200000>;
 				};
 				l22 {
 					regulator-min-microvolt = <3300000>;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD
  2018-12-12 17:13 [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD Loic Poulain
@ 2018-12-12 17:23 ` Jeffrey Hugo
  2018-12-13  7:55   ` Loic Poulain
  2018-12-12 19:46 ` Bjorn Andersson
  1 sibling, 1 reply; 8+ messages in thread
From: Jeffrey Hugo @ 2018-12-12 17:23 UTC (permalink / raw)
  To: Loic Poulain, andy.gross, david.brown, robh+dt
  Cc: linux-arm-msm, bjorn.andersson, devicetree

On 12/12/2018 10:13 AM, Loic Poulain wrote:
> In the same way as for msm8974-hammerhead, l21 load, used for SDCARD
> VMMC, needs to be increased in order to prevent any voltage drop issues
> (due to limited current) happening with some SDCARDS or during specific
> operations (e.g. write).
> 
> Fixes: 660a9763c6a9 (arm64: dts: qcom: db820c: Add pm8994 regulator node)
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> index 104cad9..c15e2c0 100644
> --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> @@ -634,6 +634,8 @@
>   				l21 {
>   					regulator-min-microvolt = <2950000>;
>   					regulator-max-microvolt = <2950000>;
> +					regulator-allow-set-load;
> +					regulator-system-load = <200000>;
>   				};
>   				l22 {
>   					regulator-min-microvolt = <3300000>;
> 

I'm curious, why not update sdhci-msm to set the load on the regulator?

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD
  2018-12-12 17:13 [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD Loic Poulain
  2018-12-12 17:23 ` Jeffrey Hugo
@ 2018-12-12 19:46 ` Bjorn Andersson
  1 sibling, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2018-12-12 19:46 UTC (permalink / raw)
  To: Loic Poulain; +Cc: andy.gross, david.brown, robh+dt, linux-arm-msm, devicetree

On Wed 12 Dec 09:13 PST 2018, Loic Poulain wrote:

> In the same way as for msm8974-hammerhead, l21 load, used for SDCARD
> VMMC, needs to be increased in order to prevent any voltage drop issues
> (due to limited current) happening with some SDCARDS or during specific
> operations (e.g. write).
> 
> Fixes: 660a9763c6a9 (arm64: dts: qcom: db820c: Add pm8994 regulator node)
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

I agree with Jeff, that the sdhci-msm (or mmc framework even) should
take care of this. But I suggest we merge this for now, regardless.

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> index 104cad9..c15e2c0 100644
> --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> @@ -634,6 +634,8 @@
>  				l21 {
>  					regulator-min-microvolt = <2950000>;
>  					regulator-max-microvolt = <2950000>;
> +					regulator-allow-set-load;
> +					regulator-system-load = <200000>;
>  				};
>  				l22 {
>  					regulator-min-microvolt = <3300000>;
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD
  2018-12-12 17:23 ` Jeffrey Hugo
@ 2018-12-13  7:55   ` Loic Poulain
  2018-12-13 14:46     ` Jeffrey Hugo
  0 siblings, 1 reply; 8+ messages in thread
From: Loic Poulain @ 2018-12-13  7:55 UTC (permalink / raw)
  To: jhugo
  Cc: andy.gross, david.brown, robh+dt, linux-arm-msm, bjorn.andersson,
	devicetree

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

Hi Jeffrey,


On Wed, 12 Dec 2018 at 18:23, Jeffrey Hugo <jhugo@codeaurora.org> wrote:

> On 12/12/2018 10:13 AM, Loic Poulain wrote:
> > In the same way as for msm8974-hammerhead, l21 load, used for SDCARD
> > VMMC, needs to be increased in order to prevent any voltage drop issues
> > (due to limited current) happening with some SDCARDS or during specific
> > operations (e.g. write).
> >
> > Fixes: 660a9763c6a9 (arm64: dts: qcom: db820c: Add pm8994 regulator node)
> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > ---
> >   arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > index 104cad9..c15e2c0 100644
> > --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > @@ -634,6 +634,8 @@
> >                               l21 {
> >                                       regulator-min-microvolt =
> <2950000>;
> >                                       regulator-max-microvolt =
> <2950000>;
> > +                                     regulator-allow-set-load;
> > +                                     regulator-system-load = <200000>;
> >                               };
> >                               l22 {
> >                                       regulator-min-microvolt =
> <3300000>;
> >
>
> I'm curious, why not update sdhci-msm to set the load on the regulator?
>

Yes you're right, and I saw that there is ongoing work:
https://patchwork.kernel.org/patch/10630731/

Howerver I thought this change would be a quicker fix and easier to
backport in stable trees.
I assume all the device-tree vmmc loads will be removed at some point when
driven from sdhci.

Regards,
Loic


>
> --
> Jeffrey Hugo
> Qualcomm Datacenter Technologies as an affiliate of Qualcomm
> Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the
> Code Aurora Forum, a Linux Foundation Collaborative Project.
>

[-- Attachment #2: Type: text/html, Size: 3090 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD
  2018-12-13  7:55   ` Loic Poulain
@ 2018-12-13 14:46     ` Jeffrey Hugo
  2018-12-13 19:14       ` Doug Anderson
  0 siblings, 1 reply; 8+ messages in thread
From: Jeffrey Hugo @ 2018-12-13 14:46 UTC (permalink / raw)
  To: Loic Poulain
  Cc: andy.gross, david.brown, robh+dt, linux-arm-msm, bjorn.andersson,
	devicetree

On 12/13/2018 12:55 AM, Loic Poulain wrote:
> Hi Jeffrey,
> 
> 
> On Wed, 12 Dec 2018 at 18:23, Jeffrey Hugo <jhugo@codeaurora.org 
> <mailto:jhugo@codeaurora.org>> wrote:
> 
>     On 12/12/2018 10:13 AM, Loic Poulain wrote:
>      > In the same way as for msm8974-hammerhead, l21 load, used for SDCARD
>      > VMMC, needs to be increased in order to prevent any voltage drop
>     issues
>      > (due to limited current) happening with some SDCARDS or during
>     specific
>      > operations (e.g. write).
>      >
>      > Fixes: 660a9763c6a9 (arm64: dts: qcom: db820c: Add pm8994
>     regulator node)
>      > Signed-off-by: Loic Poulain <loic.poulain@linaro.org
>     <mailto:loic.poulain@linaro.org>>
>      > ---
>      >   arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
>      >   1 file changed, 2 insertions(+)
>      >
>      > diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
>     b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
>      > index 104cad9..c15e2c0 100644
>      > --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
>      > +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
>      > @@ -634,6 +634,8 @@
>      >                               l21 {
>      >                                       regulator-min-microvolt =
>     <2950000>;
>      >                                       regulator-max-microvolt =
>     <2950000>;
>      > +                                     regulator-allow-set-load;
>      > +                                     regulator-system-load =
>     <200000>;
>      >                               };
>      >                               l22 {
>      >                                       regulator-min-microvolt =
>     <3300000>;
>      >
> 
>     I'm curious, why not update sdhci-msm to set the load on the regulator?
> 
> 
> Yes you're right, and I saw that there is ongoing work:
> https://patchwork.kernel.org/patch/10630731/
> 
> Howerver I thought this change would be a quicker fix and easier to 
> backport in stable trees.
> I assume all the device-tree vmmc loads will be removed at some point 
> when driven from sdhci.
> 

I hadn't seen that.  Ok, seems good to me.


-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD
  2018-12-13 14:46     ` Jeffrey Hugo
@ 2018-12-13 19:14       ` Doug Anderson
  2019-10-11 12:55         ` Loic Poulain
  0 siblings, 1 reply; 8+ messages in thread
From: Doug Anderson @ 2018-12-13 19:14 UTC (permalink / raw)
  To: jhugo
  Cc: loic.poulain, Andy Gross, David Brown, Rob Herring,
	linux-arm-msm, Bjorn Andersson, devicetree

Hi,

On Thu, Dec 13, 2018 at 6:46 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote:
>
> On 12/13/2018 12:55 AM, Loic Poulain wrote:
> > Hi Jeffrey,
> >
> >
> > On Wed, 12 Dec 2018 at 18:23, Jeffrey Hugo <jhugo@codeaurora.org
> > <mailto:jhugo@codeaurora.org>> wrote:
> >
> >     On 12/12/2018 10:13 AM, Loic Poulain wrote:
> >      > In the same way as for msm8974-hammerhead, l21 load, used for SDCARD
> >      > VMMC, needs to be increased in order to prevent any voltage drop
> >     issues
> >      > (due to limited current) happening with some SDCARDS or during
> >     specific
> >      > operations (e.g. write).
> >      >
> >      > Fixes: 660a9763c6a9 (arm64: dts: qcom: db820c: Add pm8994
> >     regulator node)
> >      > Signed-off-by: Loic Poulain <loic.poulain@linaro.org
> >     <mailto:loic.poulain@linaro.org>>
> >      > ---
> >      >   arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
> >      >   1 file changed, 2 insertions(+)
> >      >
> >      > diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> >     b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> >      > index 104cad9..c15e2c0 100644
> >      > --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> >      > +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> >      > @@ -634,6 +634,8 @@
> >      >                               l21 {
> >      >                                       regulator-min-microvolt =
> >     <2950000>;
> >      >                                       regulator-max-microvolt =
> >     <2950000>;
> >      > +                                     regulator-allow-set-load;
> >      > +                                     regulator-system-load =
> >     <200000>;
> >      >                               };
> >      >                               l22 {
> >      >                                       regulator-min-microvolt =
> >     <3300000>;
> >      >
> >
> >     I'm curious, why not update sdhci-msm to set the load on the regulator?
> >
> >
> > Yes you're right, and I saw that there is ongoing work:
> > https://patchwork.kernel.org/patch/10630731/
> >
> > Howerver I thought this change would be a quicker fix and easier to
> > backport in stable trees.
> > I assume all the device-tree vmmc loads will be removed at some point
> > when driven from sdhci.
> >
>
> I hadn't seen that.  Ok, seems good to me.

NOTE: I'm personally not convinced that adding the "set_load" calls
into the SDHCI driver actually makes any sense.  I believe it adds
complexity for no benefit.  The only time you ever need to should ever
be fiddling with "set_load" calls is if the rail you're controlling
has some hope of being able to run at a lower power mode.  If there's
no hope of it being at a lower power mode then the constraints on the
rail should just force it to high power mode and be done with it.  The
patch here (using regulator-system-load) is one way to force it to a
high power mode and seems fine, but there are other ways.  See a
previous discussion [1].

NOTE: IIRC the "ongoing work" patch you pointed at always sets the
load to a fixed level to turn it to "high power mode" when the
regulator is turned on and undoes that set_load when the regulator is
turned off.  That's no longer needed as of commit 5451781dadf8
("regulator: core: Only count load for enabled consumers").  If
someone comes up with a case where it's useful to keep the SD card
rail turned on but in "low power mode" _then_ we should actually
consider adding set_load to the SD card driver.

[1] https://lkml.kernel.org/r/CAD=FV=V4WFYoKLQ72pico4HCGgLDTae7xougivv6VWOSoPhLpg@mail.gmail.com

-Doug

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD
  2018-12-13 19:14       ` Doug Anderson
@ 2019-10-11 12:55         ` Loic Poulain
  2019-10-11 16:38           ` Bjorn Andersson
  0 siblings, 1 reply; 8+ messages in thread
From: Loic Poulain @ 2019-10-11 12:55 UTC (permalink / raw)
  To: Doug Anderson, Andy Gross, Rob Herring
  Cc: jhugo, David Brown, linux-arm-msm, Bjorn Andersson, devicetree,
	Paolo Pisati, Brian Masney

Hi Andy, Rob,

Could any of you take this patch?

On Thu, 13 Dec 2018 at 20:14, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Dec 13, 2018 at 6:46 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote:
> >
> > On 12/13/2018 12:55 AM, Loic Poulain wrote:
> > > Hi Jeffrey,
> > >
> > >
> > > On Wed, 12 Dec 2018 at 18:23, Jeffrey Hugo <jhugo@codeaurora.org
> > > <mailto:jhugo@codeaurora.org>> wrote:
> > >
> > >     On 12/12/2018 10:13 AM, Loic Poulain wrote:
> > >      > In the same way as for msm8974-hammerhead, l21 load, used for SDCARD
> > >      > VMMC, needs to be increased in order to prevent any voltage drop
> > >     issues
> > >      > (due to limited current) happening with some SDCARDS or during
> > >     specific
> > >      > operations (e.g. write).
> > >      >
> > >      > Fixes: 660a9763c6a9 (arm64: dts: qcom: db820c: Add pm8994
> > >     regulator node)
> > >      > Signed-off-by: Loic Poulain <loic.poulain@linaro.org
> > >     <mailto:loic.poulain@linaro.org>>
> > >      > ---
> > >      >   arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
> > >      >   1 file changed, 2 insertions(+)
> > >      >
> > >      > diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > >     b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > >      > index 104cad9..c15e2c0 100644
> > >      > --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > >      > +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > >      > @@ -634,6 +634,8 @@
> > >      >                               l21 {
> > >      >                                       regulator-min-microvolt =
> > >     <2950000>;
> > >      >                                       regulator-max-microvolt =
> > >     <2950000>;
> > >      > +                                     regulator-allow-set-load;
> > >      > +                                     regulator-system-load =
> > >     <200000>;
> > >      >                               };
> > >      >                               l22 {
> > >      >                                       regulator-min-microvolt =
> > >     <3300000>;
> > >      >
> > >
> > >     I'm curious, why not update sdhci-msm to set the load on the regulator?
> > >
> > >
> > > Yes you're right, and I saw that there is ongoing work:
> > > https://patchwork.kernel.org/patch/10630731/
> > >
> > > Howerver I thought this change would be a quicker fix and easier to
> > > backport in stable trees.
> > > I assume all the device-tree vmmc loads will be removed at some point
> > > when driven from sdhci.
> > >
> >
> > I hadn't seen that.  Ok, seems good to me.
>
> NOTE: I'm personally not convinced that adding the "set_load" calls
> into the SDHCI driver actually makes any sense.  I believe it adds
> complexity for no benefit.  The only time you ever need to should ever
> be fiddling with "set_load" calls is if the rail you're controlling
> has some hope of being able to run at a lower power mode.  If there's
> no hope of it being at a lower power mode then the constraints on the
> rail should just force it to high power mode and be done with it.  The
> patch here (using regulator-system-load) is one way to force it to a
> high power mode and seems fine, but there are other ways.  See a
> previous discussion [1].
>
> NOTE: IIRC the "ongoing work" patch you pointed at always sets the
> load to a fixed level to turn it to "high power mode" when the
> regulator is turned on and undoes that set_load when the regulator is
> turned off.  That's no longer needed as of commit 5451781dadf8
> ("regulator: core: Only count load for enabled consumers").  If
> someone comes up with a case where it's useful to keep the SD card
> rail turned on but in "low power mode" _then_ we should actually
> consider adding set_load to the SD card driver.
>
> [1] https://lkml.kernel.org/r/CAD=FV=V4WFYoKLQ72pico4HCGgLDTae7xougivv6VWOSoPhLpg@mail.gmail.com
>
> -Doug

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD
  2019-10-11 12:55         ` Loic Poulain
@ 2019-10-11 16:38           ` Bjorn Andersson
  0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2019-10-11 16:38 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Doug Anderson, Andy Gross, Rob Herring, jhugo, David Brown,
	linux-arm-msm, devicetree, Paolo Pisati, Brian Masney

On Fri 11 Oct 05:55 PDT 2019, Loic Poulain wrote:

> Hi Andy, Rob,
> 
> Could any of you take this patch?
> 

I've applied the patch now.

Thanks,
Bjorn

> On Thu, 13 Dec 2018 at 20:14, Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Thu, Dec 13, 2018 at 6:46 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote:
> > >
> > > On 12/13/2018 12:55 AM, Loic Poulain wrote:
> > > > Hi Jeffrey,
> > > >
> > > >
> > > > On Wed, 12 Dec 2018 at 18:23, Jeffrey Hugo <jhugo@codeaurora.org
> > > > <mailto:jhugo@codeaurora.org>> wrote:
> > > >
> > > >     On 12/12/2018 10:13 AM, Loic Poulain wrote:
> > > >      > In the same way as for msm8974-hammerhead, l21 load, used for SDCARD
> > > >      > VMMC, needs to be increased in order to prevent any voltage drop
> > > >     issues
> > > >      > (due to limited current) happening with some SDCARDS or during
> > > >     specific
> > > >      > operations (e.g. write).
> > > >      >
> > > >      > Fixes: 660a9763c6a9 (arm64: dts: qcom: db820c: Add pm8994
> > > >     regulator node)
> > > >      > Signed-off-by: Loic Poulain <loic.poulain@linaro.org
> > > >     <mailto:loic.poulain@linaro.org>>
> > > >      > ---
> > > >      >   arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 2 ++
> > > >      >   1 file changed, 2 insertions(+)
> > > >      >
> > > >      > diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > > >     b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > > >      > index 104cad9..c15e2c0 100644
> > > >      > --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > > >      > +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> > > >      > @@ -634,6 +634,8 @@
> > > >      >                               l21 {
> > > >      >                                       regulator-min-microvolt =
> > > >     <2950000>;
> > > >      >                                       regulator-max-microvolt =
> > > >     <2950000>;
> > > >      > +                                     regulator-allow-set-load;
> > > >      > +                                     regulator-system-load =
> > > >     <200000>;
> > > >      >                               };
> > > >      >                               l22 {
> > > >      >                                       regulator-min-microvolt =
> > > >     <3300000>;
> > > >      >
> > > >
> > > >     I'm curious, why not update sdhci-msm to set the load on the regulator?
> > > >
> > > >
> > > > Yes you're right, and I saw that there is ongoing work:
> > > > https://patchwork.kernel.org/patch/10630731/
> > > >
> > > > Howerver I thought this change would be a quicker fix and easier to
> > > > backport in stable trees.
> > > > I assume all the device-tree vmmc loads will be removed at some point
> > > > when driven from sdhci.
> > > >
> > >
> > > I hadn't seen that.  Ok, seems good to me.
> >
> > NOTE: I'm personally not convinced that adding the "set_load" calls
> > into the SDHCI driver actually makes any sense.  I believe it adds
> > complexity for no benefit.  The only time you ever need to should ever
> > be fiddling with "set_load" calls is if the rail you're controlling
> > has some hope of being able to run at a lower power mode.  If there's
> > no hope of it being at a lower power mode then the constraints on the
> > rail should just force it to high power mode and be done with it.  The
> > patch here (using regulator-system-load) is one way to force it to a
> > high power mode and seems fine, but there are other ways.  See a
> > previous discussion [1].
> >
> > NOTE: IIRC the "ongoing work" patch you pointed at always sets the
> > load to a fixed level to turn it to "high power mode" when the
> > regulator is turned on and undoes that set_load when the regulator is
> > turned off.  That's no longer needed as of commit 5451781dadf8
> > ("regulator: core: Only count load for enabled consumers").  If
> > someone comes up with a case where it's useful to keep the SD card
> > rail turned on but in "low power mode" _then_ we should actually
> > consider adding set_load to the SD card driver.
> >
> > [1] https://lkml.kernel.org/r/CAD=FV=V4WFYoKLQ72pico4HCGgLDTae7xougivv6VWOSoPhLpg@mail.gmail.com
> >
> > -Doug

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-10-11 16:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-12 17:13 [PATCH] arm64: dts: apq8096-db820c: Increase load on l21 for SDCARD Loic Poulain
2018-12-12 17:23 ` Jeffrey Hugo
2018-12-13  7:55   ` Loic Poulain
2018-12-13 14:46     ` Jeffrey Hugo
2018-12-13 19:14       ` Doug Anderson
2019-10-11 12:55         ` Loic Poulain
2019-10-11 16:38           ` Bjorn Andersson
2018-12-12 19:46 ` Bjorn Andersson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.