mbox series

[v3,0/3] marvell,pp2.yaml and .dtsi improvements

Message ID 20221011190613.13008-1-mig@semihalf.com
Headers show
Series marvell,pp2.yaml and .dtsi improvements | expand

Message

Michał Grzelak Oct. 11, 2022, 7:06 p.m. UTC
Hi,

This patch series introduces changes of port names from ethX to 
ethernet-port@X in all relevant .dtsi files. It includes also all
considerations from thread about v2. 

Would appreciate if you had time to review that version.

Best regards,
Michał

---
Changelog:
v2->v3
- move 'reg:description' to 'allOf:if:then'
- change '#size-cells: true' and '#address-cells: true'
  to '#size-cells: const: 0' and '#address-cells: const: 1'
- replace all occurences of pattern "^eth\{hex_num}*"
  with "^(ethernet-)?port@[0-9]+$"
- add description in 'patternProperties:^...'
- add 'patternProperties:^...:interrupt-names:minItems: 1'
- add 'patternProperties:^...:reg:description'
- update 'patternProperties:^...:port-id:description'
- add 'patternProperties:^...:required: - reg'
- update '*:description:' to uppercase
- add 'allOf:then:required:marvell,system-controller'
- skip quotation marks from 'allOf:$ref'
- add 'else' schema to match 'allOf:if:then'
- restrict 'clocks' in 'allOf:if:then'
- restrict 'clock-names' in 'allOf:if:then'
- add #address-cells=<1>; #size-cells=<0>; in 'examples:'
- change every "ethX" to "ethernet-port@X" in 'examples:'
- add "reg" and comment in all ports in 'examples:'
- change /ethernet/eth0/phy-mode in examples://Armada-375
  to "rgmii-id"
- replace each cpm_ with cp0_ in 'examples:'
- replace each _syscon0 with _clk0 in 'examples:'
- remove each eth0X label in 'examples:'
- update armada-375.dtsi and armada-cp11x.dtsi to match
  marvell,pp2.yaml

v1->v2
- move 'properties' to the front of the file
- remove blank line after 'properties'
- move 'compatible' to the front of 'properties'
- move 'clocks', 'clock-names' and 'reg' definitions to 'properties' 
- substitute all occurences of 'marvell,armada-7k-pp2' with
  'marvell,armada-7k-pp22'
- add properties:#size-cells and properties:#address-cells 
- specify list in 'interrupt-names'
- remove blank lines after 'patternProperties'
- remove '^interrupt' and '^#.*-cells$' patterns
- remove blank line after 'allOf'
- remove first 'if-then-else' block from 'allOf'
- negate the condition in allOf:if schema
- delete 'interrupt-controller' from section 'examples'
- delete '#interrupt-cells' from section 'examples'

Marcin Wojtas (2):
  arm64: dts: marvell: Update network description to match schema
  ARM: dts: armada-375: Update network description to match schema

Michał Grzelak (1):
  dt-bindings: net: marvell,pp2: convert to json-schema

 .../devicetree/bindings/net/marvell,pp2.yaml  | 286 ++++++++++++++++++
 .../devicetree/bindings/net/marvell-pp2.txt   | 141 ---------
 MAINTAINERS                                   |   2 +-
 arch/arm/boot/dts/armada-375.dtsi             |  12 +-
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi |  17 +-
 5 files changed, 306 insertions(+), 152 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,pp2.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/marvell-pp2.txt

Comments

Krzysztof Kozlowski Oct. 12, 2022, 2:32 p.m. UTC | #1
On 11/10/2022 16:34, Marcin Wojtas wrote:
>>
>> 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:.

It also starts to look a bit like a switch (see bindings/net/dsa).

> 
>> 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?

Yes, keep it like this and reference the ethernet-controller.yaml in
each port.

Best regards,
Krzysztof