Skip to contentSkip to conversation

clk: tegra: pll: Improve PLLM enable-state detection

PATCH Applied Dmitry Osipenko 3 participants 6 messages Jul 9, 2020 – Jul 28, 2020

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v4 1/1 clk: tegra: pll: Improve PLLM enable-state detection

Power Management Controller (PMC) can override the PLLM clock settings, including the enable-state. Although PMC could only act as a second level gate, meaning that PLLM needs to be enabled by the Clock and Reset Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is overridden by PMC, it needs to be enabled by CaR and ungated by PMC in order to be functional. Please note that this patch doesn't fix any known problem, and thus, it's merely a minor improvement.

---

Changelog:

v4: - Renamed pllm_pmc_clk_enabled() to pllm_clk_is_gated_by_pmc() as
it was suggested by Jon Hunter in the review comment to v3.

v3: - Dropped unintended code change that was accidentally added to v2.

v2: - Added clarifying comment to the code.

- Prettified the code.

 drivers/clk/tegra/clk-pll.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
drivers/clk/tegra/clk-pll.c +15 -5
index b2d39a66f0fa..ff13b371e176 100644
@@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
 	return clk_pll_wait_for_lock(pll);
 }
 
+static bool pllm_clk_is_gated_by_pmc(struct tegra_clk_pll *pll)
+{
+	u32 val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
+
+	return (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE) &&
+	      !(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
+}
+
 static int clk_pll_is_enabled(struct clk_hw *hw)
 {
 	struct tegra_clk_pll *pll = to_clk_pll(hw);
 	u32 val;
 
-	if (pll->params->flags & TEGRA_PLLM) {
-		val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
-		if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
-			return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;
-	}
+	/*
+	 * Power Management Controller (PMC) can override the PLLM clock
+	 * settings, including the enable-state. The PLLM is enabled when
+	 * PLLM's CaR state is ON and when PLLM isn't gated by PMC.
+	 */
+	if ((pll->params->flags & TEGRA_PLLM) && pllm_clk_is_gated_by_pmc(pll))
+		return 0;
 
 	val = pll_readl_base(pll);
 
Signature · 1 line
2.26.0
maintainer
#1 lore raw
Quoted 51 lines · Dmitry Osipenko

Power Management Controller (PMC) can override the PLLM clock settings, including the enable-state. Although PMC could only act as a second level gate, meaning that PLLM needs to be enabled by the Clock and Reset Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is overridden by PMC, it needs to be enabled by CaR and ungated by PMC in order to be functional. Please note that this patch doesn't fix any known problem, and thus, it's merely a minor improvement.

---

Changelog:

v4: - Renamed pllm_pmc_clk_enabled() to pllm_clk_is_gated_by_pmc() as
it was suggested by Jon Hunter in the review comment to v3.

v3: - Dropped unintended code change that was accidentally added to v2.

v2: - Added clarifying comment to the code.

- Prettified the code.

 drivers/clk/tegra/clk-pll.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
drivers/clk/tegra/clk-pll.c +8 -4
index b2d39a66f0fa..ff13b371e176 100644
@@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
 	return clk_pll_wait_for_lock(pll);
 }
 
+static bool pllm_clk_is_gated_by_pmc(struct tegra_clk_pll *pll)
+{
+	u32 val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
+
+	return (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE) &&
+	      !(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
+}
+
 static int clk_pll_is_enabled(struct clk_hw *hw)
 {
 	struct tegra_clk_pll *pll = to_clk_pll(hw);
 	u32 val;
 
-	if (pll->params->flags & TEGRA_PLLM) {
-		val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
-		if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
-			return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;

drivers/clk/tegra/clk-pll.c int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll) · line 351

Quoted 10 lines
drivers/clk/tegra/clk-pll.c +7 -1
-	}
+	/*
+	 * Power Management Controller (PMC) can override the PLLM clock
+	 * settings, including the enable-state. The PLLM is enabled when
+	 * PLLM's CaR state is ON and when PLLM isn't gated by PMC.
+	 */
+	if ((pll->params->flags & TEGRA_PLLM) && pllm_clk_is_gated_by_pmc(pll))
+		return 0;
 
 	val = pll_readl_base(pll);

Thanks! This looks good to me. However, I just want to do a bit of testing on Tegra124 (hopefully this week).

Jon

Signature · 1 line
nvpublic
#2 lore raw

14.07.2020 11:20, Jon Hunter пишет:

Quoted 62 lines · Dmitry Osipenko
Quoted 60 lines

Power Management Controller (PMC) can override the PLLM clock settings, including the enable-state. Although PMC could only act as a second level gate, meaning that PLLM needs to be enabled by the Clock and Reset Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is overridden by PMC, it needs to be enabled by CaR and ungated by PMC in order to be functional. Please note that this patch doesn't fix any known problem, and thus, it's merely a minor improvement.

---

Changelog:

v4: - Renamed pllm_pmc_clk_enabled() to pllm_clk_is_gated_by_pmc() as
it was suggested by Jon Hunter in the review comment to v3.

v3: - Dropped unintended code change that was accidentally added to v2.

v2: - Added clarifying comment to the code.

- Prettified the code.

 drivers/clk/tegra/clk-pll.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
drivers/clk/tegra/clk-pll.c +15 -5
index b2d39a66f0fa..ff13b371e176 100644
@@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
 	return clk_pll_wait_for_lock(pll);
 }
 
+static bool pllm_clk_is_gated_by_pmc(struct tegra_clk_pll *pll)
+{
+	u32 val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
+
+	return (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE) &&
+	      !(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
+}
+
 static int clk_pll_is_enabled(struct clk_hw *hw)
 {
 	struct tegra_clk_pll *pll = to_clk_pll(hw);
 	u32 val;
 
-	if (pll->params->flags & TEGRA_PLLM) {
-		val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
-		if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
-			return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;
-	}
+	/*
+	 * Power Management Controller (PMC) can override the PLLM clock
+	 * settings, including the enable-state. The PLLM is enabled when
+	 * PLLM's CaR state is ON and when PLLM isn't gated by PMC.
+	 */
+	if ((pll->params->flags & TEGRA_PLLM) && pllm_clk_is_gated_by_pmc(pll))
+		return 0;
 
Quoted 5 lines

drivers/clk/tegra/clk-pll.c int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll) · line 351

Quoted 1 line
drivers/clk/tegra/clk-pll.c +0 -0
 	val = pll_readl_base(pll);

Thanks! This looks good to me. However, I just want to do a bit of testing on Tegra124 (hopefully this week).

Okay, thank you :)

maintainer Reviewed-by Tested-by on v4 1/1
#3 lore raw
In reply to Dmitry Osipenko
Quoted 53 lines · Dmitry Osipenko

Power Management Controller (PMC) can override the PLLM clock settings, including the enable-state. Although PMC could only act as a second level gate, meaning that PLLM needs to be enabled by the Clock and Reset Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is overridden by PMC, it needs to be enabled by CaR and ungated by PMC in order to be functional. Please note that this patch doesn't fix any known problem, and thus, it's merely a minor improvement.

---

Changelog:

v4: - Renamed pllm_pmc_clk_enabled() to pllm_clk_is_gated_by_pmc() as
it was suggested by Jon Hunter in the review comment to v3.

v3: - Dropped unintended code change that was accidentally added to v2.

v2: - Added clarifying comment to the code.

- Prettified the code.

 drivers/clk/tegra/clk-pll.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
drivers/clk/tegra/clk-pll.c +8 -5
index b2d39a66f0fa..ff13b371e176 100644
@@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
 	return clk_pll_wait_for_lock(pll);
 }
 
