Hi Krzysztof, Let me chime in. wt., 11 paź 2022 o 21:50 Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> napisał(a): > > On 11/10/2022 15:06, Michał Grzelak wrote: > > Convert 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. > > You need to also mention other changes done during conversion - > requiring subnodes to be named "(ethernet-)?ports", deprecating port-id. > > > > > Signed-off-by: Michał Grzelak <mig@xxxxxxxxxxxx> > > --- > > .../devicetree/bindings/net/marvell,pp2.yaml | 286 ++++++++++++++++++ > > .../devicetree/bindings/net/marvell-pp2.txt | 141 --------- > > MAINTAINERS | 2 +- > > 3 files changed, 287 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..24c6aeb46814 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml > > @@ -0,0 +1,286 @@ > > +# 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@xxxxxxxxxxxx> > > + - Russell King <linux@xxxxxxxxxxxx> > > + > > +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 > > + > > + "#address-cells": > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > + 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 > > + > > + marvell,system-controller: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: a phandle to the system controller. > > + > > +patternProperties: > > + '^(ethernet-)?port@[0-9]+$': > > + type: object > > + description: subnode for each ethernet port. > > + > > + properties: > > + interrupts: > > + minItems: 1 > > + maxItems: 10 > > + description: interrupt(s) for the port > > + > > + interrupt-names: > > + minItems: 1 > > + 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. > > + > > + reg: > > + description: ID of the port from the MAC point of view. > > + > > + port-id: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > deprecated: true > > > + description: > > > + ID of the port from the MAC point of view. > > + Legacy binding for backward compatibility. > > + > > + 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 > > + - reg > > Keep the same order of items here as in list of properties > > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + > > +allOf: > > + - $ref: ethernet-controller.yaml# > > Hmm, are you sure this applies to top-level properties, not to > ethernet-port subnodes? Your ports have phy-mode and phy - just like > ethernet-controller. If I understand correctly, your Armada Ethernet > Controller actually consists of multiple ethernet controllers? > PP2 is a single controller with common HW blocks, such as queue/buffer management, parser/classifier, register space, and more. It controls up to 3 MAC's (ports) that can be connected to phys, sfp cages, etc. The latter cannot exist on their own and IMO the current hierarchy - the main controller with subnodes (ports) properly reflects the hardware. Anyway, the ethernet-controller.yaml properties fit to the subnodes. Apart from the name. The below is IMO a good description:. > If so, this should be moved to proper place inside patternProperties. > Maybe the subnodes should also be renamed from ports to just "ethernet" > (as ethernet-controller.yaml expects), but other schemas do not follow > this convention, ethernet@ { ethernet-port@0 { } ethernet-port@1 { } } What do you recommend? Thanks, Marcin
