On Fri, Jan 18, 2013 at 09:56:20AM +0000, Andrew Murray wrote: > On Wed, Jan 09, 2013 at 08:43:10PM +0000, Thierry Reding wrote: > > Move the PCIe driver from arch/arm/mach-tegra into the drivers/pci/host > > directory. The motivation is to collect various host controller drivers > > in the same location in order to facilitate refactoring. > > > > The Tegra PCIe driver has been largely rewritten, both in order to turn > > it into a proper platform driver and to add MSI (based on code by > > Krishna Kishore <kth...@nvidia.com>) as well as device tree support. > > > > Signed-off-by: Thierry Reding <thierry.red...@avionic-design.de> > > [snip] > > > +static int tegra_pcie_enable(struct tegra_pcie *pcie) > > +{ > > + struct hw_pci hw; > > + > > + memset(&hw, 0, sizeof(hw)); > > + > > + hw.nr_controllers = 1; > > + hw.private_data = (void **)&pcie; > > + hw.setup = tegra_pcie_setup; > > + hw.scan = tegra_pcie_scan_bus; > > + hw.map_irq = tegra_pcie_map_irq; > > + > > + pci_common_init(&hw); > > + > > + return 0; > > +} > > [snip] > > > +static int tegra_pcie_probe(struct platform_device *pdev) > > +{ > > + struct device_node *port; > > + struct tegra_pcie *pcie; > > + int err; > > + > > + pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL); > > + if (!pcie) > > + return -ENOMEM; > > + > > + INIT_LIST_HEAD(&pcie->ports); > > + pcie->dev = &pdev->dev; > > + > > + err = tegra_pcie_parse_dt(pcie); > > + if (err < 0) > > + return err; > > + > > + pcibios_min_mem = 0; > > + > > + err = tegra_pcie_get_resources(pcie); > > + if (err < 0) { > > + dev_err(&pdev->dev, "failed to request resources: %d\n", > > err); > > + return err; > > + } > > + > > + err = tegra_pcie_enable_controller(pcie); > > + if (err) > > + goto put_resources; > > + > > + /* probe root ports */ > > + for_each_child_of_node(pdev->dev.of_node, port) { > > + if (!of_device_is_available(port)) > > + continue; > > + > > + err = tegra_pcie_add_port(pcie, port); > > + if (err < 0) { > > + dev_err(&pdev->dev, "failed to add port %s: %d\n", > > + port->name, err); > > + } > > + } > > + > > + /* setup the AFI address translations */ > > + tegra_pcie_setup_translations(pcie); > > + > > + if (IS_ENABLED(CONFIG_PCI_MSI)) { > > + err = tegra_pcie_enable_msi(pcie); > > + if (err < 0) { > > + dev_err(&pdev->dev, > > + "failed to enable MSI support: %d\n", > > + err); > > + goto put_resources; > > + } > > + } > > + > > + err = tegra_pcie_enable(pcie); > > + if (err < 0) { > > + dev_err(&pdev->dev, "failed to enable PCIe ports: %d\n", > > err); > > + goto disable_msi; > > + } > > + > > + platform_set_drvdata(pdev, pcie); > > + return 0; > > + > > +disable_msi: > > + if (IS_ENABLED(CONFIG_PCI_MSI)) > > + tegra_pcie_disable_msi(pcie); > > +put_resources: > > + tegra_pcie_put_resources(pcie); > > + return err; > > +} > > + > > [snip] > > > + > > +static const struct of_device_id tegra_pcie_of_match[] = { > > + { .compatible = "nvidia,tegra20-pcie", }, > > + { }, > > +}; > > + > > +static struct platform_driver tegra_pcie_driver = { > > + .driver = { > > + .name = "tegra-pcie", > > + .owner = THIS_MODULE, > > + .of_match_table = tegra_pcie_of_match, > > + }, > > + .probe = tegra_pcie_probe, > > + .remove = tegra_pcie_remove, > > +}; > > +module_platform_driver(tegra_pcie_driver); > > If you have multiple 'nvidia,tegra20-pcie's in your DT then you will end up > with multiple calls to tegra_pcie_probe/tegra_pcie_enable/pci_common_init. > > However pci_common_init/pcibios_init_hw assumes it will only ever be called > once, and will thus result in trying to create multiple busses with the same > bus number. (The first root bus it creates is always zero provided you haven't > implemented hw->scan).
Right, I hadn't noticed. There's currently no hardware that actually has two PCIe host bridges but I wanted to keep the driver properly prepared in case this ever happened. Actually I've reimplemented hw->scan, but it still forwards the bus number setup by pcibios_init_hw() (sys->busnr) to pci_create_root_bus() so it will still break. I wonder, though, if a better approach would be to take this number from the bus-range property in DT instead. > I have a patch for this if you want to fold it into your series? (I see you've > made changes to bios32 for per-controller data). I would certainly like to take a look at it. Thierry
pgpVlOFSFmCLS.pgp
Description: PGP signature
_______________________________________________ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss