Re: [PATCH V6 09/21] clk: tegra: clk-super: Fix to enable PLLP branches to CPU

From: Sowjanya Komatineni
Date: Sun Jul 21 2019 - 23:16:59 EST


On 7/21/19 3:39 PM, Sowjanya Komatineni wrote:

On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
21.07.2019 22:40, Sowjanya Komatineni ÐÐÑÐÑ:
This patch has a fix to enable PLLP branches to CPU before changing
the CPU clusters clock source to PLLP for Gen5 Super clock.

During system suspend entry and exit, CPU source will be switched
to PLLP and this needs PLLP branches to be enabled to CPU prior to
the switch.

On system resume, warmboot code enables PLLP branches to CPU and
powers up the CPU with PLLP clock source.

Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
---
 drivers/clk/tegra/clk-super.c | 11 +++++++++++
 drivers/clk/tegra/clk-tegra-super-gen4.c | 4 ++--
 drivers/clk/tegra/clk.h | 4 ++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
index 39ef31b46df5..d73c587e4853 100644
--- a/drivers/clk/tegra/clk-super.c
+++ b/drivers/clk/tegra/clk-super.c
@@ -28,6 +28,9 @@
 #define super_state_to_src_shift(m, s) ((m->width * s))
 #define super_state_to_src_mask(m) (((1 << m->width) - 1))
 +#define CCLK_SRC_PLLP_OUT0 4
+#define CCLK_SRC_PLLP_OUT4 5
+
 static u8 clk_super_get_parent(struct clk_hw *hw)
 {
ÂÂÂÂÂ struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
@@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw *hw, u8 index)
ÂÂÂÂÂÂÂÂÂ if (index == mux->div2_index)
ÂÂÂÂÂÂÂÂÂÂÂÂÂ index = mux->pllx_index;
ÂÂÂÂÂ }
+
+ÂÂÂ /*
+ÂÂÂÂ * Enable PLLP branches to CPU before selecting PLLP source
+ÂÂÂÂ */
+ÂÂÂ if ((mux->flags & TEGRA_CPU_CLK) &&
+ÂÂÂÂÂÂÂ ((index == CCLK_SRC_PLLP_OUT0) || (index == CCLK_SRC_PLLP_OUT4)))
+ÂÂÂÂÂÂÂ tegra_clk_set_pllp_out_cpu(true);
Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
switching from PLLP?
PLLP may be used for other CPU clusters.

Though to avoid flag and check needed to make sure other CPU is not using before disabling PLLP branch to CPU.

But leaving it enabled shouldn't impact much as clock source mux is after this in design anyway.

But can add as well if its clear that way.

ÂÂÂÂÂ val &= ~((super_state_to_src_mask(mux)) << shift);
ÂÂÂÂÂ val |= (index & (super_state_to_src_mask(mux))) << shift;
 diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
index cdfe7c9697e1..cd208d0eca2a 100644
--- a/drivers/clk/tegra/clk-tegra-super-gen4.c
+++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
@@ -180,7 +180,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ gen_info->num_cclk_g_parents,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ CLK_SET_RATE_PARENT,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ clk_base + CCLKG_BURST_POLICY,
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ 0, 4, 8, 0, NULL);
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ TEGRA_CPU_CLK, 4, 8, 0, NULL);
ÂÂÂÂÂÂÂÂÂ } else {
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clk = tegra_clk_register_super_mux("cclk_g",
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ gen_info->cclk_g_parents,
@@ -201,7 +201,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ gen_info->num_cclk_lp_parents,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ CLK_SET_RATE_PARENT,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ clk_base + CCLKLP_BURST_POLICY,
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ 0, 4, 8, 0, NULL);
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ TEGRA_CPU_CLK, 4, 8, 0, NULL);
ÂÂÂÂÂÂÂÂÂ } else {
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clk = tegra_clk_register_super_mux("cclk_lp",
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ gen_info->cclk_lp_parents,
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index ac6de3a0b91f..c357b49e49b0 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -694,6 +694,9 @@ struct clk *tegra_clk_register_periph_data(void __iomem *clk_base,
ÂÂ * Flags:
ÂÂ * TEGRA_DIVIDER_2 - LP cluster has additional divider. This flag indicates
ÂÂ *ÂÂÂÂ that this is LP cluster clock.
+ * TEGRA_CPU_CLK - This flag indicates this is CPU cluster clock. To use PLLP
+ * for CPU clock source, need to enable PLLP branches to CPU by setting the
+ * additional bit PLLP_OUT_CPU for gen5 super clock.
ÂÂ */
 struct tegra_clk_super_mux {
ÂÂÂÂÂ struct clk_hwÂÂÂ hw;
@@ -710,6 +713,7 @@ struct tegra_clk_super_mux {
 #define to_clk_super_mux(_hw) container_of(_hw, struct tegra_clk_super_mux, hw)
  #define TEGRA_DIVIDER_2 BIT(0)
+#define TEGRA_CPU_CLKÂÂÂ BIT(1)
I'd name this TEGRA210_CPU_CLK for clarity.

 extern const struct clk_ops tegra_clk_super_ops;
 struct clk *tegra_clk_register_super_mux(const char *name,

Will be better to move the tegra_clk_set_pllp_out_cpu() definition into
this patch, otherwise this looks inconsistent for reviewer.
ok, Will move to this patch