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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <jxj23rczlysmrrrzdmtaa2ymrntamp2hgkzwnfaxgnnzsqqxoy@l5shaguts5oj>
Date: Wed, 24 Dec 2025 21:51:05 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Taniya Das <taniya.das@....qualcomm.com>
Cc: Michael Turquette <mturquette@...libre.com>, 
	Stephen Boyd <sboyd@...nel.org>, Taniya Das <quic_tdas@...cinc.com>, 
	Ajit Pandey <ajit.pandey@....qualcomm.com>, Imran Shaik <imran.shaik@....qualcomm.com>, 
	Jagadeesh Kona <jagadeesh.kona@....qualcomm.com>, linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: qcom: rcg2: compute 2d using duty fraction directly

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@...cinc.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@...cinc.com>
> >> ---
> >> Signed-off-by: Taniya Das <taniya.das@....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@....qualcomm.com>
> >>
> 
> -- 
> Thanks,
> Taniya Das
>