diff mbox series

[V3,3/8] clk: qcom: Add A53 PLL support for ipq6018 devices

Message ID 1586832922-29191-4-git-send-email-sivaprak@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Add APSS clock controller support for IPQ6018 | expand

Commit Message

Sivaprakash Murugesan April 14, 2020, 2:55 a.m. UTC
The CPUs on Qualcomm IPQ6018 platform is primarily clocked by A53 PLL.
This patch adds support for the A53 PLL on IPQ6018 devices which can
support CPU frequencies above 1Ghz.

Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 drivers/clk/qcom/a53-pll.c | 136 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 111 insertions(+), 25 deletions(-)

Comments

Stephen Boyd April 22, 2020, 9 a.m. UTC | #1
Quoting Sivaprakash Murugesan (2020-04-13 19:55:17)
> The CPUs on Qualcomm IPQ6018 platform is primarily clocked by A53 PLL.
> This patch adds support for the A53 PLL on IPQ6018 devices which can
> support CPU frequencies above 1Ghz.
> 
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
>  drivers/clk/qcom/a53-pll.c | 136 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 111 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
> index 45cfc57..a95351c 100644
> --- a/drivers/clk/qcom/a53-pll.c
> +++ b/drivers/clk/qcom/a53-pll.c
> @@ -11,11 +11,40 @@
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>

Why does this driver need to change to use of_device APIs?

