Re: [PATCH net-next v9 10/15] dpll: sit9531x: add support to adjust output phase

On 9/17/26 11:55 AM, Ivan Vecera wrote:

quoted

+	if (phase_ps == 0) {
+		abs_ps = 0;
+	} else if (phase_ps > 0) {
+		abs_ps = (u64)phase_ps;
+		div64_u64_rem(abs_ps, t_out_ps, &abs_ps);
+		phase_norm_ps = abs_ps;
+	} else {
+		u64 advance = (u64)(-(s64)phase_ps);
+
+		div64_u64_rem(advance, t_out_ps, &advance);
+		phase_norm_ps = -(s64)advance;
+		abs_ps = (advance == 0) ? 0 : (t_out_ps - advance);
+	}

This if-else branches could be reduced to:
<snip>
abs_ps = abs(phase_ps); /* Safe. INT_MIN is not possible here*/
div64_u64_rem(abs_ps, t_out_ps, &abs_ps);
phase_norm_ps = phase_ps < 0 ? -(s64)abs_ps : (s64)abs_ps;
abs_ps = phase_ps < 0 && abs_ps ? t_out_ps - abs_ps : abs_ps;
</snip>

Taken as written. v10 has those four lines in place of the three-way
branch, with only a pair of parentheses added on the last one.

Your INT_MIN note holds, and for a reason I wanted to confirm rather
than take on trust: the core rejects a phase-adjust request outside the
advertised phase range before the driver is called, and that range is
+/-1 ms expressed in ps. The value that reaches this function is
therefore bounded well inside s32, and abs() cannot be handed INT_MIN.
The comment above the block now says so, so the next reader does not
have to re-derive it.

quoted

+	coarse = mul_u64_u64_div_u64(abs_ps, fvco, 1000000000000ULL);
+	if (coarse >= (1ULL << SIT9531X_OUT_PRG_COARSE_BITS))
+		return -ERANGE;
+
+	/* Fine delay = round((abs_ps - coarse * vco_period_ps) / 30 ps) */
+	coarse_ps = mul_u64_u64_div_u64(coarse, 1000000000000ULL, fvco);
+	rem_ps = (abs_ps > coarse_ps) ? (abs_ps - coarse_ps) : 0;
+	if (rem_ps) {
+		u64 steps;
+
+		steps = div64_u64(rem_ps + SIT9531X_OUT_PRG_FINE_STEP_PS / 2,
+				  SIT9531X_OUT_PRG_FINE_STEP_PS);
+		if (steps > SIT9531X_OUT_PRG_FINE_MAX)
+			steps = SIT9531X_OUT_PRG_FINE_MAX;
+		fine = (u8)steps;
+	}

Also this whole block can be skipped (and set coarse to 0) in case of 
abs_ps == 0 to avoid expensive divisions.

Also taken. The coarse and fine encoding now sits inside an
if (abs_ps) block, and coarse is initialized to 0 at its declaration,
so a zero offset does no division at all. rem_ps moved inside the block
with it, since nothing outside needs it.

Patch 10 in v10 carries

  Suggested-by: Ivan Vecera [off-list ref]

v10 is posted:
https://lore.kernel.org/netdev/20260921201108.42676-1-arouhi@sitime.com/ (local)

Thanks,
Ali