Messages in this thread | | | Subject | Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks | From | Sowjanya Komatineni <> | Date | Mon, 15 Jul 2019 23:35:31 -0700 |
| |
On 7/15/19 10:37 PM, Dmitry Osipenko wrote: > В Mon, 15 Jul 2019 21:37:09 -0700 > Sowjanya Komatineni <skomatineni@nvidia.com> пишет: > >> 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@nvidia.com> >>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >>>>>>>>> --- >>>>>>>>> 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? > Looks like no. Seems runtime PM of that driver is intended solely for > the DFLL's clk management. > >> currently no APIs are shared b/w clk/tegra driver and CPUFreq driver >> to invoke dfll suspend/resume in CPUFreq driver >> > Just add it. Also, please note that CPUFreq driver is optional and thus > you may need to switch CPU to a safe parent on clk-core suspend as > well in order to resume properly if CPU was running off unsafe parent > during boot and CPUFreq driver is disabled in kernel build (or failed > to load). OK, Will add to CPUFreq driver... > > The other thing that also need attention is that T124 CPUFreq driver > implicitly relies on DFLL driver to be probed first, which is icky. > Should I add check for successful dfll clk register explicitly in CPUFreq driver probe and defer till dfll clk registers?
| |