Re: [PATCH net-next 1/2] dpll: add phase-adjust-gran pin attribute

[Vadim Fedorenko]

On 24/10/2025 15:49, Ivan Vecera wrote:
> Phase-adjust values are currently limited by a min-max range. Some
> hardware requires, for certain pin types, that values be multiples of
> a specific granularity, as in the zl3073x driver.
>
> Add a `phase-adjust-gran` pin attribute and an appropriate field in
> dpll_pin_properties. If set by the driver, use its value to validate
> user-provided phase-adjust values.
>
> Reviewed-by: Michal Schmidt <mschmidt@xxxxxxxxxx>
> Reviewed-by: Petr Oros <poros@xxxxxxxxxx>
> Tested-by: Prathosh Satish <Prathosh.Satish@xxxxxxxxxxxxx>
> Signed-off-by: Ivan Vecera <ivecera@xxxxxxxxxx>
> ---
>   Documentation/driver-api/dpll.rst     | 36 +++++++++++++++------------
>   Documentation/netlink/specs/dpll.yaml |  7 ++++++
>   drivers/dpll/dpll_netlink.c           | 12 ++++++++-
>   include/linux/dpll.h                  |  1 +
>   include/uapi/linux/dpll.h             |  1 +
>   5 files changed, 40 insertions(+), 17 deletions(-)
>
> @@ -1261,7 +1265,13 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
>   	if (phase_adj > pin->prop.phase_range.max ||
>   	    phase_adj < pin->prop.phase_range.min) {
>   		NL_SET_ERR_MSG_ATTR(extack, phase_adj_attr,
> -				    "phase adjust value not supported");
> +				    "phase adjust value of out range");
> +		return -EINVAL;
> +	}
> +	if (pin->prop.phase_gran && phase_adj % pin->prop.phase_gran) {
> +		NL_SET_ERR_MSG_ATTR_FMT(extack, phase_adj_attr,
> +					"phase adjust value not multiple of %u",
> +					pin->prop.phase_gran);

That is pretty strict on the uAPI input. Maybe it's better to allow any
value, but report back the one that is really applied based on driver's
understanding of hardware? I mean the driver is doing some math before
applying the math, it can potentially round any value to the closest
acceptable and report it back?

>   		return -EINVAL;
>   	}
>