2019-02-24 15:34:48

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH RE-SEND] clk: tegra: Don't enable already enabled PLLs

Initially Common Clock Framework isn't aware of the clock-enable status,
this results in enabling of clocks that were enabled by bootloader. This
is not a big deal for a regular clock-gates, but for PLL's it may have
some unpleasant consequences. Thus re-enabling PLLX (the main CPU parent
clock) may result in extra long period of PLL re-locking.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clk/tegra/clk-pll.c | 50 +++++++++++++++++++++++++++----------
1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index b50b7460014b..ebc8481a2122 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -444,6 +444,9 @@ static int clk_pll_enable(struct clk_hw *hw)
unsigned long flags = 0;
int ret;

+ if (clk_pll_is_enabled(hw))
+ return 0;
+
if (pll->lock)
spin_lock_irqsave(pll->lock, flags);

@@ -940,11 +943,16 @@ static int clk_plle_training(struct tegra_clk_pll *pll)
static int clk_plle_enable(struct clk_hw *hw)
{
struct tegra_clk_pll *pll = to_clk_pll(hw);
- unsigned long input_rate = clk_hw_get_rate(clk_hw_get_parent(hw));
struct tegra_clk_pll_freq_table sel;
+ unsigned long input_rate;
u32 val;
int err;

+ if (clk_pll_is_enabled(hw))
+ return 0;
+
+ input_rate = clk_hw_get_rate(clk_hw_get_parent(hw));
+
if (_get_table_rate(hw, &sel, pll->params->fixed_rate, input_rate))
return -EINVAL;

@@ -1355,6 +1363,9 @@ static int clk_pllc_enable(struct clk_hw *hw)
int ret;
unsigned long flags = 0;

+ if (clk_pll_is_enabled(hw))
+ return 0;
+
if (pll->lock)
spin_lock_irqsave(pll->lock, flags);

@@ -1567,7 +1578,12 @@ static int clk_plle_tegra114_enable(struct clk_hw *hw)
u32 val;
int ret;
unsigned long flags = 0;
- unsigned long input_rate = clk_hw_get_rate(clk_hw_get_parent(hw));
+ unsigned long input_rate;
+
+ if (clk_pll_is_enabled(hw))
+ return 0;
+
+ input_rate = clk_hw_get_rate(clk_hw_get_parent(hw));

if (_get_table_rate(hw, &sel, pll->params->fixed_rate, input_rate))
return -EINVAL;
@@ -1704,6 +1720,9 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw)
return -EINVAL;
}

+ if (clk_pll_is_enabled(hw))
+ return 0;
+
input_rate = clk_hw_get_rate(__clk_get_hw(osc));

if (pll->lock)
@@ -2379,6 +2398,16 @@ struct clk *tegra_clk_register_pllre_tegra210(const char *name,
return clk;
}

+static int clk_plle_tegra210_is_enabled(struct clk_hw *hw)
+{
+ struct tegra_clk_pll *pll = to_clk_pll(hw);
+ u32 val;
+
+ val = pll_readl_base(pll);
+
+ return val & PLLE_BASE_ENABLE ? 1 : 0;
+}
+
static int clk_plle_tegra210_enable(struct clk_hw *hw)
{
struct tegra_clk_pll *pll = to_clk_pll(hw);
@@ -2386,7 +2415,12 @@ static int clk_plle_tegra210_enable(struct clk_hw *hw)
u32 val;
int ret = 0;
unsigned long flags = 0;
- unsigned long input_rate = clk_hw_get_rate(clk_hw_get_parent(hw));
+ unsigned long input_rate;
+
+ if (clk_plle_tegra210_is_enabled(hw))
+ return 0;
+
+ input_rate = clk_hw_get_rate(clk_hw_get_parent(hw));

if (_get_table_rate(hw, &sel, pll->params->fixed_rate, input_rate))
return -EINVAL;
@@ -2497,16 +2531,6 @@ static void clk_plle_tegra210_disable(struct clk_hw *hw)
spin_unlock_irqrestore(pll->lock, flags);
}

