Re: [PATCH v2 1/9] irqchip/sun6i-r: Use a stacked irqchip driver

[Maxime Ripard]

Hi Samuel,

On Mon, Jun 15, 2020 at 12:29:50AM -0500, Samuel Holland wrote:
> On 6/8/20 3:48 AM, Maxime Ripard wrote:
> > On Sun, May 24, 2020 at 11:12:54PM -0500, Samuel Holland wrote:
> >> The R_INTC in the A31 and newer sun8i/sun50i SoCs is more similar to the
> >> original sun4i interrupt controller than the sun7i/sun9i NMI controller.
> >> It is used for two distinct purposes:
> >>  1) To control the trigger, latch, and mask for the NMI input pin
> >>  2) To provide the interrupt input for the ARISC coprocessor
> >>
> >> As this interrupt controller is not documented, information about it
> >> comes from vendor-provided ARISC firmware and from experimentation.
> >>
> >> Like the original sun4i interrupt controller, it has:
> >>  - A VECTOR_REG at 0x00 (configurable via the BASE_ADDR_REG at 0x04)
> >>  - A NMI_CTRL_REG, PENDING_REG, and ENABLE_REG as used by both the
> >>    sun4i and sunxi-nmi drivers
> >>  - A MASK_REG at 0x50
> >>  - A RESP_REG at 0x60
> >>
> >> Differences from the sun4i interrupt controller appear to be:
> >>  - It is only known to have one register of each kind (max 32 inputs)
> >>  - There is no FIQ-related logic
> >>  - There is no interrupt priority logic
> >>
> >> In order to fulfill its two purposes, this hardware block combines two
> >> types of IRQs. First, the NMI pin is routed to the "IRQ 0" input on this
> >> chip, with a trigger type controlled by the NMI_CTRL_REG. The "IRQ 0
> >> pending" output from this chip, if enabled, is then routed to a SPI IRQ
> >> input on the GIC, as IRQ_TYPE_LEVEL_HIGH. In other words, bit 0 of
> >> ENABLE_REG *does* affect the NMI IRQ seen at the GIC.
> >>
> >> The NMI is then followed by a contiguous block of (at least) 15 IRQ
> >> inputs that are connected in parallel to both R_INTC and the GIC. Or
> >> in other words, the other bits of ENABLE_REG *do not* affect the IRQs
> >> seen at the GIC.
> >>
> >> Finally, the global "IRQ pending" output from R_INTC, after being masked
> >> by MASK_REG and RESP_REG, is connected to the "external interrupt" input
> >> of the ARISC CPU (an OR1200). This path is not relevant to Linux.
> >>
> >> Because of the 1:1 correspondence between R_INTC and GIC inputs, this is
> >> a perfect scenario for using a stacked irqchip driver. We want to hook
> >> into enabling/disabling IRQs to add more features to the GIC
> >> (specifically to allow masking the NMI and setting its trigger type),
> >> but we don't need to actually handle the IRQ in this driver.
> >>
> >> And since R_INTC is in the always-on power domain, and its output is
> >> connected directly in to the power management coprocessor, a stacked
> >> irqchip driver provides a simple way to add wakeup support to this set
> >> of IRQs. That is a future patch; for now, just the NMI is moved over.
> >>
> >> This driver keeps the same DT binding as the existing driver. The
> >> "interrupt" property of the R_INTC node is used to determine 1) the
> >> offset between GIC and R_INTC hwirq numbers and 2) the type of trigger
> >> between the R_INTC "IRQ 0 pending" output and the GIC NMI input.
> >>
> >> This commit mostly reverts commit 173bda53b340 ("irqchip/sunxi-nmi:
> >> Support sun6i-a31-r-intc compatible").
> >>
> >> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx>
> > 
> > As usual, thanks for that commit log (and the experiments you did to
> > write it in the first place).
> > 
> > Acked-by: Maxime Ripard <mripard@xxxxxxxxxx>
> > 
> > Maxime
> 
> I've done more experimenting, and I've learned what comes after the first 16
> IRQs: all of the other SPI IRQs, multiplexed in clusters of 8, with per-IRQ
> masks for the inputs to each cluster.
> 
> In fact, the H6 has so many IRQs that it begins to use the the second register
> in each group (0x14, 0x44, 0x54). This means that more than one register in each
> group are in fact implemented.
> 
> See https://linux-sunxi.org/INTC#IRQ_Mapping for more details.
> 
> The ability to send other IRQs to the AR100 makes it possible to implement
> functionality like USB Remote Wakeup or Wake on LAN without adding complexity to
> the AR100 firmware.
> 
> I will need to update the driver to take advantage of this ability, and it
> raises some questions about the binding. Since the NMI is not the
> lowest-numbered IRQ that can be mapped,

I'm not quite sure I get that part. From the link you mentionned above,
the NMI is the interrupt 0 for all the SoCs, so we shouldn't have any
number lower?

> the numbering scheme would need to change.

As far as I know, upstream we only ever use the 0 interrupt for the AXP,
so it should be fairly easy to come up with a numbering scheme that is
backward compatible with that.

> Maybe the IRQ number should be the same as the GIC SPI IRQ number? But
> this would mean a new compatible.
> 
> The other question is which devices should be routed through this irqchip
> driver? Anything that provides a wakeup source needs to go through it, so it can
> intercept irq_set_wake. Probably other devices should not, as 1) not quite all
> IRQs can even be sent to the AR100 for wakeup (e.g. the A64 appears to stop in
> the middle of the GPU IRQs), and 2) stacking on another irqchip driver adds a
> (tiny) overhead to masking/unmasking during IRQ handling.

It's probably a bit of a non-answer, but I guess all the devices for
which it makes sense? The ethernet / USB controllers that you already
mentionned would make sense, the GPIO banks too, possibly the UARTs?

I guess we could just enable most of the one that makes sense at first,
and then discuss cases we didn't consider as we discover them?

Maxime

Attachment: signature.asc
Description: PGP signature