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

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 7 May 2026 08:12:01 +0200 Ivan Vecera wrote:
> >> @@ -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?

TAL at the fields at the beginning of struct ethtool_ops
If we had two bits in the ops struct for driver to declare / opt-in
to each context the core can avoid calling the driver if it doesn't
support a context.