On Thu, 21 Mar 2024 at 09:50, Gabor Juhos <j4g8y7@xxxxxxxxx> wrote: > > 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. I can't fully appreciate this idea. There can be cases when different PLL types share the set of ops. I think having a type is more versatile and makes the code more obvious. > > No functional changes. > > Signed-off-by: Gabor Juhos <j4g8y7@xxxxxxxxx> > --- > Changes in v2: > - no changes > - Link to v1: https://lore.kernel.org/r/20240318-apss-ipq-pll-cleanup-v1-3-52f795429d5d@xxxxxxxxx > --- > 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 8cf17374a2e2a..816b0d1f8d8c8 100644 > --- a/drivers/clk/qcom/apss-ipq-pll.c > +++ b/drivers/clk/qcom/apss-ipq-pll.c > @@ -131,37 +131,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, > }; > @@ -194,10 +188,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 > -- With best wishes Dmitry
