On Wed, Nov 18, 2015 at 03:24:35PM +1100, Alexey Kardashevskiy wrote:
>On 11/05/2015 12:12 AM, Gavin Shan wrote:
>>The pdn (struct pci_dn) instances are allocated from memblock or
>>bootmem when creating PCI controller (hoses) in setup_arch(). PCI
>>hotplug, which will be supported by proceeding patches, release
>>PCI device nodes and their corresponding pdn on unplugging event.
>>The memory chunks for pdn instances allocated from memblock or
>>bootmem are hard to reused after being released.
>>
>>This delays creating pdn in core_initcall_sync(eeh_dev_phb_init) so
>>that they are allocated from slab. In turn, the memory chunks for
>>them can be reused after being released without problem. Since the
>>pdn and eeh_dev has same life cycle, the eeh_dev is created when
>>pdn is populated. We needn't create eeh_dev with another initcall.
>>The time to create PHB PEs is delayed a bit from core_initcall() to
>>core_initcall_sync().
>
>Why is delayed? I mean what needs to be called before eeh_dev_phb_init()?
>
I think the changelog explains the "why". eeh_dev_phb_init() creates ancestor
PE for all other PEs. The ancestor PEs should be created before other PEs.
eeh_dev_phb_init() depends on PHBs (struct pci_controllers) only.
>>
>>Signed-off-by: Gavin Shan <gwshan@xxxxxxxxxxxxxxxxxx>
>>---
>> arch/powerpc/include/asm/eeh.h | 2 +-
>> arch/powerpc/include/asm/ppc-pci.h | 2 --
>> arch/powerpc/kernel/eeh_dev.c | 19 ++++-------------
>> arch/powerpc/kernel/pci_dn.c | 20 ++++++++++++++++--
>> arch/powerpc/platforms/maple/pci.c | 34 ++++++++++++++++++------------
>> arch/powerpc/platforms/pasemi/pci.c | 3 ---
>> arch/powerpc/platforms/powermac/pci.c | 38 +++++++++++++++++++++-------------
>> arch/powerpc/platforms/powernv/pci.c | 3 ---
>> arch/powerpc/platforms/pseries/setup.c | 6 +-----
>> 9 files changed, 69 insertions(+), 58 deletions(-)
>>
>>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>>index c5eb86f..27352f4 100644
>>--- a/arch/powerpc/include/asm/eeh.h
>>+++ b/arch/powerpc/include/asm/eeh.h
>>@@ -268,7 +268,7 @@ void eeh_pe_restore_bars(struct eeh_pe *pe);
>> const char *eeh_pe_loc_get(struct eeh_pe *pe);
>> struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
>>
>>-void *eeh_dev_init(struct pci_dn *pdn, void *data);
>>+struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
>> void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
>> int eeh_init(void);
>> int __init eeh_ops_register(struct eeh_ops *ops);
>>diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
>>index 8753e4e..0f73de0 100644
>>--- a/arch/powerpc/include/asm/ppc-pci.h
>>+++ b/arch/powerpc/include/asm/ppc-pci.h
>>@@ -39,8 +39,6 @@ void *pci_traverse_device_nodes(struct device_node *start,
>> void *traverse_pci_dn(struct pci_dn *root,
>> void *(*fn)(struct pci_dn *, void *),
>> void *data);
>>-
>>-extern void pci_devs_phb_init(void);
>> extern void pci_devs_phb_init_dynamic(struct pci_controller *phb);
>>
>> /* From rtas_pci.h */
>>diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
>>index aabba94..1c4bc35 100644
>>--- a/arch/powerpc/kernel/eeh_dev.c
>>+++ b/arch/powerpc/kernel/eeh_dev.c
>>@@ -44,14 +44,13 @@
>> /**
>> * eeh_dev_init - Create EEH device according to OF node
>> * @pdn: PCI device node
>>- * @data: PHB
>> *
>> * It will create EEH device according to the given OF node. The function
>> * might be called by PCI emunation, DR, PHB hotplug.
>> */
>>-void *eeh_dev_init(struct pci_dn *pdn, void *data)
>>+struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
>> {
>>- struct pci_controller *phb = data;
>>+ struct pci_controller *phb = pdn->phb;
>> struct eeh_dev *edev;
>>
>> /* Allocate EEH device */
>>@@ -68,7 +67,7 @@ void *eeh_dev_init(struct pci_dn *pdn, void *data)
>> edev->phb = phb;
>> INIT_LIST_HEAD(&edev->list);
>>
>>- return NULL;
>>+ return edev;
>> }
>>
>> /**
>>@@ -80,16 +79,8 @@ void *eeh_dev_init(struct pci_dn *pdn, void *data)
>> */
>> void eeh_dev_phb_init_dynamic(struct pci_controller *phb)
>> {
>>- struct pci_dn *root = phb->pci_data;
>>-
>> /* EEH PE for PHB */
>> eeh_phb_pe_create(phb);
>>-
>>- /* EEH device for PHB */
>>- eeh_dev_init(root, phb);
>>-
>>- /* EEH devices for children OF nodes */
>>- traverse_pci_dn(root, eeh_dev_init, phb);
>> }
>>
>> /**
>>@@ -105,9 +96,7 @@ static int __init eeh_dev_phb_init(void)
>> list_for_each_entry_safe(phb, tmp, &hose_list, list_node)
>> eeh_dev_phb_init_dynamic(phb);
>>
>>- pr_info("EEH: devices created\n");
>>-
>> return 0;
>> }
>>
>>-core_initcall(eeh_dev_phb_init);
>>+core_initcall_sync(eeh_dev_phb_init);
>
>
>May be remove core_initcall_sync and call eeh_dev_phb_init_dynamic() directly
>from the loop in pci_devs_phb_init()?
>
We can't do that as eeh_dev_phb_init_dynamic() can be called for newly added PHB.
>>diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>>index aa4110f..581612c 100644
>>--- a/arch/powerpc/kernel/pci_dn.c
>>+++ b/arch/powerpc/kernel/pci_dn.c
>>@@ -272,8 +272,11 @@ struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
>> const __be32 *regs;
>> struct device_node *parent;
>> struct pci_dn *pdn;
>>+#ifdef CONFIG_EEH
>>+ struct eeh_dev *edev;
>>+#endif
>>
>>- pdn = zalloc_maybe_bootmem(sizeof(*pdn), GFP_KERNEL);
>>+ pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
>> if (pdn == NULL)
>> return NULL;
>> dn->data = pdn;
>>@@ -302,6 +305,15 @@ struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
>> /* Extended config space */
>> pdn->pci_ext_config_space = (type && of_read_number(type, 1) == 1);
>>
>>+ /* Create EEH device */
>>+#ifdef CONFIG_EEH
>>+ edev = eeh_dev_init(pdn);
>>+ if (!edev) {
>>+ kfree(pdn);
>>+ return NULL;
>>+ }
>>+#endif
>>+
>> /* Attach to parent node */
>> INIT_LIST_HEAD(&pdn->child_list);
>> INIT_LIST_HEAD(&pdn->list);
>>@@ -486,15 +498,19 @@ void pci_devs_phb_init_dynamic(struct pci_controller *phb)
>> * pci device found underneath. This routine runs once,
>> * early in the boot sequence.
>> */
>>-void __init pci_devs_phb_init(void)
>>+static int __init pci_devs_phb_init(void)
>> {
>> struct pci_controller *phb, *tmp;
>>
>> /* This must be done first so the device nodes have valid pci info! */
>> list_for_each_entry_safe(phb, tmp, &hose_list, list_node)
>> pci_devs_phb_init_dynamic(phb);
>>+
>>+ return 0;
>> }
>>
>>+core_initcall(pci_devs_phb_init);
>>+
>> static void pci_dev_pdn_setup(struct pci_dev *pdev)
>> {
>> struct pci_dn *pdn;
>>diff --git a/arch/powerpc/platforms/maple/pci.c b/arch/powerpc/platforms/maple/pci.c
>>index a923230..a2f89e6 100644
>>--- a/arch/powerpc/platforms/maple/pci.c
>>+++ b/arch/powerpc/platforms/maple/pci.c
>>@@ -568,6 +568,26 @@ void maple_pci_irq_fixup(struct pci_dev *dev)
>> DBG(" <- maple_pci_irq_fixup\n");
>> }
>>
>>+static int maple_pci_root_bridge_prepare(struct pci_host_bridge *bridge)
>>+{
>>+ struct pci_controller *hose = pci_bus_to_host(bridge->bus);
>>+ struct device_node *np, *child;
>>+
>>+ if (hose != u3_agp)
>>+ return 0;
>>+
>>+ /* Fixup the PCI<->OF mapping for U3 AGP due to bus renumbering. We
>>+ * assume there is no P2P bridge on the AGP bus, which should be a
>>+ * safe assumptions hopefully.
>>+ */
>>+ np = hose->dn;
>>+ PCI_DN(np)->busno = 0xf0;
>>+ for_each_child_of_node(np, child)
>>+ PCI_DN(child)->busno = 0xf0;
>>+
>>+ return 0;
>>+}
>>+
>> void __init maple_pci_init(void)
>> {
>> struct device_node *np, *root;
>>@@ -605,19 +625,7 @@ void __init maple_pci_init(void)
>> if (ht && maple_add_bridge(ht) != 0)
>> of_node_put(ht);
>>
>>- /* Setup the linkage between OF nodes and PHBs */
>>- pci_devs_phb_init();
>>-
>>- /* Fixup the PCI<->OF mapping for U3 AGP due to bus renumbering. We
>>- * assume there is no P2P bridge on the AGP bus, which should be a
>>- * safe assumptions hopefully.
>>- */
>>- if (u3_agp) {
>>- struct device_node *np = u3_agp->dn;
>>- PCI_DN(np)->busno = 0xf0;
>>- for (np = np->child; np; np = np->sibling)
>>- PCI_DN(np)->busno = 0xf0;
>>- }
>>+ ppc_md.pcibios_root_bridge_prepare = maple_pci_root_bridge_prepare;
>
>
>This seems an unrelated change.
>
>What is this pcibios_root_bridge_prepare()? How come you do not need one for
>the powernv platform but do need for others? Same question about powermac.
>
The function is fixing up pdn for U3 AGP device. As the pdn creation is delayed
to core_initcall(), the pdn isn't created when maple_pci_init() is called. So
the pdn's fixup work is delay to ppc_md.pcibios_root_bridge_prepare().
>>
>> /* Tell pci.c to not change any resource allocations. */
>> pci_add_flags(PCI_PROBE_ONLY);
>>diff --git a/arch/powerpc/platforms/pasemi/pci.c b/arch/powerpc/platforms/pasemi/pci.c
>>index f3a68a0..10c4e8f 100644
>>--- a/arch/powerpc/platforms/pasemi/pci.c
>>+++ b/arch/powerpc/platforms/pasemi/pci.c
>>@@ -229,9 +229,6 @@ void __init pas_pci_init(void)
>> of_node_get(np);
>>
>> of_node_put(root);
>>-
>>- /* Setup the linkage between OF nodes and PHBs */
>>- pci_devs_phb_init();
>> }
>>
>> void __iomem *pasemi_pci_getcfgaddr(struct pci_dev *dev, int offset)
>>diff --git a/arch/powerpc/platforms/powermac/pci.c b/arch/powerpc/platforms/powermac/pci.c
>>index 59ab16f..6e06c3b 100644
>>--- a/arch/powerpc/platforms/powermac/pci.c
>>+++ b/arch/powerpc/platforms/powermac/pci.c
>>@@ -878,6 +878,29 @@ void pmac_pci_irq_fixup(struct pci_dev *dev)
>> #endif /* CONFIG_PPC32 */
>> }
>>
>>+#ifdef CONFIG_PPC64
>>+static int pmac_pci_root_bridge_prepare(struct pci_host_bridge *bridge)
>>+{
>>+ struct pci_controller *hose = pci_bus_to_host(bridge->bus);
>>+ struct device_node *np, *child;
>>+
>>+ if (hose != u3_agp)
>>+ return 0;
>>+
>>+ /* Fixup the PCI<->OF mapping for U3 AGP due to bus renumbering. We
>>+ * assume there is no P2P bridge on the AGP bus, which should be a
>>+ * safe assumptions for now. We should do something better in the
>>+ * future though
>>+ */
>>+ np = hose->dn;
>>+ PCI_DN(np)->busno = 0xf0;
>>+ for_each_child_of_node(np, child)
>>+ PCI_DN(child)->busno = 0xf0;
>>+
>>+ return 0;
>>+}
>>+#endif /* CONFIG_PPC64 */
>>+
>> void __init pmac_pci_init(void)
>> {
>> struct device_node *np, *root;
>>@@ -914,20 +937,7 @@ void __init pmac_pci_init(void)
>> if (ht && pmac_add_bridge(ht) != 0)
>> of_node_put(ht);
>>
>>- /* Setup the linkage between OF nodes and PHBs */
>>- pci_devs_phb_init();
>>-
>>- /* Fixup the PCI<->OF mapping for U3 AGP due to bus renumbering. We
>>- * assume there is no P2P bridge on the AGP bus, which should be a
>>- * safe assumptions for now. We should do something better in the
>>- * future though
>>- */
>>- if (u3_agp) {
>>- struct device_node *np = u3_agp->dn;
>>- PCI_DN(np)->busno = 0xf0;
>>- for (np = np->child; np; np = np->sibling)
>>- PCI_DN(np)->busno = 0xf0;
>>- }
>>+ ppc_md.pcibios_root_bridge_prepare = pmac_pci_root_bridge_prepare;
>> /* pmac_check_ht_link(); */
>>
>> #else /* CONFIG_PPC64 */
>>diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>>index fa99daf..d8832ea 100644
>>--- a/arch/powerpc/platforms/powernv/pci.c
>>+++ b/arch/powerpc/platforms/powernv/pci.c
>>@@ -807,9 +807,6 @@ void __init pnv_pci_init(void)
>> for_each_compatible_node(np, NULL, "ibm,ioda2-phb")
>> pnv_pci_init_ioda2_phb(np);
>>
>>- /* Setup the linkage between OF nodes and PHBs */
>>- pci_devs_phb_init();
>>-
>> /* Configure IOMMU DMA hooks */
>> set_pci_dma_ops(&dma_iommu_ops);
>> }
>>diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
>>index 6c274cb..bdf93a1 100644
>>--- a/arch/powerpc/platforms/pseries/setup.c
>>+++ b/arch/powerpc/platforms/pseries/setup.c
>>@@ -262,11 +262,8 @@ static int pci_dn_reconfig_notifier(struct notifier_block *nb, unsigned long act
>> case OF_RECONFIG_ATTACH_NODE:
>> parent = of_get_parent(np);
>> pdn = parent ? PCI_DN(parent) : NULL;
>>- if (pdn) {
>>- /* Create pdn and EEH device */
>>+ if (pdn)
>> pci_add_device_node_info(pdn->phb, np);
>>- eeh_dev_init(PCI_DN(np), pdn->phb);
>>- }
>>
>> of_node_put(parent);
>> break;
>>@@ -489,7 +486,6 @@ static void __init find_and_init_phbs(void)
>> }
>>
>> of_node_put(root);
>>- pci_devs_phb_init();
>>
>> /*
>> * PCI_PROBE_ONLY and PCI_REASSIGN_ALL_BUS can be set via properties
>>
Thanks,
Gavin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
