| Message ID | 20220425140214.32448-1-michael@walle.cc |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2,1/2] dt-bindings: interrupt-controller: fsl,ls-extirq: convert to YAML | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 88 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 25/04/2022 16:02, Michael Walle wrote: > Convert the fsl,ls-extirq binding to the new YAML format. > (...) > diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml > new file mode 100644 > index 000000000000..39d120ad7549 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml > @@ -0,0 +1,88 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/fsl,ls-extirq.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale Layerscape External Interrupt Controller > + > +maintainers: > + - Shawn Guo <shawnguo@kernel.org> > + - Li Yang <leoyang.li@nxp.com> > + > +description: | > + Some Layerscape SOCs (LS1021A, LS1043A, LS1046A LS1088A, LS208xA, > + LX216xA) support inverting the polarity of certain external interrupt > + lines. > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# I have doubts whether this is here applicable. See also Rob's comment: https://lore.kernel.org/all/YjjJpxLWJ%2FYOe0OX@robh.at.kernel.org/ This device does not have children, so the interrupt-controller schema should be probably skipped. > + > +properties: > + compatible: > + oneOf: > + - enum: > + - fsl,ls1021a-extirq > + - fsl,ls1043a-extirq > + - fsl,ls1088a-extirq > + - items: > + - enum: > + - fsl,ls1046a-extirq > + - const: fsl,ls1043a-extirq > + - items: > + - enum: > + - fsl,ls2080a-extirq > + - fsl,lx2160a-extirq > + - const: fsl,ls1088a-extirq > + > + '#interrupt-cells': > + const: 2 > + > + '#address-cells': > + const: 0 > + > + interrupt-controller: true > + > + reg: > + maxItems: 1 > + description: > + Specifies the Interrupt Polarity Control Register (INTPCR) in the > + SCFG or the External Interrupt Control Register (IRQCR) in the ISC. > + > + interrupt-map: > + description: Specifies the mapping from external interrupts to GIC interrupts. > + > + interrupt-map-mask: > + items: > + - const: 0xffffffff This looks highly permissive mask and should be instead defined per variant, for example (quickly looking at DTS): 0x7 for ls1021 0xf for ls1043a and ls1088a You might need to correct the DTS. Some confirmation from someone with datasheet would be good. Best regards, Krzysztof
Am 2022-04-25 20:36, schrieb Krzysztof Kozlowski: > On 25/04/2022 16:02, Michael Walle wrote: >> Convert the fsl,ls-extirq binding to the new YAML format. >> > > (...) > >> diff --git >> a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml >> b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml >> new file mode 100644 >> index 000000000000..39d120ad7549 >> --- /dev/null >> +++ >> b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml >> @@ -0,0 +1,88 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: >> http://devicetree.org/schemas/interrupt-controller/fsl,ls-extirq.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Freescale Layerscape External Interrupt Controller >> + >> +maintainers: >> + - Shawn Guo <shawnguo@kernel.org> >> + - Li Yang <leoyang.li@nxp.com> >> + >> +description: | >> + Some Layerscape SOCs (LS1021A, LS1043A, LS1046A LS1088A, LS208xA, >> + LX216xA) support inverting the polarity of certain external >> interrupt >> + lines. >> + >> +allOf: >> + - $ref: /schemas/interrupt-controller.yaml# > > I have doubts whether this is here applicable. See also Rob's comment: > https://lore.kernel.org/all/YjjJpxLWJ%2FYOe0OX@robh.at.kernel.org/ > > This device does not have children, so the interrupt-controller schema > should be probably skipped. ok. >> + >> +properties: >> + compatible: >> + oneOf: >> + - enum: >> + - fsl,ls1021a-extirq >> + - fsl,ls1043a-extirq >> + - fsl,ls1088a-extirq >> + - items: >> + - enum: >> + - fsl,ls1046a-extirq >> + - const: fsl,ls1043a-extirq >> + - items: >> + - enum: >> + - fsl,ls2080a-extirq >> + - fsl,lx2160a-extirq >> + - const: fsl,ls1088a-extirq >> + >> + '#interrupt-cells': >> + const: 2 >> + >> + '#address-cells': >> + const: 0 >> + >> + interrupt-controller: true >> + >> + reg: >> + maxItems: 1 >> + description: >> + Specifies the Interrupt Polarity Control Register (INTPCR) in >> the >> + SCFG or the External Interrupt Control Register (IRQCR) in the >> ISC. >> + >> + interrupt-map: btw. minItems: 12 maxItems: 12 Isn't working here, is that expected? The validator seem to get the count of the elements of one tuple wrong. I.e. arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dtb: interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too short >> + description: Specifies the mapping from external interrupts to >> GIC interrupts. >> + >> + interrupt-map-mask: >> + items: >> + - const: 0xffffffff > > This looks highly permissive mask and should be instead defined per > variant, for example (quickly looking at DTS): > 0x7 for ls1021 > 0xf for ls1043a and ls1088a Just that I understand it correctly, the result of the AND with that mask is then looked up in the interrupt-map (the first entry there)? > You might need to correct the DTS. Some confirmation from someone with > datasheet would be good. According to their datasheets they have the following number of external IRQs: - ls1021a has 6, - ls1043a has 12, - ls1046a has 12, - ls1088a has 12, - ls2080a has 12, - lx2160a has 12. That is what I need to confirm, right? Is there a better way than the following snippet: properties: interrupt-map-mask: true allOf: - if: properties: compatible: contains: enum: - fsl,ls1021a-extirq then: properties: interrupt-map-mask: items: - const: 0x7 - const: 0 - if: properties: compatible: contains: enum: - fsl,ls1043a-extirq - fsl,ls1046a-extirq - fsl,ls1088a-extirq - fsl,ls2080a-extirq - fsl,lx2160a-extirq then: properties: interrupt-map-mask: items: - const: 0xf - const: 0 -michael
On 25/04/2022 23:58, Michael Walle wrote: >>> + reg: >>> + maxItems: 1 >>> + description: >>> + Specifies the Interrupt Polarity Control Register (INTPCR) in >>> the >>> + SCFG or the External Interrupt Control Register (IRQCR) in the >>> ISC. >>> + >>> + interrupt-map: > > btw. > > minItems: 12 > maxItems: 12 > > Isn't working here, is that expected? The validator seem to get the > count > of the elements of one tuple wrong. > > I.e. > arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dtb: > interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, > 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, > 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, > 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too > short Works for me (in different schema)... maybe update your dtschema? > >>> + description: Specifies the mapping from external interrupts to >>> GIC interrupts. >>> + >>> + interrupt-map-mask: >>> + items: >>> + - const: 0xffffffff >> >> This looks highly permissive mask and should be instead defined per >> variant, for example (quickly looking at DTS): >> 0x7 for ls1021 >> 0xf for ls1043a and ls1088a > > Just that I understand it correctly, the result of the AND with that > mask is then looked up in the interrupt-map (the first entry there)? Yes, the child (first) interrupt specifier. Since address-cells are 0, this will be bit-AND with [0-5]. >> You might need to correct the DTS. Some confirmation from someone with >> datasheet would be good. > > According to their datasheets they have the following number of external > IRQs: > - ls1021a has 6, > - ls1043a has 12, > - ls1046a has 12, > - ls1088a has 12, > - ls2080a has 12, > - lx2160a has 12. > > That is what I need to confirm, right? Yes. > > Is there a better way than the following snippet: > > properties: > interrupt-map-mask: true > > allOf: > - if: > properties: > compatible: > contains: > enum: > - fsl,ls1021a-extirq > then: > properties: > interrupt-map-mask: > items: > - const: 0x7 > - const: 0 > - if: > properties: > compatible: > contains: > enum: > - fsl,ls1043a-extirq > - fsl,ls1046a-extirq > - fsl,ls1088a-extirq > - fsl,ls2080a-extirq > - fsl,lx2160a-extirq > then: > properties: > interrupt-map-mask: > items: > - const: 0xf > - const: 0 > Exactly like this, looks good. Thank you. Best regards, Krzysztof
Am 2022-04-26 08:53, schrieb Krzysztof Kozlowski: > On 25/04/2022 23:58, Michael Walle wrote: >>>> + reg: >>>> + maxItems: 1 >>>> + description: >>>> + Specifies the Interrupt Polarity Control Register (INTPCR) in >>>> the >>>> + SCFG or the External Interrupt Control Register (IRQCR) in >>>> the >>>> ISC. >>>> + >>>> + interrupt-map: >> >> btw. >> >> minItems: 12 >> maxItems: 12 >> >> Isn't working here, is that expected? The validator seem to get the >> count >> of the elements of one tuple wrong. >> >> I.e. >> arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dtb: >> interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, >> 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, >> 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, >> 8, >> 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is >> too >> short > > > Works for me (in different schema)... maybe update your dtschema? Just updated to the latest one. But I'm still getting the same errors. $ dt-validate -V 2022.4 /home/mwalle/repos/b-linux-arm64/arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb: interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too short From schema: /home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml How is the length of one entry calculated? -michael
On 26/04/2022 09:28, Michael Walle wrote: > Am 2022-04-26 08:53, schrieb Krzysztof Kozlowski: >> On 25/04/2022 23:58, Michael Walle wrote: >>>>> + reg: >>>>> + maxItems: 1 >>>>> + description: >>>>> + Specifies the Interrupt Polarity Control Register (INTPCR) in >>>>> the >>>>> + SCFG or the External Interrupt Control Register (IRQCR) in >>>>> the >>>>> ISC. >>>>> + >>>>> + interrupt-map: >>> >>> btw. >>> >>> minItems: 12 >>> maxItems: 12 >>> >>> Isn't working here, is that expected? The validator seem to get the >>> count >>> of the elements of one tuple wrong. >>> >>> I.e. >>> arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dtb: >>> interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, >>> 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, >>> 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, >>> 8, >>> 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is >>> too >>> short >> >> >> Works for me (in different schema)... maybe update your dtschema? > > Just updated to the latest one. But I'm still getting the same errors. > > $ dt-validate -V > 2022.4 > > /home/mwalle/repos/b-linux-arm64/arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb: > interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, > 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, > 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, > 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too > short > From schema: > /home/mwalle/repos/linux-mw/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml > > How is the length of one entry calculated? If you add maxItems to your original v2 binding example, it works. If you replace your example with ls1088a and use maxItems:12, it works. So maybe something is wrong in your modified patch (which we do not have so we cannot test it)? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt deleted file mode 100644 index 4d47df1a5c91..000000000000 --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt +++ /dev/null @@ -1,53 +0,0 @@ -* Freescale Layerscape external IRQs - -Some Layerscape SOCs (LS1021A, LS1043A, LS1046A -LS1088A, LS208xA, LX216xA) support inverting -the polarity of certain external interrupt lines. - -The device node must be a child of the node representing the -Supplemental Configuration Unit (SCFG). - -Required properties: -- compatible: should be "fsl,<soc-name>-extirq", e.g. "fsl,ls1021a-extirq". - "fsl,ls1043a-extirq": for LS1043A, LS1046A. - "fsl,ls1088a-extirq": for LS1088A, LS208xA, LX216xA. -- #interrupt-cells: Must be 2. The first element is the index of the - external interrupt line. The second element is the trigger type. -- #address-cells: Must be 0. -- interrupt-controller: Identifies the node as an interrupt controller -- reg: Specifies the Interrupt Polarity Control Register (INTPCR) in - the SCFG or the External Interrupt Control Register (IRQCR) in - the ISC. -- interrupt-map: Specifies the mapping from external interrupts to GIC - interrupts. -- interrupt-map-mask: Must be <0xffffffff 0>. - -Example: - scfg: scfg@1570000 { - compatible = "fsl,ls1021a-scfg", "syscon"; - reg = <0x0 0x1570000 0x0 0x10000>; - big-endian; - #address-cells = <1>; - #size-cells = <1>; - ranges = <0x0 0x0 0x1570000 0x10000>; - - extirq: interrupt-controller@1ac { - compatible = "fsl,ls1021a-extirq"; - #interrupt-cells = <2>; - #address-cells = <0>; - interrupt-controller; - reg = <0x1ac 4>; - interrupt-map = - <0 0 &gic GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>, - <1 0 &gic GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>, - <2 0 &gic GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>, - <3 0 &gic GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>, - <4 0 &gic GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>, - <5 0 &gic GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>; - interrupt-map-mask = <0xffffffff 0x0>; - }; - }; - - - interrupts-extended = <&gic GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>, - <&extirq 1 IRQ_TYPE_LEVEL_LOW>; diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml new file mode 100644 index 000000000000..39d120ad7549 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/fsl,ls-extirq.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Freescale Layerscape External Interrupt Controller + +maintainers: + - Shawn Guo <shawnguo@kernel.org> + - Li Yang <leoyang.li@nxp.com> + +description: | + Some Layerscape SOCs (LS1021A, LS1043A, LS1046A LS1088A, LS208xA, + LX216xA) support inverting the polarity of certain external interrupt + lines. + +allOf: + - $ref: /schemas/interrupt-controller.yaml# + +properties: + compatible: + oneOf: + - enum: + - fsl,ls1021a-extirq + - fsl,ls1043a-extirq + - fsl,ls1088a-extirq + - items: + - enum: + - fsl,ls1046a-extirq + - const: fsl,ls1043a-extirq + - items: + - enum: + - fsl,ls2080a-extirq + - fsl,lx2160a-extirq + - const: fsl,ls1088a-extirq + + '#interrupt-cells': + const: 2 + + '#address-cells': + const: 0 + + interrupt-controller: true + + reg: + maxItems: 1 + description: + Specifies the Interrupt Polarity Control Register (INTPCR) in the + SCFG or the External Interrupt Control Register (IRQCR) in the ISC. + + interrupt-map: + description: Specifies the mapping from external interrupts to GIC interrupts. + + interrupt-map-mask: + items: + - const: 0xffffffff + - const: 0 + +required: + - compatible + - '#interrupt-cells' + - '#address-cells' + - interrupt-controller + - reg + - interrupt-map + - interrupt-map-mask + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + interrupt-controller@1ac { + compatible = "fsl,ls1021a-extirq"; + #interrupt-cells = <2>; + #address-cells = <0>; + interrupt-controller; + reg = <0x1ac 4>; + interrupt-map = + <0 0 &gic GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>, + <1 0 &gic GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>, + <2 0 &gic GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>, + <3 0 &gic GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>, + <4 0 &gic GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>, + <5 0 &gic GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>; + interrupt-map-mask = <0xffffffff 0x0>; + };
Convert the fsl,ls-extirq binding to the new YAML format. In contrast to the original binding documentation, there are three compatibles which are used in their corresponding device trees which have a specific compatible and the (already documented) fallback compatible: - "fsl,ls1046a-extirq", "fsl,ls1043a-extirq" - "fsl,ls2080a-extirq", "fsl,ls1088a-extirq" - "fsl,lx2160a-extirq", "fsl,ls1088a-extirq" Signed-off-by: Michael Walle <michael@walle.cc> --- changes since v1: - new patch, because it's reference in patch 2/2 .../interrupt-controller/fsl,ls-extirq.txt | 53 ----------- .../interrupt-controller/fsl,ls-extirq.yaml | 88 +++++++++++++++++++ 2 files changed, 88 insertions(+), 53 deletions(-) delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml