Re: [PATCH net-next v2 2/2] dpll: use pin owner's dpll ref for pin-level attribute setting

[Ivan Vecera <ivecera@redhat.com>]

On 8/6/26 11:41 AM, Paolo Abeni wrote:
> On 8/3/26 2:02 PM, Ivan Vecera wrote:
>> ...
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>>   Documentation/driver-api/dpll.rst |  10 +-
>>   drivers/dpll/dpll_netlink.c       | 213 +++++++-----------------------
>>   2 files changed, 55 insertions(+), 168 deletions(-)
>>
>> diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
>> index f83150917814e2..6fb50e53475c09 100644
>> --- a/Documentation/driver-api/dpll.rst
>> +++ b/Documentation/driver-api/dpll.rst
>> @@ -116,8 +116,8 @@ Shared pins
>>   A single pin object can be attached to multiple dpll devices.
>>   Then there are two groups of configuration knobs:
>>   
>> -1) Set on a pin - the configuration affects all dpll devices pin is
>> -   registered to (i.e., ``DPLL_A_PIN_FREQUENCY``),
>> +1) Set on a pin - the configuration is performed through the pin owner's
>> +   dpll reference only (i.e., ``DPLL_A_PIN_FREQUENCY``),
> 
> I find the new text confusing; it seems to me that the pin configuration
> now affects a single DPLL.
> 
> Sashiko nipa has several comments, please have a look:
> 
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260803120245.56046-1-ivecera%40redhat.com
> 
> and also please be aware of net-next commit c82ff94592fb.

Paolo, thanks for pointing out...

Regarding Sashiko's findings in this patch...

 > Should this list item keep describing the observable effect rather than
 > the kernel call path? ... The new text reads as if the frequency change
 > is scoped to a single dpll device, while the commit message argues the
 > opposite.

You are right. The documentation should describe the user-visible
behavior, not the implementation. The pin is one physical entity and
the frequency change is visible on every dpll the pin is registered
to. Will reword to keep the observable-effect perspective.

 > The removed sentence carried the reason why the attribute must not be
 > nested in DPLL_A_PIN_PARENT_DEVICE ... After the rewording the uAPI
 > requirement is stated without any rationale.

Good point. Will restore the rationale — the attribute is not nested
in DPLL_A_PIN_PARENT_DEVICE because it is a property of the pin
itself, not of a specific pin-dpll relationship.

 > Does the zl3073x implementation added by the preceding patch still
 > report errors here? ... the per-channel priority commit failure is
 > only warned about, and the function then falls through to return 0.

See the reply to the preceding patch. The ref sync pair configuration
(zl3073x_ref_state_set) is the essential part and its failure is
propagated. The per-channel priority exclusion is advisory per the
datasheet — a failure there does not invalidate the ref sync pair
and is reported via dev_warn. The old code propagated the error only
because it was a single-channel write that happened to be the last
statement.

Thanks,
Ivan