Re: [PATCH net-next v3 1/2] dpll: add fractional frequency offset to pin-parent-device

[Ivan Vecera]

On 5/7/26 3:33 AM, Jakub Kicinski wrote:
> On Mon,  4 May 2026 17:53:39 +0200 Ivan Vecera wrote:
> > +          At top level this represents the RX vs TX symbol rate
> > +          offset on the media associated with the pin.
>
> Isn't this a hacky hack? I'd think that pin is in or out.
> Having a freq offset between two pins or pin and parent's
> ref lock makes sense. This new interpretation sounds like
> we are trying to shove a difference between two pins into one?

The "RX vs TX symbol rate offset" description is not something I
introduced — it is the original documentation of the
fractional-frequency-offset attribute as defined by Jiri. The -ppt
variant was added later purely for higher precision. This patch just
unifies the documentation of both attributes.

I'm not sure I fully understand what the original "RX vs TX" semantics
were meant to capture. Jiri, could you clarify what you had in mind
and whether we should keep or change that description?

> > @@ -299,6 +299,10 @@ zl3073x_dpll_input_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
> >   {
> >   	struct zl3073x_dpll_pin *pin = pin_priv;
> >   
> > +	/* Only rx vs tx symbol rate FFO is supported */
> > +	if (dpll)
> > +		return -ENODATA;
> > +
> >   	*ffo = pin->freq_offset;
>
> It's easy for driver authors to forget this sort of validation.
> We should fail close, so it's better to have some "capability"
> bits or something for the driver to opt into getting given format
> of the call.

Regarding the fail-close concern — I agree that relying on drivers
to check dpll==NULL is fragile. A capability bit alone wouldn't help
though, since the driver still needs to distinguish which FFO context
is being requested.

I can think of two approaches:
1. An explicit bool parameter (e.g. `bool per_parent`) instead of
   overloading the dpll pointer for context distinction.
2. Separate callbacks for each FFO context (e.g. ffo_get for the
   top-level and ffo_parent_get for the per-parent).

Do you have a preference, or something else in mind?

Thanks,
Ivan