[PATCH v2 00/11] Switch to using CRITICAL clks for Tegra

[PATCH v2 00/11] Switch to using CRITICAL clks for Tegra

[Rhyland Klein]

Switch to defining clks that need to be on as CRITICAL rather than
using the init_tables defined to enable clks. Some of these may be
able to be converted to HAND_OFF clks, when that is supported.

I added a patch to mark CRITICAL clks in the clk_summary table, figuring
it would be handy to be able to easily tell what clks are CRITICAL
through the table.

I added a warning for using the init_table to enable clks. I considered
removing it entirely, but I thought phasing it out might be better.

Rhyland Klein (11):
  clk: tegra: Switch to using critical clks
  clk: tegra20: Mark required clks as CRITICAL
  clk: tegra20: clean up init_table
  clk: tegra30: Mark certain clks as critical
  clk: tegra30: clean up init_table
  clk: tegra114: clean up init_table
  clk: tegra124: clean up init_table
  clk: tegra210: Mark required clks as CRITICAL
  clk: tegra210: clean up init_table
  clk: Show CRITICAL clks in clk_summary output
  clk: tegra: WARN if clk in the init_table has enable

 drivers/clk/clk.c                        |  7 ++++--
 drivers/clk/tegra/clk-tegra-periph.c     | 21 ++++++++++------
 drivers/clk/tegra/clk-tegra-super-gen4.c | 12 ++++++----
 drivers/clk/tegra/clk-tegra114.c         | 11 ++++-----
 drivers/clk/tegra/clk-tegra124.c         | 19 ++++++---------
 drivers/clk/tegra/clk-tegra20.c          | 41 +++++++++++++++-----------------
 drivers/clk/tegra/clk-tegra210.c         | 28 ++++++++--------------
 drivers/clk/tegra/clk-tegra30.c          | 28 ++++++++--------------
 drivers/clk/tegra/clk.c                  |  5 ++++
 9 files changed, 81 insertions(+), 91 deletions(-)

-- 
1.9.1

[PATCH v2 01/11] clk: tegra: Switch to using critical clks

[Rhyland Klein]

Mark some of the required-to-be-enabled clks as critical clks. These
need to be kept on through the disabling of unused clks during init.
They may not get any reference before or after init, but are required
to be on, therefore let the core enable them.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
v2:
 - Remove mention of HAND_OFF clks as they are not supported yet.

 drivers/clk/tegra/clk-tegra-periph.c     | 21 ++++++++++++++-------
 drivers/clk/tegra/clk-tegra-super-gen4.c | 12 +++++++-----
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 29d04c663abf..9365770bcaa5 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -162,6 +162,13 @@
 			_clk_num, _gate_flags, _clk_id, _parents##_idx, 0,\
 			NULL)
 
+#define MUX8_FLAGS(_name, _parents, _offset,\
+			     _clk_num, _gate_flags, _clk_id, _flags)	\
+	TEGRA_INIT_DATA_TABLE(_name, NULL, NULL, _parents, _offset,\
+			29, MASK(8), 0, 0, 8, 1, TEGRA_DIVIDER_ROUND_UP,\
+			_clk_num, _gate_flags, _clk_id, _parents##_idx, _flags,\
+			NULL)
+
 #define MUX8_NOGATE_LOCK(_name, _parents, _offset, _clk_id, _lock)	\
 	TEGRA_INIT_DATA_TABLE(_name, NULL, NULL, _parents, _offset,	\
 			      29, MASK(3), 0, 0, 8, 1, TEGRA_DIVIDER_ROUND_UP,\
@@ -651,7 +658,7 @@ static struct tegra_periph_init_data periph_clks[] = {
 	INT8("3d", mux_pllm_pllc2_c_c3_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d_8),
 	INT8("vic03", mux_pllm_pllc_pllp_plla_pllc2_c3_clkm, CLK_SOURCE_VIC03, 178, 0, tegra_clk_vic03),
 	INT8("vic03", mux_pllc_pllp_plla1_pllc2_c3_clkm, CLK_SOURCE_VIC03, 178, 0, tegra_clk_vic03_8),
-	INT_FLAGS("mselect", mux_pllp_clkm, CLK_SOURCE_MSELECT, 99, 0, tegra_clk_mselect, CLK_IGNORE_UNUSED),
+	INT_FLAGS("mselect", mux_pllp_clkm, CLK_SOURCE_MSELECT, 99, 0, tegra_clk_mselect, CLK_IGNORE_UNUSED | CLK_IS_CRITICAL),
 	MUX("i2s0", mux_pllaout0_audio0_2x_pllp_clkm, CLK_SOURCE_I2S0, 30, TEGRA_PERIPH_ON_APB, tegra_clk_i2s0),
 	MUX("i2s1", mux_pllaout0_audio1_2x_pllp_clkm, CLK_SOURCE_I2S1, 11, TEGRA_PERIPH_ON_APB, tegra_clk_i2s1),
 	MUX("i2s2", mux_pllaout0_audio2_2x_pllp_clkm, CLK_SOURCE_I2S2, 18, TEGRA_PERIPH_ON_APB, tegra_clk_i2s2),
@@ -691,8 +698,8 @@ static struct tegra_periph_init_data periph_clks[] = {
 	MUX("dsiblp", mux_pllp_pllc_clkm, CLK_SOURCE_DSIBLP, 148, 0, tegra_clk_dsiblp),
 	MUX("tsensor", mux_pllp_pllc_clkm_clk32, CLK_SOURCE_TSENSOR, 100, TEGRA_PERIPH_ON_APB, tegra_clk_tsensor),
 	MUX("actmon", mux_pllp_pllc_clk32_clkm, CLK_SOURCE_ACTMON, 119, 0, tegra_clk_actmon),
-	MUX("dfll_ref", mux_pllp_clkm, CLK_SOURCE_DFLL_REF, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_ref),
-	MUX("dfll_soc", mux_pllp_clkm, CLK_SOURCE_DFLL_SOC, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_soc),
+	MUX_FLAGS("dfll_ref", mux_pllp_clkm, CLK_SOURCE_DFLL_REF, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_ref, CLK_IS_CRITICAL),
+	MUX_FLAGS("dfll_soc", mux_pllp_clkm, CLK_SOURCE_DFLL_SOC, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_soc, CLK_IS_CRITICAL),
 	MUX("i2cslow", mux_pllp_pllc_clk32_clkm, CLK_SOURCE_I2CSLOW, 81, TEGRA_PERIPH_ON_APB, tegra_clk_i2cslow),
 	MUX("sbc1", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SBC1, 41, TEGRA_PERIPH_ON_APB, tegra_clk_sbc1),
 	MUX("sbc2", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SBC2, 44, TEGRA_PERIPH_ON_APB, tegra_clk_sbc2),
@@ -730,7 +737,7 @@ static struct tegra_periph_init_data periph_clks[] = {
 	MUX8("ndflash", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_NDFLASH, 13, TEGRA_PERIPH_ON_APB, tegra_clk_ndflash_8),
 	MUX8("ndspeed", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_NDSPEED, 80, TEGRA_PERIPH_ON_APB, tegra_clk_ndspeed_8),
 	MUX8("hdmi", mux_pllp_pllm_plld_plla_pllc_plld2_clkm, CLK_SOURCE_HDMI, 51, 0, tegra_clk_hdmi),
-	MUX8("extern1", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN1, 120, 0, tegra_clk_extern1),
+	MUX8_FLAGS("extern1", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN1, 120, 0, tegra_clk_extern1, CLK_IS_CRITICAL),
 	MUX8("extern2", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN2, 121, 0, tegra_clk_extern2),
 	MUX8("extern3", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN3, 122, 0, tegra_clk_extern3),
 	MUX8("soc_therm", mux_pllm_pllc_pllp_plla, CLK_SOURCE_SOC_THERM, 78, TEGRA_PERIPH_ON_APB, tegra_clk_soc_therm),
@@ -744,7 +751,7 @@ static struct tegra_periph_init_data periph_clks[] = {
 	MUX8("clk72mhz", mux_pllp3_pllc_clkm, CLK_SOURCE_CLK72MHZ, 177, TEGRA_PERIPH_NO_RESET, tegra_clk_clk72Mhz),
 	MUX8("clk72mhz", mux_pllp_out3_pllp_pllc_clkm, CLK_SOURCE_CLK72MHZ, 177, TEGRA_PERIPH_NO_RESET, tegra_clk_clk72Mhz_8),
 	MUX8_NOGATE_LOCK("sor0_lvds", mux_pllp_pllm_plld_plla_pllc_plld2_clkm, CLK_SOURCE_SOR0, tegra_clk_sor0_lvds, &sor0_lock),
-	MUX_FLAGS("csite", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_CSITE, 73, TEGRA_PERIPH_ON_APB, tegra_clk_csite, CLK_IGNORE_UNUSED),
+	MUX_FLAGS("csite", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_CSITE, 73, TEGRA_PERIPH_ON_APB, tegra_clk_csite, CLK_IGNORE_UNUSED | CLK_IS_CRITICAL),
 	MUX_FLAGS("csite", mux_pllp_pllre_clkm, CLK_SOURCE_CSITE, 73, TEGRA_PERIPH_ON_APB, tegra_clk_csite_8, CLK_IGNORE_UNUSED),
 	NODIV("disp1", mux_pllp_pllm_plld_plla_pllc_plld2_clkm, CLK_SOURCE_DISP1, 29, 7, 27, 0, tegra_clk_disp1, NULL),
 	NODIV("disp1", mux_pllp_plld_plld2_clkm, CLK_SOURCE_DISP1, 29, 7, 27, 0, tegra_clk_disp1_8, NULL),
@@ -816,7 +823,7 @@ static struct tegra_periph_init_data gate_clks[] = {
 	GATE("xusb_host", "xusb_host_src", 89, 0, tegra_clk_xusb_host, 0),
 	GATE("xusb_ss", "xusb_ss_src", 156, 0, tegra_clk_xusb_ss, 0),
 	GATE("xusb_dev", "xusb_dev_src", 95, 0, tegra_clk_xusb_dev, 0),
-	GATE("emc", "emc_mux", 57, 0, tegra_clk_emc, CLK_IGNORE_UNUSED),
+	GATE("emc", "emc_mux", 57, 0, tegra_clk_emc, CLK_IGNORE_UNUSED | CLK_IS_CRITICAL),
 	GATE("sata_cold", "clk_m", 129, TEGRA_PERIPH_ON_APB, tegra_clk_sata_cold, 0),
 	GATE("ispb", "clk_m", 3, 0, tegra_clk_ispb, 0),
 	GATE("vim2_clk", "clk_m", 11, 0, tegra_clk_vim2_clk, 0),
@@ -825,7 +832,7 @@ static struct tegra_periph_init_data gate_clks[] = {
 	GATE("pllg_ref", "pll_ref", 189, 0, tegra_clk_pll_g_ref, 0),
 	GATE("hsic_trk", "usb2_hsic_trk", 209, TEGRA_PERIPH_NO_RESET, tegra_clk_hsic_trk, 0),
 	GATE("usb2_trk", "usb2_hsic_trk", 210, TEGRA_PERIPH_NO_RESET, tegra_clk_usb2_trk, 0),
-	GATE("xusb_gate", "osc", 143, 0, tegra_clk_xusb_gate, 0),
+	GATE("xusb_gate", "osc", 143, 0, tegra_clk_xusb_gate, CLK_IS_CRITICAL),
 	GATE("pll_p_out_cpu", "pll_p", 223, 0, tegra_clk_pll_p_out_cpu, 0),
 	GATE("pll_p_out_adsp", "pll_p", 187, 0, tegra_clk_pll_p_out_adsp, 0),
 	GATE("apb2ape", "clk_m", 107, 0, tegra_clk_apb2ape, 0),
diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
index 474de0f0c26d..5283138af093 100644
--- a/drivers/clk/tegra/clk-tegra-super-gen4.c
+++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
@@ -116,7 +116,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
 		clk = tegra_clk_register_super_mux("sclk_mux",
 						gen_info->sclk_parents,
 						gen_info->num_sclk_parents,
-						CLK_SET_RATE_PARENT,
+						CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 						clk_base + SCLK_BURST_POLICY,
 						0, 4, 0, 0, NULL);
 		*dt_clk = clk;
@@ -137,7 +137,8 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
 			clk = tegra_clk_register_super_mux("sclk",
 						gen_info->sclk_parents,
 						gen_info->num_sclk_parents,
-						CLK_SET_RATE_PARENT,
+						CLK_SET_RATE_PARENT |
+						CLK_IS_CRITICAL,
 						clk_base + SCLK_BURST_POLICY,
 						0, 4, 0, 0, NULL);
 			*dt_clk = clk;
@@ -166,7 +167,8 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
 				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
 				   &sysrate_lock);
 	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
-				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
+				CLK_IGNORE_UNUSED | CLK_IS_CRITICAL,
+				clk_base + SYSTEM_CLK_RATE,
 				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
 	*dt_clk = clk;
 }
@@ -187,14 +189,14 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
 			clk = tegra_clk_register_super_mux("cclk_g",
 					gen_info->cclk_g_parents,
 					gen_info->num_cclk_g_parents,
-					CLK_SET_RATE_PARENT,
+					CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 					clk_base + CCLKG_BURST_POLICY,
 					0, 4, 8, 0, NULL);
 		} else {
 			clk = tegra_clk_register_super_mux("cclk_g",
 					gen_info->cclk_g_parents,
 					gen_info->num_cclk_g_parents,
-					CLK_SET_RATE_PARENT,
+					CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 					clk_base + CCLKG_BURST_POLICY,
 					0, 4, 0, 0, NULL);
 		}
-- 
1.9.1

[PATCH v2 02/11] clk: tegra20: Mark required clks as CRITICAL

[Rhyland Klein]

Mark clks that are required to be on as CRITICAL clks.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk-tegra20.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 837e5cbd60e9..4a60a25d7e61 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -715,13 +715,15 @@ static void tegra20_super_clk_init(void)
 
 	/* CCLK */
 	clk = tegra_clk_register_super_mux("cclk", cclk_parents,
-			      ARRAY_SIZE(cclk_parents), CLK_SET_RATE_PARENT,
+			      ARRAY_SIZE(cclk_parents),
+			      CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 			      clk_base + CCLK_BURST_POLICY, 0, 4, 0, 0, NULL);
 	clks[TEGRA20_CLK_CCLK] = clk;
 
 	/* SCLK */
 	clk = tegra_clk_register_super_mux("sclk", sclk_parents,
-			      ARRAY_SIZE(sclk_parents), CLK_SET_RATE_PARENT,
+			      ARRAY_SIZE(sclk_parents),
+			      CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 			      clk_base + SCLK_BURST_POLICY, 0, 4, 0, 0, NULL);
 	clks[TEGRA20_CLK_SCLK] = clk;
 
@@ -814,11 +816,11 @@ static void __init tegra20_periph_clk_init(void)
 	/* emc */
 	clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
 			       ARRAY_SIZE(mux_pllmcp_clkm),
-			       CLK_SET_RATE_NO_REPARENT,
+			       CLK_SET_RATE_NO_REPARENT | CLK_IS_CRITICAL,
 			       clk_base + CLK_SOURCE_EMC,
 			       30, 2, 0, &emc_lock);
-	clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base, 0,
-				    57, periph_clk_enb_refcnt);
+	clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base,
+				    CLK_IS_CRITICAL, 57, periph_clk_enb_refcnt);
 	clks[TEGRA20_CLK_EMC] = clk;
 
 	clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
-- 
1.9.1

[PATCH v2 03/11] clk: tegra20: clean up init_table

[Rhyland Klein]

Remove entries from the init_table where the clks are now defined
as CRITICAL clks, if we were only enabling them in the init_table.

Remove the flag to signal to enable CRITICAL clks if they are still
needed in the init_table to set other properties.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk-tegra20.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 4a60a25d7e61..c5bbd84735b8 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1021,28 +1021,23 @@ static struct tegra_cpu_car_ops tegra20_cpu_car_ops = {
 };
 
 static struct tegra_clk_init_table init_table[] __initdata = {
-	{ TEGRA20_CLK_PLL_P, TEGRA20_CLK_CLK_MAX, 216000000, 1 },
-	{ TEGRA20_CLK_PLL_P_OUT1, TEGRA20_CLK_CLK_MAX, 28800000, 1 },
-	{ TEGRA20_CLK_PLL_P_OUT2, TEGRA20_CLK_CLK_MAX, 48000000, 1 },
-	{ TEGRA20_CLK_PLL_P_OUT3, TEGRA20_CLK_CLK_MAX, 72000000, 1 },
-	{ TEGRA20_CLK_PLL_P_OUT4, TEGRA20_CLK_CLK_MAX, 24000000, 1 },
-	{ TEGRA20_CLK_PLL_C, TEGRA20_CLK_CLK_MAX, 600000000, 1 },
-	{ TEGRA20_CLK_PLL_C_OUT1, TEGRA20_CLK_CLK_MAX, 120000000, 1 },
-	{ TEGRA20_CLK_SCLK, TEGRA20_CLK_PLL_C_OUT1, 0, 1 },
-	{ TEGRA20_CLK_HCLK, TEGRA20_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA20_CLK_PCLK, TEGRA20_CLK_CLK_MAX, 60000000, 1 },
-	{ TEGRA20_CLK_CSITE, TEGRA20_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA20_CLK_EMC, TEGRA20_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA20_CLK_CCLK, TEGRA20_CLK_CLK_MAX, 0, 1 },
+	{ TEGRA20_CLK_PLL_P, TEGRA20_CLK_CLK_MAX, 216000000, 0 },
+	{ TEGRA20_CLK_PLL_P_OUT1, TEGRA20_CLK_CLK_MAX, 28800000, 0 },
+	{ TEGRA20_CLK_PLL_P_OUT2, TEGRA20_CLK_CLK_MAX, 48000000, 0 },
+	{ TEGRA20_CLK_PLL_P_OUT3, TEGRA20_CLK_CLK_MAX, 72000000, 0 },
+	{ TEGRA20_CLK_PLL_P_OUT4, TEGRA20_CLK_CLK_MAX, 24000000, 0 },
+	{ TEGRA20_CLK_PLL_C, TEGRA20_CLK_CLK_MAX, 600000000, 0 },
+	{ TEGRA20_CLK_PLL_C_OUT1, TEGRA20_CLK_CLK_MAX, 120000000, 0 },
+	{ TEGRA20_CLK_SCLK, TEGRA20_CLK_PLL_C_OUT1, 0, 0 },
+	{ TEGRA20_CLK_PCLK, TEGRA20_CLK_CLK_MAX, 60000000, 0 },
 	{ TEGRA20_CLK_UARTA, TEGRA20_CLK_PLL_P, 0, 0 },
 	{ TEGRA20_CLK_UARTB, TEGRA20_CLK_PLL_P, 0, 0 },
 	{ TEGRA20_CLK_UARTC, TEGRA20_CLK_PLL_P, 0, 0 },
 	{ TEGRA20_CLK_UARTD, TEGRA20_CLK_PLL_P, 0, 0 },
 	{ TEGRA20_CLK_UARTE, TEGRA20_CLK_PLL_P, 0, 0 },
-	{ TEGRA20_CLK_PLL_A, TEGRA20_CLK_CLK_MAX, 56448000, 1 },
-	{ TEGRA20_CLK_PLL_A_OUT0, TEGRA20_CLK_CLK_MAX, 11289600, 1 },
-	{ TEGRA20_CLK_CDEV1, TEGRA20_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA20_CLK_BLINK, TEGRA20_CLK_CLK_MAX, 32768, 1 },
+	{ TEGRA20_CLK_PLL_A, TEGRA20_CLK_CLK_MAX, 56448000, 0 },
+	{ TEGRA20_CLK_PLL_A_OUT0, TEGRA20_CLK_CLK_MAX, 11289600, 0 },
+	{ TEGRA20_CLK_BLINK, TEGRA20_CLK_CLK_MAX, 32768, 0 },
 	{ TEGRA20_CLK_I2S1, TEGRA20_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA20_CLK_I2S2, TEGRA20_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA20_CLK_SDMMC1, TEGRA20_CLK_PLL_P, 48000000, 0 },
-- 
1.9.1

[PATCH v2 04/11] clk: tegra30: Mark certain clks as critical

[Rhyland Klein]

Mark the required clks as critical so the core will enable them during
registration and therefore they will stay on.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk-tegra30.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 9396f4930da7..fc91460ab892 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -948,8 +948,8 @@ static void __init tegra30_pll_init(void)
 
 	/* PLLM */
 	clk = tegra_clk_register_pll("pll_m", "pll_ref", clk_base, pmc_base,
-			    CLK_IGNORE_UNUSED | CLK_SET_RATE_GATE,
-			    &pll_m_params, NULL);
+			    CLK_IGNORE_UNUSED | CLK_SET_RATE_GATE |
+			    CLK_IS_CRITICAL, &pll_m_params, NULL);
 	clks[TEGRA30_CLK_PLL_M] = clk;
 
 	/* PLLM_OUT1 */
@@ -1104,7 +1104,8 @@ static void __init tegra30_super_clk_init(void)
 
 	/* twd */
 	clk = clk_register_fixed_factor(NULL, "twd", "cclk_g",
-					CLK_SET_RATE_PARENT, 1, 2);
+					CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+					1, 2);
 	clks[TEGRA30_CLK_TWD] = clk;
 
 	tegra_super_clk_gen4_init(clk_base, pmc_base, tegra30_clks, NULL);
