[PATCH] clk: qcom: clk-alpha-pll: Don't reconfigure running Trion

[PATCH] clk: qcom: clk-alpha-pll: Don't reconfigure running Trion

[Bjorn Andersson]

In the event that the bootloader has configured the Trion PLL as source
for the display clocks, e.g. for the continuous splashscreen, then there
will also be RCGs that are clocked by this instance.

Reconfiguring, and in particular disabling the output of, the PLL will
cause issues for these downstream RCGs and has been shown to prevent
them from being re-parented.

Follow downstream and skip configuration if it's determined that the PLL
is already running.

Fixes: 59128c20a6a9 ("clk: qcom: clk-alpha-pll: Add support for controlling Lucid PLLs")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/clk/qcom/clk-alpha-pll.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index eaedcceb766f..b04aa0a630e9 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -1429,6 +1429,15 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_postdiv_fabia_ops);
 void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 			     const struct alpha_pll_config *config)
 {
+	/*
+	 * If the bootloader left the PLL enabled it's likely that there are
+	 * RCGs that will lock up if we disable the PLL below.
+	 */
+	if (trion_pll_is_enabled(pll, regmap)) {
+		pr_dbg("Trion PLL is already enabled, skipping configuration\n");
+		return;
+	}
+
 	clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
 	regmap_write(regmap, PLL_CAL_L_VAL(pll), TRION_PLL_CAL_VAL);
 	clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
-- 
2.33.1

[PATCH v2] clk: qcom: clk-alpha-pll: Don't reconfigure running Trion

[Bjorn Andersson]

In the event that the bootloader has configured the Trion PLL as source
for the display clocks, e.g. for the continuous splashscreen, then there
will also be RCGs that are clocked by this instance.

Reconfiguring, and in particular disabling the output of, the PLL will
cause issues for these downstream RCGs and has been shown to prevent
them from being re-parented.

Follow downstream and skip configuration if it's determined that the PLL
is already running.

Fixes: 59128c20a6a9 ("clk: qcom: clk-alpha-pll: Add support for controlling Lucid PLLs")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Forgot to commit the last minute s/pr_dbg/pr_debug/

 drivers/clk/qcom/clk-alpha-pll.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index eaedcceb766f..8f65b9bdafce 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -1429,6 +1429,15 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_postdiv_fabia_ops);
 void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 			     const struct alpha_pll_config *config)
 {
+	/*
+	 * If the bootloader left the PLL enabled it's likely that there are
+	 * RCGs that will lock up if we disable the PLL below.
+	 */
+	if (trion_pll_is_enabled(pll, regmap)) {
+		pr_debug("Trion PLL is already enabled, skipping configuration\n");
+		return;
+	}
+
 	clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
 	regmap_write(regmap, PLL_CAL_L_VAL(pll), TRION_PLL_CAL_VAL);
 	clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
-- 
2.33.1

Re: [PATCH v2] clk: qcom: clk-alpha-pll: Don't reconfigure running Trion

[Robert Foss]

Hey Bjorn,

On Tue, 23 Nov 2021 at 17:23, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> In the event that the bootloader has configured the Trion PLL as source
> for the display clocks, e.g. for the continuous splashscreen, then there
> will also be RCGs that are clocked by this instance.
>
> Reconfiguring, and in particular disabling the output of, the PLL will
> cause issues for these downstream RCGs and has been shown to prevent
> them from being re-parented.
>
> Follow downstream and skip configuration if it's determined that the PLL
> is already running.
>
> Fixes: 59128c20a6a9 ("clk: qcom: clk-alpha-pll: Add support for controlling Lucid PLLs")
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v1:
> - Forgot to commit the last minute s/pr_dbg/pr_debug/
>
>  drivers/clk/qcom/clk-alpha-pll.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
> index eaedcceb766f..8f65b9bdafce 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.c
> +++ b/drivers/clk/qcom/clk-alpha-pll.c
> @@ -1429,6 +1429,15 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_postdiv_fabia_ops);
>  void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>                              const struct alpha_pll_config *config)
>  {
> +       /*
> +        * If the bootloader left the PLL enabled it's likely that there are
> +        * RCGs that will lock up if we disable the PLL below.
> +        */
> +       if (trion_pll_is_enabled(pll, regmap)) {
> +               pr_debug("Trion PLL is already enabled, skipping configuration\n");
> +               return;
> +       }
> +
>         clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
>         regmap_write(regmap, PLL_CAL_L_VAL(pll), TRION_PLL_CAL_VAL);
>         clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
> --
> 2.33.1

This resolves an issue I was seeing related to clocks not being able
to get updated.

Reviewed-by: Robert Foss <robert.foss@linaro.org>

Re: [PATCH] clk: qcom: clk-alpha-pll: Don't reconfigure running Trion

[kernel test robot]

Hi Bjorn,

I love your patch! Yet something to improve:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on v5.16-rc2 next-20211123]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bjorn-Andersson/clk-qcom-clk-alpha-pll-Don-t-reconfigure-running-Trion/20211124-001628
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: riscv-randconfig-r042-20211123 (https://download.01.org/0day-ci/archive/20211124/202111240901.R3Fzm2O3-lkp@intel.com/config.gz)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 49e3838145dff1ec91c2e67a2cb562775c8d2a08)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/1c6539db17125d4d4eaf17c4071063fe8a7e2ca6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bjorn-Andersson/clk-qcom-clk-alpha-pll-Don-t-reconfigure-running-Trion/20211124-001628
        git checkout 1c6539db17125d4d4eaf17c4071063fe8a7e2ca6
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/clk/qcom/clk-alpha-pll.c:1437:3: error: implicit declaration of function 'pr_dbg' [-Werror,-Wimplicit-function-declaration]
                   pr_dbg("Trion PLL is already enabled, skipping configuration\n");
                   ^
   1 error generated.


vim +/pr_dbg +1437 drivers/clk/qcom/clk-alpha-pll.c

  1421	
  1422	/**
  1423	 * clk_lucid_pll_configure - configure the lucid pll
  1424	 *
  1425	 * @pll: clk alpha pll
  1426	 * @regmap: register map
  1427	 * @config: configuration to apply for pll
  1428	 */
  1429	void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
  1430				     const struct alpha_pll_config *config)
  1431	{
  1432		/*
  1433		 * If the bootloader left the PLL enabled it's likely that there are
  1434		 * RCGs that will lock up if we disable the PLL below.
  1435		 */
  1436		if (trion_pll_is_enabled(pll, regmap)) {
> 1437			pr_dbg("Trion PLL is already enabled, skipping configuration\n");
  1438			return;
  1439		}
  1440	
  1441		clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
  1442		regmap_write(regmap, PLL_CAL_L_VAL(pll), TRION_PLL_CAL_VAL);
  1443		clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
  1444		clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll),
  1445					     config->config_ctl_val);
  1446		clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll),
  1447					     config->config_ctl_hi_val);
  1448		clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll),
  1449					     config->config_ctl_hi1_val);
  1450		clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll),
  1451						config->user_ctl_val);
  1452		clk_alpha_pll_write_config(regmap, PLL_USER_CTL_U(pll),
  1453						config->user_ctl_hi_val);
  1454		clk_alpha_pll_write_config(regmap, PLL_USER_CTL_U1(pll),
  1455						config->user_ctl_hi1_val);
  1456		clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll),
  1457						config->test_ctl_val);
  1458		clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll),
  1459						config->test_ctl_hi_val);
  1460		clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll),
  1461						config->test_ctl_hi1_val);
  1462	
  1463		regmap_update_bits(regmap, PLL_MODE(pll), PLL_UPDATE_BYPASS,
  1464				   PLL_UPDATE_BYPASS);
  1465	
  1466		/* Disable PLL output */
  1467		regmap_update_bits(regmap, PLL_MODE(pll),  PLL_OUTCTRL, 0);
  1468	
  1469		/* Set operation mode to OFF */
  1470		regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY);
  1471	
  1472		/* Place the PLL in STANDBY mode */
  1473		regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
  1474	}
  1475	EXPORT_SYMBOL_GPL(clk_trion_pll_configure);
  1476	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v2] clk: qcom: clk-alpha-pll: Don't reconfigure running Trion

[Vinod Koul]

On 23-11-21, 08:25, Bjorn Andersson wrote:
> In the event that the bootloader has configured the Trion PLL as source
> for the display clocks, e.g. for the continuous splashscreen, then there
> will also be RCGs that are clocked by this instance.
> 
> Reconfiguring, and in particular disabling the output of, the PLL will
> cause issues for these downstream RCGs and has been shown to prevent
> them from being re-parented.
> 
> Follow downstream and skip configuration if it's determined that the PLL
> is already running.

Reviewed-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

Re: [PATCH] clk: qcom: clk-alpha-pll: Don't reconfigure running Trion

[kernel test robot]

Hi Bjorn,

I love your patch! Yet something to improve:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on v5.16-rc2 next-20211124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bjorn-Andersson/clk-qcom-clk-alpha-pll-Don-t-reconfigure-running-Trion/20211124-001628
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20211124/202111241836.A2WvIKic-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1c6539db17125d4d4eaf17c4071063fe8a7e2ca6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bjorn-Andersson/clk-qcom-clk-alpha-pll-Don-t-reconfigure-running-Trion/20211124-001628
        git checkout 1c6539db17125d4d4eaf17c4071063fe8a7e2ca6
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/clk/qcom/clk-alpha-pll.c: In function 'clk_trion_pll_configure':
>> drivers/clk/qcom/clk-alpha-pll.c:1437:17: error: implicit declaration of function 'pr_dbg'; did you mean 'pr_debug'? [-Werror=implicit-function-declaration]
    1437 |                 pr_dbg("Trion PLL is already enabled, skipping configuration\n");
         |                 ^~~~~~
         |                 pr_debug
   cc1: some warnings being treated as errors


