[RESEND,3/3] clk: qcom: cbf-msm8996: Add support for MSM8996 Pro
Commit Message
From: Yassine Oudjana <y.oudjana@protonmail.com>
The CBF PLL on MSM8996 Pro has a /4 post divisor instead of /2. Handle the
difference accordingly.
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
drivers/clk/qcom/clk-cbf-8996.c | 121 ++++++++++++++++++++++++++++----
1 file changed, 106 insertions(+), 15 deletions(-)
Comments
On Wed, 3 May 2023 at 16:02, Yassine Oudjana <yassine.oudjana@gmail.com> wrote:
>
> From: Yassine Oudjana <y.oudjana@protonmail.com>
>
> The CBF PLL on MSM8996 Pro has a /4 post divisor instead of /2. Handle the
> difference accordingly.
>
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
> drivers/clk/qcom/clk-cbf-8996.c | 121 ++++++++++++++++++++++++++++----
> 1 file changed, 106 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c
> index 1bb2cd956d68..a3e96578ddd9 100644
> --- a/drivers/clk/qcom/clk-cbf-8996.c
> +++ b/drivers/clk/qcom/clk-cbf-8996.c
> @@ -65,6 +65,19 @@ static const struct alpha_pll_config cbfpll_config = {
> .early_output_mask = BIT(3),
> };
>
> +static const struct alpha_pll_config cbfpll_pro_config = {
> + .l = 72,
> + .config_ctl_val = 0x200d4828,
> + .config_ctl_hi_val = 0x006,
> + .test_ctl_val = 0x1c000000,
> + .test_ctl_hi_val = 0x00004000,
> + .pre_div_mask = BIT(12),
> + .post_div_mask = 0x3 << 8,
> + .post_div_val = 0x3 << 8,
> + .main_output_mask = BIT(0),
> + .early_output_mask = BIT(3),
> +};
Granted that the difference between this and the non-pro is just the
post_div_val, would it be easier to just patch it in the probe()?
> +
> static struct clk_alpha_pll cbf_pll = {
> .offset = CBF_PLL_OFFSET,
> .regs = cbf_pll_regs,
> @@ -93,6 +106,20 @@ static struct clk_fixed_factor cbf_pll_postdiv = {
> },
> };
>
> +static struct clk_fixed_factor cbf_pro_pll_postdiv = {
> + .mult = 1,
> + .div = 4,
> + .hw.init = &(struct clk_init_data){
> + .name = "cbf_pll_postdiv",
> + .parent_hws = (const struct clk_hw*[]){
> + &cbf_pll.clkr.hw
> + },
> + .num_parents = 1,
> + .ops = &clk_fixed_factor_ops,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
Same question here.
Then we can get rid of all the duplication below.
> +
> static const struct clk_parent_data cbf_mux_parent_data[] = {
> { .index = DT_XO },
> { .hw = &cbf_pll.clkr.hw },
> @@ -100,6 +127,13 @@ static const struct clk_parent_data cbf_mux_parent_data[] = {
> { .index = DT_APCS_AUX },
> };
>
> +static const struct clk_parent_data cbf_pro_mux_parent_data[] = {
> + { .index = DT_XO },
> + { .hw = &cbf_pll.clkr.hw },
> + { .hw = &cbf_pro_pll_postdiv.hw },
> + { .index = DT_APCS_AUX },
> +};
> +
> struct clk_cbf_8996_mux {
> u32 reg;
> struct notifier_block nb;
> @@ -140,12 +174,14 @@ static int clk_cbf_8996_mux_determine_rate(struct clk_hw *hw,
> struct clk_rate_request *req)
> {
> struct clk_hw *parent;
> + struct clk_hw *post_div_hw = clk_hw_get_parent_by_index(hw, CBF_DIV_INDEX);
> + struct clk_fixed_factor *post_div = to_clk_fixed_factor(post_div_hw);
>
> - if (req->rate < (DIV_THRESHOLD / 2))
> + if (req->rate < (DIV_THRESHOLD / post_div->div))
> return -EINVAL;
>
> if (req->rate < DIV_THRESHOLD)
> - parent = clk_hw_get_parent_by_index(hw, CBF_DIV_INDEX);
> + parent = post_div_hw;
> else
> parent = clk_hw_get_parent_by_index(hw, CBF_PLL_INDEX);
>
> @@ -177,10 +213,24 @@ static struct clk_cbf_8996_mux cbf_mux = {
> },
> };
>
> +static struct clk_cbf_8996_mux cbf_pro_mux = {
> + .reg = CBF_MUX_OFFSET,
> + .nb.notifier_call = cbf_clk_notifier_cb,
> + .clkr.hw.init = &(struct clk_init_data) {
> + .name = "cbf_mux",
> + .parent_data = cbf_pro_mux_parent_data,
> + .num_parents = ARRAY_SIZE(cbf_pro_mux_parent_data),
> + .ops = &clk_cbf_8996_mux_ops,
> + /* CPU clock is critical and should never be gated */
> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> + },
> +};
> +
> static int cbf_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
> void *data)
> {
> struct clk_notifier_data *cnd = data;
> + struct clk_hw *hw = __clk_get_hw(cnd->clk);
>
> switch (event) {
> case PRE_RATE_CHANGE:
> @@ -188,19 +238,19 @@ static int cbf_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
> * Avoid overvolting. clk_core_set_rate_nolock() walks from top
> * to bottom, so it will change the rate of the PLL before
> * chaging the parent of PMUX. This can result in pmux getting
> - * clocked twice the expected rate.
> + * clocked twice (or 4 times) the expected rate.
> *
> - * Manually switch to PLL/2 here.
> + * Manually switch to PLL/DIV here.
> */
> if (cnd->old_rate > DIV_THRESHOLD &&
> cnd->new_rate < DIV_THRESHOLD)
> - clk_cbf_8996_mux_set_parent(&cbf_mux.clkr.hw, CBF_DIV_INDEX);
> + clk_cbf_8996_mux_set_parent(hw, CBF_DIV_INDEX);
> break;
> case ABORT_RATE_CHANGE:
> /* Revert manual change */
> if (cnd->new_rate < DIV_THRESHOLD &&
> cnd->old_rate > DIV_THRESHOLD)
> - clk_cbf_8996_mux_set_parent(&cbf_mux.clkr.hw, CBF_PLL_INDEX);
> + clk_cbf_8996_mux_set_parent(hw, CBF_PLL_INDEX);
> break;
> default:
> break;
> @@ -213,11 +263,50 @@ static struct clk_hw *cbf_msm8996_hw_clks[] = {
> &cbf_pll_postdiv.hw,
> };
>
> +static struct clk_hw *cbf_msm8996pro_hw_clks[] = {
> + &cbf_pro_pll_postdiv.hw,
> +};
> +
> static struct clk_regmap *cbf_msm8996_clks[] = {
> &cbf_pll.clkr,
> &cbf_mux.clkr,
> };
>
> +static struct clk_regmap *cbf_msm8996pro_clks[] = {
> + &cbf_pll.clkr,
> + &cbf_pro_mux.clkr,
> +};
> +
> +struct cbf_match_data {
> + const struct alpha_pll_config *config;
> + struct clk_fixed_factor *cbf_pll_postdiv;
> + struct clk_cbf_8996_mux *cbf_mux;
> + struct clk_hw **hw_clks;
> + size_t nr_hw_clks;
> + struct clk_regmap **clks;
> + size_t nr_clks;
> +};
> +
> +static const struct cbf_match_data cbf_msm8996_match_data = {
> + .config = &cbfpll_config,
> + .cbf_pll_postdiv = &cbf_pll_postdiv,
> + .cbf_mux = &cbf_mux,
> + .hw_clks = cbf_msm8996_hw_clks,
> + .nr_hw_clks = ARRAY_SIZE(cbf_msm8996_hw_clks),
> + .clks = cbf_msm8996_clks,
> + .nr_clks = ARRAY_SIZE(cbf_msm8996_clks)
> +};
> +
> +static const struct cbf_match_data cbf_msm8996pro_match_data = {
> + .config = &cbfpll_pro_config,
> + .cbf_pll_postdiv = &cbf_pro_pll_postdiv,
> + .cbf_mux = &cbf_pro_mux,
> + .hw_clks = cbf_msm8996pro_hw_clks,
> + .nr_hw_clks = ARRAY_SIZE(cbf_msm8996pro_hw_clks),
> + .clks = cbf_msm8996pro_clks,
> + .nr_clks = ARRAY_SIZE(cbf_msm8996pro_clks)
> +};
> +
> static const struct regmap_config cbf_msm8996_regmap_config = {
> .reg_bits = 32,
> .reg_stride = 4,
> @@ -274,6 +363,7 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
> void __iomem *base;
> struct regmap *regmap;
> struct device *dev = &pdev->dev;
> + const struct cbf_match_data *data = of_device_get_match_data(dev);
> int i, ret;
>
> base = devm_platform_ioremap_resource(pdev, 0);
> @@ -295,7 +385,7 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
> CBF_MUX_AUTO_CLK_SEL_ALWAYS_ON_MASK,
> CBF_MUX_AUTO_CLK_SEL_ALWAYS_ON_GPLL0_SEL);
>
> - clk_alpha_pll_configure(&cbf_pll, regmap, &cbfpll_config);
> + clk_alpha_pll_configure(&cbf_pll, regmap, data->config);
>
> /* Wait for PLL(s) to lock */
> udelay(50);
> @@ -311,27 +401,27 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
> /* Switch CBF to use the primary PLL */
> regmap_update_bits(regmap, CBF_MUX_OFFSET, CBF_MUX_PARENT_MASK, 0x1);
>
> - for (i = 0; i < ARRAY_SIZE(cbf_msm8996_hw_clks); i++) {
> - ret = devm_clk_hw_register(dev, cbf_msm8996_hw_clks[i]);
> + for (i = 0; i < data->nr_hw_clks; i++) {
> + ret = devm_clk_hw_register(dev, data->hw_clks[i]);
> if (ret)
> return ret;
> }
>
> - for (i = 0; i < ARRAY_SIZE(cbf_msm8996_clks); i++) {
> - ret = devm_clk_register_regmap(dev, cbf_msm8996_clks[i]);
> + for (i = 0; i < data->nr_clks; i++) {
> + ret = devm_clk_register_regmap(dev, data->clks[i]);
> if (ret)
> return ret;
> }
>
> - ret = devm_clk_notifier_register(dev, cbf_mux.clkr.hw.clk, &cbf_mux.nb);
> + ret = devm_clk_notifier_register(dev, data->cbf_mux->clkr.hw.clk, &data->cbf_mux->nb);
> if (ret)
> return ret;
>
> - ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw);
> + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &data->cbf_mux->clkr.hw);
> if (ret)
> return ret;
>
> - return qcom_msm8996_cbf_icc_register(pdev, &cbf_mux.clkr.hw);
> + return qcom_msm8996_cbf_icc_register(pdev, &data->cbf_mux->clkr.hw);
> }
>
> static int qcom_msm8996_cbf_remove(struct platform_device *pdev)
> @@ -340,7 +430,8 @@ static int qcom_msm8996_cbf_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id qcom_msm8996_cbf_match_table[] = {
> - { .compatible = "qcom,msm8996-cbf" },
> + { .compatible = "qcom,msm8996-cbf", .data = &cbf_msm8996_match_data },
> + { .compatible = "qcom,msm8996pro-cbf", .data = &cbf_msm8996pro_match_data },
> { /* sentinel */ },
> };
> MODULE_DEVICE_TABLE(of, qcom_msm8996_cbf_match_table);
> --
> 2.40.0
>
On Wed, May 3 2023 at 04:20:00 PM +03:00:00, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
> On Wed, 3 May 2023 at 16:02, Yassine Oudjana
> <yassine.oudjana@gmail.com> wrote:
>>
>> From: Yassine Oudjana <y.oudjana@protonmail.com>
>>
>> The CBF PLL on MSM8996 Pro has a /4 post divisor instead of /2.
>> Handle the
>> difference accordingly.
>>
>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
>> ---
>> drivers/clk/qcom/clk-cbf-8996.c | 121
>> ++++++++++++++++++++++++++++----
>> 1 file changed, 106 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-cbf-8996.c
>> b/drivers/clk/qcom/clk-cbf-8996.c
>> index 1bb2cd956d68..a3e96578ddd9 100644
>> --- a/drivers/clk/qcom/clk-cbf-8996.c
>> +++ b/drivers/clk/qcom/clk-cbf-8996.c
>> @@ -65,6 +65,19 @@ static const struct alpha_pll_config
>> cbfpll_config = {
>> .early_output_mask = BIT(3),
>> };
>>
>> +static const struct alpha_pll_config cbfpll_pro_config = {
>> + .l = 72,
>> + .config_ctl_val = 0x200d4828,
>> + .config_ctl_hi_val = 0x006,
>> + .test_ctl_val = 0x1c000000,
>> + .test_ctl_hi_val = 0x00004000,
>> + .pre_div_mask = BIT(12),
>> + .post_div_mask = 0x3 << 8,
>> + .post_div_val = 0x3 << 8,
>> + .main_output_mask = BIT(0),
>> + .early_output_mask = BIT(3),
>> +};
>
> Granted that the difference between this and the non-pro is just the
> post_div_val, would it be easier to just patch it in the probe()?
I wasn't sure if that approach would've been accepted since it relies
on the special case that only one instance of the CBF device will exist
in normal conditions, but if you say it's fine then sure, I can do that.
>
>> +
>> static struct clk_alpha_pll cbf_pll = {
>> .offset = CBF_PLL_OFFSET,
>> .regs = cbf_pll_regs,
>> @@ -93,6 +106,20 @@ static struct clk_fixed_factor cbf_pll_postdiv
>> = {
>> },
>> };
>>
>> +static struct clk_fixed_factor cbf_pro_pll_postdiv = {
>> + .mult = 1,
>> + .div = 4,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "cbf_pll_postdiv",
>> + .parent_hws = (const struct clk_hw*[]){
>> + &cbf_pll.clkr.hw
>> + },
>> + .num_parents = 1,
>> + .ops = &clk_fixed_factor_ops,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>
> Same question here.
>
> Then we can get rid of all the duplication below.
>
>> +
>> static const struct clk_parent_data cbf_mux_parent_data[] = {
>> { .index = DT_XO },
>> { .hw = &cbf_pll.clkr.hw },
>> @@ -100,6 +127,13 @@ static const struct clk_parent_data
>> cbf_mux_parent_data[] = {
>> { .index = DT_APCS_AUX },
>> };
>>
>> +static const struct clk_parent_data cbf_pro_mux_parent_data[] = {
>> + { .index = DT_XO },
>> + { .hw = &cbf_pll.clkr.hw },
>> + { .hw = &cbf_pro_pll_postdiv.hw },
>> + { .index = DT_APCS_AUX },
>> +};
>> +
>> struct clk_cbf_8996_mux {
>> u32 reg;
>> struct notifier_block nb;
>> @@ -140,12 +174,14 @@ static int
>> clk_cbf_8996_mux_determine_rate(struct clk_hw *hw,
>> struct clk_rate_request
>> *req)
>> {
>> struct clk_hw *parent;
>> + struct clk_hw *post_div_hw = clk_hw_get_parent_by_index(hw,
>> CBF_DIV_INDEX);
>> + struct clk_fixed_factor *post_div =
>> to_clk_fixed_factor(post_div_hw);
>>
>> - if (req->rate < (DIV_THRESHOLD / 2))
>> + if (req->rate < (DIV_THRESHOLD / post_div->div))
>> return -EINVAL;
>>
>> if (req->rate < DIV_THRESHOLD)
>> - parent = clk_hw_get_parent_by_index(hw,
>> CBF_DIV_INDEX);
>> + parent = post_div_hw;
>> else
>> parent = clk_hw_get_parent_by_index(hw,
>> CBF_PLL_INDEX);
>>
>> @@ -177,10 +213,24 @@ static struct clk_cbf_8996_mux cbf_mux = {
>> },
>> };
>>
>> +static struct clk_cbf_8996_mux cbf_pro_mux = {
>> + .reg = CBF_MUX_OFFSET,
>> + .nb.notifier_call = cbf_clk_notifier_cb,
>> + .clkr.hw.init = &(struct clk_init_data) {
>> + .name = "cbf_mux",
>> + .parent_data = cbf_pro_mux_parent_data,
>> + .num_parents = ARRAY_SIZE(cbf_pro_mux_parent_data),
>> + .ops = &clk_cbf_8996_mux_ops,
>> + /* CPU clock is critical and should never be gated
>> */
>> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>> + },
>> +};
>> +
>> static int cbf_clk_notifier_cb(struct notifier_block *nb, unsigned
>> long event,
>> void *data)
>> {
>> struct clk_notifier_data *cnd = data;
>> + struct clk_hw *hw = __clk_get_hw(cnd->clk);
>>
>> switch (event) {
>> case PRE_RATE_CHANGE:
>> @@ -188,19 +238,19 @@ static int cbf_clk_notifier_cb(struct
>> notifier_block *nb, unsigned long event,
>> * Avoid overvolting. clk_core_set_rate_nolock()
>> walks from top
>> * to bottom, so it will change the rate of the PLL
>> before
>> * chaging the parent of PMUX. This can result in
>> pmux getting
>> - * clocked twice the expected rate.
>> + * clocked twice (or 4 times) the expected rate.
>> *
>> - * Manually switch to PLL/2 here.
>> + * Manually switch to PLL/DIV here.
>> */
>> if (cnd->old_rate > DIV_THRESHOLD &&
>> cnd->new_rate < DIV_THRESHOLD)
>> -
>> clk_cbf_8996_mux_set_parent(&cbf_mux.clkr.hw, CBF_DIV_INDEX);
>> + clk_cbf_8996_mux_set_parent(hw,
>> CBF_DIV_INDEX);
>> break;
>> case ABORT_RATE_CHANGE:
>> /* Revert manual change */
>> if (cnd->new_rate < DIV_THRESHOLD &&
>> cnd->old_rate > DIV_THRESHOLD)
>> -
>> clk_cbf_8996_mux_set_parent(&cbf_mux.clkr.hw, CBF_PLL_INDEX);
>> + clk_cbf_8996_mux_set_parent(hw,
>> CBF_PLL_INDEX);
>> break;
>> default:
>> break;
>> @@ -213,11 +263,50 @@ static struct clk_hw *cbf_msm8996_hw_clks[] =
>> {
>> &cbf_pll_postdiv.hw,
>> };
>>
>> +static struct clk_hw *cbf_msm8996pro_hw_clks[] = {
>> + &cbf_pro_pll_postdiv.hw,
>> +};
>> +
>> static struct clk_regmap *cbf_msm8996_clks[] = {
>> &cbf_pll.clkr,
>> &cbf_mux.clkr,
>> };
>>
>> +static struct clk_regmap *cbf_msm8996pro_clks[] = {
>> + &cbf_pll.clkr,
>> + &cbf_pro_mux.clkr,
>> +};
>> +
>> +struct cbf_match_data {
>> + const struct alpha_pll_config *config;
>> + struct clk_fixed_factor *cbf_pll_postdiv;
>> + struct clk_cbf_8996_mux *cbf_mux;
>> + struct clk_hw **hw_clks;
>> + size_t nr_hw_clks;
>> + struct clk_regmap **clks;
>> + size_t nr_clks;
>> +};
>> +
>> +static const struct cbf_match_data cbf_msm8996_match_data = {
>> + .config = &cbfpll_config,
>> + .cbf_pll_postdiv = &cbf_pll_postdiv,
>> + .cbf_mux = &cbf_mux,
>> + .hw_clks = cbf_msm8996_hw_clks,
>> + .nr_hw_clks = ARRAY_SIZE(cbf_msm8996_hw_clks),
>> + .clks = cbf_msm8996_clks,
>> + .nr_clks = ARRAY_SIZE(cbf_msm8996_clks)
>> +};
>> +
>> +static const struct cbf_match_data cbf_msm8996pro_match_data = {
>> + .config = &cbfpll_pro_config,
>> + .cbf_pll_postdiv = &cbf_pro_pll_postdiv,
>> + .cbf_mux = &cbf_pro_mux,
>> + .hw_clks = cbf_msm8996pro_hw_clks,
>> + .nr_hw_clks = ARRAY_SIZE(cbf_msm8996pro_hw_clks),
>> + .clks = cbf_msm8996pro_clks,
>> + .nr_clks = ARRAY_SIZE(cbf_msm8996pro_clks)
>> +};
>> +
>> static const struct regmap_config cbf_msm8996_regmap_config = {
>> .reg_bits = 32,
>> .reg_stride = 4,
>> @@ -274,6 +363,7 @@ static int qcom_msm8996_cbf_probe(struct
>> platform_device *pdev)
>> void __iomem *base;
>> struct regmap *regmap;
>> struct device *dev = &pdev->dev;
>> + const struct cbf_match_data *data =
>> of_device_get_match_data(dev);
>> int i, ret;
>>
>> base = devm_platform_ioremap_resource(pdev, 0);
>> @@ -295,7 +385,7 @@ static int qcom_msm8996_cbf_probe(struct
>> platform_device *pdev)
>> CBF_MUX_AUTO_CLK_SEL_ALWAYS_ON_MASK,
>>
>> CBF_MUX_AUTO_CLK_SEL_ALWAYS_ON_GPLL0_SEL);
>>
>> - clk_alpha_pll_configure(&cbf_pll, regmap, &cbfpll_config);
>> + clk_alpha_pll_configure(&cbf_pll, regmap, data->config);
>>
>> /* Wait for PLL(s) to lock */
>> udelay(50);
>> @@ -311,27 +401,27 @@ static int qcom_msm8996_cbf_probe(struct
>> platform_device *pdev)
>> /* Switch CBF to use the primary PLL */
>> regmap_update_bits(regmap, CBF_MUX_OFFSET,
>> CBF_MUX_PARENT_MASK, 0x1);
>>
>> - for (i = 0; i < ARRAY_SIZE(cbf_msm8996_hw_clks); i++) {
>> - ret = devm_clk_hw_register(dev,
>> cbf_msm8996_hw_clks[i]);
>> + for (i = 0; i < data->nr_hw_clks; i++) {
>> + ret = devm_clk_hw_register(dev, data->hw_clks[i]);
>> if (ret)
>> return ret;
>> }
>>
>> - for (i = 0; i < ARRAY_SIZE(cbf_msm8996_clks); i++) {
>> - ret = devm_clk_register_regmap(dev,
>> cbf_msm8996_clks[i]);
>> + for (i = 0; i < data->nr_clks; i++) {
>> + ret = devm_clk_register_regmap(dev, data->clks[i]);
>> if (ret)
>> return ret;
>> }
>>
>> - ret = devm_clk_notifier_register(dev, cbf_mux.clkr.hw.clk,
>> &cbf_mux.nb);
>> + ret = devm_clk_notifier_register(dev,
>> data->cbf_mux->clkr.hw.clk, &data->cbf_mux->nb);
>> if (ret)
>> return ret;
>>
>> - ret = devm_of_clk_add_hw_provider(dev,
>> of_clk_hw_simple_get, &cbf_mux.clkr.hw);
>> + ret = devm_of_clk_add_hw_provider(dev,
>> of_clk_hw_simple_get, &data->cbf_mux->clkr.hw);
>> if (ret)
>> return ret;
>>
>> - return qcom_msm8996_cbf_icc_register(pdev,
>> &cbf_mux.clkr.hw);
>> + return qcom_msm8996_cbf_icc_register(pdev,
>> &data->cbf_mux->clkr.hw);
>> }
>>
>> static int qcom_msm8996_cbf_remove(struct platform_device *pdev)
>> @@ -340,7 +430,8 @@ static int qcom_msm8996_cbf_remove(struct
>> platform_device *pdev)
>> }
>>
>> static const struct of_device_id qcom_msm8996_cbf_match_table[] = {
>> - { .compatible = "qcom,msm8996-cbf" },
>> + { .compatible = "qcom,msm8996-cbf", .data =
>> &cbf_msm8996_match_data },
>> + { .compatible = "qcom,msm8996pro-cbf", .data =
>> &cbf_msm8996pro_match_data },
>> { /* sentinel */ },
>> };
>> MODULE_DEVICE_TABLE(of, qcom_msm8996_cbf_match_table);
>> --
>> 2.40.0
>>
>
>
> --
> With best wishes
> Dmitry
On Mon, 22 May 2023 at 20:49, Yassine Oudjana <yassine.oudjana@gmail.com> wrote:
>
>
> On Wed, May 3 2023 at 04:20:00 PM +03:00:00, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> > On Wed, 3 May 2023 at 16:02, Yassine Oudjana
> > <yassine.oudjana@gmail.com> wrote:
> >>
> >> From: Yassine Oudjana <y.oudjana@protonmail.com>
> >>
> >> The CBF PLL on MSM8996 Pro has a /4 post divisor instead of /2.
> >> Handle the
> >> difference accordingly.
> >>
> >> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> >> ---
> >> drivers/clk/qcom/clk-cbf-8996.c | 121
> >> ++++++++++++++++++++++++++++----
> >> 1 file changed, 106 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/clk-cbf-8996.c
> >> b/drivers/clk/qcom/clk-cbf-8996.c
> >> index 1bb2cd956d68..a3e96578ddd9 100644
> >> --- a/drivers/clk/qcom/clk-cbf-8996.c
> >> +++ b/drivers/clk/qcom/clk-cbf-8996.c
> >> @@ -65,6 +65,19 @@ static const struct alpha_pll_config
> >> cbfpll_config = {
> >> .early_output_mask = BIT(3),
> >> };
> >>
> >> +static const struct alpha_pll_config cbfpll_pro_config = {
> >> + .l = 72,
> >> + .config_ctl_val = 0x200d4828,
> >> + .config_ctl_hi_val = 0x006,
> >> + .test_ctl_val = 0x1c000000,
> >> + .test_ctl_hi_val = 0x00004000,
> >> + .pre_div_mask = BIT(12),
> >> + .post_div_mask = 0x3 << 8,
> >> + .post_div_val = 0x3 << 8,
> >> + .main_output_mask = BIT(0),
> >> + .early_output_mask = BIT(3),
> >> +};
> >
> > Granted that the difference between this and the non-pro is just the
> > post_div_val, would it be easier to just patch it in the probe()?
>
> I wasn't sure if that approach would've been accepted since it relies
> on the special case that only one instance of the CBF device will exist
> in normal conditions, but if you say it's fine then sure, I can do that.
We do this for other clock controllers, which we know to be singleton.
So this should be fine too.
--
With best wishes
Dmitry
@@ -65,6 +65,19 @@ static const struct alpha_pll_config cbfpll_config = {
.early_output_mask = BIT(3),
};
+static const struct alpha_pll_config cbfpll_pro_config = {
+ .l = 72,
+ .config_ctl_val = 0x200d4828,
+ .config_ctl_hi_val = 0x006,
+ .test_ctl_val = 0x1c000000,
+ .test_ctl_hi_val = 0x00004000,
+ .pre_div_mask = BIT(12),
+ .post_div_mask = 0x3 << 8,
+ .post_div_val = 0x3 << 8,
+ .main_output_mask = BIT(0),
+ .early_output_mask = BIT(3),
+};
+
static struct clk_alpha_pll cbf_pll = {
.offset = CBF_PLL_OFFSET,
.regs = cbf_pll_regs,
@@ -93,6 +106,20 @@ static struct clk_fixed_factor cbf_pll_postdiv = {
},
};
+static struct clk_fixed_factor cbf_pro_pll_postdiv = {
+ .mult = 1,
+ .div = 4,
+ .hw.init = &(struct clk_init_data){
+ .name = "cbf_pll_postdiv",
+ .parent_hws = (const struct clk_hw*[]){
+ &cbf_pll.clkr.hw
+ },
+ .num_parents = 1,
+ .ops = &clk_fixed_factor_ops,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
static const struct clk_parent_data cbf_mux_parent_data[] = {
{ .index = DT_XO },
{ .hw = &cbf_pll.clkr.hw },
@@ -100,6 +127,13 @@ static const struct clk_parent_data cbf_mux_parent_data[] = {
{ .index = DT_APCS_AUX },
};
+static const struct clk_parent_data cbf_pro_mux_parent_data[] = {
+ { .index = DT_XO },
+ { .hw = &cbf_pll.clkr.hw },
+ { .hw = &cbf_pro_pll_postdiv.hw },
+ { .index = DT_APCS_AUX },
+};
+
struct clk_cbf_8996_mux {
u32 reg;
struct notifier_block nb;
@@ -140,12 +174,14 @@ static int clk_cbf_8996_mux_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
struct clk_hw *parent;
+ struct clk_hw *post_div_hw = clk_hw_get_parent_by_index(hw, CBF_DIV_INDEX);
+ struct clk_fixed_factor *post_div = to_clk_fixed_factor(post_div_hw);
- if (req->rate < (DIV_THRESHOLD / 2))
+ if (req->rate < (DIV_THRESHOLD / post_div->div))
return -EINVAL;
if (req->rate < DIV_THRESHOLD)
- parent = clk_hw_get_parent_by_index(hw, CBF_DIV_INDEX);
+ parent = post_div_hw;
else
parent = clk_hw_get_parent_by_index(hw, CBF_PLL_INDEX);
@@ -177,10 +213,24 @@ static struct clk_cbf_8996_mux cbf_mux = {
},
};
+static struct clk_cbf_8996_mux cbf_pro_mux = {
+ .reg = CBF_MUX_OFFSET,
+ .nb.notifier_call = cbf_clk_notifier_cb,
+ .clkr.hw.init = &(struct clk_init_data) {
+ .name = "cbf_mux",
+ .parent_data = cbf_pro_mux_parent_data,
+ .num_parents = ARRAY_SIZE(cbf_pro_mux_parent_data),
+ .ops = &clk_cbf_8996_mux_ops,
+ /* CPU clock is critical and should never be gated */
+ .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+ },
+};
+
static int cbf_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
void *data)
{
struct clk_notifier_data *cnd = data;
+ struct clk_hw *hw = __clk_get_hw(cnd->clk);
switch (event) {
case PRE_RATE_CHANGE:
@@ -188,19 +238,19 @@ static int cbf_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
* Avoid overvolting. clk_core_set_rate_nolock() walks from top
* to bottom, so it will change the rate of the PLL before
* chaging the parent of PMUX. This can result in pmux getting
- * clocked twice the expected rate.
+ * clocked twice (or 4 times) the expected rate.
*
- * Manually switch to PLL/2 here.
+ * Manually switch to PLL/DIV here.
*/
if (cnd->old_rate > DIV_THRESHOLD &&
cnd->new_rate < DIV_THRESHOLD)
- clk_cbf_8996_mux_set_parent(&cbf_mux.clkr.hw, CBF_DIV_INDEX);
+ clk_cbf_8996_mux_set_parent(hw, CBF_DIV_INDEX);
break;
case ABORT_RATE_CHANGE:
/* Revert manual change */
if (cnd->new_rate < DIV_THRESHOLD &&
cnd->old_rate > DIV_THRESHOLD)
- clk_cbf_8996_mux_set_parent(&cbf_mux.clkr.hw, CBF_PLL_INDEX);
+ clk_cbf_8996_mux_set_parent(hw, CBF_PLL_INDEX);
break;
default:
break;
@@ -213,11 +263,50 @@ static struct clk_hw *cbf_msm8996_hw_clks[] = {
&cbf_pll_postdiv.hw,
};
+static struct clk_hw *cbf_msm8996pro_hw_clks[] = {
+ &cbf_pro_pll_postdiv.hw,
+};
+
static struct clk_regmap *cbf_msm8996_clks[] = {
&cbf_pll.clkr,
&cbf_mux.clkr,
};
+static struct clk_regmap *cbf_msm8996pro_clks[] = {
+ &cbf_pll.clkr,
+ &cbf_pro_mux.clkr,
+};
+
+struct cbf_match_data {
+ const struct alpha_pll_config *config;
+ struct clk_fixed_factor *cbf_pll_postdiv;
+ struct clk_cbf_8996_mux *cbf_mux;
+ struct clk_hw **hw_clks;
+ size_t nr_hw_clks;
+ struct clk_regmap **clks;
+ size_t nr_clks;
+};
+
+static const struct cbf_match_data cbf_msm8996_match_data = {
+ .config = &cbfpll_config,
+ .cbf_pll_postdiv = &cbf_pll_postdiv,
+ .cbf_mux = &cbf_mux,
+ .hw_clks = cbf_msm8996_hw_clks,
+ .nr_hw_clks = ARRAY_SIZE(cbf_msm8996_hw_clks),
+ .clks = cbf_msm8996_clks,
+ .nr_clks = ARRAY_SIZE(cbf_msm8996_clks)
+};
+
+static const struct cbf_match_data cbf_msm8996pro_match_data = {
+ .config = &cbfpll_pro_config,
+ .cbf_pll_postdiv = &cbf_pro_pll_postdiv,
+ .cbf_mux = &cbf_pro_mux,
+ .hw_clks = cbf_msm8996pro_hw_clks,
+ .nr_hw_clks = ARRAY_SIZE(cbf_msm8996pro_hw_clks),
+ .clks = cbf_msm8996pro_clks,
+ .nr_clks = ARRAY_SIZE(cbf_msm8996pro_clks)
+};
+
static const struct regmap_config cbf_msm8996_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
@@ -274,6 +363,7 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
void __iomem *base;
struct regmap *regmap;
struct device *dev = &pdev->dev;
+ const struct cbf_match_data *data = of_device_get_match_data(dev);
int i, ret;
base = devm_platform_ioremap_resource(pdev, 0);
@@ -295,7 +385,7 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
CBF_MUX_AUTO_CLK_SEL_ALWAYS_ON_MASK,
CBF_MUX_AUTO_CLK_SEL_ALWAYS_ON_GPLL0_SEL);
- clk_alpha_pll_configure(&cbf_pll, regmap, &cbfpll_config);
+ clk_alpha_pll_configure(&cbf_pll, regmap, data->config);
/* Wait for PLL(s) to lock */
udelay(50);
@@ -311,27 +401,27 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
/* Switch CBF to use the primary PLL */
regmap_update_bits(regmap, CBF_MUX_OFFSET, CBF_MUX_PARENT_MASK, 0x1);
- for (i = 0; i < ARRAY_SIZE(cbf_msm8996_hw_clks); i++) {
- ret = devm_clk_hw_register(dev, cbf_msm8996_hw_clks[i]);
+ for (i = 0; i < data->nr_hw_clks; i++) {
+ ret = devm_clk_hw_register(dev, data->hw_clks[i]);
if (ret)
return ret;
}
- for (i = 0; i < ARRAY_SIZE(cbf_msm8996_clks); i++) {
- ret = devm_clk_register_regmap(dev, cbf_msm8996_clks[i]);
+ for (i = 0; i < data->nr_clks; i++) {
+ ret = devm_clk_register_regmap(dev, data->clks[i]);
if (ret)
return ret;
}
- ret = devm_clk_notifier_register(dev, cbf_mux.clkr.hw.clk, &cbf_mux.nb);
+ ret = devm_clk_notifier_register(dev, data->cbf_mux->clkr.hw.clk, &data->cbf_mux->nb);
if (ret)
return ret;
- ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &cbf_mux.clkr.hw);
+ ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &data->cbf_mux->clkr.hw);
if (ret)
return ret;
- return qcom_msm8996_cbf_icc_register(pdev, &cbf_mux.clkr.hw);
+ return qcom_msm8996_cbf_icc_register(pdev, &data->cbf_mux->clkr.hw);
}
static int qcom_msm8996_cbf_remove(struct platform_device *pdev)
@@ -340,7 +430,8 @@ static int qcom_msm8996_cbf_remove(struct platform_device *pdev)
}
static const struct of_device_id qcom_msm8996_cbf_match_table[] = {
- { .compatible = "qcom,msm8996-cbf" },
+ { .compatible = "qcom,msm8996-cbf", .data = &cbf_msm8996_match_data },
+ { .compatible = "qcom,msm8996pro-cbf", .data = &cbf_msm8996pro_match_data },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, qcom_msm8996_cbf_match_table);