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: > > On Thu, May 11, 2017 at 4:44 AM, Ryder Lee <ryder.lee@xxxxxxxxxxxx> wrote: > > > On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote: > > >> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee <ryder.lee@xxxxxxxxxxxx> wrote: > > >> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote: > > > > >> >> > +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 > > >> On a related note, I see that you still list > > >> > > >> > +- 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. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
