Linux-Tegra Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection
@ 2020-07-09 17:20 Dmitry Osipenko
       [not found] ` <20200709172057.13951-1-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-07-09 17:20 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd
  Cc: linux-tegra, linux-clk, linux-kernel

Power Management Controller (PMC) can override the PLLM clock settings,
including the enable-state. Although PMC could only act as a second level
gate, meaning that PLLM needs to be enabled by the Clock and Reset
Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is
overridden by PMC, it needs to be enabled by CaR and ungated by PMC in
order to be functional. Please note that this patch doesn't fix any known
problem, and thus, it's merely a minor improvement.

Link: https://lore.kernel.org/linux-arm-kernel/20191210120909.GA2703785@ulmo/T/
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---

Changelog:

v4: - Renamed pllm_pmc_clk_enabled() to pllm_clk_is_gated_by_pmc() as
      it was suggested by Jon Hunter in the review comment to v3.

v3: - Dropped unintended code change that was accidentally added to v2.

v2: - Added clarifying comment to the code.

    - Prettified the code.

 drivers/clk/tegra/clk-pll.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index b2d39a66f0fa..ff13b371e176 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
 	return clk_pll_wait_for_lock(pll);
 }
 
+static bool pllm_clk_is_gated_by_pmc(struct tegra_clk_pll *pll)
+{
+	u32 val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
+
+	return (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE) &&
+	      !(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
+}
+
 static int clk_pll_is_enabled(struct clk_hw *hw)
 {
 	struct tegra_clk_pll *pll = to_clk_pll(hw);
 	u32 val;
 
-	if (pll->params->flags & TEGRA_PLLM) {
-		val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
-		if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
-			return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;
-	}
+	/*
+	 * Power Management Controller (PMC) can override the PLLM clock
+	 * settings, including the enable-state. The PLLM is enabled when
+	 * PLLM's CaR state is ON and when PLLM isn't gated by PMC.
+	 */
+	if ((pll->params->flags & TEGRA_PLLM) && pllm_clk_is_gated_by_pmc(pll))
+		return 0;
 
 	val = pll_readl_base(pll);
 
-- 
2.26.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection
       [not found] ` <20200709172057.13951-1-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2020-07-14  8:20   ` Jon Hunter
       [not found]     ` <93106a4c-b0fb-3bbf-49cb-bfe348b1ffc7-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Jon Hunter @ 2020-07-14  8:20 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA


