RE: [PATCH 3/6] dt-bindings: interrupt-controller: amazon, al-fic: add error/fatal groups

[Farber, Eliav]

On Thu, Sep 24, 2026 at 06:13:52PM +0100, Conor Dooley wrote:
> Without any explanation relating to hardware, I find this very hard to
> understand. Nodes and compatible strings are devicetree concepts, that
> portion of the commit message should explain hardware detail.

Agreed, the message failed to describe the hardware. Here it is, and v2
will carry it.

The block that this driver calls a FIC is the generic Annapurna
interrupt controller. It is built from groups of up to 32 triggers. The
number of groups differs from one controller to another. Each group has
its own 0x40 register block.

Note the granularity, because it decides the shape of this binding: a
node here describes ONE GROUP, not a whole controller. reg points at
that group's 0x40 block, and the 32 hwirqs of the domain are that
group's 32 triggers. A controller with several groups appears as several
nodes, and controllers cascade when a tree needs more triggers than one
controller has.

The group registers:

  0x00  cause
  0x10  mask           gates the INFO output
  0x28  group control  revision in bits 29-28
  0x2c  error mask     gates the ERROR output
  0x34  fatal mask     gates the FATAL output

A group has one cause register. The mask registers decide which output a
set cause bit drives. The classic revision has neither the error nor the
fatal mask, and neither output; the revision that added both reports 1 in
the control register, and every group reports it in its own.

The SoC carries INFO, ERROR and FATAL as three separate aggregation
trees, and a unit is required to keep them separate: mapping one event
to more than one severity is forbidden by the interrupt methodology, so
a group belongs to exactly one tree. A group in the error tree has the
controller's ERROR output wired towards its parent and its INFO output
unused. The error tree terminates in one dedicated GIC SPI, the fatal
tree in another. The two trees differ in trigger type as well, error
being level high and fatal edge rising.

> What this sounds like from your commit message is that you have a new
> revision of this block, and instead of adding an al-fic-v2 compatible,
> you're using two new compatibles to describe the new features and using
> the old compatible to describe the common featureset.

Not quite, on both halves.

There is a new revision, and no string encodes it, because of the
granularity above. The node describes a group, and the group reports the
revision itself, in its own control register. An amazon,al-fic-v2 would
put a controller-level version number into a node that describes one
group of that controller, and would duplicate a register that same group
already exposes. 4/6 reads the register instead. v3 reports 2 there, and
differs from v2 only by an erratum in the two new mask registers, which
is why 5/6 is four lines.

The strings name which output of the controller is the one connected, and
so which of the three mask registers the driver must program for this
group. Nothing in the block reports that. Cause, control, revision and
the 32-trigger domain are identical in all three cases, and the choice is
fixed when the SoC is wired.

> Without a dts, I cannot say for sure.

Fair, and there is nothing in tree to look at. The driver and binding
landed in 2019 without a devicetree. Here is the topology, which v2 will
add to the binding as an example:

    /* group A and group B of one controller in the error tree. The
     * controller has a single ERROR output, so both groups reach the
     * parent through the same line.
     */
    err_fic_a: interrupt-controller@fd8a8500 {
        compatible = "amazon,al-fic-error";
        reg = <0xfd8a8500 0x40>;
        interrupt-controller;
        #interrupt-cells = <2>;
        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
    };

    err_fic_b: interrupt-controller@fd8a8540 {
        compatible = "amazon,al-fic-error";
        reg = <0xfd8a8540 0x40>;
        interrupt-controller;
        #interrupt-cells = <2>;
        interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
    };

    /* a peripheral's own error group, cascaded into bit 6 of group A */
    interrupt-controller@fd8a8580 {
        compatible = "amazon,al-fic-error";
        reg = <0xfd8a8580 0x40>;
        interrupt-controller;
        #interrupt-cells = <2>;
        interrupt-parent = <&err_fic_a>;
        interrupts = <6 IRQ_TYPE_LEVEL_HIGH>;
    };

The first two nodes are what patch 2/6 is for. The groups of one
controller share that controller's outputs, so a real devicetree has
nodes on one parent line, and a chained handler can only be installed
once per parent. Cascading adds more of the same, since an aggregating
group collects many peripherals onto the line above it. I will say that
in 2/6 instead of "on some platforms".

Thanks,
Eliav