[PATCH v2 00/13] i.MX8MM power domain support

[PATCH v2 00/13] i.MX8MM power domain support

[Lucas Stach]

Hi all,

this is the second revision of the patchset to add power domain control
for the i.MX8MM SoC to the GPCv2 driver. For now I've dropped all
support (both DT binding and code) for the power domains that contain
a blk-ctl. Support for those is still under development and will probably
require some more discussions to get things right. However the cut-down
functionality provided by this series is already a big step forward, as
it allows to use USB controllers and GPUs on the i.MX8MM, without the
need to statically turn on the respective power domains before jumping
into Linux.

Regards,
Lucas

Lucas Stach (13):
  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: add defines for i.MX8MM power domains
  soc: imx: gpcv2: add support for i.MX8MM power domains
  arm64: dts: imx8mm: add GPC node and power domains
  arm64: dts: imx8mm: put USB controllers into power-domains

 .../bindings/power/fsl,imx-gpcv2.yaml         |   9 +
 arch/arm64/boot/dts/freescale/imx8mm.dtsi     |  60 +++
 drivers/soc/imx/gpcv2.c                       | 467 +++++++++++++-----
 include/dt-bindings/power/imx8mm-power.h      |  16 +
 4 files changed, 438 insertions(+), 114 deletions(-)
 create mode 100644 include/dt-bindings/power/imx8mm-power.h

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Adam Ford]

On Thu, Nov 5, 2020 at 11:44 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> 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>

I can confirm the USB is functional now without having to turn it on
in the bootloader on the imx8mm-beacon-kit.

For the series:

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rob Herring]

On Thu, Nov 05, 2020 at 06:44:29PM +0100, Lucas Stach wrote:
> 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

You need to define what each reset is.

> +
>          required:
>            - '#power-domain-cells'
>            - reg
> -- 
> 2.20.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

Am Montag, den 09.11.2020, 14:15 -0600 schrieb Rob Herring:
> On Thu, Nov 05, 2020 at 06:44:29PM +0100, Lucas Stach wrote:
> > 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
> 
> You need to define what each reset is.

I can't. The resets belong to devices located inside the power domain,
which need to be held in reset across the power-up sequence. So I have
no means to specify what each reset is in a generic power-domain
binding. Same situation as with the clocks in this binding actually.

Regards,
Lucas


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

Hi Rob,

Am Dienstag, den 17.11.2020, 15:11 +0100 schrieb Lucas Stach:
Am Montag, den 09.11.2020, 14:15 -0600 schrieb Rob Herring:
> On Thu, Nov 05, 2020 at 06:44:29PM +0100, Lucas Stach wrote:
> > 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
> 
> You need to define what each reset is.

I can't. The resets belong to devices located inside the power domain,
which need to be held in reset across the power-up sequence. So I
have no means to specify what each reset is in a generic power-domain
binding. Same situation as with the clocks in this binding actually.

Do you have any guidance on what do here? Is this binding okay with
this explanation, or do we need to do something different here?

Regards,
Lucas


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

Am Montag, dem 30.11.2020 um 10:57 +0100 schrieb Lucas Stach:
> Hi Rob,
> 
> Am Dienstag, den 17.11.2020, 15:11 +0100 schrieb Lucas Stach:
> Am Montag, den 09.11.2020, 14:15 -0600 schrieb Rob Herring:
> > On Thu, Nov 05, 2020 at 06:44:29PM +0100, Lucas Stach wrote:
> > > 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
> > 
> > You need to define what each reset is.
> 
> I can't. The resets belong to devices located inside the power domain,
> which need to be held in reset across the power-up sequence. So I
> have no means to specify what each reset is in a generic power-domain
> binding. Same situation as with the clocks in this binding actually.
> 
> Do you have any guidance on what do here? Is this binding okay with
> this explanation, or do we need to do something different here?

Any pointers on how we can make some progress with this? It's blocking
quite a bit of functionality of the i.MX8MM SoC being enabled upstream.

Regards,
Lucas


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Marek Vasut]

On 2/10/21 3:35 PM, Lucas Stach wrote:
> Am Montag, dem 30.11.2020 um 10:57 +0100 schrieb Lucas Stach:
>> Hi Rob,
>>
>> Am Dienstag, den 17.11.2020, 15:11 +0100 schrieb Lucas Stach:
>> Am Montag, den 09.11.2020, 14:15 -0600 schrieb Rob Herring:
>>> On Thu, Nov 05, 2020 at 06:44:29PM +0100, Lucas Stach wrote:
>>>> 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
>>>
>>> You need to define what each reset is.
>>
>> I can't. The resets belong to devices located inside the power domain,
>> which need to be held in reset across the power-up sequence. So I
>> have no means to specify what each reset is in a generic power-domain
>> binding. Same situation as with the clocks in this binding actually.
>>
>> Do you have any guidance on what do here? Is this binding okay with
>> this explanation, or do we need to do something different here?
> 
> Any pointers on how we can make some progress with this? It's blocking
> quite a bit of functionality of the i.MX8MM SoC being enabled upstream.

Not just MX8MM anymore, but also MX8MN and MX8MP , so it would be real 
helpful to unblock this.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Frieder Schrempf]

Hi Rob,

On 10.02.21 15:35, Lucas Stach wrote:
> Am Montag, dem 30.11.2020 um 10:57 +0100 schrieb Lucas Stach:
>> Hi Rob,
>>
>> Am Dienstag, den 17.11.2020, 15:11 +0100 schrieb Lucas Stach:
>> Am Montag, den 09.11.2020, 14:15 -0600 schrieb Rob Herring:
>>> On Thu, Nov 05, 2020 at 06:44:29PM +0100, Lucas Stach wrote:
>>>> 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
>>>
>>> You need to define what each reset is.
>>
>> I can't. The resets belong to devices located inside the power domain,
>> which need to be held in reset across the power-up sequence. So I
>> have no means to specify what each reset is in a generic power-domain
>> binding. Same situation as with the clocks in this binding actually.
>>
>> Do you have any guidance on what do here? Is this binding okay with
>> this explanation, or do we need to do something different here?
> 
> Any pointers on how we can make some progress with this? It's blocking
> quite a bit of functionality of the i.MX8MM SoC being enabled upstream.

One more ping from my side. Can you give us some feedback about whether 
we can proceed with the bindings proposed by Lucas or not?

This has been on hold now for over 5 months and it looks like one of the 
reasons is that we don't know if the bindings will be accepted.

Thanks
Frieder

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Frieder Schrempf]

On 26.04.21 11:24, Frieder Schrempf wrote:
> Hi Rob,
> 
> On 10.02.21 15:35, Lucas Stach wrote:
>> Am Montag, dem 30.11.2020 um 10:57 +0100 schrieb Lucas Stach:
>>> Hi Rob,
>>>
>>> Am Dienstag, den 17.11.2020, 15:11 +0100 schrieb Lucas Stach:
>>> Am Montag, den 09.11.2020, 14:15 -0600 schrieb Rob Herring:
>>>> On Thu, Nov 05, 2020 at 06:44:29PM +0100, Lucas Stach wrote:
>>>>> 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
>>>>
>>>> You need to define what each reset is.
>>>
>>> I can't. The resets belong to devices located inside the power domain,
>>> which need to be held in reset across the power-up sequence. So I
>>> have no means to specify what each reset is in a generic power-domain
>>> binding. Same situation as with the clocks in this binding actually.
>>>
>>> Do you have any guidance on what do here? Is this binding okay with
>>> this explanation, or do we need to do something different here?
>>
>> Any pointers on how we can make some progress with this? It's blocking
>> quite a bit of functionality of the i.MX8MM SoC being enabled upstream.
> 
> One more ping from my side. Can you give us some feedback about whether 
> we can proceed with the bindings proposed by Lucas or not?
> 
> This has been on hold now for over 5 months and it looks like one of the 
> reasons is that we don't know if the bindings will be accepted.

It looks like Rob is fine with the bindings if there is an explanation 
in the bindings for why the resets are not defined.

Quote from Rob on IRC: "Just explain why the resets are unknown and 
variable in the binding".



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Adam Ford]

On Tue, Nov 17, 2020 at 8:11 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Montag, den 09.11.2020, 14:15 -0600 schrieb Rob Herring:
> > On Thu, Nov 05, 2020 at 06:44:29PM +0100, Lucas Stach wrote:
> > > 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
> >
> > You need to define what each reset is.
>
> I can't. The resets belong to devices located inside the power domain,
> which need to be held in reset across the power-up sequence. So I have
> no means to specify what each reset is in a generic power-domain
> binding. Same situation as with the clocks in this binding actually.
>

Rob,

Do you have any guidance on how we might be able to give you want you
want so we can move forward?  This power-domain driver will be used
for a variety of Freescale/NXP IMX SoC's, and looking at other power
domain controllers [1], they don't explicitly define what each reset
is.  Since the resets for this family vary from SoC to SoC, the number
of resets will change from one SoC to another.  If you could give us a
suggestion or an example of a board that has the power-domain resets
do what you ask, it would help move this forward.  Without this
driver, several boards are unable to use a significant number of
peripherals.

adam
[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.yaml?h=v5.12-rc4

> Regards,
> Lucas
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
v2: drop defines for power-domains with blk-ctl for now
    as those need more work to enable.
---
 .../devicetree/bindings/power/fsl,imx-gpcv2.yaml |  2 ++
 include/dt-bindings/power/imx8mm-power.h         | 16 ++++++++++++++++
 2 files changed, 18 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..2d46d4d788c0
--- /dev/null
+++ b/include/dt-bindings/power/imx8mm-power.h
@@ -0,0 +1,16 @@
+/* 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
+
+#endif
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Rob Herring]

On Thu, 05 Nov 2020 18:44:31 +0100, Lucas Stach wrote:
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> v2: drop defines for power-domains with blk-ctl for now
>     as those need more work to enable.
> ---
>  .../devicetree/bindings/power/fsl,imx-gpcv2.yaml |  2 ++
>  include/dt-bindings/power/imx8mm-power.h         | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+)
>  create mode 100644 include/dt-bindings/power/imx8mm-power.h
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lucas Stach]

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>
---
v2: drop power-domains with blk-ctl for now as those need
    more work to enable.
---
 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.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 12/13] arm64: dts: imx8mm: add GPC node and power domains

[Lucas Stach]

This adds the DT nodes to describe the power domains available on the
i.MX8MM. Things are a bit more complex compared to other GPCv2 power
domain setups, as there is now a hierarchy of domains where complete
subsystems (HSIO, GPU, DISPLAY) can be gated as a whole, but also
fine granular gating within those subsystems is possible.

Note that this is still incomplete, as both VPU and DISP domains are
missing their reset clocks. Those aren't directly sourced from the CCM,
but have another level of clock gating in the BLKCTL of those domains,
which needs a separate driver.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 58 +++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index b83f400def8b..c21901a8aea9 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -4,6 +4,8 @@
  */
 
 #include <dt-bindings/clock/imx8mm-clock.h>
+#include <dt-bindings/power/imx8mm-power.h>
+#include <dt-bindings/reset/imx8mq-reset.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -547,6 +549,62 @@
 				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
 				#reset-cells = <1>;
 			};
+
+			gpc: gpc@303a0000 {
+				compatible = "fsl,imx8mm-gpc";
+				reg = <0x303a0000 0x10000>;
+				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <3>;
+
+				pgc {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					pgc_hsiomix: power-domain@0 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
+						clocks = <&clk IMX8MM_CLK_USB_BUS>;
+					};
+
+					pgc_pcie: power-domain@1 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_PCIE>;
+						power-domains = <&pgc_hsiomix>;
+					};
+
+					pgc_otg1: power-domain@2 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_OTG1>;
+						power-domains = <&pgc_hsiomix>;
+					};
+
+					pgc_otg2: power-domain@3 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_OTG2>;
+						power-domains = <&pgc_hsiomix>;
+					};
+
+					pgc_gpumix: power-domain@4 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_GPUMIX>;
+						clocks = <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+						         <&clk IMX8MM_CLK_GPU_AHB>;
+					};
+
+					pgc_gpu: power-domain@5 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MM_POWER_DOMAIN_GPU>;
+						clocks = <&clk IMX8MM_CLK_GPU_AHB>,
+						         <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
+						         <&clk IMX8MM_CLK_GPU2D_ROOT>,
+						         <&clk IMX8MM_CLK_GPU3D_ROOT>;
+						resets = <&src IMX8MQ_RESET_GPU_RESET>;
+						power-domains = <&pgc_gpumix>;
+					};
+				};
+			};
 		};
 
 		aips2: bus@30400000 {
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 12/13] arm64: dts: imx8mm: add GPC node and power domains

[Frieder Schrempf]

Hi Lucas,

On 05.11.20 18:44, Lucas Stach wrote:
> This adds the DT nodes to describe the power domains available on the
> i.MX8MM. Things are a bit more complex compared to other GPCv2 power
> domain setups, as there is now a hierarchy of domains where complete
> subsystems (HSIO, GPU, DISPLAY) can be gated as a whole, but also
> fine granular gating within those subsystems is possible.
> 
> Note that this is still incomplete, as both VPU and DISP domains are
> missing their reset clocks. Those aren't directly sourced from the CCM,
> but have another level of clock gating in the BLKCTL of those domains,
> which needs a separate driver.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 58 +++++++++++++++++++++++
>   1 file changed, 58 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index b83f400def8b..c21901a8aea9 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -4,6 +4,8 @@
>    */
>   
>   #include <dt-bindings/clock/imx8mm-clock.h>
> +#include <dt-bindings/power/imx8mm-power.h>
> +#include <dt-bindings/reset/imx8mq-reset.h>
>   #include <dt-bindings/gpio/gpio.h>
>   #include <dt-bindings/input/input.h>
>   #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -547,6 +549,62 @@
>   				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
>   				#reset-cells = <1>;
>   			};
> +
> +			gpc: gpc@303a0000 {
> +				compatible = "fsl,imx8mm-gpc";
> +				reg = <0x303a0000 0x10000>;
> +				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> +				interrupt-parent = <&gic>;
> +				interrupt-controller;
> +				#interrupt-cells = <3>;
> +
> +				pgc {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					pgc_hsiomix: power-domain@0 {
> +						#power-domain-cells = <0>;
> +						reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
> +						clocks = <&clk IMX8MM_CLK_USB_BUS>;
> +					};
> +
> +					pgc_pcie: power-domain@1 {
> +						#power-domain-cells = <0>;
> +						reg = <IMX8MM_POWER_DOMAIN_PCIE>;
> +						power-domains = <&pgc_hsiomix>;
> +					};
> +
> +					pgc_otg1: power-domain@2 {
> +						#power-domain-cells = <0>;
> +						reg = <IMX8MM_POWER_DOMAIN_OTG1>;
> +						power-domains = <&pgc_hsiomix>;
> +					};
> +
> +					pgc_otg2: power-domain@3 {
> +						#power-domain-cells = <0>;
> +						reg = <IMX8MM_POWER_DOMAIN_OTG2>;
> +						power-domains = <&pgc_hsiomix>;
> +					};

I'm currently doing some testing on top of v5.10-rc with GPC, BLK-CTL, 
DSIM, etc. I noticed that as soon as I add the nodes above for HSIO/OTG 
(even without referencing them elsewhere) my system freezes on 
suspend/resume.

The same problem exists when I remove the power domains for HSIO/USB and 
add the ones for DISPMIX and DSI to test Marek's work on BLK-CTL.

I'm not really sure at what point exactly the system freezes but this is 
what I see (no_console_suspend is set) and the system does not wake up 
anymore:

echo mem > /sys/power/state
[   13.888711] PM: suspend entry (deep)
[   13.892429] Filesystems sync: 0.000 seconds
[   13.907231] Freezing user space processes ... (elapsed 0.031 seconds) 
done.
[   13.945407] OOM killer disabled.
[   13.948642] Freezing remaining freezable tasks ... (elapsed 0.001 
seconds) done.
[   13.957216] printk: Suspending console(s) (use no_console_suspend to 
debug)

Any ideas?

Thanks
Frieder

> +
> +					pgc_gpumix: power-domain@4 {
> +						#power-domain-cells = <0>;
> +						reg = <IMX8MM_POWER_DOMAIN_GPUMIX>;
> +						clocks = <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
> +						         <&clk IMX8MM_CLK_GPU_AHB>;
> +					};
> +
> +					pgc_gpu: power-domain@5 {
> +						#power-domain-cells = <0>;
> +						reg = <IMX8MM_POWER_DOMAIN_GPU>;
> +						clocks = <&clk IMX8MM_CLK_GPU_AHB>,
> +						         <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
> +						         <&clk IMX8MM_CLK_GPU2D_ROOT>,
> +						         <&clk IMX8MM_CLK_GPU3D_ROOT>;
> +						resets = <&src IMX8MQ_RESET_GPU_RESET>;
> +						power-domains = <&pgc_gpumix>;
> +					};
> +				};
> +			};
>   		};
>   
>   		aips2: bus@30400000 {
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 12/13] arm64: dts: imx8mm: add GPC node and power domains

[Frieder Schrempf]

On 09.12.20 16:26, Frieder Schrempf wrote:
> Hi Lucas,
> 
> On 05.11.20 18:44, Lucas Stach wrote:
>> This adds the DT nodes to describe the power domains available on the
>> i.MX8MM. Things are a bit more complex compared to other GPCv2 power
>> domain setups, as there is now a hierarchy of domains where complete
>> subsystems (HSIO, GPU, DISPLAY) can be gated as a whole, but also
>> fine granular gating within those subsystems is possible.
>>
>> Note that this is still incomplete, as both VPU and DISP domains are
>> missing their reset clocks. Those aren't directly sourced from the CCM,
>> but have another level of clock gating in the BLKCTL of those domains,
>> which needs a separate driver.
>>
>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>> ---
>>   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 58 +++++++++++++++++++++++
>>   1 file changed, 58 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi 
>> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
>> index b83f400def8b..c21901a8aea9 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
>> @@ -4,6 +4,8 @@
>>    */
>>   #include <dt-bindings/clock/imx8mm-clock.h>
>> +#include <dt-bindings/power/imx8mm-power.h>
>> +#include <dt-bindings/reset/imx8mq-reset.h>
>>   #include <dt-bindings/gpio/gpio.h>
>>   #include <dt-bindings/input/input.h>
>>   #include <dt-bindings/interrupt-controller/arm-gic.h>
>> @@ -547,6 +549,62 @@
>>                   interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
>>                   #reset-cells = <1>;
>>               };
>> +
>> +            gpc: gpc@303a0000 {
>> +                compatible = "fsl,imx8mm-gpc";
>> +                reg = <0x303a0000 0x10000>;
>> +                interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>> +                interrupt-parent = <&gic>;
>> +                interrupt-controller;
>> +                #interrupt-cells = <3>;
>> +
>> +                pgc {
>> +                    #address-cells = <1>;
>> +                    #size-cells = <0>;
>> +
>> +                    pgc_hsiomix: power-domain@0 {
>> +                        #power-domain-cells = <0>;
>> +                        reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
>> +                        clocks = <&clk IMX8MM_CLK_USB_BUS>;
>> +                    };
>> +
>> +                    pgc_pcie: power-domain@1 {
>> +                        #power-domain-cells = <0>;
>> +                        reg = <IMX8MM_POWER_DOMAIN_PCIE>;
>> +                        power-domains = <&pgc_hsiomix>;
>> +                    };
>> +
>> +                    pgc_otg1: power-domain@2 {
>> +                        #power-domain-cells = <0>;
>> +                        reg = <IMX8MM_POWER_DOMAIN_OTG1>;
>> +                        power-domains = <&pgc_hsiomix>;
>> +                    };
>> +
>> +                    pgc_otg2: power-domain@3 {
>> +                        #power-domain-cells = <0>;
>> +                        reg = <IMX8MM_POWER_DOMAIN_OTG2>;
>> +                        power-domains = <&pgc_hsiomix>;
>> +                    };
> 
> I'm currently doing some testing on top of v5.10-rc with GPC, BLK-CTL, 
> DSIM, etc. I noticed that as soon as I add the nodes above for HSIO/OTG 
> (even without referencing them elsewhere) my system freezes on 
> suspend/resume.
> 
> The same problem exists when I remove the power domains for HSIO/USB and 
> add the ones for DISPMIX and DSI to test Marek's work on BLK-CTL.
> 
> I'm not really sure at what point exactly the system freezes but this is 
> what I see (no_console_suspend is set) and the system does not wake up 
> anymore:
> 
> echo mem > /sys/power/state
> [   13.888711] PM: suspend entry (deep)
> [   13.892429] Filesystems sync: 0.000 seconds
> [   13.907231] Freezing user space processes ... (elapsed 0.031 seconds) 
> done.
> [   13.945407] OOM killer disabled.
> [   13.948642] Freezing remaining freezable tasks ... (elapsed 0.001 
> seconds) done.
> [   13.957216] printk: Suspending console(s) (use no_console_suspend to 
> debug)

It seems like I failed to set no_console_suspend correctly. Here is a 
proper log with kernel 5.10.6. The system wakes up, but stalls.

Can you reproduce this on your system?

# echo mem > /sys/power/state
[ 1033.094465] PM: suspend entry (deep)
[ 1033.098195] Filesystems sync: 0.000 seconds
[ 1033.104327] Freezing user space processes ... (elapsed 0.005 seconds) 
done.
[ 1033.117406] OOM killer disabled.
[ 1033.120713] Freezing remaining freezable tasks ... (elapsed 0.001 
seconds) done.
[ 1033.207420] smsc95xx 1-1.1:1.0 eth1: entering SUSPEND2 mode
[ 1033.365054] Disabling non-boot CPUs ...
[ 1033.369460] CPU1: shutdown
[ 1033.372198] psci: CPU1 killed (polled 0 ms)
[ 1033.377338] CPU2: shutdown
[ 1033.380077] psci: CPU2 killed (polled 0 ms)
[ 1033.385079] CPU3: shutdown
[ 1033.387830] psci: CPU3 killed (polled 0 ms)
[ 1033.392617] Enabling non-boot CPUs ...
[ 1033.396924] Detected VIPT I-cache on CPU1
[ 1033.396952] GICv3: CPU1: found redistributor 1 region 
0:0x00000000388a0000
[ 1033.397004] CPU1: Booted secondary processor 0x0000000001 [0x410fd034]
[ 1033.397474] CPU1 is up
[ 1033.417628] Detected VIPT I-cache on CPU2
[ 1033.417645] GICv3: CPU2: found redistributor 2 region 
0:0x00000000388c0000
[ 1033.417671] CPU2: Booted secondary processor 0x0000000002 [0x410fd034]
[ 1033.417987] CPU2 is up
[ 1033.438139] Detected VIPT I-cache on CPU3
[ 1033.438155] GICv3: CPU3: found redistributor 3 region 
0:0x00000000388e0000
[ 1033.438183] CPU3: Booted secondary processor 0x0000000003 [0x410fd034]
[ 1033.438507] CPU3 is up
[ 1054.474194] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[ 1054.480311] rcu:     0-...0: (1 GPs behind) 
idle=732/1/0x4000000000000000 softirq=85335/85336 fqs=2029
[ 1054.489447]  (detected by 3, t=5255 jiffies, g=188005, q=6)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 12/13] arm64: dts: imx8mm: add GPC node and power domains

[Adam Ford]

On Thu, Jan 14, 2021 at 4:39 AM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
>
> On 09.12.20 16:26, Frieder Schrempf wrote:
> > Hi Lucas,
> >
> > On 05.11.20 18:44, Lucas Stach wrote:
> >> This adds the DT nodes to describe the power domains available on the
> >> i.MX8MM. Things are a bit more complex compared to other GPCv2 power
> >> domain setups, as there is now a hierarchy of domains where complete
> >> subsystems (HSIO, GPU, DISPLAY) can be gated as a whole, but also
> >> fine granular gating within those subsystems is possible.
> >>
> >> Note that this is still incomplete, as both VPU and DISP domains are
> >> missing their reset clocks. Those aren't directly sourced from the CCM,
> >> but have another level of clock gating in the BLKCTL of those domains,
> >> which needs a separate driver.
> >>
> >> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >> ---
> >>   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 58 +++++++++++++++++++++++
> >>   1 file changed, 58 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >> index b83f400def8b..c21901a8aea9 100644
> >> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >> @@ -4,6 +4,8 @@
> >>    */
> >>   #include <dt-bindings/clock/imx8mm-clock.h>
> >> +#include <dt-bindings/power/imx8mm-power.h>
> >> +#include <dt-bindings/reset/imx8mq-reset.h>
> >>   #include <dt-bindings/gpio/gpio.h>
> >>   #include <dt-bindings/input/input.h>
> >>   #include <dt-bindings/interrupt-controller/arm-gic.h>
> >> @@ -547,6 +549,62 @@
> >>                   interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> >>                   #reset-cells = <1>;
> >>               };
> >> +
> >> +            gpc: gpc@303a0000 {
> >> +                compatible = "fsl,imx8mm-gpc";
> >> +                reg = <0x303a0000 0x10000>;
> >> +                interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> >> +                interrupt-parent = <&gic>;
> >> +                interrupt-controller;
> >> +                #interrupt-cells = <3>;
> >> +
> >> +                pgc {
> >> +                    #address-cells = <1>;
> >> +                    #size-cells = <0>;
> >> +
> >> +                    pgc_hsiomix: power-domain@0 {
> >> +                        #power-domain-cells = <0>;
> >> +                        reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
> >> +                        clocks = <&clk IMX8MM_CLK_USB_BUS>;
> >> +                    };
> >> +
> >> +                    pgc_pcie: power-domain@1 {
> >> +                        #power-domain-cells = <0>;
> >> +                        reg = <IMX8MM_POWER_DOMAIN_PCIE>;
> >> +                        power-domains = <&pgc_hsiomix>;
> >> +                    };
> >> +
> >> +                    pgc_otg1: power-domain@2 {
> >> +                        #power-domain-cells = <0>;
> >> +                        reg = <IMX8MM_POWER_DOMAIN_OTG1>;
> >> +                        power-domains = <&pgc_hsiomix>;
> >> +                    };
> >> +
> >> +                    pgc_otg2: power-domain@3 {
> >> +                        #power-domain-cells = <0>;
> >> +                        reg = <IMX8MM_POWER_DOMAIN_OTG2>;
> >> +                        power-domains = <&pgc_hsiomix>;
> >> +                    };
> >
> > I'm currently doing some testing on top of v5.10-rc with GPC, BLK-CTL,
> > DSIM, etc. I noticed that as soon as I add the nodes above for HSIO/OTG
> > (even without referencing them elsewhere) my system freezes on
> > suspend/resume.
> >
> > The same problem exists when I remove the power domains for HSIO/USB and
> > add the ones for DISPMIX and DSI to test Marek's work on BLK-CTL.
> >
> > I'm not really sure at what point exactly the system freezes but this is
> > what I see (no_console_suspend is set) and the system does not wake up
> > anymore:
> >
> > echo mem > /sys/power/state
> > [   13.888711] PM: suspend entry (deep)
> > [   13.892429] Filesystems sync: 0.000 seconds
> > [   13.907231] Freezing user space processes ... (elapsed 0.031 seconds)
> > done.
> > [   13.945407] OOM killer disabled.
> > [   13.948642] Freezing remaining freezable tasks ... (elapsed 0.001
> > seconds) done.
> > [   13.957216] printk: Suspending console(s) (use no_console_suspend to
> > debug)
>
> It seems like I failed to set no_console_suspend correctly. Here is a
> proper log with kernel 5.10.6. The system wakes up, but stalls.
>
> Can you reproduce this on your system?
>
[snip]

Frieder / Lucas,

I was able to get similar behavior on the Nano.  I rebased Lucas'
patch on the 5.11 kernel, and applied the corresponding patches to my
Nano board.  It works fine until the system sleeps,  but after it
wakes, even the heartbeat LED stops beating.  I don't know if there is
a conflict between TF-A and the gpc driver in there, or if the gpcv2
driver needs to do something differently to wake the system from
sleep.

# echo mem > /sys/power/state
[ 3754.346162] PM: suspend entry (deep)
[ 3754.349872] Filesystems sync: 0.000 seconds
[ 3754.387641] Freezing user space processes ... (elapsed 0.001 seconds) done.
[ 3754.395890] OOM killer disabled.
[ 3754.399141] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) done.
[ 3754.407674] printk: Suspending console(s) (use no_console_suspend to debug)
[ 3754.992015] Disabling non-boot CPUs ...
[ 3755.027902] i.mx8mm_thermal 30260000.tmu: failed to register
thermal zone sensor[0]: -517
[ 3755.036317] OOM killer enabled.
[ 3755.039467] Restarting tasks ... done.
[ 3755.050669] PM: suspend exit
root@imx8mmevk:~#

Then it hangs.

adam

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 12/13] arm64: dts: imx8mm: add GPC node and power domains

[Adam Ford]

On Thu, Feb 18, 2021 at 6:54 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Thu, Jan 14, 2021 at 4:39 AM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
> >
> > On 09.12.20 16:26, Frieder Schrempf wrote:
> > > Hi Lucas,
> > >
> > > On 05.11.20 18:44, Lucas Stach wrote:
> > >> This adds the DT nodes to describe the power domains available on the
> > >> i.MX8MM. Things are a bit more complex compared to other GPCv2 power
> > >> domain setups, as there is now a hierarchy of domains where complete
> > >> subsystems (HSIO, GPU, DISPLAY) can be gated as a whole, but also
> > >> fine granular gating within those subsystems is possible.
> > >>
> > >> Note that this is still incomplete, as both VPU and DISP domains are
> > >> missing their reset clocks. Those aren't directly sourced from the CCM,
> > >> but have another level of clock gating in the BLKCTL of those domains,
> > >> which needs a separate driver.
> > >>
> > >> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > >> ---
> > >>   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 58 +++++++++++++++++++++++
> > >>   1 file changed, 58 insertions(+)
> > >>
> > >> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > >> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > >> index b83f400def8b..c21901a8aea9 100644
> > >> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > >> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > >> @@ -4,6 +4,8 @@
> > >>    */
> > >>   #include <dt-bindings/clock/imx8mm-clock.h>
> > >> +#include <dt-bindings/power/imx8mm-power.h>
> > >> +#include <dt-bindings/reset/imx8mq-reset.h>
> > >>   #include <dt-bindings/gpio/gpio.h>
> > >>   #include <dt-bindings/input/input.h>
> > >>   #include <dt-bindings/interrupt-controller/arm-gic.h>
> > >> @@ -547,6 +549,62 @@
> > >>                   interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > >>                   #reset-cells = <1>;
> > >>               };
> > >> +
> > >> +            gpc: gpc@303a0000 {
> > >> +                compatible = "fsl,imx8mm-gpc";
> > >> +                reg = <0x303a0000 0x10000>;
> > >> +                interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> > >> +                interrupt-parent = <&gic>;
> > >> +                interrupt-controller;
> > >> +                #interrupt-cells = <3>;
> > >> +
> > >> +                pgc {
> > >> +                    #address-cells = <1>;
> > >> +                    #size-cells = <0>;
> > >> +
> > >> +                    pgc_hsiomix: power-domain@0 {
> > >> +                        #power-domain-cells = <0>;
> > >> +                        reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
> > >> +                        clocks = <&clk IMX8MM_CLK_USB_BUS>;
> > >> +                    };
> > >> +
> > >> +                    pgc_pcie: power-domain@1 {
> > >> +                        #power-domain-cells = <0>;
> > >> +                        reg = <IMX8MM_POWER_DOMAIN_PCIE>;
> > >> +                        power-domains = <&pgc_hsiomix>;
> > >> +                    };
> > >> +
> > >> +                    pgc_otg1: power-domain@2 {
> > >> +                        #power-domain-cells = <0>;
> > >> +                        reg = <IMX8MM_POWER_DOMAIN_OTG1>;
> > >> +                        power-domains = <&pgc_hsiomix>;
> > >> +                    };
> > >> +
> > >> +                    pgc_otg2: power-domain@3 {
> > >> +                        #power-domain-cells = <0>;
> > >> +                        reg = <IMX8MM_POWER_DOMAIN_OTG2>;
> > >> +                        power-domains = <&pgc_hsiomix>;
> > >> +                    };
> > >
> > > I'm currently doing some testing on top of v5.10-rc with GPC, BLK-CTL,
> > > DSIM, etc. I noticed that as soon as I add the nodes above for HSIO/OTG
> > > (even without referencing them elsewhere) my system freezes on
> > > suspend/resume.
> > >
> > > The same problem exists when I remove the power domains for HSIO/USB and
> > > add the ones for DISPMIX and DSI to test Marek's work on BLK-CTL.
> > >
> > > I'm not really sure at what point exactly the system freezes but this is
> > > what I see (no_console_suspend is set) and the system does not wake up
> > > anymore:
> > >
> > > echo mem > /sys/power/state
> > > [   13.888711] PM: suspend entry (deep)
> > > [   13.892429] Filesystems sync: 0.000 seconds
> > > [   13.907231] Freezing user space processes ... (elapsed 0.031 seconds)
> > > done.
> > > [   13.945407] OOM killer disabled.
> > > [   13.948642] Freezing remaining freezable tasks ... (elapsed 0.001
> > > seconds) done.
> > > [   13.957216] printk: Suspending console(s) (use no_console_suspend to
> > > debug)
> >
> > It seems like I failed to set no_console_suspend correctly. Here is a
> > proper log with kernel 5.10.6. The system wakes up, but stalls.
> >
> > Can you reproduce this on your system?
> >
> [snip]
>
> Frieder / Lucas,
>
> I was able to get similar behavior on the Nano.  I rebased Lucas'
> patch on the 5.11 kernel, and applied the corresponding patches to my
> Nano board.  It works fine until the system sleeps,  but after it
> wakes, even the heartbeat LED stops beating.  I don't know if there is
> a conflict between TF-A and the gpc driver in there, or if the gpcv2
> driver needs to do something differently to wake the system from
> sleep.
>
> # echo mem > /sys/power/state
> [ 3754.346162] PM: suspend entry (deep)
> [ 3754.349872] Filesystems sync: 0.000 seconds
> [ 3754.387641] Freezing user space processes ... (elapsed 0.001 seconds) done.
> [ 3754.395890] OOM killer disabled.
> [ 3754.399141] Freezing remaining freezable tasks ... (elapsed 0.001
> seconds) done.
> [ 3754.407674] printk: Suspending console(s) (use no_console_suspend to debug)
> [ 3754.992015] Disabling non-boot CPUs ...
> [ 3755.027902] i.mx8mm_thermal 30260000.tmu: failed to register
> thermal zone sensor[0]: -517
> [ 3755.036317] OOM killer enabled.
> [ 3755.039467] Restarting tasks ... done.
> [ 3755.050669] PM: suspend exit
> root@imx8mmevk:~#
>
> Then it hangs.

I did some additional digging on both the Mini and Nano.  Both of
their reference manuals have example code which enables and disables
CM_CCGR(69), hsio_bus clock when enabling/disabling the power domains.
It looks like the offset for CCM_CCGR69 is 0x4450.  This clock appears
to be enabled by default at boot, however, there doesn't appear to be
a clock entry for this.

I tried to create one based on the 8MP that looks like:
    hws[IMX8MN_CLK_HSIO_ROOT] = imx_clk_hw_gate4("hsio_root_clk",
"ipg_root", base + 0x4450, 0);

I then added the hsio clock to the clock table and reference it from
the power-domain node, but at boot it appears to hang at boot, so I am
not sure I did it correctly.

From what I can tell, neither the NXP kernel nor their ATF show a
reference to CCGR(69),  so I also wonder if the TRM for either Mini or
Nano is correct.

adam

>
> adam

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 12/13] arm64: dts: imx8mm: add GPC node and power domains

[Frieder Schrempf]

On 18.02.21 16:19, Adam Ford wrote:
> On Thu, Feb 18, 2021 at 6:54 AM Adam Ford <aford173@gmail.com> wrote:
>>
>> On Thu, Jan 14, 2021 at 4:39 AM Frieder Schrempf
>> <frieder.schrempf@kontron.de> wrote:
>>>
>>> On 09.12.20 16:26, Frieder Schrempf wrote:
>>>> Hi Lucas,
>>>>
>>>> On 05.11.20 18:44, Lucas Stach wrote:
>>>>> This adds the DT nodes to describe the power domains available on the
>>>>> i.MX8MM. Things are a bit more complex compared to other GPCv2 power
>>>>> domain setups, as there is now a hierarchy of domains where complete
>>>>> subsystems (HSIO, GPU, DISPLAY) can be gated as a whole, but also
>>>>> fine granular gating within those subsystems is possible.
>>>>>
>>>>> Note that this is still incomplete, as both VPU and DISP domains are
>>>>> missing their reset clocks. Those aren't directly sourced from the CCM,
>>>>> but have another level of clock gating in the BLKCTL of those domains,
>>>>> which needs a separate driver.
>>>>>
>>>>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>>>>> ---
>>>>>    arch/arm64/boot/dts/freescale/imx8mm.dtsi | 58 +++++++++++++++++++++++
>>>>>    1 file changed, 58 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
>>>>> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
>>>>> index b83f400def8b..c21901a8aea9 100644
>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
>>>>> @@ -4,6 +4,8 @@
>>>>>     */
>>>>>    #include <dt-bindings/clock/imx8mm-clock.h>
>>>>> +#include <dt-bindings/power/imx8mm-power.h>
>>>>> +#include <dt-bindings/reset/imx8mq-reset.h>
>>>>>    #include <dt-bindings/gpio/gpio.h>
>>>>>    #include <dt-bindings/input/input.h>
>>>>>    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> @@ -547,6 +549,62 @@
>>>>>                    interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
>>>>>                    #reset-cells = <1>;
>>>>>                };
>>>>> +
>>>>> +            gpc: gpc@303a0000 {
>>>>> +                compatible = "fsl,imx8mm-gpc";
>>>>> +                reg = <0x303a0000 0x10000>;
>>>>> +                interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>>>>> +                interrupt-parent = <&gic>;
>>>>> +                interrupt-controller;
>>>>> +                #interrupt-cells = <3>;
>>>>> +
>>>>> +                pgc {
>>>>> +                    #address-cells = <1>;
>>>>> +                    #size-cells = <0>;
>>>>> +
>>>>> +                    pgc_hsiomix: power-domain@0 {
>>>>> +                        #power-domain-cells = <0>;
>>>>> +                        reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
>>>>> +                        clocks = <&clk IMX8MM_CLK_USB_BUS>;
>>>>> +                    };
>>>>> +
>>>>> +                    pgc_pcie: power-domain@1 {
>>>>> +                        #power-domain-cells = <0>;
>>>>> +                        reg = <IMX8MM_POWER_DOMAIN_PCIE>;
>>>>> +                        power-domains = <&pgc_hsiomix>;
>>>>> +                    };
>>>>> +
>>>>> +                    pgc_otg1: power-domain@2 {
>>>>> +                        #power-domain-cells = <0>;
>>>>> +                        reg = <IMX8MM_POWER_DOMAIN_OTG1>;
>>>>> +                        power-domains = <&pgc_hsiomix>;
>>>>> +                    };
>>>>> +
>>>>> +                    pgc_otg2: power-domain@3 {
>>>>> +                        #power-domain-cells = <0>;
>>>>> +                        reg = <IMX8MM_POWER_DOMAIN_OTG2>;
>>>>> +                        power-domains = <&pgc_hsiomix>;
>>>>> +                    };
>>>>
>>>> I'm currently doing some testing on top of v5.10-rc with GPC, BLK-CTL,
>>>> DSIM, etc. I noticed that as soon as I add the nodes above for HSIO/OTG
>>>> (even without referencing them elsewhere) my system freezes on
>>>> suspend/resume.
>>>>
>>>> The same problem exists when I remove the power domains for HSIO/USB and
>>>> add the ones for DISPMIX and DSI to test Marek's work on BLK-CTL.
>>>>
>>>> I'm not really sure at what point exactly the system freezes but this is
>>>> what I see (no_console_suspend is set) and the system does not wake up
>>>> anymore:
>>>>
>>>> echo mem > /sys/power/state
>>>> [   13.888711] PM: suspend entry (deep)
>>>> [   13.892429] Filesystems sync: 0.000 seconds
>>>> [   13.907231] Freezing user space processes ... (elapsed 0.031 seconds)
>>>> done.
>>>> [   13.945407] OOM killer disabled.
>>>> [   13.948642] Freezing remaining freezable tasks ... (elapsed 0.001
>>>> seconds) done.
>>>> [   13.957216] printk: Suspending console(s) (use no_console_suspend to
>>>> debug)
>>>
>>> It seems like I failed to set no_console_suspend correctly. Here is a
>>> proper log with kernel 5.10.6. The system wakes up, but stalls.
>>>
>>> Can you reproduce this on your system?
>>>
>> [snip]
>>
>> Frieder / Lucas,
>>
>> I was able to get similar behavior on the Nano.  I rebased Lucas'
>> patch on the 5.11 kernel, and applied the corresponding patches to my
>> Nano board.  It works fine until the system sleeps,  but after it
>> wakes, even the heartbeat LED stops beating.  I don't know if there is
>> a conflict between TF-A and the gpc driver in there, or if the gpcv2
>> driver needs to do something differently to wake the system from
>> sleep.
>>
>> # echo mem > /sys/power/state
>> [ 3754.346162] PM: suspend entry (deep)
>> [ 3754.349872] Filesystems sync: 0.000 seconds
>> [ 3754.387641] Freezing user space processes ... (elapsed 0.001 seconds) done.
>> [ 3754.395890] OOM killer disabled.
>> [ 3754.399141] Freezing remaining freezable tasks ... (elapsed 0.001
>> seconds) done.
>> [ 3754.407674] printk: Suspending console(s) (use no_console_suspend to debug)
>> [ 3754.992015] Disabling non-boot CPUs ...
>> [ 3755.027902] i.mx8mm_thermal 30260000.tmu: failed to register
>> thermal zone sensor[0]: -517
>> [ 3755.036317] OOM killer enabled.
>> [ 3755.039467] Restarting tasks ... done.
>> [ 3755.050669] PM: suspend exit
>> root@imx8mmevk:~#
>>
>> Then it hangs.
> 
> I did some additional digging on both the Mini and Nano.  Both of
> their reference manuals have example code which enables and disables
> CM_CCGR(69), hsio_bus clock when enabling/disabling the power domains.
> It looks like the offset for CCM_CCGR69 is 0x4450.  This clock appears
> to be enabled by default at boot, however, there doesn't appear to be
> a clock entry for this.
> 
> I tried to create one based on the 8MP that looks like:
>      hws[IMX8MN_CLK_HSIO_ROOT] = imx_clk_hw_gate4("hsio_root_clk",
> "ipg_root", base + 0x4450, 0);
> 
> I then added the hsio clock to the clock table and reference it from
> the power-domain node, but at boot it appears to hang at boot, so I am
> not sure I did it correctly.
> 
>  From what I can tell, neither the NXP kernel nor their ATF show a
> reference to CCGR(69),  so I also wonder if the TRM for either Mini or
> Nano is correct.

I really can't tell if the missing HSIO root clock gate is a problem or 
not. It's listed in the RM in table 5-9 as "SIM_HSIO". And it's listed 
in table 6-7 ("CCGR setting by ROM") as "Bus clock. Do not gate off."

For some reason it's **not** listed in table 5-2 ("System Clocks and 
Gating)", where I expected to find the clock root.

We also still have issues with the kernel locking up early in the boot 
using the GPC power domain driver, but only with some of our boards. I 
don't know if this is related to the lockup after resume.

I also noticed that when I tried to use USB without the power domain 
support in the GPC driver, it worked with the mainline TF-A, but it 
locked up with the NXP TF-A on USB probing.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 12/13] arm64: dts: imx8mm: add GPC node and power domains

