Re: [PATCH v4 6/9] clk: sunxi-ng: div: add read-only operation support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Inconsistent rate calculation between `determine_rate` and `recalc_rate` due to division order.
--

commit 39f872462c04ad724c3624b152234160de56a47e
Author: Jerome Brunet <jbrunet@baylibre.com>

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
> index 62d680ccb524b..d1c8c7baa12dc 100644
> --- a/drivers/clk/sunxi-ng/ccu_div.c
> +++ b/drivers/clk/sunxi-ng/ccu_div.c
[ ... ]
> @@ -84,6 +84,36 @@ static int ccu_div_determine_rate(struct clk_hw *hw,
>  					     req, ccu_div_determine_rate_helper, cd);
>  }
>  
> +static int ccu_rodiv_determine_rate(struct clk_hw *hw,
> +				    struct clk_rate_request *req)
> +{
> +	struct ccu_div *cd = hw_to_ccu_div(hw);
> +	unsigned long val;
> +	u32 reg;
> +	int ret;
> +
> +	reg = readl(cd->common.base + cd->common.reg);
> +	val = reg >> cd->div.shift;
> +	val &= (1 << cd->div.width) - 1;
> +
> +	req->rate = ccu_mux_helper_unapply_prediv(&cd->common, &cd->mux, -1,
> +						  req->rate);
> +
> +	if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
> +		req->rate *= cd->fixed_post_div;
> +
> +	ret = divider_ro_determine_rate(hw, req, cd->div.table,
> +					cd->div.width, cd->div.flags, val);
> +
> +	if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
> +		req->rate /= cd->fixed_post_div;
> +
> +	req->rate = ccu_mux_helper_apply_prediv(&cd->common, &cd->mux, -1,
> +						req->rate);

[Severity: Medium]
Will this division order cause inconsistent rate calculations compared to
ccu_div_recalc_rate()?

The divider_ro_determine_rate() helper applies DIV_ROUND_UP() to the unadjusted
parent rate, and then this function divides that result by fixed_post_div and
prediv.

However, ccu_div_recalc_rate() applies prediv to the parent rate first, then
calculates the divider rate, and finally divides by fixed_post_div.

Since the Common Clock Framework requires determine_rate and recalc_rate to be
mathematically consistent, could this discrepancy cause rate mismatches or
warnings for clocks using a pre-divider or fixed post-divider?

> +
> +	return ret;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-a733-rtc-v4-0-f330728db3d3@baylibre.com?part=6