Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks

From: Sowjanya Komatineni
Date: Tue Jul 16 2019 - 00:37:18 EST



On 7/15/19 8:50 PM, Dmitry Osipenko wrote:
16.07.2019 6:00, Sowjanya Komatineni ÐÐÑÐÑ:
On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
13.07.2019 8:54, Sowjanya Komatineni ÐÐÑÐÑ:
On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
28.06.2019 5:12, Sowjanya Komatineni ÐÐÑÐÑ:
This patch adds system suspend and resume support for Tegra210
clocks.

All the CAR controller settings are lost on suspend when core power
goes off.

This patch has implementation for saving and restoring all the PLLs
and clocks context during system suspend and resume to have the
clocks back to same state for normal operation.

Acked-by: Thierry Reding <treding@xxxxxxxxxx>
Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
---
ÂÂ drivers/clk/tegra/clk-tegra210.c | 115
++++++++++++++++++++++++++++++++++++++-
ÂÂ drivers/clk/tegra/clk.cÂÂÂÂÂÂÂÂÂ |Â 14 +++++
ÂÂ drivers/clk/tegra/clk.hÂÂÂÂÂÂÂÂÂ |ÂÂ 1 +
ÂÂ 3 files changed, 127 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra210.c
b/drivers/clk/tegra/clk-tegra210.c
index 1c08c53482a5..1b839544e086 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -9,10 +9,12 @@
ÂÂ #include <linux/clkdev.h>
ÂÂ #include <linux/of.h>
ÂÂ #include <linux/of_address.h>
+#include <linux/of_platform.h>
ÂÂ #include <linux/delay.h>
ÂÂ #include <linux/export.h>
ÂÂ #include <linux/mutex.h>
ÂÂ #include <linux/clk/tegra.h>
+#include <linux/syscore_ops.h>
ÂÂ #include <dt-bindings/clock/tegra210-car.h>
ÂÂ #include <dt-bindings/reset/tegra210-car.h>
ÂÂ #include <linux/iopoll.h>
@@ -20,6 +22,7 @@
ÂÂ #include <soc/tegra/pmc.h>
ÂÂ Â #include "clk.h"
+#include "clk-dfll.h"
ÂÂ #include "clk-id.h"
ÂÂ Â /*
@@ -225,6 +228,7 @@
ÂÂ Â #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
ÂÂ #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
+#define CPU_SOFTRST_CTRL 0x380
ÂÂ Â #define LVL2_CLK_GATE_OVRA 0xf8
ÂÂ #define LVL2_CLK_GATE_OVRC 0x3a0
@@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
ÂÂÂÂÂÂ struct tegra_clk_pll_freq_table *fentry;
ÂÂÂÂÂÂ struct tegra_clk_pll pllu;
ÂÂÂÂÂÂ u32 reg;
+ÂÂÂ int ret;
ÂÂ ÂÂÂÂÂ for (fentry = pll_u_freq_table; fentry->input_rate;
fentry++) {
ÂÂÂÂÂÂÂÂÂÂ if (fentry->input_rate == pll_ref_freq)
@@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
ÂÂÂÂÂÂ fence_udelay(1, clk_base);
ÂÂÂÂÂÂ reg |= PLL_ENABLE;
ÂÂÂÂÂÂ writel(reg, clk_base + PLLU_BASE);
+ÂÂÂ fence_udelay(1, clk_base);
ÂÂ -ÂÂÂ readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
-ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ reg & PLL_BASE_LOCK, 2, 1000);
-ÂÂÂ if (!(reg & PLL_BASE_LOCK)) {
+ÂÂÂ ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
+ÂÂÂ if (ret) {
ÂÂÂÂÂÂÂÂÂÂ pr_err("Timed out waiting for PLL_U to lock\n");
ÂÂÂÂÂÂÂÂÂÂ return -ETIMEDOUT;
ÂÂÂÂÂÂ }
@@ -3283,6 +3288,103 @@ static void tegra210_disable_cpu_clock(u32
cpu)
ÂÂ }
ÂÂ Â #ifdef CONFIG_PM_SLEEP
+static u32 cpu_softrst_ctx[3];
+static struct platform_device *dfll_pdev;
+#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) +
((_off) * 4))
+#define car_writel(_val, _base, _off) \
+ÂÂÂÂÂÂÂ writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
+
+static int tegra210_clk_suspend(void)
+{
+ÂÂÂ unsigned int i;
+ÂÂÂ struct device_node *node;
+
+ÂÂÂ tegra_cclkg_burst_policy_save_context();
+
+ÂÂÂ if (!dfll_pdev) {
+ÂÂÂÂÂÂÂ node = of_find_compatible_node(NULL, NULL,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "nvidia,tegra210-dfll");
+ÂÂÂÂÂÂÂ if (node)
+ÂÂÂÂÂÂÂÂÂÂÂ dfll_pdev = of_find_device_by_node(node);
+
+ÂÂÂÂÂÂÂ of_node_put(node);
+ÂÂÂÂÂÂÂ if (!dfll_pdev)
+ÂÂÂÂÂÂÂÂÂÂÂ pr_err("dfll node not found. no suspend for dfll\n");
+ÂÂÂ }
+
+ÂÂÂ if (dfll_pdev)
+ÂÂÂÂÂÂÂ tegra_dfll_suspend(dfll_pdev);
+
+ÂÂÂ /* Enable PLLP_OUT_CPU after dfll suspend */
+ÂÂÂ tegra_clk_set_pllp_out_cpu(true);
+
+ÂÂÂ tegra_sclk_cclklp_burst_policy_save_context();
+
+ÂÂÂ clk_save_context();
+
+ÂÂÂ for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
+ÂÂÂÂÂÂÂ cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
+
+ÂÂÂ return 0;
+}
+
+static void tegra210_clk_resume(void)
+{
+ÂÂÂ unsigned int i;
+ÂÂÂ struct clk_hw *parent;
+ÂÂÂ struct clk *clk;
+
+ÂÂÂ /*
+ÂÂÂÂ * clk_restore_context restores clocks as per the clock tree.
+ÂÂÂÂ *
+ÂÂÂÂ * dfllCPU_out is first in the clock tree to get restored and it
+ÂÂÂÂ * involves programming DFLL controller along with restoring
CPUG
+ÂÂÂÂ * clock burst policy.
+ÂÂÂÂ *
+ÂÂÂÂ * DFLL programming needs dfll_ref and dfll_soc peripheral
clocks
+ÂÂÂÂ * to be restores which are part ofthe peripheral clocks.
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ^ white-space

Please use spellchecker to avoid typos.

+ÂÂÂÂ * So, peripheral clocks restore should happen prior to dfll
clock
+ÂÂÂÂ * restore.
+ÂÂÂÂ */
+
+ÂÂÂ tegra_clk_osc_resume(clk_base);
+ÂÂÂ for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
+ÂÂÂÂÂÂÂ car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
+
+ÂÂÂ /* restore all plls and peripheral clocks */
+ÂÂÂ tegra210_init_pllu();
+ÂÂÂ clk_restore_context();
+
+ÂÂÂ fence_udelay(5, clk_base);
+
+ÂÂÂ /* resume SCLK and CPULP clocks */
+ÂÂÂ tegra_sclk_cpulp_burst_policy_restore_context();
+
+ÂÂÂ /*
+ÂÂÂÂ * restore CPUG clocks:
+ÂÂÂÂ * - enable DFLL in open loop mode
+ÂÂÂÂ * - switch CPUG to DFLL clock source
+ÂÂÂÂ * - close DFLL loop
+ÂÂÂÂ * - sync PLLX state
+ÂÂÂÂ */
+ÂÂÂ if (dfll_pdev)
+ÂÂÂÂÂÂÂ tegra_dfll_resume(dfll_pdev, false);
+
+ÂÂÂ tegra_cclkg_burst_policy_restore_context();
+ÂÂÂ fence_udelay(2, clk_base);
+
+ÂÂÂ if (dfll_pdev)
+ÂÂÂÂÂÂÂ tegra_dfll_resume(dfll_pdev, true);
+
+ÂÂÂ parent =
clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
+ÂÂÂ clk = clks[TEGRA210_CLK_PLL_X];
+ÂÂÂ if (parent != __clk_get_hw(clk))
+ÂÂÂÂÂÂÂ tegra_clk_sync_state_pll(__clk_get_hw(clk));
+
+ÂÂÂ /* Disable PLL_OUT_CPU after DFLL resume */
+ÂÂÂ tegra_clk_set_pllp_out_cpu(false);
+}
+
ÂÂ static void tegra210_cpu_clock_suspend(void)
ÂÂ {
ÂÂÂÂÂÂ /* switch coresite to clk_m, save off original source */
@@ -3298,6 +3400,11 @@ static void tegra210_cpu_clock_resume(void)
ÂÂ }
ÂÂ #endif
ÂÂ +static struct syscore_ops tegra_clk_syscore_ops = {
+ÂÂÂ .suspend = tegra210_clk_suspend,
+ÂÂÂ .resume = tegra210_clk_resume,
+};
+
ÂÂ static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
ÂÂÂÂÂÂ .wait_for_resetÂÂÂ = tegra210_wait_cpu_in_reset,
ÂÂÂÂÂÂ .disable_clockÂÂÂ = tegra210_disable_cpu_clock,
@@ -3583,5 +3690,7 @@ static void __init tegra210_clock_init(struct
device_node *np)
ÂÂÂÂÂÂ tegra210_mbist_clk_init();
ÂÂ ÂÂÂÂÂ tegra_cpu_car_ops = &tegra210_cpu_car_ops;
+
+ÂÂÂ register_syscore_ops(&tegra_clk_syscore_ops);
ÂÂ }
Is it really worthwhile to use syscore_ops for suspend/resume given
that drivers for
won't resume before the CLK driver anyway? Are there any other options
for CLK
suspend/resume?

