| Message ID | 1393550394-11071-4-git-send-email-tharvey@gateworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday, February 28, 2014 10:20 AM, Tim Harvey wrote: > > The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards > has its slot-to-bridge IRQ mapping reversed from the PCI specification: > > INTA->INTD > INTB->INTC > INTC->INTB > INTD->INTA (+cc Marek Vasut, Pratyush Anand, Kishon Vijay Abraham I, Mohit KUMAR DCG) As I mention earlier, 'i.MX6 PCIe IP' is not a culprit. The 'TI XIO2001 PCIe-to-PCI bridge' is the culprit, right? Then, './drivers/pci/host/pci-imx6.c' is not a good place to fix the board problems. Look at other i.MX6 boards. They don't have the 'TI XIO2001 PCIe-to-PCI bridge'. Please don't add this board specific code to SoC specific driver code. Best regards, Jingoo Han > > Implement a custom swizzle function that does a fixup on the interrupt for > devices on the first TI XIO2001 bridge in the tree. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Zhu <r65037@freescale.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Sean Cross <xobs@kosagi.com> > Cc: Jingoo Han <jg1.han@samsung.com> > --- > drivers/pci/host/pci-imx6.c | 36 ++++++++++++++++++++++++++++++++++++ > include/linux/pci_ids.h | 1 + > 2 files changed, 37 insertions(+) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index aaa05c8..b171a21 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -493,6 +493,39 @@ static int imx6_add_pcie_port(struct pcie_port *pp, > return 0; > } > > +/* TI XIO2001 PCIe-to-PCI bridge on GW16082 exp card has IRQs reversed */ > +u8 ventana_swizzle(struct pci_dev *dev, u8 *pin) > +{ > + u8 i = 0; > + struct pci_dev *pdev = dev; > + > + /* count number of TI XIO2001 bridges on bus */ > + while (!pci_is_root_bus(pdev->bus)) { > + if (pdev->bus && pdev->bus->self && > + (pdev->bus->self->vendor == PCI_VENDOR_ID_TI) && > + (pdev->bus->self->device == PCI_DEVICE_ID_TI_XIO2001)) { > + i++; > + } > + pdev = pdev->bus->self; > + } > + while (!pci_is_root_bus(dev->bus)) { > + /* if we are directly downstream from 1st TI XIO2001 bridge */ > + if (dev->bus && dev->bus->self && > + (dev->bus->self->vendor == PCI_VENDOR_ID_TI) && > + (dev->bus->self->device == PCI_DEVICE_ID_TI_XIO2001)) { > + if (--i == 0) { > + /* swap IRQs and swizzle backwards */ > + *pin = (15 - PCI_SLOT(dev->devfn)) + 1; > + dev = dev->bus->self; > + continue; > + } > + } > + *pin = pci_swizzle_interrupt_pin(dev, *pin); > + dev = dev->bus->self; > + } > + return PCI_SLOT(dev->devfn); > +} > + > static int __init imx6_pcie_probe(struct platform_device *pdev) > { > struct imx6_pcie *imx6_pcie; > @@ -601,6 +634,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev) > return PTR_ERR(imx6_pcie->iomuxc_gpr); > } > > + if (of_machine_is_compatible("gw,ventana")) > + pp->swizzle = ventana_swizzle; > + > ret = imx6_add_pcie_port(pp, pdev); > if (ret < 0) > return ret; > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h > index 97fbecd..4ca334f 100644 > --- a/include/linux/pci_ids.h > +++ b/include/linux/pci_ids.h > @@ -822,6 +822,7 @@ > #define PCI_DEVICE_ID_TI_XX12 0x8039 > #define PCI_DEVICE_ID_TI_XX12_FM 0x803b > #define PCI_DEVICE_ID_TI_XIO2000A 0x8231 > +#define PCI_DEVICE_ID_TI_XIO2001 0x8240 > #define PCI_DEVICE_ID_TI_1130 0xac12 > #define PCI_DEVICE_ID_TI_1031 0xac13 > #define PCI_DEVICE_ID_TI_1131 0xac15 > -- > 1.8.3.2
On Thursday 27 February 2014 17:19:54 Tim Harvey wrote: > The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards > has its slot-to-bridge IRQ mapping reversed from the PCI specification: > > INTA->INTD > INTB->INTC > INTC->INTB > INTD->INTA > > Implement a custom swizzle function that does a fixup on the interrupt for > devices on the first TI XIO2001 bridge in the tree. > I'm pretty sure you can express that by using a more specific 'interrupt-map' property that defines the correct mapping for the PCIe-to-PCI bridge in the board file. Arnd
On Fri, Feb 28, 2014 at 10:27:13AM +0100, Arnd Bergmann wrote: > On Thursday 27 February 2014 17:19:54 Tim Harvey wrote: > > The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards > > has its slot-to-bridge IRQ mapping reversed from the PCI specification: > > > > INTA->INTD > > INTB->INTC > > INTC->INTB > > INTD->INTA > > > > Implement a custom swizzle function that does a fixup on the interrupt for > > devices on the first TI XIO2001 bridge in the tree. > > > > I'm pretty sure you can express that by using a more specific > 'interrupt-map' property that defines the correct mapping for > the PCIe-to-PCI bridge in the board file. Yes, the correct way to handle this is to define a stanza for the PCIe-PCI bridge and then place an interrupt map in that stanza that describes the downstream translation. Nobody should be messing with IRQ numbers in code on DT platforms. DTs using the mvebu driver have an example of this: pcie-controller { compatible = "marvell,kirkwood-pcie"; status = "disabled"; device_type = "pci"; pcie@1,0 { device_type = "pci"; reg = <0x0800 0 0 0 0>; ^^^^^^^ This is the PCI device ID of the bridge interrupt-map-mask = <0 0 0 0>; interrupt-map = <0 0 0 0 &intc 9>; Which says 'INTA/B/C/D of devices downstream of bridge 00:01.0 map to &intc input 9' In particular, this is probably not a TI XIO2001 problem, but a board design problem - the swizzle rules were not properly followed when wiring up the PCI ISDEL and INTx pins on the downstream PCI bus. Jason
On Fri, Feb 28, 2014 at 9:39 AM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Fri, Feb 28, 2014 at 10:27:13AM +0100, Arnd Bergmann wrote: >> On Thursday 27 February 2014 17:19:54 Tim Harvey wrote: >> > The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards >> > has its slot-to-bridge IRQ mapping reversed from the PCI specification: >> > >> > INTA->INTD >> > INTB->INTC >> > INTC->INTB >> > INTD->INTA >> > >> > Implement a custom swizzle function that does a fixup on the interrupt for >> > devices on the first TI XIO2001 bridge in the tree. >> > >> >> I'm pretty sure you can express that by using a more specific >> 'interrupt-map' property that defines the correct mapping for >> the PCIe-to-PCI bridge in the board file. > > Yes, the correct way to handle this is to define a stanza for the > PCIe-PCI bridge and then place an interrupt map in that stanza that > describes the downstream translation. > > Nobody should be messing with IRQ numbers in code on DT platforms. > > DTs using the mvebu driver have an example of this: > > pcie-controller { > compatible = "marvell,kirkwood-pcie"; > status = "disabled"; > device_type = "pci"; > > pcie@1,0 { > device_type = "pci"; > reg = <0x0800 0 0 0 0>; > ^^^^^^^ This is the PCI device > ID of the bridge > interrupt-map-mask = <0 0 0 0>; > interrupt-map = <0 0 0 0 &intc 9>; > > Which says 'INTA/B/C/D of devices downstream of bridge 00:01.0 map to > &intc input 9' > Jason, In our case, the IMX6 is on a basebaord and we have optional expansion boards. One such board has a PEX 8609 9-port PCIe switch, the other has a TI XIO2001 PCIe-to-PCI bridge. So, depending on how you connect these expansion boards the bus numbers change. For example, one use case of the baseboard+XIO2001-expansion is: IMX6 -> XIO2001 PCIe-to-PCI bridge: The following shows one endpoint off the bridge (bus2 slot12): $ lspci -n 00:00.0 0604: 16c3:abcd (rev 01) 01:00.0 0604: 104c:8240 02:0c.0 0280: 168c:0027 (rev 01) $ lspci -tnv -[0000:00]---00.0-[01-02]----00.0-[02]----0c.0 168c:0027 Another use case of the same baseboard + PEX8609-expansion + XIO2001-expansion is: $ lspci -n 00:00.0 0604: 16c3:abcd (rev 01) 01:00.0 0604: 10b5:8609 (rev ba) 01:00.1 0880: 10b5:8609 (rev ba) 02:01.0 0604: 10b5:8609 (rev ba) 02:04.0 0604: 10b5:8609 (rev ba) 02:05.0 0604: 10b5:8609 (rev ba) 02:06.0 0604: 10b5:8609 (rev ba) 02:07.0 0604: 10b5:8609 (rev ba) 02:08.0 0604: 10b5:8609 (rev ba) 02:09.0 0604: 10b5:8609 (rev ba) 04:00.0 0604: 104c:8240 05:0c.0 0280: 168c:0027 (rev 01) $ lspci -tnv -[0000:00]---00.0-[01-0a]--+-00.0-[02-0a]--+-01.0-[03]-- | +-04.0-[04-05]----00.0-[05]----0c.0 168c:0027 | +-05.0-[06]-- | +-06.0-[07]-- | +-07.0-[08]-- | +-08.0-[09]-- | \-09.0-[0a]-- \-00.1 10b5:8609 > In particular, this is probably not a TI XIO2001 problem, but a board > design problem - the swizzle rules were not properly followed when > wiring up the PCI ISDEL and INTx pins on the downstream PCI bus. Correct - its not a TI XIO2001 problem, its that the interrupts from the slots to the XIO2001 don't follow the interrupt mapping called out in the spec correctly (board problem on the 'add-in' board, not the 'baseboard'). Regardless of the issue of not knowing the bus topology before-hand, I'm still trying to understand how to describe the bridge in devicetree using your example above. If I were able to define a DT node for the XIO2001 bridge and apply an interrupt-map there, how does that map get encorporated into the swizzle in the case the the bridge is in the middle of the chain of devices? In looking at the of_irq_parse_map_pci() function that will likely be called from map_irq() it would end up calling of_irq_parse_raw to map the irq and I'm not understanding how that will take into account the fact that a bridge possibly in the middle of the bus-tree had invalid mapping. Thanks, Tim > > Jason
On Fri, Feb 28, 2014 at 04:52:05PM -0800, Tim Harvey wrote: > > In particular, this is probably not a TI XIO2001 problem, but a board > > design problem - the swizzle rules were not properly followed when > > wiring up the PCI ISDEL and INTx pins on the downstream PCI bus. > > Correct - its not a TI XIO2001 problem, its that the interrupts from > the slots to the XIO2001 don't follow the interrupt mapping called out > in the spec correctly (board problem on the 'add-in' board, not the > 'baseboard'). So broken hardware requires explicit DT representation, the auto-probing mechanism only work for compliant hardware. > Regardless of the issue of not knowing the bus topology before-hand, To solve this problem, you reall need to know the bus topology before hand, so any systems that include a borken TI XIO2001 board need a special DT. Including stuff like this in code means you hobble every one who might use the TI chip correctly. It is certainly inappropriate to include a host driver, or generic PCI fixup. > I'm still trying to understand how to describe the bridge in > devicetree using your example above. If I were able to define a DT > node for the XIO2001 bridge and apply an interrupt-map there, how does > that map get encorporated into the swizzle in the case the the bridge > is in the middle of the chain of devices? You keep nesting the PCI-PCI bridges until you get to the bottom, basically following along the lspci -t output, cast into DT notation: Your first example: pcie-controller { compatible = "marvell,kirkwood-pcie"; status = "disabled"; device_type = "pci"; pcie@0,0 { device_type = "pci"; // Presumably this is the root port bridge? // 00:00.0 0604: 16c3:abcd reg = <PCI_ID(0,0,0) 0 0 0 0>; pcie@0,0 { // This is the broken TI board: // 01:00.0 0604: 104c:8240 reg = <PCI_ID(1,0,0) 0 0 0 0>; Second example: pcie-controller { compatible = "marvell,kirkwood-pcie"; status = "disabled"; device_type = "pci"; pcie@0,0 { device_type = "pci"; // Presumably this is the root port bridge? // 00:00.0 0604: 16c3:abcd reg = <PCI_ID(0,0,0) 0 0 0 0>; pcie@0,0 { // This is the PEX switch bridge to internal // 01:00.0 0604: 10b5:8609 (rev ba) reg = <PCI_ID(1,0,0) 0 0 0 0>; pcie@4,0 { // This is the PEX port bridge to // To the TI part // 02:04.0 0604: 10b5:8609 (rev ba) reg = <PCI_ID(2,4,0) 0 0 0 0>; pcie@0,0 { // This is the broken TI board: // 04:00.0 0604: 104c:8240 reg = <PCI_ID(4,0,0) 0 0 0 0>; Also, bear in mind that every single explicitly declared stanza requires a correct interrupt-map. > of_irq_parse_map_pci() function that will likely be called from > map_irq() it would end up calling of_irq_parse_raw to map the irq and > I'm not understanding how that will take into account the fact that a > bridge possibly in the middle of the bus-tree had invalid mapping. First the PCI core matches the DT nodes to the discovered topology. Then the interrupt mapper starts from a probed PCI node and traces up the tree to the root. Each time it goes up it swizzles. When it finds a node with a DT mapping it immediately switches to DT to resolve the interrupt, which uses the first interrupt map found by traversing up from the match'd DT node. Once it switches to DT mode there is no swizzling, the DT must exactly describe the interconnection. Jason
On Fri, Feb 28, 2014 at 5:22 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Fri, Feb 28, 2014 at 04:52:05PM -0800, Tim Harvey wrote: > >> > In particular, this is probably not a TI XIO2001 problem, but a board >> > design problem - the swizzle rules were not properly followed when >> > wiring up the PCI ISDEL and INTx pins on the downstream PCI bus. >> >> Correct - its not a TI XIO2001 problem, its that the interrupts from >> the slots to the XIO2001 don't follow the interrupt mapping called out >> in the spec correctly (board problem on the 'add-in' board, not the >> 'baseboard'). > > So broken hardware requires explicit DT representation, the > auto-probing mechanism only work for compliant hardware. ok - makes sense > >> Regardless of the issue of not knowing the bus topology before-hand, > > To solve this problem, you reall need to know the bus topology before > hand, so any systems that include a borken TI XIO2001 board need a > special DT. I suppose I could go about this from the bootloader as well. The bootloader could enumerate the bus and build the DT with the nodes required to handle the broken TI XIO2001 add-in cards. > > Including stuff like this in code means you hobble every one who might > use the TI chip correctly. It is certainly inappropriate to include a > host driver, or generic PCI fixup. Understood - trust me... I'm not happy about this situation. A pci fixup for the XIO2001 could at least be placed in arch/arm/mach-imx/mach-imx6q.c with a check for of_machine_is_compatible("gw, ventana"). This is currently done in order to handle GPIO on the PEX890x bridge which is used as PERST# for downstream slots (see http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-imx/mach-imx6q.c#n87). This of course, assumes that I can come up with a way for a pci fixup to replace the swizzle function of the host controller before enumeration. > >> I'm still trying to understand how to describe the bridge in >> devicetree using your example above. If I were able to define a DT >> node for the XIO2001 bridge and apply an interrupt-map there, how does >> that map get encorporated into the swizzle in the case the the bridge >> is in the middle of the chain of devices? > > You keep nesting the PCI-PCI bridges until you get to the bottom, > basically following along the lspci -t output, cast into DT notation: > > Your first example: > pcie-controller { > compatible = "marvell,kirkwood-pcie"; > status = "disabled"; > device_type = "pci"; > > pcie@0,0 { > device_type = "pci"; > // Presumably this is the root port bridge? > // 00:00.0 0604: 16c3:abcd > reg = <PCI_ID(0,0,0) 0 0 0 0>; I'm still not understanding what needs to go in the 'reg' property for a DT node representing a PCI dev (what you are using PCI_ID() for). I've looked at the PCI Bus Binding to Open Firmware doc and still don't understand (its very terse, and lacks examples). By looking through the drivers/of code it seemed that this should be devfn but after some digging through examples in Documentation/devicetree/bindings/pci it seems that it really needs to be devfn<<8. I'm not clear why the <<8 is needed. I also determined that for fsl.imx6q-pcie I needed to define #address-cells = <3> and #size-cells = <2> as the enw pcie dev nodes kept defaulting to something else - I didn't realize those were not inherited from the parent DT node. In all, your examples here were extremely helpful to me. I now know how to properly describe my GigE PCI dev node so that the driver can have a hope of getting its MAC from DT. Thank you! > pcie@0,0 { > // This is the broken TI board: > // 01:00.0 0604: 104c:8240 > reg = <PCI_ID(1,0,0) 0 0 0 0>; > <snip> > > Also, bear in mind that every single explicitly declared stanza requires > a correct interrupt-map. if not specified it wouldn't default to standard bridge mapping such that I only need to provide interrupt-map for the node with the the TI bridge? > >> of_irq_parse_map_pci() function that will likely be called from >> map_irq() it would end up calling of_irq_parse_raw to map the irq and >> I'm not understanding how that will take into account the fact that a >> bridge possibly in the middle of the bus-tree had invalid mapping. > > First the PCI core matches the DT nodes to the discovered topology. > > Then the interrupt mapper starts from a probed PCI node and traces up > the tree to the root. Each time it goes up it swizzles. > > When it finds a node with a DT mapping it immediately switches to DT > to resolve the interrupt, which uses the first interrupt map found by > traversing up from the match'd DT node. Once it switches to DT mode > there is no swizzling, the DT must exactly describe the > interconnection. Ok - so this assumes that the host controller driver _is_ calling something like of_irq_parse_and_map_pci() instead of returning based on some static mapping (like pp->irq) right? What confuses me is that swizzling is done to the pin before the call to map_irq(dev, slot, pin). It looks like that this common swizzling is ignored when using of_irq_parse_and_map_pci() which instead would perform what you describe above correct? Thanks for all the help here! Tim > > Jason
On Mon, Mar 03, 2014 at 11:59:51AM -0800, Tim Harvey wrote: > A pci fixup for the XIO2001 could at least be placed in > arch/arm/mach-imx/mach-imx6q.c with a check for > of_machine_is_compatible("gw, ventana"). This is currently done in > order to handle GPIO on the PEX890x bridge which is used as PERST# for > downstream slots (see > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-imx/mach-imx6q.c#n87). That is an odd looking thing .. PERST# of a subordinate port should be tied to the PCI_BRIDGE_CTL_BUS_RESET bit in the port's bridge configuration space - is more configuration of the PLX chip required? Also, be aware that PCI specifies a minimum time after reset-deassertion before accessing the bus, and I don't see a sleep... > I'm still not understanding what needs to go in the 'reg' property for > a DT node representing a PCI dev (what you are using PCI_ID() for). It is a 3 DW address in this format: phys.hi cell: npt000ss bbbbbbbb dddddfff rrrrrrrr phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh phys.low cell: llllllll llllllll llllllll llllllll And in this context you use 'ss' = 0 for configuration: * npt000ss bbbbbbbb dddddfff rrrrrrrr = 00000000 00000000 dddddfff 00000000 So devfn << 8 sounds right to me. I *think* you can use 0 for the bus in this context. > #address-cells = <3> and #size-cells = <2> as the enw pcie dev nodes > kept defaulting to something else - I didn't realize those were not > inherited from the parent DT node. Right, sorry, I was making it terse.. > In all, your examples here were extremely helpful to me. I now know > how to properly describe my GigE PCI dev node so that the driver can > have a hope of getting its MAC from DT. Thank you! Right! > > Also, bear in mind that every single explicitly declared stanza requires > > a correct interrupt-map. > > if not specified it wouldn't default to standard bridge mapping such > that I only need to provide interrupt-map for the node with the the TI > bridge? Nope, there are two distinct mechanisms - finding the closes DT node to the actual PCI device (of_irq_parse_pci) - this swizzles as it searches. The rational here is to support hot pluggable devices that were not discovered during initialization time. The second is the completely generic interrupt-map mechanism (of_irq_parse_raw), which takes the interrupt specification from the first phase and transates it into an actual interrupt. This isn't PCI aware and doesn't change the description in any way. So if you specify a node, you have to specify an interrupt-map that covers that node. I recommend placing the map in the node itself so the bus number is not required. > Ok - so this assumes that the host controller driver _is_ calling > something like of_irq_parse_and_map_pci() instead of returning based > on some static mapping (like pp->irq) right? Yes, absoultely. All DT based PCI drivers need to use this mechanism to get the expected interrupt-map behavior. > What confuses me is that swizzling is done to the pin before the call > to map_irq(dev, slot, pin). It looks like that this common swizzling > is ignored when using of_irq_parse_and_map_pci() which instead would > perform what you describe above correct? Right, the pin # from the PCI core isn't used in the OF code: * of_irq_parse_and_map_pci() - Decode a PCI irq from the device tree and map to a virq * @slot and @pin are unused, but included in the function so that this * function can be used directly as the map_irq callback to pci_fixup_irqs(). It re-reads the device's pin number directly and feeds that through the standard swizzle and OF's UIS system. Jason
On Mon, Mar 3, 2014 at 3:37 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Mon, Mar 03, 2014 at 11:59:51AM -0800, Tim Harvey wrote: > >> A pci fixup for the XIO2001 could at least be placed in >> arch/arm/mach-imx/mach-imx6q.c with a check for >> of_machine_is_compatible("gw, ventana"). This is currently done in >> order to handle GPIO on the PEX890x bridge which is used as PERST# for >> downstream slots (see >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-imx/mach-imx6q.c#n87). > > That is an odd looking thing .. PERST# of a subordinate port should be > tied to the PCI_BRIDGE_CTL_BUS_RESET bit in the port's bridge > configuration space - is more configuration of the PLX chip required? PCI_BRIDGE_CTL_BUS_RESET does cause a hot reset on the corresponding downstream link however the minPCIe card slot has a PERST# pin that is for a functional reset to the card. The GPIO's I mention on the PLX bridge are routed to that signal for the various slots. Regardless of what a device may do when it gets a hot-reset some devices still may require a hard reset. Think of MiniPCIe form-factor USB cellular modems for example which don't even use the PCIe bus. > > Also, be aware that PCI specifies a minimum time after > reset-deassertion before accessing the bus, and I don't see a sleep... there is an msleep(100) there after the de-assertion. > >> I'm still not understanding what needs to go in the 'reg' property for >> a DT node representing a PCI dev (what you are using PCI_ID() for). > > It is a 3 DW address in this format: > > phys.hi cell: npt000ss bbbbbbbb dddddfff rrrrrrrr > phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh > phys.low cell: llllllll llllllll llllllll llllllll > > And in this context you use 'ss' = 0 for configuration: > > * npt000ss bbbbbbbb dddddfff rrrrrrrr > = 00000000 00000000 dddddfff 00000000 > > So devfn << 8 sounds right to me. > > I *think* you can use 0 for the bus in this context. yes, it would appear bus can be 0 from my findings. Thanks for clarifying that verbiage! > >> #address-cells = <3> and #size-cells = <2> as the enw pcie dev nodes >> kept defaulting to something else - I didn't realize those were not >> inherited from the parent DT node. > > Right, sorry, I was making it terse.. > >> In all, your examples here were extremely helpful to me. I now know >> how to properly describe my GigE PCI dev node so that the driver can >> have a hope of getting its MAC from DT. Thank you! > > Right! > >> > Also, bear in mind that every single explicitly declared stanza requires >> > a correct interrupt-map. >> >> if not specified it wouldn't default to standard bridge mapping such >> that I only need to provide interrupt-map for the node with the the TI >> bridge? > > Nope, there are two distinct mechanisms - finding the closes DT node > to the actual PCI device (of_irq_parse_pci) - this swizzles as it > searches. The rational here is to support hot pluggable devices that > were not discovered during initialization time. > > The second is the completely generic interrupt-map mechanism > (of_irq_parse_raw), which takes the interrupt specification from the > first phase and transates it into an actual interrupt. This isn't PCI > aware and doesn't change the description in any way. > > So if you specify a node, you have to specify an interrupt-map that > covers that node. I recommend placing the map in the node itself so > the bus number is not required. > >> Ok - so this assumes that the host controller driver _is_ calling >> something like of_irq_parse_and_map_pci() instead of returning based >> on some static mapping (like pp->irq) right? > > Yes, absoultely. All DT based PCI drivers need to use this mechanism > to get the expected interrupt-map behavior. > >> What confuses me is that swizzling is done to the pin before the call >> to map_irq(dev, slot, pin). It looks like that this common swizzling >> is ignored when using of_irq_parse_and_map_pci() which instead would >> perform what you describe above correct? > > Right, the pin # from the PCI core isn't used in the OF code: > > * of_irq_parse_and_map_pci() - Decode a PCI irq from the device tree and map to a virq > * @slot and @pin are unused, but included in the function so that this > * function can be used directly as the map_irq callback to pci_fixup_irqs(). > > It re-reads the device's pin number directly and feeds that through > the standard swizzle and OF's UIS system. > > Jason Ok - that all makes sense and hopefully when I get the legacy PCI interrupt mapping via DT working for IMX6 I can prove it out. What did you think of my two ideas above to resolve this issue on our add-in card where the bus-topology isn't known ahead of time: 1. bootloader builds out DT with proper interrupt-map's to handle the mis-map after enumerating the bus to understand the toplogy. or 2. a pci fixup for xio2001 that further qualifies the DT machine and the 'depth' of the xio2001 (ie make sure its the first xio2001 bridge on the bus) that replaces the swizzle function. This would be in arch/arm/mach-imx/mach-imx6q.c Thanks, Tim
On Mon, Mar 03, 2014 at 04:38:10PM -0800, Tim Harvey wrote: > On Mon, Mar 3, 2014 at 3:37 PM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > > On Mon, Mar 03, 2014 at 11:59:51AM -0800, Tim Harvey wrote: > > > >> A pci fixup for the XIO2001 could at least be placed in > >> arch/arm/mach-imx/mach-imx6q.c with a check for > >> of_machine_is_compatible("gw, ventana"). This is currently done in > >> order to handle GPIO on the PEX890x bridge which is used as PERST# for > >> downstream slots (see > >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-imx/mach-imx6q.c#n87). > > > > That is an odd looking thing .. PERST# of a subordinate port should be > > tied to the PCI_BRIDGE_CTL_BUS_RESET bit in the port's bridge > > configuration space - is more configuration of the PLX chip required? > > PCI_BRIDGE_CTL_BUS_RESET does cause a hot reset on the corresponding > downstream link however the minPCIe card slot has a PERST# pin that is > for a functional reset to the card. Ah, I'm getting my PCI versions mixed up, that all sounds right to me then.. > > Also, be aware that PCI specifies a minimum time after > > reset-deassertion before accessing the bus, and I don't see a sleep... > > there is an msleep(100) there after the de-assertion. .. and I'm blind, looks good. > Ok - that all makes sense and hopefully when I get the legacy PCI > interrupt mapping via DT working for IMX6 I can prove it out. > > What did you think of my two ideas above to resolve this issue on our > add-in card where the bus-topology isn't known ahead of time: > > 1. bootloader builds out DT with proper interrupt-map's to handle the > mis-map after enumerating the bus to understand the toplogy. This is how the OF stuff for PCI has historically been used, so it is a nice OS-agnostic way to implement this work around, particularly if you have some kind of daughter card specific information you can rely on (eg a SPD that could indicate the problematic PCB revision) If you do that you could also hoist the required fixup for your switch into firmware too. > 2. a pci fixup for xio2001 that further qualifies the DT machine and > the 'depth' of the xio2001 (ie make sure its the first xio2001 bridge > on the bus) that replaces the swizzle function. This would be in > arch/arm/mach-imx/mach-imx6q.c You can probably make this work, but given there is already a DT based fix I'm not sure if people will like a .c version, especially if it requires more single-use 'general purpose' interfaces from the core code... Jason
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index aaa05c8..b171a21 100644 --- a/drivers/pci/host/pci-imx6.c +++ b/drivers/pci/host/pci-imx6.c @@ -493,6 +493,39 @@ static int imx6_add_pcie_port(struct pcie_port *pp, return 0; } +/* TI XIO2001 PCIe-to-PCI bridge on GW16082 exp card has IRQs reversed */ +u8 ventana_swizzle(struct pci_dev *dev, u8 *pin) +{ + u8 i = 0; + struct pci_dev *pdev = dev; + + /* count number of TI XIO2001 bridges on bus */ + while (!pci_is_root_bus(pdev->bus)) { + if (pdev->bus && pdev->bus->self && + (pdev->bus->self->vendor == PCI_VENDOR_ID_TI) && + (pdev->bus->self->device == PCI_DEVICE_ID_TI_XIO2001)) { + i++; + } + pdev = pdev->bus->self; + } + while (!pci_is_root_bus(dev->bus)) { + /* if we are directly downstream from 1st TI XIO2001 bridge */ + if (dev->bus && dev->bus->self && + (dev->bus->self->vendor == PCI_VENDOR_ID_TI) && + (dev->bus->self->device == PCI_DEVICE_ID_TI_XIO2001)) { + if (--i == 0) { + /* swap IRQs and swizzle backwards */ + *pin = (15 - PCI_SLOT(dev->devfn)) + 1; + dev = dev->bus->self; + continue; + } + } + *pin = pci_swizzle_interrupt_pin(dev, *pin); + dev = dev->bus->self; + } + return PCI_SLOT(dev->devfn); +} + static int __init imx6_pcie_probe(struct platform_device *pdev) { struct imx6_pcie *imx6_pcie; @@ -601,6 +634,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev) return PTR_ERR(imx6_pcie->iomuxc_gpr); } + if (of_machine_is_compatible("gw,ventana")) + pp->swizzle = ventana_swizzle; + ret = imx6_add_pcie_port(pp, pdev); if (ret < 0) return ret; diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 97fbecd..4ca334f 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -822,6 +822,7 @@ #define PCI_DEVICE_ID_TI_XX12 0x8039 #define PCI_DEVICE_ID_TI_XX12_FM 0x803b #define PCI_DEVICE_ID_TI_XIO2000A 0x8231 +#define PCI_DEVICE_ID_TI_XIO2001 0x8240 #define PCI_DEVICE_ID_TI_1130 0xac12 #define PCI_DEVICE_ID_TI_1031 0xac13 #define PCI_DEVICE_ID_TI_1131 0xac15
The TI XIO2001 PCIe-to-PCI bridge used on Ventana expansion boards has its slot-to-bridge IRQ mapping reversed from the PCI specification: INTA->INTD INTB->INTC INTC->INTB INTD->INTA Implement a custom swizzle function that does a fixup on the interrupt for devices on the first TI XIO2001 bridge in the tree. Signed-off-by: Tim Harvey <tharvey@gateworks.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Richard Zhu <r65037@freescale.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Sean Cross <xobs@kosagi.com> Cc: Jingoo Han <jg1.han@samsung.com> --- drivers/pci/host/pci-imx6.c | 36 ++++++++++++++++++++++++++++++++++++ include/linux/pci_ids.h | 1 + 2 files changed, 37 insertions(+)