[PATCH] staging: sm750fb: clean up PLL calculation code

[PATCH] staging: sm750fb: clean up PLL calculation code

[himanshu bakshi]

From: Himanshu Bakshi <himanshu.bakshi478@gmail.com>

Improve the PLL calculation function's documentation and
readability.

Simplify candidate selection by skipping candidates that do
not improve the frequency error, and defer updating the PLL
parameters until the best candidate has been selected.

Signed-off-by: Himanshu Bakshi <himanshu.bakshi478@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.c | 84 +++++++++++++++++----------
 1 file changed, 54 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index aa78d0bc1..74ce320b6 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -269,21 +269,18 @@ int ddk750_init_hw(struct initchip_param *p_init_param)
 }
 
 /*
- * monk liu @ 4/6/2011:
- *	re-write the calculatePLL function of ddk750.
- *	the original version function does not use
- *	some mathematics tricks and shortcut
- *	when it doing the calculation of the best N,M,D combination
- *	I think this version gives a little upgrade in speed
+ * Calculate the PLL parameters that produce the clock frequency
+ * closest to the requested frequency.
  *
- * 750 pll clock formular:
- * Request Clock = (Input Clock * M )/(N * X)
+ * The SM750 PLL uses the following formula:
  *
- * Input Clock = 14318181 hz
- * X = 2 power D
- * D ={0,1,2,3,4,5,6}
- * M = {1,...,255}
- * N = {2,...,15}
+ *     request_clock = (input_clock * M) / (N * 2^D)
+ *
+ * M ranges from 1 to 255, N from 2 to 15, and D from 0 to 6.
+ * The selected parameters minimize the difference between the
+ * resulting clock frequency and the requested frequency.
+ *
+ * Original implementation by Monk Liu @ 4/6/2011.
  */
 unsigned int sm750_calc_pll_value(unsigned int request_orig,
 				  struct pll_value *pll)
@@ -293,12 +290,15 @@ unsigned int sm750_calc_pll_value(unsigned int request_orig,
 	 * N located in 2,15 and M located in 1,255
 	 */
 	int N, M, X, d;
+	unsigned long N_best, M_best, POD_best, OD_best;
+	unsigned int diff;
 	int mini_diff;
 	unsigned int RN, quo, rem, fl_quo;
 	unsigned int input, request;
 	unsigned int tmp_clock, ret;
 	const int max_OD = 3;
 	int max_d = 6;
+	bool found_best = false;
 
 	if (sm750_get_chip_type() == SM750LE) {
 		/*
@@ -335,26 +335,50 @@ unsigned int sm750_calc_pll_value(unsigned int request_orig,
 			X = BIT(d);
 			M = quo * X;
 			M += fl_quo * X / 10000;
+
 			/* round step */
-			M += (fl_quo * X % 10000) > 5000 ? 1 : 0;
-			if (M < 256 && M > 0) {
-				unsigned int diff;
-
-				tmp_clock = pll->input_freq * M / N / X;
-				diff = abs(tmp_clock - request_orig);
-				if (diff < mini_diff) {
-					pll->M = M;
-					pll->N = N;
-					pll->POD = 0;
-					if (d > max_OD)
-						pll->POD = d - max_OD;
-					pll->OD = d - pll->POD;
-					mini_diff = diff;
-					ret = tmp_clock;
-				}
-			}
+			if ((fl_quo * X % 10000) > 5000)
+				M++;
+
+			/* Reject M values outside the supported range. */
+			if (M >= 256 || M <= 0)
+				continue;
+
+			/* Calculate the resulting clock frequency. */
+			tmp_clock = pll->input_freq * M / N / X;
+			diff = abs(tmp_clock - request_orig);
+
+			/*
+			 * Skip candidates that do not
+			 * improve the frequency error.
+			 */
+			if (diff >= mini_diff)
+				continue;
+
+			/* Update the best PLL parameters. */
+			M_best = M;
+			N_best = N;
+			POD_best = 0;
+
+			if (d > max_OD)
+				POD_best = d - max_OD;
+
+			OD_best = d - POD_best;
+			mini_diff = diff;
+			ret = tmp_clock;
+
+			found_best = true;
 		}
 	}
+
+	/* Apply the best PLL parameters if a valid candidate was found. */
+	if (found_best) {
+		pll->M = M_best;
+		pll->N = N_best;
+		pll->POD = POD_best;
+		pll->OD = OD_best;
+	}
+
 	return ret;
 }
 
-- 
2.56.0

Re: [PATCH] staging: sm750fb: clean up PLL calculation code

[Greg KH]

On Sat, Oct 24, 2026 at 01:05:49PM +0530, himanshu bakshi wrote:
> From: Himanshu Bakshi <himanshu.bakshi478@gmail.com>
> 
> Improve the PLL calculation function's documentation and
> readability.
> 
> Simplify candidate selection by skipping candidates that do
> not improve the frequency error, and defer updating the PLL
> parameters until the best candidate has been selected.

How was this found and tested?

thanks,

greg k-h

Re: [PATCH] staging: sm750fb: clean up PLL calculation code

[Himanshu Bakshi]

Hi Greg,

I found this while reviewing sm750_calc_pll_value() and looking for a
cleaner way to handle the PLL candidate calculation.

I tested the change as follows:

Built the sm750fb driver successfully using make -j2
M=drivers/staging/sm750fb W=1.

Ran a standalone C differential regression test comparing the original
and modified algorithms across 2,010,084 test cases, including
frequency sweeps, boundary values, and randomized requests.

Compared the return values and resulting PLL fields. All tested cases matched.

The differential test uses standalone copies of the original and
modified algorithms rather than executing the function inside the
kernel, so it verifies that the two implementations behave identically
over the tested inputs. I have not tested this on physical SM750
hardware. If by any chance you would like to see the test c file, let
me know.

Thanks and Regards,
Himanshu Bakshi


On Sat, Oct 24, 2026 at 1:32 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Oct 24, 2026 at 01:05:49PM +0530, himanshu bakshi wrote:
> > From: Himanshu Bakshi <himanshu.bakshi478@gmail.com>
> >
> > Improve the PLL calculation function's documentation and
> > readability.
> >
> > Simplify candidate selection by skipping candidates that do
> > not improve the frequency error, and defer updating the PLL
> > parameters until the best candidate has been selected.
>
> How was this found and tested?
>
> thanks,
>
> greg k-h

Re: [PATCH] staging: sm750fb: clean up PLL calculation code

[Greg KH]

On Sat, Oct 24, 2026 at 05:20:41PM +0530, Himanshu Bakshi wrote:
> Hi Greg,

Please do not top-post or send html email, the list rejects that.

> I found this while reviewing sm750_calc_pll_value() and looking for a
> cleaner way to handle the PLL candidate calculation.

But why does it need to be "cleaner"?

> I tested the change as follows:
> 
> Built the sm750fb driver successfully using make -j2
> M=drivers/staging/sm750fb W=1.
> 
> Ran a standalone C differential regression test comparing the original
> and modified algorithms across 2,010,084 test cases, including
> frequency sweeps, boundary values, and randomized requests.

Ah, you used a LLM.  You have read:
	https://lore.kernel.org/r/2026080354-skater-urgent-31b2@gregkh
right?

Sorry,

greg k-h

Re: [PATCH] staging: sm750fb: clean up PLL calculation code

[Himanshu Bakshi]

Hi Greg,

Thank you for the feedback. I have read the discussion you linked.

My motivation for this patch was that I found the existing PLL
calculation difficult to follow when I was trying to understand it. I
wanted to document the formula, make the candidate-selection logic
easier to follow, and reduce the nesting.

I understand that making code look cleaner by itself may not be a
sufficient reason to change existing kernel code. I wanted to improve
the readability and understandability of this function, but I should
have explained the concrete benefit more clearly in the commit
message.

Regarding the LLM, I used it to help generate a standalone
differential test for comparing the original and modified algorithms.
The refactoring was my attempt to improve the code while learning how
to contribute to the kernel. I ran the generated test, but I
understand that testing copies of the implementations is not the same
as testing the actual kernel function.

I am new to kernel development and did not have a good understanding
of the contribution workflow when I started. I used an LLM as a
learning aid, but I recognize that I am responsible for understanding
and justifying every change and test I submit.

I will also make sure to follow the mailing list's plain-text and
bottom-posting conventions.

Thank you for taking the time to explain these issues.

Thanks and Regards,
         Himanshu Bakshi

On Sat, Oct 24, 2026 at 7:40 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Oct 24, 2026 at 05:20:41PM +0530, Himanshu Bakshi wrote:
> > Hi Greg,
>
> Please do not top-post or send html email, the list rejects that.
>
> > I found this while reviewing sm750_calc_pll_value() and looking for a
> > cleaner way to handle the PLL candidate calculation.
>
> But why does it need to be "cleaner"?
>
> > I tested the change as follows:
> >
> > Built the sm750fb driver successfully using make -j2
> > M=drivers/staging/sm750fb W=1.
> >
> > Ran a standalone C differential regression test comparing the original
> > and modified algorithms across 2,010,084 test cases, including
> > frequency sweeps, boundary values, and randomized requests.
>
> Ah, you used a LLM.  You have read:
>         https://lore.kernel.org/r/2026080354-skater-urgent-31b2@gregkh
> right?
>
> Sorry,
>
> greg k-h