[PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[Marek Vasut]

In case the power domain clock are ungated before the reset is asserted,
the system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
domains require different reset deassertion timing, and incorrect reset
deassertion timing also leads to hang.

Add per-domain reset_{,de}assert_early flags which allow fine-grained
control of the reset assertion and deassertion sequence. Currently, on
MX8MM, the behavior is as follows and aligned with NXP downstream ATF
fork:
- VPUMIX: reset assert, reset deassert, domain power up
- GPUMIX: reset assert, domain power on, reset deassert

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Frieder Schrempf <frieder.schrempf@kontron.de>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Shawn Guo <shawnguo@kernel.org>
To: linux-arm-kernel@lists.infradead.org
---
 drivers/soc/imx/gpcv2.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 34a9ac1f2b9b1..388c4c729c95b 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -201,6 +201,9 @@ struct imx_pgc_domain {
 		u32 hskack;
 	} bits;
 
+	bool reset_assert_early;
+	bool reset_deassert_early;
+
 	const int voltage;
 	struct device *dev;
 };
@@ -237,6 +240,17 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 		}
 	}
 
+	/* delays for reset to propagate */
+	if (domain->reset_assert_early) {
+		reset_control_assert(domain->reset);
+		udelay(5);
+	}
+
+	if (domain->reset_deassert_early) {
+		reset_control_deassert(domain->reset);
+		udelay(5);
+	}
+
 	/* Enable reset clocks for all devices in the domain */
 	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
 	if (ret) {
@@ -245,6 +259,10 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 	}
 
 	if (domain->bits.pxx) {
+		/* disable power control */
+		regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
+				  GPC_PGC_CTRL_PCR);
+
 		/* request the domain to power up */
 		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
 				   domain->bits.pxx, domain->bits.pxx);
@@ -260,18 +278,19 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 			dev_err(domain->dev, "failed to command PGC\n");
 			goto out_clk_disable;
 		}
-
-		/* disable power control */
-		regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
-				  GPC_PGC_CTRL_PCR);
 	}
 
-	reset_control_assert(domain->reset);
 
-	/* delay for reset to propagate */
-	udelay(5);
+	/* delays for reset to propagate */
+	if (!domain->reset_assert_early) {
+		reset_control_assert(domain->reset);
+		udelay(5);
+	}
 
-	reset_control_deassert(domain->reset);
+	if (!domain->reset_deassert_early) {
+		reset_control_deassert(domain->reset);
+		udelay(5);
+	}
 
 	/* request the ADB400 to power up */
 	if (domain->bits.hskreq) {
@@ -676,6 +695,9 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
 		},
 		.pgc   = IMX8MM_PGC_GPU2D,
+		/* Assert reset, power up domain, deassert reset */
+		.reset_assert_early = true,
+		.reset_deassert_early = false,
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUMIX] = {
@@ -689,6 +711,9 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
 			.hskack = IMX8MM_VPUMIX_HSK_PWRDNACKN,
 		},
 		.pgc   = IMX8MM_PGC_VPUMIX,
+		/* Assert reset, deassert reset, power up domain */
+		.reset_assert_early = true,
+		.reset_deassert_early = true,
 	},
 
 	[IMX8MM_POWER_DOMAIN_VPUG1] = {
-- 
2.30.2


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

RE: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[Peng Fan]

> Subject: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock
> 
> In case the power domain clock are ungated before the reset is asserted, the
> system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
> domains require different reset deassertion timing, and incorrect reset
> deassertion timing also leads to hang.
> 
> Add per-domain reset_{,de}assert_early flags which allow fine-grained control
> of the reset assertion and deassertion sequence. Currently, on MX8MM, the
> behavior is as follows and aligned with NXP downstream ATF
> fork:
> - VPUMIX: reset assert, reset deassert, domain power up
> - GPUMIX: reset assert, domain power on, reset deassert

In 2.4 ATF, only VPU_H1/G1/G2 needs reset assert/deassert early,
but actually in first power on, it is in reset assert, so I think no need
reset assert again. I think only need to reset assert when power off.

Would the following patch help you hang case?

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 34a9ac1f2b9b..b5f9daff8b1c 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -358,6 +358,8 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
                }
        }

+       reset_control_assert(domain->reset);
+
        /* Disable reset clocks for all devices in the domain */
        clk_bulk_disable_unprepare(domain->num_clks, domain->clks);

Thanks,
Peng.

> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Frieder Schrempf <frieder.schrempf@kontron.de>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> To: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/soc/imx/gpcv2.c | 41
> +++++++++++++++++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c index
> 34a9ac1f2b9b1..388c4c729c95b 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -201,6 +201,9 @@ struct imx_pgc_domain {
>  		u32 hskack;
>  	} bits;
> 
> +	bool reset_assert_early;
> +	bool reset_deassert_early;
> +
>  	const int voltage;
>  	struct device *dev;
>  };
> @@ -237,6 +240,17 @@ static int imx_pgc_power_up(struct
> generic_pm_domain *genpd)
>  		}
>  	}
> 
> +	/* delays for reset to propagate */
> +	if (domain->reset_assert_early) {
> +		reset_control_assert(domain->reset);
> +		udelay(5);
> +	}
> +
> +	if (domain->reset_deassert_early) {
> +		reset_control_deassert(domain->reset);
> +		udelay(5);
> +	}
> +
>  	/* Enable reset clocks for all devices in the domain */
>  	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
>  	if (ret) {
> @@ -245,6 +259,10 @@ static int imx_pgc_power_up(struct
> generic_pm_domain *genpd)
>  	}
> 
>  	if (domain->bits.pxx) {
> +		/* disable power control */
> +		regmap_clear_bits(domain->regmap,
> GPC_PGC_CTRL(domain->pgc),
> +				  GPC_PGC_CTRL_PCR);
> +
>  		/* request the domain to power up */
>  		regmap_update_bits(domain->regmap,
> GPC_PU_PGC_SW_PUP_REQ,
>  				   domain->bits.pxx, domain->bits.pxx); @@ -260,18
> +278,19 @@ static int imx_pgc_power_up(struct generic_pm_domain
> *genpd)
>  			dev_err(domain->dev, "failed to command PGC\n");
>  			goto out_clk_disable;
>  		}
> -
> -		/* disable power control */
> -		regmap_clear_bits(domain->regmap,
> GPC_PGC_CTRL(domain->pgc),
> -				  GPC_PGC_CTRL_PCR);
>  	}
> 
> -	reset_control_assert(domain->reset);
> 
> -	/* delay for reset to propagate */
> -	udelay(5);
> +	/* delays for reset to propagate */
> +	if (!domain->reset_assert_early) {
> +		reset_control_assert(domain->reset);
> +		udelay(5);
> +	}
> 
> -	reset_control_deassert(domain->reset);
> +	if (!domain->reset_deassert_early) {
> +		reset_control_deassert(domain->reset);
> +		udelay(5);
> +	}
> 
>  	/* request the ADB400 to power up */
>  	if (domain->bits.hskreq) {
> @@ -676,6 +695,9 @@ static const struct imx_pgc_domain
> imx8mm_pgc_domains[] = {
>  			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
>  		},
>  		.pgc   = IMX8MM_PGC_GPU2D,
> +		/* Assert reset, power up domain, deassert reset */
> +		.reset_assert_early = true,
> +		.reset_deassert_early = false,
>  	},
> 
>  	[IMX8MM_POWER_DOMAIN_VPUMIX] = {
> @@ -689,6 +711,9 @@ static const struct imx_pgc_domain
> imx8mm_pgc_domains[] = {
>  			.hskack = IMX8MM_VPUMIX_HSK_PWRDNACKN,
>  		},
>  		.pgc   = IMX8MM_PGC_VPUMIX,
> +		/* Assert reset, deassert reset, power up domain */
> +		.reset_assert_early = true,
> +		.reset_deassert_early = true,
>  	},
> 
>  	[IMX8MM_POWER_DOMAIN_VPUG1] = {
> --
> 2.30.2


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

Re: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[Marek Vasut]

On 7/1/21 3:53 AM, Peng Fan wrote:
>> Subject: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock
>>
>> In case the power domain clock are ungated before the reset is asserted, the
>> system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
>> domains require different reset deassertion timing, and incorrect reset
>> deassertion timing also leads to hang.
>>
>> Add per-domain reset_{,de}assert_early flags which allow fine-grained control
>> of the reset assertion and deassertion sequence. Currently, on MX8MM, the
>> behavior is as follows and aligned with NXP downstream ATF
>> fork:
>> - VPUMIX: reset assert, reset deassert, domain power up
>> - GPUMIX: reset assert, domain power on, reset deassert
> 
> In 2.4 ATF, only VPU_H1/G1/G2 needs reset assert/deassert early,
> but actually in first power on, it is in reset assert, so I think no need
> reset assert again. I think only need to reset assert when power off.
> 
> Would the following patch help you hang case?

No, in my case the VPU hangs in imx_pgc_power_up() , and it generally 
happens after multiple reboots, so I think the state of the hardware is 
undefined. That's why I think we should assert the reset (like the ATF 
does), to make sure the hardware is in defined state, before operating 
the power domain controls.