I'm also not sure whether PM runtime API could be used at all in the
context of
syscore_ops ..

Secondly, what about to use generic clk_save_context() /
clk_restore_context()
helpers for the suspend-resume? It looks to me that some other
essential (and proper)
platform driver (soc/tegra/? PMC?) should suspend-resume the clocks
using the generic
CLK Framework API.
Clock resume should happen very early to restore peripheral and cpu
clocks very early than peripheral drivers resume happens.
If all peripheral drivers properly requested all of the necessary clocks
and CLK driver was a platform driver, then I guess the probe should have
been naturally ordered. But that's not very achievable with the
currently available infrastructure in the kernel, so I'm not arguing
that the clocks should be explicitly resumed before the users.

this patch series uses clk_save_context and clk_restore_context for
corresponding divider, pll, pllout.. save and restore context.
Now I see that indeed this API is utilized in this patch, thank you for
the clarification.

But as there is dependency on dfll resume and cpu and pllx clocks
restore, couldnt use clk_save_context and clk_restore_context for dfll.

So implemented recommended dfll resume sequence in main Tegra210 clock
driver along with invoking clk_save_context/clk_restore_context where
all other clocks save/restore happens as per clock tree traversal.
Could you please clarify what part of peripherals clocks is required for
DFLL's restore? Couldn't DFLL driver be changed to avoid that quirkness
and thus to make DFLL driver suspend/resume the clock?
DFLL source ref_clk and soc_clk need to be restored prior to dfll.

I see dfllCPU_out parent to CCLK_G first in the clock tree and
dfll_ref and dfll_soc peripheral clocks are not resumed by the time
dfll resume happens first.

ref_clk and soc_clk source is from pll_p and clock tree has these
registered under pll_p which happens later.

tegra210_clock_init registers in order plls, peripheral clocks,
super_clk init for cclk_g during clock driver probe and dfll probe and
register happens later.

One more thing, CLDVFS peripheral clock enable is also needed to be
enabled to program DFLL Controller and all peripheral clock context is
restored only after their PLL sources are restored.

DFLL restore involves dfll source clock resume along with CLDVFS
periheral clock enable and reset

I don't quite see why you can't simply add suspend/resume callbacks to
the CPUFreq driver to:

On suspend:
1. Switch CPU to PLLP (or whatever "safe" parent)
2. Disable/teardown DFLL

On resume:
1. Enable/restore DFLL
2. Switch CPU back to DFLL

dfll runtime suspend/resume are already part of dfll_pm_ops. Don't we want to use it for suspend/resume as well?

currently no APIs are shared b/w clk/tegra driver and CPUFreq driver to invoke dfll suspend/resume in CPUFreq driver