@@ -1164,11 +1165,12 @@ static void __init tegra30_periph_clk_init(void)
 	/* emc */
 	clk = clk_register_mux(NULL, "emc_mux", mux_pllmcp_clkm,
 			       ARRAY_SIZE(mux_pllmcp_clkm),
-			       CLK_SET_RATE_NO_REPARENT,
+			       CLK_SET_RATE_NO_REPARENT | CLK_IS_CRITICAL,
 			       clk_base + CLK_SOURCE_EMC,
 			       30, 2, 0, &emc_lock);
-	clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base, 0,
-				    57, periph_clk_enb_refcnt);
+	clk = tegra_clk_register_periph_gate("emc", "emc_mux", 0, clk_base,
+				    CLK_IS_CRITICAL, 57,
+				    periph_clk_enb_refcnt);
 	clks[TEGRA30_CLK_EMC] = clk;
 
 	clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
-- 
1.9.1

[PATCH v2 05/11] clk: tegra30: clean up init_table

[Rhyland Klein]

Remove entries from the init_table where the clks are now defined
as CRITICAL clks, if we were only enabling them in the init_table.

Remove the flag to signal to enable CRITICAL clks if they are still
needed in the init_table to set other properties.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk-tegra30.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index fc91460ab892..8ae18b63568e 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1349,12 +1349,8 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA30_CLK_UARTC, TEGRA30_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA30_CLK_UARTD, TEGRA30_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA30_CLK_UARTE, TEGRA30_CLK_PLL_P, 408000000, 0 },
-	{ TEGRA30_CLK_PLL_A, TEGRA30_CLK_CLK_MAX, 564480000, 1 },
-	{ TEGRA30_CLK_PLL_A_OUT0, TEGRA30_CLK_CLK_MAX, 11289600, 1 },
-	{ TEGRA30_CLK_EXTERN1, TEGRA30_CLK_PLL_A_OUT0, 0, 1 },
-	{ TEGRA30_CLK_CLK_OUT_1_MUX, TEGRA30_CLK_EXTERN1, 0, 0 },
-	{ TEGRA30_CLK_CLK_OUT_1, TEGRA30_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA30_CLK_BLINK, TEGRA30_CLK_CLK_MAX, 0, 1 },
+	{ TEGRA30_CLK_PLL_A, TEGRA30_CLK_CLK_MAX, 564480000, 0 },
+	{ TEGRA30_CLK_PLL_A_OUT0, TEGRA30_CLK_CLK_MAX, 11289600, 0 },
 	{ TEGRA30_CLK_I2S0, TEGRA30_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA30_CLK_I2S1, TEGRA30_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA30_CLK_I2S2, TEGRA30_CLK_PLL_A_OUT0, 11289600, 0 },
@@ -1363,11 +1359,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA30_CLK_SDMMC1, TEGRA30_CLK_PLL_P, 48000000, 0 },
 	{ TEGRA30_CLK_SDMMC2, TEGRA30_CLK_PLL_P, 48000000, 0 },
 	{ TEGRA30_CLK_SDMMC3, TEGRA30_CLK_PLL_P, 48000000, 0 },
-	{ TEGRA30_CLK_PLL_M, TEGRA30_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA30_CLK_PCLK, TEGRA30_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA30_CLK_CSITE, TEGRA30_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA30_CLK_EMC, TEGRA30_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA30_CLK_MSELECT, TEGRA30_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA30_CLK_SBC1, TEGRA30_CLK_PLL_P, 100000000, 0 },
 	{ TEGRA30_CLK_SBC2, TEGRA30_CLK_PLL_P, 100000000, 0 },
 	{ TEGRA30_CLK_SBC3, TEGRA30_CLK_PLL_P, 100000000, 0 },
@@ -1378,7 +1369,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA30_CLK_HOST1X, TEGRA30_CLK_PLL_C, 150000000, 0 },
 	{ TEGRA30_CLK_DISP1, TEGRA30_CLK_PLL_P, 600000000, 0 },
 	{ TEGRA30_CLK_DISP2, TEGRA30_CLK_PLL_P, 600000000, 0 },
-	{ TEGRA30_CLK_TWD, TEGRA30_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA30_CLK_GR2D, TEGRA30_CLK_PLL_C, 300000000, 0 },
 	{ TEGRA30_CLK_GR3D, TEGRA30_CLK_PLL_C, 300000000, 0 },
 	{ TEGRA30_CLK_GR3D2, TEGRA30_CLK_PLL_C, 300000000, 0 },
-- 
1.9.1

[PATCH v2 06/11] clk: tegra114: clean up init_table

[Rhyland Klein]

Remove entries from the init_table where the clks are now defined
as CRITICAL clks, if we were only enabling them in the init_table.

Remove the flag to signal to enable CRITICAL clks if they are still
needed in the init_table to set other properties.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk-tegra114.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index b78054fac0a8..29fc8c2e4359 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -1318,19 +1318,16 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA114_CLK_UARTB, TEGRA114_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA114_CLK_UARTC, TEGRA114_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA114_CLK_UARTD, TEGRA114_CLK_PLL_P, 408000000, 0 },
-	{ TEGRA114_CLK_PLL_A, TEGRA114_CLK_CLK_MAX, 564480000, 1 },
-	{ TEGRA114_CLK_PLL_A_OUT0, TEGRA114_CLK_CLK_MAX, 11289600, 1 },
-	{ TEGRA114_CLK_EXTERN1, TEGRA114_CLK_PLL_A_OUT0, 0, 1 },
-	{ TEGRA114_CLK_CLK_OUT_1_MUX, TEGRA114_CLK_EXTERN1, 0, 1 },
-	{ TEGRA114_CLK_CLK_OUT_1, TEGRA114_CLK_CLK_MAX, 0, 1 },
+	{ TEGRA114_CLK_PLL_A, TEGRA114_CLK_CLK_MAX, 564480000, 0 },
+	{ TEGRA114_CLK_PLL_A_OUT0, TEGRA114_CLK_CLK_MAX, 11289600, 0 },
 	{ TEGRA114_CLK_I2S0, TEGRA114_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA114_CLK_I2S1, TEGRA114_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA114_CLK_I2S2, TEGRA114_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA114_CLK_I2S3, TEGRA114_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA114_CLK_I2S4, TEGRA114_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA114_CLK_HOST1X, TEGRA114_CLK_PLL_P, 136000000, 0 },
