[PATCH 00/16] soc: imx: gpcv2: support i.MX8MM

[PATCH 00/16] soc: imx: gpcv2: support i.MX8MM

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several
minor changes from me to make it could work with i.MX BLK-CTL driver.

Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting
all the patches, Jacky Bai on help debug issues.

Lucas Stach (12):
  soc: imx: gpcv2: move to more ideomatic error handling in probe
  soc: imx: gpcv2: move domain mapping to domain driver probe
  soc: imx: gpcv2: switch to clk_bulk_* API
  soc: imx: gpcv2: split power up and power down sequence control
  soc: imx: gpcv2: wait for ADB400 handshake
  soc: imx: gpcv2: add runtime PM support for power-domains
  soc: imx: gpcv2: allow domains without power-sequence control
  dt-bindings: imx: gpcv2: add support for optional resets
  soc: imx: gpcv2: add support for optional resets
  dt-bindings: power: add defines for i.MX8MM power domains
  soc: imx: gpcv2: add support for i.MX8MM power domains
  soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power
    domains

Peng Fan (4):
  soc: imx: gpcv2: correct pm_runtime_get_sync usage
  soc: imx: gpcv2: move reset assert after requesting domain power up
  soc: imx: gpcv2: support reset defer probe
  soc: imx: gpcv2: remove waiting handshake in power up

 .../bindings/power/fsl,imx-gpcv2.yaml         |   9 +
 drivers/soc/imx/gpcv2.c                       | 534 ++++++++++++++----
 include/dt-bindings/power/imx8mm-power.h      |  22 +
 3 files changed, 450 insertions(+), 115 deletions(-)
 create mode 100644 include/dt-bindings/power/imx8mm-power.h

-- 
2.30.0

[PATCH 01/16] soc: imx: gpcv2: move to more ideomatic error handling in probe

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

Switch to "goto out..." error handling in domain driver probe to
avoid repeating all the error paths.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Marek Vasut <marex@denx.de>
Tested-by: Adam Ford <aford173@gmail.com>
---
 drivers/soc/imx/gpcv2.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index db7e7fc321b1..512e6f4acafd 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -502,18 +502,23 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 	ret = pm_genpd_init(&domain->genpd, NULL, true);
 	if (ret) {
 		dev_err(domain->dev, "Failed to init power domain\n");
-		imx_pgc_put_clocks(domain);
-		return ret;
+		goto out_put_clocks;
 	}
 
 	ret = of_genpd_add_provider_simple(domain->dev->of_node,
 					   &domain->genpd);
 	if (ret) {
 		dev_err(domain->dev, "Failed to add genpd provider\n");
-		pm_genpd_remove(&domain->genpd);
-		imx_pgc_put_clocks(domain);
+		goto out_genpd_remove;
 	}
 
+	return 0;
+
+out_genpd_remove:
+	pm_genpd_remove(&domain->genpd);
+out_put_clocks:
+	imx_pgc_put_clocks(domain);
+
 	return ret;
 }
 
-- 
2.30.0

[PATCH 02/16] soc: imx: gpcv2: move domain mapping to domain driver probe

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

As long as the power domain driver is active we want power control
over the domain (which is what the mapping bit requests), so there
is no point in whacking it for every power control action, simply
set the bit in driver probe and clear it when the driver is removed.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/imx/gpcv2.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 512e6f4acafd..552d3e6bee52 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -140,14 +140,11 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
 	int i, ret = 0;
 	u32 pxx_req;
 
-	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
-			   domain->bits.map, domain->bits.map);
-
 	if (has_regulator && on) {
 		ret = regulator_enable(domain->regulator);
 		if (ret) {
 			dev_err(domain->dev, "failed to enable regulator\n");
-			goto unmap;
+			return ret;
 		}
 	}
 
@@ -203,9 +200,7 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
 		/* Preserve earlier error code */
 		ret = ret ?: err;
 	}
-unmap:
-	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
-			   domain->bits.map, 0);
+
 	return ret;
 }
 
@@ -499,10 +494,13 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(domain->dev, ret, "Failed to get domain's clocks\n");
 
+	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
+			   domain->bits.map, domain->bits.map);
+
 	ret = pm_genpd_init(&domain->genpd, NULL, true);
 	if (ret) {
 		dev_err(domain->dev, "Failed to init power domain\n");
-		goto out_put_clocks;
+		goto out_domain_unmap;
 	}
 
 	ret = of_genpd_add_provider_simple(domain->dev->of_node,
@@ -516,7 +514,9 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 
 out_genpd_remove:
 	pm_genpd_remove(&domain->genpd);
-out_put_clocks:
+out_domain_unmap:
+	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
+			   domain->bits.map, 0);
 	imx_pgc_put_clocks(domain);
 
 	return ret;
@@ -528,6 +528,10 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
 
 	of_genpd_del_provider(domain->dev->of_node);
 	pm_genpd_remove(&domain->genpd);
+
+	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
+			   domain->bits.map, 0);
+
 	imx_pgc_put_clocks(domain);
 
 	return 0;
-- 
2.30.0

[PATCH 03/16] soc: imx: gpcv2: switch to clk_bulk_* API

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

Use clk_bulk API to simplify the code a bit. Also add some error
checking to the clk_prepare_enable calls.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/imx/gpcv2.c | 60 +++++++++--------------------------------
 1 file changed, 12 insertions(+), 48 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 552d3e6bee52..1d90c7802972 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -100,13 +100,11 @@
 
 #define GPC_PGC_CTRL_PCR		BIT(0)
 
-#define GPC_CLK_MAX		6
-
 struct imx_pgc_domain {
 	struct generic_pm_domain genpd;
 	struct regmap *regmap;
 	struct regulator *regulator;
-	struct clk *clk[GPC_CLK_MAX];
+	struct clk_bulk_data *clks;
 	int num_clks;
 
 	unsigned int pgc;
@@ -149,8 +147,12 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
 	}
 
 	/* Enable reset clocks for all devices in the domain */
-	for (i = 0; i < domain->num_clks; i++)
-		clk_prepare_enable(domain->clk[i]);
+	clk_bulk_prepare_enable(domain->num_clks, domain->clks);
+	if (ret) {
+		dev_err(domain->dev, "failed to enable reset clocks\n");
+		regulator_disable(domain->regulator);
+		return ret;
+	}
 
 	if (enable_power_control)
 		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
@@ -187,8 +189,7 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
 				   GPC_PGC_CTRL_PCR, 0);
 
 	/* Disable reset clocks for all devices in the domain */
-	for (i = 0; i < domain->num_clks; i++)
-		clk_disable_unprepare(domain->clk[i]);
+	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
 
 	if (has_regulator && !on) {
 		int err;
@@ -438,41 +439,6 @@ static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
 	.reg_access_table = &imx8m_access_table,
 };
 
-static int imx_pgc_get_clocks(struct imx_pgc_domain *domain)
-{
-	int i, ret;
-
-	for (i = 0; ; i++) {
-		struct clk *clk = of_clk_get(domain->dev->of_node, i);
-		if (IS_ERR(clk))
-			break;
-		if (i >= GPC_CLK_MAX) {
-			dev_err(domain->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:
-	while (i--)
-		clk_put(domain->clk[i]);
-
-	return ret;
-}
-
-static void imx_pgc_put_clocks(struct imx_pgc_domain *domain)
-{
-	int i;
-
-	for (i = domain->num_clks - 1; i >= 0; i--)
-		clk_put(domain->clk[i]);
-}
-
 static int imx_pgc_domain_probe(struct platform_device *pdev)
 {
 	struct imx_pgc_domain *domain = pdev->dev.platform_data;
@@ -490,9 +456,10 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 				      domain->voltage, domain->voltage);
 	}
 
-	ret = imx_pgc_get_clocks(domain);
-	if (ret)
-		return dev_err_probe(domain->dev, ret, "Failed to get domain's clocks\n");
+	domain->num_clks = devm_clk_bulk_get_all(domain->dev, &domain->clks);
+	if (domain->num_clks < 0)
+		return dev_err_probe(domain->dev, domain->num_clks,
+				     "Failed to get domain's clocks\n");
 
 	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
 			   domain->bits.map, domain->bits.map);
@@ -517,7 +484,6 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 out_domain_unmap:
 	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
 			   domain->bits.map, 0);
-	imx_pgc_put_clocks(domain);
 
 	return ret;
 }
@@ -532,8 +498,6 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
 	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
 			   domain->bits.map, 0);
 
-	imx_pgc_put_clocks(domain);
-
 	return 0;
 }
 
-- 
2.30.0

[PATCH 04/16] soc: imx: gpcv2: split power up and power down sequence control

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

The current mixed function to control both power up and power down
sequences is very hard to follow and already contains some sequence
errors like triggering the ADB400 handshake at the wrong time due to
this. Split the function into two, which results in slightly more
code, but is way easier to get right.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/imx/gpcv2.c | 141 ++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 55 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 1d90c7802972..7356e48ebdad 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -125,20 +125,19 @@ struct imx_pgc_domain_data {
 	const struct regmap_access_table *reg_access_table;
 };
 
-static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
-				      bool on)
+static inline struct imx_pgc_domain *
+to_imx_pgc_domain(struct generic_pm_domain *genpd)
 {
-	struct imx_pgc_domain *domain = container_of(genpd,
-						      struct imx_pgc_domain,
-						      genpd);
-	unsigned int offset = on ?
-		GPC_PU_PGC_SW_PUP_REQ : GPC_PU_PGC_SW_PDN_REQ;
-	const bool enable_power_control = !on;
-	const bool has_regulator = !IS_ERR(domain->regulator);
-	int i, ret = 0;
-	u32 pxx_req;
-
-	if (has_regulator && on) {
+	return container_of(genpd, struct imx_pgc_domain, genpd);
+}
+
+static int imx_pgc_power_up(struct generic_pm_domain *genpd)
+{
+	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
+	u32 reg_val;
+	int ret;
+
+	if (!IS_ERR(domain->regulator)) {
 		ret = regulator_enable(domain->regulator);
 		if (ret) {
 			dev_err(domain->dev, "failed to enable regulator\n");
@@ -147,72 +146,104 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
 	}
 
 	/* Enable reset clocks for all devices in the domain */
-	clk_bulk_prepare_enable(domain->num_clks, domain->clks);
+	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
 	if (ret) {
 		dev_err(domain->dev, "failed to enable reset clocks\n");
+		goto out_regulator_disable;
+	}
+
+	/* request the domain to power up */
+	regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
+			   domain->bits.pxx, domain->bits.pxx);
+	/*
+	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
+	 * for PUP_REQ/PDN_REQ bit to be cleared
+	 */
+	ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
+				       reg_val, !(reg_val & domain->bits.pxx),
+				       0, USEC_PER_MSEC);
+	if (ret) {
+		dev_err(domain->dev, "failed to command PGC\n");
+		goto out_clk_disable;
+	}
+
+	/* disable power control */
+	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
+			   GPC_PGC_CTRL_PCR, 0);
+
+	/* request the ADB400 to power up */
+	if (domain->bits.hsk)
+		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
+				   domain->bits.hsk, domain->bits.hsk);
+
+	/* Disable reset clocks for all devices in the domain */
+	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
+
+	return 0;
+
+out_clk_disable:
+	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
+out_regulator_disable:
+	if (!IS_ERR(domain->regulator))
 		regulator_disable(domain->regulator);
+
+	return ret;
+}
+
+static int imx_pgc_power_down(struct generic_pm_domain *genpd)
+{
+	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
+	u32 reg_val;
+	int ret;
+
+	/* Enable reset clocks for all devices in the domain */
+	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
+	if (ret) {
+		dev_err(domain->dev, "failed to enable reset clocks\n");
 		return ret;
 	}
 
-	if (enable_power_control)
-		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-				   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
-
+	/* request the ADB400 to power down */
 	if (domain->bits.hsk)
 		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
-				   domain->bits.hsk, on ? domain->bits.hsk : 0);
+				   domain->bits.hsk, 0);
 
-	regmap_update_bits(domain->regmap, offset,
-			   domain->bits.pxx, domain->bits.pxx);
+	/* enable power control */
+	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
+			   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
 
+	/* request the domain to power down */
+	regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
+			   domain->bits.pxx, domain->bits.pxx);
 	/*
 	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
 	 * for PUP_REQ/PDN_REQ bit to be cleared
 	 */
-	ret = regmap_read_poll_timeout(domain->regmap, offset, pxx_req,
-				       !(pxx_req & domain->bits.pxx),
+	ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
+				       reg_val, !(reg_val & domain->bits.pxx),
 				       0, USEC_PER_MSEC);
 	if (ret) {
 		dev_err(domain->dev, "failed to command PGC\n");
-		/*
-		 * If we were in a process of enabling a
-		 * domain and failed we might as well disable
-		 * the regulator we just enabled. And if it
-		 * was the opposite situation and we failed to
-		 * power down -- keep the regulator on
-		 */
-		on = !on;
+		goto out_clk_disable;
 	}
 
-	if (enable_power_control)
-		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-				   GPC_PGC_CTRL_PCR, 0);
-
 	/* Disable reset clocks for all devices in the domain */
 	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
 
