devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
@ 2025-08-15 14:47 Ivan Vecera
  2025-08-15 23:21 ` Andrew Lunn
  2025-08-20 21:13 ` Rob Herring
  0 siblings, 2 replies; 12+ messages in thread
From: Ivan Vecera @ 2025-08-15 14:47 UTC (permalink / raw)
  To: netdev
  Cc: mschmidt, poros, Andrew Lunn, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

In case of SyncE scenario a DPLL channels generates a clean frequency
synchronous Ethernet clock (SyncE) and feeds it into the NIC transmit
path. The DPLL channel can be locked either to the recovered clock
from the NIC's PHY (Loop timing scenario) or to some external signal
source (e.g. GNSS) (Externally timed scenario).

The example shows both situations. NIC1 recovers the input SyncE signal
that is used as an input reference for DPLL channel 1. The channel locks
to this signal, filters jitter/wander and provides holdover. On output
the channel feeds a stable, phase-aligned clock back into the NIC1.
In the 2nd case the DPLL channel 2 locks to a master clock from GNSS and
feeds a clean SyncE signal into the NIC2.

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

In the situations above the DPLL channels should be registered into
the DPLL sub-system with the same Clock Identity as PHCs present
in the NICs (for the example above DPLL channel 1 uses the same
Clock ID as NIC1's PHC and the channel 2 as NIC2's PHC).

Because a NIC PHC's Clock ID is derived from the NIC's MAC address,
add a per-channel property 'ethernet-handle' that specifies a reference
to a node representing an Ethernet device that uses this channel
to synchronize its hardware clock. Additionally convert existing
'dpll-types' list property to 'dpll-type' per-channel property.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 .../devicetree/bindings/dpll/dpll-device.yaml | 40 ++++++++++++++++---
 .../bindings/dpll/microchip,zl30731.yaml      | 29 +++++++++++++-
 2 files changed, 62 insertions(+), 7 deletions(-)

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
 
   input-pins:
     type: object
diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
index 17747f754b845..bc6d6cc1dd47f 100644
--- a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
+++ b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
@@ -44,9 +44,26 @@ examples:
       #size-cells = <0>;
 
       dpll@70 {
+        #address-cells = <0>;
+        #size-cells = <0>;
         compatible = "microchip,zl30732";
         reg = <0x70>;
-        dpll-types = "pps", "eec";
+
+        channels {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          channel@0 {
+            reg = <0>;
+            dpll-type = "pps";
+          };
+
+          channel@1 {
+            reg = <1>;
+            dpll-type = "eec";
+            ethernet-handle = <&ethernet0>;
+          };
+        };
 
         input-pins {
           #address-cells = <1>;
@@ -84,7 +101,15 @@ examples:
         reg = <0x70>;
         spi-max-frequency = <12500000>;
 
-        dpll-types = "pps";
+        channels {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          channel@0 {
+            reg = <0>;
+            dpll-type = "pps";
+          };
+        };
 
         input-pins {
           #address-cells = <1>;
-- 
2.49.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-08-15 14:47 [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property Ivan Vecera
@ 2025-08-15 23:21 ` Andrew Lunn
  2025-08-20 21:13 ` Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2025-08-15 23:21 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: netdev, mschmidt, poros, Vadim Fedorenko, Arkadiusz Kubalewski,
	Jiri Pirko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On Fri, Aug 15, 2025 at 04:47:35PM +0200, Ivan Vecera wrote:
> In case of SyncE scenario a DPLL channels generates a clean frequency
> synchronous Ethernet clock (SyncE) and feeds it into the NIC transmit
> path. The DPLL channel can be locked either to the recovered clock
> from the NIC's PHY (Loop timing scenario) or to some external signal
> source (e.g. GNSS) (Externally timed scenario).
> 
> The example shows both situations. NIC1 recovers the input SyncE signal
> that is used as an input reference for DPLL channel 1. The channel locks
> to this signal, filters jitter/wander and provides holdover. On output
> the channel feeds a stable, phase-aligned clock back into the NIC1.
> In the 2nd case the DPLL channel 2 locks to a master clock from GNSS and
> feeds a clean SyncE signal into the NIC2.
> 
> 		   +-----------+
> 		+--|   NIC 1   |<-+
> 		|  +-----------+  |
> 		|                 |
> 		| RxCLK     TxCLK |
> 		|                 |
> 		|  +-----------+  |
> 		+->| channel 1 |--+
> +------+	   |-- DPLL ---|
> | GNSS |---------->| channel 2 |--+
> +------+  RefCLK   +-----------+  |
> 				  |
> 			    TxCLK |
> 				  |
> 		   +-----------+  |
> 		   |   NIC 2   |<-+
> 		   +-----------+
> 
> In the situations above the DPLL channels should be registered into
> the DPLL sub-system with the same Clock Identity as PHCs present
> in the NICs (for the example above DPLL channel 1 uses the same
> Clock ID as NIC1's PHC and the channel 2 as NIC2's PHC).
> 
> Because a NIC PHC's Clock ID is derived from the NIC's MAC address,
> add a per-channel property 'ethernet-handle' that specifies a reference
> to a node representing an Ethernet device that uses this channel
> to synchronize its hardware clock. Additionally convert existing
> 'dpll-types' list property to 'dpll-type' per-channel property.

It would be normal to include an implementation of the binding as
patch 2/2. Looking at the implementation sometimes makes
errors/omission in the binding obvious.

> +        channels {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          channel@0 {
> +            reg = <0>;
> +            dpll-type = "pps";
> +          };
> +
> +          channel@1 {
> +            reg = <1>;
> +            dpll-type = "eec";
> +            ethernet-handle = <&ethernet0>;
> +          };

If i'm reading this correctly, eec requires a ethernet-handle. pps
does not. You should describe that in the binding using constraints.

Otherwise, this looks reasonable to me.

	   Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-08-15 14:47 [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property Ivan Vecera
  2025-08-15 23:21 ` Andrew Lunn
@ 2025-08-20 21:13 ` Rob Herring
  2025-08-29 13:29   ` Ivan Vecera
  1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2025-08-20 21:13 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: netdev, mschmidt, poros, Andrew Lunn, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On Fri, Aug 15, 2025 at 04:47:35PM +0200, Ivan Vecera wrote:
> In case of SyncE scenario a DPLL channels generates a clean frequency
> synchronous Ethernet clock (SyncE) and feeds it into the NIC transmit
> path. The DPLL channel can be locked either to the recovered clock
> from the NIC's PHY (Loop timing scenario) or to some external signal
> source (e.g. GNSS) (Externally timed scenario).
> 
> The example shows both situations. NIC1 recovers the input SyncE signal
> that is used as an input reference for DPLL channel 1. The channel locks
> to this signal, filters jitter/wander and provides holdover. On output
> the channel feeds a stable, phase-aligned clock back into the NIC1.
> In the 2nd case the DPLL channel 2 locks to a master clock from GNSS and
> feeds a clean SyncE signal into the NIC2.
> 
> 		   +-----------+
> 		+--|   NIC 1   |<-+
> 		|  +-----------+  |
> 		|                 |
> 		| RxCLK     TxCLK |
> 		|                 |
> 		|  +-----------+  |
> 		+->| channel 1 |--+
> +------+	   |-- DPLL ---|
> | GNSS |---------->| channel 2 |--+
> +------+  RefCLK   +-----------+  |
> 				  |
> 			    TxCLK |
> 				  |
> 		   +-----------+  |
> 		   |   NIC 2   |<-+
> 		   +-----------+
> 
> In the situations above the DPLL channels should be registered into
> the DPLL sub-system with the same Clock Identity as PHCs present
> in the NICs (for the example above DPLL channel 1 uses the same
> Clock ID as NIC1's PHC and the channel 2 as NIC2's PHC).
> 
> Because a NIC PHC's Clock ID is derived from the NIC's MAC address,
> add a per-channel property 'ethernet-handle' that specifies a reference
> to a node representing an Ethernet device that uses this channel
> to synchronize its hardware clock. Additionally convert existing
> 'dpll-types' list property to 'dpll-type' per-channel property.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>  .../devicetree/bindings/dpll/dpll-device.yaml | 40 ++++++++++++++++---
>  .../bindings/dpll/microchip,zl30731.yaml      | 29 +++++++++++++-
>  2 files changed, 62 insertions(+), 7 deletions(-)
> 
> 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]

Dropping this is an ABI change. You can't do that unless you are 
confident there are no users both in existing DTs and OSs.

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

Seems a bit odd to me that the ethernet controller doesn't have a link 
to this node instead. 

Rob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-08-20 21:13 ` Rob Herring
@ 2025-08-29 13:29   ` Ivan Vecera
  2025-09-04 14:43     ` Ivan Vecera
  2025-09-04 22:06     ` Rob Herring
  0 siblings, 2 replies; 12+ messages in thread
From: Ivan Vecera @ 2025-08-29 13:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: netdev, mschmidt, poros, Andrew Lunn, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

Hi Rob,

On 20. 08. 25 11:13 odp., Rob Herring wrote:
> On Fri, Aug 15, 2025 at 04:47:35PM +0200, Ivan Vecera wrote:
>> In case of SyncE scenario a DPLL channels generates a clean frequency
>> synchronous Ethernet clock (SyncE) and feeds it into the NIC transmit
>> path. The DPLL channel can be locked either to the recovered clock
>> from the NIC's PHY (Loop timing scenario) or to some external signal
>> source (e.g. GNSS) (Externally timed scenario).
>>
>> The example shows both situations. NIC1 recovers the input SyncE signal
>> that is used as an input reference for DPLL channel 1. The channel locks
>> to this signal, filters jitter/wander and provides holdover. On output
>> the channel feeds a stable, phase-aligned clock back into the NIC1.
>> In the 2nd case the DPLL channel 2 locks to a master clock from GNSS and
>> feeds a clean SyncE signal into the NIC2.
>>
>> 		   +-----------+
>> 		+--|   NIC 1   |<-+
>> 		|  +-----------+  |
>> 		|                 |
>> 		| RxCLK     TxCLK |
>> 		|                 |
>> 		|  +-----------+  |
>> 		+->| channel 1 |--+
>> +------+	   |-- DPLL ---|
>> | GNSS |---------->| channel 2 |--+
>> +------+  RefCLK   +-----------+  |
>> 				  |
>> 			    TxCLK |
>> 				  |
>> 		   +-----------+  |
>> 		   |   NIC 2   |<-+
>> 		   +-----------+
>>
>> In the situations above the DPLL channels should be registered into
>> the DPLL sub-system with the same Clock Identity as PHCs present
>> in the NICs (for the example above DPLL channel 1 uses the same
>> Clock ID as NIC1's PHC and the channel 2 as NIC2's PHC).
>>
>> Because a NIC PHC's Clock ID is derived from the NIC's MAC address,
>> add a per-channel property 'ethernet-handle' that specifies a reference
>> to a node representing an Ethernet device that uses this channel
>> to synchronize its hardware clock. Additionally convert existing
>> 'dpll-types' list property to 'dpll-type' per-channel property.
>>
>> Suggested-by: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>>   .../devicetree/bindings/dpll/dpll-device.yaml | 40 ++++++++++++++++---
>>   .../bindings/dpll/microchip,zl30731.yaml      | 29 +++++++++++++-
>>   2 files changed, 62 insertions(+), 7 deletions(-)
>>
>> 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]
> 
> Dropping this is an ABI change. You can't do that unless you are
> confident there are no users both in existing DTs and OSs.

Get it, will keep.

>> +  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
> 
> Seems a bit odd to me that the ethernet controller doesn't have a link
> to this node instead.

Do you mean to add a property (e.g. dpll-channel or dpll-device) into
net/network-class.yaml ? If so, yes, it would be possible, and the way
I look at it now, it would probably be better. The DPLL driver can
enumerate all devices across the system that has this specific property
and check its value.

See the proposal below...

Thanks,
Ivan

---
  Documentation/devicetree/bindings/dpll/dpll-device.yaml  | 6 ++++++
  Documentation/devicetree/bindings/net/network-class.yaml | 7 +++++++
  2 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml 
b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
index fb8d7a9a3693f..560351df1bec3 100644
--- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
+++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
@@ -27,6 +27,12 @@ properties:
    "#size-cells":
      const: 0

+  "#dpll-cells":
+    description: |
+      Number of cells in a dpll specifier. The cell specifies the index
+      of the channel within the DPLL device.
+    const: 1
+
    dpll-types:
      description: List of DPLL channel types, one per DPLL instance.
      $ref: /schemas/types.yaml#/definitions/non-unique-string-array
diff --git a/Documentation/devicetree/bindings/net/network-class.yaml 
b/Documentation/devicetree/bindings/net/network-class.yaml
index 06461fb92eb84..144badb3b7ff1 100644
--- a/Documentation/devicetree/bindings/net/network-class.yaml
+++ b/Documentation/devicetree/bindings/net/network-class.yaml
@@ -17,6 +17,13 @@ properties:
      default: 48
      const: 48

+  dpll:
+    description:
+      Specifies DPLL device phandle and index of the DPLL channel within
+      this device used by this network device to synchronize its hardware
+      clock.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
    local-mac-address:
      description:
        Specifies MAC address that was assigned to the network device 
described by


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-08-29 13:29   ` Ivan Vecera
@ 2025-09-04 14:43     ` Ivan Vecera
  2025-09-04 22:06     ` Rob Herring
  1 sibling, 0 replies; 12+ messages in thread
From: Ivan Vecera @ 2025-09-04 14:43 UTC (permalink / raw)
  To: Rob Herring
  Cc: netdev, mschmidt, poros, Andrew Lunn, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

Hi Rob,

any comment to my second proposal (below)?

Thank you,
Ivan

On 29. 08. 25 3:29 odp., Ivan Vecera wrote:
>> Seems a bit odd to me that the ethernet controller doesn't have a link
>> to this node instead.
> 
> Do you mean to add a property (e.g. dpll-channel or dpll-device) into
> net/network-class.yaml ? If so, yes, it would be possible, and the way
> I look at it now, it would probably be better. The DPLL driver can
> enumerate all devices across the system that has this specific property
> and check its value.
> 
> See the proposal below...
> 
> Thanks,
> Ivan
> 
> ---
>   Documentation/devicetree/bindings/dpll/dpll-device.yaml  | 6 ++++++
>   Documentation/devicetree/bindings/net/network-class.yaml | 7 +++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml b/ 
> Documentation/devicetree/bindings/dpll/dpll-device.yaml
> index fb8d7a9a3693f..560351df1bec3 100644
> --- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> @@ -27,6 +27,12 @@ properties:
>     "#size-cells":
>       const: 0
> 
> +  "#dpll-cells":
> +    description: |
> +      Number of cells in a dpll specifier. The cell specifies the index
> +      of the channel within the DPLL device.
> +    const: 1
> +
>     dpll-types:
>       description: List of DPLL channel types, one per DPLL instance.
>       $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> diff --git a/Documentation/devicetree/bindings/net/network-class.yaml b/ 
> Documentation/devicetree/bindings/net/network-class.yaml
> index 06461fb92eb84..144badb3b7ff1 100644
> --- a/Documentation/devicetree/bindings/net/network-class.yaml
> +++ b/Documentation/devicetree/bindings/net/network-class.yaml
> @@ -17,6 +17,13 @@ properties:
>       default: 48
>       const: 48
> 
> +  dpll:
> +    description:
> +      Specifies DPLL device phandle and index of the DPLL channel within
> +      this device used by this network device to synchronize its hardware
> +      clock.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
>     local-mac-address:
>       description:
>         Specifies MAC address that was assigned to the network device 
> described by


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-08-29 13:29   ` Ivan Vecera
  2025-09-04 14:43     ` Ivan Vecera
@ 2025-09-04 22:06     ` Rob Herring
  2025-09-05  6:50       ` Ivan Vecera
  1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2025-09-04 22:06 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: netdev, mschmidt, poros, Andrew Lunn, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On Fri, Aug 29, 2025 at 8:29 AM Ivan Vecera <ivecera@redhat.com> wrote:
>
> Hi Rob,
>
> On 20. 08. 25 11:13 odp., Rob Herring wrote:
> > On Fri, Aug 15, 2025 at 04:47:35PM +0200, Ivan Vecera wrote:
> >> In case of SyncE scenario a DPLL channels generates a clean frequency
> >> synchronous Ethernet clock (SyncE) and feeds it into the NIC transmit
> >> path. The DPLL channel can be locked either to the recovered clock
> >> from the NIC's PHY (Loop timing scenario) or to some external signal
> >> source (e.g. GNSS) (Externally timed scenario).
> >>
> >> The example shows both situations. NIC1 recovers the input SyncE signal
> >> that is used as an input reference for DPLL channel 1. The channel locks
> >> to this signal, filters jitter/wander and provides holdover. On output
> >> the channel feeds a stable, phase-aligned clock back into the NIC1.
> >> In the 2nd case the DPLL channel 2 locks to a master clock from GNSS and
> >> feeds a clean SyncE signal into the NIC2.
> >>
> >>                 +-----------+
> >>              +--|   NIC 1   |<-+
> >>              |  +-----------+  |
> >>              |                 |
> >>              | RxCLK     TxCLK |
> >>              |                 |
> >>              |  +-----------+  |
> >>              +->| channel 1 |--+
> >> +------+        |-- DPLL ---|
> >> | GNSS |---------->| channel 2 |--+
> >> +------+  RefCLK   +-----------+  |
> >>                                |
> >>                          TxCLK |
> >>                                |
> >>                 +-----------+  |
> >>                 |   NIC 2   |<-+
> >>                 +-----------+
> >>
> >> In the situations above the DPLL channels should be registered into
> >> the DPLL sub-system with the same Clock Identity as PHCs present
> >> in the NICs (for the example above DPLL channel 1 uses the same
> >> Clock ID as NIC1's PHC and the channel 2 as NIC2's PHC).
> >>
> >> Because a NIC PHC's Clock ID is derived from the NIC's MAC address,
> >> add a per-channel property 'ethernet-handle' that specifies a reference
> >> to a node representing an Ethernet device that uses this channel
> >> to synchronize its hardware clock. Additionally convert existing
> >> 'dpll-types' list property to 'dpll-type' per-channel property.
> >>
> >> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> >> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> >> ---
> >>   .../devicetree/bindings/dpll/dpll-device.yaml | 40 ++++++++++++++++---
> >>   .../bindings/dpll/microchip,zl30731.yaml      | 29 +++++++++++++-
> >>   2 files changed, 62 insertions(+), 7 deletions(-)
> >>
> >> 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]
> >
> > Dropping this is an ABI change. You can't do that unless you are
> > confident there are no users both in existing DTs and OSs.
>
> Get it, will keep.
>
> >> +  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
> >
> > Seems a bit odd to me that the ethernet controller doesn't have a link
> > to this node instead.
>
> Do you mean to add a property (e.g. dpll-channel or dpll-device) into
> net/network-class.yaml ? If so, yes, it would be possible, and the way
> I look at it now, it would probably be better. The DPLL driver can
> enumerate all devices across the system that has this specific property
> and check its value.

Yes. Or into ethernet-controller.yaml. Is a DPLL used with wifi,
bluetooth, etc.?

>
> See the proposal below...
>
> Thanks,
> Ivan
>
> ---
>   Documentation/devicetree/bindings/dpll/dpll-device.yaml  | 6 ++++++
>   Documentation/devicetree/bindings/net/network-class.yaml | 7 +++++++
>   2 files changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> index fb8d7a9a3693f..560351df1bec3 100644
> --- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> @@ -27,6 +27,12 @@ properties:
>     "#size-cells":
>       const: 0
>
> +  "#dpll-cells":
> +    description: |
> +      Number of cells in a dpll specifier. The cell specifies the index
> +      of the channel within the DPLL device.
> +    const: 1

If it is 1 for everyone, then you don't need a property for it. The
question is whether it would need to vary. Perhaps some configuration
flags/info might be needed? Connection type or frequency looking at
the existing configuration setting?

> +
>     dpll-types:
>       description: List of DPLL channel types, one per DPLL instance.
>       $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> diff --git a/Documentation/devicetree/bindings/net/network-class.yaml
> b/Documentation/devicetree/bindings/net/network-class.yaml
> index 06461fb92eb84..144badb3b7ff1 100644
> --- a/Documentation/devicetree/bindings/net/network-class.yaml
> +++ b/Documentation/devicetree/bindings/net/network-class.yaml
> @@ -17,6 +17,13 @@ properties:
>       default: 48
>       const: 48
>
> +  dpll:
> +    description:
> +      Specifies DPLL device phandle and index of the DPLL channel within
> +      this device used by this network device to synchronize its hardware
> +      clock.
> +    $ref: /schemas/types.yaml#/definitions/phandle

If you have cells, then this should be phandle-array.

> +
>     local-mac-address:
>       description:
>         Specifies MAC address that was assigned to the network device
> described by
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-09-04 22:06     ` Rob Herring
@ 2025-09-05  6:50       ` Ivan Vecera
  2025-09-08 22:49         ` Rob Herring
  0 siblings, 1 reply; 12+ messages in thread
From: Ivan Vecera @ 2025-09-05  6:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: netdev, mschmidt, poros, Andrew Lunn, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list



On 05. 09. 25 12:06 dop., Rob Herring wrote:
> On Fri, Aug 29, 2025 at 8:29 AM Ivan Vecera <ivecera@redhat.com> wrote:
>> ...
>>
>> Do you mean to add a property (e.g. dpll-channel or dpll-device) into
>> net/network-class.yaml ? If so, yes, it would be possible, and the way
>> I look at it now, it would probably be better. The DPLL driver can
>> enumerate all devices across the system that has this specific property
>> and check its value.
> 
> Yes. Or into ethernet-controller.yaml. Is a DPLL used with wifi,
> bluetooth, etc.?

AFAIK no... ethernet-controller makes sense.

>>
>> See the proposal below...
>>
>> Thanks,
>> Ivan
>>
>> ---
>>    Documentation/devicetree/bindings/dpll/dpll-device.yaml  | 6 ++++++
>>    Documentation/devicetree/bindings/net/network-class.yaml | 7 +++++++
>>    2 files changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>> b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>> index fb8d7a9a3693f..560351df1bec3 100644
>> --- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>> @@ -27,6 +27,12 @@ properties:
>>      "#size-cells":
>>        const: 0
>>
>> +  "#dpll-cells":
>> +    description: |
>> +      Number of cells in a dpll specifier. The cell specifies the index
>> +      of the channel within the DPLL device.
>> +    const: 1
> 
> If it is 1 for everyone, then you don't need a property for it. The
> question is whether it would need to vary. Perhaps some configuration
> flags/info might be needed? Connection type or frequency looking at
> the existing configuration setting?

Connection type maybe... What I am trying to do is define a relationship
between the network controller and the DPLL device, which together form
a single entity from a use-case perspective (e.g., Ethernet uses an
external DPLL device either to synchronize the recovered clock or to
provide a SyncE signal synchronized with an external 1PPS source).

Yesterday I was considering the implementation from the DPLL driver's
perspective and encountered a problem when the relation is defined from
the Ethernet controller's perspective. In that case, it would be
necessary to enumerate all devices that contain a “dpll” property whose
value references this DPLL device.

This approach seems quite complicated, as it would require searching
through all buses, all connected devices, and checking each fwnode for a
“dpll” property containing the given reference. I don’t think this would
be the right solution.

I then came across graph bindings and ACPI graph extensions, which are
widely used in the media and DRM subsystems to define relations between
devices. Would this be an appropriate way to define a binding between an
Ethernet controller and a DPLL device?

If so, what would such a binding roughly look like? I’m not very
experienced in this area, so I would appreciate any guidance.

If not, wouldn’t it be better to define the relation from the DPLL
device to the network controller, as originally proposed?

Thanks,
Ivan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-09-05  6:50       ` Ivan Vecera
@ 2025-09-08 22:49         ` Rob Herring
  2025-09-09 12:50           ` Ivan Vecera
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2025-09-08 22:49 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: netdev, mschmidt, poros, Andrew Lunn, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On Fri, Sep 5, 2025 at 1:50 AM Ivan Vecera <ivecera@redhat.com> wrote:
>
>
>
> On 05. 09. 25 12:06 dop., Rob Herring wrote:
> > On Fri, Aug 29, 2025 at 8:29 AM Ivan Vecera <ivecera@redhat.com> wrote:
> >> ...
> >>
> >> Do you mean to add a property (e.g. dpll-channel or dpll-device) into
> >> net/network-class.yaml ? If so, yes, it would be possible, and the way
> >> I look at it now, it would probably be better. The DPLL driver can
> >> enumerate all devices across the system that has this specific property
> >> and check its value.
> >
> > Yes. Or into ethernet-controller.yaml. Is a DPLL used with wifi,
> > bluetooth, etc.?
>
> AFAIK no... ethernet-controller makes sense.
>
> >>
> >> See the proposal below...
> >>
> >> Thanks,
> >> Ivan
> >>
> >> ---
> >>    Documentation/devicetree/bindings/dpll/dpll-device.yaml  | 6 ++++++
> >>    Documentation/devicetree/bindings/net/network-class.yaml | 7 +++++++
> >>    2 files changed, 13 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> >> b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> >> index fb8d7a9a3693f..560351df1bec3 100644
> >> --- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> >> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> >> @@ -27,6 +27,12 @@ properties:
> >>      "#size-cells":
> >>        const: 0
> >>
> >> +  "#dpll-cells":
> >> +    description: |
> >> +      Number of cells in a dpll specifier. The cell specifies the index
> >> +      of the channel within the DPLL device.
> >> +    const: 1
> >
> > If it is 1 for everyone, then you don't need a property for it. The
> > question is whether it would need to vary. Perhaps some configuration
> > flags/info might be needed? Connection type or frequency looking at
> > the existing configuration setting?
>
> Connection type maybe... What I am trying to do is define a relationship
> between the network controller and the DPLL device, which together form
> a single entity from a use-case perspective (e.g., Ethernet uses an
> external DPLL device either to synchronize the recovered clock or to
> provide a SyncE signal synchronized with an external 1PPS source).
>
> Yesterday I was considering the implementation from the DPLL driver's
> perspective and encountered a problem when the relation is defined from
> the Ethernet controller's perspective. In that case, it would be
> necessary to enumerate all devices that contain a “dpll” property whose
> value references this DPLL device.

Why is that?

>
> This approach seems quite complicated, as it would require searching
> through all buses, all connected devices, and checking each fwnode for a
> “dpll” property containing the given reference. I don’t think this would
> be the right solution.

for_each_node_with_property() provides that. No, it's not efficient,
but I doubt it needs to be. As you'd only need to do it once.

> I then came across graph bindings and ACPI graph extensions, which are
> widely used in the media and DRM subsystems to define relations between
> devices. Would this be an appropriate way to define a binding between an
> Ethernet controller and a DPLL device?

Usually the graph is used to handle complex chains of devices and how
the data flows. I'm not sure that applies here.

> If so, what would such a binding roughly look like? I’m not very
> experienced in this area, so I would appreciate any guidance.
>
> If not, wouldn’t it be better to define the relation from the DPLL
> device to the network controller, as originally proposed?

I have no idea really. I would think the DPLL is the provider and an
ethernet device is the consumer. And if the ethernet device is unused
(or disabled), then the DPLL connection associated with it is unused.
If that's the case, then I think the property belongs in the ethernet
node.

Rob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-09-08 22:49         ` Rob Herring
@ 2025-09-09 12:50           ` Ivan Vecera
  2025-09-09 13:50             ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Ivan Vecera @ 2025-09-09 12:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: netdev, mschmidt, poros, Andrew Lunn, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 09. 09. 25 12:49 dop., Rob Herring wrote:
> On Fri, Sep 5, 2025 at 1:50 AM Ivan Vecera <ivecera@redhat.com> wrote:
>>
>>
>>
>> On 05. 09. 25 12:06 dop., Rob Herring wrote:
>>> On Fri, Aug 29, 2025 at 8:29 AM Ivan Vecera <ivecera@redhat.com> wrote:
>>>> ...
>>>>
>>>> Do you mean to add a property (e.g. dpll-channel or dpll-device) into
>>>> net/network-class.yaml ? If so, yes, it would be possible, and the way
>>>> I look at it now, it would probably be better. The DPLL driver can
>>>> enumerate all devices across the system that has this specific property
>>>> and check its value.
>>>
>>> Yes. Or into ethernet-controller.yaml. Is a DPLL used with wifi,
>>> bluetooth, etc.?
>>
>> AFAIK no... ethernet-controller makes sense.
>>
>>>>
>>>> See the proposal below...
>>>>
>>>> Thanks,
>>>> Ivan
>>>>
>>>> ---
>>>>     Documentation/devicetree/bindings/dpll/dpll-device.yaml  | 6 ++++++
>>>>     Documentation/devicetree/bindings/net/network-class.yaml | 7 +++++++
>>>>     2 files changed, 13 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>>>> b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>>>> index fb8d7a9a3693f..560351df1bec3 100644
>>>> --- a/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>>>> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
>>>> @@ -27,6 +27,12 @@ properties:
>>>>       "#size-cells":
>>>>         const: 0
>>>>
>>>> +  "#dpll-cells":
>>>> +    description: |
>>>> +      Number of cells in a dpll specifier. The cell specifies the index
>>>> +      of the channel within the DPLL device.
>>>> +    const: 1
>>>
>>> If it is 1 for everyone, then you don't need a property for it. The
>>> question is whether it would need to vary. Perhaps some configuration
>>> flags/info might be needed? Connection type or frequency looking at
>>> the existing configuration setting?
>>
>> Connection type maybe... What I am trying to do is define a relationship
>> between the network controller and the DPLL device, which together form
>> a single entity from a use-case perspective (e.g., Ethernet uses an
>> external DPLL device either to synchronize the recovered clock or to
>> provide a SyncE signal synchronized with an external 1PPS source).
>>
>> Yesterday I was considering the implementation from the DPLL driver's
>> perspective and encountered a problem when the relation is defined from
>> the Ethernet controller's perspective. In that case, it would be
>> necessary to enumerate all devices that contain a “dpll” property whose
>> value references this DPLL device.
> 
> Why is that?

Because the DPLL driver has to find a mac-address of the ethernet
controller to generate clock identity that is used for DPLL device
registration.

>>
>> This approach seems quite complicated, as it would require searching
>> through all buses, all connected devices, and checking each fwnode for a
>> “dpll” property containing the given reference. I don’t think this would
>> be the right solution.
> 
> for_each_node_with_property() provides that. No, it's not efficient,
> but I doubt it needs to be. As you'd only need to do it once.

Yes, for_each_node_with_property() could be used but only for OF case. I
would like to use firmware type agnostic interface to cover also ACPI
systems where the zl3073x driver is/will be used.

I'm not aware of similar functionality for fwnode... is it an option
to write FW type agnostic macro for_each_fwnode_with_property() that
would cover OF, ACPI, software_node...?

>> I then came across graph bindings and ACPI graph extensions, which are
>> widely used in the media and DRM subsystems to define relations between
>> devices. Would this be an appropriate way to define a binding between an
>> Ethernet controller and a DPLL device?
> 
> Usually the graph is used to handle complex chains of devices and how
> the data flows. I'm not sure that applies here.

Agree.

>> If so, what would such a binding roughly look like? I’m not very
>> experienced in this area, so I would appreciate any guidance.
>>
>> If not, wouldn’t it be better to define the relation from the DPLL
>> device to the network controller, as originally proposed?
> 
> I have no idea really. I would think the DPLL is the provider and an
> ethernet device is the consumer. And if the ethernet device is unused
> (or disabled), then the DPLL connection associated with it is unused.
> If that's the case, then I think the property belongs in the ethernet
> node.

 From this point of view, this is true. DPLL is signal provider and
ethernet controller its consumer. Or in other words the PHC in the NIC
is driven by this DPLL OR the DPLL drives the PHC in this NIC.
It depends on point of view.

Thanks,
Ivan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-09-09 12:50           ` Ivan Vecera
@ 2025-09-09 13:50             ` Andrew Lunn
  2025-09-10 12:51               ` Ivan Vecera
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2025-09-09 13:50 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: Rob Herring, netdev, mschmidt, poros, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

> > > Yesterday I was considering the implementation from the DPLL driver's
> > > perspective and encountered a problem when the relation is defined from
> > > the Ethernet controller's perspective. In that case, it would be
> > > necessary to enumerate all devices that contain a “dpll” property whose
> > > value references this DPLL device.
> > 
> > Why is that?
> 
> Because the DPLL driver has to find a mac-address of the ethernet
> controller to generate clock identity that is used for DPLL device
> registration.

Maybe this API is the wrong way around? Maybe what you want is that
the MAC driver says to the DPLL driver: hey, you are my clock
provider, here is an ID to use, please start providing me a clock?

So it is the MAC driver which will follow the phandle, and then make a
call to bind the dpll to the MAC, and then provide it with the ID?

	Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-09-09 13:50             ` Andrew Lunn
@ 2025-09-10 12:51               ` Ivan Vecera
  2025-09-16  9:31                 ` Jiri Pirko
  0 siblings, 1 reply; 12+ messages in thread
From: Ivan Vecera @ 2025-09-10 12:51 UTC (permalink / raw)
  To: Andrew Lunn, Jiri Pirko
  Cc: Rob Herring, netdev, mschmidt, poros, Vadim Fedorenko,
	Arkadiusz Kubalewski, Jiri Pirko, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On 09. 09. 25 3:50 odp., Andrew Lunn wrote:
>>>> Yesterday I was considering the implementation from the DPLL driver's
>>>> perspective and encountered a problem when the relation is defined from
>>>> the Ethernet controller's perspective. In that case, it would be
>>>> necessary to enumerate all devices that contain a “dpll” property whose
>>>> value references this DPLL device.
>>>
>>> Why is that?
>>
>> Because the DPLL driver has to find a mac-address of the ethernet
>> controller to generate clock identity that is used for DPLL device
>> registration.
> 
> Maybe this API is the wrong way around? Maybe what you want is that
> the MAC driver says to the DPLL driver: hey, you are my clock
> provider, here is an ID to use, please start providing me a clock?

Yes, this could be fine but there is a problem because clock id is part
of DPLL device and pins registration and it is not possible to change
the clock id without full de-re-registration. I have provided in zl3073x
a user to change the clock id via devlink but it means that the driver
has to unregister all dpll devices and pins and register them under
different clock id.

> So it is the MAC driver which will follow the phandle, and then make a
> call to bind the dpll to the MAC, and then provide it with the ID?

In fact that would be enough to expose from the DPLL core a function
to change clock id of the existing DPLL devices.

E.g.

int dpll_clock_id_change(struct module *module, u64 clock_id,
			 u64 new_clock_id)
{
	struct dpll_device *dpll_pos;
	struct dpll_pin *pin_pos;
	unsigned long i;

	mutex_lock(&dpll_lock);
	/* Change clock_id of all devices registered by given module
	 * with given clock_id.
	 */
	xa_for_each(&dpll_device_xa, i, dpll_pos) {
		if (dpll->clock_id == clock_id &&
		    dpll->module == module)
			dpll_pos->clock_id = new_clock_id;
		}
	}
	/* Change clock_id of all pins registered by given module
	 * with given clock_id.
	 */
	xa_for_each(&dpll_pin_xa, i, pos) {
		if (pin_pos->clock_id == clock_id &&
		    pin_pos->module == module) {
			pos->clock_id = new_clock_id;
		}
	}
	mutex_unlock(&dpll_lock);
}

With this, the standalone DPLL driver can register devices and pins with
arbitrary clock_id and then the MAC driver can change it.

Thoughts?

Thanks,
Ivan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property
  2025-09-10 12:51               ` Ivan Vecera
@ 2025-09-16  9:31                 ` Jiri Pirko
  0 siblings, 0 replies; 12+ messages in thread
From: Jiri Pirko @ 2025-09-16  9:31 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: Andrew Lunn, Rob Herring, netdev, mschmidt, poros,
	Vadim Fedorenko, Arkadiusz Kubalewski, Krzysztof Kozlowski,
	Conor Dooley, Prathosh Satish,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

Wed, Sep 10, 2025 at 02:51:33PM +0200, ivecera@redhat.com wrote:
>On 09. 09. 25 3:50 odp., Andrew Lunn wrote:
>> > > > Yesterday I was considering the implementation from the DPLL driver's
>> > > > perspective and encountered a problem when the relation is defined from
>> > > > the Ethernet controller's perspective. In that case, it would be
>> > > > necessary to enumerate all devices that contain a “dpll” property whose
>> > > > value references this DPLL device.
>> > > 
>> > > Why is that?
>> > 
>> > Because the DPLL driver has to find a mac-address of the ethernet
>> > controller to generate clock identity that is used for DPLL device
>> > registration.
>> 
>> Maybe this API is the wrong way around? Maybe what you want is that
>> the MAC driver says to the DPLL driver: hey, you are my clock
>> provider, here is an ID to use, please start providing me a clock?
>
>Yes, this could be fine but there is a problem because clock id is part
>of DPLL device and pins registration and it is not possible to change
>the clock id without full de-re-registration. I have provided in zl3073x
>a user to change the clock id via devlink but it means that the driver
>has to unregister all dpll devices and pins and register them under
>different clock id.
>
>> So it is the MAC driver which will follow the phandle, and then make a
>> call to bind the dpll to the MAC, and then provide it with the ID?
>
>In fact that would be enough to expose from the DPLL core a function
>to change clock id of the existing DPLL devices.
>
>E.g.
>
>int dpll_clock_id_change(struct module *module, u64 clock_id,
>			 u64 new_clock_id)
>{
>	struct dpll_device *dpll_pos;
>	struct dpll_pin *pin_pos;
>	unsigned long i;
>
>	mutex_lock(&dpll_lock);
>	/* Change clock_id of all devices registered by given module
>	 * with given clock_id.
>	 */
>	xa_for_each(&dpll_device_xa, i, dpll_pos) {
>		if (dpll->clock_id == clock_id &&
>		    dpll->module == module)
>			dpll_pos->clock_id = new_clock_id;
>		}
>	}
>	/* Change clock_id of all pins registered by given module
>	 * with given clock_id.
>	 */
>	xa_for_each(&dpll_pin_xa, i, pos) {
>		if (pin_pos->clock_id == clock_id &&
>		    pin_pos->module == module) {
>			pos->clock_id = new_clock_id;
>		}
>	}
>	mutex_unlock(&dpll_lock);
>}
>
>With this, the standalone DPLL driver can register devices and pins with
>arbitrary clock_id and then the MAC driver can change it.
>
>Thoughts?

The clock_id in dpll is basically a property. It is not used in uapi
(other then for show and obtaining the device id). So if you introduce
a mean the change this property, I don't see a problem with that.


>
>Thanks,
>Ivan
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-09-16  9:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-15 14:47 [RFC PATCH net-next] dt-bindings: dpll: Add per-channel Ethernet reference property Ivan Vecera
2025-08-15 23:21 ` Andrew Lunn
2025-08-20 21:13 ` Rob Herring
2025-08-29 13:29   ` Ivan Vecera
2025-09-04 14:43     ` Ivan Vecera
2025-09-04 22:06     ` Rob Herring
2025-09-05  6:50       ` Ivan Vecera
2025-09-08 22:49         ` Rob Herring
2025-09-09 12:50           ` Ivan Vecera
2025-09-09 13:50             ` Andrew Lunn
2025-09-10 12:51               ` Ivan Vecera
2025-09-16  9:31                 ` Jiri Pirko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).