* [PATCH v2 1/2] clk: tegra: Don't enable already enabled PLLs @ 2018-08-30 18:42 Dmitry Osipenko 2018-08-30 18:42 ` [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs Dmitry Osipenko 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Osipenko @ 2018-08-30 18:42 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Peter De Schrijver, Prashant Gaikwad, Michael Turquette, Stephen Boyd Cc: linux-clk, linux-tegra, linux-kernel 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 <digetx@gmail.com> --- Changelog: v2: No change. 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 830d1c87fa7c..ddb431247f08 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); @@ -939,11 +942,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; @@ -1354,6 +1362,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); @@ -1566,7 +1577,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; @@ -1703,6 +1719,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) @@ -2378,6 +2397,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); @@ -2385,7 +2414,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; @@ -2496,16 +2530,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.18.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-08-30 18:42 [PATCH v2 1/2] clk: tegra: Don't enable already enabled PLLs Dmitry Osipenko @ 2018-08-30 18:42 ` Dmitry Osipenko 2018-08-31 9:29 ` Peter De Schrijver 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Osipenko @ 2018-08-30 18:42 UTC (permalink / raw) To: Thierry Reding, Jonathan Hunter, Peter De Schrijver, Prashant Gaikwad, Michael Turquette, Stephen Boyd Cc: linux-clk, linux-tegra, linux-kernel Currently all PLL's on Tegra20 use a hardcoded delay despite of having a lock-status bit. The lock-status polling was disabled ~7 years ago because PLLE was failing to lock and was a suspicion that other PLLs might be faulty too. Other PLLs are okay, hence enable the lock-status polling for them. This reduces delay of any operation that require PLL to lock. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- Changelog: v2: Don't enable polling for PLLE as it known to not being able to lock. drivers/clk/tegra/clk-tegra20.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c index cc857d4d4a86..cfde3745a0db 100644 --- a/drivers/clk/tegra/clk-tegra20.c +++ b/drivers/clk/tegra/clk-tegra20.c @@ -298,7 +298,8 @@ static struct tegra_clk_pll_params pll_c_params = { .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, .lock_delay = 300, .freq_table = pll_c_freq_table, - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | + TEGRA_PLL_USE_LOCK, }; static struct tegra_clk_pll_params pll_m_params = { @@ -314,7 +315,8 @@ static struct tegra_clk_pll_params pll_m_params = { .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, .lock_delay = 300, .freq_table = pll_m_freq_table, - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | + TEGRA_PLL_USE_LOCK, }; static struct tegra_clk_pll_params pll_p_params = { @@ -331,7 +333,7 @@ static struct tegra_clk_pll_params pll_p_params = { .lock_delay = 300, .freq_table = pll_p_freq_table, .flags = TEGRA_PLL_FIXED | TEGRA_PLL_HAS_CPCON | - TEGRA_PLL_HAS_LOCK_ENABLE, + TEGRA_PLL_HAS_LOCK_ENABLE | TEGRA_PLL_USE_LOCK, .fixed_rate = 216000000, }; @@ -348,7 +350,8 @@ static struct tegra_clk_pll_params pll_a_params = { .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, .lock_delay = 300, .freq_table = pll_a_freq_table, - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | + TEGRA_PLL_USE_LOCK, }; static struct tegra_clk_pll_params pll_d_params = { @@ -364,7 +367,8 @@ static struct tegra_clk_pll_params pll_d_params = { .lock_enable_bit_idx = PLLDU_MISC_LOCK_ENABLE, .lock_delay = 1000, .freq_table = pll_d_freq_table, - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | + TEGRA_PLL_USE_LOCK, }; static const struct pdiv_map pllu_p[] = { @@ -387,7 +391,8 @@ static struct tegra_clk_pll_params pll_u_params = { .lock_delay = 1000, .pdiv_tohw = pllu_p, .freq_table = pll_u_freq_table, - .flags = TEGRA_PLLU | TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, + .flags = TEGRA_PLLU | TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | + TEGRA_PLL_USE_LOCK, }; static struct tegra_clk_pll_params pll_x_params = { @@ -403,7 +408,8 @@ static struct tegra_clk_pll_params pll_x_params = { .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, .lock_delay = 300, .freq_table = pll_x_freq_table, - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | + TEGRA_PLL_USE_LOCK, }; static struct tegra_clk_pll_params pll_e_params = { -- 2.18.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-08-30 18:42 ` [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs Dmitry Osipenko @ 2018-08-31 9:29 ` Peter De Schrijver 2018-08-31 9:45 ` Dmitry Osipenko 2018-10-17 10:59 ` Marcel Ziswiler 0 siblings, 2 replies; 10+ messages in thread From: Peter De Schrijver @ 2018-08-31 9:29 UTC (permalink / raw) To: Dmitry Osipenko Cc: Thierry Reding, Jonathan Hunter, Prashant Gaikwad, Michael Turquette, Stephen Boyd, linux-clk, linux-tegra, linux-kernel On Thu, Aug 30, 2018 at 09:42:10PM +0300, Dmitry Osipenko wrote: > Currently all PLL's on Tegra20 use a hardcoded delay despite of having > a lock-status bit. The lock-status polling was disabled ~7 years ago > because PLLE was failing to lock and was a suspicion that other PLLs > might be faulty too. Other PLLs are okay, hence enable the lock-status > polling for them. This reduces delay of any operation that require PLL > to lock. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > > Changelog: > > v2: Don't enable polling for PLLE as it known to not being able to lock. > This isn't correct. The lock bit of PLLE can declare lock too early, but the PLL itself does lock. > drivers/clk/tegra/clk-tegra20.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c > index cc857d4d4a86..cfde3745a0db 100644 > --- a/drivers/clk/tegra/clk-tegra20.c > +++ b/drivers/clk/tegra/clk-tegra20.c > @@ -298,7 +298,8 @@ static struct tegra_clk_pll_params pll_c_params = { > .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, > .lock_delay = 300, > .freq_table = pll_c_freq_table, > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > + TEGRA_PLL_USE_LOCK, > }; > > static struct tegra_clk_pll_params pll_m_params = { > @@ -314,7 +315,8 @@ static struct tegra_clk_pll_params pll_m_params = { > .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, > .lock_delay = 300, > .freq_table = pll_m_freq_table, > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > + TEGRA_PLL_USE_LOCK, > }; > > static struct tegra_clk_pll_params pll_p_params = { > @@ -331,7 +333,7 @@ static struct tegra_clk_pll_params pll_p_params = { > .lock_delay = 300, > .freq_table = pll_p_freq_table, > .flags = TEGRA_PLL_FIXED | TEGRA_PLL_HAS_CPCON | > - TEGRA_PLL_HAS_LOCK_ENABLE, > + TEGRA_PLL_HAS_LOCK_ENABLE | TEGRA_PLL_USE_LOCK, > .fixed_rate = 216000000, > }; > > @@ -348,7 +350,8 @@ static struct tegra_clk_pll_params pll_a_params = { > .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, > .lock_delay = 300, > .freq_table = pll_a_freq_table, > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > + TEGRA_PLL_USE_LOCK, > }; > > static struct tegra_clk_pll_params pll_d_params = { > @@ -364,7 +367,8 @@ static struct tegra_clk_pll_params pll_d_params = { > .lock_enable_bit_idx = PLLDU_MISC_LOCK_ENABLE, > .lock_delay = 1000, > .freq_table = pll_d_freq_table, > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > + TEGRA_PLL_USE_LOCK, > }; > > static const struct pdiv_map pllu_p[] = { > @@ -387,7 +391,8 @@ static struct tegra_clk_pll_params pll_u_params = { > .lock_delay = 1000, > .pdiv_tohw = pllu_p, > .freq_table = pll_u_freq_table, > - .flags = TEGRA_PLLU | TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > + .flags = TEGRA_PLLU | TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > + TEGRA_PLL_USE_LOCK, > }; > > static struct tegra_clk_pll_params pll_x_params = { > @@ -403,7 +408,8 @@ static struct tegra_clk_pll_params pll_x_params = { > .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, > .lock_delay = 300, > .freq_table = pll_x_freq_table, > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > + TEGRA_PLL_USE_LOCK, > }; > > static struct tegra_clk_pll_params pll_e_params = { > -- > 2.18.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-08-31 9:29 ` Peter De Schrijver @ 2018-08-31 9:45 ` Dmitry Osipenko 2018-09-03 8:01 ` Peter De Schrijver 2018-10-17 11:52 ` Dmitry Osipenko 2018-10-17 10:59 ` Marcel Ziswiler 1 sibling, 2 replies; 10+ messages in thread From: Dmitry Osipenko @ 2018-08-31 9:45 UTC (permalink / raw) To: Peter De Schrijver Cc: Thierry Reding, Jonathan Hunter, Prashant Gaikwad, Michael Turquette, Stephen Boyd, linux-clk, linux-tegra, linux-kernel On 8/31/18 12:29 PM, Peter De Schrijver wrote: > On Thu, Aug 30, 2018 at 09:42:10PM +0300, Dmitry Osipenko wrote: >> Currently all PLL's on Tegra20 use a hardcoded delay despite of having >> a lock-status bit. The lock-status polling was disabled ~7 years ago >> because PLLE was failing to lock and was a suspicion that other PLLs >> might be faulty too. Other PLLs are okay, hence enable the lock-status >> polling for them. This reduces delay of any operation that require PLL >> to lock. >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> >> Changelog: >> >> v2: Don't enable polling for PLLE as it known to not being able to lock. >> > > This isn't correct. The lock bit of PLLE can declare lock too early, but the > PLL itself does lock. Indeed, it locks but can't be polled for the lock-status as it doesn't have the lock-status bit. Do you want me to adjust the commit description or it is fine as is? It is also a bit odd that PLLE has "lock_delay = 0", is it correct? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-08-31 9:45 ` Dmitry Osipenko @ 2018-09-03 8:01 ` Peter De Schrijver 2018-09-04 9:06 ` Dmitry Osipenko 2018-10-17 11:52 ` Dmitry Osipenko 1 sibling, 1 reply; 10+ messages in thread From: Peter De Schrijver @ 2018-09-03 8:01 UTC (permalink / raw) To: Dmitry Osipenko Cc: Thierry Reding, Jonathan Hunter, Prashant Gaikwad, Michael Turquette, Stephen Boyd, linux-clk, linux-tegra, linux-kernel On Fri, Aug 31, 2018 at 12:45:17PM +0300, Dmitry Osipenko wrote: > On 8/31/18 12:29 PM, Peter De Schrijver wrote: > > On Thu, Aug 30, 2018 at 09:42:10PM +0300, Dmitry Osipenko wrote: > >> Currently all PLL's on Tegra20 use a hardcoded delay despite of having > >> a lock-status bit. The lock-status polling was disabled ~7 years ago > >> because PLLE was failing to lock and was a suspicion that other PLLs > >> might be faulty too. Other PLLs are okay, hence enable the lock-status > >> polling for them. This reduces delay of any operation that require PLL > >> to lock. > >> > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >> --- > >> > >> Changelog: > >> > >> v2: Don't enable polling for PLLE as it known to not being able to lock. > >> > > > > This isn't correct. The lock bit of PLLE can declare lock too early, but the > > PLL itself does lock. > > Indeed, it locks but can't be polled for the lock-status as it doesn't have the > lock-status bit. > > Do you want me to adjust the commit description or it is fine as is? > I think it's better to adjust it. > It is also a bit odd that PLLE has "lock_delay = 0", is it correct? That seems odd yes.. Peter. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-09-03 8:01 ` Peter De Schrijver @ 2018-09-04 9:06 ` Dmitry Osipenko 0 siblings, 0 replies; 10+ messages in thread From: Dmitry Osipenko @ 2018-09-04 9:06 UTC (permalink / raw) To: Peter De Schrijver Cc: Thierry Reding, Jonathan Hunter, Prashant Gaikwad, Michael Turquette, Stephen Boyd, linux-clk, linux-tegra, linux-kernel On Monday 03 September 2018 11:01:11 Peter De Schrijver wrote: > On Fri, Aug 31, 2018 at 12:45:17PM +0300, Dmitry Osipenko wrote: > > On 8/31/18 12:29 PM, Peter De Schrijver wrote: > > > On Thu, Aug 30, 2018 at 09:42:10PM +0300, Dmitry Osipenko wrote: > > >> Currently all PLL's on Tegra20 use a hardcoded delay despite of having > > >> a lock-status bit. The lock-status polling was disabled ~7 years ago > > >> because PLLE was failing to lock and was a suspicion that other PLLs > > >> might be faulty too. Other PLLs are okay, hence enable the lock-status > > >> polling for them. This reduces delay of any operation that require PLL > > >> to lock. > > >> > > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > >> --- > > >> > > >> Changelog: > > >> > > >> v2: Don't enable polling for PLLE as it known to not being able to > > >> lock. > > > > > > This isn't correct. The lock bit of PLLE can declare lock too early, > > > but the PLL itself does lock. > > > > Indeed, it locks but can't be polled for the lock-status as it doesn't > > have the lock-status bit. > > > > Do you want me to adjust the commit description or it is fine as is? > > I think it's better to adjust it. Okay. I expect to get a review from you for the other clock (and related) patches too and will send the new version once all the current patches will be reviewed. Please take a look at them once you'll have some free time, thanks. > > It is also a bit odd that PLLE has "lock_delay = 0", is it correct? > > That seems odd yes.. :) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-08-31 9:45 ` Dmitry Osipenko 2018-09-03 8:01 ` Peter De Schrijver @ 2018-10-17 11:52 ` Dmitry Osipenko 1 sibling, 0 replies; 10+ messages in thread From: Dmitry Osipenko @ 2018-10-17 11:52 UTC (permalink / raw) To: Peter De Schrijver Cc: Thierry Reding, Jonathan Hunter, Prashant Gaikwad, Michael Turquette, Stephen Boyd, linux-clk, linux-tegra, linux-kernel On 8/31/18 12:45 PM, Dmitry Osipenko wrote: > On 8/31/18 12:29 PM, Peter De Schrijver wrote: >> On Thu, Aug 30, 2018 at 09:42:10PM +0300, Dmitry Osipenko wrote: >>> Currently all PLL's on Tegra20 use a hardcoded delay despite of having >>> a lock-status bit. The lock-status polling was disabled ~7 years ago >>> because PLLE was failing to lock and was a suspicion that other PLLs >>> might be faulty too. Other PLLs are okay, hence enable the lock-status >>> polling for them. This reduces delay of any operation that require PLL >>> to lock. >>> >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>> --- >>> >>> Changelog: >>> >>> v2: Don't enable polling for PLLE as it known to not being able to lock. >>> >> >> This isn't correct. The lock bit of PLLE can declare lock too early, but the >> PLL itself does lock. > > Indeed, it locks but can't be polled for the lock-status as it doesn't have the > lock-status bit. Actually it has lock-status bit. Not sure how I managed to miss it before. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-08-31 9:29 ` Peter De Schrijver 2018-08-31 9:45 ` Dmitry Osipenko @ 2018-10-17 10:59 ` Marcel Ziswiler 2018-10-17 11:41 ` Dmitry Osipenko 1 sibling, 1 reply; 10+ messages in thread From: Marcel Ziswiler @ 2018-10-17 10:59 UTC (permalink / raw) To: pdeschrijver, digetx Cc: linux-kernel, jonathanh, mturquette, pgaikwad, sboyd, thierry.reding, linux-clk, linux-tegra On Fri, 2018-08-31 at 12:29 +0300, Peter De Schrijver wrote: > On Thu, Aug 30, 2018 at 09:42:10PM +0300, Dmitry Osipenko wrote: > > Currently all PLL's on Tegra20 use a hardcoded delay despite of > > having > > a lock-status bit. The lock-status polling was disabled ~7 years > > ago > > because PLLE was failing to lock and was a suspicion that other > > PLLs > > might be faulty too. Other PLLs are okay, hence enable the lock- > > status > > polling for them. This reduces delay of any operation that require > > PLL > > to lock. > > > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > --- > > > > Changelog: > > > > v2: Don't enable polling for PLLE as it known to not being > > able to lock. > > > > This isn't correct. The lock bit of PLLE can declare lock too early, > but the > PLL itself does lock. Is there an errata documenting this? As I could not really find any mentioning of this anywhere at least up to the v11 from Dec 21, 2010 I still have access to. BTW: It looks like also PLLA may not always lock properly with those changes. Is there anything known about that as well? Here is what I get on various Colibri T20 modules (while random other ones seem to work fine): [ 0.232591] clk_pll_wait_for_lock: Timed out waiting for pll pll_a lock [ 0.232614] tegra_init_from_table: Failed to enable pll_a [ 0.232627] ------------[ cut here ]------------ [ 0.232655] WARNING: CPU: 0 PID: 1 at /run/media/zim/Build/Sources/linux- next.git/drivers/clk/tegra/clk.c:285 tegra_init_from_table+0x168/0x174 [ 0.232676] Modules linked in: [ 0.232696] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc2- next-20180903-00214-g5618a0514cf1-dirty #183 [ 0.232714] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [ 0.232753] [<c0113644>] (unwind_backtrace) from [<c010dc0c>] (show_stack+0x10/0x14) [ 0.232783] [<c010dc0c>] (show_stack) from [<c0aa66f4>] (dump_stack+0x8c/0xa0) [ 0.232808] [<c0aa66f4>] (dump_stack) from [<c0125428>] (__warn+0xe0/0xf8) [ 0.232828] [<c0125428>] (__warn) from [<c0125558>] (warn_slowpath_null+0x40/0x48) [ 0.232849] [<c0125558>] (warn_slowpath_null) from [<c0f2f284>] (tegra_init_from_table+0x168/0x174) [ 0.232874] [<c0f2f284>] (tegra_init_from_table) from [<c0f2f03c>] (tegra_clocks_apply_init_table+0x1c/0x2c) [ 0.232901] [<c0f2f03c>] (tegra_clocks_apply_init_table) from [<c0102fa0>] (do_one_initcall+0x54/0x278) [ 0.232926] [<c0102fa0>] (do_one_initcall) from [<c0f01128>] (kernel_init_freeable+0x2c0/0x354) [ 0.232949] [<c0f01128>] (kernel_init_freeable) from [<c0abb1f0>] (kernel_init+0x8/0x10c) [ 0.232970] [<c0abb1f0>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c) [ 0.232987] Exception stack(0xc4c8ffb0 to 0xc4c8fff8) [ 0.233001] ffa0: 00000000 00000000 00000000 00000000 [ 0.233021] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 0.233040] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 0.233059] ---[ end trace 3f40fa49530610b9 ]--- > > drivers/clk/tegra/clk-tegra20.c | 20 +++++++++++++------- > > 1 file changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/clk/tegra/clk-tegra20.c > > b/drivers/clk/tegra/clk-tegra20.c > > index cc857d4d4a86..cfde3745a0db 100644 > > --- a/drivers/clk/tegra/clk-tegra20.c > > +++ b/drivers/clk/tegra/clk-tegra20.c > > @@ -298,7 +298,8 @@ static struct tegra_clk_pll_params pll_c_params > > = { > > .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, > > .lock_delay = 300, > > .freq_table = pll_c_freq_table, > > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > > + TEGRA_PLL_USE_LOCK, > > }; > > > > static struct tegra_clk_pll_params pll_m_params = { > > @@ -314,7 +315,8 @@ static struct tegra_clk_pll_params pll_m_params > > = { > > .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, > > .lock_delay = 300, > > .freq_table = pll_m_freq_table, > > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > > + TEGRA_PLL_USE_LOCK, > > }; > > > > static struct tegra_clk_pll_params pll_p_params = { > > @@ -331,7 +333,7 @@ static struct tegra_clk_pll_params pll_p_params > > = { > > .lock_delay = 300, > > .freq_table = pll_p_freq_table, > > .flags = TEGRA_PLL_FIXED | TEGRA_PLL_HAS_CPCON | > > - TEGRA_PLL_HAS_LOCK_ENABLE, > > + TEGRA_PLL_HAS_LOCK_ENABLE | TEGRA_PLL_USE_LOCK, > > .fixed_rate = 216000000, > > }; > > > > @@ -348,7 +350,8 @@ static struct tegra_clk_pll_params pll_a_params > > = { > > .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, > > .lock_delay = 300, > > .freq_table = pll_a_freq_table, > > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > > + TEGRA_PLL_USE_LOCK, > > }; > > > > static struct tegra_clk_pll_params pll_d_params = { > > @@ -364,7 +367,8 @@ static struct tegra_clk_pll_params pll_d_params > > = { > > .lock_enable_bit_idx = PLLDU_MISC_LOCK_ENABLE, > > .lock_delay = 1000, > > .freq_table = pll_d_freq_table, > > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > > + TEGRA_PLL_USE_LOCK, > > }; > > > > static const struct pdiv_map pllu_p[] = { > > @@ -387,7 +391,8 @@ static struct tegra_clk_pll_params pll_u_params > > = { > > .lock_delay = 1000, > > .pdiv_tohw = pllu_p, > > .freq_table = pll_u_freq_table, > > - .flags = TEGRA_PLLU | TEGRA_PLL_HAS_CPCON | > > TEGRA_PLL_HAS_LOCK_ENABLE, > > + .flags = TEGRA_PLLU | TEGRA_PLL_HAS_CPCON | > > TEGRA_PLL_HAS_LOCK_ENABLE | > > + TEGRA_PLL_USE_LOCK, > > }; > > > > static struct tegra_clk_pll_params pll_x_params = { > > @@ -403,7 +408,8 @@ static struct tegra_clk_pll_params pll_x_params > > = { > > .lock_enable_bit_idx = PLL_MISC_LOCK_ENABLE, > > .lock_delay = 300, > > .freq_table = pll_x_freq_table, > > - .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE, > > + .flags = TEGRA_PLL_HAS_CPCON | TEGRA_PLL_HAS_LOCK_ENABLE | > > + TEGRA_PLL_USE_LOCK, > > }; > > > > static struct tegra_clk_pll_params pll_e_params = { > > -- > > 2.18.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-10-17 10:59 ` Marcel Ziswiler @ 2018-10-17 11:41 ` Dmitry Osipenko 2018-12-10 0:58 ` Dmitry Osipenko 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Osipenko @ 2018-10-17 11:41 UTC (permalink / raw) To: Marcel Ziswiler, pdeschrijver Cc: linux-kernel, jonathanh, mturquette, pgaikwad, sboyd, thierry.reding, linux-clk, linux-tegra On 10/17/18 1:59 PM, Marcel Ziswiler wrote: > On Fri, 2018-08-31 at 12:29 +0300, Peter De Schrijver wrote: >> On Thu, Aug 30, 2018 at 09:42:10PM +0300, Dmitry Osipenko wrote: >>> Currently all PLL's on Tegra20 use a hardcoded delay despite of >>> having >>> a lock-status bit. The lock-status polling was disabled ~7 years >>> ago >>> because PLLE was failing to lock and was a suspicion that other >>> PLLs >>> might be faulty too. Other PLLs are okay, hence enable the lock- >>> status >>> polling for them. This reduces delay of any operation that require >>> PLL >>> to lock. >>> >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>> --- >>> >>> Changelog: >>> >>> v2: Don't enable polling for PLLE as it known to not being >>> able to lock. >>> >> >> This isn't correct. The lock bit of PLLE can declare lock too early, >> but the >> PLL itself does lock. > > Is there an errata documenting this? As I could not really find any > mentioning of this anywhere at least up to the v11 from Dec 21, 2010 I > still have access to. > > BTW: It looks like also PLLA may not always lock properly with those > changes. Is there anything known about that as well? Here is what I get > on various Colibri T20 modules (while random other ones seem to work > fine): Could you please try to increase the timeout value? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs 2018-10-17 11:41 ` Dmitry Osipenko @ 2018-12-10 0:58 ` Dmitry Osipenko 0 siblings, 0 replies; 10+ messages in thread From: Dmitry Osipenko @ 2018-12-10 0:58 UTC (permalink / raw) To: Marcel Ziswiler, pdeschrijver Cc: linux-kernel, jonathanh, mturquette, pgaikwad, sboyd, thierry.reding, linux-clk, linux-tegra В Wed, 17 Oct 2018 14:41:35 +0300 Dmitry Osipenko <digetx@gmail.com> пишет: > On 10/17/18 1:59 PM, Marcel Ziswiler wrote: > > On Fri, 2018-08-31 at 12:29 +0300, Peter De Schrijver wrote: > >> On Thu, Aug 30, 2018 at 09:42:10PM +0300, Dmitry Osipenko wrote: > >>> Currently all PLL's on Tegra20 use a hardcoded delay despite of > >>> having > >>> a lock-status bit. The lock-status polling was disabled ~7 years > >>> ago > >>> because PLLE was failing to lock and was a suspicion that other > >>> PLLs > >>> might be faulty too. Other PLLs are okay, hence enable the lock- > >>> status > >>> polling for them. This reduces delay of any operation that require > >>> PLL > >>> to lock. > >>> > >>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >>> --- > >>> > >>> Changelog: > >>> > >>> v2: Don't enable polling for PLLE as it known to not being > >>> able to lock. > >>> > >> > >> This isn't correct. The lock bit of PLLE can declare lock too > >> early, but the > >> PLL itself does lock. > > > > Is there an errata documenting this? As I could not really find any > > mentioning of this anywhere at least up to the v11 from Dec 21, > > 2010 I still have access to. > > > > BTW: It looks like also PLLA may not always lock properly with those > > changes. Is there anything known about that as well? Here is what I > > get on various Colibri T20 modules (while random other ones seem to > > work fine): > Could you please try to increase the timeout value? Hello Marcel, Do you have any update on the PLL-lock-failure problem? ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-12-10 0:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-30 18:42 [PATCH v2 1/2] clk: tegra: Don't enable already enabled PLLs Dmitry Osipenko 2018-08-30 18:42 ` [PATCH v2 2/2] clk: tegra20: Enable lock-status polling for PLLs Dmitry Osipenko 2018-08-31 9:29 ` Peter De Schrijver 2018-08-31 9:45 ` Dmitry Osipenko 2018-09-03 8:01 ` Peter De Schrijver 2018-09-04 9:06 ` Dmitry Osipenko 2018-10-17 11:52 ` Dmitry Osipenko 2018-10-17 10:59 ` Marcel Ziswiler 2018-10-17 11:41 ` Dmitry Osipenko 2018-12-10 0:58 ` Dmitry Osipenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).