Dear Andrew Murray,
On Thu, 7 Feb 2013 15:51:17 +0000, Andrew Murray wrote:
> > First of all, I didn't reimplement the pcibios_get_phb_of_node(), but
> > instead, as Thierry Reding suggested, simply implemented the
> > hw_pci.scan() function as follows:
>
> I've not had any time to test Thierry's solution to avoid implementing
> pcibios_get_phb_of_node - but it did seem to work for him and seem correct
> at the time.
At least there are device tree nodes associated to PCI devices, so it
looks good from this perspective.
> > To me, the "struct pci_dev" representing the real PCIe devices should
> > have a NULL "struct device_node" pointer, because those device are not
> > represented in the DT. If this was the case, then the of_irq_map_pci()
> > would go one level up in the PCI hierarchy, find the "struct pci_dev"
> > that corresponds to the PCI-to-PCI bridge, which generates an laddr[]
> > having a bus number a devfn value matching the interrupt-map property.
>
> Yes this is my current understanding.
Ok. But that's not what happens: the "struct pci_dev" representing the
real PCIe device *DOES* have a non-NULL struct device_node pointer. And
this is what makes the entire thing fail.
> I would suggest the issue isn't in the PCI/DT code. This is what I see
I believe it is, because as I said above, the struct pci_dev associated
to a real PCIe device should not have a struct device_node pointer,
because this PCIe device has been dynamically enumerated and is
therefore not part of the device tree.
> with my implementation (which uses an implementation of
> pcibios_get_phb_of_node) - I'd like to believe this is correct behaviour:
>
> - of_irq_map_pci is called for an endpoint (5:0:0), its pdev->dev.of_node
> is NULL. As I don't have a representation of this endpoint in my DT the
> of_irq_map_pci code proceeds to walk the fabric.
I believe this should be the behavior. But this not what happens: the
pdev->dev.of_node of an endpoint pci_dev is not NULL.
> - Starting with the pdev for 5:0:0, of_irq_map_pci sees it has a parent
> (downstream switch bridge), in this case pdev(5:0:0)->bus->self->dev.of_node
> is NULL, due to this swizzling occurs and we walk the parent's bus (bus 2)
> - This continues to bus 1 and then bus 0 where of_irq_map_pci realises that
> it has no parent (its the host bridge). Due to the implementation of
> pcibios_get_phb_of_node a of_node is produced, this is then used to construct
> a lspec (for bus number 0).
>
> The of_irq_map_pci code stops walking as soon as it finds a function in the
> tree that has a device node. This suggests that if you represent a bridge
> you must also include an interrupt-map. The problem here is that you have
> represented a bridge but not included a map.
I understood that it walks up the PCI hierarchy, and that's fine. As
I've shown in my previous e-mail, the only problem is that this
pdev->dev.of_node should be NULL for the PCIe endpoint device. If it
were NULL, then everything would work correctly, as I could confirmed
by the hack I did in of_irq_map_pci().
> I can think of three solutions:
>
> 1. Something like this:
>
> } else {
> /* We found a P2P bridge, check if it has a node */
> ppnode = pci_device_to_OF_node(ppdev);
> + if (ppnode doesnt have an interrupt-map)//if (pdev->bus->number != 0)
> + ppnode = NULL;
> }
>
> 2. Remove the bridges from the DT? Or remove the map from pcie-controller and
> add a map each to pcie@0,1 and pcie@1,1?
>
> 3. Change the mask of your map so that it doesn't care about bus numbers. I
> have a map that looks like this:
>
> interrupt-map-mask = <0 0 0 7>;
> interrupt-map = <0 0 0 1 ... //anything coming in on INTA
> 0 0 0 2 ... //anything coming in on INTB
> 0 0 0 3 ... ...
> 0 0 0 4 ... ...
Unfortunately, I don't quite agree with any of your three solutions. I
still do believe the root problem is that pdev->dev.of_node should be
NULL for the PCIe endpoints, since those devices are not probed with
the Device Tree.
Best regards,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
