[PATCH net-next 0/2] dpll: zl3073x: Read clock ID from device property

[PATCH net-next 0/2] dpll: zl3073x: Read clock ID from device property

[Ivan Vecera]

The current ZL3073x firmware revisions do not provide any information
unique for the particular chip that could be use to generate clock ID
necessary for a DPLL device registration.

Currently the driver generates random clock ID during probe and a user
have and option to change this value using devlink interface.

The situation is very similar to network controllers that do not have
assigned a fixed MAC address.

The purpose of this series is to allow to specify the clock ID property
through DT. If the property is not provided then the driver will use
randomly generated value as fallback.

Patch breakdown:
Patch 1 - adds clock-id property to dpll-device DT schema
Patch 2 - adds support for this DT property in zl3073x driver

Ivan Vecera (2):
  dt-bindings: dpll: Add clock ID property
  dpll: zl3073x: Initialize clock ID from device property

 .../devicetree/bindings/dpll/dpll-device.yaml |  5 +++
 drivers/dpll/zl3073x/core.c                   | 32 ++++++++++++++++---
 2 files changed, 32 insertions(+), 5 deletions(-)

-- 
2.49.1

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

[Ivan Vecera]

Add property to specify the ID of the clock that the DPLL device
drives. The ID value represents Unique Clock Identified (EUI-64)
defined by IEEE 1588 standard.

The property is not mandatory because some DPLL devices can have
an ability to read this from HW. The situation is very similar
to network controllers without assigned MAC address.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 Documentation/devicetree/bindings/dpll/dpll-device.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
index fb8d7a9a3693f..8e4ffe8ca279c 100644
--- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
+++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
@@ -27,6 +27,11 @@ properties:
   "#size-cells":
     const: 0
 
+  clock-id:
+    description: Specifies ID of the clock that the DPLL device drives
+    $ref: /schemas/types.yaml#/definitions/uint64
+    minimum: 1
+
   dpll-types:
     description: List of DPLL channel types, one per DPLL instance.
     $ref: /schemas/types.yaml#/definitions/non-unique-string-array
-- 
2.49.1

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

[Krzysztof Kozlowski]

On 17/07/2025 19:10, Ivan Vecera wrote:
> Add property to specify the ID of the clock that the DPLL device
> drives. The ID value represents Unique Clock Identified (EUI-64)
> defined by IEEE 1588 standard.

With the exception of clock-output-names and gpio-hogs, we do not define
how the output looks like in the provider bindings.

I also don't understand how this maps to channels and what "device
drives a clock" means. Plus how this is not deducible from the compatible...

So many questions.

And your driver code confirms that this looks like misrepresenting
consumer properties.

Best regards,
Krzysztof

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

[Ivan Vecera]

Hi Krzysztof,

On 18. 07. 25 8:55 dop., Krzysztof Kozlowski wrote:
> On 17/07/2025 19:10, Ivan Vecera wrote:
>> Add property to specify the ID of the clock that the DPLL device
>> drives. The ID value represents Unique Clock Identified (EUI-64)
>> defined by IEEE 1588 standard.
> 
> With the exception of clock-output-names and gpio-hogs, we do not define
> how the output looks like in the provider bindings.
> 
> I also don't understand how this maps to channels and what "device
> drives a clock" means. Plus how this is not deducible from the compatible...

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.

Suggestions:
1. Use the dpll-id property in dpll-device with the description:
    "Specifies the unique ID of the DPLL device if it is not retrievable
     from the hardware."
-or-
2. Use microchip,id or microchip,dpll-id with a similar description, as
    this issue is specific to this hardware, which does not provide such
    information.

Thanks for the advice.

Ivan

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

[Krzysztof Kozlowski]

On Fri, Jul 18, 2025 at 02:16:41PM +0200, Ivan Vecera wrote:
> Hi Krzysztof,
> 
> On 18. 07. 25 8:55 dop., Krzysztof Kozlowski wrote:
> > On 17/07/2025 19:10, Ivan Vecera wrote:
> > > Add property to specify the ID of the clock that the DPLL device
> > > drives. The ID value represents Unique Clock Identified (EUI-64)
> > > defined by IEEE 1588 standard.
> > 
> > With the exception of clock-output-names and gpio-hogs, we do not define
> > how the output looks like in the provider bindings.
> > 
> > I also don't understand how this maps to channels and what "device
> > drives a clock" means. Plus how this is not deducible from the compatible...
> 
> 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?

