diff mbox series

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

Message ID 1563738060-30213-15-git-send-email-skomatineni@nvidia.com
State Superseded
Headers show
Series SC7 entry and exit support for Tegra210 | expand

Commit Message

Sowjanya Komatineni July 21, 2019, 7:40 p.m. UTC 
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@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 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(-)

Comments

Dmitry Osipenko July 21, 2019, 9:38 p.m. UTC | #1 
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@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  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?

>  		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)	\
>
Sowjanya Komatineni July 21, 2019, 10:45 p.m. UTC | #2 
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@nvidia.com>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>>   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.

>>   		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)	\
>>
Dmitry Osipenko July 22, 2019, 6:10 a.m. UTC | #3 
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@nvidia.com>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>>   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)    \
>>>
Sowjanya Komatineni July 22, 2019, 6:52 a.m. UTC | #4 
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@nvidia.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>>    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.

>>>>            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)    \
>>>>
Dmitry Osipenko July 22, 2019, 7:09 a.m. UTC | #5 
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@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>>    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]
Dmitry Osipenko July 22, 2019, 7:12 a.m. UTC | #6 
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@nvidia.com>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>>    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.
Stephen Boyd Aug. 2, 2019, 5:51 p.m. UTC | #7 
Quoting Dmitry Osipenko (2019-07-22 00:12:17)
> 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 пишет:
> >>>>>> @@ -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.

And also add a comment to this location in the code because it's
non-obvious that we can't use iopoll here.
Sowjanya Komatineni Aug. 2, 2019, 8:39 p.m. UTC | #8 
On 8/2/19 10:51 AM, Stephen Boyd wrote:
> Quoting Dmitry Osipenko (2019-07-22 00:12:17)
>> 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 пишет:
>>>>>>>> @@ -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.
> And also add a comment to this location in the code because it's
> non-obvious that we can't use iopoll here.
>
Actually added comment during function usage instead of during include 
as iopoll.h is removed.

Will add additional comment in include section as well highlighting 
reason for removal of iopoll.h
Stephen Boyd Aug. 7, 2019, 9:22 p.m. UTC | #9 
Quoting Sowjanya Komatineni (2019-08-02 13:39:57)
> 
> On 8/2/19 10:51 AM, Stephen Boyd wrote:
> > And also add a comment to this location in the code because it's
> > non-obvious that we can't use iopoll here.
> >
> Actually added comment during function usage instead of during include 
> as iopoll.h is removed.
> 
> Will add additional comment in include section as well highlighting 
> reason for removal of iopoll.h
> 

No I wasn't saying to add a comment to the include section, just add a
comment in the place where you would have called iopoll but don't. Sorry
that it wasn't clear.
diff mbox series

Patch

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) {
 		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
+	 */
+	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.
+	 */
+	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)	\