[PATCH v5 0/5] i.MX6 PU power domain support

[PATCH v5 0/5] i.MX6 PU power domain support

[Philipp Zabel]

The i.MX6Q can gate off the CPU and PU (GPU/VPU) power domains using the
Power Gating Controller (PGC) in the GPC register space. The CPU power
domain is already handled by wait state code, but the PU power domain can
be controlled using the generic power domain framework and power off the PU
supply regulator if all devices in the power domain are (runtime) suspended.

This patchset adds a GPC platform device initialized at subsys_initcall time
(after anatop regulators) that binds to the gpc device tree node and sets up
the PU power domain. The GPC node becomes a power-controller as defined by
Tomasz' OF power domain bindings. This series depends on his patch
    "base: power: Add generic OF-based power domain look-up"

Using the power-domain bindings allows to use indexed power domains inside the
gpc node. This allows to use phandles with an index cell to select the power
domains, similarly to the reset lines provided by the SRC.

I have tested this on i.MX6S and noticed hangs if the GPU is not clocked during
domain powerup. As a consequence, the domain driver no longer collects the
reset clocks from drivers in the domain (as the GPU driver might not be
enabled, or even in mainline...), but is provided with the necessary clocks
directly from the device tree. This also allowed to get rid of the bus
notifier.

Finally, the device specific timing data has been dropped for now, since the
current generic OF power domain code doesn't support it yet.

Changes since v4:
 - Use OF power domain bindings
 - Provide reset clocks directly via device tree

regards
Philipp

Philipp Zabel (5):
  Documentation: Add device tree bindings for Freescale i.MX GPC
  ARM: imx6: gpc: Add PU power domain for GPU/VPU
  ARM: dts: imx6qdl: Add power-domain information to gpc node
  ARM: dts: imx6sl: Add power-domain information to gpc node
  ARM: dts: imx6qdl: Allow disabling the PU regulator, add a enable ramp
    delay

 .../devicetree/bindings/power/fsl,imx-gpc.txt      |  54 ++++++
 arch/arm/boot/dts/imx6qdl.dtsi                     |   7 +-
 arch/arm/boot/dts/imx6sl.dtsi                      |   6 +-
 arch/arm/mach-imx/Kconfig                          |   2 +
 arch/arm/mach-imx/gpc.c                            | 182 +++++++++++++++++++++
 5 files changed, 249 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/fsl,imx-gpc.txt

-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/5] Documentation: Add device tree bindings for Freescale i.MX GPC

[Philipp Zabel]

The i.MX6 contains a power controller that controls power gating and
sequencing for the SoC's power domains.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v4:
 - Use OF power domain bindings
 - Add reset clocks to be enabled during PU powerup.
---
 .../devicetree/bindings/power/fsl,imx-gpc.txt      | 54 ++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/fsl,imx-gpc.txt

diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
new file mode 100644
index 0000000..eaa8c93
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
@@ -0,0 +1,54 @@
+Freescale i.MX General Power Controller
+=======================================
+
+The i.MX6Q General Power Control (GPC) block contains DVFS load tracking
+counters and Power Gating Control (PGC) for the CPU and PU (GPU/VPU) power
+domains.
+
+Required properties:
+- compatible: Should be "fsl,imx6q-gpc" or "fsl,imx6sl-gpc"
+- reg: should be register base and length as documented in the
+  datasheet
+- interrupts: Should contain GPC interrupt request 1
+- pu-supply: Link to the LDO regulator powering the PU power domain
+- clocks: Clock phandles to devices in the PU power domain that need
+	  to be enabled during domain power-up for reset propagation.
+- #power-domain-cells: Should be 1, see below:
+
+The gpc node is a power-controller as documented by the generic power domain
+bindings in Documentation/devicetree/bindings/power/power_domain.txt.
+
+Example:
+
+	gpc: gpc@020dc000 {
+		compatible = "fsl,imx6q-gpc";
+		reg = <0x020dc000 0x4000>;
+		interrupts = <0 89 0x04 0 90 0x04>;
+		pu-supply = <&reg_pu>;
+		clocks = <&clks 122>, <&clks 74>, <&clks 121>,
+			 <&clks 26>, <&clks 143>, <&clks 168>;
+		#power-domain-cells = <1>;
+	};
+
+
+Specifying power domain for IP modules
+======================================
+
+IP cores belonging to a power domain should contain a 'power-domain' property
+that is a phandle pointing to the gpc device node and a DOMAIN_INDEX specifying
+the power domain the device belongs to.
+
+Example of a device that is part of the PU power domain:
+
+	vpu: vpu@02040000 {
+		reg = <0x02040000 0x3c000>;
+		/* ... */
+		power-domain = <&gpc 1>;
+		/* ... */
+	};
+
+The following DOMAIN_INDEX values are valid for i.MX6Q:
+ARM_DOMAIN     0
+PU_DOMAIN      1
+The following additional DOMAIN_INDEX value is valid for i.MX6SL:
+DISPLAY_DOMAIN 2
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/5] ARM: imx6: gpc: Add PU power domain for GPU/VPU