All this must be clearly explained when you add new, generic property.

Best regards,
Krzysztof

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

[Ivan Vecera]

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.

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.

> Best regards,
> Krzysztof

Thanks,
Ivan

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

[Krzysztof Kozlowski]

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?

> 
> 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

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

[Ivan Vecera]

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
> 

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

[Krzysztof Kozlowski]

On 23/07/2025 09:23, Ivan Vecera wrote:
> 
> 
> 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.
This should be clearly explained in commit msg or pull request to dtschema.

Where are patches for U-Boot? lore gives me 0 results.

Best regards,
Krzysztof

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

[Ivan Vecera]

On 03. 08. 25 1:12 odp., Krzysztof Kozlowski wrote:
> On 23/07/2025 09:23, Ivan Vecera wrote:
>>
>>
>> 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.
> This should be clearly explained in commit msg or pull request to dtschema.
> 
> Where are patches for U-Boot? lore gives me 0 results.

Hi Krzysztof,

This was just an idea how to provide such information. But...

We had a upstream meeting regarding this issue, how to deal with this
issue in situations where a DPLL device is used to drive a PHC present
in a network controller.

Let's say we have a SyncE setup with two network controllers where each
of them feeds a DPLL channel with recovered clock received from some of
its PHY. The DPLL channel cleans/stabilizes this input signal (generates
phase aligned signal locked to the same frequency as the input one) and
routes it back to the network controller.

     +-----------+
  +--|   NIC 1   |<-+
  |  +-----------+  |
  |                 |
  | RxCLK     TxCLK |
  |                 |
  |  +-----------+  |
  +->| channel 1 |--+
     |-- DPLL ---|
  +->| channel 2 |--+
  |  +-----------+  |
  |                 |
  | RxCLK     TxCLK |
  |                 |
  |  +-----------+  |
  +--|   NIC 2   |<-+
     +-----------+

The PHCs implemented by the NICs have associated the ClockIdentity
(according IEEE 1588-2008) whose value is typically derived from
the NIC's MAC address using EUI-64. The DPLL channel should be
registered to DPLL subsystem using the same ClockIdentity as the PHC
it drives. In above example DPLL channel 1 should have the same clock ID
as NIC1 PHC and channel 2 as NIC2 PHC.

During the discussion, Andrew had the idea to provide NIC phandles
instead of clock ID values.

Something like this:

diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml 
b/Documenta
tion/devicetree/bindings/dpll/dpll-device.yaml
index fb8d7a9a3693f..159d9253bc8ae 100644
--- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
+++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
@@ -33,6 +33,13 @@ properties:
      items:
        enum: [pps, eec]

+  ethernet-handles:
+    description:
+      List of phandles to Ethernet devices, one per DPLL instance. Each of
+      these handles identifies Ethernet device that uses particular DPLL
+      instance to synchronize its hardware clock.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
    input-pins:
      type: object
      description: DPLL input pins

A DPLL driver can then use this property to identify a network
controller, use fwnode_get_mac_address() to get assigned MAC address and
generate the ClockIdentity for registration from this MAC.

WDYT about it?

Thanks,
Ivan

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

[Andrew Lunn]

