[PATCH v2 0/5] clk: qcom: remove superfluous alpha settings from PLL configs

[PATCH v2 0/5] clk: qcom: remove superfluous alpha settings from PLL configs

[Gabor Juhos]

There are several alpha PLL  configurations, where the alpha mode
is enabled even if it is not strictly required in order to get the
desired output rate of the PLL. This small series removes those
superfluous settings.

Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - extend the commit messages in the patches to indicate that dropping
    'alpha_en_mask' is safe
  - collect RB tags
  - Link to v1: https://lore.kernel.org/r/20241021-alpha-mode-cleanup-v1-0-55df8ed73645@gmail.com

---
Gabor Juhos (5):
      clk: qcom: apss-ipq-pll: drop 'alpha_en_mask' from IPQ5018 PLL config
      clk: qcom: apps-ipq-pll: drop 'alpha_en_mask' from IPQ5332 PLL config
      clk: qcom: gcc-ipq6018: remove alpha values from NSS Crypto PLL's config
      clk: qcom: dispcc-qcm2290: remove alpha values from disp_cc_pll0_config
      clk: qcom: dispcc-sm6115: remove alpha values from disp_cc_pll0_config

 drivers/clk/qcom/apss-ipq-pll.c   | 3 +--
 drivers/clk/qcom/dispcc-qcm2290.c | 2 --
 drivers/clk/qcom/dispcc-sm6115.c  | 2 --
 drivers/clk/qcom/gcc-ipq6018.c    | 4 +---
 4 files changed, 2 insertions(+), 9 deletions(-)
---
base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
change-id: 20241021-alpha-mode-cleanup-661b544644d7

Best regards,
-- 
Gabor Juhos <j4g8y7@gmail.com>

[PATCH v2 1/5] clk: qcom: apss-ipq-pll: drop 'alpha_en_mask' from IPQ5018 PLL config

[Gabor Juhos]

Since neither 'alpha' nor 'alpha_hi' is defined in the configuration,
those will be initialized with zero values  implicitly. By using zero
alpha values, the output rate of the PLL will be the same whether
alpha mode is enabled or not.

Remove the superfluous initialization of the 'alpha_en_mask' member
to make it clear that enabling alpha mode is not required to get the
desired output rate.

Despite that enabling alpha mode is not needed for the initial
configuration, the set_rate() op might require that it is enabled
already. In this particular case however, the
clk_alpha_pll_stromer_set_rate() function will get reset the
ALPHA_EN bit when the PLL's rate changes, so dropping 'alpha_en_mask'
is safe.

No functional changes, the initial rate of the PLL is the same both
before and after the patch.

Tested on TP-Link Archer AX55 v1 (IPQ5018).

Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - extend the commit message to indicate that dropping 'alpha_en_mask' is safe
  - Link to v1: https://lore.kernel.org/r/20241021-alpha-mode-cleanup-v1-1-55df8ed73645@gmail.com