+static bool pllm_clk_is_gated_by_pmc(struct tegra_clk_pll *pll)
+{
+	u32 val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
+
+	return (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE) &&
+	      !(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
+}
+
 static int clk_pll_is_enabled(struct clk_hw *hw)
 {
 	struct tegra_clk_pll *pll = to_clk_pll(hw);
 	u32 val;
 
-	if (pll->params->flags & TEGRA_PLLM) {
-		val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
-		if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
-			return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;
-	}

drivers/clk/tegra/clk-pll.c int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll) · line 352

Quoted 10 lines
drivers/clk/tegra/clk-pll.c +7 -0
+	/*
+	 * Power Management Controller (PMC) can override the PLLM clock
+	 * settings, including the enable-state. The PLLM is enabled when
+	 * PLLM's CaR state is ON and when PLLM isn't gated by PMC.
+	 */
+	if ((pll->params->flags & TEGRA_PLLM) && pllm_clk_is_gated_by_pmc(pll))
+		return 0;
 
 	val = pll_readl_base(pll);
 

I have tested this on Jetson TK1 using u-boot to verify the behaviour and it does indeed work as described here. I have also ran it through the automated testing we have for Tegra and see no immediate issue. Therefore ...

Thanks!
Jon

Signature · 1 line
nvpublic
#4 lore raw

17.07.2020 19:55, Jon Hunter пишет:
...

Quoted 10 lines

I have tested this on Jetson TK1 using u-boot to verify the behaviour and it does indeed work as described here. I have also ran it through the automated testing we have for Tegra and see no immediate issue. Therefore ...

Thanks!
Jon

Awesome, thank you!

maintainer Applied
#5 lore raw
In reply to Dmitry Osipenko

Quoting Dmitry Osipenko (2020-07-09 10:20:57)

Quoted 11 lines

Power Management Controller (PMC) can override the PLLM clock settings, including the enable-state. Although PMC could only act as a second level gate, meaning that PLLM needs to be enabled by the Clock and Reset Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is overridden by PMC, it needs to be enabled by CaR and ungated by PMC in order to be functional. Please note that this patch doesn't fix any known problem, and thus, it's merely a minor improvement.

---

Applied to clk-next