What I find puzzling is why each MIX domain has different requirements 
for the placement of reset assert/deassert calls. It is that way in ATF, 
but it is still odd.

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

Re: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[Lucas Stach]

Hi Marek,

Am Donnerstag, dem 01.07.2021 um 00:59 +0200 schrieb Marek Vasut:
> In case the power domain clock are ungated before the reset is asserted,
> the system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
> domains require different reset deassertion timing, and incorrect reset
> deassertion timing also leads to hang.
> 
> Add per-domain reset_{,de}assert_early flags which allow fine-grained
> control of the reset assertion and deassertion sequence. Currently, on
> MX8MM, the behavior is as follows and aligned with NXP downstream ATF
> fork:
> - VPUMIX: reset assert, reset deassert, domain power up
> - GPUMIX: reset assert, domain power on, reset deassert
> 
This patch should now be necessary, as my testing over the last few
days showed that the VPUMIX isn't actually different and copes just
fine with the reset being asserted early, just like the GPUMIX domain.

Regards,
Lucas

> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Frieder Schrempf <frieder.schrempf@kontron.de>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> To: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/soc/imx/gpcv2.c | 41 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 34a9ac1f2b9b1..388c4c729c95b 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -201,6 +201,9 @@ struct imx_pgc_domain {
>  		u32 hskack;
>  	} bits;
>  
> +	bool reset_assert_early;
> +	bool reset_deassert_early;
> +
>  	const int voltage;
>  	struct device *dev;
>  };
> @@ -237,6 +240,17 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>  		}
>  	}
>  
> +	/* delays for reset to propagate */
> +	if (domain->reset_assert_early) {
> +		reset_control_assert(domain->reset);
> +		udelay(5);
> +	}
> +
> +	if (domain->reset_deassert_early) {
> +		reset_control_deassert(domain->reset);
> +		udelay(5);
> +	}
> +
>  	/* Enable reset clocks for all devices in the domain */
>  	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
>  	if (ret) {
> @@ -245,6 +259,10 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>  	}
>  
>  	if (domain->bits.pxx) {
> +		/* disable power control */
> +		regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> +				  GPC_PGC_CTRL_PCR);
> +
>  		/* request the domain to power up */
>  		regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
>  				   domain->bits.pxx, domain->bits.pxx);
> @@ -260,18 +278,19 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>  			dev_err(domain->dev, "failed to command PGC\n");
>  			goto out_clk_disable;
>  		}
> -
> -		/* disable power control */
> -		regmap_clear_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> -				  GPC_PGC_CTRL_PCR);
>  	}
>  
> -	reset_control_assert(domain->reset);
>  
> -	/* delay for reset to propagate */
> -	udelay(5);
> +	/* delays for reset to propagate */
> +	if (!domain->reset_assert_early) {
> +		reset_control_assert(domain->reset);
> +		udelay(5);
> +	}
>  
> -	reset_control_deassert(domain->reset);
> +	if (!domain->reset_deassert_early) {
> +		reset_control_deassert(domain->reset);
> +		udelay(5);
> +	}
>  
>  	/* request the ADB400 to power up */
>  	if (domain->bits.hskreq) {
> @@ -676,6 +695,9 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
>  			.hskack = IMX8MM_GPU_HSK_PWRDNACKN,
>  		},
>  		.pgc   = IMX8MM_PGC_GPU2D,
> +		/* Assert reset, power up domain, deassert reset */
> +		.reset_assert_early = true,
> +		.reset_deassert_early = false,
>  	},
>  
>  	[IMX8MM_POWER_DOMAIN_VPUMIX] = {
> @@ -689,6 +711,9 @@ static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
>  			.hskack = IMX8MM_VPUMIX_HSK_PWRDNACKN,
>  		},
>  		.pgc   = IMX8MM_PGC_VPUMIX,
> +		/* Assert reset, deassert reset, power up domain */
> +		.reset_assert_early = true,
> +		.reset_deassert_early = true,
>  	},
>  
>  	[IMX8MM_POWER_DOMAIN_VPUG1] = {



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

Re: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[Marek Vasut]

On 7/17/21 1:32 AM, Lucas Stach wrote:
> Hi Marek,

Hi,

> Am Donnerstag, dem 01.07.2021 um 00:59 +0200 schrieb Marek Vasut:
>> In case the power domain clock are ungated before the reset is asserted,
>> the system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
>> domains require different reset deassertion timing, and incorrect reset
>> deassertion timing also leads to hang.
>>
>> Add per-domain reset_{,de}assert_early flags which allow fine-grained
>> control of the reset assertion and deassertion sequence. Currently, on
>> MX8MM, the behavior is as follows and aligned with NXP downstream ATF
>> fork:
>> - VPUMIX: reset assert, reset deassert, domain power up
>> - GPUMIX: reset assert, domain power on, reset deassert
>>
> This patch should now be necessary, as my testing over the last few
> days showed that the VPUMIX isn't actually different and copes just
> fine with the reset being asserted early, just like the GPUMIX domain.

Yes, this patch is absolutely essential, otherwise the system hangs at 
random, as explained in the commit message.

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

Re: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[Lucas Stach]

Am Samstag, dem 17.07.2021 um 02:55 +0200 schrieb Marek Vasut:
> On 7/17/21 1:32 AM, Lucas Stach wrote:
> > Hi Marek,
> 
> Hi,
> 
> > Am Donnerstag, dem 01.07.2021 um 00:59 +0200 schrieb Marek Vasut:
> > > In case the power domain clock are ungated before the reset is asserted,
> > > the system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
> > > domains require different reset deassertion timing, and incorrect reset
> > > deassertion timing also leads to hang.
> > > 
> > > Add per-domain reset_{,de}assert_early flags which allow fine-grained
> > > control of the reset assertion and deassertion sequence. Currently, on
> > > MX8MM, the behavior is as follows and aligned with NXP downstream ATF
> > > fork:
> > > - VPUMIX: reset assert, reset deassert, domain power up
> > > - GPUMIX: reset assert, domain power on, reset deassert
> > > 
> > This patch should now be necessary, as my testing over the last few
> > days showed that the VPUMIX isn't actually different and copes just
> > fine with the reset being asserted early, just like the GPUMIX domain.
> 
> Yes, this patch is absolutely essential, otherwise the system hangs at 
> random, as explained in the commit message.

And I was tired. This should have read *not* be necessary. Please take
a look at the series I posted, where I just reverted the patch which
changed the reset order to a late reset. With this the GPC now once
again uses the reset order as required by the GPU, without any
additional complexity.

Regards,
Lucas


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

Re: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[Marek Vasut]

On 7/17/21 11:07 AM, Lucas Stach wrote:
> Am Samstag, dem 17.07.2021 um 02:55 +0200 schrieb Marek Vasut:
>> On 7/17/21 1:32 AM, Lucas Stach wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> Am Donnerstag, dem 01.07.2021 um 00:59 +0200 schrieb Marek Vasut:
>>>> In case the power domain clock are ungated before the reset is asserted,
>>>> the system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
>>>> domains require different reset deassertion timing, and incorrect reset
>>>> deassertion timing also leads to hang.
>>>>
>>>> Add per-domain reset_{,de}assert_early flags which allow fine-grained
>>>> control of the reset assertion and deassertion sequence. Currently, on
>>>> MX8MM, the behavior is as follows and aligned with NXP downstream ATF
>>>> fork:
>>>> - VPUMIX: reset assert, reset deassert, domain power up
>>>> - GPUMIX: reset assert, domain power on, reset deassert
>>>>
>>> This patch should now be necessary, as my testing over the last few
>>> days showed that the VPUMIX isn't actually different and copes just
>>> fine with the reset being asserted early, just like the GPUMIX domain.
>>
>> Yes, this patch is absolutely essential, otherwise the system hangs at
>> random, as explained in the commit message.
> 
> And I was tired. This should have read *not* be necessary. Please take
> a look at the series I posted, where I just reverted the patch which
> changed the reset order to a late reset. With this the GPC now once
> again uses the reset order as required by the GPU, without any
> additional complexity.

During my extensive testing in the last few months, I've noticed random 
hangs of the platform and the reset/clock enablement order does matter, 
and it is different for different domains. The code in NXP ATF fork 
seems to confirm that.

Why do you think that is not the case , is there some documentation 
which confirms your hypothesis ?

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

Re: [PATCH] soc: imx: gpcv2: Assert reset before ungating clock

[Lucas Stach]

Hi Marek,

Am Samstag, dem 17.07.2021 um 14:07 +0200 schrieb Marek Vasut:
> On 7/17/21 11:07 AM, Lucas Stach wrote:
> > Am Samstag, dem 17.07.2021 um 02:55 +0200 schrieb Marek Vasut:
> > > On 7/17/21 1:32 AM, Lucas Stach wrote:
> > > > Hi Marek,
> > > 
> > > Hi,
> > > 
> > > > Am Donnerstag, dem 01.07.2021 um 00:59 +0200 schrieb Marek Vasut:
> > > > > In case the power domain clock are ungated before the reset is asserted,
> > > > > the system might freeze completely. However, the MX8MM GPUMIX and VPUMIX
> > > > > domains require different reset deassertion timing, and incorrect reset
> > > > > deassertion timing also leads to hang.
> > > > > 
> > > > > Add per-domain reset_{,de}assert_early flags which allow fine-grained
> > > > > control of the reset assertion and deassertion sequence. Currently, on
> > > > > MX8MM, the behavior is as follows and aligned with NXP downstream ATF
> > > > > fork:
> > > > > - VPUMIX: reset assert, reset deassert, domain power up
> > > > > - GPUMIX: reset assert, domain power on, reset deassert
> > > > > 
> > > > This patch should now be necessary, as my testing over the last few
> > > > days showed that the VPUMIX isn't actually different and copes just
> > > > fine with the reset being asserted early, just like the GPUMIX domain.
> > > 
> > > Yes, this patch is absolutely essential, otherwise the system hangs at
> > > random, as explained in the commit message.
> > 
> > And I was tired. This should have read *not* be necessary. Please take
> > a look at the series I posted, where I just reverted the patch which
> > changed the reset order to a late reset. With this the GPC now once
> > again uses the reset order as required by the GPU, without any
> > additional complexity.
> 
> During my extensive testing in the last few months, I've noticed random 
> hangs of the platform and the reset/clock enablement order does matter, 
> and it is different for different domains. The code in NXP ATF fork 
> seems to confirm that.
> 
> Why do you think that is not the case , is there some documentation 
> which confirms your hypothesis ?

Hahaha, documentation.

I've spent more than a week trying to make sense out of all the HW
requirements, the bits of docs that are actually in the RM and a lot of
testing of different flows. The ATF implementation was more of a
distraction than helpful in this quest. There are two things that my
testing seems to confirm:

a) The common sense assumption that the reset should be asserted
_before_  a power domain is going through the power-up flow and thus
the devices in the PD are going through undefined state seems to hold.

b) The i.MX8MM VPUMIX domain just tolerates the late reset
(reset assertion after power up), as implemented in the ATF, as there
are no devices in the MIX domain aside from the ADB, which would be
able to hang the system due to undefined state. I found no evidence
that the late reset is the preferred or even required sequence. The
VPUMIX domain powers up just fine with the reset already asserted.

This means that the domains do in fact have no differing requirements.
All domains require the reset to be asserted _before_ power-up and
require the clocks to be running for the reset to actually propagate
through. The sequence as implemented before "soc: imx: gpcv2: move
reset assert after requesting domain power up" is correct for all *MIX
domains.

In fact even the nested domains require the same sequence, the only
difference is that for the *MIX domains and non-nested peripheral
domains for reset and clocks the driver only has to deal with SRC and
CCM. For the nested domains clocks need to be enabled at the CCM level
and the BLK_CTRL level and resets aren't coming from the SRC, but from
the BLK_CTRL.

Regards,
Lucas


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