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;
Date:   Thu, 26 May 2022 13:52:40 +0100
Message-ID: <87czg0mp5z.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     richard clark <richard.xnu.clark@gmail.com>
Cc:     Robin Murphy <robin.murphy@arm.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        s32@nxp.com, leoyang.li@nxp.com, catalin-dan.udma@nxp.com,
        bogdan.hamciuc@nxp.com, bogdan.folea@nxp.com,
        ciprianmarian.costea@nxp.com, radu-nicolae.pirea@nxp.com,
        ghennadi.procopciuc@nxp.com
Subject: Re: Question about SPIs' interrupt trigger type restrictions
In-Reply-To: <CAJNi4rOHYqL8jN5Ju3ndANc-5Te4WEc-z5YGxCN-2ZtN8vf1cQ@mail.gmail.com>
References: <CAJNi4rNwPQf747UM_hiYYwL=HDxg8QnPpfFPv1PfrtN9ZP1y1g@mail.gmail.com>
        <35f95ba3-8a7b-7918-0f9d-e14274a5ffe9@arm.com>
        <CAJNi4rMbBbLP2Tsv-wnnJKt4Y0moOE5-sNaZN1fCty908pwAKw@mail.gmail.com>
        <87ee0gn5rq.wl-maz@kernel.org>
        <CAJNi4rOHYqL8jN5Ju3ndANc-5Te4WEc-z5YGxCN-2ZtN8vf1cQ@mail.gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, 26 May 2022 13:09:32 +0100,
richard clark <richard.xnu.clark@gmail.com> wrote:
>=20
> CC'ing some nxp guys for the S32G274A SOC...
>=20
> On Thu, May 26, 2022 at 2:54 PM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Thu, 26 May 2022 04:44:41 +0100,
> > richard clark <richard.xnu.clark@gmail.com> wrote:
> > >
> > > On Thu, May 26, 2022 at 3:14 AM Robin Murphy <robin.murphy@arm.com> w=
rote:
> > > >
> > > > 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_RISING)
> > > > >                  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_RIS=
ING
> > > > > 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 a=
n interrupt 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
> > > ...,
> >
> > Let me finish the sentence for you:
> >
> > <quote>
> > ... will leave the interrupt in an UNKNOWN pending state.
> > </quote>
>=20
> Context sensitive(register-update leaves UNKNOWN pending) and
>=20
> >
> > and the direction here is about the configuration bit, not the edge
> > direction.
>=20
> with this(configuration bit: either level-sensitive or
> edge-triggered), but it doesn't matter.
>=20
> >
> > > which has been confirmed by GIC-500 on my platform.
> >
> > From the GIC500 r1p1 TRM, page 2-8:
> >
> > <quote>
> > SPIs are generated either by wire inputs or by writes to the AXI4
> > slave programming interface.  The GIC-500 can support up to 960 SPIs
> > corresponding to the external spi[991:32] signal. The number of SPIs
> > available depends on the implemented configuration. The permitted
> > values are 32-960, in steps of 32. The first SPI has an ID number of
> > 32. You can configure whether each SPI is triggered on a rising edge
> > or is active-HIGH level-sensitive.
> > </quote>
> >
> > So I have no idea what you are talking about, but you definitely have
> > the wrong end of the stick. Both the architecture and the
> > implementations are aligned with what the GIC drivers do.
>=20
> What I am talking about is - The SPI is triggered on a rising edge
> only, while the falling edge is not as the document says. But I've
> observed the falling edge does trigger the SPI interrupt on my
> platform (the SOC is NXP S32G274A, an external wakeup signal with high
> to low transition to wake up the SOC - 'Wakeup/Interrupt Rising-Edge
> Event Enable Register (WIREER)' and 'Wakeup/Interrupt Falling-Edge
> Event Enable Register (WIFEER)', WIFEER set 1 and WIREER  set 0
> works).

This is thus driven by an external piece of HW, which, I expect, would
perform the signal conversion.

>=20
> I don't know why the GIC has such a behavior and what the subtle
> rationale is behind this, so just mark this as a record...

If you can prove that the GIC itself (and not some piece of HW on the
signal path) latches on a falling edge, then that would be a huge
bug. I would encourage you (or NXP) to report it to ARM so that it
would be fixed.

Now, given that GIC500 has been with us for over 8 years, such a bug
would have been witnessed on tons of existing systems (all the
SPI-based MSIs would trigger twice, for example). Since there has been
(to my knowledge) no report of such an issue, I seriously doubt what
you are seeing is a GIC misbehaviour.

	M.

--=20
Without deviation from the norm, progress is not possible.