Skip to contentSkip to conversation

clk: tegra: Don't enable already enabled PLLs

PATCH Superseded Dmitry Osipenko 2 participants 5 messages Feb 24, 2019 – Apr 14, 2019

Subsystems (3)
Common clk Tegra architecture support Tegra clock driver
lore
Share & tools
lore

Dmitry Osipenko

author
#0 lore raw

PATCH 1/1 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(-)
drivers/clk/tegra/clk-pll.c +37 -13
index b50b7460014b..ebc8481a2122 100644
@@ -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,
Signature · 1 line
2.20.1
author
#1 lore raw

24.02.2019 18:32, Dmitry Osipenko пишет:

Quoted 8 lines

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
#2 lore raw
Quoted 7 lines · Dmitry Osipenko

24.02.2019 18:32, Dmitry Osipenko пишет:

Quoted 6 lines

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.

Quoted 5 lines
Quoted 2 lines
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]>
author
#3 lore raw

04.03.2019 11:18, Peter De Schrijver пишет:

Quoted 8 lines · Dmitry Osipenko
Quoted 7 lines

24.02.2019 18:32, Dmitry Osipenko пишет:

Quoted 6 lines

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.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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!

author
#4 lore raw

04.03.2019 16:42, Dmitry Osipenko пишет:

Quoted 12 lines

04.03.2019 11:18, Peter De Schrijver пишет:

Quoted 11 lines · Dmitry Osipenko
Quoted 10 lines

24.02.2019 18:32, Dmitry Osipenko пишет:

Quoted 8 lines

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]>

---

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

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.