linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
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
Date: Thu, 26 May 2022 13:52:40 +0100	[thread overview]
Message-ID: <87czg0mp5z.wl-maz@kernel.org> (raw)
In-Reply-To: <CAJNi4rOHYqL8jN5Ju3ndANc-5Te4WEc-z5YGxCN-2ZtN8vf1cQ@mail.gmail.com>

On Thu, 26 May 2022 13:09:32 +0100,
richard clark <richard.xnu.clark@gmail.com> 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:
> >
> > 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> 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 == SPI_RANGE || range == ESPI_RANGE) &&
> > > > >              type != IRQ_TYPE_LEVEL_HIGH && type != 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_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:
> > > >         • 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>
> 
> Context sensitive(register-update leaves UNKNOWN pending) and
> 
> >
> > and the direction here is about the configuration bit, not the edge
> > direction.
> 
> with this(configuration bit: either level-sensitive or
> edge-triggered), but it doesn't matter.
> 
> >
> > > 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.
> 
> 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.

> 
> 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.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-05-26 12:53 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-25 10:01 Question about SPIs' interrupt trigger type restrictions richard clark
2022-05-25 19:14 ` Robin Murphy
2022-05-26  3:44   ` richard clark
2022-05-26  6:54     ` Marc Zyngier
2022-05-26  8:40       ` Robin Murphy
2022-05-26 12:30         ` richard clark
2022-05-26 13:00           ` Marc Zyngier
2022-05-26 12:09       ` richard clark
2022-05-26 12:52         ` Marc Zyngier [this message]
2022-05-30  8:40         ` Daniel Thompson
2022-06-05 12:03           ` richard clark
2022-06-06 10:08             ` Daniel Thompson
2022-06-07  1:29               ` richard clark
2022-06-07  8:47                 ` Daniel Thompson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87czg0mp5z.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=bogdan.folea@nxp.com \
    --cc=bogdan.hamciuc@nxp.com \
    --cc=catalin-dan.udma@nxp.com \
    --cc=ciprianmarian.costea@nxp.com \
    --cc=ghennadi.procopciuc@nxp.com \
    --cc=leoyang.li@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=radu-nicolae.pirea@nxp.com \
    --cc=richard.xnu.clark@gmail.com \
    --cc=robin.murphy@arm.com \
    --cc=s32@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).