Legacy PCI interrupt support in PCIe host driver

Legacy PCI interrupt support in PCIe host driver

[Mason]

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

Legacy PCI interrupt support in PCIe host driver

[Mason]

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

> Here is my current DT:
> 
> 		msi0: msi at 2e080 {
> 			compatible = "sigma,msi";
> 			reg = <0x2e04c 0x40>;
> 			interrupt-parent = <&irq0>;
> 			interrupts = <55 IRQ_TYPE_LEVEL_HIGH>;
> 			msi-controller;
> 			num-vectors = <32>;
> 		};
> 
> 		pcie at 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 at 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.

Legacy PCI interrupt support in PCIe host driver

[Thomas Gleixner]

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

Legacy PCI interrupt support in PCIe host driver

[Mason]

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.

Legacy PCI interrupt support in PCIe host driver

[Thomas Gleixner]

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