2017-03-16 15:41:22

by Mason

[permalink] [raw]
Subject: Legacy PCI interrupt support in PCIe host driver

Hello,

I am writing code for my platform's PCI Express controller.

I am stuck at the legacy PCI interrupt handling.

Interrupt requests are routed like this:

Cortex A9 MP <-- GIC(v1?) <-- system_intc <-- PCIe_root_complex

The PCIe root complex drives two interrupt signals to the system_intc

1) system_intc 54 = non-MSI interrupts
2) system_intc 55 = MSI interrupts

I think the MSI handling mostly works (although it's 340 lines long,
which seems excessive for such a common task; maybe the maintainers
will spot lots of redundant code when I submit).

As for non-MSI interrupts, there are 8 possible sources:

system_error
dma_read_ready
dma_write_ready
unsupported completion request
configuration request retry status
completer abort event
completion timeout event
legacy interrupt asserted (any of the 4 legacy interrupts)

Basically, I need to deal with the first 7 interrupts "internally"
in my PCIe root complex driver. But the last interrupt, I need to
"forward" it to the proper ISR (e.g. XHCI ISR).

For the "internal" handling, I think I need to register my own ISR
with the IRQF_SHARED flag. Then other drivers will be able to
register their ISR on the same interrupt line.

But how do I tell the PCI core that it's supposed to use interrupt 54
for legacy interrupts?

Here is my current DT:

msi0: msi@2e080 {
compatible = "sigma,msi";
reg = <0x2e04c 0x40>;
interrupt-parent = <&irq0>;
interrupts = <55 IRQ_TYPE_LEVEL_HIGH>;
msi-controller;
num-vectors = <32>;
};

pcie@30000000 {
compatible = "sigma,smp8759-pcie";
reg = <0x30000000 SZ_4M>, <0x2e02c 4>;
device_type = "pci";
bus-range = <0 3>;
#size-cells = <2>;
#address-cells = <3>;
#interrupt-cells = <1>;
ranges = <0x02000000 0x0 0x00400000 0x30400000 0x0 SZ_60M>;
msi-parent = <&msi0>;
interrupt-parent = <&irq0>;
interrupts = <54 IRQ_TYPE_LEVEL_HIGH>;
};


I traced the action into pdev_fixup_irq()
which calls of_irq_parse_and_map_pci()

How do I tell Linux that
- All the legacy PCI interrupts are muxed to a single line
- And this line is routed to system interrupt 54

Ooooooh... Wait...

Is this what interrupt-map is used for?

http://elinux.org/Device_Tree_Usage#Advanced_Interrupt_Mapping

Regards.


2017-03-16 17:33:43

by Mason

[permalink] [raw]
Subject: Re: Legacy PCI interrupt support in PCIe host driver

On 16/03/2017 16:40, Mason wrote:

> Here is my current DT:
>
> msi0: msi@2e080 {
> compatible = "sigma,msi";
> reg = <0x2e04c 0x40>;
> interrupt-parent = <&irq0>;
> interrupts = <55 IRQ_TYPE_LEVEL_HIGH>;
> msi-controller;
> num-vectors = <32>;
> };
>
> pcie@30000000 {
> compatible = "sigma,smp8759-pcie";
> reg = <0x30000000 SZ_4M>, <0x2e02c 4>;
> device_type = "pci";
> bus-range = <0 3>;
> #size-cells = <2>;
> #address-cells = <3>;
> #interrupt-cells = <1>;
> ranges = <0x02000000 0x0 0x00400000 0x30400000 0x0 SZ_60M>;
> msi-parent = <&msi0>;
> interrupt-parent = <&irq0>;
> interrupts = <54 IRQ_TYPE_LEVEL_HIGH>;
> };
>
>
> I traced the action into pdev_fixup_irq()
> which calls of_irq_parse_and_map_pci()
>
> How do I tell Linux that
> - All the legacy PCI interrupts are muxed to a single line
> - And this line is routed to system interrupt 54
>
> Ooooooh... Wait...
>
> Is this what interrupt-map is used for?
>
> http://elinux.org/Device_Tree_Usage#Advanced_Interrupt_Mapping

