[v2] dt-bindings: net: marvell,pp2: convert to json-schema

This converts the marvell,pp2 bindings from text to proper schema.

Move 'marvell,system-controller' and 'dma-coherent' properties from
port up to the controller node, to match what is actually done in DT.

Signed-off-by: Michał Grzelak <mig@semihalf.com>
---
 .../devicetree/bindings/net/marvell,pp2.yaml  | 241 ++++++++++++++++++
 .../devicetree/bindings/net/marvell-pp2.txt   | 141 ----------
 MAINTAINERS                                   |   2 +-
 3 files changed, 242 insertions(+), 142 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,pp2.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/marvell-pp2.txt

On 27/09/2022 01:21, Michał Grzelak wrote:
> This converts the marvell,pp2 bindings from text to proper schema.
> 
> Move 'marvell,system-controller' and 'dma-coherent' properties from
> port up to the controller node, to match what is actually done in DT.
> 
> Signed-off-by: Michał Grzelak <mig@semihalf.com>
> ---
>  .../devicetree/bindings/net/marvell,pp2.yaml  | 241 ++++++++++++++++++
>  .../devicetree/bindings/net/marvell-pp2.txt   | 141 ----------

Thank you for your patch. There is something to discuss/improve.


> +properties:
> +  compatible:
> +    enum:
> +      - marvell,armada-375-pp2
> +      - marvell,armada-7k-pp22
> +
> +  reg:
> +    minItems: 3
> +    maxItems: 4
> +    description: |
> +      For "marvell,armada-375-pp2", must contain the following register sets:
> +        - common controller registers
> +        - LMS registers
> +        - one register area per Ethernet port
> +      For "marvell,armada-7k-pp22" used by 7K/8K and CN913X, must contain the following register sets:
> +        - packet processor registers
> +        - networking interfaces registers
> +        - CM3 address space used for TX Flow Control

Instead of this description, in define them for each variant in
allOf:if:then (just like for clocks below)

> +
> +  clocks:
> +    minItems: 2
> +    items:
> +      - description: main controller clock
> +      - description: GOP clock
> +      - description: MG clock
> +      - description: MG Core clock
> +      - description: AXI clock

This needs to be restricted per variant - minItems and maxItems in
allOf:if:then.

> +
> +  clock-names:
> +    minItems: 2
> +    items:
> +      - const: pp_clk
> +      - const: gop_clk
> +      - const: mg_clk
> +      - const: mg_core_clk
> +      - const: axi_clk

The same.

> +
> +  dma-coherent: true
> +  '#size-cells': true
> +  '#address-cells': true

You need const:X for both cells (unless they come from some other schema
but then you would not need to list them here).

> +
> +  marvell,system-controller:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: a phandle to the system controller.
> +
> +patternProperties:
> +  '^eth[0-9a-f]*(@.*)?$':

The name should be "(ethernet-)?port", unless anything depends on
particular naming?

> +    type: object

You need description here.

> +    properties:
> +      interrupts:
> +        minItems: 1
> +        maxItems: 10
> +        description: interrupt(s) for the port
> +
> +      interrupt-names:
> +        items:

minItems: 1

> +          - const: hif0
> +          - const: hif1
> +          - const: hif2
> +          - const: hif3
> +          - const: hif4
> +          - const: hif5
> +          - const: hif6
> +          - const: hif7
> +          - const: hif8
> +          - const: link
> +
> +        description: >
> +          if more than a single interrupt for is given, must be the
> +          name associated to the interrupts listed. Valid names are:
> +          "hifX", with X in [0..8], and "link". The names "tx-cpu0",
> +          "tx-cpu1", "tx-cpu2", "tx-cpu3" and "rx-shared" are supported
> +          for backward compatibility but shouldn't be used for new
> +          additions.
> +
> +      port-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: ID of the port from the MAC point of view.
> +
> +      phy:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description: >
> +          a phandle to a phy node defining the PHY address
> +          (as the reg property, a single integer).
> +
> +      phy-mode:
> +        $ref: "ethernet-controller.yaml#/properties/phy-mode"

You can skip quotes.