[Adam Ford]

On Tue, Mar 2, 2021 at 9:01 AM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
>
> On 18.02.21 16:19, Adam Ford wrote:
> > On Thu, Feb 18, 2021 at 6:54 AM Adam Ford <aford173@gmail.com> wrote:
> >>
> >> On Thu, Jan 14, 2021 at 4:39 AM Frieder Schrempf
> >> <frieder.schrempf@kontron.de> wrote:
> >>>
> >>> On 09.12.20 16:26, Frieder Schrempf wrote:
> >>>> Hi Lucas,
> >>>>
> >>>> On 05.11.20 18:44, Lucas Stach wrote:
> >>>>> This adds the DT nodes to describe the power domains available on the
> >>>>> i.MX8MM. Things are a bit more complex compared to other GPCv2 power
> >>>>> domain setups, as there is now a hierarchy of domains where complete
> >>>>> subsystems (HSIO, GPU, DISPLAY) can be gated as a whole, but also
> >>>>> fine granular gating within those subsystems is possible.
> >>>>>
> >>>>> Note that this is still incomplete, as both VPU and DISP domains are
> >>>>> missing their reset clocks. Those aren't directly sourced from the CCM,
> >>>>> but have another level of clock gating in the BLKCTL of those domains,
> >>>>> which needs a separate driver.
> >>>>>
> >>>>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >>>>> ---
> >>>>>    arch/arm64/boot/dts/freescale/imx8mm.dtsi | 58 +++++++++++++++++++++++
> >>>>>    1 file changed, 58 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >>>>> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >>>>> index b83f400def8b..c21901a8aea9 100644
> >>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >>>>> @@ -4,6 +4,8 @@
> >>>>>     */
> >>>>>    #include <dt-bindings/clock/imx8mm-clock.h>
> >>>>> +#include <dt-bindings/power/imx8mm-power.h>
> >>>>> +#include <dt-bindings/reset/imx8mq-reset.h>
> >>>>>    #include <dt-bindings/gpio/gpio.h>
> >>>>>    #include <dt-bindings/input/input.h>
> >>>>>    #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>>>> @@ -547,6 +549,62 @@
> >>>>>                    interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> >>>>>                    #reset-cells = <1>;
> >>>>>                };
> >>>>> +
> >>>>> +            gpc: gpc@303a0000 {
> >>>>> +                compatible = "fsl,imx8mm-gpc";
> >>>>> +                reg = <0x303a0000 0x10000>;
> >>>>> +                interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> >>>>> +                interrupt-parent = <&gic>;
> >>>>> +                interrupt-controller;
> >>>>> +                #interrupt-cells = <3>;
> >>>>> +
> >>>>> +                pgc {
> >>>>> +                    #address-cells = <1>;
> >>>>> +                    #size-cells = <0>;
> >>>>> +
> >>>>> +                    pgc_hsiomix: power-domain@0 {
> >>>>> +                        #power-domain-cells = <0>;
> >>>>> +                        reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
> >>>>> +                        clocks = <&clk IMX8MM_CLK_USB_BUS>;
> >>>>> +                    };
> >>>>> +
> >>>>> +                    pgc_pcie: power-domain@1 {
> >>>>> +                        #power-domain-cells = <0>;
> >>>>> +                        reg = <IMX8MM_POWER_DOMAIN_PCIE>;
> >>>>> +                        power-domains = <&pgc_hsiomix>;
> >>>>> +                    };
> >>>>> +
> >>>>> +                    pgc_otg1: power-domain@2 {
> >>>>> +                        #power-domain-cells = <0>;
> >>>>> +                        reg = <IMX8MM_POWER_DOMAIN_OTG1>;
> >>>>> +                        power-domains = <&pgc_hsiomix>;
> >>>>> +                    };
> >>>>> +
> >>>>> +                    pgc_otg2: power-domain@3 {
> >>>>> +                        #power-domain-cells = <0>;
> >>>>> +                        reg = <IMX8MM_POWER_DOMAIN_OTG2>;
> >>>>> +                        power-domains = <&pgc_hsiomix>;
> >>>>> +                    };
> >>>>
> >>>> I'm currently doing some testing on top of v5.10-rc with GPC, BLK-CTL,
> >>>> DSIM, etc. I noticed that as soon as I add the nodes above for HSIO/OTG
> >>>> (even without referencing them elsewhere) my system freezes on
> >>>> suspend/resume.
> >>>>
> >>>> The same problem exists when I remove the power domains for HSIO/USB and
> >>>> add the ones for DISPMIX and DSI to test Marek's work on BLK-CTL.
> >>>>
> >>>> I'm not really sure at what point exactly the system freezes but this is
> >>>> what I see (no_console_suspend is set) and the system does not wake up
> >>>> anymore:
> >>>>
> >>>> echo mem > /sys/power/state
> >>>> [   13.888711] PM: suspend entry (deep)
> >>>> [   13.892429] Filesystems sync: 0.000 seconds
> >>>> [   13.907231] Freezing user space processes ... (elapsed 0.031 seconds)
> >>>> done.
> >>>> [   13.945407] OOM killer disabled.
> >>>> [   13.948642] Freezing remaining freezable tasks ... (elapsed 0.001
> >>>> seconds) done.
> >>>> [   13.957216] printk: Suspending console(s) (use no_console_suspend to
> >>>> debug)
> >>>
> >>> It seems like I failed to set no_console_suspend correctly. Here is a
> >>> proper log with kernel 5.10.6. The system wakes up, but stalls.
> >>>
> >>> Can you reproduce this on your system?
> >>>
> >> [snip]
> >>
> >> Frieder / Lucas,
> >>
> >> I was able to get similar behavior on the Nano.  I rebased Lucas'
> >> patch on the 5.11 kernel, and applied the corresponding patches to my
> >> Nano board.  It works fine until the system sleeps,  but after it
> >> wakes, even the heartbeat LED stops beating.  I don't know if there is
> >> a conflict between TF-A and the gpc driver in there, or if the gpcv2
> >> driver needs to do something differently to wake the system from
> >> sleep.
> >>
> >> # echo mem > /sys/power/state
> >> [ 3754.346162] PM: suspend entry (deep)
> >> [ 3754.349872] Filesystems sync: 0.000 seconds
> >> [ 3754.387641] Freezing user space processes ... (elapsed 0.001 seconds) done.
> >> [ 3754.395890] OOM killer disabled.
> >> [ 3754.399141] Freezing remaining freezable tasks ... (elapsed 0.001
> >> seconds) done.
> >> [ 3754.407674] printk: Suspending console(s) (use no_console_suspend to debug)
> >> [ 3754.992015] Disabling non-boot CPUs ...
> >> [ 3755.027902] i.mx8mm_thermal 30260000.tmu: failed to register
> >> thermal zone sensor[0]: -517
> >> [ 3755.036317] OOM killer enabled.
> >> [ 3755.039467] Restarting tasks ... done.
> >> [ 3755.050669] PM: suspend exit
> >> root@imx8mmevk:~#
> >>
> >> Then it hangs.
> >
> > I did some additional digging on both the Mini and Nano.  Both of
> > their reference manuals have example code which enables and disables
> > CM_CCGR(69), hsio_bus clock when enabling/disabling the power domains.
> > It looks like the offset for CCM_CCGR69 is 0x4450.  This clock appears
> > to be enabled by default at boot, however, there doesn't appear to be
> > a clock entry for this.
> >
> > I tried to create one based on the 8MP that looks like:
> >      hws[IMX8MN_CLK_HSIO_ROOT] = imx_clk_hw_gate4("hsio_root_clk",
> > "ipg_root", base + 0x4450, 0);
> >
> > I then added the hsio clock to the clock table and reference it from
> > the power-domain node, but at boot it appears to hang at boot, so I am
> > not sure I did it correctly.
> >
> >  From what I can tell, neither the NXP kernel nor their ATF show a
> > reference to CCGR(69),  so I also wonder if the TRM for either Mini or
> > Nano is correct.
>
> I really can't tell if the missing HSIO root clock gate is a problem or
> not. It's listed in the RM in table 5-9 as "SIM_HSIO". And it's listed
> in table 6-7 ("CCGR setting by ROM") as "Bus clock. Do not gate off."

I didn't notice the 'Do note gate off' comment, so I'm guessing its
better to leave it alone if it's on my default and shouldn't be turned
off.  It does trouble me that the Example code in the backs of the TRM
reference it if we're not really supposed to do that.

>
> For some reason it's **not** listed in table 5-2 ("System Clocks and
> Gating)", where I expected to find the clock root.
>
> We also still have issues with the kernel locking up early in the boot
> using the GPC power domain driver, but only with some of our boards. I
> don't know if this is related to the lockup after resume.
>
> I also noticed that when I tried to use USB without the power domain
> support in the GPC driver, it worked with the mainline TF-A, but it
> locked up with the NXP TF-A on USB probing.

Looking at the NXP TF-A, they don't appear to go through the effort of
shutting down the USB.

    pu_domain_status &= ~(1 << domain_id);
    if (domain_id == OTG1 || domain_id == OTG2)
        return;

Based on that, I attempted to modify Lucas' power-domain code to add
an additional flag which could determine whether or not we want to
shut it down or not.  Using that additional flag, I was able to get
the system to wake from sleep without hanging, but USB wasn't
functional after waking.

adam

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 13/13] arm64: dts: imx8mm: put USB controllers into power-domains

[Lucas Stach]

Now that we have support for the power domain controller on the i.MX8MM
we can put the USB controllers in their respective power domains to allow
them to power down the PHY when possible.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c21901a8aea9..6fba10ad7636 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -953,6 +953,7 @@
 				assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
 				fsl,usbphy = <&usbphynop1>;
 				fsl,usbmisc = <&usbmisc1 0>;
+				power-domains = <&pgc_otg1>;
 				status = "disabled";
 			};
 
@@ -972,6 +973,7 @@
 				assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
 				fsl,usbphy = <&usbphynop2>;
 				fsl,usbmisc = <&usbmisc2 0>;
+				power-domains = <&pgc_otg2>;
 				status = "disabled";
 			};
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel