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
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
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-configuration/spi-collator/spi-collator-wires?lang=en
On Thu, May 26, 2022 at 3:14 AM Robin Murphy <[email protected]> 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 ..., 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 != IRQ_TYPE_EDGE_RISING ? -EINVAL : 0;
/* SPIs have restrictions on the supported types */
- if ((range == SPI_RANGE || range == ESPI_RANGE) &&
- type != IRQ_TYPE_LEVEL_HIGH && type != IRQ_TYPE_EDGE_RISING)
+ if ((range == SPI_RANGE || range == 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-configuration/spi-collator/spi-collator-wires?lang=en