[PATCH 3/3] PCI: imx6: ventana: fixup for IRQ mismapping

[Jason Gunthorpe]

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