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

[Dmitry Osipenko]

22.07.2019 10:09, Dmitry Osipenko ÐÐÑÐÑ:
> 22.07.2019 9:52, Sowjanya Komatineni ÐÐÑÐÑ:
>>
>> On 7/21/19 11:10 PM, Dmitry Osipenko wrote:
>>> 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.
>>
>> Sorry, replied incorrect. readx_poll_timeout_atomic uses ktime_get() and
>> during resume timekeeping suspend/resume happens later than clock
>> suspend/resume. So using tegra210_wait_for_mask.
>>
>> both timekeeping and clk-tegra210 drivers are registered as syscore but
>> not ordered.
> 
> Okay, thank you for the clarification.
> 
> [snip]
> 

You should remove the 'iopoll.h' then, since it's not used anymore.