Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAJNi4rNwPQf747UM_hiYYwL=HDxg8QnPpfFPv1PfrtN9ZP1y1g@mail.gmail.com>
 <35f95ba3-8a7b-7918-0f9d-e14274a5ffe9@arm.com>
In-Reply-To: <35f95ba3-8a7b-7918-0f9d-e14274a5ffe9@arm.com>
From:   richard clark <richard.xnu.clark@gmail.com>
Date:   Thu, 26 May 2022 11:44:41 +0800
Message-ID: <CAJNi4rMbBbLP2Tsv-wnnJKt4Y0moOE5-sNaZN1fCty908pwAKw@mail.gmail.com>
Subject: Re: Question about SPIs' interrupt trigger type restrictions
To:     Robin Murphy <robin.murphy@arm.com>
Cc:     maz@kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, May 26, 2022 at 3:14 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-05-25 11:01, richard clark wrote:
> > Hi Marc,
> >
> > For below code snippet about SPI interrupt trigger type:
> >
> > static int gic_set_type(struct irq_data *d, unsigned int type)
> > {
> >          ...
> >          /* SPIs have restrictions on the supported types */
> >          if ((range =3D=3D SPI_RANGE || range =3D=3D ESPI_RANGE) &&
> >              type !=3D IRQ_TYPE_LEVEL_HIGH && type !=3D IRQ_TYPE_EDGE_R=
ISING)
> >                  return -EINVAL;
> >          ...
> > }
> >
> > We have a device at hand whose interrupt type is SPI, Falling edge
> > will trigger the interrupt. But the request_irq(50, handler,
> > IRQ_TYPE_EDGE_FALLING, ...) will return -EINVAL.
> >
> > The question is, why must the SPI interrupt use IRQ_TYPE_EDGE_RISING
> > instead of IRQ_TYPE_EDGE_FALLING?
>
> Because that's what the GIC architecture[1] says. From section 1.2.1
> "Interrupt Types":
>
> "An interrupt that is edge-triggered has the following property:
>         =E2=80=A2 It is asserted on detection of a rising edge of an inte=
rrupt signal

This rising edge detection is not true, it's also asserted by falling
edge, just like the GICD_ICFGR register says:
Changing the interrupt configuration between level-sensitive and
*edge-triggered (in either direction)* at a time when there is a
pending interrupt ..., which
has been confirmed by GIC-500 on my platform.

> and then, regardless of the state of the signal, remains asserted until
> the interrupt is acknowledged by software."
>
> External signals with the wrong polarity may need external logic to

IMO, it's not wrong polarity for a device to interrupt the processor
with a falling edge, it's normal. Actually, the GIC supports
edge-trigger type:
'0b10 Corresponding interrupt is edge-triggered', the
IRQ_TYPE_EDGE_RISING check in gic_set_type(...) is just a sanity check
from this point of view.
I would more like to have below changes applied:

--- a/linux/drivers/irqchip/irq-gic-v3.c
+++ b/linux/drivers/irqchip/irq-gic-v3.c

@@ -560,8 +560,7 @@ static int gic_set_type(struct irq_data *d,
unsigned int type)
                return type !=3D IRQ_TYPE_EDGE_RISING ? -EINVAL : 0;
        /* SPIs have restrictions on the supported types */
-       if ((range =3D=3D SPI_RANGE || range =3D=3D ESPI_RANGE) &&
-           type !=3D IRQ_TYPE_LEVEL_HIGH && type !=3D IRQ_TYPE_EDGE_RISING=
)
+       if ((range =3D=3D SPI_RANGE || range =3D=3D ESPI_RANGE) && !(type &=
 0xf))
                return -EINVAL;

 I believe irq-gic.c has the same issue, but can't confirm now.


> invert them (which might even be offered by the GIC implementation
> itself, e.g. [2]), but the programmer's model neither knows nor cares
> about such details, it only knows notions of "edge-triggered" and
> "level-sensitive", where from its point of view the asserted states are
> rising and high respectively.
>
> Robin.
>
> [1] https://developer.arm.com/documentation/ihi0069/latest
> [2]
> https://developer.arm.com/documentation/100336/0106/components-and-config=
uration/spi-collator/spi-collator-wires?lang=3Den