> +
> +      marvell,loopback:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: port is loopback mode.
> +
> +      gop-port-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: >
> +          only for marvell,armada-7k-pp22, ID of the port from the
> +          GOP (Group Of Ports) point of view. This ID is used to index the
> +          per-port registers in the second register area.
> +
> +    required:
> +      - interrupts
> +      - port-id
> +      - phy-mode
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          const: marvell,armada-7k-pp22
> +    then:
> +      patternProperties:
> +        '^eth[0-9a-f]*(@.*)?$':
> +          required:
> +            - gop-port-id

else:
  patternProperties:
     ....
       gop-port-id: false

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |

Best regards,
Krzysztof

Hi Krzysztof,

Thanks for your comments and time spent on reviewing my patch.
All of those improvements will be included in next version. 
Also, I would like to know your opinion about one. 

>> +
>> +  marvell,system-controller:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description: a phandle to the system controller.
>> +
>> +patternProperties:
>> +  '^eth[0-9a-f]*(@.*)?$':
> 
> The name should be "(ethernet-)?port", unless anything depends on
> particular naming?

What do you think about pattern "^(ethernet-)?eth[0-9a-f]+(@.*)?$"?
It resembles pattern found in net/ethernet-phy.yaml like
properties:$nodename:pattern:"^ethernet-phy(@[a-f0-9]+)?$", while 
still passing `dt_binding_check' and `dtbs_check'. It should also 
comply with your comment.

Best regards,
Michał

On 01/10/2022 17:53, Michał Grzelak wrote:
> Hi Krzysztof,
> 
> Thanks for your comments and time spent on reviewing my patch.
> All of those improvements will be included in next version. 
> Also, I would like to know your opinion about one. 
> 
>>> +
>>> +  marvell,system-controller:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description: a phandle to the system controller.
>>> +
>>> +patternProperties:
>>> +  '^eth[0-9a-f]*(@.*)?$':
>>
>> The name should be "(ethernet-)?port", unless anything depends on
>> particular naming?
> 
> What do you think about pattern "^(ethernet-)?eth[0-9a-f]+(@.*)?$"?
> It resembles pattern found in net/ethernet-phy.yaml like
> properties:$nodename:pattern:"^ethernet-phy(@[a-f0-9]+)?$", while 
> still passing `dt_binding_check' and `dtbs_check'. It should also 
> comply with your comment.

Node names like ethernet-eth do not make much sense because they contain
redundant ethernet or eth. AFAIK, all other bindings like that call
these ethernet-ports (or sometimes shorter - ports). Unless this device
is different than all others?

Best regards,
Krzysztof

niedz., 2 paź 2022 o 10:00 Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> napisał(a):
>
> On 01/10/2022 17:53, Michał Grzelak wrote:
> > Hi Krzysztof,
> >
> > Thanks for your comments and time spent on reviewing my patch.
> > All of those improvements will be included in next version.
> > Also, I would like to know your opinion about one.
> >
> >>> +
> >>> +  marvell,system-controller:
> >>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >>> +    description: a phandle to the system controller.
> >>> +
> >>> +patternProperties:
> >>> +  '^eth[0-9a-f]*(@.*)?$':
> >>
> >> The name should be "(ethernet-)?port", unless anything depends on
> >> particular naming?
> >
> > What do you think about pattern "^(ethernet-)?eth[0-9a-f]+(@.*)?$"?
> > It resembles pattern found in net/ethernet-phy.yaml like
> > properties:$nodename:pattern:"^ethernet-phy(@[a-f0-9]+)?$", while
> > still passing `dt_binding_check' and `dtbs_check'. It should also
> > comply with your comment.
>
> Node names like ethernet-eth do not make much sense because they contain
> redundant ethernet or eth. AFAIK, all other bindings like that call
> these ethernet-ports (or sometimes shorter - ports). Unless this device
> is different than all others?
>

