Hi Arnd,

I want to further explain what I have discussed in previous mail.


On Thu, 2017-05-11 at 17:08 +0800, Ryder Lee wrote:

On Thu, 2017-05-11 at 09:17 +0200, Arnd Bergmann wrote:

quoted

On Thu, May 11, 2017 at 4:44 AM, Ryder Lee [off-list ref] wrote:

quoted

On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:

quoted

On Wed, May 10, 2017 at 11:31 AM, Ryder Lee [off-list ref] wrote:

quoted

On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:

quoted

quoted

quoted

quoted

quoted

+Required properties:
+- device_type: Must be "pci"
+- assigned-addresses: Address and size of the port configuration registers
+- reg: Only the first four bytes are used to refer to the correct bus number
+  and device number.
+- #address-cells: Must be 3
+- #size-cells: Must be 2
+- #interrupt-cells: Must be 1
+- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.

Child nodes do not normally have interrupt-map properties. Isn't this
already covered by the interrupt-map in the parent?

I have one Intel 4 port ethernet card(0000:00:01) and MTK WLAN card
(0000:00:02), probe message looks good to me.

pci 0000:00:01.0: fixup irq: got 224
pci 0000:00:01.0: assigning IRQ 224
pci 0000:00:02.0: fixup irq: got 225
pci 0000:00:02.0: assigning IRQ 225

pci 0000:01:00.0: fixup irq: got 224
pci 0000:01:00.0: assigning IRQ 224
pci 0000:01:00.1: fixup irq: got 224
pci 0000:01:00.1: assigning IRQ 224
pci 0000:01:00.2: fixup irq: got 224
pci 0000:01:00.2: assigning IRQ 224
pci 0000:01:00.3: fixup irq: got 224
pci 0000:01:00.3: assigning IRQ 224

pci 0000:02:00.0: fixup irq: got 225
pci 0000:02:00.0: assigning IRQ 225


But child nodes without interrupt-map properties:
It seems incorrect.

pci 0000:00:01.0: fixup irq: got 224
pci 0000:00:01.0: assigning IRQ 224
pci 0000:00:02.0: fixup irq: got 225
pci 0000:00:02.0: assigning IRQ 225

pci 0000:01:00.0: fixup irq: got 223
pci 0000:01:00.0: assigning IRQ 223

Not entirely sure what happens here, but I guess the problem
is that the 'reg' portion of the parent interrupt-map refers to
the port devices, not the devices attached the devices behind
them.

I agree with you. That's why I need additional interrupt-map properties
to resolve IRQ correctly for the devices behind root ports.

Not sure whether other platforms have similar case like me here.

I think it's just a bug in this specific chip where the HW designers
wired the IRQs in a nonstandard way.

However, you really should not need the interrupt-map properties
in the child nodes, just change the address part in the parent
interrupt-map. Specifically, the 'bus' portion of the device address
in the interrupt-map would have to be nonzero to refer to
child devices.

This is what I modify for the parent node and remove interrupt-map
properties from child..

interrupt-map-mask = <0xff800 0 0 0>;
interrupt-map = <0x0000 0 0 0 &gic GIC_SPI 193 IRQ_TYPE_NONE>,
		 <0x0800 0 0 0 &gic GIC_SPI 194 IRQ_TYPE_NONE>,
		 <0x1000 0 0 0 &gic GIC_SPI 195 IRQ_TYPE_NONE>,

		 /* workaround here*/
		 <0x10000 0 0 0 &gic GIC_SPI 193 IRQ_TYPE_NONE>,
		 <0x20000 0 0 0 &gic GIC_SPI 194 IRQ_TYPE_NONE>,
	     <0x30000 0 0 0 &gic GIC_SPI 195 IRQ_TYPE_NONE>;

It works well. But how could we handle the situation if root port0
status = "disabled" ? I think we cannot assign child bus number
dynamically from binding.

That is to say, we route it statically if port0 (or port1) is
unavailable. The PCI child bus enumeration should look something like
this:

 pci 0000:00:01.0: fixup irq: got 224
 pci 0000:00:01.0: assigning IRQ 224
 pci 0000:00:02.0: fixup irq: got 225
 pci 0000:00:02.0: assigning IRQ 225
 
 Go wrong here! IRQ 223/224 should be assigned to the devices behind
port0 and port1.
 pci 0000:01:00.0: fixup irq: got 223
 pci 0000:01:00.0: assigning IRQ 223
 pci 0000:02:00.0: fixup irq: got 224
 pci 0000:02:00.0: assigning IRQ 224

quoted

quoted

quoted

On a related note, I see that you still list

quoted

+- interrupts: Three interrupt outputs of the controller. Must contain an
+  entry for each entry in the interrupt-names property.
+- interrupt-names: Must include the following names
+  - "pcie-int0"
+  - "pcie-int1"
+  - "pcie-int2"

This seems to be an artifact from the older version and should be
removed as the driver correctly ignores the properties now.

Actually, everything works fine without these properties however when it
loads we see a few weird error message:

pcieport 0000:00:01.0: Signaling PME with IRQ 232
pcieport 0000:00:02.0: enabling device (0140 -> 0142)
pcieport 0000:00:02.0: enabling bus mastering
irq 232: nobody cared (try booting with the "irqpoll" option)
...
[<c03f6be4>] (pcie_pme_probe) from [<c03f47b8>] (pcie_port_probe_service
+0x44/0x6c)
(pcie_port_probe_service) from [<c0454cf8>] (driver_probe_device
+0x280/0x470)
...
(pcie_port_device_register) from [<c03f51a0>] (pcie_portdrv_probe
+0x3c/0xb4)
(pcie_portdrv_probe) from [<c03e7acc>] (pci_device_probe+0x98/0xfc)
(pci_device_probe) from [<c0454cf8>] (driver_probe_device+0x280/0x470)
handlers:
[<c03f68b0>] pcie_pme_irq
Disabling IRQ #233

I haven't dig it out yet, but just keep them here to solve that.

Something is going very wrong if adding the properties helps. I can't
think of what that is, but we have to find out before the binding can
be merged.

Not really understand PME service. But I will find the reason here.

I have do some test here. PME needs port IRQs, which interrupt type was
set correctly(IRQ_TYPE_LEVEL_LOW). But we cannot set it from
interrupt-map, according to gic_set_type() /* SPIs have restrictions on
the supported types */ .

So we need to add additional interrupt properties.