> Let's say we have a SyncE setup with two network controllers where each
> of them feeds a DPLL channel with recovered clock received from some of
> its PHY. The DPLL channel cleans/stabilizes this input signal (generates
> phase aligned signal locked to the same frequency as the input one) and
> routes it back to the network controller.
> 
>     +-----------+
>  +--|   NIC 1   |<-+
>  |  +-----------+  |
>  |                 |
>  | RxCLK     TxCLK |
>  |                 |
>  |  +-----------+  |
>  +->| channel 1 |--+
>     |-- DPLL ---|
>  +->| channel 2 |--+
>  |  +-----------+  |
>  |                 |
>  | RxCLK     TxCLK |
>  |                 |
>  |  +-----------+  |
>  +--|   NIC 2   |<-+
>     +-----------+
> 
> The PHCs implemented by the NICs have associated the ClockIdentity
> (according IEEE 1588-2008) whose value is typically derived from
> the NIC's MAC address using EUI-64. The DPLL channel should be
> registered to DPLL subsystem using the same ClockIdentity as the PHC
> it drives. In above example DPLL channel 1 should have the same clock ID
> as NIC1 PHC and channel 2 as NIC2 PHC.
> 
> During the discussion, Andrew had the idea to provide NIC phandles
> instead of clock ID values.
> 
> Something like this:
> 
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> b/Documenta
> tion/devicetree/bindings/dpll/dpll-device.yaml
> index fb8d7a9a3693f..159d9253bc8ae 100644
> --- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> @@ -33,6 +33,13 @@ properties:
>      items:
>        enum: [pps, eec]
> 
> +  ethernet-handles:
> +    description:
> +      List of phandles to Ethernet devices, one per DPLL instance. Each of
> +      these handles identifies Ethernet device that uses particular DPLL
> +      instance to synchronize its hardware clock.
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +

I personally would not use a list. I would have a node per channel,
and within that node, have the ethernet-handle property. This gives
you a more flexible scheme where you can easily add more per channel
properties in the future.

It took us a while to understand what you actually wanted. The ASCII
art helps. So i would include that and some text in the binding.

	Andrew

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

[Ivan Vecera]

On 04. 08. 25 8:45 odp., Andrew Lunn wrote:
>> Let's say we have a SyncE setup with two network controllers where each
>> of them feeds a DPLL channel with recovered clock received from some of
>> its PHY. The DPLL channel cleans/stabilizes this input signal (generates
>> phase aligned signal locked to the same frequency as the input one) and
>> routes it back to the network controller.
>>
>>      +-----------+
>>   +--|   NIC 1   |<-+
>>   |  +-----------+  |
>>   |                 |
>>   | RxCLK     TxCLK |
>>   |                 |
>>   |  +-----------+  |
>>   +->| channel 1 |--+
>>      |-- DPLL ---|
>>   +->| channel 2 |--+
>>   |  +-----------+  |
>>   |                 |
>>   | RxCLK     TxCLK |
>>   |                 |
>>   |  +-----------+  |
>>   +--|   NIC 2   |<-+
>>      +-----------+
>>
>> The PHCs implemented by the NICs have associated the ClockIdentity
>> (according IEEE 1588-2008) whose value is typically derived from
>> the NIC's MAC address using EUI-64. The DPLL channel should be
>> registered to DPLL subsystem using the same ClockIdentity as the PHC
>> it drives. In above example DPLL channel 1 should have the same clock ID
>> as NIC1 PHC and channel 2 as NIC2 PHC.
>>
>> During the discussion, Andrew had the idea to provide NIC phandles
>> instead of clock ID values.
>>
>> Something like this:
>>
>> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>> b/Documenta
>> tion/devicetree/bindings/dpll/dpll-device.yaml
>> index fb8d7a9a3693f..159d9253bc8ae 100644
>> --- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>> @@ -33,6 +33,13 @@ properties:
>>       items:
>>         enum: [pps, eec]
>>
>> +  ethernet-handles:
>> +    description:
>> +      List of phandles to Ethernet devices, one per DPLL instance. Each of
>> +      these handles identifies Ethernet device that uses particular DPLL
>> +      instance to synchronize its hardware clock.
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +
> 
> I personally would not use a list. I would have a node per channel,
> and within that node, have the ethernet-handle property. This gives
> you a more flexible scheme where you can easily add more per channel
> properties in the future.
> 
> It took us a while to understand what you actually wanted. The ASCII
> art helps. So i would include that and some text in the binding.

Like this?

diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml 
b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
index fb8d7a9a3693f..798c5484657cf 100644
--- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
+++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
@@ -27,11 +27,41 @@ properties:
    "#size-cells":
      const: 0

