Re: [PATCH] clk: qcom: rcg2: compute 2d using duty fraction directly

[Taniya Das <taniya.das@oss.qualcomm.com>]

On 12/23/2025 12:39 AM, Bjorn Andersson wrote:
> On Mon, Dec 22, 2025 at 10:38:14PM +0530, Taniya Das wrote:
>> From: Taniya Das <quic_tdas@quicinc.com>
> 
> Please use oss.qualcomm.com.
> 

My bad, will update it.

>>
>> The duty-cycle calculation in clk_rcg2_set_duty_cycle() currently
>> derives an intermediate percentage `duty_per = (num * 100) / den` and
>> then computes:
>>
>>     d = DIV_ROUND_CLOSEST(n * duty_per * 2, 100);
>>
>> This introduces integer truncation at the percentage step (division by
>> `den`) and a redundant scaling by 100, which can reduce precision for
>> large `den` and skew the final rounding.
>>
>> Compute `2d` directly from the duty fraction to preserve precision and
>> avoid the unnecessary scaling:
>>
>>     d = DIV_ROUND_CLOSEST(n * duty->num * 2, duty->den);
>>
>> This keeps the intended formula `d ≈ n * 2 * (num/den)` while performing
>> a single, final rounded division, improving accuracy especially for small
>> duty cycles or large denominators. It also removes the unused `duty_per`
>> variable, simplifying the code.
>>
>> There is no functional changes beyond improved numerical accuracy.
>>
>> Fixes: 7f891faf596ed ("clk: qcom: clk-rcg2: Add support for duty-cycle for RCG")
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> ---
>> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
>> ---
>>  drivers/clk/qcom/clk-rcg2.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
>> index e18cb8807d73534c6437c08aeb524353a2eab06f..2838d4cb2d58ea1e351d6a5599045c72f4dc3801 100644
>> --- a/drivers/clk/qcom/clk-rcg2.c
>> +++ b/drivers/clk/qcom/clk-rcg2.c
>> @@ -755,7 +755,7 @@ static int clk_rcg2_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
>>  static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
>>  {
>>  	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
>> -	u32 notn_m, n, m, d, not2d, mask, duty_per, cfg;
>> +	u32 notn_m, n, m, d, not2d, mask, cfg;
>>  	int ret;
>>  
>>  	/* Duty-cycle cannot be modified for non-MND RCGs */
>> @@ -774,10 +774,8 @@ static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
>>  
>>  	n = (~(notn_m) + m) & mask;
>>  
>> -	duty_per = (duty->num * 100) / duty->den;
>> -
>>  	/* Calculate 2d value */
>> -	d = DIV_ROUND_CLOSEST(n * duty_per * 2, 100);
>> +	d = DIV_ROUND_CLOSEST(n * duty->num * 2, duty->den);
> 
> This looks better/cleaner. But for my understanding, can you share some
> example numbers that shows the problem?
> 

Sure Bjorn, will share the examples.

> Regards,
> Bjorn
> 
>>  
>>  	/*
>>  	 * Check bit widths of 2d. If D is too big reduce duty cycle.
>>
>> ---
>> base-commit: cc3aa43b44bdb43dfbac0fcb51c56594a11338a8
>> change-id: 20251222-duty_cycle_precision-796542baecab
>>
>> Best regards,
>> -- 
>> Taniya Das <taniya.das@oss.qualcomm.com>
>>

-- 
Thanks,
Taniya Das