Hi Stephen,
thanks for reviewing.
On Fri, May 15, 2020 at 5:02 AM Stephen Boyd sboyd@kernel.org wrote:
Quoting dillon.minfei@gmail.com (2020-05-12 00:03:36)
From: dillon min dillon.minfei@gmail.com
as store stm32f4_rcc_register_pll return to the wrong offset of clks,
Use () on functions, i.e. stm32f4_rcc_register_pll().
ok
so ltdc gate clk is null. need change clks[PLL_VCO_SAI] to clks[PLL_SAI]
And quote variables like 'clks[PLL_VCO_SAI]'
ok
add CLK_IGNORE_UNUSED for ltdc to make sure clk not be freed by clk_disable_unused
clk_disable_unused() doesn't free anything. Why does ltdc not need to be turned off if it isn't used? Is it critical to system operation? Should it be marked with the critical clk flag then? The CLK_IGNORE_UNUSED flag is almost always wrong to use.
Yes, you are right. thanks. CLK_IGNORE_UNUSED just hide the root cause. after deeper debugging. i need to drop the last changes , they are not the root cause.
post diff and analyse here first, i will resubmit clk's changes in next patchset with gyro and ili9341.
--- a/drivers/clk/clk-stm32f4.c +++ b/drivers/clk/clk-stm32f4.c @@ -129,8 +129,6 @@ static const struct stm32f4_gate_data stm32f429_gates[] __initconst = { { STM32F4_RCC_APB2ENR, 20, "spi5", "apb2_div" }, { STM32F4_RCC_APB2ENR, 21, "spi6", "apb2_div" }, { STM32F4_RCC_APB2ENR, 22, "sai1", "apb2_div" }, - { STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div", - CLK_IGNORE_UNUSED }, };
static const struct stm32f4_gate_data stm32f469_gates[] __initconst = { @@ -558,13 +556,13 @@ static const struct clk_div_table post_divr_table[] = {
#define MAX_POST_DIV 3 static const struct stm32f4_pll_post_div_data post_div_data[MAX_POST_DIV] = { - { CLK_I2SQ_PDIV, PLL_I2S, "plli2s-q-div", "plli2s-q", + { CLK_I2SQ_PDIV, PLL_VCO_I2S, "plli2s-q-div", "plli2s-q", CLK_SET_RATE_PARENT, STM32F4_RCC_DCKCFGR, 0, 5, 0, NULL},
- { CLK_SAIQ_PDIV, PLL_SAI, "pllsai-q-div", "pllsai-q", + { CLK_SAIQ_PDIV, PLL_VCO_SAI, "pllsai-q-div", "pllsai-q", CLK_SET_RATE_PARENT, STM32F4_RCC_DCKCFGR, 8, 5, 0, NULL },
- { NO_IDX, PLL_SAI, "pllsai-r-div", "pllsai-r", CLK_SET_RATE_PARENT, + { NO_IDX, PLL_VCO_SAI, "pllsai-r-div", "pllsai-r", CLK_SET_RATE_PARENT, STM32F4_RCC_DCKCFGR, 16, 2, 0, post_divr_table }, };
@@ -1758,7 +1756,7 @@ static void __init stm32f4_rcc_init(struct device_node *np) clks[PLL_VCO_I2S] = stm32f4_rcc_register_pll("vco_in", &data->pll_data[1], &stm32f4_clk_lock);
- clks[PLL_SAI] = stm32f4_rcc_register_pll("vco_in", + clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll("vco_in", &data->pll_data[2], &stm32f4_clk_lock);
for (n = 0; n < MAX_POST_DIV; n++) {
issue 1: ili9341 hang in clk set rate, the root cause should be PLL_VCO_SAI, PLL_SAI mismatch for 'clks[]'
1, first at stm32f4_rcc_init() , clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll("vco_in", &data->pll_data[2], &stm32f4_clk_lock); the clk_hw from stm32f4_rcc_register_pll() is store to 'clks[7]', defined in 'include/dt-bindings/clock/stm32fx-clock.h'
2, next hw = clk_register_pll_div(post_div->name, post_div->parent, post_div->flag, base + post_div->offset, post_div->shift, post_div->width, post_div->flag_div, post_div->div_table, clks[post_div->pll_num], &stm32f4_clk_lock); the 'clks[post_div->pll_num]', the pll_num is PLL_SAI, the value is 2, defined at enum { PLL, PLL_I2S, PLL_SAI, }; 'post_div_data[]'
so 7 != 2 offset of 'clks[]', input the wrong 'clks[]' to clk_register_pll_div. cause to_clk_gate result is null, crashed in ltdc driver's loading.
issue 2: clk_disable_unused() turn off ltdc clock. 1, ltdc clk is defined in 'stm32f429_gates[]', register to clk core, but there is no user to use it. ltdc driver use dts binding name "lcd", connect to CLK_LCD, then aux clk 'lcd-tft ' 2, as no one use 'stm32f429_gates[]' s ltdc clock , so the enable_count is zero, after kernel startup it's been turn off by clk_disable_unused() is fine.
my chages for this is remove the ltdc from 'stm32f429_gates[]' but this modification still need 'clk-stm32f4.c''s maintainer to check if there is side effect.
Signed-off-by: dillon min dillon.minfei@gmail.com
drivers/clk/clk-stm32f4.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c index 18117ce..0ba73de 100644 --- a/drivers/clk/clk-stm32f4.c +++ b/drivers/clk/clk-stm32f4.c @@ -129,7 +129,8 @@ static const struct stm32f4_gate_data stm32f429_gates[] __initconst = { { STM32F4_RCC_APB2ENR, 20, "spi5", "apb2_div" }, { STM32F4_RCC_APB2ENR, 21, "spi6", "apb2_div" }, { STM32F4_RCC_APB2ENR, 22, "sai1", "apb2_div" },
{ STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div" },
{ STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div",CLK_IGNORE_UNUSED },};
static const struct stm32f4_gate_data stm32f469_gates[] __initconst = { @@ -1757,7 +1758,7 @@ static void __init stm32f4_rcc_init(struct device_node *np) clks[PLL_VCO_I2S] = stm32f4_rcc_register_pll("vco_in", &data->pll_data[1], &stm32f4_clk_lock);
clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll("vco_in",
clks[PLL_SAI] = stm32f4_rcc_register_pll("vco_in", &data->pll_data[2], &stm32f4_clk_lock); for (n = 0; n < MAX_POST_DIV; n++) {