Re: [PATCH v7 2/4] clk: sunxi-ng: div: add read-only operation support

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

Pre-existing issues:
- [High] Potential division-by-zero kernel panic in `divider_ro_determine_rate` if the hardware register contains an invalid or unmapped value.
--

--- Patch [2]: [PATCH v7 2/4] clk: sunxi-ng: div: add read-only operation support ---
commit 3b6cfd40ff69176dfa1e8841c3c6d40584ab1369
Author: Jerome Brunet <[email protected]>

clk: sunxi-ng: div: add read-only operation support

Add support for sunxi-ng read-only dividers. This will be
useful to the a733 oscillator detection logic.

> diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
> --- a/drivers/clk/sunxi-ng/ccu_div.c
> +++ b/drivers/clk/sunxi-ng/ccu_div.c
> @@ -20,8 +20,22 @@ static int ccu_div_determine_rate_helper(struct ccu_mux_internal *mux,
>  	if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
>  		req->rate *= cd->fixed_post_div;
>  
> -	ret = divider_determine_rate(&cd->common.hw, req, cd->div.table,
> -				     cd->div.width, cd->div.flags);
> +	if (cd->div.flags & CLK_DIVIDER_READ_ONLY) {
> +		unsigned long val;
> +		u32 reg;
> +
> +		reg = readl(cd->common.base + cd->common.reg);
> +		val = reg >> cd->div.shift;
> +		val &= (1 << cd->div.width) - 1;
> +
> +		ret = divider_ro_determine_rate(&cd->common.hw, req, cd->div.table,
> +						cd->div.width, cd->div.flags, val);

[Severity: High]
This is a pre-existing issue, but could this expose a division by zero in
divider_ro_determine_rate() if the hardware register yields an invalid value?

The new code reads a raw divider value from the hardware register and passes
it directly into divider_ro_determine_rate(). If the register is in an
uninitialized or corrupted state containing a value not present in the
associated table, the internal _get_div() helper can return 0.

Unlike divider_recalc_rate() which safely checks the divisor,
divider_ro_determine_rate() directly evaluates DIV_ROUND_UP_ULL(..., div):

drivers/clk/clk-divider.c:divider_ro_determine_rate() {
    ...
    div = _get_div(table, val, flags, width);
    ...
    req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div);
    ...
}

Since this macro performs a division, a divisor of 0 triggers a
divide-by-zero. Should the hardware value or the resulting divisor be
validated to prevent this regression during clock initialization?

> +
> +	} else {
> +		ret = divider_determine_rate(&cd->common.hw, req, cd->div.table,
> +					     cd->div.width, cd->div.flags);
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2