Re: [PATCH net v3 3/4] dpll: zl3073x: reject inexact frequencies for N-divided outputs

[Petr Oros <poros@redhat.com>]

On 10/6/26 5:31 PM, Ivan Vecera wrote:
> For N-divided outputs the P-pin and N-pin share the output divisor and
> the N-pin frequency is synth_freq / (div * esync_n_period). The frequency
> set helper computed esync_n_period with a truncating division and only
> rejected a zero result, so a frequency that does not divide evenly was
> silently rounded. Setting the P-pin could then shift the N-pin frequency
> too (e.g. 600 MHz synth, div=60, period=10 gives P=10 MHz, N=1 MHz;
> setting P to 2.5 MHz moves N to 1.25 MHz).
> 
> Check the division remainder and reject the request if it is not exact,
> and require esync_n_period >= 2 so the N-pin frequency stays below the
> P-pin one.
> 
> Fixes: ce26d7ca50a5 ("dpll: zl3073x: Add support to get/set frequency on pins")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/dpll/zl3073x/dpll.c | 37 +++++++++++++++++++++++++++++++------
>   1 file changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
> index 7c997966c3c3..65107b4cc4f8 100644
> --- a/drivers/dpll/zl3073x/dpll.c
> +++ b/drivers/dpll/zl3073x/dpll.c
> @@ -1007,15 +1007,28 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
>   
>   	if (zl3073x_dpll_is_p_pin(pin)) {
>   		/* We are going to change output frequency for P-pin but
> -		 * if the requested frequency is less than current N-pin
> -		 * frequency then indicate a failure as we are not able
> +		 * if the requested frequency is less or equal than current
> +		 * N-pin frequency then indicate a failure as we are not able
>   		 * to compute N-pin divisor to keep its frequency unchanged.
>   		 *
>   		 * Update divisor for N-pin to keep N-pin frequency.
>   		 */
> -		out.esync_n_period = (out.esync_n_period * out.div) / new_div;
> -		if (!out.esync_n_period)
> +		u64 prod = mul_u32_u32(out.esync_n_period, out.div);
> +		u32 rem;
> +
> +		out.esync_n_period = div_u64_rem(prod, new_div, &rem);
> +		if (rem != 0) {
> +			NL_SET_ERR_MSG_FMT(extack,
> +					   "OUT%uN freq must divide OUT%uP freq",
> +					   out_id, out_id);
>   			return -EINVAL;
> +		}
> +		if (out.esync_n_period < 2) {
> +			NL_SET_ERR_MSG_FMT(extack,
> +					   "OUT%uN freq must be less than OUT%uP freq",
> +					   out_id, out_id);
> +			return -EINVAL;
> +		}
>   
>   		/* Update the output divisor */
>   		out.div = new_div;
> @@ -1030,9 +1043,21 @@ zl3073x_dpll_output_pin_frequency_set(const struct dpll_pin *dpll_pin,
>   		 *
>   		 * Update divisor for N-pin
>   		 */
> -		out.esync_n_period = div64_u64(synth_freq, frequency * out.div);
> -		if (!out.esync_n_period)
> +		u64 rem, prod = frequency * out.div;
> +
> +		out.esync_n_period = div64_u64_rem(synth_freq, prod, &rem);
> +		if (rem != 0) {
> +			NL_SET_ERR_MSG_FMT(extack,
> +					   "OUT%uN freq must divide OUT%uP freq",
> +					   out_id, out_id);
>   			return -EINVAL;
> +		}
> +		if (out.esync_n_period < 2) {
> +			NL_SET_ERR_MSG_FMT(extack,
> +					   "OUT%uN freq must be less than OUT%uP freq",
> +					   out_id, out_id);
> +			return -EINVAL;
> +		}
>   	}
>   
>   	/* For 50/50 duty cycle the divisor is equal to width */

Reviewed-by: Petr Oros <poros@redhat.com>