| Message ID | 20210103103101.33603-2-samuel@sholland.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sunxi: Support IRQ wakeup from deep sleep | expand |
Hi Samuel, Thanks a lot for working on this I'm fine with the rest of the work, but I have a couple of questions On Sun, Jan 03, 2021 at 04:30:52AM -0600, Samuel Holland wrote: > The R_INTC in the A31 and newer sun8i/sun50i SoCs has additional > functionality compared to the sun7i/sun9i NMI controller. Among other > things, it multiplexes up to 128 interrupts corresponding to (and in > parallel to) the first 128 GIC SPIs. This means the NMI is no longer the > lowest-numbered interrupt, since it is SPI 32 or 96 (depending on SoC). > > To allow access to all multiplexed IRQs, the R_INTC requires a new > binding where the interrupt number matches the GIC interrupt number. > For simplicity, copy the three-cell GIC binding; this disambiguates > interrupt 0 in the old binding (the NMI) from interrupt 0 in the new > binding (SPI 0) by the number of cells. It's not really clear to me what the ambiguity is between the NMI and the SPI 0 interrupt? In general, it looks like switching to a 3-cell binding with the GIC SPI value looks weird to me, since the GIC isn't the parent at all of these interrupts. If the ambiguity is that a stacked irqchip driver needs to have the same interrupt number than the GIC, and that the 0 interrupt for the NMI controller (used by the PMIC) and is actually the 32 (or 96) GIC interrupt and thus breaks that requirement, can't we fix this in the driver based on the compatible? Something like if the interrupt number is 0, with a A31 or newer compatible, then add the proper offset in sun6i_r_intc_domain_alloc? Maxime
On 1/8/21 3:44 AM, Maxime Ripard wrote: > Hi Samuel, > > Thanks a lot for working on this > > I'm fine with the rest of the work, but I have a couple of questions > > On Sun, Jan 03, 2021 at 04:30:52AM -0600, Samuel Holland wrote: >> The R_INTC in the A31 and newer sun8i/sun50i SoCs has additional >> functionality compared to the sun7i/sun9i NMI controller. Among other >> things, it multiplexes up to 128 interrupts corresponding to (and in >> parallel to) the first 128 GIC SPIs. This means the NMI is no longer the >> lowest-numbered interrupt, since it is SPI 32 or 96 (depending on SoC). >> >> To allow access to all multiplexed IRQs, the R_INTC requires a new >> binding where the interrupt number matches the GIC interrupt number. >> For simplicity, copy the three-cell GIC binding; this disambiguates >> interrupt 0 in the old binding (the NMI) from interrupt 0 in the new >> binding (SPI 0) by the number of cells. > > It's not really clear to me what the ambiguity is between the NMI and > the SPI 0 interrupt? Here's the ASCII art I will include in v4: NMI IRQ DIRECT IRQs MUXED IRQs bit 0 bits 1-18 bits 19-31 +---------+ +---------+ +---------+ +---------+ | NMI Pad | | IRQ d | | IRQ m | | IRQ m+7 | +---------+ +---------+ +---------+ +---------+ | | | | | | | | | | | |......| | +------V------+ +-------------+ | | | +--V------V--+ | | Invert/ | | Write | | | | | AND with | | | Edge Detect | | PENDING[0] | | | | | MUX[m/8] | | +-------------+ +-------------+ | | | +------------+ | | | | | | | | +--V-------V--+ +--V--+ | +--V--+ | +--V--+ | Set Reset| | GIC | | | GIC | | | GIC | | Latch | | SPI | | | SPI |... | ...| SPI | +-------------+ | N+d | | | m | | | m+7 | | | +-----+ | +-----+ | +-----+ | | | | +-------V-+ +-V-----------+ +---------V---+ +---------V----------+ | GIC SPI | | AND with | | AND with | | AND with | | N (=32) | | ENABLE[0] | | ENABLE[d] | | ENABLE[19+m/8] | +---------+ +-------------+ +-------------+ +--------------------+ | | | +------V------+ +------V------+ +---------V----------+ | Read | | Read | | Read | | PENDING[0] | | PENDING[d] | | PENDING[19+m/8] | +-------------+ +-------------+ +--------------------+ There are two overlapping ranges of IRQs supported by the controller, and so there are two different IRQs you could call "IRQ 0": - Bit 0 of PENDING/ENABLE/MASK, aka d==0, the NMI - This maps to bit 32 of the MUX register range (SPI 32) - This is what the old binding calls "IRQ 0" - Bit 0 of MUX, aka m==0, aka SPI 0, the UART0 IRQ - This maps to bit 19 of PENDING/ENABLE/MASK - This is what the new binding calls "IRQ 0" You can see this insertion in the middle of the MUX range when looking at the mask of implemented MUX bits in the A31 variant: 0xffffffff, 0xfff80000, <<< this gap here is for the 19 direct IRQs 0xffffffff, 0x0000000f, If you call the NMI "IRQ 0", then there is no way to specify the muxed IRQs. SPI 0 maps to bit 19, but so do SPI 1-7. So if I was to specify "IRQ 19", you wouldn't know which of those 8 muxed SPIs I am referring to. On the other hand, if you call the first muxed IRQ "IRQ 0", then there is an unambiguous number for every interrupt supported by this driver. > In general, it looks like switching to a 3-cell binding with the GIC SPI > value looks weird to me, since the GIC isn't the parent at all of these > interrupts. The GIC is *a* parent of all of these interrupts, and is *the* parent of the NMI. > If the ambiguity is that a stacked irqchip driver needs to have the same > interrupt number than the GIC, and that the 0 interrupt for the NMI > controller (used by the PMIC) and is actually the 32 (or 96) GIC > interrupt and thus breaks that requirement, can't we fix this in the > driver based on the compatible? No, while the NMI is direct "IRQ 0" at this irqchip, it is *also* muxed "IRQ 32" at this same irqchip. > Something like if the interrupt number is 0, with a A31 or newer > compatible, then add the proper offset in sun6i_r_intc_domain_alloc? If you translate 0 to 32, then you cannot represent muxed IRQ 0 (the UART0 IRQ) at all. > Maxime Cheers, Samuel
On Sun, Jan 03, 2021 at 04:30:52AM -0600, Samuel Holland wrote: > The R_INTC in the A31 and newer sun8i/sun50i SoCs has additional > functionality compared to the sun7i/sun9i NMI controller. Among other > things, it multiplexes up to 128 interrupts corresponding to (and in > parallel to) the first 128 GIC SPIs. This means the NMI is no longer the > lowest-numbered interrupt, since it is SPI 32 or 96 (depending on SoC). > > To allow access to all multiplexed IRQs, the R_INTC requires a new > binding where the interrupt number matches the GIC interrupt number. > For simplicity, copy the three-cell GIC binding; this disambiguates > interrupt 0 in the old binding (the NMI) from interrupt 0 in the new > binding (SPI 0) by the number of cells. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > .../allwinner,sun6i-a31-r-intc.yaml | 64 +++++++++++++++++++ > .../allwinner,sun7i-a20-sc-nmi.yaml | 10 --- > 2 files changed, 64 insertions(+), 10 deletions(-) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml > new file mode 100644 > index 000000000000..18805b6555c2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml > @@ -0,0 +1,64 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Allwinner A31 NMI/Wakeup Interrupt Controller Device Tree Bindings > + > +maintainers: > + - Chen-Yu Tsai <wens@csie.org> > + - Maxime Ripard <mripard@kernel.org> > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# > + > +properties: > + "#interrupt-cells": > + const: 3 > + description: > + The first cell is GIC_SPI (0), the second cell is the IRQ number, and > + the third cell is the trigger type as defined in interrupt.txt in this > + directory. > + > + compatible: > + oneOf: > + - items: > + - enum: > + - allwinner,sun8i-a83t-r-intc > + - allwinner,sun50i-a64-r-intc > + - allwinner,sun50i-h6-r-intc > + - const: allwinner,sun6i-a31-r-intc > + - const: allwinner,sun6i-a31-r-intc > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + interrupt-controller: true > + > +required: > + - "#interrupt-cells" > + - compatible > + - reg > + - interrupts > + - interrupt-controller > + > +unevaluatedProperties: false additionalProperties: false (a bit stricter and actually implemented ATM) > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + r_intc: interrupt-controller@1f00c00 { > + compatible = "allwinner,sun50i-a64-r-intc", > + "allwinner,sun6i-a31-r-intc"; > + interrupt-controller; > + #interrupt-cells = <3>; > + reg = <0x01f00c00 0x400>; > + interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>; > + }; > + > +... > diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml > index 8acca0ae3129..f34ecc8c7093 100644 > --- a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml > +++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml > @@ -22,23 +22,13 @@ properties: > > compatible: > oneOf: > - - const: allwinner,sun6i-a31-r-intc > - const: allwinner,sun6i-a31-sc-nmi > deprecated: true > - const: allwinner,sun7i-a20-sc-nmi > - - items: > - - const: allwinner,sun8i-a83t-r-intc > - - const: allwinner,sun6i-a31-r-intc > - const: allwinner,sun9i-a80-nmi > - - items: > - - const: allwinner,sun50i-a64-r-intc > - - const: allwinner,sun6i-a31-r-intc > - items: > - const: allwinner,sun50i-a100-nmi > - const: allwinner,sun9i-a80-nmi > - - items: > - - const: allwinner,sun50i-h6-r-intc > - - const: allwinner,sun6i-a31-r-intc > > reg: > maxItems: 1 > -- > 2.26.2 >
diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml new file mode 100644 index 000000000000..18805b6555c2 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml @@ -0,0 +1,64 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner A31 NMI/Wakeup Interrupt Controller Device Tree Bindings + +maintainers: + - Chen-Yu Tsai <wens@csie.org> + - Maxime Ripard <mripard@kernel.org> + +allOf: + - $ref: /schemas/interrupt-controller.yaml# + +properties: + "#interrupt-cells": + const: 3 + description: + The first cell is GIC_SPI (0), the second cell is the IRQ number, and + the third cell is the trigger type as defined in interrupt.txt in this + directory. + + compatible: + oneOf: + - items: + - enum: + - allwinner,sun8i-a83t-r-intc + - allwinner,sun50i-a64-r-intc + - allwinner,sun50i-h6-r-intc + - const: allwinner,sun6i-a31-r-intc + - const: allwinner,sun6i-a31-r-intc + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + interrupt-controller: true + +required: + - "#interrupt-cells" + - compatible + - reg + - interrupts + - interrupt-controller + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + r_intc: interrupt-controller@1f00c00 { + compatible = "allwinner,sun50i-a64-r-intc", + "allwinner,sun6i-a31-r-intc"; + interrupt-controller; + #interrupt-cells = <3>; + reg = <0x01f00c00 0x400>; + interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>; + }; + +... diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml index 8acca0ae3129..f34ecc8c7093 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml +++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml @@ -22,23 +22,13 @@ properties: compatible: oneOf: - - const: allwinner,sun6i-a31-r-intc - const: allwinner,sun6i-a31-sc-nmi deprecated: true - const: allwinner,sun7i-a20-sc-nmi - - items: - - const: allwinner,sun8i-a83t-r-intc - - const: allwinner,sun6i-a31-r-intc - const: allwinner,sun9i-a80-nmi - - items: - - const: allwinner,sun50i-a64-r-intc - - const: allwinner,sun6i-a31-r-intc - items: - const: allwinner,sun50i-a100-nmi - const: allwinner,sun9i-a80-nmi - - items: - - const: allwinner,sun50i-h6-r-intc - - const: allwinner,sun6i-a31-r-intc reg: maxItems: 1
The R_INTC in the A31 and newer sun8i/sun50i SoCs has additional functionality compared to the sun7i/sun9i NMI controller. Among other things, it multiplexes up to 128 interrupts corresponding to (and in parallel to) the first 128 GIC SPIs. This means the NMI is no longer the lowest-numbered interrupt, since it is SPI 32 or 96 (depending on SoC). To allow access to all multiplexed IRQs, the R_INTC requires a new binding where the interrupt number matches the GIC interrupt number. For simplicity, copy the three-cell GIC binding; this disambiguates interrupt 0 in the old binding (the NMI) from interrupt 0 in the new binding (SPI 0) by the number of cells. Signed-off-by: Samuel Holland <samuel@sholland.org> --- .../allwinner,sun6i-a31-r-intc.yaml | 64 +++++++++++++++++++ .../allwinner,sun7i-a20-sc-nmi.yaml | 10 --- 2 files changed, 64 insertions(+), 10 deletions(-) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/allwinner,sun6i-a31-r-intc.yaml