[PATCH v2 0/5]

[PATCH v2 0/5]

[Dmitry Baryshkov]

On SM8250 MDSS_GDSC (and the rest of display clock controller) is
supplied power by MMCX power domain. Handle this link in GDSC code by
binding the power domain in dts file.

This patchset depends on [1]

Changes since v1:
 - Define fixed-regulator-domain regulator using power domain
   performance state for enabling/disabling.
 - Rework to use new fixed regulator type (fixed-regulator-domain)
   instead of controlling power domain directly from gdsc code.

Changes since RFC:
 - Fix naming of gdsc_supply_on/gdsc_supply_off functions
 - Fix detaching of solo gdsc's power domain in error handling code
 - Drop the dts patch, as respective display nodes are still not
   submitted to the mailing list.

[1]
https://lore.kernel.org/linux-arm-msm/20200927190653.13876-1-jonathan@marek.ca/

[PATCH v2 1/5] dt-bindings: regulator: fixed: provide bindings using power domain

[Dmitry Baryshkov]

Define bindings for fixed regulator using power domain performance state
to enable/disable corresponding regulator.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../bindings/regulator/fixed-regulator.yaml   | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
index 92211f2b3b0c..d3d0dc13dd8b 100644
--- a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
@@ -26,12 +26,22 @@ if:
         const: regulator-fixed-clock
   required:
     - clocks
+else:
+  if:
+    properties:
+      compatible:
+        contains:
+          const: regulator-fixed-domain
+    required:
+      - power-domains
+      - required-opps
 
 properties:
   compatible:
     enum:
       - regulator-fixed
       - regulator-fixed-clock
+      - regulator-fixed-domain
 
   regulator-name: true
 
@@ -46,6 +56,20 @@ properties:
       is mandatory if compatible is chosen to regulator-fixed-clock.
     maxItems: 1
 
+  power-domains:
+    description:
+      Power domain to use for enable control. This binding is only
+      available if the compatible is chosen to regulator-fixed-domain.
+    maxItems: 1
+
+  required-opps:
+    description:
+      Performance state to use for enable control. This binding is only
+      available if the compatible is chosen to regulator-fixed-domain. The
+      power-domain binding is mandatory if compatible is chosen to
+      regulator-fixed-domain.
+    maxItems: 1
+
   startup-delay-us:
     description: startup time in microseconds
     $ref: /schemas/types.yaml#/definitions/uint32
@@ -89,4 +113,27 @@ examples:
       gpio-open-drain;
       vin-supply = <&parent_reg>;
     };
+    reg_1v8_clk: regulator-1v8-clk {
+      compatible = "regulator-fixed-clock";
+      regulator-name = "1v8";
+      regulator-min-microvolt = <1800000>;
+      regulator-max-microvolt = <1800000>;
+      clocks = <&clock1>;
+      startup-delay-us = <70000>;
+      enable-active-high;
+      regulator-boot-on;
+      vin-supply = <&parent_reg>;
+    };
+    reg_1v8_domain: regulator-1v8-domain {
+      compatible = "regulator-fixed-domain";
+      regulator-name = "1v8";
+      regulator-min-microvolt = <1800000>;
+      regulator-max-microvolt = <1800000>;
+      power-domains = <&domain1>;
+      required-opps = <&domain1_state1>;
+      startup-delay-us = <70000>;
+      enable-active-high;
+      regulator-boot-on;
+      vin-supply = <&parent_reg>;
+    };
 ...
-- 
2.28.0

Re: [PATCH v2 1/5] dt-bindings: regulator: fixed: provide bindings using power domain

[Mark Brown]

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

On Fri, Oct 23, 2020 at 04:19:21PM +0300, Dmitry Baryshkov wrote:
> Define bindings for fixed regulator using power domain performance state
> to enable/disable corresponding regulator.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

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

[PATCH v2 2/5] regulator: fixed: support using power domain for enable/disable

[Dmitry Baryshkov]

Adds possibility to choose the compatible "fixed-regulator-domain" for
regulators which use power domain for enabling/disabling corresponding
regulator.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/regulator/fixed.c | 63 +++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 3de7709bdcd4..02ad83153e19 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -18,6 +18,8 @@
 #include <linux/mutex.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_opp.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/fixed.h>
 #include <linux/gpio/consumer.h>
@@ -34,11 +36,13 @@ struct fixed_voltage_data {
 	struct regulator_dev *dev;
 
 	struct clk *enable_clock;
-	unsigned int clk_enable_counter;
+	unsigned int enable_counter;
+	int performance_state;
 };
 
 struct fixed_dev_type {
 	bool has_enable_clock;
+	bool has_performance_state;
 };
 
 static int reg_clock_enable(struct regulator_dev *rdev)