[Philipp Zabel]

When generic pm domain support is enabled, the PGC can be used
to completely gate power to the PU power domain containing GPU3D,
GPU2D, and VPU cores.
This code triggers the PGC powerdown sequence to disable the GPU/VPU
isolation cells and gate power and then disables the PU regulator.
To reenable, the reverse powerup sequence is triggered after the PU
regulator is enabled again.
The GPU and VPU devices in the PU power domain temporarily need
to be clocked during powerup, so that the reset machinery can work.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v4:
 - Add pu_domain struct to contain regulator and clocks
 - Obtain clocks from device tree instead of from bus notifier
 - Add compatible value for imx6sl (untested)
---
 arch/arm/mach-imx/Kconfig |   2 +
 arch/arm/mach-imx/gpc.c   | 182 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 184 insertions(+)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 33567aa..3c58f2e 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -808,6 +808,7 @@ config SOC_IMX6Q
 	select PL310_ERRATA_727915 if CACHE_PL310
 	select PL310_ERRATA_769419 if CACHE_PL310
 	select PM_OPP if PM
+	select PM_GENERIC_DOMAINS if PM
 
 	help
 	  This enables support for Freescale i.MX6 Quad processor.
@@ -827,6 +828,7 @@ config SOC_IMX6SL
 	select PL310_ERRATA_588369 if CACHE_PL310
 	select PL310_ERRATA_727915 if CACHE_PL310
 	select PL310_ERRATA_769419 if CACHE_PL310
+	select PM_GENERIC_DOMAINS if PM
 
 	help
 	  This enables support for Freescale i.MX6 SoloLite processor.
diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 586e017..a0f587e 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -10,19 +10,40 @@
  * http://www.gnu.org/copyleft/gpl.html
  */
 
+#include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_domain.h>
+#include <linux/regulator/consumer.h>
 #include <linux/irqchip/arm-gic.h>
 #include "common.h"
+#include "hardware.h"
 
+#define GPC_CNTR		0x000
 #define GPC_IMR1		0x008
+#define GPC_PGC_GPU_PDN		0x260
+#define GPC_PGC_GPU_PUPSCR	0x264
+#define GPC_PGC_GPU_PDNSCR	0x268
 #define GPC_PGC_CPU_PDN		0x2a0
 
 #define IMR_NUM			4
 
+#define GPU_VPU_PUP_REQ		BIT(1)
+#define GPU_VPU_PDN_REQ		BIT(0)
+
+struct pu_domain {
+	struct generic_pm_domain base;
+	struct regulator *reg;
+	struct clk *clk[6];
+	int num_clks;
+};
+
 static void __iomem *gpc_base;
 static u32 gpc_wake_irqs[IMR_NUM];
 static u32 gpc_saved_imrs[IMR_NUM];
@@ -138,3 +159,164 @@ void __init imx_gpc_init(void)
 	gic_arch_extn.irq_unmask = imx_gpc_irq_unmask;
 	gic_arch_extn.irq_set_wake = imx_gpc_irq_set_wake;
 }
+
+#ifdef CONFIG_PM
+
+static int imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
+{
+	struct pu_domain *pu = container_of(genpd, struct pu_domain, base);
+	int iso, iso2sw;
+	u32 val;
+
+	/* Read ISO and ISO2SW power down delays */
+	val = readl_relaxed(gpc_base + GPC_PGC_GPU_PDNSCR);
+	iso = val & 0x3f;
+	iso2sw = (val >> 8) & 0x3f;
+
+	/* Gate off PU domain when GPU/VPU when powered down */
+	writel_relaxed(0x1, gpc_base + GPC_PGC_GPU_PDN);
+
+	/* Request GPC to power down GPU/VPU */
+	val = readl_relaxed(gpc_base + GPC_CNTR);
+	val |= GPU_VPU_PDN_REQ;
+	writel_relaxed(val, gpc_base + GPC_CNTR);
+
+	/* Wait ISO + ISO2SW IPG clock cycles */
+	ndelay((iso + iso2sw) * 1000 / 66);
+
+	regulator_disable(pu->reg);
+
+	return 0;
+}
+
+static int imx6q_pm_pu_power_on(struct generic_pm_domain *genpd)
+{
+	struct pu_domain *pu = container_of(genpd, struct pu_domain, base);
+	int i, ret, sw, sw2iso;
+	u32 val;
+
+	ret = regulator_enable(pu->reg);
+	if (ret) {
+		pr_err("%s: failed to enable regulator: %d\n", __func__, ret);
+		return ret;
+	}
+
+	/* Enable reset clocks for all devices in the PU domain */
+	for (i = 0; i < pu->num_clks; i++)
+		clk_prepare_enable(pu->clk[i]);
+
+	/* Gate off PU domain when GPU/VPU when powered down */
+	writel_relaxed(0x1, gpc_base + GPC_PGC_GPU_PDN);
+
+	/* Read ISO and ISO2SW power down delays */
+	val = readl_relaxed(gpc_base + GPC_PGC_GPU_PUPSCR);
+	sw = val & 0x3f;
+	sw2iso = (val >> 8) & 0x3f;
+
+	/* Request GPC to power up GPU/VPU */
+	val = readl_relaxed(gpc_base + GPC_CNTR);
+	val |= GPU_VPU_PUP_REQ;
+	writel_relaxed(val, gpc_base + GPC_CNTR);
+
+	/* Wait ISO + ISO2SW IPG clock cycles */
+	ndelay((sw + sw2iso) * 1000 / 66);
+
+	/* Disable reset clocks for all devices in the PU domain */
+	for (i = 0; i < pu->num_clks; i++)
+		clk_disable_unprepare(pu->clk[i]);
+
+	return 0;
+}
+
+static struct generic_pm_domain imx6q_arm_domain = {
+	.name = "ARM",
+};
+
+static struct pu_domain imx6q_pu_domain = {
+	.base = {
+		.name = "PU",
+		.power_off = imx6q_pm_pu_power_off,
+		.power_on = imx6q_pm_pu_power_on,
+		.power_off_latency_ns = 25000,
+		.power_on_latency_ns = 2000000,
+	},
+};
+
+static struct generic_pm_domain imx6sl_display_domain = {
+	.name = "DISPLAY",
+};
+
+static struct generic_pm_domain *imx_gpc_domains[] = {
+	&imx6q_arm_domain,
+	&imx6q_pu_domain.base,
+	&imx6sl_display_domain,
+};
+
+static struct genpd_onecell_data imx_gpc_onecell_data = {
+	.domains = imx_gpc_domains,
+	.domain_num = ARRAY_SIZE(imx_gpc_domains),
+};
+
+#endif /* CONFIG_PM */
+
+static int imx_gpc_probe(struct platform_device *pdev)
+{
+	struct regulator *pu_reg;
+	struct clk *clk;
+	bool is_off;
+	int ret, i;
+
+	pu_reg = devm_regulator_get(&pdev->dev, "pu");
+	if (IS_ERR(pu_reg)) {
+		ret = PTR_ERR(pu_reg);
+		dev_err(&pdev->dev, "failed to get pu regulator: %d\n", ret);
+		return ret;
+	}
+
+	/* The regulator is initially enabled */
+	ret = regulator_enable(pu_reg);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to enable pu regulator: %d\n", ret);
+		return ret;
+	}
+	imx6q_pu_domain.base.of_node = pdev->dev.of_node;
+	imx6q_pu_domain.reg = pu_reg;
+
+	for (i = 0; ; i++) {
+		clk = of_clk_get(pdev->dev.of_node, i);
+		if (IS_ERR(clk))
+			break;
+		imx6q_pu_domain.clk[i] = clk;
+	}
+	imx6q_pu_domain.num_clks = i;
+
+	is_off = IS_ENABLED(CONFIG_PM_RUNTIME);
+	if (is_off)
+		imx6q_pm_pu_power_off(&imx6q_pu_domain.base);
+
+	pm_genpd_init(&imx6q_pu_domain.base, NULL, is_off);
+	of_genpd_add_provider(pdev->dev.of_node, of_genpd_xlate_onecell,
+			      &imx_gpc_onecell_data);
+	return 0;
+}
+
+static struct of_device_id imx_gpc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-gpc" },
+	{ .compatible = "fsl,imx6sl-gpc" },
+	{ }
+};
+
+static struct platform_driver imx_gpc_driver = {
+	.driver = {
+		.name = "imx-gpc",
+		.owner = THIS_MODULE,
+		.of_match_table = imx_gpc_dt_ids,
+	},
+	.probe = imx_gpc_probe,
+};
+
+static int __init imx_pgc_init(void)
+{
+	return platform_driver_register(&imx_gpc_driver);
+}
+subsys_initcall(imx_pgc_init);
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/5] ARM: imx6: gpc: Add PU power domain for GPU/VPU