-	if (has_regulator && !on) {
-		int err;
-
-		err = regulator_disable(domain->regulator);
-		if (err)
-			dev_err(domain->dev,
-				"failed to disable regulator: %d\n", err);
-		/* Preserve earlier error code */
-		ret = ret ?: err;
+	if (!IS_ERR(domain->regulator)) {
+		ret = regulator_disable(domain->regulator);
+		if (ret) {
+			dev_err(domain->dev, "failed to disable regulator\n");
+			return ret;
+		}
 	}
 
-	return ret;
-}
+	return 0;
 
-static int imx_gpc_pu_pgc_sw_pup_req(struct generic_pm_domain *genpd)
-{
-	return imx_gpc_pu_pgc_sw_pxx_req(genpd, true);
-}
+out_clk_disable:
+	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
 
-static int imx_gpc_pu_pgc_sw_pdn_req(struct generic_pm_domain *genpd)
-{
-	return imx_gpc_pu_pgc_sw_pxx_req(genpd, false);
+	return ret;
 }
 
 static const struct imx_pgc_domain imx7_pgc_domains[] = {
@@ -590,8 +621,8 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
 
 		domain = pd_pdev->dev.platform_data;
 		domain->regmap = regmap;
-		domain->genpd.power_on  = imx_gpc_pu_pgc_sw_pup_req;
-		domain->genpd.power_off = imx_gpc_pu_pgc_sw_pdn_req;
+		domain->genpd.power_on  = imx_pgc_power_up;
+		domain->genpd.power_off = imx_pgc_power_down;
 
 		pd_pdev->dev.parent = dev;
 		pd_pdev->dev.of_node = np;
-- 
2.30.0

[PATCH 05/16] soc: imx: gpcv2: wait for ADB400 handshake

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

New reference manuals show that there is actually a status bit for
the ADB400 handshake. Add a poll loop to wait for the ADB400 to
acknowledge our request.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/imx/gpcv2.c | 43 +++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 7356e48ebdad..d27025e37a9e 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -69,6 +69,9 @@
 
 #define GPC_PU_PWRHSK			0x1fc
 
+#define IMX8M_GPU_HSK_PWRDNACKN			BIT(26)
+#define IMX8M_VPU_HSK_PWRDNACKN			BIT(25)
+#define IMX8M_DISP_HSK_PWRDNACKN		BIT(24)
 #define IMX8M_GPU_HSK_PWRDNREQN			BIT(6)
 #define IMX8M_VPU_HSK_PWRDNREQN			BIT(5)
 #define IMX8M_DISP_HSK_PWRDNREQN		BIT(4)
@@ -112,7 +115,8 @@ struct imx_pgc_domain {
 	const struct {
 		u32 pxx;
 		u32 map;
-		u32 hsk;
+		u32 hskreq;
+		u32 hskack;
 	} bits;
 
 	const int voltage;
@@ -172,9 +176,19 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 			   GPC_PGC_CTRL_PCR, 0);
 
 	/* request the ADB400 to power up */
-	if (domain->bits.hsk)
+	if (domain->bits.hskreq) {
 		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
-				   domain->bits.hsk, domain->bits.hsk);
+				   domain->bits.hskreq, domain->bits.hskreq);
+
+		ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
+					       reg_val,
+					       (reg_val & domain->bits.hskack),
+					       0, USEC_PER_MSEC);
+		if (ret) {
+			dev_err(domain->dev, "failed to power up ADB400\n");
+			goto out_clk_disable;
+		}
+	}
 
 	/* Disable reset clocks for all devices in the domain */
 	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
@@ -204,9 +218,19 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 	}
 
 	/* request the ADB400 to power down */
-	if (domain->bits.hsk)
+	if (domain->bits.hskreq) {
 		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
-				   domain->bits.hsk, 0);
+				   domain->bits.hskreq, 0);
+
+		ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
+					       reg_val,
+					       !(reg_val & domain->bits.hskack),
+					       0, USEC_PER_MSEC);
+		if (ret) {
+			dev_err(domain->dev, "failed to power down ADB400\n");
+			goto out_clk_disable;
+		}
+	}
 
 	/* enable power control */
 	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
@@ -369,7 +393,8 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 		.bits  = {
 			.pxx = IMX8M_GPU_SW_Pxx_REQ,
 			.map = IMX8M_GPU_A53_DOMAIN,
-			.hsk = IMX8M_GPU_HSK_PWRDNREQN,
+			.hskreq = IMX8M_GPU_HSK_PWRDNREQN,
+			.hskack = IMX8M_GPU_HSK_PWRDNACKN,
 		},
 		.pgc   = IMX8M_PGC_GPU,
 	},
@@ -381,7 +406,8 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 		.bits  = {
 			.pxx = IMX8M_VPU_SW_Pxx_REQ,
 			.map = IMX8M_VPU_A53_DOMAIN,
-			.hsk = IMX8M_VPU_HSK_PWRDNREQN,
+			.hskreq = IMX8M_VPU_HSK_PWRDNREQN,
+			.hskack = IMX8M_VPU_HSK_PWRDNACKN,
 		},
 		.pgc   = IMX8M_PGC_VPU,
 	},
@@ -393,7 +419,8 @@ static const struct imx_pgc_domain imx8m_pgc_domains[] = {
 		.bits  = {
 			.pxx = IMX8M_DISP_SW_Pxx_REQ,
 			.map = IMX8M_DISP_A53_DOMAIN,
-			.hsk = IMX8M_DISP_HSK_PWRDNREQN,
+			.hskreq = IMX8M_DISP_HSK_PWRDNREQN,
+			.hskack = IMX8M_DISP_HSK_PWRDNACKN,
 		},
 		.pgc   = IMX8M_PGC_DISP,
 	},
-- 
2.30.0

[PATCH 06/16] soc: imx: gpcv2: add runtime PM support for power-domains

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

This allows to nest domains into other power domains and have the
parent domain powered up/down as required by the child domains.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/imx/gpcv2.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index d27025e37a9e..87165619a689 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -12,6 +12,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/sizes.h>
@@ -141,11 +142,17 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 	u32 reg_val;
 	int ret;
 
+	ret = pm_runtime_get_sync(domain->dev);
+	if (ret) {
+		pm_runtime_put_noidle(domain->dev);
+		return ret;
+	}
+
 	if (!IS_ERR(domain->regulator)) {
 		ret = regulator_enable(domain->regulator);
 		if (ret) {
 			dev_err(domain->dev, "failed to enable regulator\n");
-			return ret;
+			goto out_put_pm;
 		}
 	}
 
@@ -200,6 +207,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 out_regulator_disable:
 	if (!IS_ERR(domain->regulator))
 		regulator_disable(domain->regulator);
+out_put_pm:
+	pm_runtime_put(domain->dev);
 
 	return ret;
 }
@@ -262,6 +271,8 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 		}
 	}
 
+	pm_runtime_put(domain->dev);
+
 	return 0;
 
 out_clk_disable:
@@ -519,6 +530,8 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 		return dev_err_probe(domain->dev, domain->num_clks,
 				     "Failed to get domain's clocks\n");
 
+	pm_runtime_enable(domain->dev);
+
 	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
 			   domain->bits.map, domain->bits.map);
 
@@ -542,6 +555,7 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 out_domain_unmap:
 	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
 			   domain->bits.map, 0);
+	pm_runtime_disable(domain->dev);
 
 	return ret;
 }
@@ -556,6 +570,8 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
 	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
 			   domain->bits.map, 0);
 
+	pm_runtime_disable(domain->dev);
+
 	return 0;
 }
 
-- 
2.30.0

[PATCH 07/16] soc: imx: gpcv2: allow domains without power-sequence control

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

Some of the PGC domains only control the handshake with the ADB400
and don't have any power sequence controls. Make such domains work
by allowing the pxx and map bits to be empty and skip all actions
using those controls.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/imx/gpcv2.c | 89 +++++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 40 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 87165619a689..640f4165cfba 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -163,24 +163,27 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		goto out_regulator_disable;
 	}
 
-	/* request the domain to power up */
-	regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
-			   domain->bits.pxx, domain->bits.pxx);
-	/*
-	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
-	 * for PUP_REQ/PDN_REQ bit to be cleared
-	 */
-	ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
-				       reg_val, !(reg_val & domain->bits.pxx),
-				       0, USEC_PER_MSEC);
-	if (ret) {
-		dev_err(domain->dev, "failed to command PGC\n");
-		goto out_clk_disable;
-	}
+	if (domain->bits.pxx) {
+		/* request the domain to power up */
+		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
+				   domain->bits.pxx, domain->bits.pxx);
+		/*
+		 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
+		 * for PUP_REQ/PDN_REQ bit to be cleared
+		 */
+		ret = regmap_read_poll_timeout(domain->regmap,
+					       GPC_PU_PGC_SW_PUP_REQ, reg_val,
+					       !(reg_val & domain->bits.pxx),
+					       0, USEC_PER_MSEC);
+		if (ret) {
+			dev_err(domain->dev, "failed to command PGC\n");
+			goto out_clk_disable;
+		}
 
-	/* disable power control */
-	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-			   GPC_PGC_CTRL_PCR, 0);
+		/* disable power control */
+		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
+				   GPC_PGC_CTRL_PCR, 0);
+	}
 
 	/* request the ADB400 to power up */
 	if (domain->bits.hskreq) {
@@ -241,23 +244,26 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
 		}
 	}
 
-	/* enable power control */
-	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-			   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
-
-	/* request the domain to power down */
-	regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
-			   domain->bits.pxx, domain->bits.pxx);
-	/*
-	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
-	 * for PUP_REQ/PDN_REQ bit to be cleared
-	 */
-	ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
-				       reg_val, !(reg_val & domain->bits.pxx),
-				       0, USEC_PER_MSEC);
-	if (ret) {
-		dev_err(domain->dev, "failed to command PGC\n");
-		goto out_clk_disable;
+	if (domain->bits.pxx) {
+		/* enable power control */
+		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
+				   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
+
+		/* request the domain to power down */
+		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
+				   domain->bits.pxx, domain->bits.pxx);
+		/*
+		 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
+		 * for PUP_REQ/PDN_REQ bit to be cleared
+		 */
+		ret = regmap_read_poll_timeout(domain->regmap,
+					       GPC_PU_PGC_SW_PDN_REQ, reg_val,
+					       !(reg_val & domain->bits.pxx),
+					       0, USEC_PER_MSEC);
+		if (ret) {
+			dev_err(domain->dev, "failed to command PGC\n");
+			goto out_clk_disable;
+		}
 	}
 
 	/* Disable reset clocks for all devices in the domain */
@@ -532,8 +538,9 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(domain->dev);
 
-	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
-			   domain->bits.map, domain->bits.map);
+	if (domain->bits.map)
+		regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
+				   domain->bits.map, domain->bits.map);
 
 	ret = pm_genpd_init(&domain->genpd, NULL, true);
 	if (ret) {
@@ -553,8 +560,9 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 out_genpd_remove:
 	pm_genpd_remove(&domain->genpd);
 out_domain_unmap:
-	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
-			   domain->bits.map, 0);
+	if (domain->bits.map)
+		regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
+				   domain->bits.map, 0);
 	pm_runtime_disable(domain->dev);
 
 	return ret;
@@ -567,8 +575,9 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
 	of_genpd_del_provider(domain->dev->of_node);
 	pm_genpd_remove(&domain->genpd);
 
-	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
-			   domain->bits.map, 0);
+	if (domain->bits.map)
+		regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
+				   domain->bits.map, 0);
 
 	pm_runtime_disable(domain->dev);
 
-- 
2.30.0

[PATCH 08/16] dt-bindings: imx: gpcv2: add support for optional resets

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

For some domains the resets of the devices in the domain are not
automatically triggered. Add an optional resets property to allow
the GPC driver to trigger those resets explicitly.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
index a96e6dbf1858..4330c73a2c30 100644
--- a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
+++ b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
@@ -66,6 +66,13 @@ properties:
 
           power-supply: true
 
+          resets:
+            description: |
+              A number of phandles to resets that need to be asserted during
+              power-up sequencing of the domain.
+            minItems: 1
+            maxItems: 4
+
         required:
           - '#power-domain-cells'
           - reg
-- 
2.30.0

[PATCH 09/16] soc: imx: gpcv2: add support for optional resets

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

Normally the reset for the devices inside the power domain is
triggered automatically from the PGC in the power-up sequencing,
however on i.MX8MM this doesn't work for the GPU power domains.

Add support for triggering the reset explicitly during the power
up sequencing.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/imx/gpcv2.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 640f4165cfba..4a2c2a255d1a 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -15,6 +15,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 #include <linux/sizes.h>
 #include <dt-bindings/power/imx7-power.h>
 #include <dt-bindings/power/imx8mq-power.h>
@@ -108,6 +109,7 @@ struct imx_pgc_domain {
 	struct generic_pm_domain genpd;
 	struct regmap *regmap;
 	struct regulator *regulator;
+	struct reset_control *reset;
 	struct clk_bulk_data *clks;
 	int num_clks;
 
@@ -163,6 +165,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		goto out_regulator_disable;
 	}
 
+	reset_control_assert(domain->reset);
+
 	if (domain->bits.pxx) {
 		/* request the domain to power up */
 		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
@@ -185,6 +189,11 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 				   GPC_PGC_CTRL_PCR, 0);
 	}
 
+	/* delay for reset to propagate */
+	udelay(5);
+
+	reset_control_deassert(domain->reset);
+
 	/* request the ADB400 to power up */
 	if (domain->bits.hskreq) {
 		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
@@ -531,11 +540,17 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 				      domain->voltage, domain->voltage);
 	}
 
+
 	domain->num_clks = devm_clk_bulk_get_all(domain->dev, &domain->clks);
 	if (domain->num_clks < 0)
 		return dev_err_probe(domain->dev, domain->num_clks,
 				     "Failed to get domain's clocks\n");
 
+	domain->reset = devm_reset_control_array_get_optional_exclusive(domain->dev);
+	if (IS_ERR(domain->reset))
+		return dev_err_probe(domain->dev, PTR_ERR(domain->reset),
+				     "Failed to get domain's resets\n");
+
 	pm_runtime_enable(domain->dev);
 
 	if (domain->bits.map)
-- 
2.30.0

[PATCH 10/16] dt-bindings: power: add defines for i.MX8MM power domains

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

Adding defines for i.MX8MM GPC power domains.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../bindings/power/fsl,imx-gpcv2.yaml         |  2 ++
 include/dt-bindings/power/imx8mm-power.h      | 22 +++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 include/dt-bindings/power/imx8mm-power.h

diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
index 4330c73a2c30..d3539569d45f 100644
--- a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
+++ b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
@@ -26,6 +26,7 @@ properties:
     enum:
       - fsl,imx7d-gpc
       - fsl,imx8mq-gpc
+      - fsl,imx8mm-gpc
 
   reg:
     maxItems: 1
@@ -54,6 +55,7 @@ properties:
               Power domain index. Valid values are defined in
               include/dt-bindings/power/imx7-power.h for fsl,imx7d-gpc and
               include/dt-bindings/power/imx8m-power.h for fsl,imx8mq-gpc
+              include/dt-bindings/power/imx8mm-power.h for fsl,imx8mm-gpc
             maxItems: 1
 
           clocks:
diff --git a/include/dt-bindings/power/imx8mm-power.h b/include/dt-bindings/power/imx8mm-power.h
new file mode 100644
index 000000000000..fc9c2e16aadc
--- /dev/null
+++ b/include/dt-bindings/power/imx8mm-power.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ *  Copyright (C) 2020 Pengutronix, Lucas Stach <kernel@pengutronix.de>
+ */
+
+#ifndef __DT_BINDINGS_IMX8MM_POWER_H__
+#define __DT_BINDINGS_IMX8MM_POWER_H__
+
+#define IMX8MM_POWER_DOMAIN_HSIOMIX	0
+#define IMX8MM_POWER_DOMAIN_PCIE	1
+#define IMX8MM_POWER_DOMAIN_OTG1	2
+#define IMX8MM_POWER_DOMAIN_OTG2	3
+#define IMX8MM_POWER_DOMAIN_GPUMIX	4
+#define IMX8MM_POWER_DOMAIN_GPU		5
+#define IMX8MM_POWER_DOMAIN_VPUMIX	6
+#define IMX8MM_POWER_DOMAIN_VPUG1	7
+#define IMX8MM_POWER_DOMAIN_VPUG2	8
+#define IMX8MM_POWER_DOMAIN_VPUH1	9
+#define IMX8MM_POWER_DOMAIN_DISPMIX	10
+#define IMX8MM_POWER_DOMAIN_MIPI	11
+
+#endif
-- 
2.30.0

[PATCH 11/16] soc: imx: gpcv2: add support for i.MX8MM power domains

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

This adds support for the power domains founds on i.MX8MM. The 2D and 3D
GPU domains are abstracted as a single domain in the driver, as they can't
be powered up/down individually due to a shared reset.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/soc/imx/gpcv2.c | 168 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 4a2c2a255d1a..5642dd236c10 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -19,6 +19,7 @@
 #include <linux/sizes.h>
 #include <dt-bindings/power/imx7-power.h>
 #include <dt-bindings/power/imx8mq-power.h>
+#include <dt-bindings/power/imx8mm-power.h>
 
 #define GPC_LPCR_A_CORE_BSC			0x000
 
@@ -44,6 +45,19 @@
 #define IMX8M_PCIE1_A53_DOMAIN			BIT(3)
 #define IMX8M_MIPI_A53_DOMAIN			BIT(2)
 
+#define IMX8MM_VPUH1_A53_DOMAIN			BIT(15)
+#define IMX8MM_VPUG2_A53_DOMAIN			BIT(14)
+#define IMX8MM_VPUG1_A53_DOMAIN			BIT(13)
+#define IMX8MM_DISPMIX_A53_DOMAIN		BIT(12)
+#define IMX8MM_VPUMIX_A53_DOMAIN		BIT(10)
+#define IMX8MM_GPUMIX_A53_DOMAIN		BIT(9)
+#define IMX8MM_GPU_A53_DOMAIN			(BIT(8) | BIT(11))
+#define IMX8MM_DDR1_A53_DOMAIN			BIT(7)
+#define IMX8MM_OTG2_A53_DOMAIN			BIT(5)
+#define IMX8MM_OTG1_A53_DOMAIN			BIT(4)
+#define IMX8MM_PCIE_A53_DOMAIN			BIT(3)
+#define IMX8MM_MIPI_A53_DOMAIN			BIT(2)
+
 #define GPC_PU_PGC_SW_PUP_REQ		0x0f8
 #define GPC_PU_PGC_SW_PDN_REQ		0x104
 
@@ -67,6 +81,19 @@
 #define IMX8M_PCIE1_SW_Pxx_REQ			BIT(1)
 #define IMX8M_MIPI_SW_Pxx_REQ			BIT(0)
 
+#define IMX8MM_VPUH1_SW_Pxx_REQ			BIT(13)
+#define IMX8MM_VPUG2_SW_Pxx_REQ			BIT(12)
+#define IMX8MM_VPUG1_SW_Pxx_REQ			BIT(11)
+#define IMX8MM_DISPMIX_SW_Pxx_REQ		BIT(10)
+#define IMX8MM_VPUMIX_SW_Pxx_REQ		BIT(8)
+#define IMX8MM_GPUMIX_SW_Pxx_REQ		BIT(7)
+#define IMX8MM_GPU_SW_Pxx_REQ			(BIT(6) | BIT(9))
+#define IMX8MM_DDR1_SW_Pxx_REQ			BIT(5)
+#define IMX8MM_OTG2_SW_Pxx_REQ			BIT(3)
+#define IMX8MM_OTG1_SW_Pxx_REQ			BIT(2)
+#define IMX8MM_PCIE_SW_Pxx_REQ			BIT(1)
+#define IMX8MM_MIPI_SW_Pxx_REQ			BIT(0)
+
 #define GPC_M4_PU_PDN_FLG		0x1bc
 
 #define GPC_PU_PWRHSK			0x1fc
@@ -78,6 +105,17 @@
 #define IMX8M_VPU_HSK_PWRDNREQN			BIT(5)
 #define IMX8M_DISP_HSK_PWRDNREQN		BIT(4)
 
+
+#define IMX8MM_GPUMIX_HSK_PWRDNACKN		BIT(29)
+#define IMX8MM_GPU_HSK_PWRDNACKN		(BIT(27) | BIT(28))
+#define IMX8MM_VPUMIX_HSK_PWRDNACKN		BIT(26)
+#define IMX8MM_DISPMIX_HSK_PWRDNACKN		BIT(25)
+#define IMX8MM_HSIO_HSK_PWRDNACKN		(BIT(23) | BIT(24))
+#define IMX8MM_GPUMIX_HSK_PWRDNREQN		BIT(11)
+#define IMX8MM_GPU_HSK_PWRDNREQN		(BIT(9) | BIT(10))
+#define IMX8MM_VPUMIX_HSK_PWRDNREQN		BIT(8)
+#define IMX8MM_DISPMIX_HSK_PWRDNREQN		BIT(7)
+#define IMX8MM_HSIO_HSK_PWRDNREQN		(BIT(5) | BIT(6))
 /*
  * The PGC offset values in Reference Manual
  * (Rev. 1, 01/2018 and the older ones) GPC chapter's
@@ -100,6 +138,20 @@
 #define IMX8M_PGC_MIPI_CSI2		28
 #define IMX8M_PGC_PCIE2			29
 
+#define IMX8MM_PGC_MIPI			16
+#define IMX8MM_PGC_PCIE			17
+#define IMX8MM_PGC_OTG1			18
+#define IMX8MM_PGC_OTG2			19
+#define IMX8MM_PGC_DDR1			21
+#define IMX8MM_PGC_GPU2D		22
+#define IMX8MM_PGC_GPUMIX		23
+#define IMX8MM_PGC_VPUMIX		24
+#define IMX8MM_PGC_GPU3D		25
+#define IMX8MM_PGC_DISPMIX		26
+#define IMX8MM_PGC_VPUG1		27
+#define IMX8MM_PGC_VPUG2		28
+#define IMX8MM_PGC_VPUH1		29
+
 #define GPC_PGC_CTRL(n)			(0x800 + (n) * 0x40)
 #define GPC_PGC_SR(n)			(GPC_PGC_CTRL(n) + 0xc)
 
@@ -523,6 +575,121 @@ static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
 	.reg_access_table = &imx8m_access_table,
 };
 
+static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
+	[IMX8MM_POWER_DOMAIN_HSIOMIX] = {
+		.genpd = {
+			.name = "hsiomix",
+		},
+		.bits  = {
+			.pxx = 0, /* no power sequence control */
+			.map = 0, /* no power sequence control */
+			.hskreq = IMX8MM_HSIO_HSK_PWRDNREQN,
+			.hskack = IMX8MM_HSIO_HSK_PWRDNACKN,
+		},
+	},
+
+	[IMX8MM_POWER_DOMAIN_PCIE] = {
+		.genpd = {
+			.name = "pcie",
+		},
+		.bits  = {
+			.pxx = IMX8MM_PCIE_SW_Pxx_REQ,
+			.map = IMX8MM_PCIE_A53_DOMAIN,
+		},
+		.pgc   = IMX8MM_PGC_PCIE,
+	},
+
+	[IMX8MM_POWER_DOMAIN_OTG1] = {
+		.genpd = {
+			.name = "usb-otg1",
+		},
+		.bits  = {
+			.pxx = IMX8MM_OTG1_SW_Pxx_REQ,
+			.map = IMX8MM_OTG1_A53_DOMAIN,
+		},
+		.pgc   = IMX8MM_PGC_OTG1,
+	},
+
+	[IMX8MM_POWER_DOMAIN_OTG2] = {
+		.genpd = {
+			.name = "usb-otg2",
+		},
+		.bits  = {
+			.pxx = IMX8MM_OTG2_SW_Pxx_REQ,
+			.map = IMX8MM_OTG2_A53_DOMAIN,
+		},
+		.pgc   = IMX8MM_PGC_OTG2,
+	},
+
+	[IMX8MM_POWER_DOMAIN_GPUMIX] = {
+		.genpd = {
+			.name = "gpumix",
+		},
+		.bits  = {
+			.pxx = IMX8MM_GPUMIX_SW_Pxx_REQ,
+			.map = IMX8MM_GPUMIX_A53_DOMAIN,
+			.hskreq = IMX8MM_GPUMIX_HSK_PWRDNREQN,
+			.hskack = IMX8MM_GPUMIX_HSK_PWRDNACKN,
+		},
+		.pgc   = IMX8MM_PGC_GPUMIX,
+	},
+
+	[IMX8MM_POWER_DOMAIN_GPU] = {
+		.genpd = {
+			.name = "gpu",
+		},
+		.bits  = {
+			.pxx = IMX8MM_GPU_SW_Pxx_REQ,
+			.map = IMX8MM_GPU_A53_DOMAIN,
+			.hskreq = IMX8MM_GPU_HSK_PWRDNREQN,
+			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
+		},
+		.pgc   = IMX8MM_PGC_GPU2D,
+	},
+};
+
+static const struct regmap_range imx8mm_yes_ranges[] = {
+		regmap_reg_range(GPC_LPCR_A_CORE_BSC,
+				 GPC_PU_PWRHSK),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_MIPI),
+				 GPC_PGC_SR(IMX8MM_PGC_MIPI)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_PCIE),
+				 GPC_PGC_SR(IMX8MM_PGC_PCIE)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_OTG1),
+				 GPC_PGC_SR(IMX8MM_PGC_OTG1)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_OTG2),
+				 GPC_PGC_SR(IMX8MM_PGC_OTG2)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_DDR1),
+				 GPC_PGC_SR(IMX8MM_PGC_DDR1)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_GPU2D),
+				 GPC_PGC_SR(IMX8MM_PGC_GPU2D)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_GPUMIX),
+				 GPC_PGC_SR(IMX8MM_PGC_GPUMIX)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_VPUMIX),
+				 GPC_PGC_SR(IMX8MM_PGC_VPUMIX)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_GPU3D),
+				 GPC_PGC_SR(IMX8MM_PGC_GPU3D)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_DISPMIX),
+				 GPC_PGC_SR(IMX8MM_PGC_DISPMIX)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_VPUG1),
+				 GPC_PGC_SR(IMX8MM_PGC_VPUG1)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_VPUG2),
+				 GPC_PGC_SR(IMX8MM_PGC_VPUG2)),
+		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_VPUH1),
+				 GPC_PGC_SR(IMX8MM_PGC_VPUH1)),
+};
+
+static const struct regmap_access_table imx8mm_access_table = {
+	.yes_ranges	= imx8mm_yes_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(imx8mm_yes_ranges),
+};
+
+static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
+	.domains = imx8mm_pgc_domains,
+	.domains_num = ARRAY_SIZE(imx8mm_pgc_domains),
+	.reg_access_table = &imx8mm_access_table,
+};
+
 static int imx_pgc_domain_probe(struct platform_device *pdev)
 {
 	struct imx_pgc_domain *domain = pdev->dev.platform_data;
@@ -707,6 +874,7 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
 
 static const struct of_device_id imx_gpcv2_dt_ids[] = {
 	{ .compatible = "fsl,imx7d-gpc", .data = &imx7_pgc_domain_data, },
+	{ .compatible = "fsl,imx8mm-gpc", .data = &imx8mm_pgc_domain_data, },
 	{ .compatible = "fsl,imx8mq-gpc", .data = &imx8m_pgc_domain_data, },
 	{ }
 };
-- 
2.30.0

[PATCH 12/16] soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power domains

[Peng Fan (OSS)]

From: Lucas Stach <l.stach@pengutronix.de>

With the BLK-CTL driver now in place, let's add the missing domains.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/soc/imx/gpcv2.c | 70 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 5642dd236c10..42b9be05e1f2 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -646,6 +646,76 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 		},
 		.pgc   = IMX8MM_PGC_GPU2D,
 	},
