Re: [PATCH 2/5] clk: qcom: clk-rcg2: use 64-bit arithmetic in set_duty_cycle()

[Konrad Dybcio]

On 4/6/26 5:54 PM, Xilin Wu wrote:
> The duty cycle calculation in clk_rcg2_set_duty_cycle() computes
> "n * duty->num * 2" using u32 arithmetic. When n is large and
> duty->num is also large, the intermediate result overflows u32.
> 
> For example, requesting 50% duty on a 1 kHz output derived from a
> 19.2 MHz parent gives n=19200, duty->num=500000, duty->den=1000000:
> 
>   19200 * 500000 * 2 = 19,200,000,000 > U32_MAX (4,294,967,295)
> 
> The truncated result produces a completely wrong duty cycle (5.26%
> instead of the requested 50%).
> 
> Use DIV_ROUND_CLOSEST_ULL() with an explicit (u64) cast to prevent
> the overflow.
> 
> Fixes: 7f891faf596e ("clk: qcom: clk-rcg2: Add support for duty-cycle for RCG")
> Signed-off-by: Xilin Wu <sophon@xxxxxxxxx>
> ---

I hate that we're still hitting this class of bugs in 2026

>  drivers/clk/qcom/clk-rcg2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 82ee7ca1703a..0e8f0473897e 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -783,7 +783,7 @@ static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
>  	n = (~(notn_m) + m) & mask;
>  
>  	/* Calculate 2d value */
> -	d = DIV_ROUND_CLOSEST(n * duty->num * 2, duty->den);
> +	d = DIV_ROUND_CLOSEST_ULL((u64)n * duty->num * 2, duty->den);

This may look scary given we feed it back into a u32 again, but given the
CCF has a check that ensures 0 < duty <= 100% and I think n is generally
< 16b wide, this should indeed never overflow

Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>

Konrad