-	{ TEGRA114_CLK_DFLL_SOC, TEGRA114_CLK_PLL_P, 51000000, 1 },
-	{ TEGRA114_CLK_DFLL_REF, TEGRA114_CLK_PLL_P, 51000000, 1 },
+	{ TEGRA114_CLK_DFLL_SOC, TEGRA114_CLK_PLL_P, 51000000, 0 },
+	{ TEGRA114_CLK_DFLL_REF, TEGRA114_CLK_PLL_P, 51000000, 0 },
 	{ TEGRA114_CLK_DISP1, TEGRA114_CLK_PLL_P, 0, 0 },
 	{ TEGRA114_CLK_DISP2, TEGRA114_CLK_PLL_P, 0, 0 },
 	{ TEGRA114_CLK_GR2D, TEGRA114_CLK_PLL_C2, 300000000, 0 },
-- 
1.9.1

[PATCH v2 07/11] clk: tegra124: clean up init_table

[Rhyland Klein]

Remove entries from the init_table where the clks are now defined
as CRITICAL clks, if we were only enabling them in the init_table.

Remove the flag to signal to enable CRITICAL clks if they are still
needed in the init_table to set other properties.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk-tegra124.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index f4fbbf16a056..ab5437da63fb 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -1409,26 +1409,23 @@ static struct tegra_clk_init_table common_init_table[] __initdata = {
 	{ TEGRA124_CLK_UARTB, TEGRA124_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA124_CLK_UARTC, TEGRA124_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA124_CLK_UARTD, TEGRA124_CLK_PLL_P, 408000000, 0 },
-	{ TEGRA124_CLK_PLL_A, TEGRA124_CLK_CLK_MAX, 564480000, 1 },
-	{ TEGRA124_CLK_PLL_A_OUT0, TEGRA124_CLK_CLK_MAX, 11289600, 1 },
-	{ TEGRA124_CLK_EXTERN1, TEGRA124_CLK_PLL_A_OUT0, 0, 1 },
-	{ TEGRA124_CLK_CLK_OUT_1_MUX, TEGRA124_CLK_EXTERN1, 0, 1 },
-	{ TEGRA124_CLK_CLK_OUT_1, TEGRA124_CLK_CLK_MAX, 0, 1 },
+	{ TEGRA124_CLK_PLL_A, TEGRA124_CLK_CLK_MAX, 564480000, 0 },
+	{ TEGRA124_CLK_PLL_A_OUT0, TEGRA124_CLK_CLK_MAX, 11289600, 0 },
 	{ TEGRA124_CLK_I2S0, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA124_CLK_I2S1, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA124_CLK_I2S2, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA124_CLK_I2S3, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA124_CLK_I2S4, TEGRA124_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA124_CLK_VDE, TEGRA124_CLK_PLL_P, 0, 0 },
-	{ TEGRA124_CLK_HOST1X, TEGRA124_CLK_PLL_P, 136000000, 1 },
+	{ TEGRA124_CLK_HOST1X, TEGRA124_CLK_PLL_P, 136000000, 0 },
 	{ TEGRA124_CLK_DSIALP, TEGRA124_CLK_PLL_P, 68000000, 0 },
 	{ TEGRA124_CLK_DSIBLP, TEGRA124_CLK_PLL_P, 68000000, 0 },
-	{ TEGRA124_CLK_SCLK, TEGRA124_CLK_PLL_P_OUT2, 102000000, 1 },
-	{ TEGRA124_CLK_DFLL_SOC, TEGRA124_CLK_PLL_P, 51000000, 1 },
-	{ TEGRA124_CLK_DFLL_REF, TEGRA124_CLK_PLL_P, 51000000, 1 },
+	{ TEGRA124_CLK_SCLK, TEGRA124_CLK_PLL_P_OUT2, 102000000, 0 },
+	{ TEGRA124_CLK_DFLL_SOC, TEGRA124_CLK_PLL_P, 51000000, 0 },
+	{ TEGRA124_CLK_DFLL_REF, TEGRA124_CLK_PLL_P, 51000000, 0 },
 	{ TEGRA124_CLK_PLL_C, TEGRA124_CLK_CLK_MAX, 768000000, 0 },
 	{ TEGRA124_CLK_PLL_C_OUT1, TEGRA124_CLK_CLK_MAX, 100000000, 0 },
-	{ TEGRA124_CLK_SBC4, TEGRA124_CLK_PLL_P, 12000000, 1 },
+	{ TEGRA124_CLK_SBC4, TEGRA124_CLK_PLL_P, 12000000, 0 },
 	{ TEGRA124_CLK_TSEC, TEGRA124_CLK_PLL_C3, 0, 0 },
 	{ TEGRA124_CLK_MSENC, TEGRA124_CLK_PLL_C3, 0, 0 },
 	{ TEGRA124_CLK_PLL_RE_VCO, TEGRA124_CLK_CLK_MAX, 672000000, 0 },
@@ -1439,8 +1436,6 @@ static struct tegra_clk_init_table common_init_table[] __initdata = {
 	{ TEGRA124_CLK_XUSB_HOST_SRC, TEGRA124_CLK_PLL_RE_OUT, 112000000, 0 },
 	{ TEGRA124_CLK_SATA, TEGRA124_CLK_PLL_P, 104000000, 0 },
 	{ TEGRA124_CLK_SATA_OOB, TEGRA124_CLK_PLL_P, 204000000, 0 },
-	{ TEGRA124_CLK_MSELECT, TEGRA124_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA124_CLK_CSITE, TEGRA124_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA124_CLK_TSENSOR, TEGRA124_CLK_CLK_M, 400000, 0 },
 	/* must be the last entry */
 	{ TEGRA124_CLK_CLK_MAX, TEGRA124_CLK_CLK_MAX, 0, 0 },
-- 
1.9.1

[PATCH v2 08/11] clk: tegra210: Mark required clks as CRITICAL

[Rhyland Klein]

Mark clks that are required to be on as CRITICAL clks.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk-tegra210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index b8551813ec43..156dc8ec6bf6 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -2655,7 +2655,7 @@ static void __init tegra210_pll_init(void __iomem *clk_base,
 
 	/* PLLRE */
 	clk = tegra_clk_register_pllre_tegra210("pll_re_vco", "pll_ref",
-						clk_base, pmc, 0,
+						clk_base, pmc, CLK_IS_CRITICAL,
 						&pll_re_vco_params,
 						&pll_re_lock, pll_ref_freq);
 	clk_register_clkdev(clk, "pll_re_vco", NULL);
-- 
1.9.1

[PATCH v2 09/11] clk: tegra210: clean up init_table

[Rhyland Klein]

Remove entries from the init_table where the clks are now defined
as CRITICAL clks, if we were only enabling them in the init_table.

Remove the flag to signal to enable CRITICAL clks if they are still
needed in the init_table to set other properties.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk-tegra210.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 156dc8ec6bf6..f6d9d890502e 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -2794,24 +2794,20 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA210_CLK_UARTB, TEGRA210_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA210_CLK_UARTC, TEGRA210_CLK_PLL_P, 408000000, 0 },
 	{ TEGRA210_CLK_UARTD, TEGRA210_CLK_PLL_P, 408000000, 0 },
-	{ TEGRA210_CLK_PLL_A, TEGRA210_CLK_CLK_MAX, 564480000, 1 },
-	{ TEGRA210_CLK_PLL_A_OUT0, TEGRA210_CLK_CLK_MAX, 11289600, 1 },
-	{ TEGRA210_CLK_EXTERN1, TEGRA210_CLK_PLL_A_OUT0, 0, 1 },
-	{ TEGRA210_CLK_CLK_OUT_1_MUX, TEGRA210_CLK_EXTERN1, 0, 1 },
-	{ TEGRA210_CLK_CLK_OUT_1, TEGRA210_CLK_CLK_MAX, 0, 1 },
+	{ TEGRA210_CLK_PLL_A, TEGRA210_CLK_CLK_MAX, 564480000, 0 },
+	{ TEGRA210_CLK_PLL_A_OUT0, TEGRA210_CLK_CLK_MAX, 11289600, 0 },
 	{ TEGRA210_CLK_I2S0, TEGRA210_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA210_CLK_I2S1, TEGRA210_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA210_CLK_I2S2, TEGRA210_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA210_CLK_I2S3, TEGRA210_CLK_PLL_A_OUT0, 11289600, 0 },
 	{ TEGRA210_CLK_I2S4, TEGRA210_CLK_PLL_A_OUT0, 11289600, 0 },
-	{ TEGRA210_CLK_HOST1X, TEGRA210_CLK_PLL_P, 136000000, 1 },
-	{ TEGRA210_CLK_SCLK_MUX, TEGRA210_CLK_PLL_P, 0, 1 },
-	{ TEGRA210_CLK_SCLK, TEGRA210_CLK_CLK_MAX, 102000000, 1 },
-	{ TEGRA210_CLK_DFLL_SOC, TEGRA210_CLK_PLL_P, 51000000, 1 },
-	{ TEGRA210_CLK_DFLL_REF, TEGRA210_CLK_PLL_P, 51000000, 1 },
-	{ TEGRA210_CLK_SBC4, TEGRA210_CLK_PLL_P, 12000000, 1 },
-	{ TEGRA210_CLK_PLL_RE_VCO, TEGRA210_CLK_CLK_MAX, 672000000, 1 },
-	{ TEGRA210_CLK_XUSB_GATE, TEGRA210_CLK_CLK_MAX, 0, 1 },
+	{ TEGRA210_CLK_HOST1X, TEGRA210_CLK_PLL_P, 136000000, 0 },
+	{ TEGRA210_CLK_SCLK_MUX, TEGRA210_CLK_PLL_P, 0, 0 },
+	{ TEGRA210_CLK_SCLK, TEGRA210_CLK_CLK_MAX, 102000000, 0 },
+	{ TEGRA210_CLK_DFLL_SOC, TEGRA210_CLK_PLL_P, 51000000, 0 },
+	{ TEGRA210_CLK_DFLL_REF, TEGRA210_CLK_PLL_P, 51000000, 0 },
+	{ TEGRA210_CLK_SBC4, TEGRA210_CLK_PLL_P, 12000000, 0 },
+	{ TEGRA210_CLK_PLL_RE_VCO, TEGRA210_CLK_CLK_MAX, 672000000, 0 },
 	{ TEGRA210_CLK_XUSB_SS_SRC, TEGRA210_CLK_PLL_U_480M, 120000000, 0 },
 	{ TEGRA210_CLK_XUSB_FS_SRC, TEGRA210_CLK_PLL_U_48M, 48000000, 0 },
 	{ TEGRA210_CLK_XUSB_HS_SRC, TEGRA210_CLK_XUSB_SS_SRC, 120000000, 0 },
@@ -2821,9 +2817,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA210_CLK_XUSB_DEV_SRC, TEGRA210_CLK_PLL_P_OUT_XUSB, 102000000, 0 },
 	{ TEGRA210_CLK_SATA, TEGRA210_CLK_PLL_P, 104000000, 0 },
 	{ TEGRA210_CLK_SATA_OOB, TEGRA210_CLK_PLL_P, 204000000, 0 },
-	{ TEGRA210_CLK_EMC, TEGRA210_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA210_CLK_MSELECT, TEGRA210_CLK_CLK_MAX, 0, 1 },
-	{ TEGRA210_CLK_CSITE, TEGRA210_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA210_CLK_TSENSOR, TEGRA210_CLK_CLK_M, 400000, 0 },
 	{ TEGRA210_CLK_I2C1, TEGRA210_CLK_PLL_P, 0, 0 },
 	{ TEGRA210_CLK_I2C2, TEGRA210_CLK_PLL_P, 0, 0 },
@@ -2833,7 +2826,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA210_CLK_I2C6, TEGRA210_CLK_PLL_P, 0, 0 },
 	{ TEGRA210_CLK_PLL_DP, TEGRA210_CLK_CLK_MAX, 270000000, 0 },
 	{ TEGRA210_CLK_SOC_THERM, TEGRA210_CLK_PLL_P, 51000000, 0 },