+
+	[IMX8MM_POWER_DOMAIN_VPUMIX] = {
+		.genpd = {
+			.name = "vpumix",
+		},
+		.bits  = {
+			.pxx = IMX8MM_VPUMIX_SW_Pxx_REQ,
+			.map = IMX8MM_VPUMIX_A53_DOMAIN,
+			.hskreq = IMX8MM_VPUMIX_HSK_PWRDNREQN,
+			.hskack = IMX8MM_VPUMIX_HSK_PWRDNACKN,
+		},
+		.pgc   = IMX8MM_PGC_VPUMIX,
+	},
+
+	[IMX8MM_POWER_DOMAIN_VPUG1] = {
+		.genpd = {
+			.name = "vpu-g1",
+		},
+		.bits  = {
+			.pxx = IMX8MM_VPUG1_SW_Pxx_REQ,
+			.map = IMX8MM_VPUG1_A53_DOMAIN,
+		},
+		.pgc   = IMX8MM_PGC_VPUG1,
+	},
+
+	[IMX8MM_POWER_DOMAIN_VPUG2] = {
+		.genpd = {
+			.name = "vpu-g2",
+		},
+		.bits  = {
+			.pxx = IMX8MM_VPUG2_SW_Pxx_REQ,
+			.map = IMX8MM_VPUG2_A53_DOMAIN,
+		},
+		.pgc   = IMX8MM_PGC_VPUG2,
+	},
+
+	[IMX8MM_POWER_DOMAIN_VPUH1] = {
+		.genpd = {
+			.name = "vpu-h1",
+		},
+		.bits  = {
+			.pxx = IMX8MM_VPUH1_SW_Pxx_REQ,
+			.map = IMX8MM_VPUH1_A53_DOMAIN,
+		},
+		.pgc   = IMX8MM_PGC_VPUH1,
+	},
+
+	[IMX8MM_POWER_DOMAIN_DISPMIX] = {
+		.genpd = {
+			.name = "dispmix",
+		},
+		.bits  = {
+			.pxx = IMX8MM_DISPMIX_SW_Pxx_REQ,
+			.map = IMX8MM_DISPMIX_A53_DOMAIN,
+			.hskreq = IMX8MM_DISPMIX_HSK_PWRDNREQN,
+			.hskack = IMX8MM_DISPMIX_HSK_PWRDNACKN,
+		},
+		.pgc   = IMX8MM_PGC_DISPMIX,
+	},
+
+	[IMX8MM_POWER_DOMAIN_MIPI] = {
+		.genpd = {
+			.name = "mipi",
+		},
+		.bits  = {
+			.pxx = IMX8MM_MIPI_SW_Pxx_REQ,
+			.map = IMX8MM_MIPI_A53_DOMAIN,
+		},
+		.pgc   = IMX8MM_PGC_MIPI,
+	},
 };
 
 static const struct regmap_range imx8mm_yes_ranges[] = {
-- 
2.30.0

[PATCH 13/16] soc: imx: gpcv2: correct pm_runtime_get_sync usage

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

When the return value is negative, there is error, otherwise it is
expected.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 42b9be05e1f2..d2ce47a5ebad 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -197,7 +197,7 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 	int ret;
 
 	ret = pm_runtime_get_sync(domain->dev);
-	if (ret) {
+	if (ret < 0) {
 		pm_runtime_put_noidle(domain->dev);
 		return ret;
 	}
-- 
2.30.0

[PATCH 14/16] soc: imx: gpcv2: move reset assert after requesting domain power up

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

The i.MX8MM VPU power up sequence is a bit special, it must follow:
1. request power up
2. reset assert
3. reset deassert

This change in this patch will not affect other domains, because
the power domain default is in asserted state, unless bootloader
deassert the reset.

[Note: We expect bootloader leave the domain in asserted state,
but this may not always be true, so we might need another solution
to address the VPU domain requirements]

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index d2ce47a5ebad..072f519462a5 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -217,8 +217,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		goto out_regulator_disable;
 	}
 
-	reset_control_assert(domain->reset);
-
 	if (domain->bits.pxx) {
 		/* request the domain to power up */
 		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
@@ -241,6 +239,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 				   GPC_PGC_CTRL_PCR, 0);
 	}
 
+	reset_control_assert(domain->reset);
+
 	/* delay for reset to propagate */
 	udelay(5);
 
-- 
2.30.0

[PATCH 15/16] soc: imx: gpcv2: support reset defer probe

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

When gpcv2 probe, the reset controller might not be ready, so we need
defer probe

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 072f519462a5..49dd137f6b94 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -784,9 +784,12 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
 				     "Failed to get domain's clocks\n");
 
 	domain->reset = devm_reset_control_array_get_optional_exclusive(domain->dev);
-	if (IS_ERR(domain->reset))
+	if (IS_ERR(domain->reset)) {
+		if (PTR_ERR(domain->reset) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
 		return dev_err_probe(domain->dev, PTR_ERR(domain->reset),
 				     "Failed to get domain's resets\n");
+	}
 
 	pm_runtime_enable(domain->dev);
 
-- 
2.30.0

[PATCH 16/16] soc: imx: gpcv2: remove waiting handshake in power up

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX8MM has blk ctl module, the handshake can only finish after setting
blk ctl. The blk ctl driver will set the bus clk bit and the handshake
will finish there. we just add a delay and suppose the handshake will
finish after that.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 49dd137f6b94..04564017bfe9 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -251,14 +251,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
 				   domain->bits.hskreq, domain->bits.hskreq);
 
-		ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
-					       reg_val,
-					       (reg_val & domain->bits.hskack),
-					       0, USEC_PER_MSEC);
-		if (ret) {
-			dev_err(domain->dev, "failed to power up ADB400\n");
-			goto out_clk_disable;
-		}
 	}
 
 	/* Disable reset clocks for all devices in the domain */
-- 
2.30.0

Re: [PATCH 15/16] soc: imx: gpcv2: support reset defer probe

[Ahmad Fatoum]

Hi,

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> When gpcv2 probe, the reset controller might not be ready, so we need
> defer probe
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/soc/imx/gpcv2.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 072f519462a5..49dd137f6b94 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -784,9 +784,12 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>  				     "Failed to get domain's clocks\n");
>  
>  	domain->reset = devm_reset_control_array_get_optional_exclusive(domain->dev);
> -	if (IS_ERR(domain->reset))
> +	if (IS_ERR(domain->reset)) {
> +		if (PTR_ERR(domain->reset) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
>  		return dev_err_probe(domain->dev, PTR_ERR(domain->reset),
>  				     "Failed to get domain's resets\n");

dev_err_probe already propagates the error code in its second argument.
Seems to me this patch's only effect is to disable deferred probe reason tracking?

> +	}
>  
>  	pm_runtime_enable(domain->dev);
>  
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 00/16] soc: imx: gpcv2: support i.MX8MM

[Adam Ford]

On Thu, Apr 29, 2021 at 1:59 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several
> minor changes from me to make it could work with i.MX BLK-CTL driver.
>
> Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting
> all the patches, Jacky Bai on help debug issues.

Thank for you all the work.  I have an i.MX8M Nano that I'll work to
add support for gpcv2 unless NXP has started this already.  At one
time, I posted some patches for Nano based on Lucas' work, but since
that work wasn't accepted, mine wasn't either.

adam
>
> Lucas Stach (12):
>   soc: imx: gpcv2: move to more ideomatic error handling in probe
>   soc: imx: gpcv2: move domain mapping to domain driver probe
>   soc: imx: gpcv2: switch to clk_bulk_* API
>   soc: imx: gpcv2: split power up and power down sequence control
>   soc: imx: gpcv2: wait for ADB400 handshake
>   soc: imx: gpcv2: add runtime PM support for power-domains
>   soc: imx: gpcv2: allow domains without power-sequence control
>   dt-bindings: imx: gpcv2: add support for optional resets
>   soc: imx: gpcv2: add support for optional resets
>   dt-bindings: power: add defines for i.MX8MM power domains
>   soc: imx: gpcv2: add support for i.MX8MM power domains
>   soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power
>     domains
>
> Peng Fan (4):
>   soc: imx: gpcv2: correct pm_runtime_get_sync usage
>   soc: imx: gpcv2: move reset assert after requesting domain power up
>   soc: imx: gpcv2: support reset defer probe
>   soc: imx: gpcv2: remove waiting handshake in power up
>
>  .../bindings/power/fsl,imx-gpcv2.yaml         |   9 +
>  drivers/soc/imx/gpcv2.c                       | 534 ++++++++++++++----
>  include/dt-bindings/power/imx8mm-power.h      |  22 +
>  3 files changed, 450 insertions(+), 115 deletions(-)
>  create mode 100644 include/dt-bindings/power/imx8mm-power.h
>
> --
> 2.30.0
>

Re: [PATCH 13/16] soc: imx: gpcv2: correct pm_runtime_get_sync usage

[Lucas Stach]

Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> When the return value is negative, there is error, otherwise it is
> expected.

Good catch! As the runtime pm handling is added in this series, this
should be squashed into patch 06/16 to not add broken code and then fix
it in the same series. Change looks good to me.

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/soc/imx/gpcv2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 42b9be05e1f2..d2ce47a5ebad 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -197,7 +197,7 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>  	int ret;
>  
> 
> 
> 
>  	ret = pm_runtime_get_sync(domain->dev);
> -	if (ret) {
> +	if (ret < 0) {
>  		pm_runtime_put_noidle(domain->dev);
>  		return ret;
>  	}

Re: [PATCH 14/16] soc: imx: gpcv2: move reset assert after requesting domain power up

[Lucas Stach]

Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> The i.MX8MM VPU power up sequence is a bit special, it must follow:
> 1. request power up
> 2. reset assert
> 3. reset deassert
> 
> This change in this patch will not affect other domains, because
> the power domain default is in asserted state, unless bootloader
> deassert the reset.
> 
> [Note: We expect bootloader leave the domain in asserted state,
> but this may not always be true, so we might need another solution
> to address the VPU domain requirements]

This is only about the VPU and GPU domain, where we need to handle the
SRC reset from the GPC driver right? In that case I think it's a sane
assumption that the bootloader does not touch those resets.

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/soc/imx/gpcv2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index d2ce47a5ebad..072f519462a5 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -217,8 +217,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>  		goto out_regulator_disable;
>  	}
>  
> 
> 
> 
> 
> 
> 
> 
> -	reset_control_assert(domain->reset);
> -
>  	if (domain->bits.pxx) {
>  		/* request the domain to power up */
>  		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> @@ -241,6 +239,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>  				   GPC_PGC_CTRL_PCR, 0);
>  	}
>  
> 
> 
> 
> 
> 
> 
> 
> +	reset_control_assert(domain->reset);
> +
>  	/* delay for reset to propagate */
>  	udelay(5);

As this is a pretty arbitrary delay added by me, can you please check
with the HW team or whoever knows, if this is sufficiently long for
both GPU and VPU domains?

Regards,
Lucas

Re: [PATCH 15/16] soc: imx: gpcv2: support reset defer probe

[Lucas Stach]

Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> When gpcv2 probe, the reset controller might not be ready, so we need
> defer probe

I agree with Ahmad. dev_err_probe properly propagates the PROBE_DEFER
return, so this patch should be dropped.

