On Wed, 3 May 2023 at 16:02, Yassine Oudjana <yassine.oudjana@xxxxxxxxx> wrote:
>
> From: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
> ---
> 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
>
--
With best wishes
Dmitry
