linux-kernel - Re: [PATCH V2 3/6] clk: qcom: clk-alpha-pll: Remove explicit CAL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <df7ab6f7-6c5e-9a7d-8d9b-09ff32da34d6@quicinc.com>
Date:   Thu, 1 Jun 2023 20:03:42 +0530
From:   Jagadeesh Kona <quic_jkona@...cinc.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>
CC:     Bjorn Andersson <andersson@...nel.org>,
        Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
        Vinod Koul <vkoul@...nel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-clk@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Taniya Das <quic_tdas@...cinc.com>,
        Satya Priya Kakitapalli <quic_skakitap@...cinc.com>,
        Imran Shaik <quic_imrashai@...cinc.com>,
        Ajit Pandey <quic_ajipan@...cinc.com>
Subject: Re: [PATCH V2 3/6] clk: qcom: clk-alpha-pll: Remove explicit CAL_L
 configuration for EVO PLL

Hi Dmitry, Konrad,

On 5/26/2023 9:23 PM, Dmitry Baryshkov wrote:
> On 26/05/2023 12:33, Konrad Dybcio wrote:
>>
>>
>> On 25.05.2023 19:21, Jagadeesh Kona wrote:
>>> In lucid evo pll, the CAL_L field is part of L value register itself, 
>>> and
>>> the l value configuration passed from clock controller driver includes
>>> CAL_L and L values as well. Hence remove explicit configuration of CAL_L
>>> for evo pll.
>>>
>>> Fixes: 260e36606a03 ("clk: qcom: clk-alpha-pll: add Lucid EVO PLL 
>>> configuration interfaces")
>>> Signed-off-by: Taniya Das <quic_tdas@...cinc.com>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@...cinc.com>
>>> ---
>> Oh that isn't obvious at first sight, nice find!
>>
>> I'd suggest a different solution though:
>>
>> #define LUCID_EVO_PLL_L_LVAL    GENMASK(..
>> #define LUCID_EVO_PLL_L_CAL_L    GENMASK(..
>>
>> lval = FIELD_PREP(LUCID_EVO_PLL_L_LVAL, config->l) |
>>         FIELD_PREP(LUCID_EVO_PLL_L_CAL_L, config->cal_l);
>>
>> This would make the separation between the two parts more explicit
>>
>> however
>>
>> config->l would then represent the L value and not the end value
>> written to the L register
> 
> Yes. I think there should be separate config->l and config->cal_l values 
> (and probably ringosc_cal_l, basing on the comment in the source).
> Thanks for your suggestions. In all recent chipsets, L & CAL_L fields 
are encapsulated in the same register, so we feel it is better to 
directly pass the combined configuration value in config->l itself and 
program it directly into register without any additional handling 
required in pll driver code.

Also the evo pll code is currently reused for both lucid evo and ole 
pll's. Lucid ole PLL has an additional RINGOSC_CAL_L field along with L, 
CAL_L fields in the same L register. By passing combined configuration 
value in config->l itself, we feel we can avoid all the additional 
handling required in PLL code.

> Just a question: is camcc-sm8550 using the same PLL type or is it some 
> kind of subtype of lucid_evo PLL?
> 
No, it is not the same lucid evo PLL. It uses lucid ole PLL.

Thanks & Regards,
Jagadeesh