linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] clk: HiSilicon: clk-hi3670: mark critical clocks as such
@ 2021-08-20 15:35 Mauro Carvalho Chehab
  2021-08-20 17:52 ` Manivannan Sadhasivam
  0 siblings, 1 reply; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-20 15:35 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab,
	Manivannan Sadhasivam, linux-clk, linux-kernel

The fixed-rate and fixed_factor clocks are critical to the
system: disabling them causes HiKey970 to not finish booting.

There are a couple of other clocks that makes the Kernel to
panic with SError if they got disabled.

So, mark them with CLK_IS_CRITICAL, in order to prevent the
Clock core to disable them if they're not explicitly used by
some driver.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/clk/hisilicon/clk-hi3670.c | 96 +++++++++++++++---------------
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/drivers/clk/hisilicon/clk-hi3670.c b/drivers/clk/hisilicon/clk-hi3670.c
index 4d05a71683a5..f24bb88deb26 100644
--- a/drivers/clk/hisilicon/clk-hi3670.c
+++ b/drivers/clk/hisilicon/clk-hi3670.c
@@ -14,66 +14,66 @@
 #include "clk.h"
 
 static const struct hisi_fixed_rate_clock hi3670_fixed_rate_clks[] = {
-	{ HI3670_CLKIN_SYS, "clkin_sys", NULL, 0, 19200000, },
-	{ HI3670_CLKIN_REF, "clkin_ref", NULL, 0, 32764, },
-	{ HI3670_CLK_FLL_SRC, "clk_fll_src", NULL, 0, 134400000, },
-	{ HI3670_CLK_PPLL0, "clk_ppll0", NULL, 0, 1660000000, },
-	{ HI3670_CLK_PPLL1, "clk_ppll1", NULL, 0, 1866000000, },
-	{ HI3670_CLK_PPLL2, "clk_ppll2", NULL, 0, 1920000000, },
-	{ HI3670_CLK_PPLL3, "clk_ppll3", NULL, 0, 1200000000, },
-	{ HI3670_CLK_PPLL4, "clk_ppll4", NULL, 0, 900000000, },
-	{ HI3670_CLK_PPLL6, "clk_ppll6", NULL, 0, 393216000, },
-	{ HI3670_CLK_PPLL7, "clk_ppll7", NULL, 0, 1008000000, },
-	{ HI3670_CLK_PPLL_PCIE, "clk_ppll_pcie", NULL, 0, 100000000, },
-	{ HI3670_CLK_PCIEPLL_REV, "clk_pciepll_rev", NULL, 0, 100000000, },
-	{ HI3670_CLK_SCPLL, "clk_scpll", NULL, 0, 245760000, },
-	{ HI3670_PCLK, "pclk", NULL, 0, 20000000, },
-	{ HI3670_CLK_UART0_DBG, "clk_uart0_dbg", NULL, 0, 19200000, },
-	{ HI3670_CLK_UART6, "clk_uart6", NULL, 0, 19200000, },
-	{ HI3670_OSC32K, "osc32k", NULL, 0, 32764, },
-	{ HI3670_OSC19M, "osc19m", NULL, 0, 19200000, },
-	{ HI3670_CLK_480M, "clk_480m", NULL, 0, 480000000, },
-	{ HI3670_CLK_INVALID, "clk_invalid", NULL, 0, 10000000, },
+	{ HI3670_CLKIN_SYS, "clkin_sys", NULL, CLK_IS_CRITICAL, 19200000, },
+	{ HI3670_CLKIN_REF, "clkin_ref", NULL, CLK_IS_CRITICAL, 32764, },
+	{ HI3670_CLK_FLL_SRC, "clk_fll_src", NULL, CLK_IS_CRITICAL, 134400000, },
+	{ HI3670_CLK_PPLL0, "clk_ppll0", NULL, CLK_IS_CRITICAL, 1660000000, },
+	{ HI3670_CLK_PPLL1, "clk_ppll1", NULL, CLK_IS_CRITICAL, 1866000000, },
+	{ HI3670_CLK_PPLL2, "clk_ppll2", NULL, CLK_IS_CRITICAL, 1920000000, },
+	{ HI3670_CLK_PPLL3, "clk_ppll3", NULL, CLK_IS_CRITICAL, 1200000000, },
+	{ HI3670_CLK_PPLL4, "clk_ppll4", NULL, CLK_IS_CRITICAL, 900000000, },
+	{ HI3670_CLK_PPLL6, "clk_ppll6", NULL, CLK_IS_CRITICAL, 393216000, },
+	{ HI3670_CLK_PPLL7, "clk_ppll7", NULL,  CLK_IS_CRITICAL, 1008000000, },
+	{ HI3670_CLK_PPLL_PCIE, "clk_ppll_pcie", NULL, CLK_IS_CRITICAL, 100000000, },
+	{ HI3670_CLK_PCIEPLL_REV, "clk_pciepll_rev", NULL, CLK_IS_CRITICAL, 100000000, },
+	{ HI3670_CLK_SCPLL, "clk_scpll", NULL, CLK_IS_CRITICAL, 245760000, },
+	{ HI3670_PCLK, "pclk", NULL, CLK_IS_CRITICAL, 20000000, },
+	{ HI3670_CLK_UART0_DBG, "clk_uart0_dbg", NULL, CLK_IS_CRITICAL, 19200000, },
+	{ HI3670_CLK_UART6, "clk_uart6", NULL, CLK_IS_CRITICAL, 19200000, },
+	{ HI3670_OSC32K, "osc32k", NULL, CLK_IS_CRITICAL, 32764, },
+	{ HI3670_OSC19M, "osc19m", NULL, CLK_IS_CRITICAL, 19200000, },
+	{ HI3670_CLK_480M, "clk_480m", NULL, CLK_IS_CRITICAL, 480000000, },
+	{ HI3670_CLK_INVALID, "clk_invalid", NULL, CLK_IS_CRITICAL, 10000000, },
 };
 
 /* crgctrl */
 static const struct hisi_fixed_factor_clock hi3670_crg_fixed_factor_clks[] = {
 	{ HI3670_CLK_DIV_SYSBUS, "clk_div_sysbus", "clk_mux_sysbus",
-	  1, 7, 0, },
+	  1, 7, CLK_IS_CRITICAL },
 	{ HI3670_CLK_FACTOR_MMC, "clk_factor_mmc", "clkin_sys",
-	  1, 6, 0, },
+	  1, 6, CLK_IS_CRITICAL },
 	{ HI3670_CLK_SD_SYS, "clk_sd_sys", "clk_sd_sys_gt",
-	  1, 6, 0, },
+	  1, 6, CLK_IS_CRITICAL },
 	{ HI3670_CLK_SDIO_SYS, "clk_sdio_sys", "clk_sdio_sys_gt",
-	  1, 6, 0, },
+	  1, 6, CLK_IS_CRITICAL },
 	{ HI3670_CLK_DIV_A53HPM, "clk_div_a53hpm", "clk_a53hpm_andgt",
-	  1, 4, 0, },
+	  1, 4, CLK_IS_CRITICAL },
 	{ HI3670_CLK_DIV_320M, "clk_div_320m", "clk_320m_pll_gt",
-	  1, 5, 0, },
+	  1, 5, CLK_IS_CRITICAL },
 	{ HI3670_PCLK_GATE_UART0, "pclk_gate_uart0", "clk_mux_uartl",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_CLK_FACTOR_UART0, "clk_factor_uart0", "clk_mux_uart0",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_CLK_FACTOR_USB3PHY_PLL, "clk_factor_usb3phy_pll", "clk_ppll0",
-	  1, 60, 0, },
+	  1, 60, CLK_IS_CRITICAL },
 	{ HI3670_CLK_GATE_ABB_USB, "clk_gate_abb_usb", "clk_gate_usb_tcxo_en",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_CLK_GATE_UFSPHY_REF, "clk_gate_ufsphy_ref", "clkin_sys",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_ICS_VOLT_HIGH, "ics_volt_high", "peri_volt_hold",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_ICS_VOLT_MIDDLE, "ics_volt_middle", "peri_volt_middle",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_VENC_VOLT_HOLD, "venc_volt_hold", "peri_volt_hold",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_VDEC_VOLT_HOLD, "vdec_volt_hold", "peri_volt_hold",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_EDC_VOLT_HOLD, "edc_volt_hold", "peri_volt_hold",
-	  1, 1, 0, },
+	  1, 1, CLK_IS_CRITICAL },
 	{ HI3670_CLK_ISP_SNCLK_FAC, "clk_isp_snclk_fac", "clk_isp_snclk_angt",
-	  1, 10, 0, },
+	  1, 10, CLK_IS_CRITICAL },
 	{ HI3670_CLK_FACTOR_RXDPHY, "clk_factor_rxdphy", "clk_andgt_rxdphy",
-	  1, 6, 0, },
+	  1, 6, CLK_IS_CRITICAL },
 };
 
 static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
@@ -82,13 +82,13 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
 	{ HI3670_PPLL2_EN_ACPU, "ppll2_en_acpu", "clk_ppll2",
 	  CLK_SET_RATE_PARENT, 0x0, 3, 0, },
 	{ HI3670_PPLL3_EN_ACPU, "ppll3_en_acpu", "clk_ppll3",
-	  CLK_SET_RATE_PARENT, 0x0, 27, 0, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x0, 27, 0, },
 	{ HI3670_PPLL1_GT_CPU, "ppll1_gt_cpu", "clk_ppll1",
 	  CLK_SET_RATE_PARENT, 0x460, 16, 0, },
 	{ HI3670_PPLL2_GT_CPU, "ppll2_gt_cpu", "clk_ppll2",
 	  CLK_SET_RATE_PARENT, 0x460, 18, 0, },
 	{ HI3670_PPLL3_GT_CPU, "ppll3_gt_cpu", "clk_ppll3",
-	  CLK_SET_RATE_PARENT, 0x460, 20, 0, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x460, 20, 0, },
 	{ HI3670_CLK_GATE_PPLL2_MEDIA, "clk_gate_ppll2_media", "clk_ppll2",
 	  CLK_SET_RATE_PARENT, 0x410, 27, 0, },
 	{ HI3670_CLK_GATE_PPLL3_MEDIA, "clk_gate_ppll3_media", "clk_ppll3",
@@ -158,7 +158,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
 	{ HI3670_PCLK_GATE_MMC0_IOC, "pclk_gate_mmc0_ioc", "clk_div_mmc0bus",
 	  CLK_SET_RATE_PARENT, 0x40, 13, 0, },
 	{ HI3670_PCLK_GATE_MMC1_IOC, "pclk_gate_mmc1_ioc", "clk_div_mmc1bus",
-	  CLK_SET_RATE_PARENT, 0x420, 21, 0, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x420, 21, 0, },
 	{ HI3670_CLK_GATE_DMAC, "clk_gate_dmac", "clk_div_sysbus",
 	  CLK_SET_RATE_PARENT, 0x30, 1, 0, },
 	{ HI3670_CLK_GATE_VCODECBUS2DDR, "clk_gate_vcodecbus2ddr", "clk_div_vcodecbus",
@@ -166,7 +166,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
 	{ HI3670_CLK_CCI400_BYPASS, "clk_cci400_bypass", "clk_ddrc_freq",
 	  CLK_SET_RATE_PARENT, 0x22C, 28, 0, },
 	{ HI3670_CLK_GATE_CCI400, "clk_gate_cci400", "clk_ddrc_freq",
-	  CLK_SET_RATE_PARENT, 0x50, 14, 0, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x50, 14, 0, },
 	{ HI3670_CLK_GATE_SD, "clk_gate_sd", "clk_mux_sd_sys",
 	  CLK_SET_RATE_PARENT, 0x40, 17, 0, },
 	{ HI3670_HCLK_GATE_SD, "hclk_gate_sd", "clk_div_sysbus",
@@ -248,7 +248,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
 	{ HI3670_CLK_GATE_AO_ASP, "clk_gate_ao_asp", "clk_div_ao_asp",
 	  CLK_SET_RATE_PARENT, 0x0, 26, 0, },
 	{ HI3670_PCLK_GATE_PCTRL, "pclk_gate_pctrl", "clk_div_ptp",
-	  CLK_SET_RATE_PARENT, 0x20, 31, 0, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x20, 31, 0, },
 	{ HI3670_CLK_CSI_TRANS_GT, "clk_csi_trans_gt", "clk_div_csi_trans",
 	  CLK_SET_RATE_PARENT, 0x30, 24, 0, },
 	{ HI3670_CLK_DSI_TRANS_GT, "clk_dsi_trans_gt", "clk_div_dsi_trans",
@@ -301,7 +301,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_clks[] = {
 	{ HI3670_PCLK_ANDGT_MMC1_PCIE, "pclk_andgt_mmc1_pcie", "clk_div_320m",
 	  CLK_SET_RATE_PARENT, 0xf8, 13, CLK_GATE_HIWORD_MASK, },
 	{ HI3670_CLK_GATE_VCODECBUS_GT, "clk_gate_vcodecbus_gt", "clk_mux_vcodecbus",
-	  CLK_SET_RATE_PARENT, 0x0F0, 8, CLK_GATE_HIWORD_MASK, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x0F0, 8, CLK_GATE_HIWORD_MASK, },
 	{ HI3670_CLK_ANDGT_SD, "clk_andgt_sd", "clk_mux_sd_pll",
 	  CLK_SET_RATE_PARENT, 0xF4, 3, CLK_GATE_HIWORD_MASK, },
 	{ HI3670_CLK_SD_SYS_GT, "clk_sd_sys_gt", "clkin_sys",
@@ -331,7 +331,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_clks[] = {
 	{ HI3670_CLK_GATE_DSI_TRANS, "clk_gate_dsi_trans", "clk_ppll2",
 	  CLK_SET_RATE_PARENT, 0xF4, 1, CLK_GATE_HIWORD_MASK, },
 	{ HI3670_CLK_ANDGT_PTP, "clk_andgt_ptp", "clk_div_320m",
-	  CLK_SET_RATE_PARENT, 0xF8, 5, CLK_GATE_HIWORD_MASK, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0xF8, 5, CLK_GATE_HIWORD_MASK, },
 	{ HI3670_CLK_ANDGT_OUT0, "clk_andgt_out0", "clk_ppll0",
 	  CLK_SET_RATE_PARENT, 0xF0, 10, CLK_GATE_HIWORD_MASK, },
 	{ HI3670_CLK_ANDGT_OUT1, "clk_andgt_out1", "clk_ppll0",
@@ -569,9 +569,9 @@ static const struct hisi_gate_clock hi3670_sctrl_gate_sep_clks[] = {
 	{ HI3670_PCLK_GATE_SPI, "pclk_gate_spi", "clk_div_ioperi",
 	  CLK_SET_RATE_PARENT, 0x1B0, 10, 0, },
 	{ HI3670_CLK_GATE_UFS_SUBSYS, "clk_gate_ufs_subsys", "clk_div_ufs_subsys",
-	  CLK_SET_RATE_PARENT, 0x1B0, 14, 0, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x1B0, 14, 0, },
 	{ HI3670_CLK_GATE_UFSIO_REF, "clk_gate_ufsio_ref", "clkin_sys",
-	  CLK_SET_RATE_PARENT, 0x1b0, 12, 0, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x1b0, 12, 0, },
 	{ HI3670_PCLK_AO_GPIO0, "pclk_ao_gpio0", "clk_div_aobus",
 	  CLK_SET_RATE_PARENT, 0x160, 11, 0, },
 	{ HI3670_PCLK_AO_GPIO1, "pclk_ao_gpio1", "clk_div_aobus",
@@ -593,7 +593,7 @@ static const struct hisi_gate_clock hi3670_sctrl_gate_sep_clks[] = {
 	{ HI3670_PCLK_GATE_SYSCNT, "pclk_gate_syscnt", "clk_div_aobus",
 	  CLK_SET_RATE_PARENT, 0x160, 19, 0, },
 	{ HI3670_CLK_GATE_SYSCNT, "clk_gate_syscnt", "clkin_sys",
-	  CLK_SET_RATE_PARENT, 0x160, 20, 0, },
+	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x160, 20, 0, },
 	{ HI3670_CLK_GATE_ASP_SUBSYS_PERI, "clk_gate_asp_subsys_peri",
 	  "clk_mux_asp_subsys_peri",
 	  CLK_SET_RATE_PARENT, 0x170, 6, 0, },
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] clk: HiSilicon: clk-hi3670: mark critical clocks as such
  2021-08-20 15:35 [PATCH v3] clk: HiSilicon: clk-hi3670: mark critical clocks as such Mauro Carvalho Chehab
@ 2021-08-20 17:52 ` Manivannan Sadhasivam
  2021-08-20 18:40   ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 4+ messages in thread
