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