vim +1437 drivers/clk/qcom/clk-alpha-pll.c

  1421	
  1422	/**
  1423	 * clk_lucid_pll_configure - configure the lucid pll
  1424	 *
  1425	 * @pll: clk alpha pll
  1426	 * @regmap: register map
  1427	 * @config: configuration to apply for pll
  1428	 */
  1429	void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
  1430				     const struct alpha_pll_config *config)
  1431	{
  1432		/*
  1433		 * If the bootloader left the PLL enabled it's likely that there are
  1434		 * RCGs that will lock up if we disable the PLL below.
  1435		 */
  1436		if (trion_pll_is_enabled(pll, regmap)) {
> 1437			pr_dbg("Trion PLL is already enabled, skipping configuration\n");
  1438			return;
  1439		}
  1440	
  1441		clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
  1442		regmap_write(regmap, PLL_CAL_L_VAL(pll), TRION_PLL_CAL_VAL);
  1443		clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
  1444		clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll),
  1445					     config->config_ctl_val);
  1446		clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll),
  1447					     config->config_ctl_hi_val);
  1448		clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll),
  1449					     config->config_ctl_hi1_val);
  1450		clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll),
  1451						config->user_ctl_val);
  1452		clk_alpha_pll_write_config(regmap, PLL_USER_CTL_U(pll),
  1453						config->user_ctl_hi_val);
  1454		clk_alpha_pll_write_config(regmap, PLL_USER_CTL_U1(pll),
  1455						config->user_ctl_hi1_val);
  1456		clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll),
  1457						config->test_ctl_val);
  1458		clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll),
  1459						config->test_ctl_hi_val);
  1460		clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll),
  1461						config->test_ctl_hi1_val);
  1462	
  1463		regmap_update_bits(regmap, PLL_MODE(pll), PLL_UPDATE_BYPASS,
  1464				   PLL_UPDATE_BYPASS);
  1465	
  1466		/* Disable PLL output */
  1467		regmap_update_bits(regmap, PLL_MODE(pll),  PLL_OUTCTRL, 0);
  1468	
  1469		/* Set operation mode to OFF */
  1470		regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY);
  1471	
  1472		/* Place the PLL in STANDBY mode */
  1473		regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
  1474	}
  1475	EXPORT_SYMBOL_GPL(clk_trion_pll_configure);
  1476	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v2] clk: qcom: clk-alpha-pll: Don't reconfigure running Trion

[Stephen Boyd]

Quoting Bjorn Andersson (2021-11-23 08:25:08)
> In the event that the bootloader has configured the Trion PLL as source
> for the display clocks, e.g. for the continuous splashscreen, then there
> will also be RCGs that are clocked by this instance.
> 
> Reconfiguring, and in particular disabling the output of, the PLL will
> cause issues for these downstream RCGs and has been shown to prevent
> them from being re-parented.
> 
> Follow downstream and skip configuration if it's determined that the PLL
> is already running.
> 
> Fixes: 59128c20a6a9 ("clk: qcom: clk-alpha-pll: Add support for controlling Lucid PLLs")
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---

Applied to clk-fixes