[Shawn Guo]

On Fri, Feb 28, 2014 at 05:03:44PM +0100, Philipp Zabel wrote:
> When generic pm domain support is enabled, the PGC can be used
> to completely gate power to the PU power domain containing GPU3D,
> GPU2D, and VPU cores.
> This code triggers the PGC powerdown sequence to disable the GPU/VPU
> isolation cells and gate power and then disables the PU regulator.
> To reenable, the reverse powerup sequence is triggered after the PU
> regulator is enabled again.
> The GPU and VPU devices in the PU power domain temporarily need
> to be clocked during powerup, so that the reset machinery can work.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> Changes since v4:
>  - Add pu_domain struct to contain regulator and clocks
>  - Obtain clocks from device tree instead of from bus notifier
>  - Add compatible value for imx6sl (untested)
> ---
>  arch/arm/mach-imx/Kconfig |   2 +
>  arch/arm/mach-imx/gpc.c   | 182 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 184 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 33567aa..3c58f2e 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -808,6 +808,7 @@ config SOC_IMX6Q
>  	select PL310_ERRATA_727915 if CACHE_PL310
>  	select PL310_ERRATA_769419 if CACHE_PL310
>  	select PM_OPP if PM
> +	select PM_GENERIC_DOMAINS if PM

Would it make more sense to have HAVE_IMX_GPC select it instead?

>  
>  	help
>  	  This enables support for Freescale i.MX6 Quad processor.
> @@ -827,6 +828,7 @@ config SOC_IMX6SL
>  	select PL310_ERRATA_588369 if CACHE_PL310
>  	select PL310_ERRATA_727915 if CACHE_PL310
>  	select PL310_ERRATA_769419 if CACHE_PL310
> +	select PM_GENERIC_DOMAINS if PM
>  
>  	help
>  	  This enables support for Freescale i.MX6 SoloLite processor.
> diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
> index 586e017..a0f587e 100644
> --- a/arch/arm/mach-imx/gpc.c
> +++ b/arch/arm/mach-imx/gpc.c
> @@ -10,19 +10,40 @@
>   * http://www.gnu.org/copyleft/gpl.html
>   */
>  
> +#include <linux/clk.h>
> +#include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/irq.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_clock.h>

Is this header still needed?