@@ -50,7 +54,7 @@ static int reg_clock_enable(struct regulator_dev *rdev)
 	if (ret)
 		return ret;
 
-	priv->clk_enable_counter++;
+	priv->enable_counter++;
 
 	return ret;
 }
@@ -60,16 +64,41 @@ static int reg_clock_disable(struct regulator_dev *rdev)
 	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
 
 	clk_disable_unprepare(priv->enable_clock);
-	priv->clk_enable_counter--;
+	priv->enable_counter--;
 
 	return 0;
 }
 
-static int reg_clock_is_enabled(struct regulator_dev *rdev)
+static int reg_domain_enable(struct regulator_dev *rdev)
 {
 	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+	struct device *dev = rdev->dev.parent;
+	int ret;
+
+	ret = dev_pm_genpd_set_performance_state(dev, priv->performance_state);
+	if (ret)
+		return ret;
 
-	return priv->clk_enable_counter > 0;
+	priv->enable_counter++;
+
+	return ret;
+}
+
+static int reg_domain_disable(struct regulator_dev *rdev)
+{
+	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+	struct device *dev = rdev->dev.parent;
+
+	priv->enable_counter--;
+
+	return dev_pm_genpd_set_performance_state(dev, 0);
+}
+
+static int reg_is_enabled(struct regulator_dev *rdev)
+{
+	struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+
+	return priv->enable_counter > 0;
 }
 
 
@@ -129,7 +158,13 @@ static const struct regulator_ops fixed_voltage_ops = {
 static const struct regulator_ops fixed_voltage_clkenabled_ops = {
 	.enable = reg_clock_enable,
 	.disable = reg_clock_disable,
-	.is_enabled = reg_clock_is_enabled,
+	.is_enabled = reg_is_enabled,
+};
+
+static const struct regulator_ops fixed_voltage_domain_ops = {
+	.enable = reg_domain_enable,
+	.disable = reg_domain_disable,
+	.is_enabled = reg_is_enabled,
 };
 
 static int reg_fixed_voltage_probe(struct platform_device *pdev)
@@ -177,6 +212,14 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 			dev_err(dev, "Can't get enable-clock from devicetree\n");
 			return -ENOENT;
 		}
+	} else if (drvtype && drvtype->has_performance_state) {
+		drvdata->desc.ops = &fixed_voltage_domain_ops;
+
+		drvdata->performance_state = of_get_required_opp_performance_state(dev->of_node, 0);
+		if (drvdata->performance_state < 0) {
+			dev_err(dev, "Can't get performance state from devicetree\n");
+			return drvdata->performance_state;
+		}
 	} else {
 		drvdata->desc.ops = &fixed_voltage_ops;
 	}
@@ -260,6 +303,10 @@ static const struct fixed_dev_type fixed_clkenable_data = {
 	.has_enable_clock = true,
 };
 
+static const struct fixed_dev_type fixed_domain_data = {
+	.has_performance_state = true,
+};
+
 static const struct of_device_id fixed_of_match[] = {
 	{
 		.compatible = "regulator-fixed",
@@ -269,6 +316,10 @@ static const struct of_device_id fixed_of_match[] = {
 		.compatible = "regulator-fixed-clock",
 		.data = &fixed_clkenable_data,
 	},
+	{
+		.compatible = "regulator-fixed-domain",
+		.data = &fixed_domain_data,
+	},
 	{
 	},
 };
-- 
2.28.0

[PATCH v2 3/5] arm64: dts: qcom: sm8150: add mmcx regulator

[Dmitry Baryshkov]

Add regulator controlling MMCX power domain to be used by display clock
controller on SM8150.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index f0a872e02686..4c3d694b7dab 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -172,6 +172,13 @@ tcsr_mutex: hwlock {
 		#hwlock-cells = <1>;
 	};
 
+	mmcx_reg: mmcx-reg {
+		compatible = "regulator-fixed-domain";
+		power-domains = <&rpmhpd SM8150_MMCX>;
+		required-opps = <&rpmhpd_opp_low_svs>;
+		regulator-name = "MMCX";
+	};
+
 	memory@80000000 {
 		device_type = "memory";
 		/* We expect the bootloader to fill in the size */
-- 
2.28.0

[PATCH v2 4/5] arm64: dts: qcom: sm8250: add mmcx regulator

[Dmitry Baryshkov]

Add regulator controlling MMCX power domain to be used by display clock
controller on SM8250.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index d057d85a19fb..c1d4a63cd2de 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -214,6 +214,13 @@ memory@80000000 {
 		reg = <0x0 0x80000000 0x0 0x0>;
 	};
 
+	mmcx_reg: mmcx-reg {
+		compatible = "regulator-fixed-domain";
+		power-domains = <&rpmhpd SM8250_MMCX>;
+		required-opps = <&rpmhpd_opp_low_svs>;
+		regulator-name = "MMCX";
+	};
+
 	pmu {
 		compatible = "arm,armv8-pmuv3";
 		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.28.0

[PATCH v2 5/5] clk: qcom: dispcc-sm8250: handle MMCX power domain

[Dmitry Baryshkov]

On SM8250 MMCX power domain is required to access MMDS_GDSC registers.
This power domain is expressed as mmcx-supply regulator property. Use
this regulator as MDSS_GDSC supply.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/dispcc-sm8250.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index 07a98d3f882d..588575e1169d 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -963,6 +963,7 @@ static struct gdsc mdss_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.flags = HW_CTRL,
+	.supply = "mmcx",
 };
 
 static struct clk_regmap *disp_cc_sm8250_clocks[] = {
-- 
2.28.0

Re: [PATCH v2 5/5] clk: qcom: dispcc-sm8250: handle MMCX power domain

[Stephen Boyd]

Quoting Dmitry Baryshkov (2020-10-23 06:19:25)
> On SM8250 MMCX power domain is required to access MMDS_GDSC registers.
> This power domain is expressed as mmcx-supply regulator property. Use
> this regulator as MDSS_GDSC supply.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/clk/qcom/dispcc-sm8250.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
> index 07a98d3f882d..588575e1169d 100644
> --- a/drivers/clk/qcom/dispcc-sm8250.c
> +++ b/drivers/clk/qcom/dispcc-sm8250.c
> @@ -963,6 +963,7 @@ static struct gdsc mdss_gdsc = {
>         },
>         .pwrsts = PWRSTS_OFF_ON,
>         .flags = HW_CTRL,
> +       .supply = "mmcx",
>  };
>  

Can this patch be applied to clk tree or will it break the tree without
the dts/regulator bits in place?

Re: [PATCH v2 5/5] clk: qcom: dispcc-sm8250: handle MMCX power domain

[Dmitry Baryshkov]

On Thu, 5 Nov 2020 at 05:42, Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Dmitry Baryshkov (2020-10-23 06:19:25)
> > On SM8250 MMCX power domain is required to access MMDS_GDSC registers.
> > This power domain is expressed as mmcx-supply regulator property. Use
> > this regulator as MDSS_GDSC supply.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/clk/qcom/dispcc-sm8250.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
> > index 07a98d3f882d..588575e1169d 100644
> > --- a/drivers/clk/qcom/dispcc-sm8250.c
> > +++ b/drivers/clk/qcom/dispcc-sm8250.c
> > @@ -963,6 +963,7 @@ static struct gdsc mdss_gdsc = {
> >         },
> >         .pwrsts = PWRSTS_OFF_ON,
> >         .flags = HW_CTRL,
> > +       .supply = "mmcx",
> >  };
> >
>
> Can this patch be applied to clk tree or will it break the tree without
> the dts/regulator bits in place?

It can be applied to clk tree. Regulator bits are already accepted by
Mark Brown.


-- 
With best wishes
Dmitry

Re: [PATCH v2 5/5] clk: qcom: dispcc-sm8250: handle MMCX power domain

[Stephen Boyd]

Quoting Dmitry Baryshkov (2020-10-23 06:19:25)
> On SM8250 MMCX power domain is required to access MMDS_GDSC registers.
> This power domain is expressed as mmcx-supply regulator property. Use
> this regulator as MDSS_GDSC supply.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Applied to clk-next

Re: [PATCH v2 0/5]

[Mark Brown]

On Fri, 23 Oct 2020 16:19:20 +0300, Dmitry Baryshkov wrote:
> On SM8250 MDSS_GDSC (and the rest of display clock controller) is
> supplied power by MMCX power domain. Handle this link in GDSC code by
> binding the power domain in dts file.
> 
> This patchset depends on [1]
> 
> Changes since v1:
>  - Define fixed-regulator-domain regulator using power domain
>    performance state for enabling/disabling.
>  - Rework to use new fixed regulator type (fixed-regulator-domain)
>    instead of controlling power domain directly from gdsc code.
> 
> [...]

Applied to

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

Thanks!

[1/2] regulator: fixed: provide bindings using power domain
      commit: d4189bc55d5c40251abaa1f341796aac84ddfb10
[2/2] regulator: fixed: support using power domain for enable/disable
      commit: bf3a28cf42412c0a85631da94f198048bb37a8e5

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