Re: [PATCH v2 2/8] clk: qcom: common: Add support to configure PLL

[Jagadeesh Kona]

On 3/6/2025 5:52 PM, Konrad Dybcio wrote:
> On 6.03.2025 9:55 AM, Jagadeesh Kona wrote:
>> From: Taniya Das <quic_tdas@xxxxxxxxxxx>
>>
>> Integrate PLL configuration into clk_alpha_pll structure and add support
>> for qcom_cc_clk_alpha_pll_configure() function which can be used to
>> configure the clock controller PLLs from common core code.
>>
>> Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>
>> ---
> 
> [...]
> 
>> +static void qcom_cc_clk_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
>> +{
>> +	if (!pll->config || !pll->regs)
>> +		return;
> 
> This should probably throw some sort of a warning
> 

Yes, will add a warning here and for default case in next series.

Thanks,
Jagadeesh

>> +
>> +	switch (GET_PLL_TYPE(pll)) {
>> +	case CLK_ALPHA_PLL_TYPE_LUCID_OLE:
>> +		clk_lucid_ole_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_LUCID_EVO:
>> +		clk_lucid_evo_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_TAYCAN_ELU:
>> +		clk_taycan_elu_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_RIVIAN_EVO:
>> +		clk_rivian_evo_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_TRION:
>> +		clk_trion_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_2290:
>> +		clk_huayra_2290_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_FABIA:
>> +		clk_fabia_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_AGERA:
>> +		clk_agera_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_PONGO_ELU:
>> +		clk_pongo_elu_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_ZONDA:
>> +	case CLK_ALPHA_PLL_TYPE_ZONDA_OLE:
>> +		clk_zonda_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_STROMER:
>> +	case CLK_ALPHA_PLL_TYPE_STROMER_PLUS:
>> +		clk_stromer_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	case CLK_ALPHA_PLL_TYPE_DEFAULT:
>> +	case CLK_ALPHA_PLL_TYPE_DEFAULT_EVO:
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA:
>> +	case CLK_ALPHA_PLL_TYPE_HUAYRA_APSS:
>> +	case CLK_ALPHA_PLL_TYPE_BRAMMO:
>> +	case CLK_ALPHA_PLL_TYPE_BRAMMO_EVO:
>> +		clk_alpha_pll_configure(pll, regmap, pll->config);
>> +		break;
>> +	default:
>> +		break;
> 
> And so should the 'default' case
> 
> Konrad