On 09/07/2020 18:20, Dmitry Osipenko wrote:
> Power Management Controller (PMC) can override the PLLM clock settings,
> including the enable-state. Although PMC could only act as a second level
> gate, meaning that PLLM needs to be enabled by the Clock and Reset
> Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is
> overridden by PMC, it needs to be enabled by CaR and ungated by PMC in
> order to be functional. Please note that this patch doesn't fix any known
> problem, and thus, it's merely a minor improvement.
> 
> Link: https://lore.kernel.org/linux-arm-kernel/20191210120909.GA2703785@ulmo/T/
> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> 
> Changelog:
> 
> v4: - Renamed pllm_pmc_clk_enabled() to pllm_clk_is_gated_by_pmc() as
>       it was suggested by Jon Hunter in the review comment to v3.
> 
> v3: - Dropped unintended code change that was accidentally added to v2.
> 
> v2: - Added clarifying comment to the code.
> 
>     - Prettified the code.
> 
>  drivers/clk/tegra/clk-pll.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
> index b2d39a66f0fa..ff13b371e176 100644
> --- a/drivers/clk/tegra/clk-pll.c
> +++ b/drivers/clk/tegra/clk-pll.c
> @@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
>  	return clk_pll_wait_for_lock(pll);
>  }
>  
> +static bool pllm_clk_is_gated_by_pmc(struct tegra_clk_pll *pll)
> +{
> +	u32 val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
> +
> +	return (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE) &&
> +	      !(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
> +}
> +
>  static int clk_pll_is_enabled(struct clk_hw *hw)
>  {
>  	struct tegra_clk_pll *pll = to_clk_pll(hw);
>  	u32 val;
>  
> -	if (pll->params->flags & TEGRA_PLLM) {
> -		val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
> -		if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
> -			return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;
> -	}
> +	/*
> +	 * Power Management Controller (PMC) can override the PLLM clock
> +	 * settings, including the enable-state. The PLLM is enabled when
> +	 * PLLM's CaR state is ON and when PLLM isn't gated by PMC.
> +	 */
> +	if ((pll->params->flags & TEGRA_PLLM) && pllm_clk_is_gated_by_pmc(pll))
> +		return 0;
>  
>  	val = pll_readl_base(pll);


Thanks! This looks good to me. However, I just want to do a bit of
testing on Tegra124 (hopefully this week).

Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection
       [not found]     ` <93106a4c-b0fb-3bbf-49cb-bfe348b1ffc7-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2020-07-14  9:36       ` Dmitry Osipenko
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-07-14  9:36 UTC (permalink / raw)
  To: Jon Hunter, Thierry Reding, Peter De Schrijver, Prashant Gaikwad,
	Michael Turquette, Stephen Boyd
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

14.07.2020 11:20, Jon Hunter пишет:
> 
> On 09/07/2020 18:20, Dmitry Osipenko wrote:
>> Power Management Controller (PMC) can override the PLLM clock settings,
>> including the enable-state. Although PMC could only act as a second level
>> gate, meaning that PLLM needs to be enabled by the Clock and Reset
>> Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is
>> overridden by PMC, it needs to be enabled by CaR and ungated by PMC in
>> order to be functional. Please note that this patch doesn't fix any known
>> problem, and thus, it's merely a minor improvement.
>>
>> Link: https://lore.kernel.org/linux-arm-kernel/20191210120909.GA2703785@ulmo/T/
>> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>
>> Changelog:
>>
>> v4: - Renamed pllm_pmc_clk_enabled() to pllm_clk_is_gated_by_pmc() as
>>       it was suggested by Jon Hunter in the review comment to v3.
>>
>> v3: - Dropped unintended code change that was accidentally added to v2.
>>
>> v2: - Added clarifying comment to the code.
>>
>>     - Prettified the code.
>>
>>  drivers/clk/tegra/clk-pll.c | 20 +++++++++++++++-----
>>  1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
>> index b2d39a66f0fa..ff13b371e176 100644
>> --- a/drivers/clk/tegra/clk-pll.c
>> +++ b/drivers/clk/tegra/clk-pll.c
>> @@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
>>  	return clk_pll_wait_for_lock(pll);
>>  }
>>  
>> +static bool pllm_clk_is_gated_by_pmc(struct tegra_clk_pll *pll)
>> +{
>> +	u32 val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
>> +
>> +	return (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE) &&
>> +	      !(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
>> +}
>> +
>>  static int clk_pll_is_enabled(struct clk_hw *hw)
>>  {
>>  	struct tegra_clk_pll *pll = to_clk_pll(hw);
>>  	u32 val;
>>  
>> -	if (pll->params->flags & TEGRA_PLLM) {
>> -		val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
>> -		if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
>> -			return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;
>> -	}
>> +	/*
>> +	 * Power Management Controller (PMC) can override the PLLM clock
>> +	 * settings, including the enable-state. The PLLM is enabled when
>> +	 * PLLM's CaR state is ON and when PLLM isn't gated by PMC.
>> +	 */
>> +	if ((pll->params->flags & TEGRA_PLLM) && pllm_clk_is_gated_by_pmc(pll))
>> +		return 0;
>>  
>>  	val = pll_readl_base(pll);
> 
> 
> Thanks! This looks good to me. However, I just want to do a bit of
> testing on Tegra124 (hopefully this week).

Okay, thank you :)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection
  2020-07-09 17:20 [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection Dmitry Osipenko
       [not found] ` <20200709172057.13951-1-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2020-07-17 16:55 ` Jon Hunter
       [not found]   ` <997361b8-4466-cdf0-beeb-eedcbeb48597-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  2020-07-28  1:21 ` Stephen Boyd
  2 siblings, 1 reply; 6+ messages in thread
From: Jon Hunter @ 2020-07-17 16:55 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Peter De Schrijver,
	Prashant Gaikwad, Michael Turquette, Stephen Boyd
  Cc: linux-tegra, linux-clk, linux-kernel


On 09/07/2020 18:20, Dmitry Osipenko wrote:
> Power Management Controller (PMC) can override the PLLM clock settings,
> including the enable-state. Although PMC could only act as a second level
> gate, meaning that PLLM needs to be enabled by the Clock and Reset
> Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is
> overridden by PMC, it needs to be enabled by CaR and ungated by PMC in
> order to be functional. Please note that this patch doesn't fix any known
> problem, and thus, it's merely a minor improvement.
> 
> Link: https://lore.kernel.org/linux-arm-kernel/20191210120909.GA2703785@ulmo/T/
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> 
> Changelog:
> 
> v4: - Renamed pllm_pmc_clk_enabled() to pllm_clk_is_gated_by_pmc() as
>       it was suggested by Jon Hunter in the review comment to v3.
> 
> v3: - Dropped unintended code change that was accidentally added to v2.
> 
> v2: - Added clarifying comment to the code.
> 
>     - Prettified the code.
> 
>  drivers/clk/tegra/clk-pll.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
> index b2d39a66f0fa..ff13b371e176 100644
> --- a/drivers/clk/tegra/clk-pll.c
> +++ b/drivers/clk/tegra/clk-pll.c
> @@ -327,16 +327,26 @@ int tegra_pll_wait_for_lock(struct tegra_clk_pll *pll)
>  	return clk_pll_wait_for_lock(pll);
>  }
>  
> +static bool pllm_clk_is_gated_by_pmc(struct tegra_clk_pll *pll)
> +{
> +	u32 val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
> +
> +	return (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE) &&
> +	      !(val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE);
> +}
> +
>  static int clk_pll_is_enabled(struct clk_hw *hw)
>  {
>  	struct tegra_clk_pll *pll = to_clk_pll(hw);
>  	u32 val;
>  
> -	if (pll->params->flags & TEGRA_PLLM) {
> -		val = readl_relaxed(pll->pmc + PMC_PLLP_WB0_OVERRIDE);
> -		if (val & PMC_PLLP_WB0_OVERRIDE_PLLM_OVERRIDE)
> -			return val & PMC_PLLP_WB0_OVERRIDE_PLLM_ENABLE ? 1 : 0;
> -	}
> +	/*
> +	 * Power Management Controller (PMC) can override the PLLM clock
> +	 * settings, including the enable-state. The PLLM is enabled when
> +	 * PLLM's CaR state is ON and when PLLM isn't gated by PMC.
> +	 */
> +	if ((pll->params->flags & TEGRA_PLLM) && pllm_clk_is_gated_by_pmc(pll))
> +		return 0;
>  
>  	val = pll_readl_base(pll);
>  
> 

I have tested this on Jetson TK1 using u-boot to verify the behaviour
and it does indeed work as described here. I have also ran it through
the automated testing we have for Tegra and see no immediate issue.
Therefore ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection
       [not found]   ` <997361b8-4466-cdf0-beeb-eedcbeb48597-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2020-07-17 20:15     ` Dmitry Osipenko
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-07-17 20:15 UTC (permalink / raw)
  To: Jon Hunter, Thierry Reding, Peter De Schrijver, Prashant Gaikwad,
	Michael Turquette, Stephen Boyd
  Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

17.07.2020 19:55, Jon Hunter пишет:
...
> I have tested this on Jetson TK1 using u-boot to verify the behaviour
> and it does indeed work as described here. I have also ran it through
> the automated testing we have for Tegra and see no immediate issue.
> Therefore ...
> 
> Reviewed-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Tested-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Thanks!
> Jon

Awesome, thank you!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection
  2020-07-09 17:20 [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection Dmitry Osipenko
       [not found] ` <20200709172057.13951-1-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2020-07-17 16:55 ` Jon Hunter
@ 2020-07-28  1:21 ` Stephen Boyd
  2 siblings, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2020-07-28  1:21 UTC (permalink / raw)
  To: Dmitry Osipenko, Jonathan Hunter, Michael Turquette,
	Peter De Schrijver, Prashant Gaikwad, Thierry Reding
  Cc: linux-tegra, linux-clk, linux-kernel

Quoting Dmitry Osipenko (2020-07-09 10:20:57)
> Power Management Controller (PMC) can override the PLLM clock settings,
> including the enable-state. Although PMC could only act as a second level
> gate, meaning that PLLM needs to be enabled by the Clock and Reset
> Controller (CaR) anyways if we want it to be enabled. Hence, when PLLM is
> overridden by PMC, it needs to be enabled by CaR and ungated by PMC in
> order to be functional. Please note that this patch doesn't fix any known
> problem, and thus, it's merely a minor improvement.
> 
> Link: https://lore.kernel.org/linux-arm-kernel/20191210120909.GA2703785@ulmo/T/
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---

Applied to clk-next

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 17:20 [PATCH v4] clk: tegra: pll: Improve PLLM enable-state detection Dmitry Osipenko
     [not found] ` <20200709172057.13951-1-digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-07-14  8:20   ` Jon Hunter
     [not found]     ` <93106a4c-b0fb-3bbf-49cb-bfe348b1ffc7-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2020-07-14  9:36       ` Dmitry Osipenko
2020-07-17 16:55 ` Jon Hunter
     [not found]   ` <997361b8-4466-cdf0-beeb-eedcbeb48597-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2020-07-17 20:15     ` Dmitry Osipenko
2020-07-28  1:21 ` Stephen Boyd

Linux-Tegra Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-tegra/0 linux-tegra/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-tegra linux-tegra/ https://lore.kernel.org/linux-tegra \
		linux-tegra@vger.kernel.org
	public-inbox-index linux-tegra

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-tegra


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git