Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Mon, 6 Jun 2022 11:08:48 +0100
From:   Daniel Thompson <daniel.thompson@linaro.org>
To:     richard clark <richard.xnu.clark@gmail.com>
Cc:     Marc Zyngier <maz@kernel.org>, 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
Message-ID: <20220606100848.ir4bkj5qsskxe6in@maple.lan>
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>
 <20220530084039.7rjjbm4gkplg5747@maple.lan>
 <CAJNi4rNBXs4mwv2dsR+DrV9Of6_eb3Wm3FAJxZsWv5LHnVmVxQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAJNi4rNBXs4mwv2dsR+DrV9Of6_eb3Wm3FAJxZsWv5LHnVmVxQ@mail.gmail.com>
Precedence: bulk

On Sun, Jun 05, 2022 at 08:03:02PM +0800, richard clark wrote:
> On Mon, May 30, 2022 at 4:40 PM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > On Thu, May 26, 2022 at 08:09:32PM +0800, richard clark wrote:
> > > CC'ing some nxp guys for the S32G274A SOC...
> > >
> > > On Thu, May 26, 2022 at 2:54 PM Marc Zyngier <maz@kernel.org> wrote:
> > > > richard clark <richard.xnu.clark@gmail.com> wrote:
> > > > > 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:
> > > > 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.
> > >
> > > 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).
> > >
> > > 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...
> >
> > Are you really describing the GIC behaviour here? It sounds like you are
> > describing the behaviour of the Wakeup Unit.
> 
> Definitely it's behavior of GIC, not WKPU's

I don't understand what evidence you are basing this on. Everything you
say below contradicts this assertion.


> > The SPI that goes to the GIC is the *output* of the WKPU. However the
> > registers you mention above all control edge detection at the input to
> > the WKPU. If so, the kernel would need an WKPU irqchip driver in order
> > to manage the trigger mode registers above (and to clear them).
>
> external wakeup signal has a transition from High to Low to the SOC,
> then output of WIREER (rising detect) or WIFEER(falling detect) to
> generate INTID to the GIC, you have to enable WIFEER to generate the
> IRQ signal to the GIC which is also an evidence that the external
> wakup is falling edge.

That's what I mean.

How can you reason about the behaviour of the GIC when every
observation you make is based on the behaviour of a second level
interrupt controller (the WKPU)?

I have no doubt you are observing a falling edge being delivered to the
SoC... but that falling edge is *not* delivered to the GIC; it is
delivered to the WKPU.

The WKPU then delivers an active-high signal to the GIC.

> With this clear *falling edge*, I have to
> write the below irq_request code as:
>
>   request_irq(50, wkup12_interrupt, IRQ_TYPE_EDGE_RISING...)
>
> IMO, this is very weird because the wakeup signal is falling edge from
> the point of SOC/GIC side, but I have to name it as
> *IRQ_TYPE_EDGE_RISING*, but it works just to pass the sanity
> check(although I think which is not necessary as the fact shows)

It is indeed very weird.

However it is not the falling edge from the SoC/GIC side that makes it
weird! In fact there is *not* a falling edge from the SoC *and* the GIC
because they are not the same thing. There is a secondary interrupt
controller between the SoC pin and the GIC which inverts the logic (and
also obviates any need to deploy the GIC's edge detection features at
all... IMHO the GIC trigger mode should be active high).

Instead, I think the reason your code is weird is because the irqchip
driver for the WKPU is missing or broken. A secondary interrupt
controller requires an irqchip driver or you will end up with pieces of
the interrupt controller management code (e.g. weird pokes to the WKPU
to acknowledge things) appearing in all manner of inappropriate places.


> > PS I can't find any sign of a WKPU driver in the mainline kernel and
> >    AFAICT this would make wake up sources unusable. What kernel have
> >    you been running your experiments on?
> 
> 5.10.44- BSP code from NXP:
> https://source.codeaurora.org/external/autobsps32/linux

Given you are running a vendor kernel I think you need to discuss this
with that vendor's support channels. To stop your code being weird then
you need to obtain (or implement) an irqchip driver for the WKPU.


Daniel.