-  dpll-types:
-    description: List of DPLL channel types, one per DPLL instance.
-    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
-    items:
-      enum: [pps, eec]
+  channels:
+    type: object
+    description: DPLL channels
+    unevaluatedProperties: false
+
+    properties:
+      "#address-cells":
+        const: 1
+      "#size-cells":
+        const: 0
+
+    patternProperties:
+      "^channel@[0-9a-f]+$":
+        type: object
+        description: DPLL channel
+        unevaluatedProperties: false
+
+        properties:
+          reg:
+            description: Hardware index of the DPLL channel
+            maxItems: 1
+
+          dpll-type:
+            description: DPLL channel type
+            $ref: /schemas/types.yaml#/definitions/string
+            enum: [pps, eec]
+
+          ethernet-handle:
+            description:
+              Specifies a reference to a node representing an Ethernet 
device
+              that uses this channel to synchronize its hardware clock.
+            $ref: /schemas/types.yaml#/definitions/phandle
+
+        required:
+          - reg

Thanks,
Ivan

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

[Andrew Lunn]

> Like this?
> 
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> index fb8d7a9a3693f..798c5484657cf 100644
> --- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> @@ -27,11 +27,41 @@ properties:
>    "#size-cells":
>      const: 0
> 
> -  dpll-types:
> -    description: List of DPLL channel types, one per DPLL instance.
> -    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> -    items:
> -      enum: [pps, eec]
> +  channels:
> +    type: object
> +    description: DPLL channels
> +    unevaluatedProperties: false
> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +      "#size-cells":
> +        const: 0
> +
> +    patternProperties:
> +      "^channel@[0-9a-f]+$":
> +        type: object
> +        description: DPLL channel
> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            description: Hardware index of the DPLL channel
> +            maxItems: 1
> +
> +          dpll-type:
> +            description: DPLL channel type
> +            $ref: /schemas/types.yaml#/definitions/string
> +            enum: [pps, eec]
> +
> +          ethernet-handle:
> +            description:
> +              Specifies a reference to a node representing an Ethernet
> device
> +              that uses this channel to synchronize its hardware clock.
> +            $ref: /schemas/types.yaml#/definitions/phandle
> +
> +        required:
> +          - reg

Yes, but i will leave the DT Maintainers to give it a deeper review,
but this what i meant. And it makes your dpll-type much easier to
handle.

	Andrew

[PATCH net-next 2/2] dpll: zl3073x: Initialize clock ID from device property

[Ivan Vecera]

The clock ID value can be specified by the platform via 'clock-id'
property. Use this property value to initialize clock ID and if it
is not specified generate random one as fallback.

Tested-by: Prathosh Satish <prathosh.satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/core.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 7ebcfc5ec1f09..f5245225f1d3b 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -9,6 +9,8 @@
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/netlink.h>
+#include <linux/property.h>
+#include <linux/random.h>
 #include <linux/regmap.h>
 #include <linux/sprintf.h>
 #include <linux/string_choices.h>
@@ -932,6 +934,29 @@ zl3073x_dev_phase_meas_setup(struct zl3073x_dev *zldev, int num_channels)
 	return zl3073x_write_u8(zldev, ZL_REG_DPLL_PHASE_ERR_READ_MASK, mask);
 }
 
+/**
+ * zl3073x_dev_clock_id_init - initialize clock ID
+ * @zldev: pointer to zl3073x device
+ *
+ * Initializes clock ID using device property if it is provided or
+ * generates random one.
+ */
+static void
+zl3073x_dev_clock_id_init(struct zl3073x_dev *zldev)
+{
+	u64 clock_id;
+	int rc;
+
+	/* Try to read clock ID from device property */
+	rc = device_property_read_u64(zldev->dev, "clock-id", &clock_id);
+
+	/* Generate random id if the property does not exist or value is zero */
+	if (rc || !clock_id)
+		clock_id = get_random_u64();
+
+	zldev->clock_id = clock_id;
+}
+
 /**
  * zl3073x_dev_probe - initialize zl3073x device
  * @zldev: pointer to zl3073x device
@@ -985,11 +1010,8 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
 		FIELD_GET(GENMASK(15, 8), cfg_ver),
 		FIELD_GET(GENMASK(7, 0), cfg_ver));
 
-	/* Generate random clock ID as the device has not such property that
-	 * could be used for this purpose. A user can later change this value
-	 * using devlink.
-	 */
-	zldev->clock_id = get_random_u64();
+	/* Initialize clock ID */
+	zl3073x_dev_clock_id_init(zldev);
 
 	/* Initialize mutex for operations where multiple reads, writes
 	 * and/or polls are required to be done atomically.
-- 
2.49.1