Received-SPF: pass (google.com: domain of linux-kernel+bounces-96648-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <1e353c1a-e267-14d0-fdf4-36aea9062ed3@quicinc.com>
Date: Fri, 8 Mar 2024 13:56:51 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.0
Subject: Re: [PATCH 2/5] clk: qcom: clk-alpha-pll: Add support for Regera PLL
 ops
Content-Language: en-US
To: Konrad Dybcio <konrad.dybcio@linaro.org>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        "Stephen
 Boyd" <sboyd@kernel.org>,
        Abhishek Sahu <absahu@codeaurora.org>, Rob Herring
	<robh@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>
CC: Stephen Boyd <sboyd@codeaurora.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-clk@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>, Ajit Pandey <quic_ajipan@quicinc.com>,
        "Imran
 Shaik" <quic_imrashai@quicinc.com>,
        Taniya Das <quic_tdas@quicinc.com>,
        Jagadeesh Kona <quic_jkona@quicinc.com>
References: <20240229-camcc-support-sm8150-v1-0-8c28c6c87990@quicinc.com>
 <20240229-camcc-support-sm8150-v1-2-8c28c6c87990@quicinc.com>
 <630bb10a-2197-4573-a6d5-01fa6650c315@linaro.org>
From: "Satya Priya Kakitapalli (Temp)" <quic_skakitap@quicinc.com>
In-Reply-To: <630bb10a-2197-4573-a6d5-01fa6650c315@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 3/2/2024 5:26 AM, Konrad Dybcio wrote:
> On 29.02.2024 06:38, Satya Priya Kakitapalli wrote:
>> From: Taniya Das <quic_tdas@quicinc.com>
>>
>> Regera PLL ops are required to control the Regera PLL from clock
>> controller drivers, thus add support for the same.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> ---
> [...]
>
>
>> +static int clk_regera_pll_enable(struct clk_hw *hw)
> This function is 1:1 clk_zonda_pll_enable() logic-wise, except for
> the `if (val & ZONDA_STAY_IN_CFA)` part. Would it be an issue on
> Regera?


Yes, that is only applicable for Zonda PLL, hence we cannot re-use the 
same code for Regera.


>> +static void clk_regera_pll_disable(struct clk_hw *hw)
> This again is clk_zonda_pll_disable(), except the very last value written
> to PLL_OPMODE is different. Could you commonize them?
>

This difference is there between Zonda and regera PLLs as per the HW 
recommendation, hence we cannot re-use this.


>> +
>> +static void zonda_pll_adjust_l_val(unsigned long rate, unsigned long prate,
>> +									u32 *l)
> (Ugly wrapping, please touch it up)
>
> ..should it apply to zonda as the name suggests? Also, any explanations?


Yes, it is applicable for Zonda PLL as well, will update the Zonda pll 
set rate in next post. The L value needs to be adjusted to handle the 16 
bit signed alpha.


>> +	u64 remainder, quotient;
>> +
>> +	quotient = rate;
>> +	remainder = do_div(quotient, prate);
>> +	*l = quotient;
>> +
>> +	if ((remainder * 2) / prate)
>> +		*l = *l + 1;
>> +}
>> +
>> +static int clk_regera_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>> +				  unsigned long prate)
>> +{
>> +	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>> +	unsigned long rrate;
>> +	u32 l, alpha_width = pll_alpha_width(pll);
>> +	u64 a;
>> +	int ret;
>> +
>> +	rrate = alpha_pll_round_rate(rate, prate, &l, &a, alpha_width);
>> +
>> +	ret = alpha_pll_check_rate_margin(hw, rrate, rate);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (a && (a & BIT(15)))
> What is BIT(15)?
>
> Also, the left part of the condition is totally bogus, if a is 0 then
> a & BIT(15) will surely be zero as well.


Sure, will correct this.


> Konrad
>
>