From: Manivannan Sadhasivam @ 2021-08-20 17:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Stephen Boyd, Michael Turquette, linuxarm, mauro.chehab,
	linux-clk, linux-kernel

On Fri, Aug 20, 2021 at 05:35:43PM +0200, Mauro Carvalho Chehab wrote:
> The fixed-rate and fixed_factor clocks are critical to the
> system: disabling them causes HiKey970 to not finish booting.
> 

You mean all fixed rate and fixed factor clocks are critical to the system?
I don't think so.

You should identify the gateable clocks that are causing the issues and see if
they can be marked as critical.

Also, at what point of time you are seeing the boot issue?

Thanks,
Mani

> There are a couple of other clocks that makes the Kernel to
> panic with SError if they got disabled.
> 
> So, mark them with CLK_IS_CRITICAL, in order to prevent the
> Clock core to disable them if they're not explicitly used by
> some driver.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/clk/hisilicon/clk-hi3670.c | 96 +++++++++++++++---------------
>  1 file changed, 48 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/clk/hisilicon/clk-hi3670.c b/drivers/clk/hisilicon/clk-hi3670.c
> index 4d05a71683a5..f24bb88deb26 100644
> --- a/drivers/clk/hisilicon/clk-hi3670.c
> +++ b/drivers/clk/hisilicon/clk-hi3670.c
> @@ -14,66 +14,66 @@
>  #include "clk.h"
>  
>  static const struct hisi_fixed_rate_clock hi3670_fixed_rate_clks[] = {
> -	{ HI3670_CLKIN_SYS, "clkin_sys", NULL, 0, 19200000, },
> -	{ HI3670_CLKIN_REF, "clkin_ref", NULL, 0, 32764, },
> -	{ HI3670_CLK_FLL_SRC, "clk_fll_src", NULL, 0, 134400000, },
> -	{ HI3670_CLK_PPLL0, "clk_ppll0", NULL, 0, 1660000000, },
> -	{ HI3670_CLK_PPLL1, "clk_ppll1", NULL, 0, 1866000000, },
> -	{ HI3670_CLK_PPLL2, "clk_ppll2", NULL, 0, 1920000000, },
> -	{ HI3670_CLK_PPLL3, "clk_ppll3", NULL, 0, 1200000000, },
> -	{ HI3670_CLK_PPLL4, "clk_ppll4", NULL, 0, 900000000, },
> -	{ HI3670_CLK_PPLL6, "clk_ppll6", NULL, 0, 393216000, },
> -	{ HI3670_CLK_PPLL7, "clk_ppll7", NULL, 0, 1008000000, },
> -	{ HI3670_CLK_PPLL_PCIE, "clk_ppll_pcie", NULL, 0, 100000000, },
> -	{ HI3670_CLK_PCIEPLL_REV, "clk_pciepll_rev", NULL, 0, 100000000, },
> -	{ HI3670_CLK_SCPLL, "clk_scpll", NULL, 0, 245760000, },
> -	{ HI3670_PCLK, "pclk", NULL, 0, 20000000, },
> -	{ HI3670_CLK_UART0_DBG, "clk_uart0_dbg", NULL, 0, 19200000, },
> -	{ HI3670_CLK_UART6, "clk_uart6", NULL, 0, 19200000, },
> -	{ HI3670_OSC32K, "osc32k", NULL, 0, 32764, },
> -	{ HI3670_OSC19M, "osc19m", NULL, 0, 19200000, },
> -	{ HI3670_CLK_480M, "clk_480m", NULL, 0, 480000000, },
> -	{ HI3670_CLK_INVALID, "clk_invalid", NULL, 0, 10000000, },
> +	{ HI3670_CLKIN_SYS, "clkin_sys", NULL, CLK_IS_CRITICAL, 19200000, },
> +	{ HI3670_CLKIN_REF, "clkin_ref", NULL, CLK_IS_CRITICAL, 32764, },
> +	{ HI3670_CLK_FLL_SRC, "clk_fll_src", NULL, CLK_IS_CRITICAL, 134400000, },
> +	{ HI3670_CLK_PPLL0, "clk_ppll0", NULL, CLK_IS_CRITICAL, 1660000000, },
> +	{ HI3670_CLK_PPLL1, "clk_ppll1", NULL, CLK_IS_CRITICAL, 1866000000, },
> +	{ HI3670_CLK_PPLL2, "clk_ppll2", NULL, CLK_IS_CRITICAL, 1920000000, },
> +	{ HI3670_CLK_PPLL3, "clk_ppll3", NULL, CLK_IS_CRITICAL, 1200000000, },
> +	{ HI3670_CLK_PPLL4, "clk_ppll4", NULL, CLK_IS_CRITICAL, 900000000, },
> +	{ HI3670_CLK_PPLL6, "clk_ppll6", NULL, CLK_IS_CRITICAL, 393216000, },
> +	{ HI3670_CLK_PPLL7, "clk_ppll7", NULL,  CLK_IS_CRITICAL, 1008000000, },
> +	{ HI3670_CLK_PPLL_PCIE, "clk_ppll_pcie", NULL, CLK_IS_CRITICAL, 100000000, },
> +	{ HI3670_CLK_PCIEPLL_REV, "clk_pciepll_rev", NULL, CLK_IS_CRITICAL, 100000000, },
> +	{ HI3670_CLK_SCPLL, "clk_scpll", NULL, CLK_IS_CRITICAL, 245760000, },
> +	{ HI3670_PCLK, "pclk", NULL, CLK_IS_CRITICAL, 20000000, },
> +	{ HI3670_CLK_UART0_DBG, "clk_uart0_dbg", NULL, CLK_IS_CRITICAL, 19200000, },
> +	{ HI3670_CLK_UART6, "clk_uart6", NULL, CLK_IS_CRITICAL, 19200000, },
> +	{ HI3670_OSC32K, "osc32k", NULL, CLK_IS_CRITICAL, 32764, },
> +	{ HI3670_OSC19M, "osc19m", NULL, CLK_IS_CRITICAL, 19200000, },
> +	{ HI3670_CLK_480M, "clk_480m", NULL, CLK_IS_CRITICAL, 480000000, },
> +	{ HI3670_CLK_INVALID, "clk_invalid", NULL, CLK_IS_CRITICAL, 10000000, },
>  };
>  
>  /* crgctrl */
>  static const struct hisi_fixed_factor_clock hi3670_crg_fixed_factor_clks[] = {
>  	{ HI3670_CLK_DIV_SYSBUS, "clk_div_sysbus", "clk_mux_sysbus",
> -	  1, 7, 0, },
> +	  1, 7, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_FACTOR_MMC, "clk_factor_mmc", "clkin_sys",
> -	  1, 6, 0, },
> +	  1, 6, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_SD_SYS, "clk_sd_sys", "clk_sd_sys_gt",
> -	  1, 6, 0, },
> +	  1, 6, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_SDIO_SYS, "clk_sdio_sys", "clk_sdio_sys_gt",
> -	  1, 6, 0, },
> +	  1, 6, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_DIV_A53HPM, "clk_div_a53hpm", "clk_a53hpm_andgt",
> -	  1, 4, 0, },
> +	  1, 4, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_DIV_320M, "clk_div_320m", "clk_320m_pll_gt",
> -	  1, 5, 0, },
> +	  1, 5, CLK_IS_CRITICAL },
>  	{ HI3670_PCLK_GATE_UART0, "pclk_gate_uart0", "clk_mux_uartl",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_FACTOR_UART0, "clk_factor_uart0", "clk_mux_uart0",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_FACTOR_USB3PHY_PLL, "clk_factor_usb3phy_pll", "clk_ppll0",
> -	  1, 60, 0, },
> +	  1, 60, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_GATE_ABB_USB, "clk_gate_abb_usb", "clk_gate_usb_tcxo_en",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_GATE_UFSPHY_REF, "clk_gate_ufsphy_ref", "clkin_sys",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_ICS_VOLT_HIGH, "ics_volt_high", "peri_volt_hold",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_ICS_VOLT_MIDDLE, "ics_volt_middle", "peri_volt_middle",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_VENC_VOLT_HOLD, "venc_volt_hold", "peri_volt_hold",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_VDEC_VOLT_HOLD, "vdec_volt_hold", "peri_volt_hold",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_EDC_VOLT_HOLD, "edc_volt_hold", "peri_volt_hold",
> -	  1, 1, 0, },
> +	  1, 1, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_ISP_SNCLK_FAC, "clk_isp_snclk_fac", "clk_isp_snclk_angt",
> -	  1, 10, 0, },
> +	  1, 10, CLK_IS_CRITICAL },
>  	{ HI3670_CLK_FACTOR_RXDPHY, "clk_factor_rxdphy", "clk_andgt_rxdphy",
> -	  1, 6, 0, },
> +	  1, 6, CLK_IS_CRITICAL },
>  };
>  
>  static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
> @@ -82,13 +82,13 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
>  	{ HI3670_PPLL2_EN_ACPU, "ppll2_en_acpu", "clk_ppll2",
>  	  CLK_SET_RATE_PARENT, 0x0, 3, 0, },
>  	{ HI3670_PPLL3_EN_ACPU, "ppll3_en_acpu", "clk_ppll3",
> -	  CLK_SET_RATE_PARENT, 0x0, 27, 0, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x0, 27, 0, },
>  	{ HI3670_PPLL1_GT_CPU, "ppll1_gt_cpu", "clk_ppll1",
>  	  CLK_SET_RATE_PARENT, 0x460, 16, 0, },
>  	{ HI3670_PPLL2_GT_CPU, "ppll2_gt_cpu", "clk_ppll2",
>  	  CLK_SET_RATE_PARENT, 0x460, 18, 0, },
>  	{ HI3670_PPLL3_GT_CPU, "ppll3_gt_cpu", "clk_ppll3",
> -	  CLK_SET_RATE_PARENT, 0x460, 20, 0, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x460, 20, 0, },
>  	{ HI3670_CLK_GATE_PPLL2_MEDIA, "clk_gate_ppll2_media", "clk_ppll2",
>  	  CLK_SET_RATE_PARENT, 0x410, 27, 0, },
>  	{ HI3670_CLK_GATE_PPLL3_MEDIA, "clk_gate_ppll3_media", "clk_ppll3",
> @@ -158,7 +158,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
>  	{ HI3670_PCLK_GATE_MMC0_IOC, "pclk_gate_mmc0_ioc", "clk_div_mmc0bus",
>  	  CLK_SET_RATE_PARENT, 0x40, 13, 0, },
>  	{ HI3670_PCLK_GATE_MMC1_IOC, "pclk_gate_mmc1_ioc", "clk_div_mmc1bus",
> -	  CLK_SET_RATE_PARENT, 0x420, 21, 0, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x420, 21, 0, },
>  	{ HI3670_CLK_GATE_DMAC, "clk_gate_dmac", "clk_div_sysbus",
>  	  CLK_SET_RATE_PARENT, 0x30, 1, 0, },
>  	{ HI3670_CLK_GATE_VCODECBUS2DDR, "clk_gate_vcodecbus2ddr", "clk_div_vcodecbus",
> @@ -166,7 +166,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
>  	{ HI3670_CLK_CCI400_BYPASS, "clk_cci400_bypass", "clk_ddrc_freq",
>  	  CLK_SET_RATE_PARENT, 0x22C, 28, 0, },
>  	{ HI3670_CLK_GATE_CCI400, "clk_gate_cci400", "clk_ddrc_freq",
> -	  CLK_SET_RATE_PARENT, 0x50, 14, 0, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x50, 14, 0, },
>  	{ HI3670_CLK_GATE_SD, "clk_gate_sd", "clk_mux_sd_sys",
>  	  CLK_SET_RATE_PARENT, 0x40, 17, 0, },
>  	{ HI3670_HCLK_GATE_SD, "hclk_gate_sd", "clk_div_sysbus",
> @@ -248,7 +248,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_sep_clks[] = {
>  	{ HI3670_CLK_GATE_AO_ASP, "clk_gate_ao_asp", "clk_div_ao_asp",
>  	  CLK_SET_RATE_PARENT, 0x0, 26, 0, },
>  	{ HI3670_PCLK_GATE_PCTRL, "pclk_gate_pctrl", "clk_div_ptp",
> -	  CLK_SET_RATE_PARENT, 0x20, 31, 0, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x20, 31, 0, },
>  	{ HI3670_CLK_CSI_TRANS_GT, "clk_csi_trans_gt", "clk_div_csi_trans",
>  	  CLK_SET_RATE_PARENT, 0x30, 24, 0, },
>  	{ HI3670_CLK_DSI_TRANS_GT, "clk_dsi_trans_gt", "clk_div_dsi_trans",
> @@ -301,7 +301,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_clks[] = {
>  	{ HI3670_PCLK_ANDGT_MMC1_PCIE, "pclk_andgt_mmc1_pcie", "clk_div_320m",
>  	  CLK_SET_RATE_PARENT, 0xf8, 13, CLK_GATE_HIWORD_MASK, },
>  	{ HI3670_CLK_GATE_VCODECBUS_GT, "clk_gate_vcodecbus_gt", "clk_mux_vcodecbus",
> -	  CLK_SET_RATE_PARENT, 0x0F0, 8, CLK_GATE_HIWORD_MASK, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x0F0, 8, CLK_GATE_HIWORD_MASK, },
>  	{ HI3670_CLK_ANDGT_SD, "clk_andgt_sd", "clk_mux_sd_pll",
>  	  CLK_SET_RATE_PARENT, 0xF4, 3, CLK_GATE_HIWORD_MASK, },
>  	{ HI3670_CLK_SD_SYS_GT, "clk_sd_sys_gt", "clkin_sys",
> @@ -331,7 +331,7 @@ static const struct hisi_gate_clock hi3670_crgctrl_gate_clks[] = {
>  	{ HI3670_CLK_GATE_DSI_TRANS, "clk_gate_dsi_trans", "clk_ppll2",
>  	  CLK_SET_RATE_PARENT, 0xF4, 1, CLK_GATE_HIWORD_MASK, },
>  	{ HI3670_CLK_ANDGT_PTP, "clk_andgt_ptp", "clk_div_320m",
> -	  CLK_SET_RATE_PARENT, 0xF8, 5, CLK_GATE_HIWORD_MASK, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0xF8, 5, CLK_GATE_HIWORD_MASK, },
>  	{ HI3670_CLK_ANDGT_OUT0, "clk_andgt_out0", "clk_ppll0",
>  	  CLK_SET_RATE_PARENT, 0xF0, 10, CLK_GATE_HIWORD_MASK, },
>  	{ HI3670_CLK_ANDGT_OUT1, "clk_andgt_out1", "clk_ppll0",
> @@ -569,9 +569,9 @@ static const struct hisi_gate_clock hi3670_sctrl_gate_sep_clks[] = {
>  	{ HI3670_PCLK_GATE_SPI, "pclk_gate_spi", "clk_div_ioperi",
>  	  CLK_SET_RATE_PARENT, 0x1B0, 10, 0, },
>  	{ HI3670_CLK_GATE_UFS_SUBSYS, "clk_gate_ufs_subsys", "clk_div_ufs_subsys",
> -	  CLK_SET_RATE_PARENT, 0x1B0, 14, 0, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x1B0, 14, 0, },
>  	{ HI3670_CLK_GATE_UFSIO_REF, "clk_gate_ufsio_ref", "clkin_sys",
> -	  CLK_SET_RATE_PARENT, 0x1b0, 12, 0, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x1b0, 12, 0, },
>  	{ HI3670_PCLK_AO_GPIO0, "pclk_ao_gpio0", "clk_div_aobus",
>  	  CLK_SET_RATE_PARENT, 0x160, 11, 0, },
>  	{ HI3670_PCLK_AO_GPIO1, "pclk_ao_gpio1", "clk_div_aobus",
> @@ -593,7 +593,7 @@ static const struct hisi_gate_clock hi3670_sctrl_gate_sep_clks[] = {
>  	{ HI3670_PCLK_GATE_SYSCNT, "pclk_gate_syscnt", "clk_div_aobus",
>  	  CLK_SET_RATE_PARENT, 0x160, 19, 0, },
>  	{ HI3670_CLK_GATE_SYSCNT, "clk_gate_syscnt", "clkin_sys",
> -	  CLK_SET_RATE_PARENT, 0x160, 20, 0, },
> +	  CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x160, 20, 0, },
>  	{ HI3670_CLK_GATE_ASP_SUBSYS_PERI, "clk_gate_asp_subsys_peri",
>  	  "clk_mux_asp_subsys_peri",
>  	  CLK_SET_RATE_PARENT, 0x170, 6, 0, },
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] clk: HiSilicon: clk-hi3670: mark critical clocks as such
  2021-08-20 17:52 ` Manivannan Sadhasivam
@ 2021-08-20 18:40   ` Mauro Carvalho Chehab
  2021-09-15 12:38     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2021-08-20 18:40 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Stephen Boyd, Michael Turquette, linuxarm, mauro.chehab,
	linux-clk, linux-kernel

Em Fri, 20 Aug 2021 23:22:33 +0530
Manivannan Sadhasivam <mani@kernel.org> escreveu:

> On Fri, Aug 20, 2021 at 05:35:43PM +0200, Mauro Carvalho Chehab wrote:
> > The fixed-rate and fixed_factor clocks are critical to the
> > system: disabling them causes HiKey970 to not finish booting.
> >   
> 
> You mean all fixed rate and fixed factor clocks are critical to the system?
> I don't think so.

I tested one by one a couple of times. It took me ~2 weeks or so to produce
this patch. I applied first a patch adding CLK_IGNORE_UNUSED to
all clocks. Then, I broke the big change into hundreds of individual
patches with:

	TOP=$(git log -1 --pretty=%h)

	mkdir -p patches/
	rm patches/*

	COUNT=$(git grep CLK_IGNORE_UNUSED drivers/clk/hisilicon/clk-hi3670.c|wc -l)
	for i in $(seq 1 $COUNT); do
		sed '0,/ | CLK_IGNORE_UNUSED/s///' -i drivers/clk/hisilicon/clk-hi3670.c; git commit -a -m "change $i" --no-edit
	done

	git format-patch ${TOP}.. -o patches/
	(cd patches/; ls *.patch >series)

	git reset --hard $TOP
	git commit -a -m TEST --no-edit --allow-empty

Then, I ran this script in order to test each change individually[1]:

	#!/bin/bash
	trap ctrl_c INT
	
	function ctrl_c() {
	        echo "** Aborting script"
		exit 1
	}
	
	LANG=C
		
	LAST=$(cat patches/series|sort -n|tail -1)

	while :; do
		TOP=$(quilt top)
		echo $TOP

		git commit --amend -a --allow-empty --no-edit
		./test970.sh
		./test_host || break
		ssh hikey970 ./check_kernel
		if [ "$?" != "0" ]; then
			TOP=`quilt top`
			quilt pop
			toquilt $TOP
		fi
		if [ "$TOP" == "patches/$LAST" ]; then
			echo "Finishing."
			exit 0
		fi
		quilt push
	done

Every time the script fails, I had to power off it by hand, power it
on and then boot it with a working Kernel, moving the offending patch
to the end of quilt, with:

	TOP=`quilt top`; quilt pop; toquilt $TOP

[1] This script uses a couple of ancillary scripts:

     a) test970.sh:
	pushes the change from my git tree into the HiKey970,
	builds a new kernel and boots it;
     b) test_host:
	check if the board's network came up after booting;
     c) ./check_kernel:
	checks if the boot succeeded and the board is at a sane state;
     d) toquilt:
	moves a patch to the end of the quilt tree.

Once I finished. I re-checked the patches, in order to double check
that the unapplied patches are all causing issues.

At the end, I renamed the flags with:

	$ sed s,CLK_IGNORE_UNUSED,CLK_IS_CRITICAL, -i drivers/clk/hisilicon/clk-hi3670.c
      
So, yeah, I'm pretty sure that fixed rate and fixed factor clocks
are all critical to the system.

> You should identify the gateable clocks that are causing the issues and see if
> they can be marked as critical.

They were already checked by the above procedure.

> 
> Also, at what point of time you are seeing the boot issue?

That depends on the clock. See, this device is taking ~11 seconds to finish
booting and starting a prompt at the console here.

Most of critical clocks cause a panic() with SError up to ~2 seconds after 
start booting:

	[    2.173211] SError Interrupt on CPU3, code 0xbf000002 -- SError
	[    2.173216] CPU: 3 PID: 102 Comm: kworker/3:1H Not tainted 5.14.0-rc1+ #723
	[    2.173218] Hardware name: HiKey970 (DT)
	[    2.173220] Workqueue: kblockd blk_mq_run_work_fn
	[    2.173226] pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO BTYPE=--)
	[    2.173228] pc : el1_interrupt+0x20/0x80
	[    2.173229] lr : el1h_64_irq_handler+0x18/0x24
	[    2.173231] sp : ffff80001283b920
	[    2.173233] x29: ffff80001283b920 x28: ffff000100b6c740 x27: ffff0001011e4040
	[    2.173239] x26: 0000000000000001 x25: ffff0001011e4dc0 x24: 000000000000001b
	[    2.173244] x23: 0000000080000005 x22: ffff800010ed5078 x21: ffff80001283baa0
	[    2.173249] x20: ffff8000104e42e0 x19: ffff80001283b950 x18: ffff8000121db8c0
	[    2.173255] x17: 0000000000001800 x16: 0000000000000000 x15: 0000000000000000
	[    2.173260] x14: 00000000000002a7 x13: ffff000101d56298 x12: 0000000000000001
	[    2.173266] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
	[    2.173271] x8 : 0000000000000000 x7 : 0001000000a30100 x6 : 00000000016ec5e1
	[    2.173276] x5 : 00ffffffffffffff x4 : 0000000000000000 x3 : ffff0001011e2030
	[    2.173281] x2 : 0000000000000000 x1 : 00000000000000c0 x0 : ffff80001283b950
	[    2.173288] Kernel panic - not syncing: Asynchronous SError Interrupt
	[    2.173290] CPU: 3 PID: 102 Comm: kworker/3:1H Not tainted 5.14.0-rc1+ #723
	[    2.173292] Hardware name: HiKey970 (DT)
	[    2.173294] Workqueue: kblockd blk_mq_run_work_fn
	[    2.173297] Call trace:
	[    2.173298]  dump_backtrace+0x0/0x1e0
	[    2.173300]  show_stack+0x18/0x24
	[    2.173302]  dump_stack_lvl+0x68/0x84
	[    2.173304]  dump_stack+0x18/0x34
	[    2.173305]  panic+0x16c/0x334
	[    2.173307]  nmi_panic+0x8c/0x90
	[    2.173309]  arm64_serror_panic+0x78/0x84
	[    2.173310]  do_serror+0x58/0x5c
	[    2.173312]  el1h_64_error_handler+0x30/0x50
	[    2.173314]  el1h_64_error+0x78/0x7c
	[    2.173316]  el1_interrupt+0x20/0x80
	[    2.173318]  el1h_64_irq_handler+0x18/0x24
	[    2.173319]  el1h_64_irq+0x78/0x7c
	[    2.173321]  _raw_spin_unlock_irqrestore+0x18/0x50
	[    2.173323]  ufshcd_queuecommand+0x34c/0x480
	[    2.173324]  scsi_queue_rq+0x420/0x96c
	[    2.173327]  blk_mq_dispatch_rq_list+0x118/0x800
	[    2.173328]  __blk_mq_do_dispatch_sched+0x238/0x2b4
	[    2.173330]  __blk_mq_sched_dispatch_requests+0x110/0x170
	[    2.173332]  blk_mq_sched_dispatch_requests+0x3c/0x80
	[    2.173334]  __blk_mq_run_hw_queue+0x54/0x90
	[    2.173336]  blk_mq_run_work_fn+0x24/0x30
	[    2.173338]  process_one_work+0x1cc/0x350
	[    2.173340]  worker_thread+0x138/0x46c
	[    2.173341]  kthread+0x150/0x160
	[    2.173343]  ret_from_fork+0x10/0x18
	[    2.173365] SMP: stopping secondary CPUs
	[    2.173367] Kernel Offset: disabled
	[    2.173369] CPU features: 0x00003051,00000846
	[    2.173371] Memory Limit: none

a couple of clocks that cause the device to stop producing any dmesg 
after ~2 seconds:

	...
	[    1.886460] printk: bootconsole [pl11] disabled
	[    1.886460] printk: bootconsole [pl11] disabled
	[    1.924965] i2c 0-004e: Fixing up cyclic dependency with hikey_usbhub
	[    1.925433]  sdc: sdc1 sdc2 sdc3 sdc4 sdc5 sdc6 sdc7 sdc8 sdc9 sdc10 sdc11 sdc12
	[    1.935446] dwmmc_k3 fc183000.dwmmc2: fifo-depth property not found, using value of FIFOTH register as default
	[    1.935977] ALSA device list:
	[    1.935977]   No soundcards found.
	[    1.935977] sd 0:0:0:3: [sdc] Attached SCSI disk
	[    1.935977] uart-pl011 fff32000.serial: no DMA platform data
	<no more dmesg>

There are some clocks that cause SError panic() after ~4 seconds or so,
when some driver would be probed, like this one:

	[    4.012925] Kernel panic - not syncing: Asynchronous SError Interrupt
	[    4.012926] CPU: 5 PID: 142 Comm: kworker/u16:3 Not tainted 5.14.0-rc1+ #718
	[    4.012927] Hardware name: HiKey970 (DT)
	[    4.012928] Workqueue: events_unbound deferred_probe_work_func
	[    4.012930] Call trace:
	[    4.012930]  dump_backtrace+0x0/0x1e0
	[    4.012931]  show_stack+0x18/0x24
	[    4.012932]  dump_stack_lvl+0x68/0x84
	[    4.012932]  dump_stack+0x18/0x34
	[    4.012933]  panic+0x16c/0x334
	[    4.012934]  nmi_panic+0x8c/0x90
	[    4.012934]  arm64_serror_panic+0x78/0x84
	[    4.012935]  do_serror+0x58/0x5c
	[    4.012936]  el1h_64_error_handler+0x30/0x50
	[    4.012937]  el1h_64_error+0x78/0x7c
	[    4.012938]  pin_request+0x8c/0x2b0
	[    4.012939]  pinmux_request_gpio+0x50/0x9c
	[    4.012939]  pinctrl_gpio_request+0x64/0x1bc
	[    4.012940]  gpiochip_generic_request+0x30/0x4c
	[    4.012941]  gpiod_request_commit+0xf8/0x1d0
	[    4.012942]  gpiod_request.part.0+0xd0/0x10c
	[    4.012942]  gpiod_request+0x44/0xd0
	[    4.012943]  gpio_request+0x30/0x50
	[    4.012944]  devm_gpio_request+0x54/0xb0
	[    4.012945]  kirin_pcie_probe+0x738/0x88c [pcie_kirin]
	[    4.012945]  platform_probe+0x68/0xe0
	[    4.012946]  really_probe+0x1b0/0x42c
	[    4.012947]  __driver_probe_device+0x114/0x190
	[    4.012948]  driver_probe_device+0x40/0x100
	[    4.012949]  __device_attach_driver+0x98/0x130
	[    4.012950]  bus_for_each_drv+0x78/0xd0
	[    4.012951]  __device_attach+0xdc/0x1c0
	[    4.012951]  device_initial_probe+0x14/0x20
	[    4.012952]  bus_probe_device+0x98/0xa0
	[    4.012953]  deferred_probe_work_func+0x9c/0xf0
	[    4.012954]  process_one_work+0x1cc/0x350
	[    4.012954]  worker_thread+0x138/0x46c
	[    4.012955]  kthread+0x150/0x160
	[    4.012956]  ret_from_fork+0x10/0x18
	[    4.012978] SMP: stopping secondary CPUs
	[    4.012979] Kernel Offset: disabled
	[    4.012979] CPU features: 0x00003051,00000846
	[    4.012980] Memory Limit: none


There are some clocks that make the device to not panic(). Instead, it seems
to disable the access to the disk, causing UFS to fail:

	[    2.701778] ufshcd-hisi ff3c0000.ufs: uic cmd 0x1 with arg3 0x0 completion timeout
	[    2.709352] ufshcd-hisi ff3c0000.ufs: dme-get: attr-id 0x1571 failed 0 retries
	[    2.817775] ufshcd-hisi ff3c0000.ufs: __ufshcd_issue_tm_cmd: task management cmd 0x80 timed-out
	[    3.830445] ------------[ cut here ]------------
	[    3.835068] ufshcd-hisi ff3c0000.ufs: __ufshcd_issue_tm_cmd: unable to clear tm cmd (slot 2) after timeout
	[    3.844736] WARNING: CPU: 6 PID: 140 at drivers/scsi/ufs/ufshcd.c:6575 __ufshcd_issue_tm_cmd+0x2dc/0x2fc
	[    3.854225] Modules linked in:
	[    3.857277] CPU: 6 PID: 140 Comm: kworker/u16:2 Not tainted 5.14.0-rc1+ #725
	[    3.864321] Hardware name: HiKey970 (DT)
	[    3.868238] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
	[    3.873288] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
	[    3.879289] pc : __ufshcd_issue_tm_cmd+0x2dc/0x2fc
	[    3.884075] lr : __ufshcd_issue_tm_cmd+0x2dc/0x2fc
	[    3.888858] sp : ffff800012a3bb90
	[    3.892164] x29: ffff800012a3bb90 x28: 00000000000000a0 x27: ffff0001011e2920
	[    3.899297] x26: ffff0001011e2000 x25: 0000000000000000 x24: ffff0001014e0280
	[    3.906429] x23: 0000000000000004 x22: 0000000000000000 x21: 0000000000000002
	[    3.913560] x20: ffff80001115cf98 x19: ffff0001011e2860 x18: 0000000000000000
	[    3.920691] x17: 7261656c63206f74 x16: 20656c62616e7520 x15: 0720072007200720
	[    3.927822] x14: 0720072007200720 x13: ffff0001b87e0000 x12: 0000000000000444
	[    3.934953] x11: 000000000000016c x10: ffff0001bf2386c0 x9 : 00000000ffff0000
	[    3.942083] x8 : ffff0001b87e0000 x7 : ffff0001bf2386c0 x6 : 0000000000000000
	[    3.949215] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
	[    3.956345] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000101071c80
	[    3.963477] Call trace:
	[    3.965916]  __ufshcd_issue_tm_cmd+0x2dc/0x2fc
	[    3.970353]  ufshcd_issue_tm_cmd+0x74/0xf0
	[    3.974442]  ufshcd_try_to_abort_task+0x84/0x21c
	[    3.979053]  ufshcd_err_handler+0x4a0/0xad0
	[    3.983229]  process_one_work+0x1cc/0x350
	[    3.987236]  worker_thread+0x138/0x46c
	[    3.990978]  kthread+0x150/0x160
	[    3.994200]  ret_from_fork+0x10/0x18
	[    3.997772] ---[ end trace 7cf4829a5cdb528e ]---
	[    4.002392] ufshcd-hisi ff3c0000.ufs: ufshcd_try_to_abort_task: no response from device. tag = 18, err -110
	[    4.045167] ufshcd-hisi ff3c0000.ufs: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
	[    4.074477] ufshcd-hisi ff3c0000.ufs: ufshcd_print_pwr_info:[RX, TX]: gear=[3, 3], lane[2, 2], pwr[FAST MODE, FAST MODE], rate = 2
	[    5.614829] ufshcd-hisi ff3c0000.ufs: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
	[    7.150830] ufshcd-hisi ff3c0000.ufs: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
	[    8.686827] ufshcd-hisi ff3c0000.ufs: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
	[    8.697091] ufshcd-hisi ff3c0000.ufs: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
	[    8.709437] ufshcd-hisi ff3c0000.ufs: ufshcd_set_active_icc_lvl: Failed reading power descriptor.len = 98 ret = -11
	[    8.756444] ufshcd-hisi ff3c0000.ufs: ufshcd_intr: Unhandled interrupt 0x00000000 (0x00000010, 0x00000010)
	...
	[   98.230000] ufshcd-hisi ff3c0000.ufs: ufshcd_print_pwr_info:[RX, TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE], rate = 0
	[   98.259323] ufshcd-hisi ff3c0000.ufs: ufshcd_print_pwr_info:[RX, TX]: gear=[3, 3], lane[2, 2], pwr[FAST MODE, FAST MODE], rate = 2
	[   99.790788] ufshcd-hisi ff3c0000.ufs: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
	[  101.326787] ufshcd-hisi ff3c0000.ufs: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
	[  102.862787] ufshcd-hisi ff3c0000.ufs: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
	[  102.873051] ufshcd-hisi ff3c0000.ufs: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
	[  102.885396] ufshcd-hisi ff3c0000.ufs: ufshcd_set_active_icc_lvl: Failed reading power descriptor.len = 98 ret = -11
	[  102.932393] ufshcd-hisi ff3c0000.ufs: ufshcd_intr: Unhandled interrupt 0x00000000 (0x00000010, 0x00000010)
	...

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] clk: HiSilicon: clk-hi3670: mark critical clocks as such
  2021-08-20 18:40   ` Mauro Carvalho Chehab
@ 2021-09-15 12:38     ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2021-09-15 12:38 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Stephen Boyd
  Cc: Michael Turquette, linuxarm, mauro.chehab, linux-clk,
	linux-kernel, Kees Cook, Greg Kroah-Hartman

Em Fri, 20 Aug 2021 20:40:23 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> Em Fri, 20 Aug 2021 23:22:33 +0530
> Manivannan Sadhasivam <mani@kernel.org> escreveu:
> 
> > On Fri, Aug 20, 2021 at 05:35:43PM +0200, Mauro Carvalho Chehab wrote:
> > > The fixed-rate and fixed_factor clocks are critical to the
> > > system: disabling them causes HiKey970 to not finish booting.
> > >   
> > 
> > You mean all fixed rate and fixed factor clocks are critical to the system?
> > I don't think so.

I suspect that the actual issue is related to this (at 
drivers/clk/clk.c):

	late_initcall_sync(clk_disable_unused);

Based on my tests, clk_disable_unused() is called too early,
before the init code from the drivers built as module to be called.

See, if the system is left to disable unused clocks, those are the last
messages at the console:

	[   22.348592] initcall acpi_gpio_handle_deferred_request_irqs+0x0/0xa8 returned 0 after 1 usecs, irqs_disabled() 0
	[   22.366973] calling  fb_logo_late_init+0x0/0x20 @ 1 irqs_disabled() 0
	[   22.373432] initcall fb_logo_late_init+0x0/0x20 returned 0 after 1 usecs, irqs_disabled() 0

	[   22.381800] calling  clk_disable_unused+0x0/0xe8 @ 1 irqs_disabled() 0
				==================

	<SoC dies here... no other messages>

When I disable clk_disable_unused, There are several other initcall logs
after it:

	[   22.340305] calling  acpi_gpio_handle_deferred_request_irqs+0x0/0xa8 @ 1 irqs_disabled() 0
	[   22.348594] dwmmc_k3 fc183000.dwmmc2: card claims to support voltages below defined range
	[   22.348592] initcall acpi_gpio_handle_deferred_request_irqs+0x0/0xa8 returned 0 after 1 usecs, irqs_disabled() 0
	[   22.366973] calling  fb_logo_late_init+0x0/0x20 @ 1 irqs_disabled() 0
	[   22.373432] initcall fb_logo_late_init+0x0/0x20 returned 0 after 1 usecs, irqs_disabled() 0

	[   22.356984] initcall clk_disable_unused+0x0/0xe8 returned 0 after 117 usecs, irqs_disabled() 0
				==================

	[   22.372335] initcall imx_clk_disable_uart+0x0/0x88 returned 0 after 1 usecs, irqs_disabled() 0
	[   22.387946] initcall regulator_init_complete+0x0/0x58 returned 0 after 2 usecs, irqs_disabled() 0
	[   22.404163] initcall of_platform_sync_state_init+0x0/0x20 returned 0 after 1 usecs, irqs_disabled() 0
	[   22.426508] initcall alsa_sound_last_init+0x0/0x90 returned 0 after 6239 usecs, irqs_disabled() 0
	[   22.703071] initcall inet6_init+0x0/0x358 [ipv6] returned 0 after 13341 usecs, irqs_disabled() 0
	[   22.723861] initcall xt_init+0x0/0x1000 [x_tables] returned 0 after 8 usecs, irqs_disabled() 0
	[   22.744405] initcall ip_tables_init+0x0/0x1000 [ip_tables] returned 0 after 23 usecs, irqs_disabled() 0
	[   23.467003] initcall fuse_init+0x0/0x154 [fuse] returned 0 after 392 usecs, irqs_disabled() 0
	[   23.537742] initcall drm_core_init+0x0/0x1000 [drm] returned 0 after 122 usecs, irqs_disabled() 0
	[   24.519076] initcall rfkill_init+0x0/0x12c [rfkill] returned 0 after 15654 usecs, irqs_disabled() 0
	[   24.622168] initcall hi3670_pcie_phy_driver_init+0x0/0x1000 [phy_hi3670_pcie] returned 0 after 836 usecs, irqs_disabled() 0
	[   24.665100] initcall hi3670_phy_driver_init+0x0/0x1000 [phy_hi3670_usb3] returned 0 after 1888 usecs, irqs_disabled() 0
	[   24.694668] initcall typec_init+0x0/0x1000 [typec] returned 0 after 89 usecs, irqs_disabled() 0
	[   24.732557] initcall cpu_feature_match_ASIMD_init+0x0/0x1000 [crct10dif_ce] returned 0 after 8838 usecs, irqs_disabled() 0
	[   24.746636] initcall tcpci_i2c_driver_init+0x0/0x1000 [tcpci] returned 0 after 8607 usecs, irqs_disabled() 0
	[   24.774541] initcall hisi_hikey_usb_driver_init+0x0/0x1000 [hisi_hikey_usb] returned 0 after 35860 usecs, irqs_disabled() 0
	[   24.892957] initcall rt1711h_i2c_driver_init+0x0/0x1000 [tcpci_rt1711h] returned 0 after 21500 usecs, irqs_disabled() 0
	[   24.956528] initcall wl1271_init+0x0/0x1000 [wlcore_sdio] returned 0 after 83582 usecs, irqs_disabled() 0
	[   25.039853] initcall cfg80211_init+0x0/0xdc [cfg80211] returned 0 after 26291 usecs, irqs_disabled() 0
	[   25.118288] initcall ieee80211_init+0x0/0x40 [mac80211] returned 0 after 15 usecs, irqs_disabled() 0
	[   25.335203] initcall wl18xx_driver_init+0x0/0x1000 [wl18xx] returned 0 after 134423 usecs, irqs_disabled() 0
	[   26.277300] initcall ecdh_init+0x0/0xd0 [ecdh_generic] returned 0 after 302 usecs, irqs_disabled() 0
	[   26.435409] initcall bt_init+0x0/0xcc [bluetooth] returned 0 after 63051 usecs, irqs_disabled() 0
	[   26.508033] initcall btusb_driver_init+0x0/0x1000 [btusb] returned 0 after 305 usecs, irqs_disabled() 0
	[   27.333049] initcall kirin_pcie_driver_init+0x0/0x1000 [pcie_kirin] returned 0 after 805983 usecs, irqs_disabled() 0

It may be unrelated, but, when checking .config, the above init codes
seem to belong to the drivers compiled as modules:

	CONFIG_RFKILL=m
	CONFIG_DRM=m
	CONFIG_TYPEC_RT1711H=m
	CONFIG_TYPEC_TCPCI=m
	CONFIG_WL18XX=m
	CONFIG_BT=m
	CONFIG_HISI_HIKEY_USB=m
	CONFIG_PHY_HI3670_USB=m
	CONFIG_PHY_HI3670_PCIE=m

On other words, late_initcall_sync() is calling clk_disable_unused()
too early. Either its implementation should be changed to take
module driver init into account, or something like:

	#define after_all_initcalls(fn)		__define_initcall(fn, 8)

would need to be added, in order for this to work properly.

Another alternative would be to start a thread (or a delayed thread)
via late_initcall_sync() that would only call clk_disable_unused()
after being sure that *all* driver init code will have finished.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-15 12:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 15:35 [PATCH v3] clk: HiSilicon: clk-hi3670: mark critical clocks as such Mauro Carvalho Chehab
2021-08-20 17:52 ` Manivannan Sadhasivam
2021-08-20 18:40   ` Mauro Carvalho Chehab
2021-09-15 12:38     ` Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).