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?