I added this to my pcie@30000000 node:

interrupt-map = <0 0 0 1 &irq0 54 IRQ_TYPE_LEVEL_HIGH>;

to map INTA to system IRQ 54.

And now it works much better :-)

# cat /proc/interrupts
CPU0
19: 1529 GIC-0 29 Edge twd
20: 125 irq0 1 Level serial
22: 0 irq0 54 Level tutu, xhci-hcd:usb1

About shared ISRs. Are they supposed to return IRQ_HANDLED
if and only if they handled something?

Will that stop the next ISR from being called?

I guess if two interrupts fire at the same time, we'll
just take two separate exceptions?

Regards.

2017-03-16 17:48:32

by Thomas Gleixner

[permalink] [raw]
Subject: Re: Legacy PCI interrupt support in PCIe host driver

On Thu, 16 Mar 2017, Mason wrote:
> About shared ISRs. Are they supposed to return IRQ_HANDLED
> if and only if they handled something?

Every interrupt handler is supposed to return IRQ_NONE if it did not handle
it. That does not depend on shared or not. The handler does not know if
it's on a shared interrupt line or not. IRQF_SHARED only tells, that the
handler is capable of sharing the interrupt line with another device.

> Will that stop the next ISR from being called?

No.

> I guess if two interrupts fire at the same time, we'll just take two
> separate exceptions?

Wrong guess. That might work with level interupts, but with other types
nothing will raise another exception. Sharing interrupts on edge types is a
stupid idea, but hardware folks insist on implementing stupid ideas.

Thanks,

tglx

2017-03-16 19:14:30

by Mason

[permalink] [raw]
Subject: Re: Legacy PCI interrupt support in PCIe host driver

On 16/03/2017 18:47, Thomas Gleixner wrote:
> On Thu, 16 Mar 2017, Mason wrote:
>> I guess if two interrupts fire at the same time, we'll just take two
>> separate exceptions?
>
> Wrong guess. That might work with level interrupts, but with other types
> nothing will raise another exception. Sharing interrupts on edge types is a
> stupid idea, but hardware folks insist on implementing stupid ideas.

When you say "That might work with level interrupts",
what is "that" ?

In my case,

interrupt-map = <0 0 0 1 &irq0 54 IRQ_TYPE_LEVEL_HIGH>;
interrupts = <54 IRQ_TYPE_LEVEL_HIGH>;

Both ISRs expect LEVEL_HIGH. In fact, doesn't request_irq
return an error if the triggers are different?

Regards.

2017-03-16 19:36:45

by Thomas Gleixner

[permalink] [raw]
Subject: Re: Legacy PCI interrupt support in PCIe host driver

On Thu, 16 Mar 2017, Mason wrote:

> On 16/03/2017 18:47, Thomas Gleixner wrote:
> > On Thu, 16 Mar 2017, Mason wrote:
> >> I guess if two interrupts fire at the same time, we'll just take two
> >> separate exceptions?
> >
> > Wrong guess. That might work with level interrupts, but with other types
> > nothing will raise another exception. Sharing interrupts on edge types is a
> > stupid idea, but hardware folks insist on implementing stupid ideas.
>
> When you say "That might work with level interrupts", what is "that" ?

That you take two exceptions because if both have raised the irq it will
stay raised when you only handle one. Non level types will not keep it
raised in the CPU and you lost.

> In my case,
>
> interrupt-map = <0 0 0 1 &irq0 54 IRQ_TYPE_LEVEL_HIGH>;
> interrupts = <54 IRQ_TYPE_LEVEL_HIGH>;
>
> Both ISRs expect LEVEL_HIGH. In fact, doesn't request_irq
> return an error if the triggers are different?

That's completely irrelevant.

Thanks,

tglx