Hi Dan Carpenter,
Thanks for the feedback.
> -----Original Message-----
> From: Dan Carpenter <error27@xxxxxxxxx>
> Sent: Tuesday, April 11, 2023 9:21 AM
> To: oe-kbuild@xxxxxxxxxxxxxxx; Biju Das <biju.das.jz@xxxxxxxxxxxxxx>;
> Michael Turquette <mturquette@xxxxxxxxxxxx>; Stephen Boyd <sboyd@xxxxxxxxxx>
> Cc: lkp@xxxxxxxxx; oe-kbuild-all@xxxxxxxxxxxxxxx; Biju Das
> <biju.das.jz@xxxxxxxxxxxxxx>; linux-clk@xxxxxxxxxxxxxxx; Geert Uytterhoeven
> <geert+renesas@xxxxxxxxx>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@xxxxxxxxxxxxxx>; linux-renesas-soc@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 2/3] drivers: clk: Add support for versa3 clock
> driver
>
> Hi Biju,
>
> kernel test robot noticed the following build warnings:
>
> url:
> base: 1bd575707d704530a52d5dd320c29d79e9cff42d
> patch link:
> patch subject: [PATCH v4 2/3] drivers: clk: Add support for versa3 clock
> driver
> config: arc-randconfig-m031-20230405
> compiler: arc-elf-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
> | Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> | Link:
>
> smatch warnings:
> drivers/clk/clk-versaclock3.c:546 vc3_clk_mux_determine_rate() warn: replace
> divide condition 'req->best_parent_rate / req->rate' with 'req-
> >best_parent_rate >= req->rate'
>
> vim +546 drivers/clk/clk-versaclock3.c
>
> fe68e1ca2f2871 Biju Das 2023-04-04 538 static int
> vc3_clk_mux_determine_rate(struct clk_hw *hw,
> fe68e1ca2f2871 Biju Das 2023-04-04 539 struct
> clk_rate_request *req)
> fe68e1ca2f2871 Biju Das 2023-04-04 540 {
> fe68e1ca2f2871 Biju Das 2023-04-04 541 int ret;
> fe68e1ca2f2871 Biju Das 2023-04-04 542 int frc;
> fe68e1ca2f2871 Biju Das 2023-04-04 543
> fe68e1ca2f2871 Biju Das 2023-04-04 544 ret =
> clk_mux_determine_rate_flags(hw, req, CLK_SET_RATE_PARENT);
> fe68e1ca2f2871 Biju Das 2023-04-04 545 if (ret) {
> fe68e1ca2f2871 Biju Das 2023-04-04 @546 if (req->best_parent_rate
> / req->rate) {
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ What is this testing? In terms of micro
> optimizing, divide operations are normally slow. And >= is more readable.
> But maybe this is something which makes sense with more context...
I agree ">=" sufficient here.
For eg: 8000Hz playback. 8000 is multiple of 12280000.
When clk_mux_determine_rate_flags fails,
it finds a factor for best_parent_rate and req->rate eg:( 11289600/2822400) = 4
With this it again calls clk_mux_determine_rate_flags, that results in (11289600/2048000) = 6
2048000 * 6 = 12280000 and select the clocksrc with "12280000" rate.
Logs:
Stream parameters are [ 41.694036] ####vc3_clk_mux_determine_rate 11289600/2822400/4###
8000Hz, S16_LE, [ 46.777454] ####vc3_clk_mux_determine_rate 11289600/2048000/6###
Similarly, 11025Hz playback,11025 is multiple of 11289600.
When clk_mux_determine_rate_flags fails,
then it finds a factor for best_parent_rate and req->rate eg:( 11289600/5644800) = 2
and select the clksrc with "11289600" rate.
Logs:
Stream parameters are 11025Hz, S16_LE, 2 chann[ 36.591803] ####vc3_clk_mux_determine_rate 11289600/5644800/2###
Cheers,
Biju
>
> fe68e1ca2f2871 Biju Das 2023-04-04 547 frc =
> DIV_ROUND_CLOSEST_ULL(req->best_parent_rate,
> fe68e1ca2f2871 Biju Das 2023-04-04 548
> req->rate);
> fe68e1ca2f2871 Biju Das 2023-04-04 549 req->rate *= frc;
> fe68e1ca2f2871 Biju Das 2023-04-04 550 return
> clk_mux_determine_rate_flags(hw, req,
> fe68e1ca2f2871 Biju Das 2023-04-04 551
> CLK_SET_RATE_PARENT);
> fe68e1ca2f2871 Biju Das 2023-04-04 552 }
> fe68e1ca2f2871 Biju Das 2023-04-04 553 ret = 0;
> fe68e1ca2f2871 Biju Das 2023-04-04 554 }
> fe68e1ca2f2871 Biju Das 2023-04-04 555
> fe68e1ca2f2871 Biju Das 2023-04-04 556 return ret;
> fe68e1ca2f2871 Biju Das 2023-04-04 557 }
>
