All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] i.MX6 power gating controller rework
@ 2017-01-20 15:52 ` Lucas Stach
  0 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ

This is a new attempt at rewriting the i.MX6 GPC driver to properly
support multiple power domains. This needs a new DT binding to properly
describe the power domain properties in the devicetree. Newer SoCs
like the i.MX6SX are exposing more than 1 device power domain.

This new series splits out the PGC part of the GPC driver and moves it
from the architecture code to drivers/soc. It is essentially a complete
re-write of the driver, so this time I didn't try to keep it in one
commit, as this seems to cause more confusion than good for the reviewers.

The new driver keeps compatibility with the old DT binding, but new
features will only be added using the new binding.

Lucas Stach (4):
  dt-bindings: add multidomain support to i.MX GPC DT binding
  ARM: imx6: remove PGC handling from GPC driver
  soc/imx: add new GPC driver
  ARM: imx6: adopt DT to new GPC binding

 .../devicetree/bindings/power/fsl,imx-gpc.txt      |  81 ++--
 arch/arm/boot/dts/imx6q.dtsi                       |   2 +-
 arch/arm/boot/dts/imx6qdl.dtsi                     |  38 +-
 arch/arm/mach-imx/gpc.c                            | 217 ----------
 drivers/soc/Makefile                               |   1 +
 drivers/soc/imx/Makefile                           |   1 +
 drivers/soc/imx/gpc.c                              | 480 +++++++++++++++++++++
 7 files changed, 564 insertions(+), 256 deletions(-)
 create mode 100644 drivers/soc/imx/Makefile
 create mode 100644 drivers/soc/imx/gpc.c

-- 
2.11.0

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 0/4] i.MX6 power gating controller rework
@ 2017-01-20 15:52 ` Lucas Stach
  0 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

This is a new attempt at rewriting the i.MX6 GPC driver to properly
support multiple power domains. This needs a new DT binding to properly
describe the power domain properties in the devicetree. Newer SoCs
like the i.MX6SX are exposing more than 1 device power domain.

This new series splits out the PGC part of the GPC driver and moves it
from the architecture code to drivers/soc. It is essentially a complete
re-write of the driver, so this time I didn't try to keep it in one
commit, as this seems to cause more confusion than good for the reviewers.

The new driver keeps compatibility with the old DT binding, but new
features will only be added using the new binding.

Lucas Stach (4):
  dt-bindings: add multidomain support to i.MX GPC DT binding
  ARM: imx6: remove PGC handling from GPC driver
  soc/imx: add new GPC driver
  ARM: imx6: adopt DT to new GPC binding

 .../devicetree/bindings/power/fsl,imx-gpc.txt      |  81 ++--
 arch/arm/boot/dts/imx6q.dtsi                       |   2 +-
 arch/arm/boot/dts/imx6qdl.dtsi                     |  38 +-
 arch/arm/mach-imx/gpc.c                            | 217 ----------
 drivers/soc/Makefile                               |   1 +
 drivers/soc/imx/Makefile                           |   1 +
 drivers/soc/imx/gpc.c                              | 480 +++++++++++++++++++++
 7 files changed, 564 insertions(+), 256 deletions(-)
 create mode 100644 drivers/soc/imx/Makefile
 create mode 100644 drivers/soc/imx/gpc.c

-- 
2.11.0

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 1/4] dt-bindings: add multidomain support to i.MX GPC DT binding
  2017-01-20 15:52 ` Lucas Stach
@ 2017-01-20 15:52     ` Lucas Stach
  -1 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ

This adds a new binding for the Freescale i.MX GPC block, which allows
to describe multiple power domains in a more natural way. The driver
will continue to support the old binding for existing DTBs, but new
features like the additional domains present on i.MX6SX will only be
usable with the new binding.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 .../devicetree/bindings/power/fsl,imx-gpc.txt      | 81 ++++++++++++++--------
 1 file changed, 54 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
index 65cc0345747d..e5b660018d63 100644
--- a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
+++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
@@ -1,22 +1,41 @@
 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.
+The i.MX6 General Power Control (GPC) block contains DVFS load tracking
+counters and Power Gating Control (PGC).
 
 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:
+- interrupts: Should contain one interrupt specifier for the GPC interrupt
+- clocks: Must contain an entry for each entry in clock-names.
+  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - ipg
+- pgc: a list of child nodes describing the SoC power domains controlled by the
+  power gating controller.
 
-The gpc node is a power-controller as documented by the generic power domain
-bindings in Documentation/devicetree/bindings/power/power_domain.txt.
+The power domains are generic power domain providers as documented in
+Documentation/devicetree/bindings/power/power_domain.txt. They are described as
+subnodes of the GPC and should contain the following
+
+Required properties:
+- reg: the DOMAIN_INDEX as used by the client devices to refer to this
+  power domain
+  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
+
+- #power-domain-cells: Should be 0
+
+Optional properties:
+- clocks: a number of phandles to clocks that need to be enabled during domain
+  power-up sequencing to ensure reset propagation into devices located inside
+  this power domain
+- domain-supply: a phandle to the regulator powering this domain
 
 Example:
 
@@ -25,14 +44,29 @@ Example:
 		reg = <0x020dc000 0x4000>;
 		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
 			     <0 90 IRQ_TYPE_LEVEL_HIGH>;
-		pu-supply = <&reg_pu>;
-		clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
-			 <&clks IMX6QDL_CLK_GPU3D_SHADER>,
-			 <&clks IMX6QDL_CLK_GPU2D_CORE>,
-			 <&clks IMX6QDL_CLK_GPU2D_AXI>,
-			 <&clks IMX6QDL_CLK_OPENVG_AXI>,
-			 <&clks IMX6QDL_CLK_VPU_AXI>;
-		#power-domain-cells = <1>;
+		clocks = <&clks IMX6QDL_CLK_IPG>;
+		clock-names = "ipg";
+		
+		pgc {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			power-domain@0 {
+				reg = <0>;
+				#power-domain-cells = <0>;
+			};
+			pd_pu: power-domain@1 {
+				reg = <1>;
+				#power-domain-cells = <0>;
+				domain-supply = <&reg_pu>;
+				clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
+				         <&clks IMX6QDL_CLK_GPU3D_SHADER>,
+				         <&clks IMX6QDL_CLK_GPU2D_CORE>,
+				         <&clks IMX6QDL_CLK_GPU2D_AXI>,
+				         <&clks IMX6QDL_CLK_OPENVG_AXI>,
+				         <&clks IMX6QDL_CLK_VPU_AXI>;
+			};
+		};
 	};
 
 
@@ -40,20 +74,13 @@ Specifying power domain for IP modules
 ======================================
 
 IP cores belonging to a power domain should contain a 'power-domains' property
-that is a phandle pointing to the gpc device node and a DOMAIN_INDEX specifying
-the power domain the device belongs to.
+that is a phandle pointing to 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-domains = <&gpc 1>;
+		power-domains = <&pd_pu>;
 		/* ... */
 	};
-
-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
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 1/4] dt-bindings: add multidomain support to i.MX GPC DT binding
@ 2017-01-20 15:52     ` Lucas Stach
  0 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

This adds a new binding for the Freescale i.MX GPC block, which allows
to describe multiple power domains in a more natural way. The driver
will continue to support the old binding for existing DTBs, but new
features like the additional domains present on i.MX6SX will only be
usable with the new binding.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../devicetree/bindings/power/fsl,imx-gpc.txt      | 81 ++++++++++++++--------
 1 file changed, 54 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
index 65cc0345747d..e5b660018d63 100644
--- a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
+++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
@@ -1,22 +1,41 @@
 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.
+The i.MX6 General Power Control (GPC) block contains DVFS load tracking
+counters and Power Gating Control (PGC).
 
 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:
+- interrupts: Should contain one interrupt specifier for the GPC interrupt
+- clocks: Must contain an entry for each entry in clock-names.
+  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - ipg
+- pgc: a list of child nodes describing the SoC power domains controlled by the
+  power gating controller.
 
-The gpc node is a power-controller as documented by the generic power domain
-bindings in Documentation/devicetree/bindings/power/power_domain.txt.
+The power domains are generic power domain providers as documented in
+Documentation/devicetree/bindings/power/power_domain.txt. They are described as
+subnodes of the GPC and should contain the following
+
+Required properties:
+- reg: the DOMAIN_INDEX as used by the client devices to refer to this
+  power domain
+  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
+
+- #power-domain-cells: Should be 0
+
+Optional properties:
+- clocks: a number of phandles to clocks that need to be enabled during domain
+  power-up sequencing to ensure reset propagation into devices located inside
+  this power domain
+- domain-supply: a phandle to the regulator powering this domain
 
 Example:
 
@@ -25,14 +44,29 @@ Example:
 		reg = <0x020dc000 0x4000>;
 		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
 			     <0 90 IRQ_TYPE_LEVEL_HIGH>;
-		pu-supply = <&reg_pu>;
-		clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
-			 <&clks IMX6QDL_CLK_GPU3D_SHADER>,
-			 <&clks IMX6QDL_CLK_GPU2D_CORE>,
-			 <&clks IMX6QDL_CLK_GPU2D_AXI>,
-			 <&clks IMX6QDL_CLK_OPENVG_AXI>,
-			 <&clks IMX6QDL_CLK_VPU_AXI>;
-		#power-domain-cells = <1>;
+		clocks = <&clks IMX6QDL_CLK_IPG>;
+		clock-names = "ipg";
+		
+		pgc {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			power-domain at 0 {
+				reg = <0>;
+				#power-domain-cells = <0>;
+			};
+			pd_pu: power-domain at 1 {
+				reg = <1>;
+				#power-domain-cells = <0>;
+				domain-supply = <&reg_pu>;
+				clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
+				         <&clks IMX6QDL_CLK_GPU3D_SHADER>,
+				         <&clks IMX6QDL_CLK_GPU2D_CORE>,
+				         <&clks IMX6QDL_CLK_GPU2D_AXI>,
+				         <&clks IMX6QDL_CLK_OPENVG_AXI>,
+				         <&clks IMX6QDL_CLK_VPU_AXI>;
+			};
+		};
 	};
 
 
@@ -40,20 +74,13 @@ Specifying power domain for IP modules
 ======================================
 
 IP cores belonging to a power domain should contain a 'power-domains' property
-that is a phandle pointing to the gpc device node and a DOMAIN_INDEX specifying
-the power domain the device belongs to.
+that is a phandle pointing to the power domain the device belongs to.
 
 Example of a device that is part of the PU power domain:
 
 	vpu: vpu at 02040000 {
 		reg = <0x02040000 0x3c000>;
 		/* ... */
-		power-domains = <&gpc 1>;
+		power-domains = <&pd_pu>;
 		/* ... */
 	};
-
-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
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/4] ARM: imx6: remove PGC handling from GPC driver
  2017-01-20 15:52 ` Lucas Stach
@ 2017-01-20 15:52     ` Lucas Stach
  -1 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ

This removes all functionality regarding the power gating controller
from the GPC driver. This will be added back in a re-written driver
outside of the architecture code.

This keeps only the IRQ controller code in the architecture, as this
is closely coupled to the CPU idle implementation.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/mach-imx/gpc.c | 217 ------------------------------------------------
 1 file changed, 217 deletions(-)

diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 1dc2a34b9dbd..93f584ba0130 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -10,26 +10,17 @@
  * 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/irqchip.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
-#include <linux/platform_device.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 GPC_PGC_CPU_PUPSCR	0x2a4
 #define GPC_PGC_CPU_PDNSCR	0x2a8
@@ -39,18 +30,6 @@
 #define IMR_NUM			4
 #define GPC_MAX_IRQS		(IMR_NUM * 32)
 
-#define GPU_VPU_PUP_REQ		BIT(1)
-#define GPU_VPU_PDN_REQ		BIT(0)
-
-#define GPC_CLK_MAX		6
-
-struct pu_domain {
-	struct generic_pm_domain base;
-	struct regulator *reg;
-	struct clk *clk[GPC_CLK_MAX];
-	int num_clks;
-};
-
 static void __iomem *gpc_base;
 static u32 gpc_wake_irqs[IMR_NUM];
 static u32 gpc_saved_imrs[IMR_NUM];
@@ -296,199 +275,3 @@ void __init imx_gpc_check_dt(void)
 		gpc_base = of_iomap(np, 0);
 	}
 }
-
-static void _imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
-{
-	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);
-}
-
-static int imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
-{
-	struct pu_domain *pu = container_of(genpd, struct pu_domain, base);
-
-	_imx6q_pm_pu_power_off(genpd);
-
-	if (pu->reg)
-		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;
-
-	if (pu->reg)
-		ret = regulator_enable(pu->reg);
-	if (pu->reg && 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,
-	},
-};
-
-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,
-	.num_domains = ARRAY_SIZE(imx_gpc_domains),
-};
-
-static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
-{
-	struct clk *clk;
-	int i, ret;
-
-	imx6q_pu_domain.reg = pu_reg;
-
-	for (i = 0; ; i++) {
-		clk = of_clk_get(dev->of_node, i);
-		if (IS_ERR(clk))
-			break;
-		if (i >= GPC_CLK_MAX) {
-			dev_err(dev, "more than %d clocks\n", GPC_CLK_MAX);
-			goto clk_err;
-		}
-		imx6q_pu_domain.clk[i] = clk;
-	}
-	imx6q_pu_domain.num_clks = i;
-
-	/* Enable power always in case bootloader disabled it. */
-	imx6q_pm_pu_power_on(&imx6q_pu_domain.base);
-
-	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
-		return 0;
-
-	imx6q_pu_domain.base.states = devm_kzalloc(dev,
-					sizeof(*imx6q_pu_domain.base.states),
-					GFP_KERNEL);
-	if (!imx6q_pu_domain.base.states)
-		return -ENOMEM;
-
-	imx6q_pu_domain.base.states[0].power_off_latency_ns = 25000;
-	imx6q_pu_domain.base.states[0].power_on_latency_ns = 2000000;
-	imx6q_pu_domain.base.state_count = 1;
-
-	for (i = 0; i < ARRAY_SIZE(imx_gpc_domains); i++)
-		pm_genpd_init(imx_gpc_domains[i], NULL, false);
-
-	ret =  of_genpd_add_provider_onecell(dev->of_node,
-					     &imx_gpc_onecell_data);
-	if (ret)
-		goto power_off;
-
-	return 0;
-
-power_off:
-	imx6q_pm_pu_power_off(&imx6q_pu_domain.base);
-clk_err:
-	while (i--)
-		clk_put(imx6q_pu_domain.clk[i]);
-	imx6q_pu_domain.reg = NULL;
-	return -EINVAL;
-}
-
-static int imx_gpc_probe(struct platform_device *pdev)
-{
-	struct regulator *pu_reg;
-	int ret;
-
-	/* bail out if DT too old and doesn't provide the necessary info */
-	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells"))
-		return 0;
-
-	pu_reg = devm_regulator_get_optional(&pdev->dev, "pu");
-	if (PTR_ERR(pu_reg) == -ENODEV)
-		pu_reg = NULL;
-	if (IS_ERR(pu_reg)) {
-		ret = PTR_ERR(pu_reg);
-		dev_err(&pdev->dev, "failed to get pu regulator: %d\n", ret);
-		return ret;
-	}
-
-	return imx_gpc_genpd_init(&pdev->dev, pu_reg);
-}
-
-static const 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",
-		.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);
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/4] ARM: imx6: remove PGC handling from GPC driver
@ 2017-01-20 15:52     ` Lucas Stach
  0 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