Regards,
Lucas

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/soc/imx/gpcv2.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 072f519462a5..49dd137f6b94 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -784,9 +784,12 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>  				     "Failed to get domain's clocks\n");
>  
> 
> 
> 
>  	domain->reset = devm_reset_control_array_get_optional_exclusive(domain->dev);
> -	if (IS_ERR(domain->reset))
> +	if (IS_ERR(domain->reset)) {
> +		if (PTR_ERR(domain->reset) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
>  		return dev_err_probe(domain->dev, PTR_ERR(domain->reset),
>  				     "Failed to get domain's resets\n");
> +	}
>  
> 
> 
> 
>  	pm_runtime_enable(domain->dev);
>  
> 
> 
> 

Re: [PATCH 16/16] soc: imx: gpcv2: remove waiting handshake in power up

[Lucas Stach]

Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX8MM has blk ctl module, the handshake can only finish after setting
> blk ctl. The blk ctl driver will set the bus clk bit and the handshake
> will finish there. we just add a delay and suppose the handshake will
> finish after that.

Instead of this patch, just drop patch 05/16 from this series. I think
we should leave a comment in the code on why we aren't waiting for the
handshake ack, as this is non-obvious from looking at the driver code. 

Basically add a polished version of the commit log from this patch into
the driver code.

Regards,
Lucas

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/soc/imx/gpcv2.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 49dd137f6b94..04564017bfe9 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -251,14 +251,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>  		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
>  				   domain->bits.hskreq, domain->bits.hskreq);
>  
> 
> 
> 
> -		ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
> -					       reg_val,
> -					       (reg_val & domain->bits.hskack),
> -					       0, USEC_PER_MSEC);
> -		if (ret) {
> -			dev_err(domain->dev, "failed to power up ADB400\n");
> -			goto out_clk_disable;
> -		}
>  	}
>  
> 
> 
> 
>  	/* Disable reset clocks for all devices in the domain */

RE: [PATCH 00/16] soc: imx: gpcv2: support i.MX8MM

[Peng Fan]

> Subject: Re: [PATCH 00/16] soc: imx: gpcv2: support i.MX8MM
> 
> On Thu, Apr 29, 2021 at 1:59 AM Peng Fan (OSS) <peng.fan@oss.nxp.com>
> wrote:
> >
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several
> > minor changes from me to make it could work with i.MX BLK-CTL driver.
> >
> > Thanks for Lucas's work and suggestion, Frieder Schrempf for
> > collecting all the patches, Jacky Bai on help debug issues.
> 
> Thank for you all the work.  I have an i.MX8M Nano that I'll work to add
> support for gpcv2 unless NXP has started this already.  At one time, I posted
> some patches for Nano based on Lucas' work, but since that work wasn't
> accepted, mine wasn't either.

Please continue your work on i.MX8MN, I not work on this. The following
work from me is i.MX8MP.

Thanks,
Peng.

> 
> adam
> >
> > Lucas Stach (12):
> >   soc: imx: gpcv2: move to more ideomatic error handling in probe
> >   soc: imx: gpcv2: move domain mapping to domain driver probe
> >   soc: imx: gpcv2: switch to clk_bulk_* API
> >   soc: imx: gpcv2: split power up and power down sequence control
> >   soc: imx: gpcv2: wait for ADB400 handshake
> >   soc: imx: gpcv2: add runtime PM support for power-domains
> >   soc: imx: gpcv2: allow domains without power-sequence control
> >   dt-bindings: imx: gpcv2: add support for optional resets
> >   soc: imx: gpcv2: add support for optional resets
> >   dt-bindings: power: add defines for i.MX8MM power domains
> >   soc: imx: gpcv2: add support for i.MX8MM power domains
> >   soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power
> >     domains
> >
> > Peng Fan (4):
> >   soc: imx: gpcv2: correct pm_runtime_get_sync usage
> >   soc: imx: gpcv2: move reset assert after requesting domain power up
> >   soc: imx: gpcv2: support reset defer probe
> >   soc: imx: gpcv2: remove waiting handshake in power up
> >
> >  .../bindings/power/fsl,imx-gpcv2.yaml         |   9 +
> >  drivers/soc/imx/gpcv2.c                       | 534
> ++++++++++++++----
> >  include/dt-bindings/power/imx8mm-power.h      |  22 +
> >  3 files changed, 450 insertions(+), 115 deletions(-)  create mode
> > 100644 include/dt-bindings/power/imx8mm-power.h
> >
> > --
> > 2.30.0
> >

Re: [PATCH 00/16] soc: imx: gpcv2: support i.MX8MM

[Adam Ford]

On Thu, Apr 29, 2021 at 8:34 PM Peng Fan <peng.fan@nxp.com> wrote:
>
> > Subject: Re: [PATCH 00/16] soc: imx: gpcv2: support i.MX8MM
> >
> > On Thu, Apr 29, 2021 at 1:59 AM Peng Fan (OSS) <peng.fan@oss.nxp.com>
> > wrote:
> > >
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several
> > > minor changes from me to make it could work with i.MX BLK-CTL driver.
> > >
> > > Thanks for Lucas's work and suggestion, Frieder Schrempf for
> > > collecting all the patches, Jacky Bai on help debug issues.
> >
> > Thank for you all the work.  I have an i.MX8M Nano that I'll work to add
> > support for gpcv2 unless NXP has started this already.  At one time, I posted
> > some patches for Nano based on Lucas' work, but since that work wasn't
> > accepted, mine wasn't either.
>
> Please continue your work on i.MX8MN, I not work on this. The following
> work from me is i.MX8MP.

No problem.  I thought the focus would be on the 8MP,m so I went ahead
and posted a series [1] for enabling the gpcv2 for the Nano and the
power domains which don't require blk-ctl for now which include the
USB OTG, and the GPU.

If you and/or your colleagues have time to review it, it would be
appreciated.  I was able to suspend and resume with USB attached, and
it continued to operate.  I didn't do extensive testing yet.
I'm starting on the blk-ctl stuff now.  It seems to have changed quite
a bit since the initial submission from Abel, so I'll have to spend a
bit more time porting what I had before.

Thanks again for this series.  It will be nice to have the Mini, Nano
and Plus domains functional.

adam

[1] - https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210429211625.1835702-2-aford173@gmail.com/

>
> Thanks,
> Peng.
>
> >
> > adam
> > >
> > > Lucas Stach (12):
> > >   soc: imx: gpcv2: move to more ideomatic error handling in probe
> > >   soc: imx: gpcv2: move domain mapping to domain driver probe
> > >   soc: imx: gpcv2: switch to clk_bulk_* API
> > >   soc: imx: gpcv2: split power up and power down sequence control
> > >   soc: imx: gpcv2: wait for ADB400 handshake
> > >   soc: imx: gpcv2: add runtime PM support for power-domains
> > >   soc: imx: gpcv2: allow domains without power-sequence control
> > >   dt-bindings: imx: gpcv2: add support for optional resets
> > >   soc: imx: gpcv2: add support for optional resets
> > >   dt-bindings: power: add defines for i.MX8MM power domains
> > >   soc: imx: gpcv2: add support for i.MX8MM power domains
> > >   soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power
> > >     domains
> > >
> > > Peng Fan (4):
> > >   soc: imx: gpcv2: correct pm_runtime_get_sync usage
> > >   soc: imx: gpcv2: move reset assert after requesting domain power up
> > >   soc: imx: gpcv2: support reset defer probe
> > >   soc: imx: gpcv2: remove waiting handshake in power up
> > >
> > >  .../bindings/power/fsl,imx-gpcv2.yaml         |   9 +
> > >  drivers/soc/imx/gpcv2.c                       | 534
> > ++++++++++++++----
> > >  include/dt-bindings/power/imx8mm-power.h      |  22 +
> > >  3 files changed, 450 insertions(+), 115 deletions(-)  create mode
> > > 100644 include/dt-bindings/power/imx8mm-power.h
> > >
> > > --
> > > 2.30.0
> > >

Re: [PATCH 13/16] soc: imx: gpcv2: correct pm_runtime_get_sync usage

[Peng Fan (OSS)]

On 2021/4/29 22:25, Lucas Stach wrote:
> Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> When the return value is negative, there is error, otherwise it is
>> expected.
> 
> Good catch! As the runtime pm handling is added in this series, this
> should be squashed into patch 06/16 to not add broken code and then fix
> it in the same series. Change looks good to me.

Sure. This will be squashed into patch 06/16 in V2.

Thanks,
Peng.

> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>   drivers/soc/imx/gpcv2.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
>> index 42b9be05e1f2..d2ce47a5ebad 100644
>> --- a/drivers/soc/imx/gpcv2.c
>> +++ b/drivers/soc/imx/gpcv2.c
>> @@ -197,7 +197,7 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>>   	int ret;
>>   
>>
>>
>>
>>   	ret = pm_runtime_get_sync(domain->dev);
>> -	if (ret) {
>> +	if (ret < 0) {
>>   		pm_runtime_put_noidle(domain->dev);
>>   		return ret;
>>   	}
> 
> 

Re: [PATCH 14/16] soc: imx: gpcv2: move reset assert after requesting domain power up

[Peng Fan (OSS)]

On 2021/4/29 22:28, Lucas Stach wrote:
> Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> The i.MX8MM VPU power up sequence is a bit special, it must follow:
>> 1. request power up
>> 2. reset assert
>> 3. reset deassert
>>
>> This change in this patch will not affect other domains, because
>> the power domain default is in asserted state, unless bootloader
>> deassert the reset.
>>
>> [Note: We expect bootloader leave the domain in asserted state,
>> but this may not always be true, so we might need another solution
>> to address the VPU domain requirements]
> 
> This is only about the VPU and GPU domain, where we need to handle the
> SRC reset from the GPC driver right?

For GPU, I have not tried. From current ATF implementation, I think yes.

  In that case I think it's a sane
> assumption that the bootloader does not touch those resets.
> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>   drivers/soc/imx/gpcv2.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
>> index d2ce47a5ebad..072f519462a5 100644
>> --- a/drivers/soc/imx/gpcv2.c
>> +++ b/drivers/soc/imx/gpcv2.c
>> @@ -217,8 +217,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>>   		goto out_regulator_disable;
>>   	}
>>   
>>
>>
>>
>>
>>
>>
>>
>> -	reset_control_assert(domain->reset);
>> -
>>   	if (domain->bits.pxx) {
>>   		/* request the domain to power up */
>>   		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
>> @@ -241,6 +239,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>>   				   GPC_PGC_CTRL_PCR, 0);
>>   	}
>>   
>>
>>
>>
>>
>>
>>
>>
>> +	reset_control_assert(domain->reset);
>> +
>>   	/* delay for reset to propagate */
>>   	udelay(5);
> 
> As this is a pretty arbitrary delay added by me, can you please check
> with the HW team or whoever knows, if this is sufficiently long for
> both GPU and VPU domains?

For VPU, from my test, it is enough. For GPU, ATF code use 10us, let me
try to enable GPU and do some test, I think 5us is long enough here.

Thanks,
Peng.

> 
> Regards,
> Lucas
> 

Re: [PATCH 15/16] soc: imx: gpcv2: support reset defer probe

[Peng Fan (OSS)]

On 2021/4/29 22:30, Lucas Stach wrote:
> Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> When gpcv2 probe, the reset controller might not be ready, so we need
>> defer probe
> 
> I agree with Ahmad. dev_err_probe properly propagates the PROBE_DEFER
> return, so this patch should be dropped.

Sure. Drop in V2.

Thanks,
Peng.

> 
> Regards,
> Lucas
> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>   drivers/soc/imx/gpcv2.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
>> index 072f519462a5..49dd137f6b94 100644
>> --- a/drivers/soc/imx/gpcv2.c
>> +++ b/drivers/soc/imx/gpcv2.c
>> @@ -784,9 +784,12 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>>   				     "Failed to get domain's clocks\n");
>>   
>>
>>
>>
>>   	domain->reset = devm_reset_control_array_get_optional_exclusive(domain->dev);
>> -	if (IS_ERR(domain->reset))
>> +	if (IS_ERR(domain->reset)) {
>> +		if (PTR_ERR(domain->reset) == -EPROBE_DEFER)
>> +			return -EPROBE_DEFER;
>>   		return dev_err_probe(domain->dev, PTR_ERR(domain->reset),
>>   				     "Failed to get domain's resets\n");
>> +	}
>>   
>>
>>
>>
>>   	pm_runtime_enable(domain->dev);
>>   
>>
>>
>>
> 
> 

Re: [PATCH 16/16] soc: imx: gpcv2: remove waiting handshake in power up

[Peng Fan (OSS)]

On 2021/4/29 22:34, Lucas Stach wrote:
> Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> i.MX8MM has blk ctl module, the handshake can only finish after setting
>> blk ctl. The blk ctl driver will set the bus clk bit and the handshake
>> will finish there. we just add a delay and suppose the handshake will
>> finish after that.
> 
> Instead of this patch, just drop patch 05/16 from this series. I think
> we should leave a comment in the code on why we aren't waiting for the
> handshake ack, as this is non-obvious from looking at the driver code.
> 
> Basically add a polished version of the commit log from this patch into
> the driver code.

I see. Will do it in V2.

Thanks,
Peng.

> 
> Regards,
> Lucas
> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>   drivers/soc/imx/gpcv2.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
>> index 49dd137f6b94..04564017bfe9 100644
>> --- a/drivers/soc/imx/gpcv2.c
>> +++ b/drivers/soc/imx/gpcv2.c
>> @@ -251,14 +251,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>>   		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
>>   				   domain->bits.hskreq, domain->bits.hskreq);
>>   
>>
>>
>>
>> -		ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
>> -					       reg_val,
>> -					       (reg_val & domain->bits.hskack),
>> -					       0, USEC_PER_MSEC);
>> -		if (ret) {
>> -			dev_err(domain->dev, "failed to power up ADB400\n");
>> -			goto out_clk_disable;
>> -		}
>>   	}
>>   
>>
>>
>>
>>   	/* Disable reset clocks for all devices in the domain */
> 
> 