-	{ TEGRA210_CLK_CCLK_G, TEGRA210_CLK_CLK_MAX, 0, 1 },
 	/* This MUST be the last entry. */
 	{ TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 },
 };
-- 
1.9.1

[PATCH v2 10/11] clk: Show CRITICAL clks in clk_summary output

[Rhyland Klein]

Add a '^' character to the beginning of clk entries that are for
CRITICAL clks.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/clk.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 874c7dd8ef66..22dd0ca1e491 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1948,8 +1948,9 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c,
 	if (!c)
 		return;
 
-	seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n",
-		   level * 3 + 1, "",
+	seq_printf(s, "%s%*s%-*s %11d %12d %11lu %10lu %-3d\n",
+		   (c->flags & CLK_IS_CRITICAL ? "^" : ""),
+		   level * 3 + (c->flags & CLK_IS_CRITICAL ? 0 : 1), "",
 		   30 - level * 3, c->name,
 		   c->enable_count, c->prepare_count, clk_core_get_rate(c),
 		   clk_core_get_accuracy(c), clk_core_get_phase(c));
@@ -1985,6 +1986,8 @@ static int clk_summary_show(struct seq_file *s, void *data)
 
 	clk_prepare_unlock();
 
+	seq_puts(s, "clocks starting with ^ are marked as CRITICAL\n");
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH v2 11/11] clk: tegra: WARN if clk in the init_table has enable

[Rhyland Klein]

Enabling clocks through the init_table mechanism is deprecated. Clocks
that need to be enabled early and stay on should be marked as CRITICAL.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/clk/tegra/clk.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
index b2cdd9a235f4..898fe922c742 100644
--- a/drivers/clk/tegra/clk.c
+++ b/drivers/clk/tegra/clk.c
@@ -262,6 +262,11 @@ void __init tegra_init_from_table(struct tegra_clk_init_table *tbl,
 				WARN_ON(1);
 			}
 
