Re: [PATCH V6 14/21] clk: tegra210: Add suspend and resume support

[Dmitry Osipenko]

22.07.2019 1:45, Sowjanya Komatineni ÐÐÑÐÑ:
> 
> On 7/21/19 2:38 PM, Dmitry Osipenko wrote:
>> 21.07.2019 22:40, Sowjanya Komatineni ÐÐÑÐÑ:
>>> This patch adds support for clk: tegra210: suspend-resume.
>>>
>>> All the CAR controller settings are lost on suspend when core
>>> power goes off.
>>>
>>> This patch has implementation for saving and restoring all 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 | 68
>>> ++++++++++++++++++++++++++++++++++++++--
>>> Â drivers/clk/tegra/clk.cÂÂÂÂÂÂÂÂÂ | 14 +++++++++
>>> Â drivers/clk/tegra/clk.hÂÂÂÂÂÂÂÂÂ |Â 1 +
>>> Â 3 files changed, 80 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>> b/drivers/clk/tegra/clk-tegra210.c
>>> index 55a88c0824a5..68271873acc1 100644
>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>> @@ -9,6 +9,7 @@
>>> Â #include <linux/clkdev.h>
>>> Â #include <linux/of.h>
>>> Â #include <linux/of_address.h>
>>> +#include <linux/syscore_ops.h>
>>> Â #include <linux/delay.h>
>>> Â #include <linux/export.h>
>>> Â #include <linux/mutex.h>
>>> @@ -220,11 +221,15 @@
>>> Â #define CLK_M_DIVISOR_SHIFT 2
>>> Â #define CLK_M_DIVISOR_MASK 0x3
>>> Â +#define CLK_MASK_ARMÂÂÂ 0x44
>>> +#define MISC_CLK_ENBÂÂÂ 0x48
>>> +
>>> Â #define RST_DFLL_DVCO 0x2f4
>>> Â #define DVFS_DFLL_RESET_SHIFT 0
>>> Â Â #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
>>> @@ -2825,6 +2830,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)
>>> @@ -2853,9 +2859,8 @@ static int tegra210_enable_pllu(void)
>>> ÂÂÂÂÂ reg |= PLL_ENABLE;
>>> ÂÂÂÂÂ writel(reg, clk_base + PLLU_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) {
>> Why this is needed? Was there a bug?
>>
> during resume pllu init is needed and to use same terga210_init_pllu,
> poll_timeout_atomic can't be used as its ony for atomic context.
> 
> So changed to use wait_for_mask which should work in both cases.

Atomic variant could be used from any context, not sure what do you
mean. The 'atomic' part only means that function won't cause scheduling
and that's it.

>>> ÂÂÂÂÂÂÂÂÂ pr_err("Timed out waiting for PLL_U to lock\n");
>>> ÂÂÂÂÂÂÂÂÂ return -ETIMEDOUT;
>>> ÂÂÂÂÂ }
>>> @@ -3288,6 +3293,56 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>>> Â }
>>> Â Â #ifdef CONFIG_PM_SLEEP
>>> +#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 u32 spare_reg_ctx, misc_clk_enb_ctx, clk_msk_arm_ctx;
>>> +static u32 cpu_softrst_ctx[3];
>>> +
>>> +static int tegra210_clk_suspend(void)
>>> +{
>>> +ÂÂÂ unsigned int i;
>>> +
>>> +ÂÂÂ clk_save_context();
>>> +
>>> +ÂÂÂ /*
>>> +ÂÂÂÂ * save the bootloader configured clock registers SPARE_REG0,
>>> +ÂÂÂÂ * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL
>> Nit: Start all multi-line comments with a capital letter and put dot in
>> the end of sentence.
>>
>>> +ÂÂÂÂ */
>>> +ÂÂÂ spare_reg_ctx = readl_relaxed(clk_base + SPARE_REG0);
>>> +ÂÂÂ misc_clk_enb_ctx = readl_relaxed(clk_base + MISC_CLK_ENB);
>>> +ÂÂÂ clk_msk_arm_ctx = readl_relaxed(clk_base + CLK_MASK_ARM);
>>> +
>>> +ÂÂÂ 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;
>>> +
>>> +ÂÂÂ tegra_clk_osc_resume(clk_base);
>>> +
>>> +ÂÂÂ /*
>>> +ÂÂÂÂ * restore the bootloader configured clock registers SPARE_REG0,
>>> +ÂÂÂÂ * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL from saved context.
>> Same here.
>>
>>> +ÂÂÂÂ */
>>> +ÂÂÂ writel_relaxed(spare_reg_ctx, clk_base + SPARE_REG0);
>>> +ÂÂÂ writel_relaxed(misc_clk_enb_ctx, clk_base + MISC_CLK_ENB);
>>> +ÂÂÂ writel_relaxed(clk_msk_arm_ctx, clk_base + CLK_MASK_ARM);
>>> +
>>> +ÂÂÂ for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>> +ÂÂÂÂÂÂÂ car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>> +
>>> +ÂÂÂ fence_udelay(5, clk_base);
>>> +
>>> +ÂÂÂ tegra210_init_pllu();
>>> +ÂÂÂ clk_restore_context();
>>> +}
>>> +
>>> Â static void tegra210_cpu_clock_suspend(void)
>>> Â {
>>> ÂÂÂÂÂ /* switch coresite to clk_m, save off original source */
>>> @@ -3303,6 +3358,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,
>>> @@ -3587,5 +3647,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);
>>> Â }
>>> Â CLK_OF_DECLARE(tegra210, "nvidia,tegra210-car", tegra210_clock_init);
>>> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
>>> index 573e3c967ae1..eb08047fd02f 100644
>>> --- a/drivers/clk/tegra/clk.c
>>> +++ b/drivers/clk/tegra/clk.c
>>> @@ -23,6 +23,7 @@
>>> Â #define CLK_OUT_ENB_WÂÂÂÂÂÂÂÂÂÂÂ 0x364
>>> Â #define CLK_OUT_ENB_XÂÂÂÂÂÂÂÂÂÂÂ 0x280
>>> Â #define CLK_OUT_ENB_YÂÂÂÂÂÂÂÂÂÂÂ 0x298
>>> +#define CLK_ENB_PLLP_OUT_CPUÂÂÂÂÂÂÂ BIT(31)
>>> Â #define CLK_OUT_ENB_SET_LÂÂÂÂÂÂÂ 0x320
>>> Â #define CLK_OUT_ENB_CLR_LÂÂÂÂÂÂÂ 0x324
>>> Â #define CLK_OUT_ENB_SET_HÂÂÂÂÂÂÂ 0x328
>>> @@ -199,6 +200,19 @@ const struct tegra_clk_periph_regs
>>> *get_reg_bank(int clkid)
>>> ÂÂÂÂÂ }
>>> Â }
>>> Â +void tegra_clk_set_pllp_out_cpu(bool enable)
>>> +{
>>> +ÂÂÂ u32 val;
>>> +
>>> +ÂÂÂ val = readl_relaxed(clk_base + CLK_OUT_ENB_Y);
>>> +ÂÂÂ if (enable)
>>> +ÂÂÂÂÂÂÂ val |= CLK_ENB_PLLP_OUT_CPU;
>>> +ÂÂÂ else
>>> +ÂÂÂÂÂÂÂ val &= ~CLK_ENB_PLLP_OUT_CPU;
>>> +
>>> +ÂÂÂ writel_relaxed(val, clk_base + CLK_OUT_ENB_Y);
>>> +}
>>> +
>>> Â struct clk ** __init tegra_clk_init(void __iomem *regs, int num,
>>> int banks)
>>> Â {
>>> ÂÂÂÂÂ clk_base = regs;
>>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>>> index 562a3ee2d537..0ffa763c755b 100644
>>> --- a/drivers/clk/tegra/clk.h
>>> +++ b/drivers/clk/tegra/clk.h
>>> @@ -863,6 +863,7 @@ int div_frac_get(unsigned long rate, unsigned
>>> parent_rate, u8 width,
>>> ÂÂÂÂÂÂÂÂÂÂ u8 frac_width, u8 flags);
>>> Â void tegra_clk_sync_state_pll(struct clk_hw *hw);
>>> Â void tegra_clk_osc_resume(void __iomem *clk_base);
>>> +void tegra_clk_set_pllp_out_cpu(bool enable);
>>> Â Â /* Combined read fence with delay */
>>> Â #define fence_udelay(delay, reg)ÂÂÂ \
>>>