Re: [PATCH net-next 1/2] dt-bindings: dpll: Add clock ID property

[Ivan Vecera <ivecera@redhat.com>]

On 23. 07. 25 8:25 dop., Krzysztof Kozlowski wrote:
> On 21/07/2025 14:54, Ivan Vecera wrote:
>> On 21. 07. 25 11:23 dop., Krzysztof Kozlowski wrote:
>>> On Fri, Jul 18, 2025 at 02:16:41PM +0200, Ivan Vecera wrote:
>>>> Hi Krzysztof,
>>>>
>>>> ...
>>>>
>>>> The clock-id property name may have been poorly chosen. This ID is used by
>>>> the DPLL subsystem during the registration of a DPLL channel, along with its
>>>> channel ID. A driver that provides DPLL functionality can compute this
>>>> clock-id from any unique chip information, such as a serial number.
>>>>
>>>> Currently, other drivers that implement DPLL functionality are network
>>>> drivers, and they generate the clock-id from one of their MAC addresses by
>>>> extending it to an EUI-64.
>>>>
>>>> A standalone DPLL device, like the zl3073x, could use a unique property such
>>>> as its serial number, but the zl3073x does not have one. This patch-set is
>>>> motivated by the need to support such devices by allowing the DPLL device ID
>>>> to be passed via the Device Tree (DT), which is similar to how NICs without
>>>> an assigned MAC address are handled.
>>>
>>> You use words like "unique" and MAC, thus I fail to see how one fixed
>>> string for all boards matches this. MACs are unique. Property value set
>>> in DTS for all devices is not.
>>>> You also need to explain who assigns this value (MACs are assigned) or
>>> if no one, then why you cannot use random? I also do not see how this
>>> property solves this...  One person would set it to value "1", other to
>>> "2" but third decide to reuse "1"? How do you solve it for all projects
>>> in the upstream?
>>
>> Some background: Any DPLL driver has to use a unique number during the
>> DPLL device/channel registration. The number must be unique for the
>> device across a clock domain (e.g., a single PTP network).
>>
>> NIC drivers that expose DPLL functionality usually use their MAC address
>> to generate such a unique ID. A standalone DPLL driver does not have
>> this option, as there are no NIC ports and therefore no MAC addresses.
>> Such a driver can use any other source for the ID (e.g., the chip's
>> serial number). Unfortunately, this is not the case for zl3073x-based
>> hardware, as its current firmware revisions do not expose information
>> that could be used to generate the clock ID (this may change in the
>> future).
>>
>> There is no authority that assigns clock ID value ranges similarly to
>> MAC addresses (OUIs, etc.), but as mentioned above, uniqueness is
>> required across a single PTP network so duplicates outside this
>> single network are not a problem.
> 
> You did not address main concern. You will configure the same value for
> all boards, so how do you solve uniqueness within PTP network?

This value differs across boards, similar to the local-mac-address. The
device tree specifies the entry, and the bootloader or system firmware
(like U-Boot) provides the actual value.

>> A randomly generated clock ID works, but the problem is that the value
>> is different after each reboot. Yes, there is an option to override the
>> clock ID using the devlink interface, but this also has to be done after
>> every reboot or power-up.
>>
>>> All this must be clearly explained when you add new, generic property.
>>
>> Would it be acceptable to define a hardware-specific property, since
>> only this hardware has this particular problem (the absence of a chip
>> unique attribute)? I'm referring to a property like 'microchip,id' or
>> 'microchip,dpll-id' defined in microchip,zl30731.yaml.
> 
> It does not change anything, no problems solved.
> 
> 
> Best regards,
> Krzysztof
>