This removes all functionality regarding the power gating controller
from the GPC driver. This will be added back in a re-written driver
outside of the architecture code.

This keeps only the IRQ controller code in the architecture, as this
is closely coupled to the CPU idle implementation.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/mach-imx/gpc.c | 217 ------------------------------------------------
 1 file changed, 217 deletions(-)

diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 1dc2a34b9dbd..93f584ba0130 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -10,26 +10,17 @@
  * 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/irqchip.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
-#include <linux/platform_device.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 GPC_PGC_CPU_PUPSCR	0x2a4
 #define GPC_PGC_CPU_PDNSCR	0x2a8
@@ -39,18 +30,6 @@
 #define IMR_NUM			4
 #define GPC_MAX_IRQS		(IMR_NUM * 32)
 
-#define GPU_VPU_PUP_REQ		BIT(1)
-#define GPU_VPU_PDN_REQ		BIT(0)
-
-#define GPC_CLK_MAX		6
-
-struct pu_domain {
-	struct generic_pm_domain base;
-	struct regulator *reg;
-	struct clk *clk[GPC_CLK_MAX];
-	int num_clks;
-};
-
 static void __iomem *gpc_base;
 static u32 gpc_wake_irqs[IMR_NUM];
 static u32 gpc_saved_imrs[IMR_NUM];
@@ -296,199 +275,3 @@ void __init imx_gpc_check_dt(void)
 		gpc_base = of_iomap(np, 0);
 	}
 }
-
-static void _imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
-{
-	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);
-}
-
-static int imx6q_pm_pu_power_off(struct generic_pm_domain *genpd)
-{
-	struct pu_domain *pu = container_of(genpd, struct pu_domain, base);
-
-	_imx6q_pm_pu_power_off(genpd);
-
-	if (pu->reg)
-		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;
-
-	if (pu->reg)
-		ret = regulator_enable(pu->reg);
-	if (pu->reg && 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,
-	},
-};
-
-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,
-	.num_domains = ARRAY_SIZE(imx_gpc_domains),
-};
-
-static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg)
-{
-	struct clk *clk;
-	int i, ret;
-
-	imx6q_pu_domain.reg = pu_reg;
-
-	for (i = 0; ; i++) {
-		clk = of_clk_get(dev->of_node, i);
-		if (IS_ERR(clk))
-			break;
-		if (i >= GPC_CLK_MAX) {
-			dev_err(dev, "more than %d clocks\n", GPC_CLK_MAX);
-			goto clk_err;
-		}
-		imx6q_pu_domain.clk[i] = clk;
-	}
-	imx6q_pu_domain.num_clks = i;
-
-	/* Enable power always in case bootloader disabled it. */
-	imx6q_pm_pu_power_on(&imx6q_pu_domain.base);
-
-	if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS))
-		return 0;
-
-	imx6q_pu_domain.base.states = devm_kzalloc(dev,
-					sizeof(*imx6q_pu_domain.base.states),
-					GFP_KERNEL);
-	if (!imx6q_pu_domain.base.states)
-		return -ENOMEM;
-
-	imx6q_pu_domain.base.states[0].power_off_latency_ns = 25000;
-	imx6q_pu_domain.base.states[0].power_on_latency_ns = 2000000;
-	imx6q_pu_domain.base.state_count = 1;
-
-	for (i = 0; i < ARRAY_SIZE(imx_gpc_domains); i++)
-		pm_genpd_init(imx_gpc_domains[i], NULL, false);
-
-	ret =  of_genpd_add_provider_onecell(dev->of_node,
-					     &imx_gpc_onecell_data);
-	if (ret)
-		goto power_off;
-
-	return 0;
-
-power_off:
-	imx6q_pm_pu_power_off(&imx6q_pu_domain.base);
-clk_err:
-	while (i--)
-		clk_put(imx6q_pu_domain.clk[i]);
-	imx6q_pu_domain.reg = NULL;
-	return -EINVAL;
-}
-
-static int imx_gpc_probe(struct platform_device *pdev)
-{
-	struct regulator *pu_reg;
-	int ret;
-
-	/* bail out if DT too old and doesn't provide the necessary info */
-	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells"))
-		return 0;
-
-	pu_reg = devm_regulator_get_optional(&pdev->dev, "pu");
-	if (PTR_ERR(pu_reg) == -ENODEV)
-		pu_reg = NULL;
-	if (IS_ERR(pu_reg)) {
-		ret = PTR_ERR(pu_reg);
-		dev_err(&pdev->dev, "failed to get pu regulator: %d\n", ret);
-		return ret;
-	}
-
-	return imx_gpc_genpd_init(&pdev->dev, pu_reg);
-}
-
-static const 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",
-		.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);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 3/4] soc/imx: add new GPC driver
  2017-01-20 15:52 ` Lucas Stach
@ 2017-01-20 15:52     ` Lucas Stach
  -1 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ

This is an almost complete re-write of the previous GPC power gating control
code found in the IMX architecture code. It supports both the old and the new
DT binding, allowing more domains to be added later and generally makes the
driver easier to extend, while keeping compatibility with existing DTBs.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/soc/Makefile     |   1 +
 drivers/soc/imx/Makefile |   1 +
 drivers/soc/imx/gpc.c    | 480 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 482 insertions(+)
 create mode 100644 drivers/soc/imx/Makefile
 create mode 100644 drivers/soc/imx/gpc.c

diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 50c23d0bd457..42b3fc7687bb 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -6,6 +6,7 @@ obj-y				+= bcm/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
+obj-$(CONFIG_ARCH_MXC)		+= imx/
 obj-$(CONFIG_ARCH_MEDIATEK)	+= mediatek/
 obj-$(CONFIG_ARCH_QCOM)		+= qcom/
 obj-$(CONFIG_ARCH_RENESAS)	+= renesas/
diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
new file mode 100644
index 000000000000..35861f5b2802
--- /dev/null
+++ b/drivers/soc/imx/Makefile
@@ -0,0 +1 @@
+obj-y += gpc.o
diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c
new file mode 100644
index 000000000000..24bc0c6b72d8
--- /dev/null
+++ b/drivers/soc/imx/gpc.c
@@ -0,0 +1,480 @@
+/*
+ * Copyright 2015-2017 Pengutronix, Lucas Stach <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
+ * Copyright 2011-2013 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#define GPC_CNTR		0x000
+
+#define GPC_PGC_PDN_OFFS	0x0
+#define GPC_PGC_PUPSCR_OFFS	0x4
+#define GPC_PGC_PDNSCR_OFFS	0x8
+#define GPC_PGC_SW2ISO_SHIFT	0x8
+#define GPC_PGC_SW_SHIFT	0x0
+
+#define GPC_PGC_GPU_PDN		0x260
+#define GPC_PGC_GPU_PUPSCR	0x264
+#define GPC_PGC_GPU_PDNSCR	0x268
+
+#define GPU_VPU_PUP_REQ		BIT(1)
+#define GPU_VPU_PDN_REQ		BIT(0)
+
+#define GPC_CLK_MAX		6
+
+struct imx_pm_domain {
+	struct generic_pm_domain base;
+	struct regmap *regmap;
+	struct regulator *supply;
+	struct clk *clk[GPC_CLK_MAX];
+	int num_clks;
+	unsigned int reg_offs;
+	signed char cntr_pdn_bit;
+	unsigned int ipg_rate_mhz;
+};
+
+static inline struct imx_pm_domain *
+to_imx_pm_domain(struct generic_pm_domain *genpd)
+{
+	return container_of(genpd, struct imx_pm_domain, base);
+}
+
+static int imx6_pm_domain_power_off(struct generic_pm_domain *genpd)
+{
+	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
+	int iso, iso2sw;
+	u32 val;
+
+	/* Read ISO and ISO2SW power down delays */
+	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
+	iso = val & 0x3f;
+	iso2sw = (val >> 8) & 0x3f;
+
+	/* Gate off domain when powered down */
+	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
+			   0x1, 0x1);
+
+	/* Request GPC to power down domain */
+	val = BIT(pd->cntr_pdn_bit);
+	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
+
+	/* Wait ISO + ISO2SW IPG clock cycles */
+	udelay(DIV_ROUND_UP(iso + iso2sw, pd->ipg_rate_mhz));
+
+	if (pd->supply)
+		regulator_disable(pd->supply);
+
+	return 0;
+}
+
+static int imx6_pm_domain_power_on(struct generic_pm_domain *genpd)
+{
+	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
+	int i, ret, sw, sw2iso;
+	u32 val;
+
+	if (pd->supply) {
+		ret = regulator_enable(pd->supply);
+		if (ret) {
+			pr_err("%s: failed to enable regulator: %d\n",
+			       __func__, ret);
+			return ret;
+		}
+	}
+
+	/* Enable reset clocks for all devices in the domain */
+	for (i = 0; i < pd->num_clks; i++)
+		clk_prepare_enable(pd->clk[i]);
+
+	/* Gate off domain when powered down */
+	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
+			   0x1, 0x1);
+
+	/* Read ISO and ISO2SW power down delays */
+	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
+	sw = val & 0x3f;
+	sw2iso = (val >> 8) & 0x3f;
+
+	/* Request GPC to power up domain */
+	val = BIT(pd->cntr_pdn_bit + 1);
+	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
+
+	/* Wait ISO + ISO2SW IPG clock cycles */
+	udelay(DIV_ROUND_UP(sw + sw2iso, pd->ipg_rate_mhz));
+
+	/* Disable reset clocks for all devices in the domain */
+	for (i = 0; i < pd->num_clks; i++)
+		clk_disable_unprepare(pd->clk[i]);
+
+	return 0;
+}
+
+static int imx_pgc_get_clocks(struct device *dev, struct imx_pm_domain *domain)
+{
+	int i, ret;
+
+	for (i = 0; ; i++) {
+		struct clk *clk = of_clk_get(dev->of_node, i);
+		if (IS_ERR(clk))
+			break;
+		if (i >= GPC_CLK_MAX) {
+			dev_err(dev, "more than %d clocks\n", GPC_CLK_MAX);
+			ret = -EINVAL;
+			goto clk_err;
+		}
+		domain->clk[i] = clk;
+	}
+	domain->num_clks = i;
+
+	return 0;
+
+clk_err:
+	for (; i >= 0; i--)
+		clk_put(domain->clk[i]);
+
+	return ret;
+}
+
+static void imx_pgc_put_clocks(struct imx_pm_domain *domain)
+{
+	int i;
+
+	for (i = domain->num_clks - 1; i >= 0; i--)
+		clk_put(domain->clk[i]);
+}
+
+static int imx_pgc_parse_dt(struct device *dev, struct imx_pm_domain *domain)
+{
+	/* try to get the domain supply regulator */
+	domain->supply = devm_regulator_get_optional(dev, "domain");
+	if (IS_ERR(domain->supply)) {
+		if (PTR_ERR(domain->supply) == -ENODEV)
+			domain->supply = NULL;
+		else
+			return PTR_ERR(domain->supply);
+	}
+
+	/* try to get all clocks needed for reset propagation */
+	return imx_pgc_get_clocks(dev, domain);
+}
+
+static int imx_pgc_power_domain_probe(struct platform_device *pdev)
+{
+	struct imx_pm_domain *domain = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	/* if this PD is associated with a DT node try to parse it */
+	if (dev->of_node) {
+		ret = imx_pgc_parse_dt(dev, domain);
+		if (ret)
+			return ret;
+	}
+
+	/* initially power on the domain */
+	if (domain->base.power_on)
+		domain->base.power_on(&domain->base);
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		pm_genpd_init(&domain->base, NULL, false);
+		ret = of_genpd_add_provider_simple(dev->of_node, &domain->base);
+		if (ret)
+			goto genpd_err;
+	}
+
+	return 0;
+
+genpd_err:
+	pm_genpd_remove(&domain->base);
+	imx_pgc_put_clocks(domain);
+
+	return ret;
+}
+
+static int imx_pgc_power_domain_remove(struct platform_device *pdev)
+{
+	struct imx_pm_domain *domain = pdev->dev.platform_data;
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		of_genpd_del_provider(pdev->dev.of_node);
+		pm_genpd_remove(&domain->base);
+		imx_pgc_put_clocks(domain);
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id imx_pgc_power_domain_id[] = {
+	{ "imx-pgc-power-domain"},
+	{ },
+};
+
+static struct platform_driver imx_pgc_power_domain_driver = {
+	.driver = {
+		.name = "imx-pgc-pd",
+	},
+	.probe = imx_pgc_power_domain_probe,
+	.remove = imx_pgc_power_domain_remove,
+	.id_table = imx_pgc_power_domain_id,
+};
+builtin_platform_driver(imx_pgc_power_domain_driver)
+
+static struct genpd_power_state imx6_pm_domain_pu_state = {
+	.power_off_latency_ns = 25000,
+	.power_on_latency_ns = 2000000,
+};
+
+static struct imx_pm_domain imx_gpc_domains[] = {
+	{
+		.base = {
+			.name = "ARM",
+		},
+	}, {
+		.base = {
+			.name = "PU",
+			.power_off = imx6_pm_domain_power_off,
+			.power_on = imx6_pm_domain_power_on,
+			.states = &imx6_pm_domain_pu_state,
+			.state_count = 1,
+		},
+		.reg_offs = 0x260,
+		.cntr_pdn_bit = 0,
+	}, {
+		.base = {
+			.name = "DISPLAY",
+			.power_off = imx6_pm_domain_power_off,
+			.power_on = imx6_pm_domain_power_on,
+		},
+		.reg_offs = 0x240,
+		.cntr_pdn_bit = 4,
+	}
+};
+
+struct imx_gpc_dt_data {
+	int num_domains;
+};
+
+static const struct imx_gpc_dt_data imx6q_dt_data = {
+	.num_domains = 2,
+};
+
+static const struct imx_gpc_dt_data imx6sl_dt_data = {
+	.num_domains = 3,
+};
+
+static const struct of_device_id imx_gpc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
+	{ .compatible = "fsl,imx6sl-gpc", .data = &imx6sl_dt_data },
+	{ }
+};
+
+static bool imx_gpc_readable_reg(struct device *dev, unsigned int reg)
+{
+	return (reg % 4 == 0) && (reg <= 0x2ac);
+}
+
+static bool imx_gpc_volatile_reg(struct device *dev, unsigned int reg)
+{
+	if (reg == GPC_CNTR)
+		return true;
+
+	return false;
+}
+
+static const struct regmap_config imx_gpc_regmap_config = {
+	.cache_type = REGCACHE_FLAT,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+
+	.readable_reg = imx_gpc_readable_reg,
+	.volatile_reg = imx_gpc_volatile_reg,
+
+	.max_register = 0x2ac,
+};
+
+static struct generic_pm_domain *imx_gpc_onecell_domains[] = {
+	&imx_gpc_domains[0].base,
+	&imx_gpc_domains[1].base,
+};
+
+static struct genpd_onecell_data imx_gpc_onecell_data = {
+	.domains = imx_gpc_onecell_domains,
+	.num_domains = 2,
+};
+
+static int imx_gpc_old_dt_init(struct device *dev, struct regmap *regmap)
+{
+	struct imx_pm_domain *domain;
+	int i, ret;
+
+	for (i = 0; i < 2; i++) {
+		domain = &imx_gpc_domains[i];
+		domain->regmap = regmap;
+		domain->ipg_rate_mhz = 33;
+
+		if (i == 1) {
+			domain->supply = devm_regulator_get(dev, "pu");
+			if (IS_ERR(domain->supply))
+				return PTR_ERR(domain->supply);;
+
+			ret = imx_pgc_get_clocks(dev, domain);
+			if (ret)
+				goto clk_err;
+
+			domain->base.power_on(&domain->base);
+		}
+	}
+
+	for (i = 0; i < 2; i++)
+		pm_genpd_init(&imx_gpc_domains[i].base, NULL, false);
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		ret = of_genpd_add_provider_onecell(dev->of_node,
+						    &imx_gpc_onecell_data);
+		if (ret)
+			goto genpd_err;
+	}
+
+	return 0;
+
+genpd_err:
+	pm_genpd_remove(&imx_gpc_domains[1].base);
+	imx_pgc_put_clocks(&imx_gpc_domains[1]);
+clk_err:
+	pm_genpd_remove(&imx_gpc_domains[0].base);
+
+	return ret;
+}
+
+static int imx_gpc_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *of_id =
+			of_match_device(imx_gpc_dt_ids, &pdev->dev);
+	const struct imx_gpc_dt_data *of_id_data = of_id->data;
+	struct device_node *pgc_node;
+	struct regmap *regmap;
+	struct resource *res;
+	void __iomem *base;
+	int ret;
+
+	pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc");
+
+	/* bail out if DT too old and doesn't provide the necessary info */
+	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells") &&
+	    !pgc_node)
+		return 0;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
+					   &imx_gpc_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(&pdev->dev, "failed to init regmap: %d\n",
+			ret);
+		return ret;
+	}
+
+	if (!pgc_node) {
+		/* old DT layout is only supported for mx6q aka 2 domains */
+		if (of_id_data->num_domains != 2) {
+			dev_err(&pdev->dev, "could not find pgc DT node\n");
+			return -ENODEV;
+		}
+
+		ret = imx_gpc_old_dt_init(&pdev->dev, regmap);
+		if (ret)
+			return ret;
+	} else {
+		struct imx_pm_domain *domain;
+		struct platform_device *pd_pdev;
+		struct device_node *np;
+		struct clk *ipg_clk;
+		unsigned int ipg_rate_mhz;
+		int domain_index;
+
+		ipg_clk = devm_clk_get(&pdev->dev, "ipg");
+		if (IS_ERR(ipg_clk))
+			return PTR_ERR(ipg_clk);
+		ipg_rate_mhz = clk_get_rate(ipg_clk) / 1000000;
+
+		for_each_child_of_node(pgc_node, np) {
+			ret = of_property_read_u32(np, "reg", &domain_index);
+			if (ret) {
+				of_node_put(np);
+				return ret;
+			}
+
+			domain = &imx_gpc_domains[domain_index];
+			domain->regmap = regmap;
+			domain->ipg_rate_mhz = ipg_rate_mhz;
+
+			pd_pdev = platform_device_alloc("imx-pgc-power-domain",
+							domain_index);
+			pd_pdev->dev.platform_data = domain;
+			pd_pdev->dev.parent = &pdev->dev;
+			pd_pdev->dev.of_node = np;
+
+			ret = platform_device_add(pd_pdev);
+			if (ret) {
+				platform_device_put(pd_pdev);
+				of_node_put(np);
+				return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int imx_gpc_remove(struct platform_device *pdev)
+{
+	int ret;
+
+	/*
+	 * If the old DT binding is used the toplevel driver needs to
+	 * de-register the power domains
+	 */
+	if (!of_get_child_by_name(pdev->dev.of_node, "pgc")) {
+		of_genpd_del_provider(pdev->dev.of_node);
+
+		ret = pm_genpd_remove(&imx_gpc_domains[1].base);
+		if (ret)
+			return ret;
+		imx_pgc_put_clocks(&imx_gpc_domains[1]);
+
+		ret = pm_genpd_remove(&imx_gpc_domains[0].base);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver imx_gpc_driver = {
+	.driver = {
+		.name = "imx-gpc",
+		.of_match_table = imx_gpc_dt_ids,
+	},
+	.probe = imx_gpc_probe,
+	.remove = imx_gpc_remove,
+};
+builtin_platform_driver(imx_gpc_driver)
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 3/4] soc/imx: add new GPC driver
@ 2017-01-20 15:52     ` Lucas Stach
  0 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

This is an almost complete re-write of the previous GPC power gating control
code found in the IMX architecture code. It supports both the old and the new
DT binding, allowing more domains to be added later and generally makes the
driver easier to extend, while keeping compatibility with existing DTBs.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/Makefile     |   1 +
 drivers/soc/imx/Makefile |   1 +
 drivers/soc/imx/gpc.c    | 480 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 482 insertions(+)
 create mode 100644 drivers/soc/imx/Makefile
 create mode 100644 drivers/soc/imx/gpc.c

diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 50c23d0bd457..42b3fc7687bb 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -6,6 +6,7 @@ obj-y				+= bcm/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
+obj-$(CONFIG_ARCH_MXC)		+= imx/
 obj-$(CONFIG_ARCH_MEDIATEK)	+= mediatek/
 obj-$(CONFIG_ARCH_QCOM)		+= qcom/
 obj-$(CONFIG_ARCH_RENESAS)	+= renesas/
diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
new file mode 100644
index 000000000000..35861f5b2802
--- /dev/null
+++ b/drivers/soc/imx/Makefile
@@ -0,0 +1 @@
+obj-y += gpc.o
diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c
new file mode 100644
index 000000000000..24bc0c6b72d8
--- /dev/null
+++ b/drivers/soc/imx/gpc.c
@@ -0,0 +1,480 @@
+/*
+ * Copyright 2015-2017 Pengutronix, Lucas Stach <kernel@pengutronix.de>
+ * Copyright 2011-2013 Freescale Semiconductor, Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#define GPC_CNTR		0x000
+
+#define GPC_PGC_PDN_OFFS	0x0
+#define GPC_PGC_PUPSCR_OFFS	0x4
+#define GPC_PGC_PDNSCR_OFFS	0x8
+#define GPC_PGC_SW2ISO_SHIFT	0x8
+#define GPC_PGC_SW_SHIFT	0x0
+
+#define GPC_PGC_GPU_PDN		0x260
+#define GPC_PGC_GPU_PUPSCR	0x264
+#define GPC_PGC_GPU_PDNSCR	0x268
+
+#define GPU_VPU_PUP_REQ		BIT(1)
+#define GPU_VPU_PDN_REQ		BIT(0)
+
+#define GPC_CLK_MAX		6
+
+struct imx_pm_domain {
+	struct generic_pm_domain base;
+	struct regmap *regmap;
+	struct regulator *supply;
+	struct clk *clk[GPC_CLK_MAX];
+	int num_clks;
+	unsigned int reg_offs;
+	signed char cntr_pdn_bit;
+	unsigned int ipg_rate_mhz;
+};
+
+static inline struct imx_pm_domain *
+to_imx_pm_domain(struct generic_pm_domain *genpd)
+{
+	return container_of(genpd, struct imx_pm_domain, base);
+}
+
+static int imx6_pm_domain_power_off(struct generic_pm_domain *genpd)
+{
+	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
+	int iso, iso2sw;
+	u32 val;
+
+	/* Read ISO and ISO2SW power down delays */
+	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
+	iso = val & 0x3f;
+	iso2sw = (val >> 8) & 0x3f;
+
+	/* Gate off domain when powered down */
+	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
+			   0x1, 0x1);
+
+	/* Request GPC to power down domain */
+	val = BIT(pd->cntr_pdn_bit);
+	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
+
+	/* Wait ISO + ISO2SW IPG clock cycles */
+	udelay(DIV_ROUND_UP(iso + iso2sw, pd->ipg_rate_mhz));
+
+	if (pd->supply)
+		regulator_disable(pd->supply);
+
+	return 0;
+}
+
+static int imx6_pm_domain_power_on(struct generic_pm_domain *genpd)
+{
+	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
+	int i, ret, sw, sw2iso;
+	u32 val;
+
+	if (pd->supply) {
+		ret = regulator_enable(pd->supply);
+		if (ret) {
+			pr_err("%s: failed to enable regulator: %d\n",
+			       __func__, ret);
+			return ret;
+		}
+	}
+
+	/* Enable reset clocks for all devices in the domain */
+	for (i = 0; i < pd->num_clks; i++)
+		clk_prepare_enable(pd->clk[i]);
+
+	/* Gate off domain when powered down */
+	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
+			   0x1, 0x1);
+
+	/* Read ISO and ISO2SW power down delays */
+	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
+	sw = val & 0x3f;
+	sw2iso = (val >> 8) & 0x3f;
+
+	/* Request GPC to power up domain */
+	val = BIT(pd->cntr_pdn_bit + 1);
+	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
+
+	/* Wait ISO + ISO2SW IPG clock cycles */
+	udelay(DIV_ROUND_UP(sw + sw2iso, pd->ipg_rate_mhz));
+
+	/* Disable reset clocks for all devices in the domain */
+	for (i = 0; i < pd->num_clks; i++)
+		clk_disable_unprepare(pd->clk[i]);
+
+	return 0;
+}
+
+static int imx_pgc_get_clocks(struct device *dev, struct imx_pm_domain *domain)
+{
+	int i, ret;
+
+	for (i = 0; ; i++) {
+		struct clk *clk = of_clk_get(dev->of_node, i);
+		if (IS_ERR(clk))
+			break;
+		if (i >= GPC_CLK_MAX) {
+			dev_err(dev, "more than %d clocks\n", GPC_CLK_MAX);
+			ret = -EINVAL;
+			goto clk_err;
+		}
+		domain->clk[i] = clk;
+	}
+	domain->num_clks = i;
+
+	return 0;
+
+clk_err:
+	for (; i >= 0; i--)
+		clk_put(domain->clk[i]);
+
+	return ret;
+}
+
+static void imx_pgc_put_clocks(struct imx_pm_domain *domain)
+{
+	int i;
+
+	for (i = domain->num_clks - 1; i >= 0; i--)
+		clk_put(domain->clk[i]);
+}
+
+static int imx_pgc_parse_dt(struct device *dev, struct imx_pm_domain *domain)
+{
+	/* try to get the domain supply regulator */
+	domain->supply = devm_regulator_get_optional(dev, "domain");
+	if (IS_ERR(domain->supply)) {
+		if (PTR_ERR(domain->supply) == -ENODEV)
+			domain->supply = NULL;
+		else
+			return PTR_ERR(domain->supply);
+	}
+
+	/* try to get all clocks needed for reset propagation */
+	return imx_pgc_get_clocks(dev, domain);
+}
+
+static int imx_pgc_power_domain_probe(struct platform_device *pdev)
+{
+	struct imx_pm_domain *domain = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	/* if this PD is associated with a DT node try to parse it */
+	if (dev->of_node) {
+		ret = imx_pgc_parse_dt(dev, domain);
+		if (ret)
+			return ret;
+	}
+
+	/* initially power on the domain */
+	if (domain->base.power_on)
+		domain->base.power_on(&domain->base);
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		pm_genpd_init(&domain->base, NULL, false);
+		ret = of_genpd_add_provider_simple(dev->of_node, &domain->base);
+		if (ret)
+			goto genpd_err;
+	}
+
+	return 0;
+
+genpd_err:
+	pm_genpd_remove(&domain->base);
+	imx_pgc_put_clocks(domain);
+
+	return ret;
+}
+
+static int imx_pgc_power_domain_remove(struct platform_device *pdev)
+{
+	struct imx_pm_domain *domain = pdev->dev.platform_data;
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		of_genpd_del_provider(pdev->dev.of_node);
+		pm_genpd_remove(&domain->base);
+		imx_pgc_put_clocks(domain);
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id imx_pgc_power_domain_id[] = {
+	{ "imx-pgc-power-domain"},
+	{ },
+};
+
+static struct platform_driver imx_pgc_power_domain_driver = {
+	.driver = {
+		.name = "imx-pgc-pd",
+	},
+	.probe = imx_pgc_power_domain_probe,
+	.remove = imx_pgc_power_domain_remove,
+	.id_table = imx_pgc_power_domain_id,
+};
+builtin_platform_driver(imx_pgc_power_domain_driver)
+
+static struct genpd_power_state imx6_pm_domain_pu_state = {
+	.power_off_latency_ns = 25000,
+	.power_on_latency_ns = 2000000,
+};
+
+static struct imx_pm_domain imx_gpc_domains[] = {
+	{
+		.base = {
+			.name = "ARM",
+		},
+	}, {
+		.base = {
+			.name = "PU",
+			.power_off = imx6_pm_domain_power_off,
+			.power_on = imx6_pm_domain_power_on,
+			.states = &imx6_pm_domain_pu_state,
+			.state_count = 1,
+		},
+		.reg_offs = 0x260,
+		.cntr_pdn_bit = 0,
+	}, {
+		.base = {
+			.name = "DISPLAY",
+			.power_off = imx6_pm_domain_power_off,
+			.power_on = imx6_pm_domain_power_on,
+		},
+		.reg_offs = 0x240,
+		.cntr_pdn_bit = 4,
+	}
+};
+
+struct imx_gpc_dt_data {
+	int num_domains;
+};
+
+static const struct imx_gpc_dt_data imx6q_dt_data = {
+	.num_domains = 2,
+};
+
+static const struct imx_gpc_dt_data imx6sl_dt_data = {
+	.num_domains = 3,
+};
+
+static const struct of_device_id imx_gpc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
+	{ .compatible = "fsl,imx6sl-gpc", .data = &imx6sl_dt_data },
+	{ }
+};
+
+static bool imx_gpc_readable_reg(struct device *dev, unsigned int reg)
+{
+	return (reg % 4 == 0) && (reg <= 0x2ac);
+}
+
+static bool imx_gpc_volatile_reg(struct device *dev, unsigned int reg)
+{
+	if (reg == GPC_CNTR)
+		return true;
+
+	return false;
+}
+
+static const struct regmap_config imx_gpc_regmap_config = {
+	.cache_type = REGCACHE_FLAT,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+
+	.readable_reg = imx_gpc_readable_reg,
+	.volatile_reg = imx_gpc_volatile_reg,
+
+	.max_register = 0x2ac,
+};
+
+static struct generic_pm_domain *imx_gpc_onecell_domains[] = {
+	&imx_gpc_domains[0].base,
+	&imx_gpc_domains[1].base,
+};
+
+static struct genpd_onecell_data imx_gpc_onecell_data = {
+	.domains = imx_gpc_onecell_domains,
+	.num_domains = 2,
+};
+
+static int imx_gpc_old_dt_init(struct device *dev, struct regmap *regmap)
+{
+	struct imx_pm_domain *domain;
+	int i, ret;
+
+	for (i = 0; i < 2; i++) {
+		domain = &imx_gpc_domains[i];
+		domain->regmap = regmap;
+		domain->ipg_rate_mhz = 33;
+
+		if (i == 1) {
+			domain->supply = devm_regulator_get(dev, "pu");
+			if (IS_ERR(domain->supply))
+				return PTR_ERR(domain->supply);;
+
+			ret = imx_pgc_get_clocks(dev, domain);
+			if (ret)
+				goto clk_err;
+
+			domain->base.power_on(&domain->base);
+		}
+	}
+
+	for (i = 0; i < 2; i++)
+		pm_genpd_init(&imx_gpc_domains[i].base, NULL, false);
+
+	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
+		ret = of_genpd_add_provider_onecell(dev->of_node,
+						    &imx_gpc_onecell_data);
+		if (ret)
+			goto genpd_err;
+	}
+
+	return 0;
+
+genpd_err:
+	pm_genpd_remove(&imx_gpc_domains[1].base);
+	imx_pgc_put_clocks(&imx_gpc_domains[1]);
+clk_err:
+	pm_genpd_remove(&imx_gpc_domains[0].base);
+
+	return ret;
+}
+
+static int imx_gpc_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *of_id =
+			of_match_device(imx_gpc_dt_ids, &pdev->dev);
+	const struct imx_gpc_dt_data *of_id_data = of_id->data;
+	struct device_node *pgc_node;
+	struct regmap *regmap;
+	struct resource *res;
+	void __iomem *base;
+	int ret;
+
+	pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc");
+
+	/* bail out if DT too old and doesn't provide the necessary info */
+	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells") &&
+	    !pgc_node)
+		return 0;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
+					   &imx_gpc_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(&pdev->dev, "failed to init regmap: %d\n",
+			ret);
+		return ret;
+	}
+
+	if (!pgc_node) {
+		/* old DT layout is only supported for mx6q aka 2 domains */
+		if (of_id_data->num_domains != 2) {
+			dev_err(&pdev->dev, "could not find pgc DT node\n");
+			return -ENODEV;
+		}
+
+		ret = imx_gpc_old_dt_init(&pdev->dev, regmap);
+		if (ret)
+			return ret;
+	} else {
+		struct imx_pm_domain *domain;
+		struct platform_device *pd_pdev;
+		struct device_node *np;
+		struct clk *ipg_clk;
+		unsigned int ipg_rate_mhz;
+		int domain_index;
+
+		ipg_clk = devm_clk_get(&pdev->dev, "ipg");
+		if (IS_ERR(ipg_clk))
+			return PTR_ERR(ipg_clk);
+		ipg_rate_mhz = clk_get_rate(ipg_clk) / 1000000;
+
+		for_each_child_of_node(pgc_node, np) {
+			ret = of_property_read_u32(np, "reg", &domain_index);
+			if (ret) {
+				of_node_put(np);
+				return ret;
+			}
+
+			domain = &imx_gpc_domains[domain_index];
+			domain->regmap = regmap;
+			domain->ipg_rate_mhz = ipg_rate_mhz;
+
+			pd_pdev = platform_device_alloc("imx-pgc-power-domain",
+							domain_index);
+			pd_pdev->dev.platform_data = domain;
+			pd_pdev->dev.parent = &pdev->dev;
+			pd_pdev->dev.of_node = np;
+
+			ret = platform_device_add(pd_pdev);
+			if (ret) {
+				platform_device_put(pd_pdev);
+				of_node_put(np);
+				return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int imx_gpc_remove(struct platform_device *pdev)
+{
+	int ret;
+
+	/*
+	 * If the old DT binding is used the toplevel driver needs to
+	 * de-register the power domains
+	 */
+	if (!of_get_child_by_name(pdev->dev.of_node, "pgc")) {
+		of_genpd_del_provider(pdev->dev.of_node);
+
+		ret = pm_genpd_remove(&imx_gpc_domains[1].base);
+		if (ret)
+			return ret;
+		imx_pgc_put_clocks(&imx_gpc_domains[1]);
+
+		ret = pm_genpd_remove(&imx_gpc_domains[0].base);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver imx_gpc_driver = {
+	.driver = {
+		.name = "imx-gpc",
+		.of_match_table = imx_gpc_dt_ids,
+	},
+	.probe = imx_gpc_probe,
+	.remove = imx_gpc_remove,
+};
+builtin_platform_driver(imx_gpc_driver)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 4/4] ARM: imx6: adopt DT to new GPC binding
  2017-01-20 15:52 ` Lucas Stach
@ 2017-01-20 15:52     ` Lucas Stach
  -1 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ

Adopt the i.MX6Q/DL DT to the new and more flexible GPC binding.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/imx6q.dtsi   |  2 +-
 arch/arm/boot/dts/imx6qdl.dtsi | 38 +++++++++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index e9a5d0b8c7b0..dd33849335b2 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -125,7 +125,7 @@
 			clocks = <&clks IMX6QDL_CLK_OPENVG_AXI>,
 				 <&clks IMX6QDL_CLK_GPU2D_CORE>;
 			clock-names = "bus", "core";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		ipu2: ipu@02800000 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 53e6e63cbb02..ce7069be5852 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1,3 +1,4 @@
+
 /*
  * Copyright 2011 Freescale Semiconductor, Inc.
  * Copyright 2011 Linaro Ltd.
@@ -148,7 +149,7 @@
 				 <&clks IMX6QDL_CLK_GPU3D_CORE>,
 				 <&clks IMX6QDL_CLK_GPU3D_SHADER>;
 			clock-names = "bus", "core", "shader";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		gpu_2d: gpu@00134000 {
@@ -158,7 +159,7 @@
 			clocks = <&clks IMX6QDL_CLK_GPU2D_AXI>,
 				 <&clks IMX6QDL_CLK_GPU2D_CORE>;
 			clock-names = "bus", "core";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		timer@00a00600 {
@@ -425,7 +426,7 @@
 				clocks = <&clks IMX6QDL_CLK_VPU_AXI>,
 					 <&clks IMX6QDL_CLK_MMDC_CH0_AXI>;
 				clock-names = "per", "ahb";
-				power-domains = <&gpc 1>;
+				power-domains = <&pd_pu>;
 				resets = <&src 1>;
 				iram = <&ocram>;
 			};
@@ -788,14 +789,29 @@
 				interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 90 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-parent = <&intc>;
-				pu-supply = <&reg_pu>;
-				clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
-					 <&clks IMX6QDL_CLK_GPU3D_SHADER>,
-					 <&clks IMX6QDL_CLK_GPU2D_CORE>,
-					 <&clks IMX6QDL_CLK_GPU2D_AXI>,
-					 <&clks IMX6QDL_CLK_OPENVG_AXI>,
-					 <&clks IMX6QDL_CLK_VPU_AXI>;
-				#power-domain-cells = <1>;
+				clocks = <&clks IMX6QDL_CLK_IPG>;
+				clock-names = "ipg";
+
+				pgc {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					power-domain@0 {
+						reg = <0>;
+						#power-domain-cells = <0>;
+					};
+					pd_pu: power-domain@1 {
+						reg = <1>;
+						#power-domain-cells = <0>;
+						domain-supply = <&reg_pu>;
+						clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
+						         <&clks IMX6QDL_CLK_GPU3D_SHADER>,
+						         <&clks IMX6QDL_CLK_GPU2D_CORE>,
+						         <&clks IMX6QDL_CLK_GPU2D_AXI>,
+						         <&clks IMX6QDL_CLK_OPENVG_AXI>,
+						         <&clks IMX6QDL_CLK_VPU_AXI>;
+					};
+				};
 			};
 
 			gpr: iomuxc-gpr@020e0000 {
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 4/4] ARM: imx6: adopt DT to new GPC binding
@ 2017-01-20 15:52     ` Lucas Stach
  0 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-20 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

Adopt the i.MX6Q/DL DT to the new and more flexible GPC binding.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/boot/dts/imx6q.dtsi   |  2 +-
 arch/arm/boot/dts/imx6qdl.dtsi | 38 +++++++++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index e9a5d0b8c7b0..dd33849335b2 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -125,7 +125,7 @@
 			clocks = <&clks IMX6QDL_CLK_OPENVG_AXI>,
 				 <&clks IMX6QDL_CLK_GPU2D_CORE>;
 			clock-names = "bus", "core";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		ipu2: ipu at 02800000 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 53e6e63cbb02..ce7069be5852 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1,3 +1,4 @@
+
 /*
  * Copyright 2011 Freescale Semiconductor, Inc.
  * Copyright 2011 Linaro Ltd.
@@ -148,7 +149,7 @@
 				 <&clks IMX6QDL_CLK_GPU3D_CORE>,
 				 <&clks IMX6QDL_CLK_GPU3D_SHADER>;
 			clock-names = "bus", "core", "shader";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		gpu_2d: gpu at 00134000 {
@@ -158,7 +159,7 @@
 			clocks = <&clks IMX6QDL_CLK_GPU2D_AXI>,
 				 <&clks IMX6QDL_CLK_GPU2D_CORE>;
 			clock-names = "bus", "core";
-			power-domains = <&gpc 1>;
+			power-domains = <&pd_pu>;
 		};
 
 		timer at 00a00600 {
@@ -425,7 +426,7 @@
 				clocks = <&clks IMX6QDL_CLK_VPU_AXI>,
 					 <&clks IMX6QDL_CLK_MMDC_CH0_AXI>;
 				clock-names = "per", "ahb";
-				power-domains = <&gpc 1>;
+				power-domains = <&pd_pu>;
 				resets = <&src 1>;
 				iram = <&ocram>;
 			};
@@ -788,14 +789,29 @@
 				interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 90 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-parent = <&intc>;
-				pu-supply = <&reg_pu>;
-				clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
-					 <&clks IMX6QDL_CLK_GPU3D_SHADER>,
-					 <&clks IMX6QDL_CLK_GPU2D_CORE>,
-					 <&clks IMX6QDL_CLK_GPU2D_AXI>,
-					 <&clks IMX6QDL_CLK_OPENVG_AXI>,
-					 <&clks IMX6QDL_CLK_VPU_AXI>;
-				#power-domain-cells = <1>;
+				clocks = <&clks IMX6QDL_CLK_IPG>;
+				clock-names = "ipg";
+
+				pgc {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					power-domain at 0 {
+						reg = <0>;
+						#power-domain-cells = <0>;
+					};
+					pd_pu: power-domain at 1 {
+						reg = <1>;
+						#power-domain-cells = <0>;
+						domain-supply = <&reg_pu>;
+						clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
+						         <&clks IMX6QDL_CLK_GPU3D_SHADER>,
+						         <&clks IMX6QDL_CLK_GPU2D_CORE>,
+						         <&clks IMX6QDL_CLK_GPU2D_AXI>,
+						         <&clks IMX6QDL_CLK_OPENVG_AXI>,
+						         <&clks IMX6QDL_CLK_VPU_AXI>;
+					};
+				};
 			};
 
 			gpr: iomuxc-gpr at 020e0000 {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] dt-bindings: add multidomain support to i.MX GPC DT binding
  2017-01-20 15:52     ` Lucas Stach
@ 2017-01-23 16:26       ` Rob Herring
  -1 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2017-01-23 16:26 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Mark Rutland, devicetree, patchwork-lst, kernel, Fabio Estevam,
	Shawn Guo, linux-arm-kernel

On Fri, Jan 20, 2017 at 04:52:22PM +0100, Lucas Stach wrote:
> This adds a new binding for the Freescale i.MX GPC block, which allows
> to describe multiple power domains in a more natural way. The driver
> will continue to support the old binding for existing DTBs, but new
> features like the additional domains present on i.MX6SX will only be
> usable with the new binding.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../devicetree/bindings/power/fsl,imx-gpc.txt      | 81 ++++++++++++++--------
>  1 file changed, 54 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> index 65cc0345747d..e5b660018d63 100644
> --- a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> +++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> @@ -1,22 +1,41 @@
>  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.
> +The i.MX6 General Power Control (GPC) block contains DVFS load tracking
> +counters and Power Gating Control (PGC).
>  
>  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:
> +- interrupts: Should contain one interrupt specifier for the GPC interrupt
> +- clocks: Must contain an entry for each entry in clock-names.
> +  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +  - ipg
> +- pgc: a list of child nodes describing the SoC power domains controlled by the
> +  power gating controller.

pgc is a node, not a property. Call it 'power-domains' instead.

>  
> -The gpc node is a power-controller as documented by the generic power domain
> -bindings in Documentation/devicetree/bindings/power/power_domain.txt.
> +The power domains are generic power domain providers as documented in
> +Documentation/devicetree/bindings/power/power_domain.txt. They are described as
> +subnodes of the GPC and should contain the following
> +
> +Required properties:
> +- reg: the DOMAIN_INDEX as used by the client devices to refer to this
> +  power domain
> +  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
> +
> +- #power-domain-cells: Should be 0
> +
> +Optional properties:
> +- clocks: a number of phandles to clocks that need to be enabled during domain
> +  power-up sequencing to ensure reset propagation into devices located inside
> +  this power domain
> +- domain-supply: a phandle to the regulator powering this domain

'power-supply' is more common for when we don't really have a name.

>  Example:
>  
> @@ -25,14 +44,29 @@ Example:
>  		reg = <0x020dc000 0x4000>;
>  		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
>  			     <0 90 IRQ_TYPE_LEVEL_HIGH>;
> -		pu-supply = <&reg_pu>;
> -		clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
> -			 <&clks IMX6QDL_CLK_GPU3D_SHADER>,
> -			 <&clks IMX6QDL_CLK_GPU2D_CORE>,
> -			 <&clks IMX6QDL_CLK_GPU2D_AXI>,
> -			 <&clks IMX6QDL_CLK_OPENVG_AXI>,
> -			 <&clks IMX6QDL_CLK_VPU_AXI>;
> -		#power-domain-cells = <1>;
> +		clocks = <&clks IMX6QDL_CLK_IPG>;
> +		clock-names = "ipg";
> +		
> +		pgc {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			power-domain@0 {
> +				reg = <0>;
> +				#power-domain-cells = <0>;
> +			};
> +			pd_pu: power-domain@1 {
> +				reg = <1>;
> +				#power-domain-cells = <0>;
> +				domain-supply = <&reg_pu>;
> +				clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
> +				         <&clks IMX6QDL_CLK_GPU3D_SHADER>,
> +				         <&clks IMX6QDL_CLK_GPU2D_CORE>,
> +				         <&clks IMX6QDL_CLK_GPU2D_AXI>,
> +				         <&clks IMX6QDL_CLK_OPENVG_AXI>,
> +				         <&clks IMX6QDL_CLK_VPU_AXI>;
> +			};
> +		};
>  	};
>  
>  
> @@ -40,20 +74,13 @@ Specifying power domain for IP modules
>  ======================================
>  
>  IP cores belonging to a power domain should contain a 'power-domains' property
> -that is a phandle pointing to the gpc device node and a DOMAIN_INDEX specifying
> -the power domain the device belongs to.
> +that is a phandle pointing to 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-domains = <&gpc 1>;
> +		power-domains = <&pd_pu>;
>  		/* ... */
>  	};
> -
> -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
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 1/4] dt-bindings: add multidomain support to i.MX GPC DT binding
@ 2017-01-23 16:26       ` Rob Herring
  0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2017-01-23 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 20, 2017 at 04:52:22PM +0100, Lucas Stach wrote:
> This adds a new binding for the Freescale i.MX GPC block, which allows
> to describe multiple power domains in a more natural way. The driver
> will continue to support the old binding for existing DTBs, but new
> features like the additional domains present on i.MX6SX will only be
> usable with the new binding.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../devicetree/bindings/power/fsl,imx-gpc.txt      | 81 ++++++++++++++--------
>  1 file changed, 54 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> index 65cc0345747d..e5b660018d63 100644
> --- a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> +++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> @@ -1,22 +1,41 @@
>  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.
> +The i.MX6 General Power Control (GPC) block contains DVFS load tracking
> +counters and Power Gating Control (PGC).
>  
>  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:
> +- interrupts: Should contain one interrupt specifier for the GPC interrupt
> +- clocks: Must contain an entry for each entry in clock-names.
> +  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +  - ipg
> +- pgc: a list of child nodes describing the SoC power domains controlled by the
> +  power gating controller.

pgc is a node, not a property. Call it 'power-domains' instead.

>  
> -The gpc node is a power-controller as documented by the generic power domain
> -bindings in Documentation/devicetree/bindings/power/power_domain.txt.
> +The power domains are generic power domain providers as documented in
> +Documentation/devicetree/bindings/power/power_domain.txt. They are described as
> +subnodes of the GPC and should contain the following
> +
> +Required properties:
> +- reg: the DOMAIN_INDEX as used by the client devices to refer to this
> +  power domain
> +  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
> +
> +- #power-domain-cells: Should be 0
> +
> +Optional properties:
> +- clocks: a number of phandles to clocks that need to be enabled during domain
> +  power-up sequencing to ensure reset propagation into devices located inside
> +  this power domain
> +- domain-supply: a phandle to the regulator powering this domain

'power-supply' is more common for when we don't really have a name.

>  Example:
>  
> @@ -25,14 +44,29 @@ Example:
>  		reg = <0x020dc000 0x4000>;
>  		interrupts = <0 89 IRQ_TYPE_LEVEL_HIGH>,
>  			     <0 90 IRQ_TYPE_LEVEL_HIGH>;
> -		pu-supply = <&reg_pu>;
> -		clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
> -			 <&clks IMX6QDL_CLK_GPU3D_SHADER>,
> -			 <&clks IMX6QDL_CLK_GPU2D_CORE>,
> -			 <&clks IMX6QDL_CLK_GPU2D_AXI>,
> -			 <&clks IMX6QDL_CLK_OPENVG_AXI>,
> -			 <&clks IMX6QDL_CLK_VPU_AXI>;
> -		#power-domain-cells = <1>;
> +		clocks = <&clks IMX6QDL_CLK_IPG>;
> +		clock-names = "ipg";
> +		
> +		pgc {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			power-domain at 0 {
> +				reg = <0>;
> +				#power-domain-cells = <0>;
> +			};
> +			pd_pu: power-domain at 1 {
> +				reg = <1>;
> +				#power-domain-cells = <0>;
> +				domain-supply = <&reg_pu>;
> +				clocks = <&clks IMX6QDL_CLK_GPU3D_CORE>,
> +				         <&clks IMX6QDL_CLK_GPU3D_SHADER>,
> +				         <&clks IMX6QDL_CLK_GPU2D_CORE>,
> +				         <&clks IMX6QDL_CLK_GPU2D_AXI>,
> +				         <&clks IMX6QDL_CLK_OPENVG_AXI>,
> +				         <&clks IMX6QDL_CLK_VPU_AXI>;
> +			};
> +		};
>  	};
>  
>  
> @@ -40,20 +74,13 @@ Specifying power domain for IP modules
>  ======================================
>  
>  IP cores belonging to a power domain should contain a 'power-domains' property
> -that is a phandle pointing to the gpc device node and a DOMAIN_INDEX specifying
> -the power domain the device belongs to.
> +that is a phandle pointing to the power domain the device belongs to.
>  
>  Example of a device that is part of the PU power domain:
>  
>  	vpu: vpu at 02040000 {
>  		reg = <0x02040000 0x3c000>;
>  		/* ... */
> -		power-domains = <&gpc 1>;
> +		power-domains = <&pd_pu>;
>  		/* ... */
>  	};
> -
> -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
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] soc/imx: add new GPC driver
  2017-01-20 15:52     ` Lucas Stach
@ 2017-01-24 12:49         ` Shawn Guo
  -1 siblings, 0 replies; 18+ messages in thread
From: Shawn Guo @ 2017-01-24 12:49 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ

On Fri, Jan 20, 2017 at 04:52:24PM +0100, Lucas Stach wrote:
> This is an almost complete re-write of the previous GPC power gating control
> code found in the IMX architecture code. It supports both the old and the new
> DT binding, allowing more domains to be added later and generally makes the
> driver easier to extend, while keeping compatibility with existing DTBs.

Shouldn't we generally wrap the commit log around column 72?

> 
> Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  drivers/soc/Makefile     |   1 +
>  drivers/soc/imx/Makefile |   1 +
>  drivers/soc/imx/gpc.c    | 480 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 482 insertions(+)
>  create mode 100644 drivers/soc/imx/Makefile
>  create mode 100644 drivers/soc/imx/gpc.c
> 
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 50c23d0bd457..42b3fc7687bb 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -6,6 +6,7 @@ obj-y				+= bcm/
>  obj-$(CONFIG_ARCH_DOVE)		+= dove/
>  obj-$(CONFIG_MACH_DOVE)		+= dove/
>  obj-y				+= fsl/
> +obj-$(CONFIG_ARCH_MXC)		+= imx/
>  obj-$(CONFIG_ARCH_MEDIATEK)	+= mediatek/
>  obj-$(CONFIG_ARCH_QCOM)		+= qcom/
>  obj-$(CONFIG_ARCH_RENESAS)	+= renesas/
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> new file mode 100644
> index 000000000000..35861f5b2802
> --- /dev/null
> +++ b/drivers/soc/imx/Makefile
> @@ -0,0 +1 @@
> +obj-y += gpc.o
> diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c
> new file mode 100644
> index 000000000000..24bc0c6b72d8
> --- /dev/null
> +++ b/drivers/soc/imx/gpc.c
> @@ -0,0 +1,480 @@
> +/*
> + * Copyright 2015-2017 Pengutronix, Lucas Stach <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + * Copyright 2011-2013 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define GPC_CNTR		0x000
> +
> +#define GPC_PGC_PDN_OFFS	0x0
> +#define GPC_PGC_PUPSCR_OFFS	0x4
> +#define GPC_PGC_PDNSCR_OFFS	0x8
> +#define GPC_PGC_SW2ISO_SHIFT	0x8
> +#define GPC_PGC_SW_SHIFT	0x0
> +
> +#define GPC_PGC_GPU_PDN		0x260
> +#define GPC_PGC_GPU_PUPSCR	0x264
> +#define GPC_PGC_GPU_PDNSCR	0x268
> +
> +#define GPU_VPU_PUP_REQ		BIT(1)
> +#define GPU_VPU_PDN_REQ		BIT(0)
> +
> +#define GPC_CLK_MAX		6
> +
> +struct imx_pm_domain {
> +	struct generic_pm_domain base;
> +	struct regmap *regmap;
> +	struct regulator *supply;
> +	struct clk *clk[GPC_CLK_MAX];
> +	int num_clks;
> +	unsigned int reg_offs;
> +	signed char cntr_pdn_bit;
> +	unsigned int ipg_rate_mhz;
> +};
> +
> +static inline struct imx_pm_domain *
> +to_imx_pm_domain(struct generic_pm_domain *genpd)
> +{
> +	return container_of(genpd, struct imx_pm_domain, base);
> +}
> +
> +static int imx6_pm_domain_power_off(struct generic_pm_domain *genpd)
> +{
> +	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
> +	int iso, iso2sw;
> +	u32 val;
> +
> +	/* Read ISO and ISO2SW power down delays */
> +	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
> +	iso = val & 0x3f;
> +	iso2sw = (val >> 8) & 0x3f;
> +
> +	/* Gate off domain when powered down */
> +	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
> +			   0x1, 0x1);
> +
> +	/* Request GPC to power down domain */
> +	val = BIT(pd->cntr_pdn_bit);
> +	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
> +
> +	/* Wait ISO + ISO2SW IPG clock cycles */
> +	udelay(DIV_ROUND_UP(iso + iso2sw, pd->ipg_rate_mhz));
> +
> +	if (pd->supply)
> +		regulator_disable(pd->supply);
> +
> +	return 0;
> +}
> +
> +static int imx6_pm_domain_power_on(struct generic_pm_domain *genpd)
> +{
> +	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
> +	int i, ret, sw, sw2iso;
> +	u32 val;
> +
> +	if (pd->supply) {
> +		ret = regulator_enable(pd->supply);
> +		if (ret) {
> +			pr_err("%s: failed to enable regulator: %d\n",
> +			       __func__, ret);
> +			return ret;
> +		}
> +	}
> +
> +	/* Enable reset clocks for all devices in the domain */
> +	for (i = 0; i < pd->num_clks; i++)
> +		clk_prepare_enable(pd->clk[i]);
> +
> +	/* Gate off domain when powered down */
> +	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
> +			   0x1, 0x1);
> +
> +	/* Read ISO and ISO2SW power down delays */
> +	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
> +	sw = val & 0x3f;
> +	sw2iso = (val >> 8) & 0x3f;
> +
> +	/* Request GPC to power up domain */
> +	val = BIT(pd->cntr_pdn_bit + 1);
> +	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
> +
> +	/* Wait ISO + ISO2SW IPG clock cycles */
> +	udelay(DIV_ROUND_UP(sw + sw2iso, pd->ipg_rate_mhz));
> +
> +	/* Disable reset clocks for all devices in the domain */
> +	for (i = 0; i < pd->num_clks; i++)
> +		clk_disable_unprepare(pd->clk[i]);
> +
> +	return 0;
> +}
> +
> +static int imx_pgc_get_clocks(struct device *dev, struct imx_pm_domain *domain)
> +{
> +	int i, ret;
> +
> +	for (i = 0; ; i++) {
> +		struct clk *clk = of_clk_get(dev->of_node, i);
> +		if (IS_ERR(clk))
> +			break;
> +		if (i >= GPC_CLK_MAX) {
> +			dev_err(dev, "more than %d clocks\n", GPC_CLK_MAX);
> +			ret = -EINVAL;
> +			goto clk_err;
> +		}
> +		domain->clk[i] = clk;
> +	}
> +	domain->num_clks = i;
> +
> +	return 0;
> +
> +clk_err:
> +	for (; i >= 0; i--)
> +		clk_put(domain->clk[i]);
> +
> +	return ret;
> +}
> +
> +static void imx_pgc_put_clocks(struct imx_pm_domain *domain)
> +{
> +	int i;
> +
> +	for (i = domain->num_clks - 1; i >= 0; i--)
> +		clk_put(domain->clk[i]);
> +}
> +
> +static int imx_pgc_parse_dt(struct device *dev, struct imx_pm_domain *domain)
> +{
> +	/* try to get the domain supply regulator */
> +	domain->supply = devm_regulator_get_optional(dev, "domain");
> +	if (IS_ERR(domain->supply)) {
> +		if (PTR_ERR(domain->supply) == -ENODEV)
> +			domain->supply = NULL;
> +		else
> +			return PTR_ERR(domain->supply);
> +	}
> +
> +	/* try to get all clocks needed for reset propagation */
> +	return imx_pgc_get_clocks(dev, domain);
> +}
> +
> +static int imx_pgc_power_domain_probe(struct platform_device *pdev)
> +{
> +	struct imx_pm_domain *domain = pdev->dev.platform_data;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	/* if this PD is associated with a DT node try to parse it */
> +	if (dev->of_node) {
> +		ret = imx_pgc_parse_dt(dev, domain);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* initially power on the domain */
> +	if (domain->base.power_on)
> +		domain->base.power_on(&domain->base);
> +
> +	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> +		pm_genpd_init(&domain->base, NULL, false);
> +		ret = of_genpd_add_provider_simple(dev->of_node, &domain->base);
> +		if (ret)
> +			goto genpd_err;
> +	}
> +
> +	return 0;
> +
> +genpd_err:
> +	pm_genpd_remove(&domain->base);
> +	imx_pgc_put_clocks(domain);
> +
> +	return ret;
> +}
> +
> +static int imx_pgc_power_domain_remove(struct platform_device *pdev)
> +{
> +	struct imx_pm_domain *domain = pdev->dev.platform_data;
> +
> +	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> +		of_genpd_del_provider(pdev->dev.of_node);
> +		pm_genpd_remove(&domain->base);
> +		imx_pgc_put_clocks(domain);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id imx_pgc_power_domain_id[] = {
> +	{ "imx-pgc-power-domain"},
> +	{ },
> +};
> +
> +static struct platform_driver imx_pgc_power_domain_driver = {
> +	.driver = {
> +		.name = "imx-pgc-pd",
> +	},
> +	.probe = imx_pgc_power_domain_probe,
> +	.remove = imx_pgc_power_domain_remove,

Not sure how .remove hook is going to be useful for a
builtin_platform_driver.

> +	.id_table = imx_pgc_power_domain_id,
> +};
> +builtin_platform_driver(imx_pgc_power_domain_driver)
> +
> +static struct genpd_power_state imx6_pm_domain_pu_state = {
> +	.power_off_latency_ns = 25000,
> +	.power_on_latency_ns = 2000000,
> +};
> +
> +static struct imx_pm_domain imx_gpc_domains[] = {
> +	{
> +		.base = {
> +			.name = "ARM",
> +		},
> +	}, {
> +		.base = {
> +			.name = "PU",
> +			.power_off = imx6_pm_domain_power_off,
> +			.power_on = imx6_pm_domain_power_on,
> +			.states = &imx6_pm_domain_pu_state,
> +			.state_count = 1,
> +		},
> +		.reg_offs = 0x260,
> +		.cntr_pdn_bit = 0,
> +	}, {
> +		.base = {
> +			.name = "DISPLAY",
> +			.power_off = imx6_pm_domain_power_off,
> +			.power_on = imx6_pm_domain_power_on,
> +		},
> +		.reg_offs = 0x240,
> +		.cntr_pdn_bit = 4,
> +	}
> +};
> +
> +struct imx_gpc_dt_data {
> +	int num_domains;
> +};
> +
> +static const struct imx_gpc_dt_data imx6q_dt_data = {
> +	.num_domains = 2,
> +};
> +
> +static const struct imx_gpc_dt_data imx6sl_dt_data = {
> +	.num_domains = 3,
> +};
> +
> +static const struct of_device_id imx_gpc_dt_ids[] = {
> +	{ .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
> +	{ .compatible = "fsl,imx6sl-gpc", .data = &imx6sl_dt_data },
> +	{ }
> +};
> +
> +static bool imx_gpc_readable_reg(struct device *dev, unsigned int reg)
> +{
> +	return (reg % 4 == 0) && (reg <= 0x2ac);
> +}
> +
> +static bool imx_gpc_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	if (reg == GPC_CNTR)
> +		return true;
> +
> +	return false;
> +}
> +
> +static const struct regmap_config imx_gpc_regmap_config = {
> +	.cache_type = REGCACHE_FLAT,
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +
> +	.readable_reg = imx_gpc_readable_reg,
> +	.volatile_reg = imx_gpc_volatile_reg,
> +
> +	.max_register = 0x2ac,
> +};
> +
> +static struct generic_pm_domain *imx_gpc_onecell_domains[] = {
> +	&imx_gpc_domains[0].base,
> +	&imx_gpc_domains[1].base,
> +};
> +
> +static struct genpd_onecell_data imx_gpc_onecell_data = {
> +	.domains = imx_gpc_onecell_domains,
> +	.num_domains = 2,
> +};
> +
> +static int imx_gpc_old_dt_init(struct device *dev, struct regmap *regmap)
> +{
> +	struct imx_pm_domain *domain;
> +	int i, ret;
> +
> +	for (i = 0; i < 2; i++) {
> +		domain = &imx_gpc_domains[i];
> +		domain->regmap = regmap;
> +		domain->ipg_rate_mhz = 33;

I see it's being 66MHz in the existing driver code.  What's the reason
behind this change?

> +
> +		if (i == 1) {
> +			domain->supply = devm_regulator_get(dev, "pu");
> +			if (IS_ERR(domain->supply))
> +				return PTR_ERR(domain->supply);;
> +
> +			ret = imx_pgc_get_clocks(dev, domain);
> +			if (ret)
> +				goto clk_err;
> +
> +			domain->base.power_on(&domain->base);
> +		}
> +	}
> +
> +	for (i = 0; i < 2; i++)
> +		pm_genpd_init(&imx_gpc_domains[i].base, NULL, false);
> +
> +	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> +		ret = of_genpd_add_provider_onecell(dev->of_node,
> +						    &imx_gpc_onecell_data);
> +		if (ret)
> +			goto genpd_err;
> +	}
> +
> +	return 0;
> +
> +genpd_err:
> +	pm_genpd_remove(&imx_gpc_domains[1].base);
> +	imx_pgc_put_clocks(&imx_gpc_domains[1]);
> +clk_err:
> +	pm_genpd_remove(&imx_gpc_domains[0].base);

I do not think that imx_gpc_domains[0].base is already registered on
clk_err path.

> +
> +	return ret;
> +}
> +
> +static int imx_gpc_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *of_id =
> +			of_match_device(imx_gpc_dt_ids, &pdev->dev);
> +	const struct imx_gpc_dt_data *of_id_data = of_id->data;
> +	struct device_node *pgc_node;
> +	struct regmap *regmap;
> +	struct resource *res;
> +	void __iomem *base;
> +	int ret;
> +
> +	pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc");
> +
> +	/* bail out if DT too old and doesn't provide the necessary info */
> +	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells") &&
> +	    !pgc_node)
> +		return 0;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
> +					   &imx_gpc_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(&pdev->dev, "failed to init regmap: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	if (!pgc_node) {
> +		/* old DT layout is only supported for mx6q aka 2 domains */
> +		if (of_id_data->num_domains != 2) {
> +			dev_err(&pdev->dev, "could not find pgc DT node\n");
> +			return -ENODEV;
> +		}
> +
> +		ret = imx_gpc_old_dt_init(&pdev->dev, regmap);
> +		if (ret)
> +			return ret;
> +	} else {
> +		struct imx_pm_domain *domain;
> +		struct platform_device *pd_pdev;
> +		struct device_node *np;
> +		struct clk *ipg_clk;
> +		unsigned int ipg_rate_mhz;
> +		int domain_index;
> +
> +		ipg_clk = devm_clk_get(&pdev->dev, "ipg");
> +		if (IS_ERR(ipg_clk))
> +			return PTR_ERR(ipg_clk);
> +		ipg_rate_mhz = clk_get_rate(ipg_clk) / 1000000;
> +
> +		for_each_child_of_node(pgc_node, np) {
> +			ret = of_property_read_u32(np, "reg", &domain_index);
> +			if (ret) {
> +				of_node_put(np);
> +				return ret;
> +			}

Should we have a check here to ensure that domain_index doesn't exceed
imx_gpc_domains[] array size?

Shawn

> +
> +			domain = &imx_gpc_domains[domain_index];
> +			domain->regmap = regmap;
> +			domain->ipg_rate_mhz = ipg_rate_mhz;
> +
> +			pd_pdev = platform_device_alloc("imx-pgc-power-domain",
> +							domain_index);
> +			pd_pdev->dev.platform_data = domain;
> +			pd_pdev->dev.parent = &pdev->dev;
> +			pd_pdev->dev.of_node = np;
> +
> +			ret = platform_device_add(pd_pdev);
> +			if (ret) {
> +				platform_device_put(pd_pdev);
> +				of_node_put(np);
> +				return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int imx_gpc_remove(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	/*
> +	 * If the old DT binding is used the toplevel driver needs to
> +	 * de-register the power domains
> +	 */
> +	if (!of_get_child_by_name(pdev->dev.of_node, "pgc")) {
> +		of_genpd_del_provider(pdev->dev.of_node);
> +
> +		ret = pm_genpd_remove(&imx_gpc_domains[1].base);
> +		if (ret)
> +			return ret;
> +		imx_pgc_put_clocks(&imx_gpc_domains[1]);
> +
> +		ret = pm_genpd_remove(&imx_gpc_domains[0].base);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct platform_driver imx_gpc_driver = {
> +	.driver = {
> +		.name = "imx-gpc",
> +		.of_match_table = imx_gpc_dt_ids,
> +	},
> +	.probe = imx_gpc_probe,
> +	.remove = imx_gpc_remove,
> +};
> +builtin_platform_driver(imx_gpc_driver)
> -- 
> 2.11.0
> 
--
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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 3/4] soc/imx: add new GPC driver
@ 2017-01-24 12:49         ` Shawn Guo
  0 siblings, 0 replies; 18+ messages in thread
From: Shawn Guo @ 2017-01-24 12:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 20, 2017 at 04:52:24PM +0100, Lucas Stach wrote:
> This is an almost complete re-write of the previous GPC power gating control
> code found in the IMX architecture code. It supports both the old and the new
> DT binding, allowing more domains to be added later and generally makes the
> driver easier to extend, while keeping compatibility with existing DTBs.

Shouldn't we generally wrap the commit log around column 72?

> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/soc/Makefile     |   1 +
>  drivers/soc/imx/Makefile |   1 +
>  drivers/soc/imx/gpc.c    | 480 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 482 insertions(+)
>  create mode 100644 drivers/soc/imx/Makefile
>  create mode 100644 drivers/soc/imx/gpc.c
> 
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 50c23d0bd457..42b3fc7687bb 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -6,6 +6,7 @@ obj-y				+= bcm/
>  obj-$(CONFIG_ARCH_DOVE)		+= dove/
>  obj-$(CONFIG_MACH_DOVE)		+= dove/
>  obj-y				+= fsl/
> +obj-$(CONFIG_ARCH_MXC)		+= imx/
>  obj-$(CONFIG_ARCH_MEDIATEK)	+= mediatek/
>  obj-$(CONFIG_ARCH_QCOM)		+= qcom/
>  obj-$(CONFIG_ARCH_RENESAS)	+= renesas/
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> new file mode 100644
> index 000000000000..35861f5b2802
> --- /dev/null
> +++ b/drivers/soc/imx/Makefile
> @@ -0,0 +1 @@
> +obj-y += gpc.o
> diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c
> new file mode 100644
> index 000000000000..24bc0c6b72d8
> --- /dev/null
> +++ b/drivers/soc/imx/gpc.c
> @@ -0,0 +1,480 @@
> +/*
> + * Copyright 2015-2017 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + * Copyright 2011-2013 Freescale Semiconductor, Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define GPC_CNTR		0x000
> +
> +#define GPC_PGC_PDN_OFFS	0x0
> +#define GPC_PGC_PUPSCR_OFFS	0x4
> +#define GPC_PGC_PDNSCR_OFFS	0x8
> +#define GPC_PGC_SW2ISO_SHIFT	0x8
> +#define GPC_PGC_SW_SHIFT	0x0
> +
> +#define GPC_PGC_GPU_PDN		0x260
> +#define GPC_PGC_GPU_PUPSCR	0x264
> +#define GPC_PGC_GPU_PDNSCR	0x268
> +
> +#define GPU_VPU_PUP_REQ		BIT(1)
> +#define GPU_VPU_PDN_REQ		BIT(0)
> +
> +#define GPC_CLK_MAX		6
> +
> +struct imx_pm_domain {
> +	struct generic_pm_domain base;
> +	struct regmap *regmap;
> +	struct regulator *supply;
> +	struct clk *clk[GPC_CLK_MAX];
> +	int num_clks;
> +	unsigned int reg_offs;
> +	signed char cntr_pdn_bit;
> +	unsigned int ipg_rate_mhz;
> +};
> +
> +static inline struct imx_pm_domain *
> +to_imx_pm_domain(struct generic_pm_domain *genpd)
> +{
> +	return container_of(genpd, struct imx_pm_domain, base);
> +}
> +
> +static int imx6_pm_domain_power_off(struct generic_pm_domain *genpd)
> +{
> +	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
> +	int iso, iso2sw;
> +	u32 val;
> +
> +	/* Read ISO and ISO2SW power down delays */
> +	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
> +	iso = val & 0x3f;
> +	iso2sw = (val >> 8) & 0x3f;
> +
> +	/* Gate off domain when powered down */
> +	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
> +			   0x1, 0x1);
> +
> +	/* Request GPC to power down domain */
> +	val = BIT(pd->cntr_pdn_bit);
> +	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
> +
> +	/* Wait ISO + ISO2SW IPG clock cycles */
> +	udelay(DIV_ROUND_UP(iso + iso2sw, pd->ipg_rate_mhz));
> +
> +	if (pd->supply)
> +		regulator_disable(pd->supply);
> +
> +	return 0;
> +}
> +
> +static int imx6_pm_domain_power_on(struct generic_pm_domain *genpd)
> +{
> +	struct imx_pm_domain *pd = to_imx_pm_domain(genpd);
> +	int i, ret, sw, sw2iso;
> +	u32 val;
> +
> +	if (pd->supply) {
> +		ret = regulator_enable(pd->supply);
> +		if (ret) {
> +			pr_err("%s: failed to enable regulator: %d\n",
> +			       __func__, ret);
> +			return ret;
> +		}
> +	}
> +
> +	/* Enable reset clocks for all devices in the domain */
> +	for (i = 0; i < pd->num_clks; i++)
> +		clk_prepare_enable(pd->clk[i]);
> +
> +	/* Gate off domain when powered down */
> +	regmap_update_bits(pd->regmap, pd->reg_offs + GPC_PGC_PDN_OFFS,
> +			   0x1, 0x1);
> +
> +	/* Read ISO and ISO2SW power down delays */
> +	regmap_read(pd->regmap, pd->reg_offs + GPC_PGC_PUPSCR_OFFS, &val);
> +	sw = val & 0x3f;
> +	sw2iso = (val >> 8) & 0x3f;
> +
> +	/* Request GPC to power up domain */
> +	val = BIT(pd->cntr_pdn_bit + 1);
> +	regmap_update_bits(pd->regmap, GPC_CNTR, val, val);
> +
> +	/* Wait ISO + ISO2SW IPG clock cycles */
> +	udelay(DIV_ROUND_UP(sw + sw2iso, pd->ipg_rate_mhz));
> +
> +	/* Disable reset clocks for all devices in the domain */
> +	for (i = 0; i < pd->num_clks; i++)
> +		clk_disable_unprepare(pd->clk[i]);
> +
> +	return 0;
> +}
> +
> +static int imx_pgc_get_clocks(struct device *dev, struct imx_pm_domain *domain)
> +{
> +	int i, ret;
> +
> +	for (i = 0; ; i++) {
> +		struct clk *clk = of_clk_get(dev->of_node, i);
> +		if (IS_ERR(clk))
> +			break;
> +		if (i >= GPC_CLK_MAX) {
> +			dev_err(dev, "more than %d clocks\n", GPC_CLK_MAX);
> +			ret = -EINVAL;
> +			goto clk_err;
> +		}
> +		domain->clk[i] = clk;
> +	}
> +	domain->num_clks = i;
> +
> +	return 0;
> +
> +clk_err:
> +	for (; i >= 0; i--)
> +		clk_put(domain->clk[i]);
> +
> +	return ret;
> +}
> +
> +static void imx_pgc_put_clocks(struct imx_pm_domain *domain)
> +{
> +	int i;
> +
> +	for (i = domain->num_clks - 1; i >= 0; i--)
> +		clk_put(domain->clk[i]);
> +}
> +
> +static int imx_pgc_parse_dt(struct device *dev, struct imx_pm_domain *domain)
> +{
> +	/* try to get the domain supply regulator */
> +	domain->supply = devm_regulator_get_optional(dev, "domain");
> +	if (IS_ERR(domain->supply)) {
> +		if (PTR_ERR(domain->supply) == -ENODEV)
> +			domain->supply = NULL;
> +		else
> +			return PTR_ERR(domain->supply);
> +	}
> +
> +	/* try to get all clocks needed for reset propagation */
> +	return imx_pgc_get_clocks(dev, domain);
> +}
> +
> +static int imx_pgc_power_domain_probe(struct platform_device *pdev)
> +{
> +	struct imx_pm_domain *domain = pdev->dev.platform_data;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	/* if this PD is associated with a DT node try to parse it */
> +	if (dev->of_node) {
> +		ret = imx_pgc_parse_dt(dev, domain);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* initially power on the domain */
> +	if (domain->base.power_on)
> +		domain->base.power_on(&domain->base);
> +
> +	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> +		pm_genpd_init(&domain->base, NULL, false);
> +		ret = of_genpd_add_provider_simple(dev->of_node, &domain->base);
> +		if (ret)
> +			goto genpd_err;
> +	}
> +
> +	return 0;
> +
> +genpd_err:
> +	pm_genpd_remove(&domain->base);
> +	imx_pgc_put_clocks(domain);
> +
> +	return ret;
> +}
> +
> +static int imx_pgc_power_domain_remove(struct platform_device *pdev)
> +{
> +	struct imx_pm_domain *domain = pdev->dev.platform_data;
> +
> +	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> +		of_genpd_del_provider(pdev->dev.of_node);
> +		pm_genpd_remove(&domain->base);
> +		imx_pgc_put_clocks(domain);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id imx_pgc_power_domain_id[] = {
> +	{ "imx-pgc-power-domain"},
> +	{ },
> +};
> +
> +static struct platform_driver imx_pgc_power_domain_driver = {
> +	.driver = {
> +		.name = "imx-pgc-pd",
> +	},
> +	.probe = imx_pgc_power_domain_probe,
> +	.remove = imx_pgc_power_domain_remove,

Not sure how .remove hook is going to be useful for a
builtin_platform_driver.

> +	.id_table = imx_pgc_power_domain_id,
> +};
> +builtin_platform_driver(imx_pgc_power_domain_driver)
> +
> +static struct genpd_power_state imx6_pm_domain_pu_state = {
> +	.power_off_latency_ns = 25000,
> +	.power_on_latency_ns = 2000000,
> +};
> +
> +static struct imx_pm_domain imx_gpc_domains[] = {
> +	{
> +		.base = {
> +			.name = "ARM",
> +		},
> +	}, {
> +		.base = {
> +			.name = "PU",
> +			.power_off = imx6_pm_domain_power_off,
> +			.power_on = imx6_pm_domain_power_on,
> +			.states = &imx6_pm_domain_pu_state,
> +			.state_count = 1,
> +		},
> +		.reg_offs = 0x260,
> +		.cntr_pdn_bit = 0,
> +	}, {
> +		.base = {
> +			.name = "DISPLAY",
> +			.power_off = imx6_pm_domain_power_off,
> +			.power_on = imx6_pm_domain_power_on,
> +		},
> +		.reg_offs = 0x240,
> +		.cntr_pdn_bit = 4,
> +	}
> +};
> +
> +struct imx_gpc_dt_data {
> +	int num_domains;
> +};
> +
> +static const struct imx_gpc_dt_data imx6q_dt_data = {
> +	.num_domains = 2,
> +};
> +
> +static const struct imx_gpc_dt_data imx6sl_dt_data = {
> +	.num_domains = 3,
> +};
> +
> +static const struct of_device_id imx_gpc_dt_ids[] = {
> +	{ .compatible = "fsl,imx6q-gpc", .data = &imx6q_dt_data },
> +	{ .compatible = "fsl,imx6sl-gpc", .data = &imx6sl_dt_data },
> +	{ }
> +};
> +
> +static bool imx_gpc_readable_reg(struct device *dev, unsigned int reg)
> +{
> +	return (reg % 4 == 0) && (reg <= 0x2ac);
> +}
> +
> +static bool imx_gpc_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	if (reg == GPC_CNTR)
> +		return true;
> +
> +	return false;
> +}
> +
> +static const struct regmap_config imx_gpc_regmap_config = {
> +	.cache_type = REGCACHE_FLAT,
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +
> +	.readable_reg = imx_gpc_readable_reg,
> +	.volatile_reg = imx_gpc_volatile_reg,
> +
> +	.max_register = 0x2ac,
> +};
> +
> +static struct generic_pm_domain *imx_gpc_onecell_domains[] = {
> +	&imx_gpc_domains[0].base,
> +	&imx_gpc_domains[1].base,
> +};
> +
> +static struct genpd_onecell_data imx_gpc_onecell_data = {
> +	.domains = imx_gpc_onecell_domains,
> +	.num_domains = 2,
> +};
> +
> +static int imx_gpc_old_dt_init(struct device *dev, struct regmap *regmap)
> +{
> +	struct imx_pm_domain *domain;
> +	int i, ret;
> +
> +	for (i = 0; i < 2; i++) {
> +		domain = &imx_gpc_domains[i];
> +		domain->regmap = regmap;
> +		domain->ipg_rate_mhz = 33;

I see it's being 66MHz in the existing driver code.  What's the reason
behind this change?

> +
> +		if (i == 1) {
> +			domain->supply = devm_regulator_get(dev, "pu");
> +			if (IS_ERR(domain->supply))
> +				return PTR_ERR(domain->supply);;
> +
> +			ret = imx_pgc_get_clocks(dev, domain);
> +			if (ret)
> +				goto clk_err;
> +
> +			domain->base.power_on(&domain->base);
> +		}
> +	}
> +
> +	for (i = 0; i < 2; i++)
> +		pm_genpd_init(&imx_gpc_domains[i].base, NULL, false);
> +
> +	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> +		ret = of_genpd_add_provider_onecell(dev->of_node,
> +						    &imx_gpc_onecell_data);
> +		if (ret)
> +			goto genpd_err;
> +	}
> +
> +	return 0;
> +
> +genpd_err:
> +	pm_genpd_remove(&imx_gpc_domains[1].base);
> +	imx_pgc_put_clocks(&imx_gpc_domains[1]);
> +clk_err:
> +	pm_genpd_remove(&imx_gpc_domains[0].base);

