Re: [PATCH 6/8] clk: clocking-wizard: Add static-config clock provider support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Division by zero in `clk_wzrd_determine_rate` when the requested rate is significantly larger than the parent rate.
- [High] Logic bug with ignored `min_t` return value leads to unbounded register writes.
- [High] Division by zero in `clk_wzrd_get_divisors` and `clk_wzrd_get_divisors_ver` when the target rate is 0.
--

commit b31cd102641055f0a34c6e79498f21e726bb2a91
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>

clk: clocking-wizard: Add static-config clock provider support

When xlnx,static-config is present the divider/multiplier path is
synthesized inside the Wizard without exposing runtime MMIO
reconfiguration, so omit the AXI register mapping and advertise each
routed output clock as a clk_fixed_factor child of clk_in1 using the
synthesized ratios exported through xlnx,clk-mul-div.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index ffc78c90bee6f..5470a717fccc8 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c

[ ... ]

This isn't a bug introduced by this patch, but I noticed a potential division
by zero in the unpatched code:

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_determine_rate() {
    ...
    div = DIV_ROUND_CLOSEST(req->best_parent_rate, req->rate);

    req->rate = req->best_parent_rate / div;
    ...
}

[Severity: High]
If the requested rate is significantly larger than the best parent rate, 
could div evaluate to 0 and trigger a division by zero exception here?


This is a pre-existing issue, but the capping logic appears to be unused
in the following code:

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_dynamic_reconfig() {
    ...
    /* Cap the value to max */
    min_t(u32, value, WZRD_DR_MAX_INT_DIV_VALUE);

    /* Set divisor and clear phase offset */
    writel(value, div_addr);
    ...
}

[Severity: High]
Does this code leave value unbounded before the register write since the
return value of min_t() is not assigned to anything?


This isn't a bug introduced by this patch, but there might be another division
by zero risk in the divisor calculations:

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_get_divisors_ver() {
    ...
    o = DIV_ROUND_CLOSEST_ULL(vco_freq, rate);
    ...
}

And similarly in clk_wzrd_get_divisors():

drivers/clk/xilinx/clk-xlnx-clock-wizard.c:clk_wzrd_get_divisors() {
    ...
    o = DIV_ROUND_CLOSEST_ULL(vco_freq, rate);
    ...
}

[Severity: High]
If rate is exactly 0, can this cause a kernel panic due to a division
by zero?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615034845.3320286-1-shubhrajyoti.datta@amd.com?part=6