> +#include <linux/pm_domain.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/irqchip/arm-gic.h>
>  #include "common.h"
> +#include "hardware.h"
>  
> +#define GPC_CNTR		0x000
>  #define GPC_IMR1		0x008
> +#define GPC_PGC_GPU_PDN		0x260
> +#define GPC_PGC_GPU_PUPSCR	0x264
> +#define GPC_PGC_GPU_PDNSCR	0x268
>  #define GPC_PGC_CPU_PDN		0x2a0
>  
>  #define IMR_NUM			4
>  
> +#define GPU_VPU_PUP_REQ		BIT(1)
> +#define GPU_VPU_PDN_REQ		BIT(0)
> +
> +struct pu_domain {
> +	struct generic_pm_domain base;
> +	struct regulator *reg;
> +	struct clk *clk[6];

Define a macro for this number?

> +	int num_clks;
> +};
> +
>  static void __iomem *gpc_base;
>  static u32 gpc_wake_irqs[IMR_NUM];
>  static u32 gpc_saved_imrs[IMR_NUM];
> @@ -138,3 +159,164 @@ void __init imx_gpc_init(void)
>  	gic_arch_extn.irq_unmask = imx_gpc_irq_unmask;
>  	gic_arch_extn.irq_set_wake = imx_gpc_irq_set_wake;
>  }
> +
> +#ifdef CONFIG_PM
> +
> +static int imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
> +{
> +	struct pu_domain *pu = container_of(genpd, struct pu_domain, base);
> +	int iso, iso2sw;
> +	u32 val;
> +
> +	/* Read ISO and ISO2SW power down delays */
> +	val = readl_relaxed(gpc_base + GPC_PGC_GPU_PDNSCR);
> +	iso = val & 0x3f;
> +	iso2sw = (val >> 8) & 0x3f;
> +
> +	/* Gate off PU domain when GPU/VPU when powered down */
> +	writel_relaxed(0x1, gpc_base + GPC_PGC_GPU_PDN);
> +
> +	/* Request GPC to power down GPU/VPU */
> +	val = readl_relaxed(gpc_base + GPC_CNTR);
> +	val |= GPU_VPU_PDN_REQ;
> +	writel_relaxed(val, gpc_base + GPC_CNTR);
> +
> +	/* Wait ISO + ISO2SW IPG clock cycles */
> +	ndelay((iso + iso2sw) * 1000 / 66);
> +
> +	regulator_disable(pu->reg);
> +
> +	return 0;
> +}
> +
> +static int imx6q_pm_pu_power_on(struct generic_pm_domain *genpd)
> +{
> +	struct pu_domain *pu = container_of(genpd, struct pu_domain, base);
> +	int i, ret, sw, sw2iso;
> +	u32 val;
> +
> +	ret = regulator_enable(pu->reg);
> +	if (ret) {
> +		pr_err("%s: failed to enable regulator: %d\n", __func__, ret);
> +		return ret;
> +	}
> +
> +	/* Enable reset clocks for all devices in the PU domain */
> +	for (i = 0; i < pu->num_clks; i++)
> +		clk_prepare_enable(pu->clk[i]);
> +
> +	/* Gate off PU domain when GPU/VPU when powered down */
> +	writel_relaxed(0x1, gpc_base + GPC_PGC_GPU_PDN);
> +
> +	/* Read ISO and ISO2SW power down delays */
> +	val = readl_relaxed(gpc_base + GPC_PGC_GPU_PUPSCR);
> +	sw = val & 0x3f;
> +	sw2iso = (val >> 8) & 0x3f;
> +
> +	/* Request GPC to power up GPU/VPU */
> +	val = readl_relaxed(gpc_base + GPC_CNTR);
> +	val |= GPU_VPU_PUP_REQ;
> +	writel_relaxed(val, gpc_base + GPC_CNTR);
> +
> +	/* Wait ISO + ISO2SW IPG clock cycles */
> +	ndelay((sw + sw2iso) * 1000 / 66);
> +
> +	/* Disable reset clocks for all devices in the PU domain */
> +	for (i = 0; i < pu->num_clks; i++)
> +		clk_disable_unprepare(pu->clk[i]);
> +
> +	return 0;
> +}
> +
> +static struct generic_pm_domain imx6q_arm_domain = {
> +	.name = "ARM",
> +};
> +
> +static struct pu_domain imx6q_pu_domain = {
> +	.base = {
> +		.name = "PU",
> +		.power_off = imx6q_pm_pu_power_off,
> +		.power_on = imx6q_pm_pu_power_on,
> +		.power_off_latency_ns = 25000,
> +		.power_on_latency_ns = 2000000,
> +	},
> +};
> +
> +static struct generic_pm_domain imx6sl_display_domain = {
> +	.name = "DISPLAY",
> +};
> +
> +static struct generic_pm_domain *imx_gpc_domains[] = {
> +	&imx6q_arm_domain,
> +	&imx6q_pu_domain.base,
> +	&imx6sl_display_domain,
> +};
> +
> +static struct genpd_onecell_data imx_gpc_onecell_data = {
> +	.domains = imx_gpc_domains,
> +	.domain_num = ARRAY_SIZE(imx_gpc_domains),
> +};
> +
> +#endif /* CONFIG_PM */
> +
> +static int imx_gpc_probe(struct platform_device *pdev)
> +{
> +	struct regulator *pu_reg;
> +	struct clk *clk;
> +	bool is_off;
> +	int ret, i;
> +
> +	pu_reg = devm_regulator_get(&pdev->dev, "pu");
> +	if (IS_ERR(pu_reg)) {
> +		ret = PTR_ERR(pu_reg);
> +		dev_err(&pdev->dev, "failed to get pu regulator: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* The regulator is initially enabled */
> +	ret = regulator_enable(pu_reg);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to enable pu regulator: %d\n", ret);
> +		return ret;
> +	}
> +	imx6q_pu_domain.base.of_node = pdev->dev.of_node;

How does this work with !CONFIG_PM build?

Shawn

> +	imx6q_pu_domain.reg = pu_reg;
> +
> +	for (i = 0; ; i++) {
> +		clk = of_clk_get(pdev->dev.of_node, i);
> +		if (IS_ERR(clk))
> +			break;
> +		imx6q_pu_domain.clk[i] = clk;
> +	}
> +	imx6q_pu_domain.num_clks = i;
> +
> +	is_off = IS_ENABLED(CONFIG_PM_RUNTIME);
> +	if (is_off)
> +		imx6q_pm_pu_power_off(&imx6q_pu_domain.base);
> +
> +	pm_genpd_init(&imx6q_pu_domain.base, NULL, is_off);
> +	of_genpd_add_provider(pdev->dev.of_node, of_genpd_xlate_onecell,
> +			      &imx_gpc_onecell_data);
> +	return 0;
> +}
> +
> +static struct of_device_id imx_gpc_dt_ids[] = {
> +	{ .compatible = "fsl,imx6q-gpc" },
> +	{ .compatible = "fsl,imx6sl-gpc" },
> +	{ }
> +};
> +
> +static struct platform_driver imx_gpc_driver = {
> +	.driver = {
> +		.name = "imx-gpc",
> +		.owner = THIS_MODULE,
> +		.of_match_table = imx_gpc_dt_ids,
> +	},
> +	.probe = imx_gpc_probe,
> +};
> +
> +static int __init imx_pgc_init(void)
> +{
> +	return platform_driver_register(&imx_gpc_driver);
> +}
> +subsys_initcall(imx_pgc_init);
> -- 
> 1.8.5.3
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/5] ARM: imx6: gpc: Add PU power domain for GPU/VPU

