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: > >> On Wed, May 10, 2017 at 4:07 AM, Ryder Lee <ryder.lee@xxxxxxxxxxxx> wrote: > >> > >> > +- ranges: > >> > + - The first three entries are expected to translate the addresses for the root > >> > + port registers, which are referenced by the assigned-addresses property of > >> > + the root port nodes (see below). > >> > >> I don't understand this part. Why do you need a static translation for these? > >> Shouldn't they just be listed in the 'reg' property of the parent node now that > >> you have the clk/reset/phy properties in the parent as well? > > > > At first, I did like that. But I noticed that someone suggest it's > > better to use 'assigned-addresses' to handle per-port registers, the > > same path as tegra and marvell did, in other platform discussion thread. > > So I just put shared register in root node. It could be rolled back if > > you feel this is inappropriate. > > The marvell case is not a good example for your case: their top-level > device is made up by the OS to help with the shared resource allocation, > while in your case the bus bridge actually exists in hardware. > > I'm not too familiar with the Tegra case, and haven't looked at that here, > but it could be an artifact of how for a while we used to list the config > space access in the top-level "ranges" instead of the "reg" property. > > I'd vote for moving it back, for consistency with the other port specific > properties that are now in the root node. Once you do that, the port > nodes can be removed completely, which is what I was aiming for with > the comments on the previous version. I'll move it back. > >> > +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. > 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. -- 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
