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] [day] [month] [year] [list]

Message-ID: <bc9c90c0-0b5a-4459-aeea-606f946a7fe7@quicinc.com>
Date: Fri, 6 Sep 2024 16:37:14 +0530
From: Satya Priya Kakitapalli <quic_skakitap@...cinc.com>
To: 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>
Subject: Re: [PATCH] clk: qcom: clk-alpha-pll: Replace divide operator with
 comparison

Hi Vladimir,


On 8/14/2024 1:31 AM, 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.
>
>> 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/
>> 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);
>

I tried removing the quotient variable, but it is leading to below build 
errors on arm32 architectures, so I will add the quotient variable back 
on V3,  to make the pointer type compatible for both arm32 and arm64.


error: passing argument 1 of '__div64_32' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
   238 |   __rem = __div64_32(&(n), __base);


expected 'uint64_t *' {aka 'long long unsigned int *'} but argument is 
of type 'long unsigned int *'
    24 | static inline uint32_t __div64_32(uint64_t *n, uint32_t base)


>>   -    if ((remainder * 2) / prate)
>> +    if ((remainder * 2) >= prate)
>>           *l = *l + 1;
>
> *l = rate + (u32)(remainder * 2 >= prate);
>
> I hope the assignment above is quite clear...
>
>>   }
>
> With the review comments above implemented, feel free to add to v2
>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>
>
> -- 
> Best wishes,
> Vladimir