-static int clk_plle_tegra210_is_enabled(struct clk_hw *hw)
-{
- struct tegra_clk_pll *pll = to_clk_pll(hw);
- u32 val;
-
- val = pll_readl_base(pll);
-
- return val & PLLE_BASE_ENABLE ? 1 : 0;
-}
-
static const struct clk_ops tegra_clk_plle_tegra210_ops = {
.is_enabled = clk_plle_tegra210_is_enabled,
.enable = clk_plle_tegra210_enable,
--
2.20.1



2019-02-27 15:29:32

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH RE-SEND] clk: tegra: Don't enable already enabled PLLs

24.02.2019 18:32, Dmitry Osipenko пишет:
> Initially Common Clock Framework isn't aware of the clock-enable status,
> this results in enabling of clocks that were enabled by bootloader. This
> is not a big deal for a regular clock-gates, but for PLL's it may have
> some unpleasant consequences. Thus re-enabling PLLX (the main CPU parent
> clock) may result in extra long period of PLL re-locking.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---

Peter (and everyone else), yours r-b and ACK will be appreciated since you kinda already took a look at this patch in the past and were okay with it (IIRC, we had some brief discussion on the #tegra IRC a few months ago), thanks.


2019-03-04 08:19:36

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH RE-SEND] clk: tegra: Don't enable already enabled PLLs

On Wed, Feb 27, 2019 at 06:15:04PM +0300, Dmitry Osipenko wrote:
> 24.02.2019 18:32, Dmitry Osipenko пишет:
> > Initially Common Clock Framework isn't aware of the clock-enable status,
> > this results in enabling of clocks that were enabled by bootloader. This
> > is not a big deal for a regular clock-gates, but for PLL's it may have
> > some unpleasant consequences. Thus re-enabling PLLX (the main CPU parent
> > clock) may result in extra long period of PLL re-locking.
> >
> > Signed-off-by: Dmitry Osipenko <[email protected]>
> > ---
>
> Peter (and everyone else), yours r-b and ACK will be appreciated since you kinda already took a look at this patch in the past and were okay with it (IIRC, we had some brief discussion on the #tegra IRC a few months ago), thanks.
>

Acked-By: Peter De Schrijver <[email protected]>


2019-03-04 13:54:44

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH RE-SEND] clk: tegra: Don't enable already enabled PLLs

04.03.2019 11:18, Peter De Schrijver пишет:
> On Wed, Feb 27, 2019 at 06:15:04PM +0300, Dmitry Osipenko wrote:
>> 24.02.2019 18:32, Dmitry Osipenko пишет:
>>> Initially Common Clock Framework isn't aware of the clock-enable status,
>>> this results in enabling of clocks that were enabled by bootloader. This
>>> is not a big deal for a regular clock-gates, but for PLL's it may have
>>> some unpleasant consequences. Thus re-enabling PLLX (the main CPU parent
>>> clock) may result in extra long period of PLL re-locking.
>>>
>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>> ---
>>
>> Peter (and everyone else), yours r-b and ACK will be appreciated since you kinda already took a look at this patch in the past and were okay with it (IIRC, we had some brief discussion on the #tegra IRC a few months ago), thanks.
>>
>
> Acked-By: Peter De Schrijver <[email protected]>
>

Awesome, thanks again!

2019-04-14 15:29:25

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH RE-SEND] clk: tegra: Don't enable already enabled PLLs

04.03.2019 16:42, Dmitry Osipenko пишет:
> 04.03.2019 11:18, Peter De Schrijver пишет:
>> On Wed, Feb 27, 2019 at 06:15:04PM +0300, Dmitry Osipenko wrote:
>>> 24.02.2019 18:32, Dmitry Osipenko пишет:
>>>> Initially Common Clock Framework isn't aware of the clock-enable status,
>>>> this results in enabling of clocks that were enabled by bootloader. This
>>>> is not a big deal for a regular clock-gates, but for PLL's it may have
>>>> some unpleasant consequences. Thus re-enabling PLLX (the main CPU parent
>>>> clock) may result in extra long period of PLL re-locking.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>>> ---
>>>
>>> Peter (and everyone else), yours r-b and ACK will be appreciated since you kinda already took a look at this patch in the past and were okay with it (IIRC, we had some brief discussion on the #tegra IRC a few months ago), thanks.
>>>
>>
>> Acked-By: Peter De Schrijver <[email protected]>
>>
>
> Awesome, thanks again!
>

Hello Stephen and Michael,

Could you please pick up this patch to get it into linux-next to get some more testing exposure before it will hit upstream? Thanks in advance.