This series contains a few patches to perform some cleanup in the
apss-ipq-pll driver.
The set is based on v6.8 and it depends on the following patches:
- "clk: qcom: apss-ipq-pll: use stromer ops for IPQ5018 to fix boot failure"
Link: https://lore.kernel.org/r/[email protected]
- "clk: qcom: clk-alpha-pll: Stromer register cleanup"
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Gabor Juhos <[email protected]>
---
Gabor Juhos (5):
clk: qcom: apss-ipq-pll: reuse Stromer reg offsets from 'clk_alpha_pll_regs'
clk: qcom: apss-ipq-pll: use an 1-D array for Huayra pll register offsets
clk: qcom: apss-ipq-pll: remove 'pll_type' field from struct 'apss_pll_data'
clk: qcom: apss-ipq-pll: constify match data structures
clk: qcom: apss-ipq-pll: constify clk_init_data structures
drivers/clk/qcom/apss-ipq-pll.c | 70 +++++++++++++++--------------------------
1 file changed, 25 insertions(+), 45 deletions(-)
---
base-commit: 9b65bf93985e8bd5bc5476beef59ba56f3f99697
change-id: 20240315-apss-ipq-pll-cleanup-a1e99af9d854
Best regards,
--
Gabor Juhos <[email protected]>
The 'ipq_pll_offsets' is defined as a two-dimensional array, but it
contains a sole element only so convert it to an one-dimensional
array. Also, rename the variable to better reflect that it is used
for the Huayra PLLs.
No functional changes.
Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/clk/qcom/apss-ipq-pll.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index ed3e6405f99cb..f5c7eaf8db374 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -13,22 +13,20 @@
* are different from the one mentioned in the clk-alpha-pll.c, since the
* PLL is specific to APSS, so lets the define the same.
*/
-static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
- [CLK_ALPHA_PLL_TYPE_HUAYRA] = {
- [PLL_OFF_L_VAL] = 0x08,
- [PLL_OFF_ALPHA_VAL] = 0x10,
- [PLL_OFF_USER_CTL] = 0x18,
- [PLL_OFF_CONFIG_CTL] = 0x20,
- [PLL_OFF_CONFIG_CTL_U] = 0x24,
- [PLL_OFF_STATUS] = 0x28,
- [PLL_OFF_TEST_CTL] = 0x30,
- [PLL_OFF_TEST_CTL_U] = 0x34,
- },
+static const u8 ipq_pll_huayra_regs[PLL_OFF_MAX_REGS] = {
+ [PLL_OFF_L_VAL] = 0x08,
+ [PLL_OFF_ALPHA_VAL] = 0x10,
+ [PLL_OFF_USER_CTL] = 0x18,
+ [PLL_OFF_CONFIG_CTL] = 0x20,
+ [PLL_OFF_CONFIG_CTL_U] = 0x24,
+ [PLL_OFF_STATUS] = 0x28,
+ [PLL_OFF_TEST_CTL] = 0x30,
+ [PLL_OFF_TEST_CTL_U] = 0x34,
};
static struct clk_alpha_pll ipq_pll_huayra = {
.offset = 0x0,
- .regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
+ .regs = ipq_pll_huayra_regs,
.flags = SUPPORTS_DYNAMIC_UPDATE,
.clkr = {
.enable_reg = 0x0,
--
2.44.0
The value of the 'pll_type' field of 'struct apps_pll_data'
is used only by the probe function to decide which config
function should be called for the actual PLL. However this
can be derived also from the 'pll' field which makes the
'pll_type' field redundant.
Additionally, the CLK_ALPHA_PLL_TYPE_* enumeration values
are meant to be used as indices to the 'clk_alpha_pll_regs'
array so using those to define the pll type in this driver
is misleading anyway.
Change the probe function to use the 'pll' field to determine
the configuration function to be used, and remove the
'pll_type' field to simplify the code.
No functional changes.
Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/clk/qcom/apss-ipq-pll.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index f5c7eaf8db374..fb015385dae4b 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -147,37 +147,31 @@ static const struct alpha_pll_config ipq9574_pll_config = {
};
struct apss_pll_data {
- int pll_type;
struct clk_alpha_pll *pll;
const struct alpha_pll_config *pll_config;
};
static const struct apss_pll_data ipq5018_pll_data = {
- .pll_type = CLK_ALPHA_PLL_TYPE_STROMER,
.pll = &ipq_pll_stromer,
.pll_config = &ipq5018_pll_config,
};
static struct apss_pll_data ipq5332_pll_data = {
- .pll_type = CLK_ALPHA_PLL_TYPE_STROMER_PLUS,
.pll = &ipq_pll_stromer_plus,
.pll_config = &ipq5332_pll_config,
};
static struct apss_pll_data ipq8074_pll_data = {
- .pll_type = CLK_ALPHA_PLL_TYPE_HUAYRA,
.pll = &ipq_pll_huayra,
.pll_config = &ipq8074_pll_config,
};
static struct apss_pll_data ipq6018_pll_data = {
- .pll_type = CLK_ALPHA_PLL_TYPE_HUAYRA,
.pll = &ipq_pll_huayra,
.pll_config = &ipq6018_pll_config,
};
static struct apss_pll_data ipq9574_pll_data = {
- .pll_type = CLK_ALPHA_PLL_TYPE_HUAYRA,
.pll = &ipq_pll_huayra,
.pll_config = &ipq9574_pll_config,
};
@@ -210,10 +204,10 @@ static int apss_ipq_pll_probe(struct platform_device *pdev)
if (!data)
return -ENODEV;
- if (data->pll_type == CLK_ALPHA_PLL_TYPE_HUAYRA)
+ if (data->pll == &ipq_pll_huayra)
clk_alpha_pll_configure(data->pll, regmap, data->pll_config);
- else if (data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER ||
- data->pll_type == CLK_ALPHA_PLL_TYPE_STROMER_PLUS)
+ else if (data->pll == &ipq_pll_stromer ||
+ data->pll == &ipq_pll_stromer_plus)
clk_stromer_pll_configure(data->pll, regmap, data->pll_config);
ret = devm_clk_register_regmap(dev, &data->pll->clkr);
--
2.44.0
The match data structures are used only by the apss_ipq_pll_probe()
function and are never modified so mark those as constant.
No functional changes.
Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/clk/qcom/apss-ipq-pll.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index fb015385dae4b..6ee71ed6baed1 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -156,22 +156,22 @@ static const struct apss_pll_data ipq5018_pll_data = {
.pll_config = &ipq5018_pll_config,
};
-static struct apss_pll_data ipq5332_pll_data = {
+static const struct apss_pll_data ipq5332_pll_data = {
.pll = &ipq_pll_stromer_plus,
.pll_config = &ipq5332_pll_config,
};
-static struct apss_pll_data ipq8074_pll_data = {
+static const struct apss_pll_data ipq8074_pll_data = {
.pll = &ipq_pll_huayra,
.pll_config = &ipq8074_pll_config,
};
-static struct apss_pll_data ipq6018_pll_data = {
+static const struct apss_pll_data ipq6018_pll_data = {
.pll = &ipq_pll_huayra,
.pll_config = &ipq6018_pll_config,
};
-static struct apss_pll_data ipq9574_pll_data = {
+static const struct apss_pll_data ipq9574_pll_data = {
.pll = &ipq_pll_huayra,
.pll_config = &ipq9574_pll_config,
};
--
2.44.0
The clk_init_data structures are never modified, so add const
qualifier to the ones where it is missing.
No functional changes.
Signed-off-by: Gabor Juhos <[email protected]>
---
drivers/clk/qcom/apss-ipq-pll.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index 6ee71ed6baed1..6cf093f7f91eb 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -31,7 +31,7 @@ static struct clk_alpha_pll ipq_pll_huayra = {
.clkr = {
.enable_reg = 0x0,
.enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
+ .hw.init = &(const struct clk_init_data) {
.name = "a53pll",
.parent_data = &(const struct clk_parent_data) {
.fw_name = "xo",
@@ -71,7 +71,7 @@ static struct clk_alpha_pll ipq_pll_stromer_plus = {
.clkr = {
.enable_reg = 0x0,
.enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
+ .hw.init = &(const struct clk_init_data) {
.name = "a53pll",
.parent_data = &(const struct clk_parent_data) {
.fw_name = "xo",
--
2.44.0
The register offset array defined locally for the
CLK_ALPHA_PLL_TYPE_STROMER_PLUS is the same as the
entry defined for CLK_ALPHA_PLL_TYPE_STROMER in the
'clk_alpha_pll_regs' array.
To avoid code duplication, remove the local definition
and use the global one instead.
No functional changes.
Signed-off-by: Gabor Juhos <[email protected]>
---
Depends on the following patches:
- "clk: qcom: apss-ipq-pll: use stromer ops for IPQ5018 to fix boot failure"
Link: https://lore.kernel.org/r/[email protected]
- "clk: qcom: clk-alpha-pll: Stromer register cleanup"
Link: https://lore.kernel.org/r/[email protected]
---
drivers/clk/qcom/apss-ipq-pll.c | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
index dfffec2f06ae7..ed3e6405f99cb 100644
--- a/drivers/clk/qcom/apss-ipq-pll.c
+++ b/drivers/clk/qcom/apss-ipq-pll.c
@@ -24,17 +24,6 @@ static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
[PLL_OFF_TEST_CTL] = 0x30,
[PLL_OFF_TEST_CTL_U] = 0x34,
},
- [CLK_ALPHA_PLL_TYPE_STROMER_PLUS] = {
- [PLL_OFF_L_VAL] = 0x08,
- [PLL_OFF_ALPHA_VAL] = 0x10,
- [PLL_OFF_ALPHA_VAL_U] = 0x14,
- [PLL_OFF_USER_CTL] = 0x18,
- [PLL_OFF_USER_CTL_U] = 0x1c,
- [PLL_OFF_CONFIG_CTL] = 0x20,
- [PLL_OFF_STATUS] = 0x28,
- [PLL_OFF_TEST_CTL] = 0x30,
- [PLL_OFF_TEST_CTL_U] = 0x34,
- },
};
static struct clk_alpha_pll ipq_pll_huayra = {
@@ -57,12 +46,7 @@ static struct clk_alpha_pll ipq_pll_huayra = {
static struct clk_alpha_pll ipq_pll_stromer = {
.offset = 0x0,
- /*
- * Reuse CLK_ALPHA_PLL_TYPE_STROMER_PLUS register offsets.
- * Although this is a bit confusing, but the offset values
- * are correct nevertheless.
- */
- .regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_STROMER_PLUS],
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_STROMER],
.flags = SUPPORTS_DYNAMIC_UPDATE,
.clkr = {
.enable_reg = 0x0,
@@ -80,7 +64,11 @@ static struct clk_alpha_pll ipq_pll_stromer = {
static struct clk_alpha_pll ipq_pll_stromer_plus = {
.offset = 0x0,
- .regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_STROMER_PLUS],
+ /*
+ * The register offsets of the Stromer Plus PLL used in IPQ5332
+ * are the same as the Stromer PLL's offsets.
+ */
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_STROMER],
.flags = SUPPORTS_DYNAMIC_UPDATE,
.clkr = {
.enable_reg = 0x0,
--
2.44.0
On Mon, 18 Mar 2024 at 13:20, Gabor Juhos <[email protected]> wrote:
>
> The register offset array defined locally for the
> CLK_ALPHA_PLL_TYPE_STROMER_PLUS is the same as the
> entry defined for CLK_ALPHA_PLL_TYPE_STROMER in the
> 'clk_alpha_pll_regs' array.
>
> To avoid code duplication, remove the local definition
> and use the global one instead.
>
> No functional changes.
>
> Signed-off-by: Gabor Juhos <[email protected]>
> ---
> Depends on the following patches:
> - "clk: qcom: apss-ipq-pll: use stromer ops for IPQ5018 to fix boot failure"
> Link: https://lore.kernel.org/r/[email protected]
> - "clk: qcom: clk-alpha-pll: Stromer register cleanup"
> Link: https://lore.kernel.org/r/[email protected]
> ---
> drivers/clk/qcom/apss-ipq-pll.c | 24 ++++++------------------
> 1 file changed, 6 insertions(+), 18 deletions(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Mon, 18 Mar 2024 at 13:20, Gabor Juhos <[email protected]> wrote:
>
> The match data structures are used only by the apss_ipq_pll_probe()
> function and are never modified so mark those as constant.
>
> No functional changes.
>
> Signed-off-by: Gabor Juhos <[email protected]>
> ---
> drivers/clk/qcom/apss-ipq-pll.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Mon, 18 Mar 2024 at 13:20, Gabor Juhos <[email protected]> wrote:
>
> The 'ipq_pll_offsets' is defined as a two-dimensional array, but it
> contains a sole element only so convert it to an one-dimensional
> array. Also, rename the variable to better reflect that it is used
> for the Huayra PLLs.
>
> No functional changes.
>
> Signed-off-by: Gabor Juhos <[email protected]>
> ---
> drivers/clk/qcom/apss-ipq-pll.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/qcom/apss-ipq-pll.c b/drivers/clk/qcom/apss-ipq-pll.c
> index ed3e6405f99cb..f5c7eaf8db374 100644
> --- a/drivers/clk/qcom/apss-ipq-pll.c
> +++ b/drivers/clk/qcom/apss-ipq-pll.c
> @@ -13,22 +13,20 @@
> * are different from the one mentioned in the clk-alpha-pll.c, since the
> * PLL is specific to APSS, so lets the define the same.
> */
> -static const u8 ipq_pll_offsets[][PLL_OFF_MAX_REGS] = {
> - [CLK_ALPHA_PLL_TYPE_HUAYRA] = {
> - [PLL_OFF_L_VAL] = 0x08,
> - [PLL_OFF_ALPHA_VAL] = 0x10,
> - [PLL_OFF_USER_CTL] = 0x18,
> - [PLL_OFF_CONFIG_CTL] = 0x20,
> - [PLL_OFF_CONFIG_CTL_U] = 0x24,
> - [PLL_OFF_STATUS] = 0x28,
> - [PLL_OFF_TEST_CTL] = 0x30,
> - [PLL_OFF_TEST_CTL_U] = 0x34,
> - },
> +static const u8 ipq_pll_huayra_regs[PLL_OFF_MAX_REGS] = {
> + [PLL_OFF_L_VAL] = 0x08,
> + [PLL_OFF_ALPHA_VAL] = 0x10,
> + [PLL_OFF_USER_CTL] = 0x18,
> + [PLL_OFF_CONFIG_CTL] = 0x20,
> + [PLL_OFF_CONFIG_CTL_U] = 0x24,
> + [PLL_OFF_STATUS] = 0x28,
> + [PLL_OFF_TEST_CTL] = 0x30,
> + [PLL_OFF_TEST_CTL_U] = 0x34,
> };
Can you please move this to clk_alpha_pll? We can then drop it from
clk-cbf-8996.c too.
>
> static struct clk_alpha_pll ipq_pll_huayra = {
> .offset = 0x0,
> - .regs = ipq_pll_offsets[CLK_ALPHA_PLL_TYPE_HUAYRA],
> + .regs = ipq_pll_huayra_regs,
> .flags = SUPPORTS_DYNAMIC_UPDATE,
> .clkr = {
> .enable_reg = 0x0,
>
> --
> 2.44.0
>
>
--
With best wishes
Dmitry
2024. 03. 18. 15:16 keltezéssel, Dmitry Baryshkov írta:
..
>> +static const u8 ipq_pll_huayra_regs[PLL_OFF_MAX_REGS] = {
>> + [PLL_OFF_L_VAL] = 0x08,
>> + [PLL_OFF_ALPHA_VAL] = 0x10,
>> + [PLL_OFF_USER_CTL] = 0x18,
>> + [PLL_OFF_CONFIG_CTL] = 0x20,
>> + [PLL_OFF_CONFIG_CTL_U] = 0x24,
>> + [PLL_OFF_STATUS] = 0x28,
>> + [PLL_OFF_TEST_CTL] = 0x30,
>> + [PLL_OFF_TEST_CTL_U] = 0x34,
>> };
>
> Can you please move this to clk_alpha_pll? We can then drop it from
> clk-cbf-8996.c too.
Sure, I can do that. By any chance, do you have a suggestion for the name of the
new enum value to be used in the clk_alpha_pll_regs array?
CLK_ALPHA_PLL_TYPE_HUAYRA_IPQ seems too generic, and it would be a bit
misleading using that for MSM8996 CBF.
CLK_ALPHA_PLL_TYPE_HUAYRA_IPQ6018_A53 is quite long and it is also misleading.
Maybe we could use CLK_ALPHA_PLL_TYPE_IPQ6018_A53 which is short and unique
enough and we could add an alias for that like CLK_ALPHA_PLL_TYPE_MSM8996_CBF or
something similar.
Regards,
Gabor
On Mon, 18 Mar 2024 at 23:23, Gabor Juhos <[email protected]> wrote:
>
> 2024. 03. 18. 15:16 keltezéssel, Dmitry Baryshkov írta:
>
> ...
>
> >> +static const u8 ipq_pll_huayra_regs[PLL_OFF_MAX_REGS] = {
> >> + [PLL_OFF_L_VAL] = 0x08,
> >> + [PLL_OFF_ALPHA_VAL] = 0x10,
> >> + [PLL_OFF_USER_CTL] = 0x18,
> >> + [PLL_OFF_CONFIG_CTL] = 0x20,
> >> + [PLL_OFF_CONFIG_CTL_U] = 0x24,
> >> + [PLL_OFF_STATUS] = 0x28,
> >> + [PLL_OFF_TEST_CTL] = 0x30,
> >> + [PLL_OFF_TEST_CTL_U] = 0x34,
> >> };
> >
> > Can you please move this to clk_alpha_pll? We can then drop it from
> > clk-cbf-8996.c too.
>
> Sure, I can do that. By any chance, do you have a suggestion for the name of the
> new enum value to be used in the clk_alpha_pll_regs array?
>
> CLK_ALPHA_PLL_TYPE_HUAYRA_IPQ seems too generic, and it would be a bit
> misleading using that for MSM8996 CBF.
>
> CLK_ALPHA_PLL_TYPE_HUAYRA_IPQ6018_A53 is quite long and it is also misleading.
>
> Maybe we could use CLK_ALPHA_PLL_TYPE_IPQ6018_A53 which is short and unique
> enough and we could add an alias for that like CLK_ALPHA_PLL_TYPE_MSM8996_CBF or
> something similar.
HUAYRA_APSS ?
>
> Regards,
> Gabor
>
--
With best wishes
Dmitry
2024. 03. 19. 1:09 keltezéssel, Dmitry Baryshkov írta:
> On Mon, 18 Mar 2024 at 23:23, Gabor Juhos <[email protected]> wrote:
>>
>> 2024. 03. 18. 15:16 keltezéssel, Dmitry Baryshkov írta:
>>
>> ...
>>
>>>> +static const u8 ipq_pll_huayra_regs[PLL_OFF_MAX_REGS] = {
>>>> + [PLL_OFF_L_VAL] = 0x08,
>>>> + [PLL_OFF_ALPHA_VAL] = 0x10,
>>>> + [PLL_OFF_USER_CTL] = 0x18,
>>>> + [PLL_OFF_CONFIG_CTL] = 0x20,
>>>> + [PLL_OFF_CONFIG_CTL_U] = 0x24,
>>>> + [PLL_OFF_STATUS] = 0x28,
>>>> + [PLL_OFF_TEST_CTL] = 0x30,
>>>> + [PLL_OFF_TEST_CTL_U] = 0x34,
>>>> };
>>>
>>> Can you please move this to clk_alpha_pll? We can then drop it from
>>> clk-cbf-8996.c too.
>>
>> Sure, I can do that. By any chance, do you have a suggestion for the name of the
>> new enum value to be used in the clk_alpha_pll_regs array?
>>
>> CLK_ALPHA_PLL_TYPE_HUAYRA_IPQ seems too generic, and it would be a bit
>> misleading using that for MSM8996 CBF.
>>
>> CLK_ALPHA_PLL_TYPE_HUAYRA_IPQ6018_A53 is quite long and it is also misleading.
>>
>> Maybe we could use CLK_ALPHA_PLL_TYPE_IPQ6018_A53 which is short and unique
>> enough and we could add an alias for that like CLK_ALPHA_PLL_TYPE_MSM8996_CBF or
>> something similar.
>
> HUAYRA_APSS ?
Ok, sounds good.