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

[Jon Hunter]

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@xxxxxxxxx>
> > > > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > > > Closes: https://lore.kernel.org/r/202408110724.8pqbpDiD-lkp@xxxxxxxxx/
> >
> > There is also the above smatch warning that was reported.
> >
> > > > Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@xxxxxxxxxxx>
> > > > ---
> > > >   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@xxxxxxxxxxx/


Have you push a V3 yet?

Thanks
Jon

--
nvpublic