On Wednesday 02 January 2008, Li Li wrote:
> #ifdef CONFIG_PCI
> - for_each_compatible_node(np, "pci", "fsl,mpc8349-pci")
> - mpc83xx_add_bridge(np);
> + for_each_compatible_node(np, "pci", "fsl,mpc8349-pci") {
> + if (primary_pci_bus) {
> + mpc83xx_add_bridge(np, PPC_83XX_PCI |
> PPC_83XX_PCI_PRIMARY);
> + primary_pci_bus = 0;
> + } else
> + mpc83xx_add_bridge(np, PPC_83XX_PCI);
> + }
> +
> + for_each_compatible_node(np, "pci", "fsl,mpc8377-pcie") {
> + if (primary_pci_bus) {
> + mpc83xx_add_bridge(np, PPC_83XX_PCIE |
> PPC_83XX_PCI_PRIMARY);
> + primary_pci_bus = 0;
> + } else
> + mpc83xx_add_bridge(np, PPC_83XX_PCIE);
> + }
> +
> + ppc_md.pci_exclude_device = mpc837x_exclude_device;
> #endif
A few comments on how I think this can be improved:
* Instead of duplicating this code across all platforms, make a single
function that does the probing in one place (including the #ifdef
CONFIG_PCI).
* Better yet, get rid of mpc83xx_add_bridge and make it possible for the
mpc83xx code to use the of_pci_phb_driver from
arch/powerpc/kernel/of_platform.c. This used to be impossible because
of the differences between 32 and 64 bit code for PCI probing, but
I think with the work than benh put into unifying the two, we should
be pretty close now.
* The detection method for the primary bus is somewhat fragile, because
we depend on the order of the nodes in the device tree, which is not
specified to have a meaning. /Maybe/ there could be a property in
(at most) one of the PCI host bridge nodes saying that this specific bus
is the primary one.
* Since you are using exactly the same probing code for pci and pcie,
you may want to check for a more generic "compatible" property than
the specific model, so you can use the same code for both.
Arnd <><
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
