[PATCH] [RFC] soc: imx: gpcv2: Split clock prepare from clock enable in the domain

[Peng Fan]

Hi Marek,

> Subject: [PATCH] [RFC] soc: imx: gpcv2: Split clock prepare from clock enable
> in the domain
> 
> It is possible for clk_disable_unused() to trigger lockdep warning regarding
> lock ordering in this driver. This happens in case of the following conditions:
> 
> A) clock core clk_disable_unused() triggers the following sequence in a
>    driver which also uses GPCv2 domain:
>    - clk_prepare_lock() -> obtains clock core prepare_lock
>    - pm_runtime_get*() -> obtains &blk_ctrl_genpd_lock_class
> 
> B) driver powers up a power domain and triggers the following sequence
>    in GPCv2:
>    - pm_runtime_get_sync() -> obtains &blk_ctrl_genpd_lock_class
>    - clk_bulk_prepare_enable() -> obtains clock core prepare_lock
> 
> This can lead to a deadlock in case A and B runs on separate CPUs.
> 
> To avoid the deadlock, split clk_*prepare() from clk_*enable() and call the
> former before pm_runtime_get_sync(). This way, the GPCv2 driver always
> claims the prepare_lock before blk_ctrl_genpd_lock_class and the deadlock
> is avoided.

Is this trying to address the i.MX8MP audiomix blk ctrl deadlock issue?

Thanks,
Peng.
> 
> The imx8m and imx8mp block controller drivers likely need similar fix.
> 
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: Fabio Estevam <festevam at denx.de>
> Cc: Jacky Bai <ping.bai at nxp.com>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Lucas Stach <l.stach at pengutronix.de>
> Cc: Peng Fan <peng.fan at nxp.com>
> Cc: Shawn Guo <shawnguo at kernel.org>
> Cc: Shengjiu Wang <shengjiu.wang at nxp.com>
> Cc: Stephen Boyd <sboyd at kernel.org>
> Cc: linux-clk at vger.kernel.org
> Cc: linux-imx at nxp.com
> To: linux-arm-kernel at lists.infradead.org
> ---
>  drivers/soc/imx/gpcv2.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c index
> 88aee59730e39..ee9294ad25ba1 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -319,10 +319,16 @@ static int imx_pgc_power_up(struct
> generic_pm_domain *genpd)
>  	u32 reg_val, pgc;
>  	int ret;
> 
> +	ret = clk_bulk_prepare(domain->num_clks, domain->clks);
> +	if (ret) {
> +		dev_err(domain->dev, "failed to prepare reset clocks\n");
> +		return ret;
> +	}
> +
>  	ret = pm_runtime_get_sync(domain->dev);
>  	if (ret < 0) {
>  		pm_runtime_put_noidle(domain->dev);
> -		return ret;
> +		goto out_clock_unprepare;
>  	}
> 
>  	if (!IS_ERR(domain->regulator)) {
> @@ -338,7 +344,7 @@ static int imx_pgc_power_up(struct
> generic_pm_domain *genpd)
>  	reset_control_assert(domain->reset);
> 
>  	/* Enable reset clocks for all devices in the domain */
> -	ret = clk_bulk_prepare_enable(domain->num_clks, domain->clks);
> +	ret = clk_bulk_enable(domain->num_clks, domain->clks);
>  	if (ret) {
>  		dev_err(domain->dev, "failed to enable reset clocks\n");
>  		goto out_regulator_disable;
> @@ -402,12 +408,14 @@ static int imx_pgc_power_up(struct
> generic_pm_domain *genpd)
>  	return 0;
> 
>  out_clk_disable:
> -	clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
> +	clk_bulk_disable(domain->num_clks, domain->clks);
>  out_regulator_disable:
>  	if (!IS_ERR(domain->regulator))
>  		regulator_disable(domain->regulator);
>  out_put_pm:
>  	pm_runtime_put(domain->dev);
> +out_clock_unprepare:
> +	clk_bulk_unprepare(domain->num_clks, domain->clks);
> 
>  	return ret;
>  }
> --
> 2.35.1