[PATCH 0/4] clk: qcom: gdsc: Handle supply regulators

[PATCH 0/4] clk: qcom: gdsc: Handle supply regulators

[Bjorn Andersson]

Handle supply regulators for GDSCs to allow probe deferral when regulators are
not yet present/enabled and to allow the GDSC to enable/disable dependencies as
needed.

Bjorn Andersson (3):
  clk: qcom: gdsc: Handle GDSC regulator supplies
  clk: qcom: mmcc-msm8996: Properly describe GPU_GX gdsc
  arm64: dts: qcom: msm8996: Make GPU node control GPU_GX GDSC

Rajendra Nayak (1):
  arm64: dts: qcom: db820c: Add s2 regulator in pmi8994

 .../devicetree/bindings/clock/qcom,mmcc.yaml  |  4 ++++
 arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi  | 14 +++++++++++
 arch/arm64/boot/dts/qcom/msm8996.dtsi         |  2 +-
 arch/arm64/boot/dts/qcom/pmi8994.dtsi         |  6 +++++
 drivers/clk/qcom/gdsc.c                       | 24 +++++++++++++++++++
 drivers/clk/qcom/gdsc.h                       |  4 ++++
 drivers/clk/qcom/mmcc-msm8996.c               |  2 ++
 7 files changed, 55 insertions(+), 1 deletion(-)

-- 
2.24.0

[PATCH 1/4] clk: qcom: gdsc: Handle GDSC regulator supplies

[Bjorn Andersson]

Certain GDSCs, such as the GPU_GX on MSM8996, requires that the upstream
regulator supply is powered in order to be turned on.

It's not guaranteed that the bootloader will leave these supplies on and
the driver core will attempt to enable any GDSCs before allowing the
individual drivers to probe defer on the PMIC regulator driver not yet
being present.

So the gdsc driver needs to be made aware of supplying regulators and
probe defer on their absence, and it needs to enable and disable the
regulator accordingly.

Voltage adjustments of the supplying regulator are deferred to the
client drivers themselves.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/clk/qcom/gdsc.c | 24 ++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h |  4 ++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index a250f59708d8..3528789cc9d0 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -13,6 +13,7 @@
 #include <linux/regmap.h>
 #include <linux/reset-controller.h>
 #include <linux/slab.h>
+#include <linux/regulator/consumer.h>
 #include "gdsc.h"
 
 #define PWR_ON_MASK		BIT(31)
@@ -112,6 +113,12 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
 	int ret;
 	u32 val = (status == GDSC_ON) ? 0 : SW_COLLAPSE_MASK;
 
+	if (status == GDSC_ON && sc->rsupply) {
+		ret = regulator_enable(sc->rsupply);
+		if (ret < 0)
+			return ret;
+	}
+
 	ret = regmap_update_bits(sc->regmap, sc->gdscr, SW_COLLAPSE_MASK, val);
 	if (ret)
 		return ret;
@@ -143,6 +150,13 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
 
 	ret = gdsc_poll_status(sc, status);
 	WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
+
+	if (!ret && status == GDSC_OFF && sc->rsupply) {
+		ret = regulator_disable(sc->rsupply);
+		if (ret < 0)
+			return ret;
+	}
+
 	return ret;
 }
 
@@ -371,6 +385,16 @@ int gdsc_register(struct gdsc_desc *desc,
 	if (!data->domains)
 		return -ENOMEM;
 
+	/* Resolve any regulator supplies */
+	for (i = 0; i < num; i++) {
+		if (!scs[i] || !scs[i]->supply)
+			continue;
+
+		scs[i]->rsupply = devm_regulator_get(dev, scs[i]->supply);
+		if (IS_ERR(scs[i]->rsupply))
+			return PTR_ERR(scs[i]->rsupply);
+	}
+
 	data->num_domains = num;
 	for (i = 0; i < num; i++) {
 		if (!scs[i])
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 64cdc8cf0d4d..c36fc26dcdff 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -10,6 +10,7 @@
 #include <linux/pm_domain.h>
 
 struct regmap;
+struct regulator;
 struct reset_controller_dev;
 
 /**
@@ -52,6 +53,9 @@ struct gdsc {
 	struct reset_controller_dev	*rcdev;
 	unsigned int			*resets;
 	unsigned int			reset_count;
+
+	const char 			*supply;
+	struct regulator		*rsupply;
 };
 
 struct gdsc_desc {
-- 
2.24.0

Re: [PATCH 1/4] clk: qcom: gdsc: Handle GDSC regulator supplies

[Stephen Boyd]

Quoting Bjorn Andersson (2020-03-18 22:38:59)
> Certain GDSCs, such as the GPU_GX on MSM8996, requires that the upstream
> regulator supply is powered in order to be turned on.
> 
> It's not guaranteed that the bootloader will leave these supplies on and
> the driver core will attempt to enable any GDSCs before allowing the
> individual drivers to probe defer on the PMIC regulator driver not yet
> being present.
> 
> So the gdsc driver needs to be made aware of supplying regulators and
> probe defer on their absence, and it needs to enable and disable the
> regulator accordingly.
> 
> Voltage adjustments of the supplying regulator are deferred to the
> client drivers themselves.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

Ok. Looks mostly fine to me.

>  drivers/clk/qcom/gdsc.c | 24 ++++++++++++++++++++++++
>  drivers/clk/qcom/gdsc.h |  4 ++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index a250f59708d8..3528789cc9d0 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -13,6 +13,7 @@
>  #include <linux/regmap.h>
>  #include <linux/reset-controller.h>
>  #include <linux/slab.h>
> +#include <linux/regulator/consumer.h>

Alphabetical?

>  #include "gdsc.h"
>  
>  #define PWR_ON_MASK            BIT(31)
> @@ -371,6 +385,16 @@ int gdsc_register(struct gdsc_desc *desc,
>         if (!data->domains)
>                 return -ENOMEM;
>  
> +       /* Resolve any regulator supplies */

Drop the comment please, it's just saying what the code is doing.

> +       for (i = 0; i < num; i++) {
> +               if (!scs[i] || !scs[i]->supply)
> +                       continue;
> +
> +               scs[i]->rsupply = devm_regulator_get(dev, scs[i]->supply);
> +               if (IS_ERR(scs[i]->rsupply))
> +                       return PTR_ERR(scs[i]->rsupply);
> +       }
> +
>         data->num_domains = num;
>         for (i = 0; i < num; i++) {
>                 if (!scs[i])

Re: [PATCH 1/4] clk: qcom: gdsc: Handle GDSC regulator supplies

[Taniya Das]

Hi Stephen,

I think the upstream design always wanted the client/consumer to enable 
the GPU Rail and then turn ON the GDSC?

Why are we going ahead with adding the support of regulator in the GDSC 
driver?

On 3/19/2020 11:08 AM, Bjorn Andersson wrote:
> Certain GDSCs, such as the GPU_GX on MSM8996, requires that the upstream
> regulator supply is powered in order to be turned on.
> 
> It's not guaranteed that the bootloader will leave these supplies on and
> the driver core will attempt to enable any GDSCs before allowing the
> individual drivers to probe defer on the PMIC regulator driver not yet
> being present.
> 
> So the gdsc driver needs to be made aware of supplying regulators and
> probe defer on their absence, and it needs to enable and disable the
> regulator accordingly.
> 
> Voltage adjustments of the supplying regulator are deferred to the
> client drivers themselves.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>   drivers/clk/qcom/gdsc.c | 24 ++++++++++++++++++++++++
>   drivers/clk/qcom/gdsc.h |  4 ++++
>   2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index a250f59708d8..3528789cc9d0 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -13,6 +13,7 @@
>   #include <linux/regmap.h>
>   #include <linux/reset-controller.h>
>   #include <linux/slab.h>
> +#include <linux/regulator/consumer.h>
>   #include "gdsc.h"
>   
>   #define PWR_ON_MASK		BIT(31)
> @@ -112,6 +113,12 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
>   	int ret;
>   	u32 val = (status == GDSC_ON) ? 0 : SW_COLLAPSE_MASK;
>   
> +	if (status == GDSC_ON && sc->rsupply) {
> +		ret = regulator_enable(sc->rsupply);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>   	ret = regmap_update_bits(sc->regmap, sc->gdscr, SW_COLLAPSE_MASK, val);
>   	if (ret)
>   		return ret;
> @@ -143,6 +150,13 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
>   
>   	ret = gdsc_poll_status(sc, status);
>   	WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
> +
> +	if (!ret && status == GDSC_OFF && sc->rsupply) {
> +		ret = regulator_disable(sc->rsupply);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>   	return ret;
>   }
>   
> @@ -371,6 +385,16 @@ int gdsc_register(struct gdsc_desc *desc,
>   	if (!data->domains)
>   		return -ENOMEM;
>   
> +	/* Resolve any regulator supplies */
> +	for (i = 0; i < num; i++) {
> +		if (!scs[i] || !scs[i]->supply)
> +			continue;
> +
> +		scs[i]->rsupply = devm_regulator_get(dev, scs[i]->supply);
> +		if (IS_ERR(scs[i]->rsupply))
> +			return PTR_ERR(scs[i]->rsupply);
> +	}
> +
>   	data->num_domains = num;
>   	for (i = 0; i < num; i++) {
>   		if (!scs[i])
> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> index 64cdc8cf0d4d..c36fc26dcdff 100644
> --- a/drivers/clk/qcom/gdsc.h
> +++ b/drivers/clk/qcom/gdsc.h
> @@ -10,6 +10,7 @@
>   #include <linux/pm_domain.h>
>   
>   struct regmap;
> +struct regulator;
>   struct reset_controller_dev;
>   
>   /**
> @@ -52,6 +53,9 @@ struct gdsc {
>   	struct reset_controller_dev	*rcdev;
>   	unsigned int			*resets;
>   	unsigned int			reset_count;
> +
> +	const char 			*supply;
> +	struct regulator		*rsupply;
>   };
>   
>   struct gdsc_desc {
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [PATCH 1/4] clk: qcom: gdsc: Handle GDSC regulator supplies

[Bjorn Andersson]

On Mon 30 Mar 22:35 PDT 2020, Taniya Das wrote:

> Hi Stephen,
> 
> I think the upstream design always wanted the client/consumer to enable the
> GPU Rail and then turn ON the GDSC?
> 
> Why are we going ahead with adding the support of regulator in the GDSC
> driver?
> 

As I (partially) describe below the mdss driver on 8996 doesn't probe
because the GDSC fails to enable, because the upstream supply is not
enabled, so the mdss driver can't turn on the regulator needed by the
GDSC.

I don't see any other way to handle this than extending the gdsc
implementation, hence my proposal to change the design.
Suggestions/feedback are welcome though.

Regards,
Bjorn

> On 3/19/2020 11:08 AM, Bjorn Andersson wrote:
> > Certain GDSCs, such as the GPU_GX on MSM8996, requires that the upstream
> > regulator supply is powered in order to be turned on.
> > 
> > It's not guaranteed that the bootloader will leave these supplies on and
> > the driver core will attempt to enable any GDSCs before allowing the
> > individual drivers to probe defer on the PMIC regulator driver not yet
> > being present.
> > 
> > So the gdsc driver needs to be made aware of supplying regulators and
> > probe defer on their absence, and it needs to enable and disable the
> > regulator accordingly.
> > 
> > Voltage adjustments of the supplying regulator are deferred to the
> > client drivers themselves.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >   drivers/clk/qcom/gdsc.c | 24 ++++++++++++++++++++++++
> >   drivers/clk/qcom/gdsc.h |  4 ++++
> >   2 files changed, 28 insertions(+)
> > 
> > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> > index a250f59708d8..3528789cc9d0 100644
> > --- a/drivers/clk/qcom/gdsc.c
> > +++ b/drivers/clk/qcom/gdsc.c
> > @@ -13,6 +13,7 @@
> >   #include <linux/regmap.h>
> >   #include <linux/reset-controller.h>
> >   #include <linux/slab.h>
> > +#include <linux/regulator/consumer.h>
> >   #include "gdsc.h"
> >   #define PWR_ON_MASK		BIT(31)
> > @@ -112,6 +113,12 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
> >   	int ret;
> >   	u32 val = (status == GDSC_ON) ? 0 : SW_COLLAPSE_MASK;
> > +	if (status == GDSC_ON && sc->rsupply) {
> > +		ret = regulator_enable(sc->rsupply);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> >   	ret = regmap_update_bits(sc->regmap, sc->gdscr, SW_COLLAPSE_MASK, val);
> >   	if (ret)
> >   		return ret;
> > @@ -143,6 +150,13 @@ static int gdsc_toggle_logic(struct gdsc *sc, enum gdsc_status status)
> >   	ret = gdsc_poll_status(sc, status);
> >   	WARN(ret, "%s status stuck at 'o%s'", sc->pd.name, status ? "ff" : "n");
> > +
> > +	if (!ret && status == GDSC_OFF && sc->rsupply) {
> > +		ret = regulator_disable(sc->rsupply);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> >   	return ret;
> >   }
> > @@ -371,6 +385,16 @@ int gdsc_register(struct gdsc_desc *desc,
> >   	if (!data->domains)
> >   		return -ENOMEM;
> > +	/* Resolve any regulator supplies */
> > +	for (i = 0; i < num; i++) {
> > +		if (!scs[i] || !scs[i]->supply)
> > +			continue;
> > +
> > +		scs[i]->rsupply = devm_regulator_get(dev, scs[i]->supply);
> > +		if (IS_ERR(scs[i]->rsupply))
> > +			return PTR_ERR(scs[i]->rsupply);
> > +	}
> > +
> >   	data->num_domains = num;
> >   	for (i = 0; i < num; i++) {
> >   		if (!scs[i])
> > diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
> > index 64cdc8cf0d4d..c36fc26dcdff 100644
> > --- a/drivers/clk/qcom/gdsc.h
> > +++ b/drivers/clk/qcom/gdsc.h
> > @@ -10,6 +10,7 @@
> >   #include <linux/pm_domain.h>
> >   struct regmap;
> > +struct regulator;
> >   struct reset_controller_dev;
> >   /**
> > @@ -52,6 +53,9 @@ struct gdsc {
> >   	struct reset_controller_dev	*rcdev;
> >   	unsigned int			*resets;
> >   	unsigned int			reset_count;
> > +
> > +	const char 			*supply;
> > +	struct regulator		*rsupply;
> >   };
> >   struct gdsc_desc {
> > 
> 
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation.
> 
> --

[PATCH 2/4] clk: qcom: mmcc-msm8996: Properly describe GPU_GX gdsc

[Bjorn Andersson]

The GPU_GX GDSC depends on both GPU GDSC being enabled and that the
VDD_GX rail is powered, so update the description of the node to cover
these requirements.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 Documentation/devicetree/bindings/clock/qcom,mmcc.yaml | 4 ++++
 drivers/clk/qcom/mmcc-msm8996.c                        | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
index 85518494ce43..65d9aa790581 100644
--- a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
@@ -67,6 +67,10 @@ properties:
     description:
        Protected clock specifier list as per common clock binding
 
+  vdd_gfx-supply:
+    description:
+      Regulator supply for the GPU_GX GDSC
+
 required:
   - compatible
   - reg
diff --git a/drivers/clk/qcom/mmcc-msm8996.c b/drivers/clk/qcom/mmcc-msm8996.c
index 6c7592ddf8bb..fd43a35db13b 100644
--- a/drivers/clk/qcom/mmcc-msm8996.c
+++ b/drivers/clk/qcom/mmcc-msm8996.c
@@ -3064,7 +3064,9 @@ static struct gdsc gpu_gx_gdsc = {
 		.name = "gpu_gx",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
+	.parent = &gpu_gdsc.pd,
 	.flags = CLAMP_IO,
+	.supply = "vdd_gfx",
 };
 
 static struct clk_regmap *mmcc_msm8996_clocks[] = {
-- 
2.24.0

Re: [PATCH 2/4] clk: qcom: mmcc-msm8996: Properly describe GPU_GX gdsc

[Stephen Boyd]

Quoting Bjorn Andersson (2020-03-18 22:39:00)
> diff --git a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> index 85518494ce43..65d9aa790581 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> @@ -67,6 +67,10 @@ properties:
>      description:
>         Protected clock specifier list as per common clock binding
>  
> +  vdd_gfx-supply:

Why not vdd-gfx-supply? What's with the underscore?

> +    description:
> +      Regulator supply for the GPU_GX GDSC
> +
>  required:
>    - compatible
>    - reg

Re: [PATCH 2/4] clk: qcom: mmcc-msm8996: Properly describe GPU_GX gdsc

[Bjorn Andersson]

On Fri 20 Mar 16:31 PDT 2020, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2020-03-18 22:39:00)
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > index 85518494ce43..65d9aa790581 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > @@ -67,6 +67,10 @@ properties:
> >      description:
> >         Protected clock specifier list as per common clock binding
> >  
> > +  vdd_gfx-supply:
> 
> Why not vdd-gfx-supply? What's with the underscore?
> 

The pad is named "VDD_GFX" in the datasheet and the schematics. I see
that we've started y/_/-/ in some of the newly added bindings, would you
prefer I follow this?

Regards,
Bjorn

> > +    description:
> > +      Regulator supply for the GPU_GX GDSC
> > +
> >  required:
> >    - compatible
> >    - reg

Re: [PATCH 2/4] clk: qcom: mmcc-msm8996: Properly describe GPU_GX gdsc

[Stephen Boyd]

Quoting Bjorn Andersson (2020-03-20 22:16:12)
> On Fri 20 Mar 16:31 PDT 2020, Stephen Boyd wrote:
> 
> > Quoting Bjorn Andersson (2020-03-18 22:39:00)
> > > diff --git a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > > index 85518494ce43..65d9aa790581 100644
> > > --- a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > > @@ -67,6 +67,10 @@ properties:
> > >      description:
> > >         Protected clock specifier list as per common clock binding
> > >  
> > > +  vdd_gfx-supply:
> > 
> > Why not vdd-gfx-supply? What's with the underscore?
> > 
> 
> The pad is named "VDD_GFX" in the datasheet and the schematics. I see
> that we've started y/_/-/ in some of the newly added bindings, would you
> prefer I follow this?

If the datasheet has this then I guess it's fine. I'll wait for Rob to
ack.

Re: [PATCH 2/4] clk: qcom: mmcc-msm8996: Properly describe GPU_GX gdsc

[Rob Herring]

On Sat, Mar 21, 2020 at 11:43:12AM -0700, Stephen Boyd wrote:
> Quoting Bjorn Andersson (2020-03-20 22:16:12)
> > On Fri 20 Mar 16:31 PDT 2020, Stephen Boyd wrote:
> > 
> > > Quoting Bjorn Andersson (2020-03-18 22:39:00)
> > > > diff --git a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > > > index 85518494ce43..65d9aa790581 100644
> > > > --- a/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > > > +++ b/Documentation/devicetree/bindings/clock/qcom,mmcc.yaml
> > > > @@ -67,6 +67,10 @@ properties:
> > > >      description:
> > > >         Protected clock specifier list as per common clock binding
> > > >  
> > > > +  vdd_gfx-supply:
> > > 
> > > Why not vdd-gfx-supply? What's with the underscore?
> > > 
> > 
> > The pad is named "VDD_GFX" in the datasheet and the schematics. I see
> > that we've started y/_/-/ in some of the newly added bindings, would you
> > prefer I follow this?
> 
> If the datasheet has this then I guess it's fine. I'll wait for Rob to
> ack.

vddgfx or vdd-gfx.

Rob

[PATCH 3/4] arm64: dts: qcom: db820c: Add s2 regulator in pmi8994

[Bjorn Andersson]

From: Rajendra Nayak <rnayak@codeaurora.org>

Add the SPMI regulator node in the PMI8994, use it to give us VDD_GX
at a fixed max nominal voltage for the db820c and specify this as supply
for the MMSS GPU_GX GDSC.

With the introduction of CPR support the range for VDD_GX should be
expanded.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
[bjorn: Split between pmi8994 and db820c, changed voltage, rewrote commit message]
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 14 ++++++++++++++
 arch/arm64/boot/dts/qcom/pmi8994.dtsi        |  6 ++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
index 4692b7ad16b7..075cebaec3f3 100644
--- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
@@ -251,6 +251,10 @@ &mdss {
 	status = "okay";
 };
 
+&mmcc {
+	vdd_gfx-supply = <&vdd_gfx>;
+};
+
 &msmgpio {
 	gpio-line-names =
 		"[SPI0_DOUT]", /* GPIO_0, BLSP1_SPI_MOSI, LSEC pin 14 */
@@ -688,6 +692,16 @@ pinconf {
 	};
 };
 
+
+&pmi8994_spmi_regulators {
+	vdd_gfx: s2@1700 {
+		reg = <0x1700 0x100>;
+		regulator-name = "VDD_GFX";
+		regulator-min-microvolt = <980000>;
+		regulator-max-microvolt = <980000>;
+	};
+};
+
 &rpm_requests {
 	pm8994-regulators {
 		compatible = "qcom,rpm-pm8994-regulators";
diff --git a/arch/arm64/boot/dts/qcom/pmi8994.dtsi b/arch/arm64/boot/dts/qcom/pmi8994.dtsi
index 21e05215abe4..e5ed28ab9b2d 100644
--- a/arch/arm64/boot/dts/qcom/pmi8994.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmi8994.dtsi
@@ -26,5 +26,11 @@ pmic@3 {
 		reg = <0x3 SPMI_USID>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+
+		pmi8994_spmi_regulators: regulators {
+			compatible = "qcom,pmi8994-regulators";
+			#address-cells = <1>;
+			#size-cells = <1>;
+		};
 	};
 };
-- 
2.24.0

Re: [PATCH 3/4] arm64: dts: qcom: db820c: Add s2 regulator in pmi8994

[Stephen Boyd]

Quoting Bjorn Andersson (2020-03-18 22:39:01)
> From: Rajendra Nayak <rnayak@codeaurora.org>
> 
> Add the SPMI regulator node in the PMI8994, use it to give us VDD_GX
> at a fixed max nominal voltage for the db820c and specify this as supply
> for the MMSS GPU_GX GDSC.
> 
> With the introduction of CPR support the range for VDD_GX should be
> expanded.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> [bjorn: Split between pmi8994 and db820c, changed voltage, rewrote commit message]
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

These two dts patches don't need to go through clk tree right? And the
first patch can be applied and regulator core will just return us a
dummy supply so it's safe to apply now?

Re: [PATCH 3/4] arm64: dts: qcom: db820c: Add s2 regulator in pmi8994

[Bjorn Andersson]

On Fri 20 Mar 16:33 PDT 2020, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2020-03-18 22:39:01)
> > From: Rajendra Nayak <rnayak@codeaurora.org>
> > 
> > Add the SPMI regulator node in the PMI8994, use it to give us VDD_GX
> > at a fixed max nominal voltage for the db820c and specify this as supply
> > for the MMSS GPU_GX GDSC.
> > 
> > With the introduction of CPR support the range for VDD_GX should be
> > expanded.
> > 
> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> > [bjorn: Split between pmi8994 and db820c, changed voltage, rewrote commit message]
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> 
> These two dts patches don't need to go through clk tree right? And the
> first patch can be applied and regulator core will just return us a
> dummy supply so it's safe to apply now?

Right, the two pairs can be merged completely independently; we just
need to agree on the _ vs -

Regards,
Bjorn

[PATCH 4/4] arm64: dts: qcom: msm8996: Make GPU node control GPU_GX GDSC

[Bjorn Andersson]

Presumably the GPU node needs to control both the GPU and GPU GX power
domains, but given that GPU GX now depends on the GPU GDSC both can
effectively be controlled by controlling GPU GX. So use this instead.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 14827adebd94..f29f45e9737b 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -639,7 +639,7 @@ gpu@b00000 {
 				"mem",
 				"mem_iface";
 
-			power-domains = <&mmcc GPU_GDSC>;
+			power-domains = <&mmcc GPU_GX_GDSC>;
 			iommus = <&adreno_smmu 0>;
 
 			nvmem-cells = <&gpu_speed_bin>;
-- 
2.24.0