| Message ID | 20231228060825.1380439-2-james.tai@realtek.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Initial support for the Realtek interrupt controller | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 28/12/2023 07:08, James Tai wrote: > Add the YAML documentation for Realtek DHC (Digital Home Center) SoCs. > > Signed-off-by: James Tai <james.tai@realtek.com> Thank you for your patch. There is something to discuss/improve. > + interrupts: > + minItems: 1 > + maxItems: 3 > + description: > + Contains the GIC SPI IRQs mapped to the external interrupt lines. > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - '#address-cells' > + - interrupts > + > +additionalProperties: false > + > +allOf: If there is going to be new version/resend, allOf: block goes before additionalProperties:. > + - $ref: /schemas/interrupt-controller.yaml# > + - if: > + properties: > + compatible: > + enum: > + - realtek,rtd1319-intc-iso > + then: > + properties: > + interrupts: > + minItems: 1 Why the second interrupt is optional? It's a SoC, the pins are not configurable usually. Same question for other cases. > +... Best regards, Krzysztof
Hi Krzysztof, >> Add the YAML documentation for Realtek DHC (Digital Home Center) SoCs. >> >> Signed-off-by: James Tai <james.tai@realtek.com> > >Thank you for your patch. There is something to discuss/improve. > >> + interrupts: >> + minItems: 1 >> + maxItems: 3 >> + description: >> + Contains the GIC SPI IRQs mapped to the external interrupt lines. >> + >> +required: >> + - compatible >> + - reg >> + - interrupt-controller >> + - '#interrupt-cells' >> + - '#address-cells' >> + - interrupts >> + >> +additionalProperties: false >> + >> +allOf: > >If there is going to be new version/resend, allOf: block goes before >additionalProperties:. > I will move the 'allOf: block' to go before 'additionalPropertie' in next patches. >> + - $ref: /schemas/interrupt-controller.yaml# >> + - if: >> + properties: >> + compatible: >> + enum: >> + - realtek,rtd1319-intc-iso >> + then: >> + properties: >> + interrupts: >> + minItems: 1 > >Why the second interrupt is optional? It's a SoC, the pins are not configurable >usually. Same question for other cases. > I thought it was defined this way to accommodate different SoCs. I will remove the 'minItems'. Should the correct version look like the following? allOf: - $ref: /schemas/interrupt-controller.yaml# - if: properties: compatible: enum: - realtek,rtd1319-intc-iso then: properties: interrupts: items: - description: isolation irqs - description: rtc irq ... ... Thanks for your feedback. Regards, James
diff --git a/Documentation/devicetree/bindings/interrupt-controller/realtek,intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/realtek,intc.yaml new file mode 100644 index 000000000000..d3e4919850cc --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/realtek,intc.yaml @@ -0,0 +1,131 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/realtek,intc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Realtek DHC SoCs Interrupt Controller + +maintainers: + - James Tai <james.tai@realtek.com> + +description: + This interrupt controller is a component of Realtek DHC (Digital Home Center) + SoCs and is designed to receive interrupts from peripheral devices. + + Each DHC SoC has two sets of interrupt controllers, each capable of + handling up to 32 interrupts. + +properties: + compatible: + enum: + - realtek,rtd1319-intc-iso + - realtek,rtd1319-intc-misc + - realtek,rtd1319d-intc-iso + - realtek,rtd1319d-intc-misc + - realtek,rtd1325-intc-iso + - realtek,rtd1325-intc-misc + - realtek,rtd1619b-intc-iso + - realtek,rtd1619b-intc-misc + + reg: + maxItems: 1 + + interrupt-controller: true + + '#interrupt-cells': + const: 1 + + '#address-cells': + const: 0 + + interrupts: + minItems: 1 + maxItems: 3 + description: + Contains the GIC SPI IRQs mapped to the external interrupt lines. + +required: + - compatible + - reg + - interrupt-controller + - '#interrupt-cells' + - '#address-cells' + - interrupts + +additionalProperties: false + +allOf: + - $ref: /schemas/interrupt-controller.yaml# + - if: + properties: + compatible: + enum: + - realtek,rtd1319-intc-iso + then: + properties: + interrupts: + minItems: 1 + items: + - description: isolation irqs + - description: rtc irq + - if: + properties: + compatible: + enum: + - realtek,rtd1319d-intc-iso + - realtek,rtd1325-intc-iso + - realtek,rtd1619b-intc-iso + then: + properties: + interrupts: + minItems: 1 + items: + - description: isolation irqs + - description: watchdog irq + - if: + properties: + compatible: + enum: + - realtek,rtd1319-intc-misc + then: + properties: + interrupts: + minItems: 3 + items: + - description: miscellaneous irqs + - description: watchdog irq + - description: uart1 irq + - description: uart2 irq + - if: + properties: + compatible: + enum: + - realtek,rtd1319d-intc-misc + - realtek,rtd1325-intc-misc + - realtek,rtd1619b-intc-misc + then: + properties: + interrupts: + minItems: 2 + items: + - description: miscellaneous irqs + - description: uart1 irq + - description: uart2 irq + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + realtek_iso_intc: interrupt-controller@40 { + compatible = "realtek,rtd1319-intc-iso"; + reg = <0x00 0x40>; + interrupt-parent = <&gic>; + interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>; + interrupt-controller; + #address-cells = <0>; + #interrupt-cells = <1>; + }; +...
Add the YAML documentation for Realtek DHC (Digital Home Center) SoCs. Signed-off-by: James Tai <james.tai@realtek.com> --- CC: Thomas Gleixner <tglx@linutronix.de> CC: Marc Zyngier <maz@kernel.org> CC: Rob Herring <robh+dt@kernel.org> CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> CC: Conor Dooley <conor+dt@kernel.org> CC: linux-kernel@vger.kernel.org CC: devicetree@vger.kernel.org v3 to v4 change: - Adjusted the allOf block to add constraints on the interrupts per variant v2 to v3 change: - Retested the bindings using the new version of the dtschema - Fixed the order of property items - Removed redundant files and replaced them with 'realtek,intc.yaml' - Replaced 'interrupts-extended' with 'interrupts' - Added a description for 'interrupts' - Reduced the example code v1 to v2 change: - Tested the bindings using 'make dt_binding_check' - Fixed code style issues .../interrupt-controller/realtek,intc.yaml | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/realtek,intc.yaml