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

[Samuel Holland <samuel@sholland.org>]

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