[Philipp Zabel]

Hi Shawn,

thank you for the comments.

Am Mittwoch, den 05.03.2014, 14:56 +0800 schrieb Shawn Guo:
> > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> > index 33567aa..3c58f2e 100644
> > --- a/arch/arm/mach-imx/Kconfig
> > +++ b/arch/arm/mach-imx/Kconfig
> > @@ -808,6 +808,7 @@ config SOC_IMX6Q
> >  	select PL310_ERRATA_727915 if CACHE_PL310
> >  	select PL310_ERRATA_769419 if CACHE_PL310
> >  	select PM_OPP if PM
> > +	select PM_GENERIC_DOMAINS if PM
> 
> Would it make more sense to have HAVE_IMX_GPC select it instead?

Yes, I'll change that.

> > +#include <linux/pm_clock.h>
> 
> Is this header still needed?

No, will remove.

[...]
> > +struct pu_domain {
> > +	struct generic_pm_domain base;
> > +	struct regulator *reg;
> > +	struct clk *clk[6];
> 
> Define a macro for this number?

#define GPC_CLK_MAX   6

[...]
> > +static int imx_gpc_probe(struct platform_device *pdev)
> > +{
> > +	struct regulator *pu_reg;
> > +	struct clk *clk;
> > +	bool is_off;
> > +	int ret, i;
> > +
> > +	pu_reg = devm_regulator_get(&pdev->dev, "pu");
> > +	if (IS_ERR(pu_reg)) {
> > +		ret = PTR_ERR(pu_reg);
> > +		dev_err(&pdev->dev, "failed to get pu regulator: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	/* The regulator is initially enabled */
> > +	ret = regulator_enable(pu_reg);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "failed to enable pu regulator: %d\n", ret);
> > +		return ret;
> > +	}
> > +	imx6q_pu_domain.base.of_node = pdev->dev.of_node;
> 
> How does this work with !CONFIG_PM build?

Since I removed the regulator-always-on property from the PU regulator
in the device tree, if CONFIG_PM is disabled, we have to request and
enable pu_reg here. Otherwise the regulator framework will helpfully
turn it off (imx6q-cpufreq never enables pu_reg).
With the PU regulator disabled, the system would hang as soon as the
CODA VPU or one of the Vivante GPU cores is accessed if !CONFIG_PM.

> > +	imx6q_pu_domain.reg = pu_reg;
> > +
> > +	for (i = 0; ; i++) {
> > +		clk = of_clk_get(pdev->dev.of_node, i);
> > +		if (IS_ERR(clk))
> > +			break;

Also, I should probably add
+		if (i >= GPC_CLK_MAX) {
+			dev_err(&pdev->dev, "more than %d clocks\n",
+				GPC_CLK_MAX);
+			return -EINVAL;
+		}
here.

> > +		imx6q_pu_domain.clk[i] = clk;
> > +	}
> > +	imx6q_pu_domain.num_clks = i;
[...]

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/5] ARM: imx6: gpc: Add PU power domain for GPU/VPU

[Shawn Guo]

On Wed, Mar 05, 2014 at 11:05:24AM +0100, Philipp Zabel wrote:
> > > +static int imx_gpc_probe(struct platform_device *pdev)
> > > +{
> > > +	struct regulator *pu_reg;
> > > +	struct clk *clk;
> > > +	bool is_off;
> > > +	int ret, i;
> > > +
> > > +	pu_reg = devm_regulator_get(&pdev->dev, "pu");
> > > +	if (IS_ERR(pu_reg)) {
> > > +		ret = PTR_ERR(pu_reg);
> > > +		dev_err(&pdev->dev, "failed to get pu regulator: %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* The regulator is initially enabled */
> > > +	ret = regulator_enable(pu_reg);
> > > +	if (ret < 0) {
> > > +		dev_err(&pdev->dev, "failed to enable pu regulator: %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +	imx6q_pu_domain.base.of_node = pdev->dev.of_node;
> > 
> > How does this work with !CONFIG_PM build?
> 
> Since I removed the regulator-always-on property from the PU regulator
> in the device tree, if CONFIG_PM is disabled, we have to request and
> enable pu_reg here. Otherwise the regulator framework will helpfully
> turn it off (imx6q-cpufreq never enables pu_reg).
> With the PU regulator disabled, the system would hang as soon as the
> CODA VPU or one of the Vivante GPU cores is accessed if !CONFIG_PM.

Sorry, I should have been more specific.  My question is how stuff like
imx6q_pu_domain are available for !CONFIG_PM build, since they are only
defined in #ifdef CONFIG_PM right above imx_gpc_probe().

Shawn

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/5] ARM: imx6: gpc: Add PU power domain for GPU/VPU

[Philipp Zabel]

Am Mittwoch, den 05.03.2014, 19:05 +0800 schrieb Shawn Guo:
> On Wed, Mar 05, 2014 at 11:05:24AM +0100, Philipp Zabel wrote:
> > > > +static int imx_gpc_probe(struct platform_device *pdev)
> > > > +{
> > > > +	struct regulator *pu_reg;
> > > > +	struct clk *clk;
> > > > +	bool is_off;
> > > > +	int ret, i;
> > > > +
> > > > +	pu_reg = devm_regulator_get(&pdev->dev, "pu");
> > > > +	if (IS_ERR(pu_reg)) {
> > > > +		ret = PTR_ERR(pu_reg);
> > > > +		dev_err(&pdev->dev, "failed to get pu regulator: %d\n", ret);
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	/* The regulator is initially enabled */
> > > > +	ret = regulator_enable(pu_reg);
> > > > +	if (ret < 0) {
> > > > +		dev_err(&pdev->dev, "failed to enable pu regulator: %d\n", ret);
> > > > +		return ret;
> > > > +	}
> > > > +	imx6q_pu_domain.base.of_node = pdev->dev.of_node;
> > > 
> > > How does this work with !CONFIG_PM build?
> > 
> > Since I removed the regulator-always-on property from the PU regulator
> > in the device tree, if CONFIG_PM is disabled, we have to request and
> > enable pu_reg here. Otherwise the regulator framework will helpfully
> > turn it off (imx6q-cpufreq never enables pu_reg).
> > With the PU regulator disabled, the system would hang as soon as the
> > CODA VPU or one of the Vivante GPU cores is accessed if !CONFIG_PM.
> 
> Sorry, I should have been more specific.  My question is how stuff like
> imx6q_pu_domain are available for !CONFIG_PM build, since they are only
> defined in #ifdef CONFIG_PM right above imx_gpc_probe().

Ouch, I misread the quoted context. Yes, everything starting from
imx6q_pu_domain.base.of_node = pdev->dev.of_node;
needs to be #ifdef CONFIG_PM.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 3/5] ARM: dts: imx6qdl: Add power-domain information to gpc node

[Philipp Zabel]

The PGC that is part of GPC controls isolation and power sequencing of the
power domains. The PU power domain will be handled by the generic pm domain
framework. It needs a phandle to the PU regulator to turn off power when
the domain is disabled, and a list of phandles to all clocks that must be
enabled during powerup for reset propagation.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v4:
 - Use OF power domain bindings
 - Added reset clocks
---
 arch/arm/boot/dts/imx6qdl.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index fb28b2e..00f0bf0 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -600,6 +600,10 @@
 				compatible = "fsl,imx6q-gpc";
 				reg = <0x020dc000 0x4000>;
 				interrupts = <0 89 0x04 0 90 0x04>;
+				pu-supply = <&reg_pu>;
+				clocks = <&clks 122>, <&clks 74>, <&clks 121>,
+					 <&clks 26>, <&clks 143>, <&clks 168>;
+				#power-domain-cells = <1>;
 			};
 
 			gpr: iomuxc-gpr@020e0000 {
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 4/5] ARM: dts: imx6sl: Add power-domain information to gpc node

[Philipp Zabel]

The PGC that is part of GPC controls isolation and power sequencing of the
power domains. The PU power domain will be handled by the generic pm domain
framework. It needs a phandle to the PU regulator to turn off power when
the domain is disabled and a list of clocks to be enabled during powerup
for reset propagation.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v4:
 - Use OF power domain bindings
 - Added reset clocks
---
 arch/arm/boot/dts/imx6sl.dtsi | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 28558f1..0e61262 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -529,9 +529,13 @@
 			};
 
 			gpc: gpc@020dc000 {
-				compatible = "fsl,imx6sl-gpc", "fsl,imx6q-gpc";
+				compatible = "fsl,imx6sl-gpc";
 				reg = <0x020dc000 0x4000>;
 				interrupts = <0 89 0x04>;
+				pu-supply = <&reg_pu>;
+				clocks = <&clks IMX6SL_CLK_GPU2D_OVG>,
+					 <&clks IMX6SL_CLK_GPU2D_PODF>;
+				#power-domain-cells = <1>;
 			};
 
 			gpr: iomuxc-gpr@020e0000 {
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 4/5] ARM: dts: imx6sl: Add power-domain information to gpc node

[Shawn Guo]

On Fri, Feb 28, 2014 at 05:03:46PM +0100, Philipp Zabel wrote:
> The PGC that is part of GPC controls isolation and power sequencing of the
> power domains. The PU power domain will be handled by the generic pm domain
> framework. It needs a phandle to the PU regulator to turn off power when
> the domain is disabled and a list of clocks to be enabled during powerup
> for reset propagation.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> Changes since v4:
>  - Use OF power domain bindings
>  - Added reset clocks
> ---
>  arch/arm/boot/dts/imx6sl.dtsi | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
> index 28558f1..0e61262 100644
> --- a/arch/arm/boot/dts/imx6sl.dtsi
> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> @@ -529,9 +529,13 @@
>  			};
>  
>  			gpc: gpc@020dc000 {
> -				compatible = "fsl,imx6sl-gpc", "fsl,imx6q-gpc";
> +				compatible = "fsl,imx6sl-gpc";

This change will break function imx_gpc_init(), which looks up
"fsl,imx6q-gpc" even on imx6sl.

Shawn

>  				reg = <0x020dc000 0x4000>;
>  				interrupts = <0 89 0x04>;
> +				pu-supply = <&reg_pu>;
> +				clocks = <&clks IMX6SL_CLK_GPU2D_OVG>,
> +					 <&clks IMX6SL_CLK_GPU2D_PODF>;
> +				#power-domain-cells = <1>;
>  			};
>  
>  			gpr: iomuxc-gpr@020e0000 {
> -- 
> 1.8.5.3
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 4/5] ARM: dts: imx6sl: Add power-domain information to gpc node

[Philipp Zabel]

Am Mittwoch, den 05.03.2014, 15:00 +0800 schrieb Shawn Guo:
> On Fri, Feb 28, 2014 at 05:03:46PM +0100, Philipp Zabel wrote:
> > The PGC that is part of GPC controls isolation and power sequencing of the
> > power domains. The PU power domain will be handled by the generic pm domain
> > framework. It needs a phandle to the PU regulator to turn off power when
> > the domain is disabled and a list of clocks to be enabled during powerup
> > for reset propagation.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > Changes since v4:
> >  - Use OF power domain bindings
> >  - Added reset clocks
> > ---
> >  arch/arm/boot/dts/imx6sl.dtsi | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
> > index 28558f1..0e61262 100644
> > --- a/arch/arm/boot/dts/imx6sl.dtsi
> > +++ b/arch/arm/boot/dts/imx6sl.dtsi
> > @@ -529,9 +529,13 @@
> >  			};
> >  
> >  			gpc: gpc@020dc000 {
> > -				compatible = "fsl,imx6sl-gpc", "fsl,imx6q-gpc";
> > +				compatible = "fsl,imx6sl-gpc";
> 
> This change will break function imx_gpc_init(), which looks up
> "fsl,imx6q-gpc" even on imx6sl.

Sorry, I'll drop this change.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 5/5] ARM: dts: imx6qdl: Allow disabling the PU regulator, add a enable ramp delay

[Philipp Zabel]

The PU regulator is enabled during boot, but not necessarily always-on.
It can be disabled by the generic pm domain framework when the PU power
domain is shut down. The ramp delay of 150 us might be a bit conservative,
the value is taken from the Freescale kernel.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 00f0bf0..57e990e 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -515,7 +515,8 @@
 					regulator-name = "vddpu";
 					regulator-min-microvolt = <725000>;
 					regulator-max-microvolt = <1450000>;
-					regulator-always-on;
+					regulator-enable-ramp-delay = <150>;
+					regulator-boot-on;
 					anatop-reg-offset = <0x140>;
 					anatop-vol-bit-shift = <9>;
 					anatop-vol-bit-width = <5>;
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html