Re: [PATCH v8 19/30] powerpc/pci: Use pci_scan_host_bridge() for simplicity

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2015/3/26 6:13, Daniel Axtens wrote:
Hi Yijing,

I wasn't quite sure I understood your comments, so I was trying to apply
your patch series and test it, but patch 3 doesn't apply cleanly to
4.0-rc5 or master. Can you respin the series?

Hi Daniel,
   Could you pull the series from Bjorn's git tree ? The URL is
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git pci/enumeration-yw8

Thanks!
Yijing.


Thanks,
Daniel


Hi Daniel, thanks for your review and comments. We want to make a generic pci_host_bridge,
which would hold the common host information, for example, pci domain is common info for
pci host bridge, this series saved domain in pci_host_bridge, then we no need to
extract out domain by pci_bus->sysdata by platform specific pci_domain_nr().
Also we store the sysdata in pci_host_bridge, and pci_bus_to_host() is the platform
interface, I think use the common interface would be better.

+
+	/* Get probe mode and perform scan */
+	if (hose->dn && ppc_md.pci_probe_mode)
+		mode = ppc_md.pci_probe_mode(bus);
+
+	pr_debug("    probe mode: %d\n", mode);
+	if (mode == PCI_PROBE_DEVTREE)
+		of_scan_bus(hose->dn, bus);
+
+	if (mode == PCI_PROBE_NORMAL) {
+		pci_bus_update_busn_res_end(bus, 255);
+		hose->last_busno = pci_scan_child_bus(bus);
+		pci_bus_update_busn_res_end(bus, hose->last_busno);
+	}
+
+	return pci_bus_child_max_busnr(bus);
+}
+
I'm having trouble convincing myself that this patch covers every
variation within our PCI implementations. In particular, there's a
stanza in of_scan_pci_bridge in kernel/pci_of_scan.c that's almost
identical to this function. Does that implementation need to be cleaned
up and replaced with this function too?


This is a pci_host_bridge_ops hook function, which would be called in
PCI core, and after applied this series, we only need to call pci_scan_host_bridge()
to scan pci devices, and this function is also extracted from the pcibios_scan_phb(),
it's not the redundant code.


@@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose)
 		ppc_md.pcibios_fixup_phb(hose);
 
 	/* Configure PCI Express settings */
-	if (bus && !pci_has_flag(PCI_PROBE_ONLY)) {
+	if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) {
 		struct pci_bus *child;
-		list_for_each_entry(child, &bus->children, node)
+		list_for_each_entry(child, &host->bus->children, node)
 			pcie_bus_configure_settings(child);
 	}
 }
Two things: Firstly, the function uses hose throughout, not host.
Secondly, you're not deleting the bus variable: what's the purpose of
this change?

host is the common pci_host_bridge which is created by PCI core for pci host bridge driver,
the hose is the platform data used in powerpc. The purpose of the patch/series is to simplify
pci enumeration interface, and try to reduce the weak functions which were used to setup pci bus/devices
during PCI enumeration.


Regards,
Daniel






-- 
Thanks!
Yijing

--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux