archive mirror
 help / color / mirror / Atom feed
From: Jerome Brunet <>
To: Neil Armstrong <>,
	Stephen Boyd <>
	Neil Armstrong <>
Subject: Re: [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate
Date: Thu, 19 Sep 2019 15:01:28 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Thu 19 Sep 2019 at 11:38, Neil Armstrong <> wrote:

> Make sure we always enable a PLL on a set_rate() when the PLL is
> flagged as critical.
> This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the
> PSCI firmware when resuming from suspend-to-memory, in the case
> where the CPU was not clocked by the SYS_PLL, but by the fixed PLL
> fixed divisors.
> In this particular case, when changing the PLL rate, CCF doesn't handle
> the fact the PLL could have been disabled in the meantime and set_rate()
> only changes the rate and never enables it again.
> Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled')
> Signed-off-by: Neil Armstrong <>
> ---
>  drivers/clk/meson/clk-pll.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index ddb1e5634739..8c5adccb7959 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>  	}
>  	/* If the pll is stopped, bail out now */
> -	if (!enabled)
> +	if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled)

This is surely a work around to the issue at hand but:

* Enabling the clock, critical or not, should not be done but the
set_rate() callback. This is not the purpose of this callback.

* Enabling the clock in such way does not walk the tree. So, if there is
ever another PSCI Fw which disable we would get into the same issue
again. IOW, This is not specific to the PLL driver so it should not have
to deal with this.

Since this clock can change out of CCF maybe it should be marked with

When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree,
in addition to to calling get_rate(), CCF could also call is_enabled()
if the clock has CLK_IS_CRITICAL and possibly .enable() ?

Stephen, what do you think ?

>  		return 0;
>  	if (meson_clk_pll_enable(hw)) {
> -- 
> 2.22.0

linux-amlogic mailing list

  reply	other threads:[~2019-09-19 13:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-19  9:36 [PATCH 0/3] clk: meson: g12a: fixes for DVFS Neil Armstrong
2019-09-19  9:36 ` [PATCH 1/3] clk: meson: g12a: fix cpu clock rate setting Neil Armstrong
2019-09-19  9:36 ` [PATCH 2/3] clk: meson: g12a: set CLK_MUX_ROUND_CLOSEST on the cpu clock muxes Neil Armstrong
2019-09-19  9:38 ` [PATCH 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate Neil Armstrong
2019-09-19 13:01   ` Jerome Brunet [this message]
2019-09-19 17:06     ` Stephen Boyd
2019-09-20  8:06       ` Neil Armstrong
2019-09-20 16:52         ` Stephen Boyd
2019-10-01 13:15 ` [PATCH 0/3] clk: meson: g12a: fixes for DVFS Jerome Brunet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).