Re: [PATCH v3 4/9] clk: imx: disable i.mx7ulp composite clock during initialization

[Abel Vesa <abel.vesa@nxp.com>]

On 21-09-14 14:52:03, Jacky Bai wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> i.MX7ULP peripheral clock ONLY allow parent/rate to be changed
> with clock gated, however, during clock tree initialization, the
> peripheral clock could be enabled by bootloader, but the prepare
> count in clock tree is still zero, so clock core driver will allow
> parent/rate changed even with CLK_SET_RATE_GATE/CLK_SET_PARENT_GATE
> set, but the change will fail due to HW NOT allow parent/rate change
> with clock enabled. It will cause clock HW status mismatch with
> clock tree info and lead to function issue. Below is an example:
> 
> usdhc0's pcc clock value is 0xC5000000 during kernel boot up, it
> means usdhc0 clock is enabled, its parent is APLL_PFD1. In DT file,
> the usdhc0 clock settings are as below:
> 
> assigned-clocks = <&pcc2 IMX7ULP_CLK_USDHC0>;
> assigned-clock-parents = <&scg1 IMX7ULP_CLK_NIC1_DIV>;
> 
> when kernel boot up, the clock tree info is as below, but the usdhc0
> PCC register is still 0xC5000000, which means its parent is still
> from APLL_PFD1, which is incorrect and cause usdhc0 NOT work.
> 
> nic1_clk       2        2        0   176000000          0     0  50000
>     usdhc0       0        0        0   176000000          0     0  50000
> 
> After making sure the peripheral clock is disabled during clock tree
> initialization, the usdhc0 is working, and this change is necessary
> for all i.MX7ULP peripheral clocks.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>

Reviewed-by: Abel Vesa <abel.vesa@nxp.com>

> ---
>   v3 changes: no
> ---
>  drivers/clk/imx/clk-composite-7ulp.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/clk/imx/clk-composite-7ulp.c b/drivers/clk/imx/clk-composite-7ulp.c
> index 50ed383320bf..92908ee4509d 100644
> --- a/drivers/clk/imx/clk-composite-7ulp.c
> +++ b/drivers/clk/imx/clk-composite-7ulp.c
> @@ -8,6 +8,7 @@
>  #include <linux/bits.h>
>  #include <linux/clk-provider.h>
>  #include <linux/err.h>
> +#include <linux/io.h>
>  #include <linux/slab.h>
>  
>  #include "../clk-fractional-divider.h"
> @@ -73,6 +74,7 @@ static struct clk_hw *imx_ulp_clk_hw_composite(const char *name,
>  	struct clk_gate *gate = NULL;
>  	struct clk_mux *mux = NULL;
>  	struct clk_hw *hw;
> +	u32 val;
>  
>  	if (mux_present) {
>  		mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> @@ -111,6 +113,18 @@ static struct clk_hw *imx_ulp_clk_hw_composite(const char *name,
>  		gate_hw = &gate->hw;
>  		gate->reg = reg;
>  		gate->bit_idx = PCG_CGC_SHIFT;
> +		/*
> +		 * make sure clock is gated during clock tree initialization,
> +		 * the HW ONLY allow clock parent/rate changed with clock gated,
> +		 * during clock tree initialization, clocks could be enabled
> +		 * by bootloader, so the HW status will mismatch with clock tree
> +		 * prepare count, then clock core driver will allow parent/rate
> +		 * change since the prepare count is zero, but HW actually
> +		 * prevent the parent/rate change due to the clock is enabled.
> +		 */
> +		val = readl_relaxed(reg);
> +		val &= ~(1 << PCG_CGC_SHIFT);
> +		writel_relaxed(val, reg);
>  	}
>  
>  	hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
> -- 
> 2.26.2
>