clk: tegra: pll: Improve PLLM enable-state detection
Subsystems (3)
PATCH v3 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:
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
@@ -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_pmc_clk_enabled (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 disabled by PMC. +*/ +if ((pll->params->flags &TEGRA_PLLM ) && !pllm_pmc_clk_enabled (pll)) +return 0 ; val =pll_readl_base (pll);
Signature · 1 line
2.26.0
Jon Hunter
maintainerQuoted 29 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.
Signed-off-by Dmitry Osipenko <[email protected]>---
Changelog:
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 +0
-0@@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)return clk_pll_wait_for_lock (pll);
drivers/clk/tegra/clk-pll.c int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll) · line 337
Quoted 10 lines
drivers/clk/tegra/clk-pll.c +8
-0} +static bool pllm_pmc_clk_enabled (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 ); +} +
I am not sure that the name of the above function really reflects what it is doing. If it was enabled, isn't it the AND of these bits?
Futhermore, what we really want to know is if the override is enabled, but the PMC PLLM enable is not set. In other words, the PMC is gating the clock. So maybe we should have a function that is called something like pllm_clk_is_gated_by_pmc().
Jon
Signature · 1 line
nvpublic
09.07.2020 12:49, Jon Hunter пишет:
Quoted 46 lines · Dmitry Osipenko
Quoted 29 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.
Signed-off-by Dmitry Osipenko <[email protected]>---
Changelog:
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 +0
-0@@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)return clk_pll_wait_for_lock (pll);drivers/clk/tegra/clk-pll.c int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll) · line 337
Quoted 10 lines
drivers/clk/tegra/clk-pll.c +8
-0} +static bool pllm_pmc_clk_enabled (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 ); +} +I am not sure that the name of the above function really reflects what it is doing. If it was enabled, isn't it the AND of these bits?
Quoted 4 lines
Futhermore, what we really want to know is if the override is enabled, but the PMC PLLM enable is not set. In other words, the PMC is gating the clock. So maybe we should have a function that is called something like
pllm_clk_is_gated_by_pmc().
Yeah, the name indeed could be improved + the logic could be inverted in order to make it all more clear. Thank you for the suggestion! I'll prepare the v4.