On 2018-03-02 20:49, Rob Herring wrote: > On Fri, Feb 23, 2018 at 10:09:00PM +0100, Rasmus Villemoes wrote: >> This adds Device Tree binding documentation for the external interrupt >> lines with configurable polarity present on some Layerscape SOCs. >> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx> >> --- >> .../interrupt-controller/fsl,ls-extirq.txt | 44 ++++++++++++++++++++++ >> 1 file changed, 44 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt >> >> diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt >> new file mode 100644 >> index 000000000000..e510c715e8f6 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.txt >> @@ -0,0 +1,44 @@ >> +* Freescale Layerscape external IRQs >> + >> +Some Layerscape SOCs (LS1021A, LS1043A, LS1046A) 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". >> +- interrupt-controller: Identifies the node as an interrupt controller >> +- #interrupt-cells: Must be 2. The first element is the index of the >> + external interrupt line. The second element is the trigger type. >> +- interrupt-parent: phandle of GIC. >> +- reg: Specifies the Interrupt Polarity Control Register (INTPCR) in the SCFG. >> +- fsl,extirq-map: Specifies the mapping to interrupt numbers in the parent >> + interrupt controller. Interrupts are mapped one-to-one to parent >> + interrupts. > > Use the interrupt-map property for this. Please point me at some documentation for that. AFAICT, that would require the property to consist of n*(3+2)*4 values, instead of just n (with 3+2 being sum of #interrupt-cells and 4 being the four different allowed incoming IRQ_TYPE_*). That seems quite excessive. I used a private property based on advice from Marc Most interrupt controllers use a private property, potentially with a range (see the recent example of the Qualcomm PDC [1]). [1] https://patchwork.kernel.org/patch/10208037/ and the qcom,pdc-ranges has this description (and that patch has your Reviewed-by): +- qcom,pdc-ranges: + Usage: required + Value type: <u32 array> + Definition: Specifies the PDC pin offset and the number of PDC ports. + The tuples indicates the valid mapping of valid PDC ports + and their hwirq mapping. + The first element of the tuple is the starting PDC port. + The second element is the GIC hwirq number for the PDC port. + The third element is the number of interrupts in sequence. In my case, this is simplified by the external irq lines being numbered consecutively from 0, so the array index itself serves as the "starting pdc port". I also omit the "number of interrupts in sequence", and have that be 1 implicitly, since it will only ever be either 6 or 12 elements. So I end up with a simple array of GIC hwirq numbers. >> + >> +Optional properties: >> +- fsl,bit-reverse: This boolean property should be set on the LS1021A >> + if the SCFGREVCR register has been set to all-ones (which is usually >> + the case), meaning that all reads and writes of SCFG registers are >> + implicitly bit-reversed. Other compatible platforms do not have such >> + a register. >> + >> +Example: >> + scfg: scfg@1570000 { >> + compatible = "fsl,ls1021a-scfg", "syscon"; >> + ... >> + extirq: interrupt-controller { >> + compatible = "fsl,ls1021a-extirq"; >> + #interrupt-cells = <2>; >> + interrupt-controller; >> + interrupt-parent = <&gic>; >> + reg = <0x1ac>; > > This needs the length too. What is buys us is following the standard in > which mmio has a #size-cells >= 1. BTW, you need a #size-cells and > #address-cells properties in the parent. (I think dtc will complain if > not). Well, the parent consists solely of 32 bit registers, so I think it would make sense to have #size-cells = 0, to avoid redundant boilerplate in subnodes' reg properties. But you're right, the ls1021a scfg node currently has neither #size-cells or #address-cells, so I'll have to add a patch adding those before adding this subnode. And if #size-cells=0 is somehow frowned upon, I'll just make it 1. -- Rasmus Villemoes Software Developer Prevas A/S Hedeager 3 DK-8200 Aarhus N +45 51210274 rasmus.villemoes@xxxxxxxxx www.prevas.dk -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