+		/* Using the init_table to enable clks at boot is
+		 * deprecated. Clks that need to be enabled through early
+		 * boot, they should be marked as CLK_IS_CRITICAL
+		 */
+		WARN_ON_ONCE(tbl->state);
 		if (tbl->state)
 			if (clk_prepare_enable(clk)) {
 				pr_err("%s: Failed to enable %s\n", __func__,
-- 
1.9.1

Re: [PATCH v2 00/11] Switch to using CRITICAL clks for Tegra

[Rhyland Klein]

On 5/27/2016 4:38 PM, Rhyland Klein wrote:
> Switch to defining clks that need to be on as CRITICAL rather than
> using the init_tables defined to enable clks. Some of these may be
> able to be converted to HAND_OFF clks, when that is supported.
> 
> I added a patch to mark CRITICAL clks in the clk_summary table, figuring
> it would be handy to be able to easily tell what clks are CRITICAL
> through the table.
> 
> I added a warning for using the init_table to enable clks. I considered
> removing it entirely, but I thought phasing it out might be better.
> 
> Rhyland Klein (11):
>   clk: tegra: Switch to using critical clks
>   clk: tegra20: Mark required clks as CRITICAL
>   clk: tegra20: clean up init_table
>   clk: tegra30: Mark certain clks as critical
>   clk: tegra30: clean up init_table
>   clk: tegra114: clean up init_table
>   clk: tegra124: clean up init_table
>   clk: tegra210: Mark required clks as CRITICAL
>   clk: tegra210: clean up init_table
>   clk: Show CRITICAL clks in clk_summary output
>   clk: tegra: WARN if clk in the init_table has enable
> 
>  drivers/clk/clk.c                        |  7 ++++--
>  drivers/clk/tegra/clk-tegra-periph.c     | 21 ++++++++++------
>  drivers/clk/tegra/clk-tegra-super-gen4.c | 12 ++++++----
>  drivers/clk/tegra/clk-tegra114.c         | 11 ++++-----
>  drivers/clk/tegra/clk-tegra124.c         | 19 ++++++---------
>  drivers/clk/tegra/clk-tegra20.c          | 41 +++++++++++++++-----------------
>  drivers/clk/tegra/clk-tegra210.c         | 28 ++++++++--------------
>  drivers/clk/tegra/clk-tegra30.c          | 28 ++++++++--------------
>  drivers/clk/tegra/clk.c                  |  5 ++++
>  9 files changed, 81 insertions(+), 91 deletions(-)
> 

Has anyone had a change to take a look at these patches yet?

-rhyland

-- 
nvpublic

Re: [PATCH v2 01/11] clk: tegra: Switch to using critical clks

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 8719 bytes --]

On Fri, May 27, 2016 at 04:38:04PM -0400, Rhyland Klein wrote:
> Mark some of the required-to-be-enabled clks as critical clks. These
> need to be kept on through the disabling of unused clks during init.
> They may not get any reference before or after init, but are required
> to be on, therefore let the core enable them.
> 
> Signed-off-by: Rhyland Klein <rklein@nvidia.com>
> ---
> v2:
>  - Remove mention of HAND_OFF clks as they are not supported yet.
> 
>  drivers/clk/tegra/clk-tegra-periph.c     | 21 ++++++++++++++-------
>  drivers/clk/tegra/clk-tegra-super-gen4.c | 12 +++++++-----
>  2 files changed, 21 insertions(+), 12 deletions(-)

I have some difficulty to follow why some of these clocks are critical.
It might help if the commit message mentioned why each of these need to
remain enabled forever, even if never used.

Also, it's fairly unlikely that pll_p for example would ever get
disabled because a bunch of others are derived from it. I'm also not
quite convinced yet that it really is critical. What does it drive which
isn't claimed by any drivers?

A few more comments below.

> diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
> index 29d04c663abf..9365770bcaa5 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -162,6 +162,13 @@
>  			_clk_num, _gate_flags, _clk_id, _parents##_idx, 0,\
>  			NULL)
>  
> +#define MUX8_FLAGS(_name, _parents, _offset,\
> +			     _clk_num, _gate_flags, _clk_id, _flags)	\
> +	TEGRA_INIT_DATA_TABLE(_name, NULL, NULL, _parents, _offset,\
> +			29, MASK(8), 0, 0, 8, 1, TEGRA_DIVIDER_ROUND_UP,\
> +			_clk_num, _gate_flags, _clk_id, _parents##_idx, _flags,\
> +			NULL)
> +
>  #define MUX8_NOGATE_LOCK(_name, _parents, _offset, _clk_id, _lock)	\
>  	TEGRA_INIT_DATA_TABLE(_name, NULL, NULL, _parents, _offset,	\
>  			      29, MASK(3), 0, 0, 8, 1, TEGRA_DIVIDER_ROUND_UP,\
> @@ -651,7 +658,7 @@ static struct tegra_periph_init_data periph_clks[] = {
>  	INT8("3d", mux_pllm_pllc2_c_c3_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d_8),
>  	INT8("vic03", mux_pllm_pllc_pllp_plla_pllc2_c3_clkm, CLK_SOURCE_VIC03, 178, 0, tegra_clk_vic03),
>  	INT8("vic03", mux_pllc_pllp_plla1_pllc2_c3_clkm, CLK_SOURCE_VIC03, 178, 0, tegra_clk_vic03_8),
> -	INT_FLAGS("mselect", mux_pllp_clkm, CLK_SOURCE_MSELECT, 99, 0, tegra_clk_mselect, CLK_IGNORE_UNUSED),
> +	INT_FLAGS("mselect", mux_pllp_clkm, CLK_SOURCE_MSELECT, 99, 0, tegra_clk_mselect, CLK_IGNORE_UNUSED | CLK_IS_CRITICAL),

Doesn't CLK_IS_CRITICAL always trump CLK_IGNORE_UNUSED anyway? Why do we
need to keep both?

>  	MUX("i2s0", mux_pllaout0_audio0_2x_pllp_clkm, CLK_SOURCE_I2S0, 30, TEGRA_PERIPH_ON_APB, tegra_clk_i2s0),
>  	MUX("i2s1", mux_pllaout0_audio1_2x_pllp_clkm, CLK_SOURCE_I2S1, 11, TEGRA_PERIPH_ON_APB, tegra_clk_i2s1),
>  	MUX("i2s2", mux_pllaout0_audio2_2x_pllp_clkm, CLK_SOURCE_I2S2, 18, TEGRA_PERIPH_ON_APB, tegra_clk_i2s2),
> @@ -691,8 +698,8 @@ static struct tegra_periph_init_data periph_clks[] = {
>  	MUX("dsiblp", mux_pllp_pllc_clkm, CLK_SOURCE_DSIBLP, 148, 0, tegra_clk_dsiblp),
>  	MUX("tsensor", mux_pllp_pllc_clkm_clk32, CLK_SOURCE_TSENSOR, 100, TEGRA_PERIPH_ON_APB, tegra_clk_tsensor),
>  	MUX("actmon", mux_pllp_pllc_clk32_clkm, CLK_SOURCE_ACTMON, 119, 0, tegra_clk_actmon),
> -	MUX("dfll_ref", mux_pllp_clkm, CLK_SOURCE_DFLL_REF, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_ref),
> -	MUX("dfll_soc", mux_pllp_clkm, CLK_SOURCE_DFLL_SOC, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_soc),
> +	MUX_FLAGS("dfll_ref", mux_pllp_clkm, CLK_SOURCE_DFLL_REF, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_ref, CLK_IS_CRITICAL),
> +	MUX_FLAGS("dfll_soc", mux_pllp_clkm, CLK_SOURCE_DFLL_SOC, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_soc, CLK_IS_CRITICAL),

Aren't these used by the CPU frequency scaling code? Do they have to
remain on if we don't enable CPU frequency scaling?

>  	MUX("i2cslow", mux_pllp_pllc_clk32_clkm, CLK_SOURCE_I2CSLOW, 81, TEGRA_PERIPH_ON_APB, tegra_clk_i2cslow),
>  	MUX("sbc1", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SBC1, 41, TEGRA_PERIPH_ON_APB, tegra_clk_sbc1),
>  	MUX("sbc2", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SBC2, 44, TEGRA_PERIPH_ON_APB, tegra_clk_sbc2),
> @@ -730,7 +737,7 @@ static struct tegra_periph_init_data periph_clks[] = {
>  	MUX8("ndflash", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_NDFLASH, 13, TEGRA_PERIPH_ON_APB, tegra_clk_ndflash_8),
>  	MUX8("ndspeed", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_NDSPEED, 80, TEGRA_PERIPH_ON_APB, tegra_clk_ndspeed_8),
>  	MUX8("hdmi", mux_pllp_pllm_plld_plla_pllc_plld2_clkm, CLK_SOURCE_HDMI, 51, 0, tegra_clk_hdmi),
> -	MUX8("extern1", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN1, 120, 0, tegra_clk_extern1),
> +	MUX8_FLAGS("extern1", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN1, 120, 0, tegra_clk_extern1, CLK_IS_CRITICAL),

Would you mind refreshing my memory about what this is and why it is
critical?

> @@ -825,7 +832,7 @@ static struct tegra_periph_init_data gate_clks[] = {
>  	GATE("pllg_ref", "pll_ref", 189, 0, tegra_clk_pll_g_ref, 0),
>  	GATE("hsic_trk", "usb2_hsic_trk", 209, TEGRA_PERIPH_NO_RESET, tegra_clk_hsic_trk, 0),
>  	GATE("usb2_trk", "usb2_hsic_trk", 210, TEGRA_PERIPH_NO_RESET, tegra_clk_usb2_trk, 0),
> -	GATE("xusb_gate", "osc", 143, 0, tegra_clk_xusb_gate, 0),
> +	GATE("xusb_gate", "osc", 143, 0, tegra_clk_xusb_gate, CLK_IS_CRITICAL),

Wouldn't this better be handled by the XUSB driver? It certainly seems
very specific to XUSB and I don't see why we would want to keep it
enabled even if XUSB wasn't used.

>  	GATE("pll_p_out_cpu", "pll_p", 223, 0, tegra_clk_pll_p_out_cpu, 0),
>  	GATE("pll_p_out_adsp", "pll_p", 187, 0, tegra_clk_pll_p_out_adsp, 0),
>  	GATE("apb2ape", "clk_m", 107, 0, tegra_clk_apb2ape, 0),
> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
> index 474de0f0c26d..5283138af093 100644
> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
> @@ -116,7 +116,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>  		clk = tegra_clk_register_super_mux("sclk_mux",
>  						gen_info->sclk_parents,
>  						gen_info->num_sclk_parents,
> -						CLK_SET_RATE_PARENT,
> +						CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>  						clk_base + SCLK_BURST_POLICY,
>  						0, 4, 0, 0, NULL);
>  		*dt_clk = clk;
> @@ -137,7 +137,8 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>  			clk = tegra_clk_register_super_mux("sclk",
>  						gen_info->sclk_parents,
>  						gen_info->num_sclk_parents,
> -						CLK_SET_RATE_PARENT,
> +						CLK_SET_RATE_PARENT |
> +						CLK_IS_CRITICAL,
>  						clk_base + SCLK_BURST_POLICY,
>  						0, 4, 0, 0, NULL);
>  			*dt_clk = clk;
> @@ -166,7 +167,8 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>  				   &sysrate_lock);
>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
> +				CLK_IGNORE_UNUSED | CLK_IS_CRITICAL,
> +				clk_base + SYSTEM_CLK_RATE,
>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>  	*dt_clk = clk;
>  }

Same comments as for pll_p.

> @@ -187,14 +189,14 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
>  			clk = tegra_clk_register_super_mux("cclk_g",
>  					gen_info->cclk_g_parents,
>  					gen_info->num_cclk_g_parents,
> -					CLK_SET_RATE_PARENT,
> +					CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>  					clk_base + CCLKG_BURST_POLICY,
>  					0, 4, 8, 0, NULL);
>  		} else {
>  			clk = tegra_clk_register_super_mux("cclk_g",
>  					gen_info->cclk_g_parents,
>  					gen_info->num_cclk_g_parents,
> -					CLK_SET_RATE_PARENT,
> +					CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>  					clk_base + CCLKG_BURST_POLICY,
>  					0, 4, 0, 0, NULL);
>  		}

I thought this was driving the G cluster, what if we switched to the LP
cluster, wouldn't that mean we can disable cclk_g?

I've only briefly scanned through the remainder of the series, and I
suspect that many of the below changes are simply moving the data out of
the init tables, so not effectively changing anything, but I'd still
like to get this documented a little better. As it is, it's completely
non-obvious why some of these clocks have the CLK_IS_CRITICAL flag while
others don't. It's almost as obfuscated in the current init tables, but
them being in those tables indicates that they're somehow special,
whereas that's much more difficult to see with the conversion to the
CLK_IS_CRITICAL flag.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 10/11] clk: Show CRITICAL clks in clk_summary output

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

On Fri, May 27, 2016 at 04:38:13PM -0400, Rhyland Klein wrote:
> Add a '^' character to the beginning of clk entries that are for
> CRITICAL clks.
> 
> Signed-off-by: Rhyland Klein <rklein@nvidia.com>
> ---
>  drivers/clk/clk.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 874c7dd8ef66..22dd0ca1e491 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1948,8 +1948,9 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c,
>  	if (!c)
>  		return;
>  
> -	seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n",
> -		   level * 3 + 1, "",
> +	seq_printf(s, "%s%*s%-*s %11d %12d %11lu %10lu %-3d\n",
> +		   (c->flags & CLK_IS_CRITICAL ? "^" : ""),
> +		   level * 3 + (c->flags & CLK_IS_CRITICAL ? 0 : 1), "",

Maybe output " " instead of "" for CLK_IS_CRITICAL, that way you can
omit the second conditional.

I wonder if it might be easier to read if this flag was at the end of
the line. There's also the fact that someone may have written a script
that expects the clock name as the first word on the line and may get
confused by this change. If you put it at the very end of the line the
likelihood of upsetting scripts will be reduced.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 10/11] clk: Show CRITICAL clks in clk_summary output

[Rhyland Klein]

On 6/22/2016 8:24 AM, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, May 27, 2016 at 04:38:13PM -0400, Rhyland Klein wrote:
>> Add a '^' character to the beginning of clk entries that are for
>> CRITICAL clks.
>>
>> Signed-off-by: Rhyland Klein <rklein@nvidia.com>
>> ---
>>  drivers/clk/clk.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 874c7dd8ef66..22dd0ca1e491 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -1948,8 +1948,9 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c,
>>  	if (!c)
>>  		return;
>>  
>> -	seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n",
>> -		   level * 3 + 1, "",
>> +	seq_printf(s, "%s%*s%-*s %11d %12d %11lu %10lu %-3d\n",
>> +		   (c->flags & CLK_IS_CRITICAL ? "^" : ""),
>> +		   level * 3 + (c->flags & CLK_IS_CRITICAL ? 0 : 1), "",
> 
> Maybe output " " instead of "" for CLK_IS_CRITICAL, that way you can
> omit the second conditional.
> 
> I wonder if it might be easier to read if this flag was at the end of
> the line. There's also the fact that someone may have written a script
> that expects the clock name as the first word on the line and may get
> confused by this change. If you put it at the very end of the line the
> likelihood of upsetting scripts will be reduced.

Yah we can put the mark at the end of the line. I wasn't sure if there
was a strong motivation to avoid extending the the width of each line,
as sometimes people prefer to try to keep it close to 80 char as
possible. I think right now, it was close to that, but might be a little
over already. I can switch to that though, as it is less likely to break
any automatic parsing scripts.

-rhyland

-- 
nvpublic

Re: [PATCH v2 01/11] clk: tegra: Switch to using critical clks

[Peter De Schrijver]

On Wed, Jun 22, 2016 at 02:16:30PM +0200, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, May 27, 2016 at 04:38:04PM -0400, Rhyland Klein wrote:
> > Mark some of the required-to-be-enabled clks as critical clks. These
> > need to be kept on through the disabling of unused clks during init.
> > They may not get any reference before or after init, but are required
> > to be on, therefore let the core enable them.
> > 
> > Signed-off-by: Rhyland Klein <rklein@nvidia.com>
> > ---
> > v2:
> >  - Remove mention of HAND_OFF clks as they are not supported yet.
> > 
> >  drivers/clk/tegra/clk-tegra-periph.c     | 21 ++++++++++++++-------
> >  drivers/clk/tegra/clk-tegra-super-gen4.c | 12 +++++++-----
> >  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> I have some difficulty to follow why some of these clocks are critical.
> It might help if the commit message mentioned why each of these need to
> remain enabled forever, even if never used.
> 
> Also, it's fairly unlikely that pll_p for example would ever get
> disabled because a bunch of others are derived from it. I'm also not
> quite convinced yet that it really is critical. What does it drive which
> isn't claimed by any drivers?
> 

mselect and sclk are clocked from pll_p, however we don't have drivers for
those. Most other peripherals clocked from pll_p can be clockgated by their
driver which can lead to pll_p being turned off causing the system to hang.

Cheers,

Peter.

Re: [PATCH v2 10/11] clk: Show CRITICAL clks in clk_summary output

[Stephen Boyd]

On 06/22, Rhyland Klein wrote:
> On 6/22/2016 8:24 AM, Thierry Reding wrote:
> > 
> > Maybe output " " instead of "" for CLK_IS_CRITICAL, that way you can
> > omit the second conditional.
> > 
> > I wonder if it might be easier to read if this flag was at the end of
> > the line. There's also the fact that someone may have written a script
> > that expects the clock name as the first word on the line and may get
> > confused by this change. If you put it at the very end of the line the
> > likelihood of upsetting scripts will be reduced.
> 
> Yah we can put the mark at the end of the line. I wasn't sure if there
> was a strong motivation to avoid extending the the width of each line,
> as sometimes people prefer to try to keep it close to 80 char as
> possible. I think right now, it was close to that, but might be a little
> over already. I can switch to that though, as it is less likely to break
> any automatic parsing scripts.
> 

Nak. clk_summary is about taking a snapshot of the system state
for things that may be changing rapidly, like consumers (which
sounds fun to add!), rates, enable/prepare state. Flags are not
changing. If you want to add flag info into some summary then a
script should be able to augment clk_summary info (really should
use the clk_dump in this case though) with whatever flags can be
read through debugfs already.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 10/11] clk: Show CRITICAL clks in clk_summary output

[Rhyland Klein]

On 6/28/2016 1:40 PM, Stephen Boyd wrote:
> On 06/22, Rhyland Klein wrote:
>> On 6/22/2016 8:24 AM, Thierry Reding wrote:
>>>
>>> Maybe output " " instead of "" for CLK_IS_CRITICAL, that way you can
>>> omit the second conditional.
>>>
>>> I wonder if it might be easier to read if this flag was at the end of
>>> the line. There's also the fact that someone may have written a script
>>> that expects the clock name as the first word on the line and may get
>>> confused by this change. If you put it at the very end of the line the
>>> likelihood of upsetting scripts will be reduced.
>>
>> Yah we can put the mark at the end of the line. I wasn't sure if there
>> was a strong motivation to avoid extending the the width of each line,
>> as sometimes people prefer to try to keep it close to 80 char as
>> possible. I think right now, it was close to that, but might be a little
>> over already. I can switch to that though, as it is less likely to break
>> any automatic parsing scripts.
>>
> 
> Nak. clk_summary is about taking a snapshot of the system state
> for things that may be changing rapidly, like consumers (which
> sounds fun to add!), rates, enable/prepare state. Flags are not
> changing. If you want to add flag info into some summary then a
> script should be able to augment clk_summary info (really should
> use the clk_dump in this case though) with whatever flags can be
> read through debugfs already.
> 

That is fine with me. This was more of something I was using locally to
verify things and thought it might be useful in some manner upstream.
However, a script which read the clk_flags from debugfs for each clk
could do the same thing without changing the summary.

-rhyland

-- 
nvpublic

Re: [PATCH v2 01/11] clk: tegra: Switch to using critical clks

[Rhyland Klein]

On 6/22/2016 8:16 AM, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, May 27, 2016 at 04:38:04PM -0400, Rhyland Klein wrote:
>> Mark some of the required-to-be-enabled clks as critical clks. These
>> need to be kept on through the disabling of unused clks during init.
>> They may not get any reference before or after init, but are required
>> to be on, therefore let the core enable them.
>>
>> Signed-off-by: Rhyland Klein <rklein@nvidia.com>
>> ---
>> v2:
>>  - Remove mention of HAND_OFF clks as they are not supported yet.
>>
>>  drivers/clk/tegra/clk-tegra-periph.c     | 21 ++++++++++++++-------
>>  drivers/clk/tegra/clk-tegra-super-gen4.c | 12 +++++++-----
>>  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> I have some difficulty to follow why some of these clocks are critical.
> It might help if the commit message mentioned why each of these need to
> remain enabled forever, even if never used.

Sure, I can add what I found/know for each.

> 
> Also, it's fairly unlikely that pll_p for example would ever get
> disabled because a bunch of others are derived from it. I'm also not
> quite convinced yet that it really is critical. What does it drive which
> isn't claimed by any drivers?
> 

I think Peter already responded to this comment correctly, citing
mselect and sclk as necessary CRITICAL clk children of pll_p.

> A few more comments below.
> 
>> diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
>> index 29d04c663abf..9365770bcaa5 100644
>> --- a/drivers/clk/tegra/clk-tegra-periph.c
>> +++ b/drivers/clk/tegra/clk-tegra-periph.c
>> @@ -162,6 +162,13 @@
>>  			_clk_num, _gate_flags, _clk_id, _parents##_idx, 0,\
>>  			NULL)
>>  
>> +#define MUX8_FLAGS(_name, _parents, _offset,\
>> +			     _clk_num, _gate_flags, _clk_id, _flags)	\
>> +	TEGRA_INIT_DATA_TABLE(_name, NULL, NULL, _parents, _offset,\
>> +			29, MASK(8), 0, 0, 8, 1, TEGRA_DIVIDER_ROUND_UP,\
>> +			_clk_num, _gate_flags, _clk_id, _parents##_idx, _flags,\
>> +			NULL)
>> +
>>  #define MUX8_NOGATE_LOCK(_name, _parents, _offset, _clk_id, _lock)	\
>>  	TEGRA_INIT_DATA_TABLE(_name, NULL, NULL, _parents, _offset,	\
>>  			      29, MASK(3), 0, 0, 8, 1, TEGRA_DIVIDER_ROUND_UP,\
>> @@ -651,7 +658,7 @@ static struct tegra_periph_init_data periph_clks[] = {
>>  	INT8("3d", mux_pllm_pllc2_c_c3_pllp_plla, CLK_SOURCE_3D, 24, 0, tegra_clk_gr3d_8),
>>  	INT8("vic03", mux_pllm_pllc_pllp_plla_pllc2_c3_clkm, CLK_SOURCE_VIC03, 178, 0, tegra_clk_vic03),
>>  	INT8("vic03", mux_pllc_pllp_plla1_pllc2_c3_clkm, CLK_SOURCE_VIC03, 178, 0, tegra_clk_vic03_8),
>> -	INT_FLAGS("mselect", mux_pllp_clkm, CLK_SOURCE_MSELECT, 99, 0, tegra_clk_mselect, CLK_IGNORE_UNUSED),
>> +	INT_FLAGS("mselect", mux_pllp_clkm, CLK_SOURCE_MSELECT, 99, 0, tegra_clk_mselect, CLK_IGNORE_UNUSED | CLK_IS_CRITICAL),
> 
> Doesn't CLK_IS_CRITICAL always trump CLK_IGNORE_UNUSED anyway? Why do we
> need to keep both?

Yep, makes sense to remove CLK_IGNORE_UNUSED.

> 
>>  	MUX("i2s0", mux_pllaout0_audio0_2x_pllp_clkm, CLK_SOURCE_I2S0, 30, TEGRA_PERIPH_ON_APB, tegra_clk_i2s0),
>>  	MUX("i2s1", mux_pllaout0_audio1_2x_pllp_clkm, CLK_SOURCE_I2S1, 11, TEGRA_PERIPH_ON_APB, tegra_clk_i2s1),
>>  	MUX("i2s2", mux_pllaout0_audio2_2x_pllp_clkm, CLK_SOURCE_I2S2, 18, TEGRA_PERIPH_ON_APB, tegra_clk_i2s2),
>> @@ -691,8 +698,8 @@ static struct tegra_periph_init_data periph_clks[] = {
>>  	MUX("dsiblp", mux_pllp_pllc_clkm, CLK_SOURCE_DSIBLP, 148, 0, tegra_clk_dsiblp),
>>  	MUX("tsensor", mux_pllp_pllc_clkm_clk32, CLK_SOURCE_TSENSOR, 100, TEGRA_PERIPH_ON_APB, tegra_clk_tsensor),
>>  	MUX("actmon", mux_pllp_pllc_clk32_clkm, CLK_SOURCE_ACTMON, 119, 0, tegra_clk_actmon),
>> -	MUX("dfll_ref", mux_pllp_clkm, CLK_SOURCE_DFLL_REF, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_ref),
>> -	MUX("dfll_soc", mux_pllp_clkm, CLK_SOURCE_DFLL_SOC, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_soc),
>> +	MUX_FLAGS("dfll_ref", mux_pllp_clkm, CLK_SOURCE_DFLL_REF, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_ref, CLK_IS_CRITICAL),
>> +	MUX_FLAGS("dfll_soc", mux_pllp_clkm, CLK_SOURCE_DFLL_SOC, 155, TEGRA_PERIPH_ON_APB, tegra_clk_dfll_soc, CLK_IS_CRITICAL),
> 
> Aren't these used by the CPU frequency scaling code? Do they have to
> remain on if we don't enable CPU frequency scaling?

They are. They seem to be only required to remain on through boot if
cpufreq is enabled. I am only seeing problems with them on Tegra124.
Right now there is no way to only mark them CRITICAL for T124 (or more
accurately, only enable them if cpufreq is enabled). Thats why I had
them marked CRITICAL.

> 
>>  	MUX("i2cslow", mux_pllp_pllc_clk32_clkm, CLK_SOURCE_I2CSLOW, 81, TEGRA_PERIPH_ON_APB, tegra_clk_i2cslow),
>>  	MUX("sbc1", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SBC1, 41, TEGRA_PERIPH_ON_APB, tegra_clk_sbc1),
>>  	MUX("sbc2", mux_pllp_pllc_pllm_clkm, CLK_SOURCE_SBC2, 44, TEGRA_PERIPH_ON_APB, tegra_clk_sbc2),
>> @@ -730,7 +737,7 @@ static struct tegra_periph_init_data periph_clks[] = {
>>  	MUX8("ndflash", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_NDFLASH, 13, TEGRA_PERIPH_ON_APB, tegra_clk_ndflash_8),
>>  	MUX8("ndspeed", mux_pllp_pllc2_c_c3_pllm_clkm, CLK_SOURCE_NDSPEED, 80, TEGRA_PERIPH_ON_APB, tegra_clk_ndspeed_8),
>>  	MUX8("hdmi", mux_pllp_pllm_plld_plla_pllc_plld2_clkm, CLK_SOURCE_HDMI, 51, 0, tegra_clk_hdmi),
>> -	MUX8("extern1", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN1, 120, 0, tegra_clk_extern1),
>> +	MUX8_FLAGS("extern1", mux_plla_clk32_pllp_clkm_plle, CLK_SOURCE_EXTERN1, 120, 0, tegra_clk_extern1, CLK_IS_CRITICAL),
> 
> Would you mind refreshing my memory about what this is and why it is
> critical?

I think this was carry over from the init table. Removing it, I can
still boot fine on A44 Smaug.

> 
>> @@ -825,7 +832,7 @@ static struct tegra_periph_init_data gate_clks[] = {
>>  	GATE("pllg_ref", "pll_ref", 189, 0, tegra_clk_pll_g_ref, 0),
>>  	GATE("hsic_trk", "usb2_hsic_trk", 209, TEGRA_PERIPH_NO_RESET, tegra_clk_hsic_trk, 0),
>>  	GATE("usb2_trk", "usb2_hsic_trk", 210, TEGRA_PERIPH_NO_RESET, tegra_clk_usb2_trk, 0),
>> -	GATE("xusb_gate", "osc", 143, 0, tegra_clk_xusb_gate, 0),
>> +	GATE("xusb_gate", "osc", 143, 0, tegra_clk_xusb_gate, CLK_IS_CRITICAL),
> 
> Wouldn't this better be handled by the XUSB driver? It certainly seems
> very specific to XUSB and I don't see why we would want to keep it
> enabled even if XUSB wasn't used.

Yes it should be.

> 
>>  	GATE("pll_p_out_cpu", "pll_p", 223, 0, tegra_clk_pll_p_out_cpu, 0),
>>  	GATE("pll_p_out_adsp", "pll_p", 187, 0, tegra_clk_pll_p_out_adsp, 0),
>>  	GATE("apb2ape", "clk_m", 107, 0, tegra_clk_apb2ape, 0),
>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> index 474de0f0c26d..5283138af093 100644
>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> @@ -116,7 +116,7 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>  		clk = tegra_clk_register_super_mux("sclk_mux",
>>  						gen_info->sclk_parents,
>>  						gen_info->num_sclk_parents,
>> -						CLK_SET_RATE_PARENT,
>> +						CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>>  						clk_base + SCLK_BURST_POLICY,
>>  						0, 4, 0, 0, NULL);
>>  		*dt_clk = clk;
>> @@ -137,7 +137,8 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>  			clk = tegra_clk_register_super_mux("sclk",
>>  						gen_info->sclk_parents,
>>  						gen_info->num_sclk_parents,
>> -						CLK_SET_RATE_PARENT,
>> +						CLK_SET_RATE_PARENT |
>> +						CLK_IS_CRITICAL,
>>  						clk_base + SCLK_BURST_POLICY,
>>  						0, 4, 0, 0, NULL);
>>  			*dt_clk = clk;
>> @@ -166,7 +167,8 @@ static void __init tegra_sclk_init(void __iomem *clk_base,
>>  				   clk_base + SYSTEM_CLK_RATE, 0, 2, 0,
>>  				   &sysrate_lock);
>>  	clk = clk_register_gate(NULL, "pclk", "pclk_div", CLK_SET_RATE_PARENT |
>> -				CLK_IGNORE_UNUSED, clk_base + SYSTEM_CLK_RATE,
>> +				CLK_IGNORE_UNUSED | CLK_IS_CRITICAL,
>> +				clk_base + SYSTEM_CLK_RATE,
>>  				3, CLK_GATE_SET_TO_DISABLE, &sysrate_lock);
>>  	*dt_clk = clk;
>>  }
> 
> Same comments as for pll_p.
> 
>> @@ -187,14 +189,14 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
>>  			clk = tegra_clk_register_super_mux("cclk_g",
>>  					gen_info->cclk_g_parents,
>>  					gen_info->num_cclk_g_parents,
>> -					CLK_SET_RATE_PARENT,
>> +					CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>>  					clk_base + CCLKG_BURST_POLICY,
>>  					0, 4, 8, 0, NULL);
>>  		} else {
>>  			clk = tegra_clk_register_super_mux("cclk_g",
>>  					gen_info->cclk_g_parents,
>>  					gen_info->num_cclk_g_parents,
>> -					CLK_SET_RATE_PARENT,
>> +					CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>>  					clk_base + CCLKG_BURST_POLICY,
>>  					0, 4, 0, 0, NULL);
>>  		}
> 
> I thought this was driving the G cluster, what if we switched to the LP
> cluster, wouldn't that mean we can disable cclk_g?

This is a care over of converting init_table enables to CRITICAL CLKS.
Without this being critical, things seem to be booting just fine.

> 
> I've only briefly scanned through the remainder of the series, and I
> suspect that many of the below changes are simply moving the data out of
> the init tables, so not effectively changing anything, but I'd still
> like to get this documented a little better. As it is, it's completely
> non-obvious why some of these clocks have the CLK_IS_CRITICAL flag while
> others don't. It's almost as obfuscated in the current init tables, but
> them being in those tables indicates that they're somehow special,
> whereas that's much more difficult to see with the conversion to the
> CLK_IS_CRITICAL flag.

I went through and verified which clks should be CRITICAL. Apparently I
missed some of the clks which aren't required when I was going though
the ones that we already being enabled through the clk init_table (which
has the same effect, always leaving them enabled).

-rhyland

-- 
nvpublic