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

[Ivan Vecera <ivecera@redhat.com>]

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