Re: [PATCH 16/16] soc: imx: gpcv2: remove waiting handshake in power up

[Peng Fan (OSS)]

On 2021/4/29 22:34, Lucas Stach wrote:
> Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> i.MX8MM has blk ctl module, the handshake can only finish after setting
>> blk ctl. The blk ctl driver will set the bus clk bit and the handshake
>> will finish there. we just add a delay and suppose the handshake will
>> finish after that.
> 
> Instead of this patch, just drop patch 05/16 from this series. I think
> we should leave a comment in the code on why we aren't waiting for the
> handshake ack, as this is non-obvious from looking at the driver code.
> 
> Basically add a polished version of the commit log from this patch into
> the driver code.

After thinking more, for power down, we need wait handshake. For power 
up, we could ignore handshake, because BLK-CTL setting bus clk en
will auto trigger handshake.

For power down, the bus clk en already set, and we need wait, otherwise
we need add delay there.

So I would only drop the power up waiting and add some comments there.

Thanks,
Peng.

> 
> Regards,
> Lucas
> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>>   drivers/soc/imx/gpcv2.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
>> index 49dd137f6b94..04564017bfe9 100644
>> --- a/drivers/soc/imx/gpcv2.c
>> +++ b/drivers/soc/imx/gpcv2.c
>> @@ -251,14 +251,6 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>>   		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
>>   				   domain->bits.hskreq, domain->bits.hskreq);
>>   
>>
>>
>>
>> -		ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
>> -					       reg_val,
>> -					       (reg_val & domain->bits.hskack),
>> -					       0, USEC_PER_MSEC);
>> -		if (ret) {
>> -			dev_err(domain->dev, "failed to power up ADB400\n");
>> -			goto out_clk_disable;
>> -		}
>>   	}
>>   
>>
>>
>>
>>   	/* Disable reset clocks for all devices in the domain */
> 
> 

Re: [PATCH 16/16] soc: imx: gpcv2: remove waiting handshake in power up

[Lucas Stach]

Am Freitag, dem 30.04.2021 um 15:14 +0800 schrieb Peng Fan (OSS):
> 
> On 2021/4/29 22:34, Lucas Stach wrote:
> > Am Donnerstag, dem 29.04.2021 um 15:30 +0800 schrieb Peng Fan (OSS):
> > > From: Peng Fan <peng.fan@nxp.com>
> > > 
> > > i.MX8MM has blk ctl module, the handshake can only finish after setting
> > > blk ctl. The blk ctl driver will set the bus clk bit and the handshake
> > > will finish there. we just add a delay and suppose the handshake will
> > > finish after that.
> > 
> > Instead of this patch, just drop patch 05/16 from this series. I think
> > we should leave a comment in the code on why we aren't waiting for the
> > handshake ack, as this is non-obvious from looking at the driver code.
> > 
> > Basically add a polished version of the commit log from this patch into
> > the driver code.
> 
> After thinking more, for power down, we need wait handshake. For power 
> up, we could ignore handshake, because BLK-CTL setting bus clk en
> will auto trigger handshake.
> 
> For power down, the bus clk en already set, and we need wait, otherwise
> we need add delay there.
> 
> So I would only drop the power up waiting and add some comments there.

Ah, right. This way makes a lot of sense.

Regards,
Lucas

Re: [PATCH 01/16] soc: imx: gpcv2: move to more ideomatic error handling in probe

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> Switch to "goto out..." error handling in domain driver probe to
> avoid repeating all the error paths.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Reviewed-by: Marek Vasut <marex@denx.de>
> Tested-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>   drivers/soc/imx/gpcv2.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index db7e7fc321b1..512e6f4acafd 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -502,18 +502,23 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   	ret = pm_genpd_init(&domain->genpd, NULL, true);
>   	if (ret) {
>   		dev_err(domain->dev, "Failed to init power domain\n");
> -		imx_pgc_put_clocks(domain);
> -		return ret;
> +		goto out_put_clocks;
>   	}
>   
>   	ret = of_genpd_add_provider_simple(domain->dev->of_node,
>   					   &domain->genpd);
>   	if (ret) {
>   		dev_err(domain->dev, "Failed to add genpd provider\n");
> -		pm_genpd_remove(&domain->genpd);
> -		imx_pgc_put_clocks(domain);
> +		goto out_genpd_remove;
>   	}
>   
> +	return 0;
> +
> +out_genpd_remove:
> +	pm_genpd_remove(&domain->genpd);
> +out_put_clocks:
> +	imx_pgc_put_clocks(domain);
> +
>   	return ret;
>   }
>   
> 

Re: [PATCH 02/16] soc: imx: gpcv2: move domain mapping to domain driver probe

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> As long as the power domain driver is active we want power control
> over the domain (which is what the mapping bit requests), so there
> is no point in whacking it for every power control action, simply
> set the bit in driver probe and clear it when the driver is removed.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>   drivers/soc/imx/gpcv2.c | 22 +++++++++++++---------
>   1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 512e6f4acafd..552d3e6bee52 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -140,14 +140,11 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
>   	int i, ret = 0;
>   	u32 pxx_req;
>   
> -	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> -			   domain->bits.map, domain->bits.map);
> -
>   	if (has_regulator && on) {
>   		ret = regulator_enable(domain->regulator);
>   		if (ret) {
>   			dev_err(domain->dev, "failed to enable regulator\n");
> -			goto unmap;
> +			return ret;
>   		}
>   	}
>   
> @@ -203,9 +200,7 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
>   		/* Preserve earlier error code */
>   		ret = ret ?: err;
>   	}
> -unmap:
> -	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> -			   domain->bits.map, 0);
> +
>   	return ret;
>   }
>   
> @@ -499,10 +494,13 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   	if (ret)
>   		return dev_err_probe(domain->dev, ret, "Failed to get domain's clocks\n");
>   
> +	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> +			   domain->bits.map, domain->bits.map);
> +
>   	ret = pm_genpd_init(&domain->genpd, NULL, true);
>   	if (ret) {
>   		dev_err(domain->dev, "Failed to init power domain\n");
> -		goto out_put_clocks;
> +		goto out_domain_unmap;
>   	}
>   
>   	ret = of_genpd_add_provider_simple(domain->dev->of_node,
> @@ -516,7 +514,9 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   
>   out_genpd_remove:
>   	pm_genpd_remove(&domain->genpd);
> -out_put_clocks:
> +out_domain_unmap:
> +	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> +			   domain->bits.map, 0);
>   	imx_pgc_put_clocks(domain);
>   
>   	return ret;
> @@ -528,6 +528,10 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
>   
>   	of_genpd_del_provider(domain->dev->of_node);
>   	pm_genpd_remove(&domain->genpd);
> +
> +	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> +			   domain->bits.map, 0);
> +
>   	imx_pgc_put_clocks(domain);
>   
>   	return 0;
> 

Re: [PATCH 03/16] soc: imx: gpcv2: switch to clk_bulk_* API

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> Use clk_bulk API to simplify the code a bit. Also add some error
> checking to the clk_prepare_enable calls.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>   drivers/soc/imx/gpcv2.c | 60 +++++++++--------------------------------
>   1 file changed, 12 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 552d3e6bee52..1d90c7802972 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -100,13 +100,11 @@
>   
>   #define GPC_PGC_CTRL_PCR		BIT(0)
>   
> -#define GPC_CLK_MAX		6
> -
>   struct imx_pgc_domain {
>   	struct generic_pm_domain genpd;
>   	struct regmap *regmap;
>   	struct regulator *regulator;
> -	struct clk *clk[GPC_CLK_MAX];
> +	struct clk_bulk_data *clks;
>   	int num_clks;
>   
>   	unsigned int pgc;
> @@ -149,8 +147,12 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
>   	}
>   
>   	/* Enable reset clocks for all devices in the domain */
> -	for (i = 0; i < domain->num_clks; i++)
> -		clk_prepare_enable(domain->clk[i]);
> +	clk_bulk_prepare_enable(domain->num_clks, domain->clks);

Looks like this fails to catch the return value. This is fixed in patch 
04/13 but should happen here already. All the rest looks good to me.

> +	if (ret) {
> +		dev_err(domain->dev, "failed to enable reset clocks\n");
> +		regulator_disable(domain->regulator);
> +		return ret;
> +	}
>   
>   	if (enable_power_control)
>   		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> @@ -187,8 +189,7 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
>   				   GPC_PGC_CTRL_PCR, 0);
>   
>   	/* Disable reset clocks for all devices in the domain */
> -	for (i = 0; i < domain->num_clks; i++)
> -		clk_disable_unprepare(domain->clk[i]);
> +	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
>   
>   	if (has_regulator && !on) {
>   		int err;
> @@ -438,41 +439,6 @@ static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
>   	.reg_access_table = &imx8m_access_table,
>   };
>   
> -static int imx_pgc_get_clocks(struct imx_pgc_domain *domain)
> -{
> -	int i, ret;
> -
> -	for (i = 0; ; i++) {
> -		struct clk *clk = of_clk_get(domain->dev->of_node, i);
> -		if (IS_ERR(clk))
> -			break;
> -		if (i >= GPC_CLK_MAX) {
> -			dev_err(domain->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:
> -	while (i--)
> -		clk_put(domain->clk[i]);
> -
> -	return ret;
> -}
> -
> -static void imx_pgc_put_clocks(struct imx_pgc_domain *domain)
> -{
> -	int i;
> -
> -	for (i = domain->num_clks - 1; i >= 0; i--)
> -		clk_put(domain->clk[i]);
> -}
> -
>   static int imx_pgc_domain_probe(struct platform_device *pdev)
>   {
>   	struct imx_pgc_domain *domain = pdev->dev.platform_data;
> @@ -490,9 +456,10 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   				      domain->voltage, domain->voltage);
>   	}
>   
> -	ret = imx_pgc_get_clocks(domain);
> -	if (ret)
> -		return dev_err_probe(domain->dev, ret, "Failed to get domain's clocks\n");
> +	domain->num_clks = devm_clk_bulk_get_all(domain->dev, &domain->clks);
> +	if (domain->num_clks < 0)
> +		return dev_err_probe(domain->dev, domain->num_clks,
> +				     "Failed to get domain's clocks\n");
>   
>   	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
>   			   domain->bits.map, domain->bits.map);
> @@ -517,7 +484,6 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   out_domain_unmap:
>   	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
>   			   domain->bits.map, 0);
> -	imx_pgc_put_clocks(domain);
>   
>   	return ret;
>   }
> @@ -532,8 +498,6 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
>   	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
>   			   domain->bits.map, 0);
>   
> -	imx_pgc_put_clocks(domain);
> -
>   	return 0;
>   }
>   
> 

Re: [PATCH 04/16] soc: imx: gpcv2: split power up and power down sequence control

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> The current mixed function to control both power up and power down
> sequences is very hard to follow and already contains some sequence
> errors like triggering the ADB400 handshake at the wrong time due to
> this. Split the function into two, which results in slightly more
> code, but is way easier to get right.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>   drivers/soc/imx/gpcv2.c | 141 ++++++++++++++++++++++++----------------
>   1 file changed, 86 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 1d90c7802972..7356e48ebdad 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -125,20 +125,19 @@ struct imx_pgc_domain_data {
>   	const struct regmap_access_table *reg_access_table;
>   };
>   
> -static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
> -				      bool on)
> +static inline struct imx_pgc_domain *
> +to_imx_pgc_domain(struct generic_pm_domain *genpd)
>   {
> -	struct imx_pgc_domain *domain = container_of(genpd,
> -						      struct imx_pgc_domain,
> -						      genpd);
> -	unsigned int offset = on ?
> -		GPC_PU_PGC_SW_PUP_REQ : GPC_PU_PGC_SW_PDN_REQ;
> -	const bool enable_power_control = !on;
> -	const bool has_regulator = !IS_ERR(domain->regulator);
> -	int i, ret = 0;
> -	u32 pxx_req;
> -
> -	if (has_regulator && on) {
> +	return container_of(genpd, struct imx_pgc_domain, genpd);
> +}
> +
> +static int imx_pgc_power_up(struct generic_pm_domain *genpd)
> +{
> +	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
> +	u32 reg_val;
> +	int ret;
> +
> +	if (!IS_ERR(domain->regulator)) {
>   		ret = regulator_enable(domain->regulator);
>   		if (ret) {
>   			dev_err(domain->dev, "failed to enable regulator\n");
> @@ -147,72 +146,104 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
>   	}
>   
>   	/* Enable reset clocks for all devices in the domain */
> -	clk_bulk_prepare_enable(domain->num_clks, domain->clks);
> +	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);

As mentioned, the change above should be moved to patch 03/16.

>   	if (ret) {
>   		dev_err(domain->dev, "failed to enable reset clocks\n");
> +		goto out_regulator_disable;
> +	}
> +
> +	/* request the domain to power up */
> +	regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> +			   domain->bits.pxx, domain->bits.pxx);
> +	/*
> +	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> +	 * for PUP_REQ/PDN_REQ bit to be cleared
> +	 */
> +	ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> +				       reg_val, !(reg_val & domain->bits.pxx),
> +				       0, USEC_PER_MSEC);
> +	if (ret) {
> +		dev_err(domain->dev, "failed to command PGC\n");
> +		goto out_clk_disable;
> +	}
> +
> +	/* disable power control */
> +	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> +			   GPC_PGC_CTRL_PCR, 0);