---
 drivers/clk/qcom/apss-ipq-pll.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index e8632db2c542806e9527a22b54fe169e3e398a7a..dec2a5019cc77bf60142a86453883e336afc860f 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -73,7 +73,6 @@ static const struct alpha_pll_config ipq5018_pll_config = {
 	.main_output_mask = BIT(0),
 	.aux_output_mask = BIT(1),
 	.early_output_mask = BIT(3),
-	.alpha_en_mask = BIT(24),
 	.status_val = 0x3,
 	.status_mask = GENMASK(10, 8),
 	.lock_det = BIT(2),

-- 
2.47.0

Re: [PATCH v2 1/5] clk: qcom: apss-ipq-pll: drop 'alpha_en_mask' from IPQ5018 PLL config

[Dmitry Baryshkov]

On Mon, Oct 28, 2024 at 07:48:15PM +0100, Gabor Juhos wrote:
> Since neither 'alpha' nor 'alpha_hi' is defined in the configuration,
> those will be initialized with zero values  implicitly. By using zero
> alpha values, the output rate of the PLL will be the same whether
> alpha mode is enabled or not.
> 
> Remove the superfluous initialization of the 'alpha_en_mask' member
> to make it clear that enabling alpha mode is not required to get the
> desired output rate.
> 
> Despite that enabling alpha mode is not needed for the initial
> configuration, the set_rate() op might require that it is enabled
> already. In this particular case however, the
> clk_alpha_pll_stromer_set_rate() function will get reset the
> ALPHA_EN bit when the PLL's rate changes, so dropping 'alpha_en_mask'
> is safe.
> 
> No functional changes, the initial rate of the PLL is the same both
> before and after the patch.
> 
> Tested on TP-Link Archer AX55 v1 (IPQ5018).
> 
> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
> ---
> Changes in v2:
>   - extend the commit message to indicate that dropping 'alpha_en_mask' is safe
>   - Link to v1: https://lore.kernel.org/r/20241021-alpha-mode-cleanup-v1-1-55df8ed73645@gmail.com
> ---
>  drivers/clk/qcom/apss-ipq-pll.c | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

[PATCH v2 2/5] clk: qcom: apps-ipq-pll: drop 'alpha_en_mask' from IPQ5332 PLL config

[Gabor Juhos]

Since neither 'alpha' nor 'alpha_hi' is defined in the configuration,
those will be initialized with zero values implicitly. By using zero
alpha values, the output rate of the PLL will be the same whether
alpha mode is enabled or not.

Remove the superfluous initialization of the 'alpha_en_mask' member
to make it clear that enabling alpha mode is not required to get the
desired output rate.

Despite that enabling alpha mode is not needed for the initial
configuration, the set_rate() op might require that it is enabled
already. In this particular case however, the
clk_alpha_pll_stromer_plus_set_rate() function will get reset the
ALPHA_EN bit when the PLL's rate changes, so dropping 'alpha_en_mask'
is safe.

While at it, also add a comment to indicate the frequency the PLL
runs at with the current configuration.

No functional changes, the initial rate of the PLL is the same both
before and after the patch.

Tested on Xiaomi Router BE3600 2.5G (IPQ5312, out-of-tree board).

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - extend the commit message to indicate that dropping 'alpha_en_mask' is safe
  - add RB tag from Dmitry
  - Link to v1: https://lore.kernel.org/r/20241021-alpha-mode-cleanup-v1-2-55df8ed73645@gmail.com
---
 drivers/clk/qcom/apss-ipq-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index dec2a5019cc77bf60142a86453883e336afc860f..d6c1aea7e9e1e50a8d7561ce352feac4e76fb1e3 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -79,13 +79,13 @@ static const struct alpha_pll_config ipq5018_pll_config = {
 	.test_ctl_hi_val = 0x00400003,
 };
 
+/* 1.080 GHz configuration */
 static const struct alpha_pll_config ipq5332_pll_config = {
 	.l = 0x2d,
 	.config_ctl_val = 0x4001075b,
 	.main_output_mask = BIT(0),
 	.aux_output_mask = BIT(1),
 	.early_output_mask = BIT(3),
-	.alpha_en_mask = BIT(24),
 	.status_val = 0x3,
 	.status_mask = GENMASK(10, 8),
 	.lock_det = BIT(2),

-- 
2.47.0

[PATCH v2 3/5] clk: qcom: gcc-ipq6018: remove alpha values from NSS Crypto PLL's config

[Gabor Juhos]

Since both the 'alpha' and 'alpha_hi' members of the configuration is
initialized with zero values, the output rate of the PLL will be the
same whether alpha mode is enabled or not.

Remove the initialization of the alpha* members to make it clear that
alpha mode is not required to get the desired output rate.

Despite that enabling alpha mode is not needed for the initial
configuration, the set_rate() op might require that it is enabled
already. In this particular case however, the clk_alpha_pll_set_rate()
function will get reset the ALPHA_EN bit when the PLL's rate changes,
so dropping 'alpha_en_mask' is safe.

While at it, also add a comment to indicate the frequency the PLL runs
at with the current configuration.

No functional changes, the PLL runs at 1.2 GHz both before and after
the change.

Tested on Xiaomi Mi Router AX1800 (IPQ6018, out-of-tree board).

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - extend the commit message to indicate that dropping 'alpha_en_mask' is safe
  - add RB tag from Dmitry
  - Link to v1: https://lore.kernel.org/r/20241021-alpha-mode-cleanup-v1-3-55df8ed73645@gmail.com
---
 drivers/clk/qcom/gcc-ipq6018.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-ipq6018.c b/drivers/clk/qcom/gcc-ipq6018.c
index ab0f7fc665a9790dd8edba0cf4b86c5c672a337d..d861191b0c85ccc105ac0e62d7a68210c621fc13 100644
--- a/drivers/clk/qcom/gcc-ipq6018.c
+++ b/drivers/clk/qcom/gcc-ipq6018.c
@@ -4194,10 +4194,9 @@ static const struct alpha_pll_config ubi32_pll_config = {
 	.test_ctl_hi_val = 0x4000,
 };
 
+/* 1200 MHz configuration */
 static const struct alpha_pll_config nss_crypto_pll_config = {
 	.l = 0x32,
-	.alpha = 0x0,
-	.alpha_hi = 0x0,
 	.config_ctl_val = 0x4001055b,
 	.main_output_mask = BIT(0),
 	.pre_div_val = 0x0,
@@ -4206,7 +4205,6 @@ static const struct alpha_pll_config nss_crypto_pll_config = {
 	.post_div_mask = GENMASK(11, 8),
 	.vco_mask = GENMASK(21, 20),
 	.vco_val = 0x0,
-	.alpha_en_mask = BIT(24),
 };
 
 static struct clk_hw *gcc_ipq6018_hws[] = {

-- 
2.47.0

[PATCH v2 4/5] clk: qcom: dispcc-qcm2290: remove alpha values from disp_cc_pll0_config

[Gabor Juhos]

Since both the 'alpha' and 'alpha_hi' members of the configuration is
initialized (the latter is implicitly) with zero values, the output
rate of the PLL will be the same whether alpha mode is enabled or not.

Remove the initialization of the alpha* members to make it clear that
the alpha mode is not required to get the desired output rate.

Despite that enabling alpha mode is not needed for the initial
configuration, the set_rate() op might require that it is enabled
already. In this particular case however, the clk_alpha_pll_set_rate()
function will get reset the ALPHA_EN bit when the PLL's rate changes,
so dropping 'alpha_en_mask' is safe.

No functional changes intended, compile tested only.

Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - extend the commit message to indicate that dropping 'alpha_en_mask' is safe
  - Link to v1: https://lore.kernel.org/r/20241021-alpha-mode-cleanup-v1-4-55df8ed73645@gmail.com
---
 drivers/clk/qcom/dispcc-qcm2290.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
index 449ffea2295d3760f40abe8b1195e9022f46a9b0..d7bb1399e1022afc68e45ee335d615d4a5be5add 100644
--- a/drivers/clk/qcom/dispcc-qcm2290.c
+++ b/drivers/clk/qcom/dispcc-qcm2290.c
@@ -40,8 +40,6 @@ static const struct pll_vco spark_vco[] = {
 /* 768MHz configuration */
 static const struct alpha_pll_config disp_cc_pll0_config = {
 	.l = 0x28,
-	.alpha = 0x0,
-	.alpha_en_mask = BIT(24),
 	.vco_val = 0x2 << 20,
 	.vco_mask = GENMASK(21, 20),
 	.main_output_mask = BIT(0),

-- 
2.47.0

Re: [PATCH v2 4/5] clk: qcom: dispcc-qcm2290: remove alpha values from disp_cc_pll0_config

[Dmitry Baryshkov]

On Mon, Oct 28, 2024 at 07:48:18PM +0100, Gabor Juhos wrote:
> Since both the 'alpha' and 'alpha_hi' members of the configuration is
> initialized (the latter is implicitly) with zero values, the output
> rate of the PLL will be the same whether alpha mode is enabled or not.
> 
> Remove the initialization of the alpha* members to make it clear that
> the alpha mode is not required to get the desired output rate.
> 
> Despite that enabling alpha mode is not needed for the initial
> configuration, the set_rate() op might require that it is enabled
> already. In this particular case however, the clk_alpha_pll_set_rate()
> function will get reset the ALPHA_EN bit when the PLL's rate changes,
> so dropping 'alpha_en_mask' is safe.
> 
> No functional changes intended, compile tested only.
> 
> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
> ---
> Changes in v2:
>   - extend the commit message to indicate that dropping 'alpha_en_mask' is safe
>   - Link to v1: https://lore.kernel.org/r/20241021-alpha-mode-cleanup-v1-4-55df8ed73645@gmail.com
> ---
>  drivers/clk/qcom/dispcc-qcm2290.c | 2 --
>  1 file changed, 2 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

[PATCH v2 5/5] clk: qcom: dispcc-sm6115: remove alpha values from disp_cc_pll0_config

[Gabor Juhos]

Since both the 'alpha' and 'alpha_hi' members of the configuration is
initialized (the latter is implicitly) with zero values, the output
rate of the PLL will be the same whether alpha mode is enabled or not.

Remove the initialization of the alpha* members to make it clear that
the alpha mode is not required to get the desired output rate.

Despite that enabling alpha mode is not needed for the initial
configuration, the set_rate() op might require that it is enabled
already. In this particular case however, the clk_alpha_pll_set_rate()
function will get reset the ALPHA_EN bit when the PLL's rate changes,
so dropping 'alpha_en_mask' is safe.

No functional changes intended, compile tested only.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - extend the commit message to indicate that dropping 'alpha_en_mask' is safe
  - add RB tag from Dmitry
  - Link to v1: https://lore.kernel.org/r/20241021-alpha-mode-cleanup-v1-5-55df8ed73645@gmail.com
---
 drivers/clk/qcom/dispcc-sm6115.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/clk/qcom/dispcc-sm6115.c b/drivers/clk/qcom/dispcc-sm6115.c
index 939887f82ecc3da21a5f26168c3161aa8cfeb3cb..2b236d52b29fe72b8979da85c8bd4bfd1db54c0b 100644
--- a/drivers/clk/qcom/dispcc-sm6115.c
+++ b/drivers/clk/qcom/dispcc-sm6115.c
@@ -48,8 +48,6 @@ static const struct pll_vco spark_vco[] = {
 /* 768MHz configuration */
 static const struct alpha_pll_config disp_cc_pll0_config = {
 	.l = 0x28,
-	.alpha = 0x0,
-	.alpha_en_mask = BIT(24),
 	.vco_val = 0x2 << 20,
 	.vco_mask = GENMASK(21, 20),
 	.main_output_mask = BIT(0),

-- 
2.47.0

Re: [PATCH v2 0/5] clk: qcom: remove superfluous alpha settings from PLL configs

[Bjorn Andersson]

On Mon, 28 Oct 2024 19:48:14 +0100, Gabor Juhos wrote:
> There are several alpha PLL  configurations, where the alpha mode
> is enabled even if it is not strictly required in order to get the
> desired output rate of the PLL. This small series removes those
> superfluous settings.
> 
> 

Applied, thanks!

[1/5] clk: qcom: apss-ipq-pll: drop 'alpha_en_mask' from IPQ5018 PLL config
      commit: 3e632d9c33d8a4f0d8f98e69b56bd2f2145d91c4
[2/5] clk: qcom: apps-ipq-pll: drop 'alpha_en_mask' from IPQ5332 PLL config
      commit: 41c5ebfb9b4b42b1479f16a50d96146fa945abce
[3/5] clk: qcom: gcc-ipq6018: remove alpha values from NSS Crypto PLL's config
      commit: 5d11fd2a919b153c68cda42c175b4e45668654a9
[4/5] clk: qcom: dispcc-qcm2290: remove alpha values from disp_cc_pll0_config
      commit: b9286a91b59fe9c7f8e231fce8e51df3b1442f6b
[5/5] clk: qcom: dispcc-sm6115: remove alpha values from disp_cc_pll0_config
      commit: 320f7a476c5fe79f50d00c75debfd9f63a0e713f

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>