>  
>  #include "clk-pll.h"
>  #include "clk-regmap.h"
> +#include "clk-alpha-pll.h"
>  
> -static const struct pll_freq_tbl a53pll_freq[] = {
> +struct a53_alpha_pll {
> +       struct alpha_pll_config *pll_config;
> +       struct clk_alpha_pll *pll;
> +};
> +
> +union a53pll {
> +       struct clk_pll *pll;
> +       struct a53_alpha_pll alpha_pll;
> +};
> +
> +struct a53pll_data {
> +#define PLL_IS_ALPHA BIT(0)
> +       u8 flags;
> +       union a53pll a53pll;

Why is there a union? Can't we have different clk ops for the two types
of PLLs and then use container_of to get it from the clk ops?

> +};
> +
> +static const u8 ipq_pll_offsets[] = {
> +       [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 struct pll_freq_tbl msm8996_a53pll_freq[] = {
>         {  998400000, 52, 0x0, 0x1, 0 },
>         { 1094400000, 57, 0x0, 0x1, 0 },
>         { 1152000000, 62, 0x0, 0x1, 0 },
> @@ -26,6 +55,64 @@ static const struct pll_freq_tbl a53pll_freq[] = {
>         { }
>  };
>  
> +static struct clk_pll msm8996_pll = {
> +       .mode_reg = 0x0,
> +       .l_reg = 0x04,
> +       .m_reg = 0x08,
> +       .n_reg = 0x0c,
> +       .config_reg = 0x14,
> +       .status_reg = 0x1c,
> +       .status_bit = 16,
> +       .freq_tbl = msm8996_a53pll_freq,
> +       .clkr.hw.init = &(struct clk_init_data){
> +               .name = "a53pll",
> +               .flags = CLK_IS_CRITICAL,
> +               .parent_data = &(const struct clk_parent_data){
> +                       .fw_name = "xo",
> +                       .name = "xo",
> +               },
> +               .num_parents = 1,
> +               .ops = &clk_pll_sr2_ops,
> +       },
> +};
> +
> +static struct clk_alpha_pll ipq6018_pll = {
> +       .offset = 0x0,
> +       .regs = ipq_pll_offsets,
> +       .flags = SUPPORTS_DYNAMIC_UPDATE,
> +       .clkr = {
> +               .enable_reg = 0x0,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "a53pll",
> +                       .flags = CLK_IS_CRITICAL,
> +                       .parent_data = &(const struct clk_parent_data){
> +                               .fw_name = "xo",
> +                       },
> +                       .num_parents = 1,
> +                       .ops = &clk_alpha_pll_huayra_ops,
> +               },
> +       },
> +};
> +
> +static struct alpha_pll_config ipq6018_pll_config = {

Can this be const?

> +       .l = 0x37,
> +       .config_ctl_val = 0x04141200,
> +       .config_ctl_hi_val = 0x0,
> +       .early_output_mask = BIT(3),
> +       .main_output_mask = BIT(0),
> +};
> +
> +static struct a53pll_data msm8996pll_data = {
> +       .a53pll.pll = &msm8996_pll,
> +};
> +
> +static struct a53pll_data ipq6018pll_data = {
> +       .flags = PLL_IS_ALPHA,
> +       .a53pll.alpha_pll.pll = &ipq6018_pll,
> +       .a53pll.alpha_pll.pll_config = &ipq6018_pll_config,
> +};
> +
>  static const struct regmap_config a53pll_regmap_config = {
>         .reg_bits               = 32,
>         .reg_stride             = 4,
> @@ -39,14 +126,16 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>         struct device *dev = &pdev->dev;
>         struct regmap *regmap;
>         struct resource *res;
> -       struct clk_pll *pll;
> +       const struct a53pll_data *pll_data;
> +       struct clk_regmap *clkr;
>         void __iomem *base;
> -       struct clk_init_data init = { };
>         int ret;
>  
> -       pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
> -       if (!pll)
> -               return -ENOMEM;
> +       pll_data = of_device_get_match_data(dev);

Use device_get_match_data() please.

> +       if (!pll_data) {
> +               dev_err(dev, "failed to get platform data\n");

No error message please.

> +               return -ENODEV;
> +       }
>  
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         base = devm_ioremap_resource(dev, res);
> @@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>         if (IS_ERR(regmap))
>                 return PTR_ERR(regmap);
>  
> -       pll->l_reg = 0x04;
> -       pll->m_reg = 0x08;
> -       pll->n_reg = 0x0c;
> -       pll->config_reg = 0x14;
> -       pll->mode_reg = 0x00;
> -       pll->status_reg = 0x1c;
> -       pll->status_bit = 16;
> -       pll->freq_tbl = a53pll_freq;
> -
> -       init.name = "a53pll";
> -       init.parent_names = (const char *[]){ "xo" };
> -       init.num_parents = 1;
> -       init.ops = &clk_pll_sr2_ops;
> -       init.flags = CLK_IS_CRITICAL;

Please document why a clk is critical.

> -       pll->clkr.hw.init = &init;
> -
> -       ret = devm_clk_register_regmap(dev, &pll->clkr);
> +       if (pll_data->flags & PLL_IS_ALPHA) {
> +               struct clk_alpha_pll *alpha_pll =
> +                       pll_data->a53pll.alpha_pll.pll;
> +               struct alpha_pll_config *alpha_pll_config =
> +                       pll_data->a53pll.alpha_pll.pll_config;
> +
> +               clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
> +               clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
> +       } else {
> +               clkr = &pll_data->a53pll.pll->clkr;
> +       }

Sorry, the design is confusing.
Sivaprakash Murugesan April 22, 2020, 10:44 a.m. UTC | #2
Hi Stephen,

On 4/22/2020 2:30 PM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-04-13 19:55:17)
>> The CPUs on Qualcomm IPQ6018 platform is primarily clocked by A53 PLL.
>> This patch adds support for the A53 PLL on IPQ6018 devices which can
>> support CPU frequencies above 1Ghz.
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>>   drivers/clk/qcom/a53-pll.c | 136 ++++++++++++++++++++++++++++++++++++---------
>>   1 file changed, 111 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
>> index 45cfc57..a95351c 100644
>> --- a/drivers/clk/qcom/a53-pll.c
>> +++ b/drivers/clk/qcom/a53-pll.c
>> @@ -11,11 +11,40 @@
>>   #include <linux/platform_device.h>
>>   #include <linux/regmap.h>
>>   #include <linux/module.h>
>> +#include <linux/of_device.h>
> Why does this driver need to change to use of_device APIs?
we can use devm APIs and avoid this in next patch.
>
>>   
>>   #include "clk-pll.h"
>>   #include "clk-regmap.h"
>> +#include "clk-alpha-pll.h"
>>   
>> -static const struct pll_freq_tbl a53pll_freq[] = {
>> +struct a53_alpha_pll {
>> +       struct alpha_pll_config *pll_config;
>> +       struct clk_alpha_pll *pll;
>> +};
>> +
>> +union a53pll {
>> +       struct clk_pll *pll;
>> +       struct a53_alpha_pll alpha_pll;
>> +};
>> +
>> +struct a53pll_data {
>> +#define PLL_IS_ALPHA BIT(0)
>> +       u8 flags;
>> +       union a53pll a53pll;
> Why is there a union? Can't we have different clk ops for the two types
> of PLLs and then use container_of to get it from the clk ops?
ok.
>
>> +};
>> +
>> +static const u8 ipq_pll_offsets[] = {
>> +       [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 struct pll_freq_tbl msm8996_a53pll_freq[] = {
>>          {  998400000, 52, 0x0, 0x1, 0 },
>>          { 1094400000, 57, 0x0, 0x1, 0 },
>>          { 1152000000, 62, 0x0, 0x1, 0 },
>> @@ -26,6 +55,64 @@ static const struct pll_freq_tbl a53pll_freq[] = {
>>          { }
>>   };
>>   
>> +static struct clk_pll msm8996_pll = {
>> +       .mode_reg = 0x0,
>> +       .l_reg = 0x04,
>> +       .m_reg = 0x08,
>> +       .n_reg = 0x0c,
>> +       .config_reg = 0x14,
>> +       .status_reg = 0x1c,
>> +       .status_bit = 16,
>> +       .freq_tbl = msm8996_a53pll_freq,
>> +       .clkr.hw.init = &(struct clk_init_data){
>> +               .name = "a53pll",
>> +               .flags = CLK_IS_CRITICAL,
>> +               .parent_data = &(const struct clk_parent_data){
>> +                       .fw_name = "xo",
>> +                       .name = "xo",
>> +               },
>> +               .num_parents = 1,
>> +               .ops = &clk_pll_sr2_ops,
>> +       },
>> +};
>> +
>> +static struct clk_alpha_pll ipq6018_pll = {
>> +       .offset = 0x0,
>> +       .regs = ipq_pll_offsets,
>> +       .flags = SUPPORTS_DYNAMIC_UPDATE,
>> +       .clkr = {
>> +               .enable_reg = 0x0,
>> +               .enable_mask = BIT(0),
>> +               .hw.init = &(struct clk_init_data){
>> +                       .name = "a53pll",
>> +                       .flags = CLK_IS_CRITICAL,
>> +                       .parent_data = &(const struct clk_parent_data){
>> +                               .fw_name = "xo",
>> +                       },
>> +                       .num_parents = 1,
>> +                       .ops = &clk_alpha_pll_huayra_ops,
>> +               },
>> +       },
>> +};
>> +
>> +static struct alpha_pll_config ipq6018_pll_config = {
> Can this be const?
yeah it can be. will fix up in next patch.
>
>> +       .l = 0x37,
>> +       .config_ctl_val = 0x04141200,
>> +       .config_ctl_hi_val = 0x0,
>> +       .early_output_mask = BIT(3),
>> +       .main_output_mask = BIT(0),
>> +};
>> +
>> +static struct a53pll_data msm8996pll_data = {
>> +       .a53pll.pll = &msm8996_pll,
>> +};
>> +
>> +static struct a53pll_data ipq6018pll_data = {
>> +       .flags = PLL_IS_ALPHA,
>> +       .a53pll.alpha_pll.pll = &ipq6018_pll,
>> +       .a53pll.alpha_pll.pll_config = &ipq6018_pll_config,
>> +};
>> +
>>   static const struct regmap_config a53pll_regmap_config = {
>>          .reg_bits               = 32,
>>          .reg_stride             = 4,
>> @@ -39,14 +126,16 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>>          struct device *dev = &pdev->dev;
>>          struct regmap *regmap;
>>          struct resource *res;
>> -       struct clk_pll *pll;
>> +       const struct a53pll_data *pll_data;
>> +       struct clk_regmap *clkr;
>>          void __iomem *base;
>> -       struct clk_init_data init = { };
>>          int ret;
>>   
>> -       pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
>> -       if (!pll)
>> -               return -ENOMEM;
>> +       pll_data = of_device_get_match_data(dev);
> Use device_get_match_data() please.
ok.
>
>> +       if (!pll_data) {
>> +               dev_err(dev, "failed to get platform data\n");
> No error message please.
ok.
>
>> +               return -ENODEV;
>> +       }
>>   
>>          res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>          base = devm_ioremap_resource(dev, res);
>> @@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>>          if (IS_ERR(regmap))
>>                  return PTR_ERR(regmap);
>>   
>> -       pll->l_reg = 0x04;
>> -       pll->m_reg = 0x08;
>> -       pll->n_reg = 0x0c;
>> -       pll->config_reg = 0x14;
>> -       pll->mode_reg = 0x00;
>> -       pll->status_reg = 0x1c;
>> -       pll->status_bit = 16;
>> -       pll->freq_tbl = a53pll_freq;
>> -
>> -       init.name = "a53pll";
>> -       init.parent_names = (const char *[]){ "xo" };
>> -       init.num_parents = 1;
>> -       init.ops = &clk_pll_sr2_ops;
>> -       init.flags = CLK_IS_CRITICAL;
> Please document why a clk is critical.
ok
>
>> -       pll->clkr.hw.init = &init;
>> -
>> -       ret = devm_clk_register_regmap(dev, &pll->clkr);
>> +       if (pll_data->flags & PLL_IS_ALPHA) {
>> +               struct clk_alpha_pll *alpha_pll =
>> +                       pll_data->a53pll.alpha_pll.pll;
>> +               struct alpha_pll_config *alpha_pll_config =
>> +                       pll_data->a53pll.alpha_pll.pll_config;
>> +
>> +               clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
>> +               clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
>> +       } else {
>> +               clkr = &pll_data->a53pll.pll->clkr;
>> +       }
> Sorry, the design is confusing.

The basic idea is to add support for various PLLs available to clock the 
A53 core.

if this messing up the code, can the alpha pll support be moved to a 
separate file?

It would be very helpful if you provide your input on this.
Stephen Boyd May 14, 2020, 8:40 p.m. UTC | #3
Quoting Sivaprakash Murugesan (2020-04-22 03:44:33)
> On 4/22/2020 2:30 PM, Stephen Boyd wrote:
> > Quoting Sivaprakash Murugesan (2020-04-13 19:55:17)
> >> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
> >> index 45cfc57..a95351c 100644
> >> --- a/drivers/clk/qcom/a53-pll.c
> >> +++ b/drivers/clk/qcom/a53-pll.c
> >> @@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
> >>          if (IS_ERR(regmap))
> >>                  return PTR_ERR(regmap);
> >>   
> >> -       pll->l_reg = 0x04;
> >> -       pll->m_reg = 0x08;
> >> -       pll->n_reg = 0x0c;
> >> -       pll->config_reg = 0x14;
> >> -       pll->mode_reg = 0x00;
> >> -       pll->status_reg = 0x1c;
> >> -       pll->status_bit = 16;
> >> -       pll->freq_tbl = a53pll_freq;
> >> -
> >> -       init.name = "a53pll";
> >> -       init.parent_names = (const char *[]){ "xo" };
> >> -       init.num_parents = 1;
> >> -       init.ops = &clk_pll_sr2_ops;
> >> -       init.flags = CLK_IS_CRITICAL;
> > Please document why a clk is critical.
> ok
> >
> >> -       pll->clkr.hw.init = &init;
> >> -
> >> -       ret = devm_clk_register_regmap(dev, &pll->clkr);
> >> +       if (pll_data->flags & PLL_IS_ALPHA) {
> >> +               struct clk_alpha_pll *alpha_pll =
> >> +                       pll_data->a53pll.alpha_pll.pll;
> >> +               struct alpha_pll_config *alpha_pll_config =
> >> +                       pll_data->a53pll.alpha_pll.pll_config;
> >> +
> >> +               clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
> >> +               clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
> >> +       } else {
> >> +               clkr = &pll_data->a53pll.pll->clkr;
> >> +       }
> > Sorry, the design is confusing.
> 
> The basic idea is to add support for various PLLs available to clock the 
> A53 core.
> 
> if this messing up the code, can the alpha pll support be moved to a 
> separate file?
> 
> It would be very helpful if you provide your input on this.

Isn't the alpha PLL support already in a different file? Is it sometimes
an alpha pll and other times it is something else?
Sivaprakash Murugesan May 24, 2020, 9:36 a.m. UTC | #4
On 5/15/2020 2:10 AM, Stephen Boyd wrote:
> Quoting Sivaprakash Murugesan (2020-04-22 03:44:33)
>> On 4/22/2020 2:30 PM, Stephen Boyd wrote:
>>> Quoting Sivaprakash Murugesan (2020-04-13 19:55:17)
>>>> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
>>>> index 45cfc57..a95351c 100644
>>>> --- a/drivers/clk/qcom/a53-pll.c
>>>> +++ b/drivers/clk/qcom/a53-pll.c
>>>> @@ -57,30 +146,26 @@ static int qcom_a53pll_probe(struct platform_device *pdev)
>>>>           if (IS_ERR(regmap))
>>>>                   return PTR_ERR(regmap);
>>>>    
>>>> -       pll->l_reg = 0x04;
>>>> -       pll->m_reg = 0x08;
>>>> -       pll->n_reg = 0x0c;
>>>> -       pll->config_reg = 0x14;
>>>> -       pll->mode_reg = 0x00;
>>>> -       pll->status_reg = 0x1c;
>>>> -       pll->status_bit = 16;
>>>> -       pll->freq_tbl = a53pll_freq;
>>>> -
>>>> -       init.name = "a53pll";
>>>> -       init.parent_names = (const char *[]){ "xo" };
>>>> -       init.num_parents = 1;
>>>> -       init.ops = &clk_pll_sr2_ops;
>>>> -       init.flags = CLK_IS_CRITICAL;
>>> Please document why a clk is critical.
>> ok
>>>> -       pll->clkr.hw.init = &init;
>>>> -
>>>> -       ret = devm_clk_register_regmap(dev, &pll->clkr);
>>>> +       if (pll_data->flags & PLL_IS_ALPHA) {
>>>> +               struct clk_alpha_pll *alpha_pll =
>>>> +                       pll_data->a53pll.alpha_pll.pll;
>>>> +               struct alpha_pll_config *alpha_pll_config =
>>>> +                       pll_data->a53pll.alpha_pll.pll_config;
>>>> +
>>>> +               clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
>>>> +               clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
>>>> +       } else {
>>>> +               clkr = &pll_data->a53pll.pll->clkr;
>>>> +       }
>>> Sorry, the design is confusing.
>> The basic idea is to add support for various PLLs available to clock the
>> A53 core.
>>
>> if this messing up the code, can the alpha pll support be moved to a
>> separate file?
>>
>> It would be very helpful if you provide your input on this.
> Isn't the alpha PLL support already in a different file? Is it sometimes
> an alpha pll and other times it is something else?

alpha pll for cpufreq is not yet available, and for ipq based devices it 
is alpha pll, and I guess

for other mobile based devices it is something else.

I have raised a patch set keeping the alpha pll as a separate file, 
could you please take a

look into it?
diff mbox series

Patch

diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
index 45cfc57..a95351c 100644
--- a/drivers/clk/qcom/a53-pll.c
+++ b/drivers/clk/qcom/a53-pll.c
@@ -11,11 +11,40 @@ 
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 
 #include "clk-pll.h"
 #include "clk-regmap.h"
+#include "clk-alpha-pll.h"
 
-static const struct pll_freq_tbl a53pll_freq[] = {
+struct a53_alpha_pll {
+	struct alpha_pll_config *pll_config;
+	struct clk_alpha_pll *pll;
+};
+
+union a53pll {
+	struct clk_pll *pll;
+	struct a53_alpha_pll alpha_pll;
+};
+
+struct a53pll_data {
+#define PLL_IS_ALPHA BIT(0)
+	u8 flags;
+	union a53pll a53pll;
+};
+
+static const u8 ipq_pll_offsets[] = {
+	[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 struct pll_freq_tbl msm8996_a53pll_freq[] = {
 	{  998400000, 52, 0x0, 0x1, 0 },
 	{ 1094400000, 57, 0x0, 0x1, 0 },
 	{ 1152000000, 62, 0x0, 0x1, 0 },
@@ -26,6 +55,64 @@  static const struct pll_freq_tbl a53pll_freq[] = {
 	{ }
 };
 
+static struct clk_pll msm8996_pll = {
+	.mode_reg = 0x0,
+	.l_reg = 0x04,
+	.m_reg = 0x08,
+	.n_reg = 0x0c,
+	.config_reg = 0x14,
+	.status_reg = 0x1c,
+	.status_bit = 16,
+	.freq_tbl = msm8996_a53pll_freq,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "a53pll",
+		.flags = CLK_IS_CRITICAL,
+		.parent_data = &(const struct clk_parent_data){
+			.fw_name = "xo",
+			.name = "xo",
+		},
+		.num_parents = 1,
+		.ops = &clk_pll_sr2_ops,
+	},
+};
+
+static struct clk_alpha_pll ipq6018_pll = {
+	.offset = 0x0,
+	.regs = ipq_pll_offsets,
+	.flags = SUPPORTS_DYNAMIC_UPDATE,
+	.clkr = {
+		.enable_reg = 0x0,
+		.enable_mask = BIT(0),
+		.hw.init = &(struct clk_init_data){
+			.name = "a53pll",
+			.flags = CLK_IS_CRITICAL,
+			.parent_data = &(const struct clk_parent_data){
+				.fw_name = "xo",
+			},
+			.num_parents = 1,
+			.ops = &clk_alpha_pll_huayra_ops,
+		},
+	},
+};
+
+static struct alpha_pll_config ipq6018_pll_config = {
+	.l = 0x37,
+	.config_ctl_val = 0x04141200,
+	.config_ctl_hi_val = 0x0,
+	.early_output_mask = BIT(3),
+	.main_output_mask = BIT(0),
+};
+
+static struct a53pll_data msm8996pll_data = {
+	.a53pll.pll = &msm8996_pll,
+};
+
+static struct a53pll_data ipq6018pll_data = {
+	.flags = PLL_IS_ALPHA,
+	.a53pll.alpha_pll.pll = &ipq6018_pll,
+	.a53pll.alpha_pll.pll_config = &ipq6018_pll_config,
+};
+
 static const struct regmap_config a53pll_regmap_config = {
 	.reg_bits		= 32,
 	.reg_stride		= 4,
@@ -39,14 +126,16 @@  static int qcom_a53pll_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
 	struct resource *res;
-	struct clk_pll *pll;
+	const struct a53pll_data *pll_data;
+	struct clk_regmap *clkr;
 	void __iomem *base;
-	struct clk_init_data init = { };
 	int ret;
 
-	pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
-	if (!pll)
-		return -ENOMEM;
+	pll_data = of_device_get_match_data(dev);
+	if (!pll_data) {
+		dev_err(dev, "failed to get platform data\n");
+		return -ENODEV;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(dev, res);
@@ -57,30 +146,26 @@  static int qcom_a53pll_probe(struct platform_device *pdev)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	pll->l_reg = 0x04;
-	pll->m_reg = 0x08;
-	pll->n_reg = 0x0c;
-	pll->config_reg = 0x14;
-	pll->mode_reg = 0x00;
-	pll->status_reg = 0x1c;
-	pll->status_bit = 16;
-	pll->freq_tbl = a53pll_freq;
-
-	init.name = "a53pll";
-	init.parent_names = (const char *[]){ "xo" };
-	init.num_parents = 1;
-	init.ops = &clk_pll_sr2_ops;
-	init.flags = CLK_IS_CRITICAL;
-	pll->clkr.hw.init = &init;
-
-	ret = devm_clk_register_regmap(dev, &pll->clkr);
+	if (pll_data->flags & PLL_IS_ALPHA) {
+		struct clk_alpha_pll *alpha_pll =
+			pll_data->a53pll.alpha_pll.pll;
+		struct alpha_pll_config *alpha_pll_config =
+			pll_data->a53pll.alpha_pll.pll_config;
+
+		clk_alpha_pll_configure(alpha_pll, regmap, alpha_pll_config);
+		clkr = &pll_data->a53pll.alpha_pll.pll->clkr;
+	} else {
+		clkr = &pll_data->a53pll.pll->clkr;
+	}
+
+	ret = devm_clk_register_regmap(dev, clkr);
 	if (ret) {
 		dev_err(dev, "failed to register regmap clock: %d\n", ret);
 		return ret;
 	}
 
 	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
-					  &pll->clkr.hw);
+					  &clkr->hw);
 	if (ret) {
 		dev_err(dev, "failed to add clock provider: %d\n", ret);
 		return ret;
@@ -90,7 +175,8 @@  static int qcom_a53pll_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_a53pll_match_table[] = {
-	{ .compatible = "qcom,msm8916-a53pll" },
+	{ .compatible = "qcom,msm8916-a53pll", .data = &msm8996pll_data},
+	{ .compatible = "qcom,ipq6018-a53pll", .data = &ipq6018pll_data},
 	{ }
 };