As we already touch this here, we could also simplify this a bit using 
regmap_clear_bits() instead.

> +
> +	/* request the ADB400 to power up */
> +	if (domain->bits.hsk)
> +		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
> +				   domain->bits.hsk, domain->bits.hsk);
> +
> +	/* Disable reset clocks for all devices in the domain */
> +	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
> +
> +	return 0;
> +
> +out_clk_disable:
> +	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
> +out_regulator_disable:
> +	if (!IS_ERR(domain->regulator))
>   		regulator_disable(domain->regulator);
> +
> +	return ret;
> +}
> +
> +static int imx_pgc_power_down(struct generic_pm_domain *genpd)
> +{
> +	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
> +	u32 reg_val;
> +	int ret;
> +
> +	/* Enable reset clocks for all devices in the domain */
> +	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
> +	if (ret) {
> +		dev_err(domain->dev, "failed to enable reset clocks\n");
>   		return ret;
>   	}
>   
> -	if (enable_power_control)
> -		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> -				   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
> -
> +	/* request the ADB400 to power down */
>   	if (domain->bits.hsk)
>   		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
> -				   domain->bits.hsk, on ? domain->bits.hsk : 0);
> +				   domain->bits.hsk, 0);

See above, regmap_clear_bits()

>   
> -	regmap_update_bits(domain->regmap, offset,
> -			   domain->bits.pxx, domain->bits.pxx);
> +	/* enable power control */
> +	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> +			   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
>   
> +	/* request the domain to power down */
> +	regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
> +			   domain->bits.pxx, domain->bits.pxx);
>   	/*
>   	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
>   	 * for PUP_REQ/PDN_REQ bit to be cleared
>   	 */
> -	ret = regmap_read_poll_timeout(domain->regmap, offset, pxx_req,
> -				       !(pxx_req & domain->bits.pxx),
> +	ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
> +				       reg_val, !(reg_val & domain->bits.pxx),
>   				       0, USEC_PER_MSEC);
>   	if (ret) {
>   		dev_err(domain->dev, "failed to command PGC\n");
> -		/*
> -		 * If we were in a process of enabling a
> -		 * domain and failed we might as well disable
> -		 * the regulator we just enabled. And if it
> -		 * was the opposite situation and we failed to
> -		 * power down -- keep the regulator on
> -		 */
> -		on = !on;
> +		goto out_clk_disable;
>   	}
>   
> -	if (enable_power_control)
> -		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> -				   GPC_PGC_CTRL_PCR, 0);
> -
>   	/* Disable reset clocks for all devices in the domain */
>   	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
>   
> -	if (has_regulator && !on) {
> -		int err;
> -
> -		err = regulator_disable(domain->regulator);
> -		if (err)
> -			dev_err(domain->dev,
> -				"failed to disable regulator: %d\n", err);
> -		/* Preserve earlier error code */
> -		ret = ret ?: err;
> +	if (!IS_ERR(domain->regulator)) {
> +		ret = regulator_disable(domain->regulator);
> +		if (ret) {
> +			dev_err(domain->dev, "failed to disable regulator\n");
> +			return ret;
> +		}
>   	}
>   
> -	return ret;
> -}
> +	return 0;
>   
> -static int imx_gpc_pu_pgc_sw_pup_req(struct generic_pm_domain *genpd)
> -{
> -	return imx_gpc_pu_pgc_sw_pxx_req(genpd, true);
> -}
> +out_clk_disable:
> +	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
>   
> -static int imx_gpc_pu_pgc_sw_pdn_req(struct generic_pm_domain *genpd)
> -{
> -	return imx_gpc_pu_pgc_sw_pxx_req(genpd, false);
> +	return ret;
>   }
>   
>   static const struct imx_pgc_domain imx7_pgc_domains[] = {
> @@ -590,8 +621,8 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
>   
>   		domain = pd_pdev->dev.platform_data;
>   		domain->regmap = regmap;
> -		domain->genpd.power_on  = imx_gpc_pu_pgc_sw_pup_req;
> -		domain->genpd.power_off = imx_gpc_pu_pgc_sw_pdn_req;
> +		domain->genpd.power_on  = imx_pgc_power_up;
> +		domain->genpd.power_off = imx_pgc_power_down;
>   
>   		pd_pdev->dev.parent = dev;
>   		pd_pdev->dev.of_node = np;
> 

Re: [PATCH 06/16] soc: imx: gpcv2: add runtime PM support for power-domains

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> This allows to nest domains into other power domains and have the
> parent domain powered up/down as required by the child domains.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

With the fix from patch 13/16 squashed:

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>   drivers/soc/imx/gpcv2.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index d27025e37a9e..87165619a689 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -12,6 +12,7 @@
>   #include <linux/of_device.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
>   #include <linux/regmap.h>
>   #include <linux/regulator/consumer.h>
>   #include <linux/sizes.h>
> @@ -141,11 +142,17 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>   	u32 reg_val;
>   	int ret;
>   
> +	ret = pm_runtime_get_sync(domain->dev);
> +	if (ret) {
> +		pm_runtime_put_noidle(domain->dev);
> +		return ret;
> +	}
> +
>   	if (!IS_ERR(domain->regulator)) {
>   		ret = regulator_enable(domain->regulator);
>   		if (ret) {
>   			dev_err(domain->dev, "failed to enable regulator\n");
> -			return ret;
> +			goto out_put_pm;
>   		}
>   	}
>   
> @@ -200,6 +207,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>   out_regulator_disable:
>   	if (!IS_ERR(domain->regulator))
>   		regulator_disable(domain->regulator);
> +out_put_pm:
> +	pm_runtime_put(domain->dev);
>   
>   	return ret;
>   }
> @@ -262,6 +271,8 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
>   		}
>   	}
>   
> +	pm_runtime_put(domain->dev);
> +
>   	return 0;
>   
>   out_clk_disable:
> @@ -519,6 +530,8 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   		return dev_err_probe(domain->dev, domain->num_clks,
>   				     "Failed to get domain's clocks\n");
>   
> +	pm_runtime_enable(domain->dev);
> +
>   	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
>   			   domain->bits.map, domain->bits.map);
>   
> @@ -542,6 +555,7 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   out_domain_unmap:
>   	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
>   			   domain->bits.map, 0);
> +	pm_runtime_disable(domain->dev);
>   
>   	return ret;
>   }
> @@ -556,6 +570,8 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
>   	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
>   			   domain->bits.map, 0);
>   
> +	pm_runtime_disable(domain->dev);
> +
>   	return 0;
>   }
>   
> 

Re: [PATCH 07/16] soc: imx: gpcv2: allow domains without power-sequence control

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> Some of the PGC domains only control the handshake with the ADB400
> and don't have any power sequence controls. Make such domains work
> by allowing the pxx and map bits to be empty and skip all actions
> using those controls.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>   drivers/soc/imx/gpcv2.c | 89 +++++++++++++++++++++++------------------
>   1 file changed, 49 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 87165619a689..640f4165cfba 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -163,24 +163,27 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>   		goto out_regulator_disable;
>   	}
>   
> -	/* request the domain to power up */
> -	regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> -			   domain->bits.pxx, domain->bits.pxx);
> -	/*
> -	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> -	 * for PUP_REQ/PDN_REQ bit to be cleared
> -	 */
> -	ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> -				       reg_val, !(reg_val & domain->bits.pxx),
> -				       0, USEC_PER_MSEC);
> -	if (ret) {
> -		dev_err(domain->dev, "failed to command PGC\n");
> -		goto out_clk_disable;
> -	}
> +	if (domain->bits.pxx) {
> +		/* request the domain to power up */
> +		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> +				   domain->bits.pxx, domain->bits.pxx);
> +		/*
> +		 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> +		 * for PUP_REQ/PDN_REQ bit to be cleared
> +		 */
> +		ret = regmap_read_poll_timeout(domain->regmap,
> +					       GPC_PU_PGC_SW_PUP_REQ, reg_val,
> +					       !(reg_val & domain->bits.pxx),
> +					       0, USEC_PER_MSEC);
> +		if (ret) {
> +			dev_err(domain->dev, "failed to command PGC\n");
> +			goto out_clk_disable;
> +		}
>   
> -	/* disable power control */
> -	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> -			   GPC_PGC_CTRL_PCR, 0);
> +		/* disable power control */
> +		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> +				   GPC_PGC_CTRL_PCR, 0);
> +	}
>   
>   	/* request the ADB400 to power up */
>   	if (domain->bits.hskreq) {
> @@ -241,23 +244,26 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
>   		}
>   	}
>   
> -	/* enable power control */
> -	regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> -			   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
> -
> -	/* request the domain to power down */
> -	regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
> -			   domain->bits.pxx, domain->bits.pxx);
> -	/*
> -	 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> -	 * for PUP_REQ/PDN_REQ bit to be cleared
> -	 */
> -	ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
> -				       reg_val, !(reg_val & domain->bits.pxx),
> -				       0, USEC_PER_MSEC);
> -	if (ret) {
> -		dev_err(domain->dev, "failed to command PGC\n");
> -		goto out_clk_disable;
> +	if (domain->bits.pxx) {
> +		/* enable power control */
> +		regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> +				   GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
> +
> +		/* request the domain to power down */
> +		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
> +				   domain->bits.pxx, domain->bits.pxx);
> +		/*
> +		 * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> +		 * for PUP_REQ/PDN_REQ bit to be cleared
> +		 */
> +		ret = regmap_read_poll_timeout(domain->regmap,
> +					       GPC_PU_PGC_SW_PDN_REQ, reg_val,
> +					       !(reg_val & domain->bits.pxx),
> +					       0, USEC_PER_MSEC);
> +		if (ret) {
> +			dev_err(domain->dev, "failed to command PGC\n");
> +			goto out_clk_disable;
> +		}
>   	}
>   
>   	/* Disable reset clocks for all devices in the domain */
> @@ -532,8 +538,9 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   
>   	pm_runtime_enable(domain->dev);
>   
> -	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> -			   domain->bits.map, domain->bits.map);
> +	if (domain->bits.map)
> +		regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> +				   domain->bits.map, domain->bits.map);
>   
>   	ret = pm_genpd_init(&domain->genpd, NULL, true);
>   	if (ret) {
> @@ -553,8 +560,9 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   out_genpd_remove:
>   	pm_genpd_remove(&domain->genpd);
>   out_domain_unmap:
> -	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> -			   domain->bits.map, 0);
> +	if (domain->bits.map)
> +		regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> +				   domain->bits.map, 0);
>   	pm_runtime_disable(domain->dev);
>   
>   	return ret;
> @@ -567,8 +575,9 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
>   	of_genpd_del_provider(domain->dev->of_node);
>   	pm_genpd_remove(&domain->genpd);
>   
> -	regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> -			   domain->bits.map, 0);
> +	if (domain->bits.map)
> +		regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> +				   domain->bits.map, 0);
>   
>   	pm_runtime_disable(domain->dev);
>   
> 

Re: [PATCH 08/16] dt-bindings: imx: gpcv2: add support for optional resets

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> For some domains the resets of the devices in the domain are not
> automatically triggered. Add an optional resets property to allow
> the GPC driver to trigger those resets explicitly.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Rob has approved this if it includes a proper explanation on why the 
resets are not defined. See: 
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20201105174434.1817539-9-l.stach@pengutronix.de/#24147743.

Can you add an explanation to the bindings in the next version?

> ---
>   Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
> index a96e6dbf1858..4330c73a2c30 100644
> --- a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
> +++ b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
> @@ -66,6 +66,13 @@ properties:
>   
>             power-supply: true
>   
> +          resets:
> +            description: |
> +              A number of phandles to resets that need to be asserted during
> +              power-up sequencing of the domain.
> +            minItems: 1
> +            maxItems: 4
> +
>           required:
>             - '#power-domain-cells'
>             - reg
> 

Re: [PATCH 09/16] soc: imx: gpcv2: add support for optional resets

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> Normally the reset for the devices inside the power domain is
> triggered automatically from the PGC in the power-up sequencing,
> however on i.MX8MM this doesn't work for the GPU power domains.
> 
> Add support for triggering the reset explicitly during the power
> up sequencing.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>   drivers/soc/imx/gpcv2.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 640f4165cfba..4a2c2a255d1a 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -15,6 +15,7 @@
>   #include <linux/pm_runtime.h>
>   #include <linux/regmap.h>
>   #include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
>   #include <linux/sizes.h>
>   #include <dt-bindings/power/imx7-power.h>
>   #include <dt-bindings/power/imx8mq-power.h>
> @@ -108,6 +109,7 @@ struct imx_pgc_domain {
>   	struct generic_pm_domain genpd;
>   	struct regmap *regmap;
>   	struct regulator *regulator;
> +	struct reset_control *reset;
>   	struct clk_bulk_data *clks;
>   	int num_clks;
>   
> @@ -163,6 +165,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>   		goto out_regulator_disable;
>   	}
>   
> +	reset_control_assert(domain->reset);
> +
>   	if (domain->bits.pxx) {
>   		/* request the domain to power up */
>   		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> @@ -185,6 +189,11 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>   				   GPC_PGC_CTRL_PCR, 0);
>   	}
>   
> +	/* delay for reset to propagate */
> +	udelay(5);
> +
> +	reset_control_deassert(domain->reset);
> +
>   	/* request the ADB400 to power up */
>   	if (domain->bits.hskreq) {
>   		regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
> @@ -531,11 +540,17 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
>   				      domain->voltage, domain->voltage);
>   	}
>   
> +

