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> <CAJNi4rMbBbLP2Tsv-wnnJKt4Y0moOE5-sNaZN1fCty908pwAKw@mail.gmail.com>
 <87ee0gn5rq.wl-maz@kernel.org> <5a8d5c51-ae02-a335-6768-2bedf809ab63@arm.com>
In-Reply-To: <5a8d5c51-ae02-a335-6768-2bedf809ab63@arm.com>
From:   richard clark <richard.xnu.clark@gmail.com>
Date:   Thu, 26 May 2022 20:30:35 +0800
Message-ID: <CAJNi4rOtsfzJWsgx7CABczbf3DqqsigErf1YFw6hKOhWnACnHQ@mail.gmail.com>
Subject: Re: Question about SPIs' interrupt trigger type restrictions
To:     Robin Murphy <robin.murphy@arm.com>
Cc:     Marc Zyngier <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 4:41 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-05-26 07:54, Marc Zyngier 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> wr=
ote:
> >>>
> >>> 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_ED=
GE_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_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=
 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>
> >
> > and the direction here is about the configuration bit, not the edge
> > direction.
>
> Indeed it's clearly referring to either direction of *the change*, i.e.
> from edge to level and from level to edge.
>
> >> 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.
> >
> > If your system behaves differently, this is because something is
> > inverting the signal, which is extremely common. Just describe this in
> > your device tree, or lie to the kernel, whichever way you want.
>
> I think the important concept to grasp here is that what we describe in
> DT is not properties of the device in isolation, but properties of its
> integration into the system as a whole. Consider the "reg" property,
> which in 99% of cases has nothing to do with the actual device it
> belongs to, but is instead describing a property of the interconnect,
> namely how its address map decodes to a particular interface, to which
> the given device happens to be attached.

I don't care about the DT at all... The essential is- does the GIC
only support rising edge detection really just as the document says,
I'm doubtful about that ;-)

>
> At the HDL level, the device block may very well have an output signal
> which idles at logic-high, and pulses low to indicate an event, however
> it only becomes an *interrupt* if it is wired up to an interrupt
> controller; on its own it's just some output signal. What the DT
> interrupt specifier describes is that wiring, *from the interrupt
> controller's point of view*. If a pulsed signal is fed into an Arm GIC
> SPI input then as an interrupt it *is* IRQ_TYPE_EDGE_RISING, because
> that's how the GIC hardware will treat it. The integration as a whole

EDGE_RISING can leave its mark in the GIC, that's the *how*, but why
EDGE_FALLING can't, any reasons to justify this behavior?
I believe that the drivers still work if the trigger type sanity check
in the GIC driver is removed.

Thanks

> takes care of the details and makes that happen, so what the logic
> levels at some arbitrary HDL boundary in the middle might be is simply
> not meaningful.
>
> Thanks,
> Robin.