Re: [PATCH net-next v9 04/15] dpll: sit9531x: read DPLL types and pin properties from system firmware

[Ivan Vecera]

On 9/15/26 2:00 AM, Ali Rouhi wrote:
> From: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@xxxxxxxxxxxxxxxxx>
>
> The DPLL core wants a type per device and a property set per pin: a
> label, the direction, the capabilities, and the frequencies the pin
> supports.  None of that can be read from the chip -- which frequencies a
> board actually presents on a given input, and whether an output's
> embedded sync is wired to be controllable, are board facts -- so they
> come from the firmware node, with defaults for a node that does not
> describe them.
>
> Input pins are interleaved P and N lanes of four differential pairs, so a
> logical index maps to a pair and a lane, and a pair configured
> single-ended presents two independent inputs where a differential one
> presents one.  The labels follow from that, and the two extra input
> positions -- the crystal and the inter-PLL sync net -- are named
> separately.
>
> Kept in its own file, and introduced before anything is registered, so
> the registration code that follows has nothing to say about firmware.
>
> Signed-off-by: Oleg Zadorozhnyi <Oleg.Zadorozhnyi@xxxxxxxxxxxxxxxxx>
> Assisted-by: Claude:claude-4-opus [chat]
> Signed-off-by: Ali Rouhi <arouhi@xxxxxxxxxx>
> ---
>   drivers/dpll/sit9531x/Makefile |   2 +-
>   drivers/dpll/sit9531x/core.h   |  72 ++++++
>   drivers/dpll/sit9531x/prop.c   | 437 +++++++++++++++++++++++++++++++++
>   drivers/dpll/sit9531x/prop.h   |  39 +++
>   4 files changed, 549 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/dpll/sit9531x/prop.c
>   create mode 100644 drivers/dpll/sit9531x/prop.h
>
> ...
> +
> +               /*
> +                * Allow phase-adjust over a +/-1 ms window.  The subsystem
> +                * rejects pin_set(phase-adjust, X) when X falls outside
> +                * [min, max], so leaving these at 0 silently blocks every
> +                * netlink call.  1 ms is well beyond the DCO dynamic range
> +                * but costs nothing.  Only outputs get a range: input pins
> +                * have no .phase_adjust_set, and advertising one there would
> +                * promise userspace something every set would refuse.
> +                */
> +               props->dpll_props.phase_range.min = -1000000000; /* -1 ms in ps */
> +               props->dpll_props.phase_range.max =  1000000000; /* +1 ms in ps */
> +               /*
> +                * The fine step is 30 ps, but requests are accepted at 1 ps
> +                * resolution and rounded to the nearest achievable delay, so
> +                * advertise the request granularity, not the hardware step.
> +                */
> +               props->dpll_props.phase_gran = 1;

What is the real HW granularity? If 30ps then you should announce 30 to
inform the userspace that this is the step supported by the HW.

Thanks,
Ivan