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

[Peng Fan <peng.fan@nxp.com>]

> 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