wt., 11 paź 2022 o 22:34 Marcin Wojtas <mw@xxxxxxxxxxxx> napisał(a): > > 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? > I moved the ethernet-controller.yaml reference to under the subnode (this allowed me to remove phy and phy-mode description)) and it doesn't complain about the node naming. Please let me know if below would be acceptable. --- a/Documentation/devicetree/bindings/net/marvell,pp2.yaml +++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml @@ -61,7 +61,11 @@ patternProperties: type: object description: subnode for each ethernet port. + allOf: + - $ref: ethernet-controller.yaml# + properties: interrupts: minItems: 1 maxItems: 10 @@ -95,19 +99,11 @@ patternProperties: 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. @@ -132,7 +128,6 @@ required: - clock-names allOf: - - $ref: ethernet-controller.yaml# - if: Best regards, Marcin
