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

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

Posted by Taniya Das 1 month, 2 weeks ago

From: Taniya Das <quic_tdas@quicinc.com>

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);
 
 	/*
 	 * 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>

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

Posted by Bjorn Andersson 1 month, 2 weeks ago

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.

> 
> 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?

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>
>

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

Posted by Stephen Boyd 1 month, 2 weeks ago

Quoting Bjorn Andersson (2025-12-22 09:09:54)
> On Mon, Dec 22, 2025 at 10:38:14PM +0530, Taniya Das wrote:
> > @@ -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?
> 

Even better would be to add some KUnit tests for the qcom clk driver
functions like this so we know they're still working.

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

Posted by Taniya Das 1 month, 1 week ago


On 12/25/2025 3:25 AM, Stephen Boyd wrote:
> Quoting Bjorn Andersson (2025-12-22 09:09:54)
>> On Mon, Dec 22, 2025 at 10:38:14PM +0530, Taniya Das wrote:
>>> @@ -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?
>>
> 
> Even better would be to add some KUnit tests for the qcom clk driver
> functions like this so we know they're still working.


Sure Stephen, let me take a look.

-- 
Thanks,
Taniya Das

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

Posted by Taniya Das 1 month, 2 weeks ago


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

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

Posted by Bjorn Andersson 1 month, 1 week ago

On Tue, Dec 23, 2025 at 04:18:20PM +0530, Taniya Das wrote:
> 
> 
> 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.
> 

I don't think these examples need to necessarily be added in the git
history - in particular since the proposed new style looks more
reasonable than what's currently is in the code.

So, providing them here would suffice, for me at least.


Adding kunit tests certainly sounds useful though.

Regards,
Bjorn

> > 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
>

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

Posted by Taniya Das 1 month, 1 week ago


On 12/25/2025 9:21 AM, Bjorn Andersson wrote:
> On Tue, Dec 23, 2025 at 04:18:20PM +0530, Taniya Das wrote:
>>
>>
>> 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.
>>
> 
> I don't think these examples need to necessarily be added in the git
> history - in particular since the proposed new style looks more
> reasonable than what's currently is in the code.
> 
> So, providing them here would suffice, for me at least.

Frequency requirement from customers as below.

F(10000, P_BI_TCXO, 2, 1, 960),

For example, with N = 960.

Duty cycle(%)| num/den | d_old |NOT_2D(old)| d_new |NOT_2D(new)|Match
--------------------------------------------------------------------
0.05         | 1/2000  | 0     |0x0000FFFF |  1    |0x0000FFFE |No
0.10         | 1/1000  | 0     |0x0000FFFF |  2    |0x0000FFFD |No
0.3125       | 1/320   | 0     |0x0000FFFF |  6    |0x0000FFF9 |No
0.50         | 1/200   | 0     |0x0000FFFF |  10   |0x0000FFF5 |No
0.78125      | 1/128   | 0     |0x0000FFFF |  15   |0x0000FFF0 |No
2.00         | 1/50    | 38    |0x0000FFD9 |  38   |0x0000FFD9 |Yes
2.10         | 7/333   | 38    |0x0000FFD9 |  40   |0x0000FFD7 |No
2.50         | 1/40    | 38    |0x0000FFD9 |  48   |0x0000FFCF |No
3.00         | 3/100   | 58    |0x0000FFC5 |  58   |0x0000FFC5 |Yes



> 
> 
> Adding kunit tests certainly sounds useful though.
> 

Sure, will take a look.


-- 
Thanks,
Taniya Das