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 <[email protected]>
---
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
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 <[email protected]>
---
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
Hey Bjorn,
On Tue, 23 Nov 2021 at 17:23, Bjorn Andersson
<[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
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/[email protected]/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 <[email protected]>
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/[email protected]
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 <[email protected]>
--
~Vinod
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/[email protected]/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 <[email protected]>
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/[email protected]
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 <[email protected]>
> ---
Applied to clk-fixes