On 03/11/2022 13:51, Lorenzo Bianconi wrote: >>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-boot.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-boot.yaml >>> new file mode 100644 >>> index 000000000000..6c3c514c48ef >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-boot.yaml >> >> arm is only for top-level stuff. Choose appropriate subsystem, soc as >> last resort. > > these chips are used only for networking so is net folder fine? So this is some MMIO and no actual device? Then rather soc. > >> >>> @@ -0,0 +1,47 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mt7986-wo-boot.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: >>> + MediaTek Wireless Ethernet Dispatch WO boot controller interface for MT7986 >>> + >>> +maintainers: >>> + - Lorenzo Bianconi <lorenzo@xxxxxxxxxx> >>> + - Felix Fietkau <nbd@xxxxxxxx> >>> + >>> +description: >>> + The mediatek wo-boot provides a configuration interface for WED WO >>> + boot controller on MT7986 soc. >> >> And what is "WED WO boot controller? > > WED WO is a chip used for networking packet processing offloaded to the Soc > (e.g. packet reordering). WED WO boot is the memory used to store start address > of wo firmware. Anyway I will let Sujuan comment on this. A bit more should be in description. > >> >>> + >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - mediatek,mt7986-wo-boot >>> + - const: syscon >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + soc { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + wo_boot: syscon@15194000 { >>> + compatible = "mediatek,mt7986-wo-boot", "syscon"; >>> + reg = <0 0x15194000 0 0x1000>; >>> + }; >>> + }; >>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-ccif.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-ccif.yaml >>> new file mode 100644 >>> index 000000000000..6357a206587a >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-ccif.yaml >>> @@ -0,0 +1,50 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mt7986-wo-ccif.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: MediaTek Wireless Ethernet Dispatch WO controller interface for MT7986 >>> + >>> +maintainers: >>> + - Lorenzo Bianconi <lorenzo@xxxxxxxxxx> >>> + - Felix Fietkau <nbd@xxxxxxxx> >>> + >>> +description: >>> + The mediatek wo-ccif provides a configuration interface for WED WO >>> + controller on MT7986 soc. >> >> All previous comments apply. >> >>> + >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - mediatek,mt7986-wo-ccif >>> + - const: syscon >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + soc { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + wo_ccif0: syscon@151a5000 { >>> + compatible = "mediatek,mt7986-wo-ccif", "syscon"; >>> + reg = <0 0x151a5000 0 0x1000>; >>> + interrupts = <GIC_SPI 205 IRQ_TYPE_LEVEL_HIGH>; >>> + }; >>> + }; >>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-dlm.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-dlm.yaml >>> new file mode 100644 >>> index 000000000000..a499956d9e07 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mt7986-wo-dlm.yaml >>> @@ -0,0 +1,50 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/arm/mediatek/mediatek,mt7986-wo-dlm.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: MediaTek Wireless Ethernet Dispatch WO hw rx ring interface for MT7986 >>> + >>> +maintainers: >>> + - Lorenzo Bianconi <lorenzo@xxxxxxxxxx> >>> + - Felix Fietkau <nbd@xxxxxxxx> >>> + >>> +description: >>> + The mediatek wo-dlm provides a configuration interface for WED WO >>> + rx ring on MT7986 soc. >>> + >>> +properties: >>> + compatible: >>> + const: mediatek,mt7986-wo-dlm >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + resets: >>> + maxItems: 1 >>> + >>> + reset-names: >>> + maxItems: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - resets >>> + - reset-names >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + soc { >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + wo_dlm0: wo-dlm@151e8000 { >> >> Node names should be generic. >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > DLM is a chip used to store the data rx ring of wo firmware. I do not have a > better node name (naming is always hard). Can you please suggest a better name? The problem is that you added three new devices which seem to be for the same device - WED. It looks like some hacky way of avoid proper hardware description - let's model everything as MMIO and syscons... For such model - register addresses exposed as separate devices - I do not have appropriate name, but the real problem is not in the name. It's in the hardware description. Best regards, Krzysztof