I do not think that imx_gpc_domains[0].base is already registered on
clk_err path.

> +
> +	return ret;
> +}
> +
> +static int imx_gpc_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *of_id =
> +			of_match_device(imx_gpc_dt_ids, &pdev->dev);
> +	const struct imx_gpc_dt_data *of_id_data = of_id->data;
> +	struct device_node *pgc_node;
> +	struct regmap *regmap;
> +	struct resource *res;
> +	void __iomem *base;
> +	int ret;
> +
> +	pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc");
> +
> +	/* bail out if DT too old and doesn't provide the necessary info */
> +	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells") &&
> +	    !pgc_node)
> +		return 0;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
> +					   &imx_gpc_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(&pdev->dev, "failed to init regmap: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	if (!pgc_node) {
> +		/* old DT layout is only supported for mx6q aka 2 domains */
> +		if (of_id_data->num_domains != 2) {
> +			dev_err(&pdev->dev, "could not find pgc DT node\n");
> +			return -ENODEV;
> +		}
> +
> +		ret = imx_gpc_old_dt_init(&pdev->dev, regmap);
> +		if (ret)
> +			return ret;
> +	} else {
> +		struct imx_pm_domain *domain;
> +		struct platform_device *pd_pdev;
> +		struct device_node *np;
> +		struct clk *ipg_clk;
> +		unsigned int ipg_rate_mhz;
> +		int domain_index;
> +
> +		ipg_clk = devm_clk_get(&pdev->dev, "ipg");
> +		if (IS_ERR(ipg_clk))
> +			return PTR_ERR(ipg_clk);
> +		ipg_rate_mhz = clk_get_rate(ipg_clk) / 1000000;
> +
> +		for_each_child_of_node(pgc_node, np) {
> +			ret = of_property_read_u32(np, "reg", &domain_index);
> +			if (ret) {
> +				of_node_put(np);
> +				return ret;
> +			}

Should we have a check here to ensure that domain_index doesn't exceed
imx_gpc_domains[] array size?

Shawn

> +
> +			domain = &imx_gpc_domains[domain_index];
> +			domain->regmap = regmap;
> +			domain->ipg_rate_mhz = ipg_rate_mhz;
> +
> +			pd_pdev = platform_device_alloc("imx-pgc-power-domain",
> +							domain_index);
> +			pd_pdev->dev.platform_data = domain;
> +			pd_pdev->dev.parent = &pdev->dev;
> +			pd_pdev->dev.of_node = np;
> +
> +			ret = platform_device_add(pd_pdev);
> +			if (ret) {
> +				platform_device_put(pd_pdev);
> +				of_node_put(np);
> +				return ret;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int imx_gpc_remove(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	/*
> +	 * If the old DT binding is used the toplevel driver needs to
> +	 * de-register the power domains
> +	 */
> +	if (!of_get_child_by_name(pdev->dev.of_node, "pgc")) {
> +		of_genpd_del_provider(pdev->dev.of_node);
> +
> +		ret = pm_genpd_remove(&imx_gpc_domains[1].base);
> +		if (ret)
> +			return ret;
> +		imx_pgc_put_clocks(&imx_gpc_domains[1]);
> +
> +		ret = pm_genpd_remove(&imx_gpc_domains[0].base);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct platform_driver imx_gpc_driver = {
> +	.driver = {
> +		.name = "imx-gpc",
> +		.of_match_table = imx_gpc_dt_ids,
> +	},
> +	.probe = imx_gpc_probe,
> +	.remove = imx_gpc_remove,
> +};
> +builtin_platform_driver(imx_gpc_driver)
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] soc/imx: add new GPC driver
  2017-01-24 12:49         ` Shawn Guo
@ 2017-01-24 16:54           ` Lucas Stach
  -1 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-24 16:54 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Fabio Estevam, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	patchwork-lst-bIcnvbaLZ9MEGnE8C9+IrQ

Hi Shawn,

thanks for the review.

Am Dienstag, den 24.01.2017, 20:49 +0800 schrieb Shawn Guo:
> On Fri, Jan 20, 2017 at 04:52:24PM +0100, Lucas Stach wrote:
> > This is an almost complete re-write of the previous GPC power gating control
> > code found in the IMX architecture code. It supports both the old and the new
> > DT binding, allowing more domains to be added later and generally makes the
> > driver easier to extend, while keeping compatibility with existing DTBs.
> 
> Shouldn't we generally wrap the commit log around column 72?
> 
I don't see why we should wrap commit messages earlier than the actual
code, but I think thats mostly an issue of personal preference.

> > 
> > Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> >  drivers/soc/Makefile     |   1 +
> >  drivers/soc/imx/Makefile |   1 +
> >  drivers/soc/imx/gpc.c    | 480 +++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 482 insertions(+)
> >  create mode 100644 drivers/soc/imx/Makefile
> >  create mode 100644 drivers/soc/imx/gpc.c

[...]
> > +static struct platform_driver imx_pgc_power_domain_driver = {
> > +	.driver = {
> > +		.name = "imx-pgc-pd",
> > +	},
> > +	.probe = imx_pgc_power_domain_probe,
> > +	.remove = imx_pgc_power_domain_remove,
> 
> Not sure how .remove hook is going to be useful for a
> builtin_platform_driver.
> 
There is nothing preventing a manual unbind/bind on a builtin driver.
But yes, this is mostly future proofing right now.

[...]
> > +static int imx_gpc_old_dt_init(struct device *dev, struct regmap *regmap)
> > +{
> > +	struct imx_pm_domain *domain;
> > +	int i, ret;
> > +
> > +	for (i = 0; i < 2; i++) {
> > +		domain = &imx_gpc_domains[i];
> > +		domain->regmap = regmap;
> > +		domain->ipg_rate_mhz = 33;
> 
> I see it's being 66MHz in the existing driver code.  What's the reason
> behind this change?
> 
Thanks for the catch. This should stay at 66MHz.

> > +
> > +		if (i == 1) {
> > +			domain->supply = devm_regulator_get(dev, "pu");
> > +			if (IS_ERR(domain->supply))
> > +				return PTR_ERR(domain->supply);;
> > +
> > +			ret = imx_pgc_get_clocks(dev, domain);
> > +			if (ret)
> > +				goto clk_err;
> > +
> > +			domain->base.power_on(&domain->base);
> > +		}
> > +	}
> > +
> > +	for (i = 0; i < 2; i++)
> > +		pm_genpd_init(&imx_gpc_domains[i].base, NULL, false);
> > +
> > +	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> > +		ret = of_genpd_add_provider_onecell(dev->of_node,
> > +						    &imx_gpc_onecell_data);
> > +		if (ret)
> > +			goto genpd_err;
> > +	}
> > +
> > +	return 0;
> > +
> > +genpd_err:
> > +	pm_genpd_remove(&imx_gpc_domains[1].base);
> > +	imx_pgc_put_clocks(&imx_gpc_domains[1]);
> > +clk_err:
> > +	pm_genpd_remove(&imx_gpc_domains[0].base);
> 
> I do not think that imx_gpc_domains[0].base is already registered on
> clk_err path.
> 
Um right, this seems to be a leftover from an earlier version.

> > +
> > +	return ret;
> > +}
> > +
> > +static int imx_gpc_probe(struct platform_device *pdev)
> > +{
> > +	const struct of_device_id *of_id =
> > +			of_match_device(imx_gpc_dt_ids, &pdev->dev);
> > +	const struct imx_gpc_dt_data *of_id_data = of_id->data;
> > +	struct device_node *pgc_node;
> > +	struct regmap *regmap;
> > +	struct resource *res;
> > +	void __iomem *base;
> > +	int ret;
> > +
> > +	pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc");
> > +
> > +	/* bail out if DT too old and doesn't provide the necessary info */
> > +	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells") &&
> > +	    !pgc_node)
> > +		return 0;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> > +
> > +	regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
> > +					   &imx_gpc_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		ret = PTR_ERR(regmap);
> > +		dev_err(&pdev->dev, "failed to init regmap: %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +
> > +	if (!pgc_node) {
> > +		/* old DT layout is only supported for mx6q aka 2 domains */
> > +		if (of_id_data->num_domains != 2) {
> > +			dev_err(&pdev->dev, "could not find pgc DT node\n");
> > +			return -ENODEV;
> > +		}
> > +
> > +		ret = imx_gpc_old_dt_init(&pdev->dev, regmap);
> > +		if (ret)
> > +			return ret;
> > +	} else {
> > +		struct imx_pm_domain *domain;
> > +		struct platform_device *pd_pdev;
> > +		struct device_node *np;
> > +		struct clk *ipg_clk;
> > +		unsigned int ipg_rate_mhz;
> > +		int domain_index;
> > +
> > +		ipg_clk = devm_clk_get(&pdev->dev, "ipg");
> > +		if (IS_ERR(ipg_clk))
> > +			return PTR_ERR(ipg_clk);
> > +		ipg_rate_mhz = clk_get_rate(ipg_clk) / 1000000;
> > +
> > +		for_each_child_of_node(pgc_node, np) {
> > +			ret = of_property_read_u32(np, "reg", &domain_index);
> > +			if (ret) {
> > +				of_node_put(np);
> > +				return ret;
> > +			}
> 
> Should we have a check here to ensure that domain_index doesn't exceed
> imx_gpc_domains[] array size?

Yes.

Regards,
Lucas

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 3/4] soc/imx: add new GPC driver
@ 2017-01-24 16:54           ` Lucas Stach
  0 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-24 16:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Shawn,

thanks for the review.

Am Dienstag, den 24.01.2017, 20:49 +0800 schrieb Shawn Guo:
> On Fri, Jan 20, 2017 at 04:52:24PM +0100, Lucas Stach wrote:
> > This is an almost complete re-write of the previous GPC power gating control
> > code found in the IMX architecture code. It supports both the old and the new
> > DT binding, allowing more domains to be added later and generally makes the
> > driver easier to extend, while keeping compatibility with existing DTBs.
> 
> Shouldn't we generally wrap the commit log around column 72?
> 
I don't see why we should wrap commit messages earlier than the actual
code, but I think thats mostly an issue of personal preference.

> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/soc/Makefile     |   1 +
> >  drivers/soc/imx/Makefile |   1 +
> >  drivers/soc/imx/gpc.c    | 480 +++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 482 insertions(+)
> >  create mode 100644 drivers/soc/imx/Makefile
> >  create mode 100644 drivers/soc/imx/gpc.c

[...]
> > +static struct platform_driver imx_pgc_power_domain_driver = {
> > +	.driver = {
> > +		.name = "imx-pgc-pd",
> > +	},
> > +	.probe = imx_pgc_power_domain_probe,
> > +	.remove = imx_pgc_power_domain_remove,
> 
> Not sure how .remove hook is going to be useful for a
> builtin_platform_driver.
> 
There is nothing preventing a manual unbind/bind on a builtin driver.
But yes, this is mostly future proofing right now.

[...]
> > +static int imx_gpc_old_dt_init(struct device *dev, struct regmap *regmap)
> > +{
> > +	struct imx_pm_domain *domain;
> > +	int i, ret;
> > +
> > +	for (i = 0; i < 2; i++) {
> > +		domain = &imx_gpc_domains[i];
> > +		domain->regmap = regmap;
> > +		domain->ipg_rate_mhz = 33;
> 
> I see it's being 66MHz in the existing driver code.  What's the reason
> behind this change?
> 
Thanks for the catch. This should stay at 66MHz.

> > +
> > +		if (i == 1) {
> > +			domain->supply = devm_regulator_get(dev, "pu");
> > +			if (IS_ERR(domain->supply))
> > +				return PTR_ERR(domain->supply);;
> > +
> > +			ret = imx_pgc_get_clocks(dev, domain);
> > +			if (ret)
> > +				goto clk_err;
> > +
> > +			domain->base.power_on(&domain->base);
> > +		}
> > +	}
> > +
> > +	for (i = 0; i < 2; i++)
> > +		pm_genpd_init(&imx_gpc_domains[i].base, NULL, false);
> > +
> > +	if (IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) {
> > +		ret = of_genpd_add_provider_onecell(dev->of_node,
> > +						    &imx_gpc_onecell_data);
> > +		if (ret)
> > +			goto genpd_err;
> > +	}
> > +
> > +	return 0;
> > +
> > +genpd_err:
> > +	pm_genpd_remove(&imx_gpc_domains[1].base);
> > +	imx_pgc_put_clocks(&imx_gpc_domains[1]);
> > +clk_err:
> > +	pm_genpd_remove(&imx_gpc_domains[0].base);
> 
> I do not think that imx_gpc_domains[0].base is already registered on
> clk_err path.
> 
Um right, this seems to be a leftover from an earlier version.

> > +
> > +	return ret;
> > +}
> > +
> > +static int imx_gpc_probe(struct platform_device *pdev)
> > +{
> > +	const struct of_device_id *of_id =
> > +			of_match_device(imx_gpc_dt_ids, &pdev->dev);
> > +	const struct imx_gpc_dt_data *of_id_data = of_id->data;
> > +	struct device_node *pgc_node;
> > +	struct regmap *regmap;
> > +	struct resource *res;
> > +	void __iomem *base;
> > +	int ret;
> > +
> > +	pgc_node = of_get_child_by_name(pdev->dev.of_node, "pgc");
> > +
> > +	/* bail out if DT too old and doesn't provide the necessary info */
> > +	if (!of_property_read_bool(pdev->dev.of_node, "#power-domain-cells") &&
> > +	    !pgc_node)
> > +		return 0;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> > +
> > +	regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
> > +					   &imx_gpc_regmap_config);
> > +	if (IS_ERR(regmap)) {
> > +		ret = PTR_ERR(regmap);
> > +		dev_err(&pdev->dev, "failed to init regmap: %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +
> > +	if (!pgc_node) {
> > +		/* old DT layout is only supported for mx6q aka 2 domains */
> > +		if (of_id_data->num_domains != 2) {
> > +			dev_err(&pdev->dev, "could not find pgc DT node\n");
> > +			return -ENODEV;
> > +		}
> > +
> > +		ret = imx_gpc_old_dt_init(&pdev->dev, regmap);
> > +		if (ret)
> > +			return ret;
> > +	} else {
> > +		struct imx_pm_domain *domain;
> > +		struct platform_device *pd_pdev;
> > +		struct device_node *np;
> > +		struct clk *ipg_clk;
> > +		unsigned int ipg_rate_mhz;
> > +		int domain_index;
> > +
> > +		ipg_clk = devm_clk_get(&pdev->dev, "ipg");
> > +		if (IS_ERR(ipg_clk))
> > +			return PTR_ERR(ipg_clk);
> > +		ipg_rate_mhz = clk_get_rate(ipg_clk) / 1000000;
> > +
> > +		for_each_child_of_node(pgc_node, np) {
> > +			ret = of_property_read_u32(np, "reg", &domain_index);
> > +			if (ret) {
> > +				of_node_put(np);
> > +				return ret;
> > +			}
> 
> Should we have a check here to ensure that domain_index doesn't exceed
> imx_gpc_domains[] array size?

Yes.

Regards,
Lucas

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] dt-bindings: add multidomain support to i.MX GPC DT binding
  2017-01-23 16:26       ` Rob Herring
@ 2017-01-27 18:02         ` Lucas Stach
  -1 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-27 18:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, patchwork-lst, kernel, Fabio Estevam,
	Shawn Guo, linux-arm-kernel

Am Montag, den 23.01.2017, 10:26 -0600 schrieb Rob Herring:
> On Fri, Jan 20, 2017 at 04:52:22PM +0100, Lucas Stach wrote:
> > This adds a new binding for the Freescale i.MX GPC block, which allows
> > to describe multiple power domains in a more natural way. The driver
> > will continue to support the old binding for existing DTBs, but new
> > features like the additional domains present on i.MX6SX will only be
> > usable with the new binding.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  .../devicetree/bindings/power/fsl,imx-gpc.txt      | 81 ++++++++++++++--------
> >  1 file changed, 54 insertions(+), 27 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> > index 65cc0345747d..e5b660018d63 100644
> > --- a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> > +++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> > @@ -1,22 +1,41 @@
> >  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.
> > +The i.MX6 General Power Control (GPC) block contains DVFS load tracking
> > +counters and Power Gating Control (PGC).
> >  
> >  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:
> > +- interrupts: Should contain one interrupt specifier for the GPC interrupt
> > +- clocks: Must contain an entry for each entry in clock-names.
> > +  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
> > +- clock-names: Must include the following entries:
> > +  - ipg
> > +- pgc: a list of child nodes describing the SoC power domains controlled by the
> > +  power gating controller.
> 
> pgc is a node, not a property. Call it 'power-domains' instead.
> 
'power-domains' is commonly used to refer to the the domains from the
client devices. While it should be technically possible to have it as a
node in the GPC, I think this is confusing.
'pgc' is the name used in the reference manual for the sub-block of the
GPC that controls the power domains, so the current naming is actually a
pretty accurate DT description of the hardware organization.

> >  
> > -The gpc node is a power-controller as documented by the generic power domain
> > -bindings in Documentation/devicetree/bindings/power/power_domain.txt.
> > +The power domains are generic power domain providers as documented in
> > +Documentation/devicetree/bindings/power/power_domain.txt. They are described as
> > +subnodes of the GPC and should contain the following
> > +
> > +Required properties:
> > +- reg: the DOMAIN_INDEX as used by the client devices to refer to this
> > +  power domain
> > +  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
> > +
> > +- #power-domain-cells: Should be 0
> > +
> > +Optional properties:
> > +- clocks: a number of phandles to clocks that need to be enabled during domain
> > +  power-up sequencing to ensure reset propagation into devices located inside
> > +  this power domain
> > +- domain-supply: a phandle to the regulator powering this domain
> 
> 'power-supply' is more common for when we don't really have a name.
> 
Will change.

Regards,
Lucas 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 1/4] dt-bindings: add multidomain support to i.MX GPC DT binding
@ 2017-01-27 18:02         ` Lucas Stach
  0 siblings, 0 replies; 18+ messages in thread
From: Lucas Stach @ 2017-01-27 18:02 UTC (permalink / raw)
  To: linux-arm-kernel

Am Montag, den 23.01.2017, 10:26 -0600 schrieb Rob Herring:
> On Fri, Jan 20, 2017 at 04:52:22PM +0100, Lucas Stach wrote:
> > This adds a new binding for the Freescale i.MX GPC block, which allows
> > to describe multiple power domains in a more natural way. The driver
> > will continue to support the old binding for existing DTBs, but new
> > features like the additional domains present on i.MX6SX will only be
> > usable with the new binding.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  .../devicetree/bindings/power/fsl,imx-gpc.txt      | 81 ++++++++++++++--------
> >  1 file changed, 54 insertions(+), 27 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> > index 65cc0345747d..e5b660018d63 100644
> > --- a/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> > +++ b/Documentation/devicetree/bindings/power/fsl,imx-gpc.txt
> > @@ -1,22 +1,41 @@
> >  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.
> > +The i.MX6 General Power Control (GPC) block contains DVFS load tracking
> > +counters and Power Gating Control (PGC).
> >  
> >  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:
> > +- interrupts: Should contain one interrupt specifier for the GPC interrupt
> > +- clocks: Must contain an entry for each entry in clock-names.
> > +  See Documentation/devicetree/bindings/clocks/clock-bindings.txt for details.
> > +- clock-names: Must include the following entries:
> > +  - ipg
> > +- pgc: a list of child nodes describing the SoC power domains controlled by the
> > +  power gating controller.
> 
> pgc is a node, not a property. Call it 'power-domains' instead.
> 
'power-domains' is commonly used to refer to the the domains from the
client devices. While it should be technically possible to have it as a
node in the GPC, I think this is confusing.
'pgc' is the name used in the reference manual for the sub-block of the
GPC that controls the power domains, so the current naming is actually a
pretty accurate DT description of the hardware organization.

> >  
> > -The gpc node is a power-controller as documented by the generic power domain
> > -bindings in Documentation/devicetree/bindings/power/power_domain.txt.
> > +The power domains are generic power domain providers as documented in
> > +Documentation/devicetree/bindings/power/power_domain.txt. They are described as
> > +subnodes of the GPC and should contain the following
> > +
> > +Required properties:
> > +- reg: the DOMAIN_INDEX as used by the client devices to refer to this
> > +  power domain
> > +  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
> > +
> > +- #power-domain-cells: Should be 0
> > +
> > +Optional properties:
> > +- clocks: a number of phandles to clocks that need to be enabled during domain
> > +  power-up sequencing to ensure reset propagation into devices located inside
> > +  this power domain
> > +- domain-supply: a phandle to the regulator powering this domain
> 
> 'power-supply' is more common for when we don't really have a name.
> 
Will change.

Regards,
Lucas 

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-01-27 18:02 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-20 15:52 [PATCH 0/4] i.MX6 power gating controller rework Lucas Stach
2017-01-20 15:52 ` Lucas Stach
     [not found] ` <20170120155225.31905-1-l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2017-01-20 15:52   ` [PATCH 1/4] dt-bindings: add multidomain support to i.MX GPC DT binding Lucas Stach
2017-01-20 15:52     ` Lucas Stach
2017-01-23 16:26     ` Rob Herring
2017-01-23 16:26       ` Rob Herring
2017-01-27 18:02       ` Lucas Stach
2017-01-27 18:02         ` Lucas Stach
2017-01-20 15:52   ` [PATCH 2/4] ARM: imx6: remove PGC handling from GPC driver Lucas Stach
2017-01-20 15:52     ` Lucas Stach
2017-01-20 15:52   ` [PATCH 3/4] soc/imx: add new " Lucas Stach
2017-01-20 15:52     ` Lucas Stach
     [not found]     ` <20170120155225.31905-4-l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2017-01-24 12:49       ` Shawn Guo
2017-01-24 12:49         ` Shawn Guo
2017-01-24 16:54         ` Lucas Stach
2017-01-24 16:54           ` Lucas Stach
2017-01-20 15:52   ` [PATCH 4/4] ARM: imx6: adopt DT to new GPC binding Lucas Stach
2017-01-20 15:52     ` Lucas Stach

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.