diff mbox series

[v3,01/10] dt-bindings: irq: sun6i-r: Split the binding from sun7i-nmi

Message ID 20210103103101.33603-2-samuel@sholland.org
State Superseded
Headers show
Series sunxi: Support IRQ wakeup from deep sleep | expand

Commit Message

Samuel Holland Jan. 3, 2021, 10:30 a.m. UTC 
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

Comments

Maxime Ripard Jan. 8, 2021, 9:44 a.m. UTC | #1 
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
Samuel Holland Jan. 8, 2021, 3:40 p.m. UTC | #2 
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
Rob Herring (Arm) Jan. 11, 2021, 10:29 p.m. UTC | #3 
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 mbox series

Patch

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