2019-07-31 00:24:13

by Sowjanya Komatineni

[permalink] [raw]
Subject: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore

This patch adds support for saving OSC clock frequency and the
drive-strength during OSC clock init and creates an API to restore
OSC control register value from the saved context.

This API is invoked by Tegra210 clock driver during system resume
to restore the OSC clock settings.

Acked-by: Thierry Reding <[email protected]>
Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
drivers/clk/tegra/clk.h | 1 +
2 files changed, 16 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra-fixed.c b/drivers/clk/tegra/clk-tegra-fixed.c
index 8d91b2b191cf..7c6c8abfcde6 100644
--- a/drivers/clk/tegra/clk-tegra-fixed.c
+++ b/drivers/clk/tegra/clk-tegra-fixed.c
@@ -17,6 +17,10 @@
#define OSC_CTRL 0x50
#define OSC_CTRL_OSC_FREQ_SHIFT 28
#define OSC_CTRL_PLL_REF_DIV_SHIFT 26
+#define OSC_CTRL_MASK (0x3f2 | \
+ (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
+
+static u32 osc_ctrl_ctx;

int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
unsigned long *input_freqs, unsigned int num,
@@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
unsigned osc_idx;

val = readl_relaxed(clk_base + OSC_CTRL);
+ osc_ctrl_ctx = val & OSC_CTRL_MASK;
osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;

if (osc_idx < num)
@@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk *tegra_clks)
*dt_clk = clk;
}
}
+
+void tegra_clk_osc_resume(void __iomem *clk_base)
+{
+ u32 val;
+
+ val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
+ val |= osc_ctrl_ctx;
+ writel_relaxed(val, clk_base + OSC_CTRL);
+ fence_udelay(2, clk_base);
+}
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index f1ef6ae8c979..abba6d8a04cd 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -842,6 +842,7 @@ u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
u8 frac_width, u8 flags);
+void tegra_clk_osc_resume(void __iomem *clk_base);


/* Combined read fence with delay */
--
2.7.4


2019-07-31 11:32:47

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore

31.07.2019 3:20, Sowjanya Komatineni пишет:
> This patch adds support for saving OSC clock frequency and the
> drive-strength during OSC clock init and creates an API to restore
> OSC control register value from the saved context.
>
> This API is invoked by Tegra210 clock driver during system resume
> to restore the OSC clock settings.
>
> Acked-by: Thierry Reding <[email protected]>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
> drivers/clk/tegra/clk.h | 1 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c b/drivers/clk/tegra/clk-tegra-fixed.c
> index 8d91b2b191cf..7c6c8abfcde6 100644
> --- a/drivers/clk/tegra/clk-tegra-fixed.c
> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
> @@ -17,6 +17,10 @@
> #define OSC_CTRL 0x50
> #define OSC_CTRL_OSC_FREQ_SHIFT 28
> #define OSC_CTRL_PLL_REF_DIV_SHIFT 26
> +#define OSC_CTRL_MASK (0x3f2 | \
> + (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
> +
> +static u32 osc_ctrl_ctx;
>
> int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
> unsigned long *input_freqs, unsigned int num,
> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
> unsigned osc_idx;
>
> val = readl_relaxed(clk_base + OSC_CTRL);
> + osc_ctrl_ctx = val & OSC_CTRL_MASK;
> osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>
> if (osc_idx < num)
> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk *tegra_clks)
> *dt_clk = clk;
> }
> }
> +
> +void tegra_clk_osc_resume(void __iomem *clk_base)
> +{
> + u32 val;
> +
> + val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
> + val |= osc_ctrl_ctx;
> + writel_relaxed(val, clk_base + OSC_CTRL);

Why a full raw u32 OSC_CTRL value couldn't be simply saved and restored?

> + fence_udelay(2, clk_base);
> +}
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index f1ef6ae8c979..abba6d8a04cd 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -842,6 +842,7 @@ u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
> int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
> u8 frac_width, u8 flags);
> +void tegra_clk_osc_resume(void __iomem *clk_base);
>
>
> /* Combined read fence with delay */
>

2019-07-31 22:19:02

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore


On 7/31/19 4:11 AM, Dmitry Osipenko wrote:
> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>> This patch adds support for saving OSC clock frequency and the
>> drive-strength during OSC clock init and creates an API to restore
>> OSC control register value from the saved context.
>>
>> This API is invoked by Tegra210 clock driver during system resume
>> to restore the OSC clock settings.
>>
>> Acked-by: Thierry Reding <[email protected]>
>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>> ---
>> drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
>> drivers/clk/tegra/clk.h | 1 +
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c b/drivers/clk/tegra/clk-tegra-fixed.c
>> index 8d91b2b191cf..7c6c8abfcde6 100644
>> --- a/drivers/clk/tegra/clk-tegra-fixed.c
>> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
>> @@ -17,6 +17,10 @@
>> #define OSC_CTRL 0x50
>> #define OSC_CTRL_OSC_FREQ_SHIFT 28
>> #define OSC_CTRL_PLL_REF_DIV_SHIFT 26
>> +#define OSC_CTRL_MASK (0x3f2 | \
>> + (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
>> +
>> +static u32 osc_ctrl_ctx;
>>
>> int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
>> unsigned long *input_freqs, unsigned int num,
>> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem *clk_base, struct tegra_clk *clks,
>> unsigned osc_idx;
>>
>> val = readl_relaxed(clk_base + OSC_CTRL);
>> + osc_ctrl_ctx = val & OSC_CTRL_MASK;
>> osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>>
>> if (osc_idx < num)
>> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk *tegra_clks)
>> *dt_clk = clk;
>> }
>> }
>> +
>> +void tegra_clk_osc_resume(void __iomem *clk_base)
>> +{
>> + u32 val;
>> +
>> + val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
>> + val |= osc_ctrl_ctx;
>> + writel_relaxed(val, clk_base + OSC_CTRL);
> Why a full raw u32 OSC_CTRL value couldn't be simply saved and restored?

Storing and restoring only required fields to avoid accidental
misconfiguration.

OSC_CTRL register has other bits (PLL_REF_DIV) which are configured by
BR depending on OSC_FREQ and also setting PLL_REF_DIV while PLLS are in
use is not safe.

>> + fence_udelay(2, clk_base);
>> +}
>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>> index f1ef6ae8c979..abba6d8a04cd 100644
>> --- a/drivers/clk/tegra/clk.h
>> +++ b/drivers/clk/tegra/clk.h
>> @@ -842,6 +842,7 @@ u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
>> int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
>> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
>> u8 frac_width, u8 flags);
>> +void tegra_clk_osc_resume(void __iomem *clk_base);
>>
>>
>> /* Combined read fence with delay */
>>

2019-08-01 11:06:06

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore

01.08.2019 0:04, Sowjanya Komatineni пишет:
>
> On 7/31/19 4:11 AM, Dmitry Osipenko wrote:
>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>> This patch adds support for saving OSC clock frequency and the
>>> drive-strength during OSC clock init and creates an API to restore
>>> OSC control register value from the saved context.
>>>
>>> This API is invoked by Tegra210 clock driver during system resume
>>> to restore the  OSC clock settings.
>>>
>>> Acked-by: Thierry Reding <[email protected]>
>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>> ---
>>>   drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
>>>   drivers/clk/tegra/clk.h             |  1 +
>>>   2 files changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c
>>> b/drivers/clk/tegra/clk-tegra-fixed.c
>>> index 8d91b2b191cf..7c6c8abfcde6 100644
>>> --- a/drivers/clk/tegra/clk-tegra-fixed.c
>>> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
>>> @@ -17,6 +17,10 @@
>>>   #define OSC_CTRL            0x50
>>>   #define OSC_CTRL_OSC_FREQ_SHIFT        28
>>>   #define OSC_CTRL_PLL_REF_DIV_SHIFT    26
>>> +#define OSC_CTRL_MASK            (0x3f2 |    \
>>> +                    (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
>>> +
>>> +static u32 osc_ctrl_ctx;
>>>     int __init tegra_osc_clk_init(void __iomem *clk_base, struct
>>> tegra_clk *clks,
>>>                     unsigned long *input_freqs, unsigned int num,
>>> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem
>>> *clk_base, struct tegra_clk *clks,
>>>       unsigned osc_idx;
>>>         val = readl_relaxed(clk_base + OSC_CTRL);
>>> +    osc_ctrl_ctx = val & OSC_CTRL_MASK;
>>>       osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>>>         if (osc_idx < num)
>>> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk
>>> *tegra_clks)
>>>           *dt_clk = clk;
>>>       }
>>>   }
>>> +
>>> +void tegra_clk_osc_resume(void __iomem *clk_base)
>>> +{
>>> +    u32 val;
>>> +
>>> +    val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
>>> +    val |= osc_ctrl_ctx;
>>> +    writel_relaxed(val, clk_base + OSC_CTRL);
>> Why a full raw u32 OSC_CTRL value couldn't be simply saved and restored?
>
> Storing and restoring only required fields to avoid accidental
> misconfiguration.
>
> OSC_CTRL register has other bits (PLL_REF_DIV) which are configured by
> BR depending on OSC_FREQ and also setting PLL_REF_DIV while PLLS are in
> use is not safe.

I'm looking at the clk-driver sources and see that none of the Tegra
drivers ever change the OSC_CTRL configuration, T30/114 even have
#defines for the OSC_CTRL that are unused.

So, this leads to a question.. does any bootloader really ever change
the OSC_CTRL such that it differs after resume from suspend in
comparison to the value at the time of kernel's booting up?

2019-08-01 18:43:23

by Sowjanya Komatineni

[permalink] [raw]
Subject: Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore


On 8/1/19 3:53 AM, Dmitry Osipenko wrote:
> 01.08.2019 0:04, Sowjanya Komatineni пишет:
>> On 7/31/19 4:11 AM, Dmitry Osipenko wrote:
>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>> This patch adds support for saving OSC clock frequency and the
>>>> drive-strength during OSC clock init and creates an API to restore
>>>> OSC control register value from the saved context.
>>>>
>>>> This API is invoked by Tegra210 clock driver during system resume
>>>> to restore the  OSC clock settings.
>>>>
>>>> Acked-by: Thierry Reding <[email protected]>
>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>> ---
>>>>   drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
>>>>   drivers/clk/tegra/clk.h             |  1 +
>>>>   2 files changed, 16 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c
>>>> b/drivers/clk/tegra/clk-tegra-fixed.c
>>>> index 8d91b2b191cf..7c6c8abfcde6 100644
>>>> --- a/drivers/clk/tegra/clk-tegra-fixed.c
>>>> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
>>>> @@ -17,6 +17,10 @@
>>>>   #define OSC_CTRL            0x50
>>>>   #define OSC_CTRL_OSC_FREQ_SHIFT        28
>>>>   #define OSC_CTRL_PLL_REF_DIV_SHIFT    26
>>>> +#define OSC_CTRL_MASK            (0x3f2 |    \
>>>> +                    (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
>>>> +
>>>> +static u32 osc_ctrl_ctx;
>>>>     int __init tegra_osc_clk_init(void __iomem *clk_base, struct
>>>> tegra_clk *clks,
>>>>                     unsigned long *input_freqs, unsigned int num,
>>>> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem
>>>> *clk_base, struct tegra_clk *clks,
>>>>       unsigned osc_idx;
>>>>         val = readl_relaxed(clk_base + OSC_CTRL);
>>>> +    osc_ctrl_ctx = val & OSC_CTRL_MASK;
>>>>       osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>>>>         if (osc_idx < num)
>>>> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk
>>>> *tegra_clks)
>>>>           *dt_clk = clk;
>>>>       }
>>>>   }
>>>> +
>>>> +void tegra_clk_osc_resume(void __iomem *clk_base)
>>>> +{
>>>> +    u32 val;
>>>> +
>>>> +    val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
>>>> +    val |= osc_ctrl_ctx;
>>>> +    writel_relaxed(val, clk_base + OSC_CTRL);
>>> Why a full raw u32 OSC_CTRL value couldn't be simply saved and restored?
>> Storing and restoring only required fields to avoid accidental
>> misconfiguration.
>>
>> OSC_CTRL register has other bits (PLL_REF_DIV) which are configured by
>> BR depending on OSC_FREQ and also setting PLL_REF_DIV while PLLS are in
>> use is not safe.
> I'm looking at the clk-driver sources and see that none of the Tegra
> drivers ever change the OSC_CTRL configuration, T30/114 even have
> #defines for the OSC_CTRL that are unused.
>
> So, this leads to a question.. does any bootloader really ever change
> the OSC_CTRL such that it differs after resume from suspend in
> comparison to the value at the time of kernel's booting up?

For Tegra210, bootloader programs OSC_CTRL register for drivestrength
programming.

These settings need to be restored to the same on SC7 exit as they gets
reset during SC7 entry.


2019-08-01 18:50:41

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore

01.08.2019 21:06, Sowjanya Komatineni пишет:
>
> On 8/1/19 3:53 AM, Dmitry Osipenko wrote:
>> 01.08.2019 0:04, Sowjanya Komatineni пишет:
>>> On 7/31/19 4:11 AM, Dmitry Osipenko wrote:
>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>> This patch adds support for saving OSC clock frequency and the
>>>>> drive-strength during OSC clock init and creates an API to restore
>>>>> OSC control register value from the saved context.
>>>>>
>>>>> This API is invoked by Tegra210 clock driver during system resume
>>>>> to restore the  OSC clock settings.
>>>>>
>>>>> Acked-by: Thierry Reding <[email protected]>
>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>> ---
>>>>>    drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
>>>>>    drivers/clk/tegra/clk.h             |  1 +
>>>>>    2 files changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> b/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> index 8d91b2b191cf..7c6c8abfcde6 100644
>>>>> --- a/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> @@ -17,6 +17,10 @@
>>>>>    #define OSC_CTRL            0x50
>>>>>    #define OSC_CTRL_OSC_FREQ_SHIFT        28
>>>>>    #define OSC_CTRL_PLL_REF_DIV_SHIFT    26
>>>>> +#define OSC_CTRL_MASK            (0x3f2 |    \
>>>>> +                    (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
>>>>> +
>>>>> +static u32 osc_ctrl_ctx;
>>>>>      int __init tegra_osc_clk_init(void __iomem *clk_base, struct
>>>>> tegra_clk *clks,
>>>>>                      unsigned long *input_freqs, unsigned int num,
>>>>> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem
>>>>> *clk_base, struct tegra_clk *clks,
>>>>>        unsigned osc_idx;
>>>>>          val = readl_relaxed(clk_base + OSC_CTRL);
>>>>> +    osc_ctrl_ctx = val & OSC_CTRL_MASK;
>>>>>        osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>>>>>          if (osc_idx < num)
>>>>> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk
>>>>> *tegra_clks)
>>>>>            *dt_clk = clk;
>>>>>        }
>>>>>    }
>>>>> +
>>>>> +void tegra_clk_osc_resume(void __iomem *clk_base)
>>>>> +{
>>>>> +    u32 val;
>>>>> +
>>>>> +    val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
>>>>> +    val |= osc_ctrl_ctx;
>>>>> +    writel_relaxed(val, clk_base + OSC_CTRL);
>>>> Why a full raw u32 OSC_CTRL value couldn't be simply saved and
>>>> restored?
>>> Storing and restoring only required fields to avoid accidental
>>> misconfiguration.
>>>
>>> OSC_CTRL register has other bits (PLL_REF_DIV) which are configured by
>>> BR depending on OSC_FREQ and also setting PLL_REF_DIV while PLLS are in
>>> use is not safe.
>> I'm looking at the clk-driver sources and see that none of the Tegra
>> drivers ever change the OSC_CTRL configuration, T30/114 even have
>> #defines for the OSC_CTRL that are unused.
>>
>> So, this leads to a question.. does any bootloader really ever change
>> the OSC_CTRL such that it differs after resume from suspend in
>> comparison to the value at the time of kernel's booting up?
>
> For Tegra210, bootloader programs OSC_CTRL register for drivestrength
> programming.
>
> These settings need to be restored to the same on SC7 exit as they gets
> reset during SC7 entry.

Okay, thank you for the clarification.