IMO "^(ethernet-)?port@[0-9]+$" for the subnodes' names could be fine
(as long as we don't have to modify the existing .dtsi files) - there
is no dependency in the driver code on that.

Best regards,
Marcin

On 02/10/2022 10:23, Marcin Wojtas wrote:
>niedz., 2 paź 2022 o 10:00 Krzysztof Kozlowski
><krzysztof.kozlowski@linaro.org> napisał(a):
>>
>> On 01/10/2022 17:53, Michał Grzelak wrote:
>> > Hi Krzysztof,
>> >
>> > Thanks for your comments and time spent on reviewing my patch.
>> > All of those improvements will be included in next version.
>> > Also, I would like to know your opinion about one.
>> >
>> >>> +
>> >>> +  marvell,system-controller:
>> >>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> >>> +    description: a phandle to the system controller.
>> >>> +
>> >>> +patternProperties:
>> >>> +  '^eth[0-9a-f]*(@.*)?$':
>> >>
>> >> The name should be "(ethernet-)?port", unless anything depends on
>> >> particular naming?
>> >
>> > What do you think about pattern "^(ethernet-)?eth[0-9a-f]+(@.*)?$"?
>> > It resembles pattern found in net/ethernet-phy.yaml like
>> > properties:$nodename:pattern:"^ethernet-phy(@[a-f0-9]+)?$", while
>> > still passing `dt_binding_check' and `dtbs_check'. It should also
>> > comply with your comment.
>>
>> Node names like ethernet-eth do not make much sense because they contain
>> redundant ethernet or eth. AFAIK, all other bindings like that call
>> these ethernet-ports (or sometimes shorter - ports). Unless this device
>> is different than all others?
>>
>
>IMO "^(ethernet-)?port@[0-9]+$" for the subnodes' names could be fine
>(as long as we don't have to modify the existing .dtsi files) - there
>is no dependency in the driver code on that.

Indeed, driver's code isn't dependent; however, there is a dependency
on 'eth[0-2]' name in all relevant .dts and .dtsi files, e.g.:

https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/armada-375.dtsi#L190
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi#L72

Ports under 'ethernet' node are named eth[0-2], thus those and all .dts files 
including the above would have to be modified to pass through `dtbs_check'.

Best regards,
Michał

pon., 3 paź 2022 o 19:06 Michał Grzelak <mig@semihalf.com> napisał(a):
>
> On 02/10/2022 10:23, Marcin Wojtas wrote:
> >niedz., 2 paź 2022 o 10:00 Krzysztof Kozlowski
> ><krzysztof.kozlowski@linaro.org> napisał(a):
> >>
> >> On 01/10/2022 17:53, Michał Grzelak wrote:
> >> > Hi Krzysztof,
> >> >
> >> > Thanks for your comments and time spent on reviewing my patch.
> >> > All of those improvements will be included in next version.
> >> > Also, I would like to know your opinion about one.
> >> >
> >> >>> +
> >> >>> +  marvell,system-controller:
> >> >>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >> >>> +    description: a phandle to the system controller.
> >> >>> +
> >> >>> +patternProperties:
> >> >>> +  '^eth[0-9a-f]*(@.*)?$':
> >> >>
> >> >> The name should be "(ethernet-)?port", unless anything depends on
> >> >> particular naming?
> >> >
> >> > What do you think about pattern "^(ethernet-)?eth[0-9a-f]+(@.*)?$"?
> >> > It resembles pattern found in net/ethernet-phy.yaml like
> >> > properties:$nodename:pattern:"^ethernet-phy(@[a-f0-9]+)?$", while
> >> > still passing `dt_binding_check' and `dtbs_check'. It should also
> >> > comply with your comment.
> >>
> >> Node names like ethernet-eth do not make much sense because they contain
> >> redundant ethernet or eth. AFAIK, all other bindings like that call
> >> these ethernet-ports (or sometimes shorter - ports). Unless this device
> >> is different than all others?
> >>
> >
> >IMO "^(ethernet-)?port@[0-9]+$" for the subnodes' names could be fine
> >(as long as we don't have to modify the existing .dtsi files) - there
> >is no dependency in the driver code on that.
>
> Indeed, driver's code isn't dependent; however, there is a dependency
> on 'eth[0-2]' name in all relevant .dts and .dtsi files, e.g.:
>
> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/armada-375.dtsi#L190
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi#L72
>
> Ports under 'ethernet' node are named eth[0-2], thus those and all .dts files
> including the above would have to be modified to pass through `dtbs_check'.
>

Can you please double check?
The .dts files use labels, the node name they relate to should be irrelevant:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/marvell/cn9130-db.dtsi?h=v6.0#n122
(BTW, for A7k8k/CN913x example please use updated names, i.e.
s/cpm_/cp0_/ and s/cps_/cp1_/)
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/armada-375-db.dts?h=v6.0#n167

Best regards,
Marcin

On 03/10/2022 19:06, Michał Grzelak wrote:
> On 02/10/2022 10:23, Marcin Wojtas wrote:
>> niedz., 2 paź 2022 o 10:00 Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> napisał(a):
>>>
>>> On 01/10/2022 17:53, Michał Grzelak wrote:
>>>> Hi Krzysztof,
>>>>
>>>> Thanks for your comments and time spent on reviewing my patch.
>>>> All of those improvements will be included in next version.
>>>> Also, I would like to know your opinion about one.
>>>>
>>>>>> +
>>>>>> +  marvell,system-controller:
>>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>>>>> +    description: a phandle to the system controller.
>>>>>> +
>>>>>> +patternProperties:
>>>>>> +  '^eth[0-9a-f]*(@.*)?$':
>>>>>
>>>>> The name should be "(ethernet-)?port", unless anything depends on
>>>>> particular naming?
>>>>
>>>> What do you think about pattern "^(ethernet-)?eth[0-9a-f]+(@.*)?$"?
>>>> It resembles pattern found in net/ethernet-phy.yaml like
>>>> properties:$nodename:pattern:"^ethernet-phy(@[a-f0-9]+)?$", while
>>>> still passing `dt_binding_check' and `dtbs_check'. It should also
>>>> comply with your comment.
>>>
>>> Node names like ethernet-eth do not make much sense because they contain
>>> redundant ethernet or eth. AFAIK, all other bindings like that call
>>> these ethernet-ports (or sometimes shorter - ports). Unless this device
>>> is different than all others?
>>>
>>
>> IMO "^(ethernet-)?port@[0-9]+$" for the subnodes' names could be fine
>> (as long as we don't have to modify the existing .dtsi files) - there
>> is no dependency in the driver code on that.
> 
> Indeed, driver's code isn't dependent; however, there is a dependency
> on 'eth[0-2]' name in all relevant .dts and .dtsi files, e.g.:
> 
> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/armada-375.dtsi#L190
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi#L72
> 
> Ports under 'ethernet' node are named eth[0-2], thus those and all .dts files 
> including the above would have to be modified to pass through `dtbs_check'.

I didn't get it. What is the "dependency"? Usage of some names is not a
dependency... Old bindings were not precising any specific name of
subnodes, therefore I commented to change it. If the DTS already use
some other name, you can change them if none of upstream implementations
(BSD, bootloaders, firmware, Linux kernel) depend on it.

Best regards,
Krzysztof

pon., 3 paź 2022 o 19:29 Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> napisał(a):
>
> On 03/10/2022 19:06, Michał Grzelak wrote:
> > On 02/10/2022 10:23, Marcin Wojtas wrote:
> >> niedz., 2 paź 2022 o 10:00 Krzysztof Kozlowski
> >> <krzysztof.kozlowski@linaro.org> napisał(a):
> >>>
> >>> On 01/10/2022 17:53, Michał Grzelak wrote:
> >>>> Hi Krzysztof,
> >>>>
> >>>> Thanks for your comments and time spent on reviewing my patch.
> >>>> All of those improvements will be included in next version.
> >>>> Also, I would like to know your opinion about one.
> >>>>
> >>>>>> +
> >>>>>> +  marvell,system-controller:
> >>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle
> >>>>>> +    description: a phandle to the system controller.
> >>>>>> +
> >>>>>> +patternProperties:
> >>>>>> +  '^eth[0-9a-f]*(@.*)?$':
> >>>>>
> >>>>> The name should be "(ethernet-)?port", unless anything depends on
> >>>>> particular naming?
> >>>>
> >>>> What do you think about pattern "^(ethernet-)?eth[0-9a-f]+(@.*)?$"?
> >>>> It resembles pattern found in net/ethernet-phy.yaml like
> >>>> properties:$nodename:pattern:"^ethernet-phy(@[a-f0-9]+)?$", while
> >>>> still passing `dt_binding_check' and `dtbs_check'. It should also
> >>>> comply with your comment.
> >>>
> >>> Node names like ethernet-eth do not make much sense because they contain
> >>> redundant ethernet or eth. AFAIK, all other bindings like that call
> >>> these ethernet-ports (or sometimes shorter - ports). Unless this device
> >>> is different than all others?
> >>>
> >>
> >> IMO "^(ethernet-)?port@[0-9]+$" for the subnodes' names could be fine
> >> (as long as we don't have to modify the existing .dtsi files) - there
> >> is no dependency in the driver code on that.
> >
> > Indeed, driver's code isn't dependent; however, there is a dependency
> > on 'eth[0-2]' name in all relevant .dts and .dtsi files, e.g.:
> >
> > https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/armada-375.dtsi#L190
> > https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi#L72
> >
> > Ports under 'ethernet' node are named eth[0-2], thus those and all .dts files
> > including the above would have to be modified to pass through `dtbs_check'.
>
> I didn't get it. What is the "dependency"? Usage of some names is not a
> dependency... Old bindings were not precising any specific name of
> subnodes, therefore I commented to change it. If the DTS already use
> some other name, you can change them if none of upstream implementations
> (BSD, bootloaders, firmware, Linux kernel) depend on it.
>

None of the PP2 drivers depends on nodes' names, so indeed we can
safely modify that and update the relevant .dtsi files. One comment
here, though - if we switch to e.g. ethernet-port@0 subnode, there is
a requirement of specifying a 'reg' property: See below warning:

Documentation/devicetree/bindings/net/marvell,pp2.example.dts:36.27-41.13:
Warning (unit_address_vs_reg):
/example-0/ethernet@f0000/ethernet-port@0: node has a unit name, but
no reg or ranges property

I think this convention is good and my idea is to use 'reg' property
as a port ID (like the DSA does) - it would become required. However,
we should retain 'port-id' to maintain backward compatibility. Once
this schema gets accepted, I'll prepare a driver update.

Best regards,
Marcin

Hi Michał,

Additional comments inline.

wt., 27 wrz 2022 o 01:22 Michał Grzelak <mig@semihalf.com> napisał(a):
>
> This converts the marvell,pp2 bindings from text to proper schema.
>
> Move 'marvell,system-controller' and 'dma-coherent' properties from
> port up to the controller node, to match what is actually done in DT.
>
> Signed-off-by: Michał Grzelak <mig@semihalf.com>
> ---
>  .../devicetree/bindings/net/marvell,pp2.yaml  | 241 ++++++++++++++++++
>  .../devicetree/bindings/net/marvell-pp2.txt   | 141 ----------
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 242 insertions(+), 142 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,pp2.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/marvell-pp2.txt
>
> diff --git a/Documentation/devicetree/bindings/net/marvell,pp2.yaml b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> new file mode 100644
> index 000000000000..6faa4c87dfc6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> @@ -0,0 +1,241 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell CN913X / Marvell Armada 375, 7K, 8K Ethernet Controller
> +
> +maintainers:
> +  - Marcin Wojtas <mw@semihalf.com>
> +  - Russell King <linux@armlinux.org>
> +
> +description: |
> +  Marvell Armada 375 Ethernet Controller (PPv2.1)
> +  Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
> +  Marvell CN913X Ethernet Controller (PPv2.3)
> +
> +properties:
> +  compatible:
> +    enum:
> +      - marvell,armada-375-pp2
> +      - marvell,armada-7k-pp22
> +
> +  reg:
> +    minItems: 3
> +    maxItems: 4
> +    description: |
> +      For "marvell,armada-375-pp2", must contain the following register sets:
> +        - common controller registers
> +        - LMS registers
> +        - one register area per Ethernet port
> +      For "marvell,armada-7k-pp22" used by 7K/8K and CN913X, must contain the following register sets:
> +        - packet processor registers
> +        - networking interfaces registers
> +        - CM3 address space used for TX Flow Control
> +
> +  clocks:
> +    minItems: 2
> +    items:
> +      - description: main controller clock
> +      - description: GOP clock
> +      - description: MG clock
> +      - description: MG Core clock
> +      - description: AXI clock
> +
> +  clock-names:
> +    minItems: 2
> +    items:
> +      - const: pp_clk
> +      - const: gop_clk
> +      - const: mg_clk
> +      - const: mg_core_clk
> +      - const: axi_clk
> +
> +  dma-coherent: true
> +  '#size-cells': true
> +  '#address-cells': true
> +
> +  marvell,system-controller:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: a phandle to the system controller.
> +
> +patternProperties:
> +  '^eth[0-9a-f]*(@.*)?$':
> +    type: object
> +    properties:
> +      interrupts:
> +        minItems: 1
> +        maxItems: 10
> +        description: interrupt(s) for the port
> +
> +      interrupt-names:
> +        items:
> +          - const: hif0
> +          - const: hif1
> +          - const: hif2
> +          - const: hif3
> +          - const: hif4
> +          - const: hif5
> +          - const: hif6
> +          - const: hif7
> +          - const: hif8
> +          - const: link
> +
> +        description: >
> +          if more than a single interrupt for is given, must be the
> +          name associated to the interrupts listed. Valid names are:
> +          "hifX", with X in [0..8], and "link". The names "tx-cpu0",
> +          "tx-cpu1", "tx-cpu2", "tx-cpu3" and "rx-shared" are supported
> +          for backward compatibility but shouldn't be used for new
> +          additions.
> +
> +      port-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: ID of the port from the MAC point of view.
> +
> +      phy:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description: >
> +          a phandle to a phy node defining the PHY address
> +          (as the reg property, a single integer).
> +
> +      phy-mode:
> +        $ref: "ethernet-controller.yaml#/properties/phy-mode"
> +
> +      marvell,loopback:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: port is loopback mode.
> +
> +      gop-port-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: >
> +          only for marvell,armada-7k-pp22, ID of the port from the
> +          GOP (Group Of Ports) point of view. This ID is used to index the
> +          per-port registers in the second register area.
> +
> +    required:
> +      - interrupts
> +      - port-id
> +      - phy-mode
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          const: marvell,armada-7k-pp22
> +    then:
> +      patternProperties:
> +        '^eth[0-9a-f]*(@.*)?$':
> +          required:
> +            - gop-port-id

For this variant, 'marvell,system-controller' should also be marked as required.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    // For Armada 375 variant
> +    #include <dt-bindings/interrupt-controller/mvebu-icu.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    ethernet@f0000 {
> +      compatible = "marvell,armada-375-pp2";
> +      reg = <0xf0000 0xa000>,
> +            <0xc0000 0x3060>,
> +            <0xc4000 0x100>,
> +            <0xc5000 0x100>;
> +      clocks = <&gateclk 3>, <&gateclk 19>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      clock-names = "pp_clk", "gop_clk";
> +
> +      eth0: eth0@c4000 {
> +        reg = <0xc4000>;
> +        interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
> +        port-id = <0>;
> +        phy = <&phy0>;
> +        phy-mode = "gmii";
> +      };
> +
> +      eth1: eth1@c5000 {
> +        reg = <0xc5000>;
> +        interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> +        port-id = <1>;
> +        phy = <&phy3>;
> +        phy-mode = "gmii";
> +      };
> +    };
> +
> +  - |
> +    // For Armada 7k/8k and Cn913x variants
> +    #include <dt-bindings/interrupt-controller/mvebu-icu.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    cpm_ethernet: ethernet@0 {
> +      compatible = "marvell,armada-7k-pp22";
> +      reg = <0x0 0x100000>, <0x129000 0xb000>, <0x220000 0x800>;
> +      clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>,
> +               <&cpm_syscon0 1 5>, <&cpm_syscon0 1 6>, <&cpm_syscon0 1 18>;

s/syscon0/clk/

Also add missing marvell,system-controller.

Best regards,
Marcin