Spurious blank line. Otherwise:

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

>   	domain->num_clks = devm_clk_bulk_get_all(domain->dev, &domain->clks);
>   	if (domain->num_clks < 0)
>   		return dev_err_probe(domain->dev, domain->num_clks,
>   				     "Failed to get domain's clocks\n");
>   
> +	domain->reset = devm_reset_control_array_get_optional_exclusive(domain->dev);
> +	if (IS_ERR(domain->reset))
> +		return dev_err_probe(domain->dev, PTR_ERR(domain->reset),
> +				     "Failed to get domain's resets\n");
> +
>   	pm_runtime_enable(domain->dev);
>   
>   	if (domain->bits.map)
> 

Re: [PATCH 11/16] soc: imx: gpcv2: add support for i.MX8MM power domains

[Frieder Schrempf]

On 29.04.21 09:30, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> This adds support for the power domains founds on i.MX8MM. The 2D and 3D

s/founds/found/

> GPU domains are abstracted as a single domain in the driver, as they can't
> be powered up/down individually due to a shared reset.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>

> ---
>   drivers/soc/imx/gpcv2.c | 168 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 168 insertions(+)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 4a2c2a255d1a..5642dd236c10 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -19,6 +19,7 @@
>   #include <linux/sizes.h>
>   #include <dt-bindings/power/imx7-power.h>
>   #include <dt-bindings/power/imx8mq-power.h>
> +#include <dt-bindings/power/imx8mm-power.h>
>   
>   #define GPC_LPCR_A_CORE_BSC			0x000
>   
> @@ -44,6 +45,19 @@
>   #define IMX8M_PCIE1_A53_DOMAIN			BIT(3)
>   #define IMX8M_MIPI_A53_DOMAIN			BIT(2)
>   
> +#define IMX8MM_VPUH1_A53_DOMAIN			BIT(15)
> +#define IMX8MM_VPUG2_A53_DOMAIN			BIT(14)
> +#define IMX8MM_VPUG1_A53_DOMAIN			BIT(13)
> +#define IMX8MM_DISPMIX_A53_DOMAIN		BIT(12)
> +#define IMX8MM_VPUMIX_A53_DOMAIN		BIT(10)
> +#define IMX8MM_GPUMIX_A53_DOMAIN		BIT(9)
> +#define IMX8MM_GPU_A53_DOMAIN			(BIT(8) | BIT(11))
> +#define IMX8MM_DDR1_A53_DOMAIN			BIT(7)
> +#define IMX8MM_OTG2_A53_DOMAIN			BIT(5)
> +#define IMX8MM_OTG1_A53_DOMAIN			BIT(4)
> +#define IMX8MM_PCIE_A53_DOMAIN			BIT(3)
> +#define IMX8MM_MIPI_A53_DOMAIN			BIT(2)
> +
>   #define GPC_PU_PGC_SW_PUP_REQ		0x0f8
>   #define GPC_PU_PGC_SW_PDN_REQ		0x104
>   
> @@ -67,6 +81,19 @@
>   #define IMX8M_PCIE1_SW_Pxx_REQ			BIT(1)
>   #define IMX8M_MIPI_SW_Pxx_REQ			BIT(0)
>   
> +#define IMX8MM_VPUH1_SW_Pxx_REQ			BIT(13)
> +#define IMX8MM_VPUG2_SW_Pxx_REQ			BIT(12)
> +#define IMX8MM_VPUG1_SW_Pxx_REQ			BIT(11)
> +#define IMX8MM_DISPMIX_SW_Pxx_REQ		BIT(10)
> +#define IMX8MM_VPUMIX_SW_Pxx_REQ		BIT(8)
> +#define IMX8MM_GPUMIX_SW_Pxx_REQ		BIT(7)
> +#define IMX8MM_GPU_SW_Pxx_REQ			(BIT(6) | BIT(9))
> +#define IMX8MM_DDR1_SW_Pxx_REQ			BIT(5)
> +#define IMX8MM_OTG2_SW_Pxx_REQ			BIT(3)
> +#define IMX8MM_OTG1_SW_Pxx_REQ			BIT(2)
> +#define IMX8MM_PCIE_SW_Pxx_REQ			BIT(1)
> +#define IMX8MM_MIPI_SW_Pxx_REQ			BIT(0)
> +
>   #define GPC_M4_PU_PDN_FLG		0x1bc
>   
>   #define GPC_PU_PWRHSK			0x1fc
> @@ -78,6 +105,17 @@
>   #define IMX8M_VPU_HSK_PWRDNREQN			BIT(5)
>   #define IMX8M_DISP_HSK_PWRDNREQN		BIT(4)
>   
> +
> +#define IMX8MM_GPUMIX_HSK_PWRDNACKN		BIT(29)
> +#define IMX8MM_GPU_HSK_PWRDNACKN		(BIT(27) | BIT(28))
> +#define IMX8MM_VPUMIX_HSK_PWRDNACKN		BIT(26)
> +#define IMX8MM_DISPMIX_HSK_PWRDNACKN		BIT(25)
> +#define IMX8MM_HSIO_HSK_PWRDNACKN		(BIT(23) | BIT(24))
> +#define IMX8MM_GPUMIX_HSK_PWRDNREQN		BIT(11)
> +#define IMX8MM_GPU_HSK_PWRDNREQN		(BIT(9) | BIT(10))
> +#define IMX8MM_VPUMIX_HSK_PWRDNREQN		BIT(8)
> +#define IMX8MM_DISPMIX_HSK_PWRDNREQN		BIT(7)
> +#define IMX8MM_HSIO_HSK_PWRDNREQN		(BIT(5) | BIT(6))
>   /*
>    * The PGC offset values in Reference Manual
>    * (Rev. 1, 01/2018 and the older ones) GPC chapter's
> @@ -100,6 +138,20 @@
>   #define IMX8M_PGC_MIPI_CSI2		28
>   #define IMX8M_PGC_PCIE2			29
>   
> +#define IMX8MM_PGC_MIPI			16
> +#define IMX8MM_PGC_PCIE			17
> +#define IMX8MM_PGC_OTG1			18
> +#define IMX8MM_PGC_OTG2			19
> +#define IMX8MM_PGC_DDR1			21
> +#define IMX8MM_PGC_GPU2D		22
> +#define IMX8MM_PGC_GPUMIX		23
> +#define IMX8MM_PGC_VPUMIX		24
> +#define IMX8MM_PGC_GPU3D		25
> +#define IMX8MM_PGC_DISPMIX		26
> +#define IMX8MM_PGC_VPUG1		27
> +#define IMX8MM_PGC_VPUG2		28
> +#define IMX8MM_PGC_VPUH1		29
> +
>   #define GPC_PGC_CTRL(n)			(0x800 + (n) * 0x40)
>   #define GPC_PGC_SR(n)			(GPC_PGC_CTRL(n) + 0xc)
>   
> @@ -523,6 +575,121 @@ static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
>   	.reg_access_table = &imx8m_access_table,
>   };
>   
> +static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> +	[IMX8MM_POWER_DOMAIN_HSIOMIX] = {
> +		.genpd = {
> +			.name = "hsiomix",
> +		},
> +		.bits  = {
> +			.pxx = 0, /* no power sequence control */
> +			.map = 0, /* no power sequence control */
> +			.hskreq = IMX8MM_HSIO_HSK_PWRDNREQN,
> +			.hskack = IMX8MM_HSIO_HSK_PWRDNACKN,
> +		},
> +	},
> +
> +	[IMX8MM_POWER_DOMAIN_PCIE] = {
> +		.genpd = {
> +			.name = "pcie",
> +		},
> +		.bits  = {
> +			.pxx = IMX8MM_PCIE_SW_Pxx_REQ,
> +			.map = IMX8MM_PCIE_A53_DOMAIN,
> +		},
> +		.pgc   = IMX8MM_PGC_PCIE,
> +	},
> +
> +	[IMX8MM_POWER_DOMAIN_OTG1] = {
> +		.genpd = {
> +			.name = "usb-otg1",
> +		},
> +		.bits  = {
> +			.pxx = IMX8MM_OTG1_SW_Pxx_REQ,
> +			.map = IMX8MM_OTG1_A53_DOMAIN,
> +		},
> +		.pgc   = IMX8MM_PGC_OTG1,
> +	},
> +
> +	[IMX8MM_POWER_DOMAIN_OTG2] = {
> +		.genpd = {
> +			.name = "usb-otg2",
> +		},
> +		.bits  = {
> +			.pxx = IMX8MM_OTG2_SW_Pxx_REQ,
> +			.map = IMX8MM_OTG2_A53_DOMAIN,
> +		},
> +		.pgc   = IMX8MM_PGC_OTG2,
> +	},
> +
> +	[IMX8MM_POWER_DOMAIN_GPUMIX] = {
> +		.genpd = {
> +			.name = "gpumix",
> +		},
> +		.bits  = {
> +			.pxx = IMX8MM_GPUMIX_SW_Pxx_REQ,
> +			.map = IMX8MM_GPUMIX_A53_DOMAIN,
> +			.hskreq = IMX8MM_GPUMIX_HSK_PWRDNREQN,
> +			.hskack = IMX8MM_GPUMIX_HSK_PWRDNACKN,
> +		},
> +		.pgc   = IMX8MM_PGC_GPUMIX,
> +	},
> +
> +	[IMX8MM_POWER_DOMAIN_GPU] = {
> +		.genpd = {
> +			.name = "gpu",
> +		},
> +		.bits  = {
> +			.pxx = IMX8MM_GPU_SW_Pxx_REQ,
> +			.map = IMX8MM_GPU_A53_DOMAIN,
> +			.hskreq = IMX8MM_GPU_HSK_PWRDNREQN,
> +			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
> +		},
> +		.pgc   = IMX8MM_PGC_GPU2D,
> +	},
> +};
> +
> +static const struct regmap_range imx8mm_yes_ranges[] = {
> +		regmap_reg_range(GPC_LPCR_A_CORE_BSC,
> +				 GPC_PU_PWRHSK),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_MIPI),
> +				 GPC_PGC_SR(IMX8MM_PGC_MIPI)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_PCIE),
> +				 GPC_PGC_SR(IMX8MM_PGC_PCIE)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_OTG1),
> +				 GPC_PGC_SR(IMX8MM_PGC_OTG1)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_OTG2),
> +				 GPC_PGC_SR(IMX8MM_PGC_OTG2)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_DDR1),
> +				 GPC_PGC_SR(IMX8MM_PGC_DDR1)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_GPU2D),
> +				 GPC_PGC_SR(IMX8MM_PGC_GPU2D)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_GPUMIX),
> +				 GPC_PGC_SR(IMX8MM_PGC_GPUMIX)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_VPUMIX),
> +				 GPC_PGC_SR(IMX8MM_PGC_VPUMIX)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_GPU3D),
> +				 GPC_PGC_SR(IMX8MM_PGC_GPU3D)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_DISPMIX),
> +				 GPC_PGC_SR(IMX8MM_PGC_DISPMIX)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_VPUG1),
> +				 GPC_PGC_SR(IMX8MM_PGC_VPUG1)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_VPUG2),
> +				 GPC_PGC_SR(IMX8MM_PGC_VPUG2)),
> +		regmap_reg_range(GPC_PGC_CTRL(IMX8MM_PGC_VPUH1),
> +				 GPC_PGC_SR(IMX8MM_PGC_VPUH1)),
> +};
> +
> +static const struct regmap_access_table imx8mm_access_table = {
> +	.yes_ranges	= imx8mm_yes_ranges,
> +	.n_yes_ranges	= ARRAY_SIZE(imx8mm_yes_ranges),
> +};
> +
> +static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
> +	.domains = imx8mm_pgc_domains,
> +	.domains_num = ARRAY_SIZE(imx8mm_pgc_domains),
> +	.reg_access_table = &imx8mm_access_table,
> +};
> +
>   static int imx_pgc_domain_probe(struct platform_device *pdev)
>   {
>   	struct imx_pgc_domain *domain = pdev->dev.platform_data;
> @@ -707,6 +874,7 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
>   
>   static const struct of_device_id imx_gpcv2_dt_ids[] = {
>   	{ .compatible = "fsl,imx7d-gpc", .data = &imx7_pgc_domain_data, },
> +	{ .compatible = "fsl,imx8mm-gpc", .data = &imx8mm_pgc_domain_data, },
>   	{ .compatible = "fsl,imx8mq-gpc", .data = &imx8m_pgc_domain_data, },
>   	{ }
>   };
> 

Re: [PATCH 10/16] dt-bindings: power: add defines for i.MX8MM power domains

[Rob Herring]

On Thu, 29 Apr 2021 15:30:44 +0800, Peng Fan (OSS) wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> Adding defines for i.MX8MM GPC power domains.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../bindings/power/fsl,imx-gpcv2.yaml         |  2 ++
>  include/dt-bindings/power/imx8mm-power.h      | 22 +++++++++++++++++++
>  2 files changed, 24 insertions(+)
>  create mode 100644 include/dt-bindings/power/imx8mm-power.h
> 

Acked-by: Rob Herring <robh@kernel.org>