Re: [PATCH net-next 01/12] dt-bindings: dpll: add common dpll-pin-consumer schema

[Ivan Vecera <ivecera@redhat.com>]

On 1/17/26 12:39 AM, Rob Herring wrote:
> On Fri, Jan 16, 2026 at 1:00 PM Ivan Vecera <ivecera@redhat.com> wrote:
>>
>> On 1/16/26 4:23 PM, Rob Herring wrote:
>>> On Thu, Jan 15, 2026 at 6:02 AM Ivan Vecera <ivecera@redhat.com> wrote:
>>>>
>>>> On 1/8/26 7:23 PM, Ivan Vecera wrote:
>>>>> Introduce a common schema for DPLL pin consumers. Devices such as Ethernet
>>>>> controllers and PHYs may require connections to DPLL pins for Synchronous
>>>>> Ethernet (SyncE) or other frequency synchronization tasks.
>>>>>
>>>>> Defining these properties in a shared schema ensures consistency across
>>>>> different device types that consume DPLL resources.
>>>>>
>>>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>>>>> ---
>>>>>     .../bindings/dpll/dpll-pin-consumer.yaml      | 30 +++++++++++++++++++
>>>>>     MAINTAINERS                                   |  1 +
>>>>>     2 files changed, 31 insertions(+)
>>>>>     create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin-consumer.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/dpll/dpll-pin-consumer.yaml b/Documentation/devicetree/bindings/dpll/dpll-pin-consumer.yaml
>>>>> new file mode 100644
>>>>> index 0000000000000..60c184c18318a
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/dpll/dpll-pin-consumer.yaml
>>>>> @@ -0,0 +1,30 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/dpll/dpll-pin-consumer.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: DPLL Pin Consumer
>>>>> +
>>>>> +maintainers:
>>>>> +  - Ivan Vecera <ivecera@redhat.com>
>>>>> +
>>>>> +description: |
>>>>> +  Common properties for devices that require connection to DPLL (Digital Phase
>>>>> +  Locked Loop) pins for frequency synchronization (e.g. SyncE).
>>>>> +
>>>>> +properties:
>>>>> +  dpll-pins:
>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>> +    description:
>>>>> +      List of phandles to the DPLL pin nodes connected to this device.
>>>>> +
>>>>> +  dpll-pin-names:
>>>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>>>> +    description:
>>>>> +      Names for the DPLL pins defined in 'dpll-pins', in the same order.
>>>>> +
>>>>> +dependencies:
>>>>> +  dpll-pin-names: [ dpll-pins ]
>>>>> +
>>>>> +additionalProperties: true
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index 765ad2daa2183..f6f58dfb20931 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -7648,6 +7648,7 @@ M:      Jiri Pirko <jiri@resnulli.us>
>>>>>     L:  netdev@vger.kernel.org
>>>>>     S:  Supported
>>>>>     F:  Documentation/devicetree/bindings/dpll/dpll-device.yaml
>>>>> +F:   Documentation/devicetree/bindings/dpll/dpll-pin-consumer.yaml
>>>>>     F:  Documentation/devicetree/bindings/dpll/dpll-pin.yaml
>>>>>     F:  Documentation/driver-api/dpll.rst
>>>>>     F:  drivers/dpll/
>>>>
>>>> Based on private discussion with Andrew Lunn (thanks a lot), this is
>>>> wrong approach. Referencing directly dpll-pin nodes and using their
>>>> phandles in consumers is at least unusual.
>>>>
>>>> The right approach should be referencing dpll-device and use cells
>>>> to specify the dpll pin that is used.
>>>
>>> You only need a cells property if you expect the number of cells to
>>> vary by provider.
>>>
>>> However, the DPLL device just appears to be a clock provider and
>>> consumer, so why not just use the clock binding here? Also, there is
>>> no rule that using foo binding means you have to use foo subsystem in
>>> the kernel.
>>
>> Hmm, do you mean something like this example?
>>
>> &dpll0 {
>>       ...
>>       #clock-cells = <2>; /* 1st pin index, 2nd pin type (input/output) */
>>
>>       input-pins {
>>           pin@2 {
>>               reg = <2>;
>>               ...
>>           };
>>           pin@4 {
>>               reg = <4>;
>>               ...
>>           };
>>       };
>>       output-pins {
>>           pin@3 {
>>               reg = <3>;
>>           };
>>       };
>> };
>> &phy0 {
>>       ...
>>       clock-names = "rclk0", "rclk1", "synce_ref";
>>       clocks = <&dpll0 2 DPLL_INPUT>,
>>                <&dpll0 4 DPLL_INPUT>,
>>                <&dpll0 3 DPLL_OUTPUT>;
>>       ...
>> };
> 
> No, clock providers are always the clock outputs, and clock consumers
> are the clock inputs. So something like this:
> 
> &dpll0 {
>       ...
>       #clock-cells = <1>; /* 1st pin index */
> 
>       // clocks index corresponds to input pins on dpll0 */
>       clocks = <&phy0 0>, <&phy0 1>, <&phy1 0>, <&phy1 1>
> };
> &phy0 {
>       ...
>       #clock-cells = <1>;
>       clocks = <&dpll0 3>;
>       ...
> };

Fully understand now... will modify the patch-set accordingly.
Thanks for the advises.

Ivan