linux-kernel - Re: [PATCH] clk: qcom: clk-alpha-pll: Replace divide operator with comparison

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: <5b24f86a-b2cc-4c36-aa2f-2a01cbae12fa@nvidia.com>
Date: Wed, 4 Sep 2024 22:26:09 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: Satya Priya Kakitapalli <quic_skakitap@...cinc.com>,
 Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
 Bjorn Andersson <andersson@...nel.org>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org, Ajit Pandey <quic_ajipan@...cinc.com>,
 Imran Shaik <quic_imrashai@...cinc.com>, Taniya Das <quic_tdas@...cinc.com>,
 Jagadeesh Kona <quic_jkona@...cinc.com>, kernel test robot <lkp@...el.com>,
 Dan Carpenter <dan.carpenter@...aro.org>,
 "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH] clk: qcom: clk-alpha-pll: Replace divide operator with
 comparison

Hi Satya,

On 30/08/2024 06:33, Satya Priya Kakitapalli wrote:
> Hi Jon,
> 
> 
> On 8/28/2024 7:17 PM, Jon Hunter wrote:
>> Hi Satya, Vladimir,
>>
>> On 13/08/2024 21:01, Vladimir Zapolskiy wrote:
>>> On 8/13/24 12:40, Satya Priya Kakitapalli wrote:
>>>> In zonda_pll_adjust_l_val() replace the divide operator with comparison
>>>> operator since comparisons are faster than divisions.
>>>>
>>>> Fixes: f4973130d255 ("clk: qcom: clk-alpha-pll: Update set_rate for 
>>>> Zonda PLL")
>>>
>>> Apparently the change is not a fix, therefore I believe the Fixes tag
>>> shall be removed.
>>
>>
>> From the commit message it is not clear that this is a fix, but I
>> believe that it is. With the current -next I am seeing the following
>> build error (with GCC 7.3.1) on ARM ...
>>
>> drivers/clk/qcom/clk-alpha-pll.o: In function `clk_zonda_pll_set_rate':
>> clk-alpha-pll.c:(.text+0x45dc): undefined reference to `__aeabi_uldivmod'
>>
>>>> Reported-by: kernel test robot <lkp@...el.com>
>>>> Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
>>>> Closes: https://lore.kernel.org/r/202408110724.8pqbpDiD-lkp@intel.com/
>>
>> There is also the above smatch warning that was reported.
>>
>>>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@...cinc.com>
>>>> ---
>>>>   drivers/clk/qcom/clk-alpha-pll.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/clk-alpha-pll.c 
>>>> b/drivers/clk/qcom/clk-alpha-pll.c
>>>> index 2f620ccb41cb..fd8a82bb3690 100644
>>>> --- a/drivers/clk/qcom/clk-alpha-pll.c
>>>> +++ b/drivers/clk/qcom/clk-alpha-pll.c
>>>> @@ -2126,7 +2126,7 @@ static void zonda_pll_adjust_l_val(unsigned 
>>>> long rate, unsigned long prate, u32
>>>>       remainder = do_div(quotient, prate);
>>>>       *l = quotient;
>>>
>>> Since it's not a fix, but a simplification, you may wish to remove
>>> an unnecessary 'quotient' local variable:
>>>
>>> remainder = do_div(rate, prate);
>>>
>>>> -    if ((remainder * 2) / prate)
>>>> +    if ((remainder * 2) >= prate)
>>>>           *l = *l + 1;
>>>
>>> *l = rate + (u32)(remainder * 2 >= prate);
>>
>>
>> The above change does fix this build error for me.
>>
>> Satya, did you intend this to be a fix? Can we get this into -next?
>>
> 
> Yes, I have posted a v2 for this last week, but there are few open 
> comments on that, I'll address them and post V3 including the build 
> error you reported in commit-text.
> 
> 
> [v2] 
> https://lore.kernel.org/linux-clk/20240814102005.33493-1-quic_skakitap@quicinc.com/


Have you push a V3 yet?

Thanks
Jon

-- 
nvpublic