[PATCH V4 0/3] PCI patch set description

The first patch fixed a kernel panic when scanning PCI bus if it is working
in agent mode.

In later two patches we unified PCI initialization code by changing fsl_pci
to a platform drvier. The approach will affect swiotlb init and this issue
is addressed in the second patch.

We now just convet IBM 44x, MPC85xxDS and QEMU to this new mechanism. All
other boards will be coverted if this patch set is accepted.

We change fsl_add_bridge to return -ENODEV if the controller is working in
agent mode. Then check the return value of fsl_add_bridge to guarantee
that only successfully added host bus will be scanned.

Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---
 arch/powerpc/sysdev/fsl_pci.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 50a38b3..6938792 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c

@@ -465,7 +465,7 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
 			iounmap(hose->cfg_data);
 		iounmap(hose->cfg_addr);
 		pcibios_free_controller(hose);
-		return 0;
+		return -ENODEV;
 	}
 
 	setup_pci_cmd(hose);

@@ -828,6 +828,7 @@ struct device_node *fsl_pci_primary;
 
 void __devinit fsl_pci_init(void)
 {
+	int ret;
 	struct device_node *node;
 	struct pci_controller *hose;
 	dma_addr_t max = 0xffffffff;

@@ -856,10 +857,12 @@ void __devinit fsl_pci_init(void)
 			if (!fsl_pci_primary)
 				fsl_pci_primary = node;
 
-			fsl_add_bridge(node, fsl_pci_primary == node);
-			hose = pci_find_hose_for_OF_device(node);
-			max = min(max, hose->dma_window_base_cur +
-					hose->dma_window_size);
+			ret = fsl_add_bridge(node, fsl_pci_primary == node);
+			if (ret == 0) {
+				hose = pci_find_hose_for_OF_device(node);
+				max = min(max, hose->dma_window_base_cur +
+						hose->dma_window_size);
+			}
 		}
 	}
 

-- 
1.7.5.1

Remove the dependency on PCI initialization for SWIOTLB initialization.
So that PCI can be initialized at proper time.

SWIOTLB is partly determined by PCI inbound/outbound map which is assigned
in PCI initialization. But swiotlb_init() should be done at the stage of
mem_init() which is much earlier than PCI initialization. So we reserve the
memory for SWIOTLB first and free it if not necessary.

All boards are converted to fit this change.

Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---
 arch/powerpc/include/asm/swiotlb.h       |    6 ++++++
 arch/powerpc/kernel/dma-swiotlb.c        |   20 ++++++++++++++++++++
 arch/powerpc/mm/mem.c                    |    3 +--
 arch/powerpc/platforms/44x/currituck.c   |   10 ++--------
 arch/powerpc/platforms/85xx/mpc85xx_ds.c |    1 +
 arch/powerpc/platforms/85xx/qemu_e500.c  |    2 +-
 arch/powerpc/sysdev/fsl_pci.c            |    5 +----
 7 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/swiotlb.h b/arch/powerpc/include/asm/swiotlb.h
index 8979d4c..de99d6e 100644
--- a/arch/powerpc/include/asm/swiotlb.h
+++ b/arch/powerpc/include/asm/swiotlb.h

@@ -22,4 +22,10 @@ int __init swiotlb_setup_bus_notifier(void);
 
 extern void pci_dma_dev_setup_swiotlb(struct pci_dev *pdev);
 
+#ifdef CONFIG_SWIOTLB
+void swiotlb_detect_4g(void);
+#else
+static inline void swiotlb_detect_4g(void) {}
+#endif
+
 #endif /* __ASM_SWIOTLB_H */

diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
index 4ab88da..aa85550 100644
--- a/arch/powerpc/kernel/dma-swiotlb.c
+++ b/arch/powerpc/kernel/dma-swiotlb.c

@@ -104,3 +104,23 @@ int __init swiotlb_setup_bus_notifier(void)
 			      &ppc_swiotlb_plat_bus_notifier);
 	return 0;
 }
+
+void swiotlb_detect_4g(void)
+{
+	if ((memblock_end_of_DRAM() - 1) > 0xffffffff)
+		ppc_swiotlb_enable = 1;
+}
+
+static int __init swiotlb_late_init(void)
+{
+	if (ppc_swiotlb_enable) {
+		swiotlb_print_info();
+		set_pci_dma_ops(&swiotlb_dma_ops);
+		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
+	} else {
+		swiotlb_free();
+	}
+
+	return 0;
+}
+subsys_initcall(swiotlb_late_init);

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index baaafde..f23c4e0 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c

@@ -300,8 +300,7 @@ void __init mem_init(void)
 	unsigned long reservedpages = 0, codesize, initsize, datasize, bsssize;
 
 #ifdef CONFIG_SWIOTLB
-	if (ppc_swiotlb_enable)
-		swiotlb_init(1);
+	swiotlb_init(0);
 #endif
 
 	num_physpages = memblock_phys_mem_size() >> PAGE_SHIFT;

diff --git a/arch/powerpc/platforms/44x/currituck.c b/arch/powerpc/platforms/44x/currituck.c
index 9f6c33d..6bd89a0 100644
--- a/arch/powerpc/platforms/44x/currituck.c
+++ b/arch/powerpc/platforms/44x/currituck.c

@@ -21,7 +21,6 @@
  */
 
 #include <linux/init.h>
-#include <linux/memblock.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/rtc.h>

@@ -159,13 +158,8 @@ static void __init ppc47x_setup_arch(void)
 
 	/* No need to check the DMA config as we /know/ our windows are all of
  	 * RAM.  Lets hope that doesn't change */
-#ifdef CONFIG_SWIOTLB
-	if ((memblock_end_of_DRAM() - 1) > 0xffffffff) {
-		ppc_swiotlb_enable = 1;
-		set_pci_dma_ops(&swiotlb_dma_ops);
-		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
-	}
-#endif
+	swiotlb_detect_4g();
+
 	ppc47x_smp_init();
 }
 

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
index 6d3265f..56f8c8f 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c

@@ -159,6 +159,7 @@ static void __init mpc85xx_ds_setup_arch(void)
 	if (ppc_md.progress)
 		ppc_md.progress("mpc85xx_ds_setup_arch()", 0);
 
+	swiotlb_detect_4g();
 	mpc85xx_ds_pci_init();
 	mpc85xx_smp_init();
 

diff --git a/arch/powerpc/platforms/85xx/qemu_e500.c b/arch/powerpc/platforms/85xx/qemu_e500.c
index 95a2e53..04260cd 100644
--- a/arch/powerpc/platforms/85xx/qemu_e500.c
+++ b/arch/powerpc/platforms/85xx/qemu_e500.c

@@ -41,7 +41,7 @@ static void __init qemu_e500_setup_arch(void)
 {
 	ppc_md.progress("qemu_e500_setup_arch()", 0);
 
-	fsl_pci_init();
+	swiotlb_detect_4g();
 	mpc85xx_smp_init();
 }
 

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 6938792..da7a3d7 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c

@@ -872,11 +872,8 @@ void __devinit fsl_pci_init(void)
 	 * we need SWIOTLB to handle buffers located outside of
 	 * dma capable memory region
 	 */
-	if (memblock_end_of_DRAM() - 1 > max) {
+	if (memblock_end_of_DRAM() - 1 > max)
 		ppc_swiotlb_enable = 1;
-		set_pci_dma_ops(&swiotlb_dma_ops);
-		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
-	}
 #endif
 }
 #endif

-- 
1.7.5.1

We unified the Freescale pci/pcie initialization by changing the fsl_pci
to a platform driver. In previous PCI code architecture the initialization
routine is called at board_setup_arch stage. Now the initialization is done
in probe function which is architectural better. Also It's convenient for
adding PM support for PCI controller in later patch.

Now we registered pci controllers as platform devices. So we combine two
initialization code as one platform driver.

Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
Signed-off-by: Chunhe Lan <redacted>
---
 arch/powerpc/platforms/85xx/mpc85xx_ds.c |   32 ++--------
 arch/powerpc/sysdev/fsl_pci.c            |  102 ++++++++++++++++++-----------
 arch/powerpc/sysdev/fsl_pci.h            |    6 +-
 drivers/edac/mpc85xx_edac.c              |   43 ++++---------
 4 files changed, 83 insertions(+), 100 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
index 56f8c8f..f2c7b1c 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c

@@ -20,7 +20,6 @@
 #include <linux/seq_file.h>
 #include <linux/interrupt.h>
 #include <linux/of_platform.h>
-#include <linux/memblock.h>
 
 #include <asm/time.h>
 #include <asm/machdep.h>

@@ -117,40 +116,16 @@ void __init mpc85xx_ds_pic_init(void)
 extern int uli_exclude_device(struct pci_controller *hose,
 				u_char bus, u_char devfn);
 
-static struct device_node *pci_with_uli;
-
 static int mpc85xx_exclude_device(struct pci_controller *hose,
 				   u_char bus, u_char devfn)
 {
-	if (hose->dn == pci_with_uli)
+	if (hose->dn == fsl_pci_primary)
 		return uli_exclude_device(hose, bus, devfn);
 
 	return PCIBIOS_SUCCESSFUL;
 }
 #endif	/* CONFIG_PCI */
 
-static void __init mpc85xx_ds_pci_init(void)
-{
-#ifdef CONFIG_PCI
-	struct device_node *node;
-
-	fsl_pci_init();
-
-	/* See if we have a ULI under the primary */
-
-	node = of_find_node_by_name(NULL, "uli1575");
-	while ((pci_with_uli = of_get_parent(node))) {
-		of_node_put(node);
-		node = pci_with_uli;
-
-		if (pci_with_uli == fsl_pci_primary) {
-			ppc_md.pci_exclude_device = mpc85xx_exclude_device;
-			break;
-		}
-	}
-#endif
-}
-
 /*
  * Setup the architecture
  */

@@ -159,8 +134,11 @@ static void __init mpc85xx_ds_setup_arch(void)
 	if (ppc_md.progress)
 		ppc_md.progress("mpc85xx_ds_setup_arch()", 0);
 
+#ifdef CONFIG_PCI
+	ppc_md.pci_exclude_device = mpc85xx_exclude_device;
+#endif
+
 	swiotlb_detect_4g();
-	mpc85xx_ds_pci_init();
 	mpc85xx_smp_init();
 
 	printk("MPC85xx DS board from Freescale Semiconductor\n");

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index da7a3d7..6408d9d 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c

@@ -826,54 +826,78 @@ static const struct of_device_id pci_ids[] = {
 
 struct device_node *fsl_pci_primary;
 
-void __devinit fsl_pci_init(void)
+/* Checkout if PCI contains ISA node */
+static int of_pci_has_isa(struct device_node *pci_node)
+{
+	struct device_node *np;
+	int ret = 0;
+
+	if (!pci_node)
+		return 0;
+
+	read_lock(&devtree_lock);
+	np = pci_node->allnext;
+
+	/* Only scan the children of PCI node */
+	for (; np != pci_node->sibling; np = np->allnext) {
+		if (np->type && (of_node_cmp(np->type, "isa") == 0)
+		    && of_node_get(np)) {
+			ret = 1;
+			break;
+		}
+	}
+
+	of_node_put(pci_node);
+	read_unlock(&devtree_lock);
+
+	return ret;
+}
+
+static int __devinit fsl_pci_probe(struct platform_device *pdev)
 {
 	int ret;
-	struct device_node *node;
 	struct pci_controller *hose;
-	dma_addr_t max = 0xffffffff;
+	int is_primary = 0;
 
-	/* Callers can specify the primary bus using other means. */
 	if (!fsl_pci_primary) {
-		/* If a PCI host bridge contains an ISA node, it's primary. */
-		node = of_find_node_by_type(NULL, "isa");
-		while ((fsl_pci_primary = of_get_parent(node))) {
-			of_node_put(node);
-			node = fsl_pci_primary;
-
-			if (of_match_node(pci_ids, node))
-				break;
-		}
+		is_primary = of_pci_has_isa(pdev->dev.of_node);
+		if (is_primary)
+			fsl_pci_primary = pdev->dev.of_node;
 	}
 
-	node = NULL;
-	for_each_node_by_type(node, "pci") {
-		if (of_match_node(pci_ids, node)) {
-			/*
-			 * If there's no PCI host bridge with ISA, arbitrarily
-			 * designate one as primary.  This can go away once
-			 * various bugs with primary-less systems are fixed.
-			 */
-			if (!fsl_pci_primary)
-				fsl_pci_primary = node;
-
-			ret = fsl_add_bridge(node, fsl_pci_primary == node);
-			if (ret == 0) {
-				hose = pci_find_hose_for_OF_device(node);
-				max = min(max, hose->dma_window_base_cur +
-						hose->dma_window_size);
-			}
-		}
-	}
+	ret = fsl_add_bridge(pdev->dev.of_node, is_primary);
 
 #ifdef CONFIG_SWIOTLB
-	/*
-	 * if we couldn't map all of DRAM via the dma windows
-	 * we need SWIOTLB to handle buffers located outside of
-	 * dma capable memory region
-	 */
-	if (memblock_end_of_DRAM() - 1 > max)
-		ppc_swiotlb_enable = 1;
+	if (ret == 0) {
+		hose = pci_find_hose_for_OF_device(pdev->dev.of_node);
+
+		/*
+		 * if we couldn't map all of DRAM via the dma windows
+		 * we need SWIOTLB to handle buffers located outside of
+		 * dma capable memory region
+		 */
+		if (memblock_end_of_DRAM() - 1 > hose->dma_window_base_cur +
+				hose->dma_window_size)
+			ppc_swiotlb_enable = 1;
+	}
 #endif
+
+	mpc85xx_pci_err_probe(pdev);
+
+	return 0;
+}
+
+static struct platform_driver fsl_pci_driver = {
+	.driver = {
+		.name = "fsl-pci",
+		.of_match_table = pci_ids,
+	},
+	.probe = fsl_pci_probe,
+};
+
+static int __init fsl_pci_init(void)
+{
+	return platform_driver_register(&fsl_pci_driver);
 }
+arch_initcall(fsl_pci_init);
 #endif

diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index baa0fd1..e3fcc6c 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h

@@ -95,10 +95,10 @@ u64 fsl_pci_immrbar_base(struct pci_controller *hose);
 
 extern struct device_node *fsl_pci_primary;
 
-#ifdef CONFIG_FSL_PCI
-void fsl_pci_init(void);
+#ifdef CONFIG_EDAC_MPC85XX
+int mpc85xx_pci_err_probe(struct platform_device *op);
 #else
-static inline void fsl_pci_init(void) {}
+static inline int mpc85xx_pci_err_probe(struct platform_device *op) {}
 #endif
 
 #endif /* __POWERPC_FSL_PCI_H */

diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index 0e37462..2677883 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c

@@ -200,7 +200,7 @@ static irqreturn_t mpc85xx_pci_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int __devinit mpc85xx_pci_err_probe(struct platform_device *op)
+int __devinit mpc85xx_pci_err_probe(struct platform_device *op)
 {
 	struct edac_pci_ctl_info *pci;
 	struct mpc85xx_pci_pdata *pdata;

@@ -214,6 +214,16 @@ static int __devinit mpc85xx_pci_err_probe(struct platform_device *op)
 	if (!pci)
 		return -ENOMEM;
 
+	/* make sure error reporting method is sane */
+	switch (edac_op_state) {
+	case EDAC_OPSTATE_POLL:
+	case EDAC_OPSTATE_INT:
+		break;
+	default:
+		edac_op_state = EDAC_OPSTATE_INT;
+		break;
+	}
+
 	pdata = pci->pvt_info;
 	pdata->name = "mpc85xx_pci_err";
 	pdata->irq = NO_IRQ;

@@ -303,6 +313,7 @@ err:
 	devres_release_group(&op->dev, mpc85xx_pci_err_probe);
 	return res;
 }
+EXPORT_SYMBOL_GPL(mpc85xx_pci_err_probe);
 
 static int mpc85xx_pci_err_remove(struct platform_device *op)
 {

@@ -326,27 +337,6 @@ static int mpc85xx_pci_err_remove(struct platform_device *op)
 	return 0;
 }
 
-static struct of_device_id mpc85xx_pci_err_of_match[] = {
-	{
-	 .compatible = "fsl,mpc8540-pcix",
-	 },
-	{
-	 .compatible = "fsl,mpc8540-pci",
-	},
-	{},
-};
-MODULE_DEVICE_TABLE(of, mpc85xx_pci_err_of_match);
-
-static struct platform_driver mpc85xx_pci_err_driver = {
-	.probe = mpc85xx_pci_err_probe,
-	.remove = __devexit_p(mpc85xx_pci_err_remove),
-	.driver = {
-		.name = "mpc85xx_pci_err",
-		.owner = THIS_MODULE,
-		.of_match_table = mpc85xx_pci_err_of_match,
-	},
-};
-
 #endif				/* CONFIG_PCI */
 
 /**************************** L2 Err device ***************************/

@@ -1193,12 +1183,6 @@ static int __init mpc85xx_mc_init(void)
 	if (res)
 		printk(KERN_WARNING EDAC_MOD_STR "L2 fails to register\n");
 
-#ifdef CONFIG_PCI
-	res = platform_driver_register(&mpc85xx_pci_err_driver);
-	if (res)
-		printk(KERN_WARNING EDAC_MOD_STR "PCI fails to register\n");
-#endif
-
 #ifdef CONFIG_FSL_SOC_BOOKE
 	pvr = mfspr(SPRN_PVR);
 

@@ -1235,9 +1219,6 @@ static void __exit mpc85xx_mc_exit(void)
 		on_each_cpu(mpc85xx_mc_restore_hid1, NULL, 0);
 	}
 #endif
-#ifdef CONFIG_PCI
-	platform_driver_unregister(&mpc85xx_pci_err_driver);
-#endif
 	platform_driver_unregister(&mpc85xx_l2_err_driver);
 	platform_driver_unregister(&mpc85xx_mc_err_driver);
 }

-- 
1.7.5.1

On Aug 2, 2012, at 6:42 AM, Jia Hongtao wrote:

We unified the Freescale pci/pcie initialization by changing the =

fsl_pci

to a platform driver. In previous PCI code architecture the =

initialization

routine is called at board_setup_arch stage. Now the initialization is =

done

in probe function which is architectural better. Also It's convenient =

for

adding PM support for PCI controller in later patch.
=20
Now we registered pci controllers as platform devices. So we combine =

two

initialization code as one platform driver.
=20
Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
Signed-off-by: Chunhe Lan <redacted>
---
arch/powerpc/platforms/85xx/mpc85xx_ds.c |   32 ++--------
arch/powerpc/sysdev/fsl_pci.c            |  102 =

++++++++++++++++++-----------

arch/powerpc/sysdev/fsl_pci.h            |    6 +-
drivers/edac/mpc85xx_edac.c              |   43 ++++---------
4 files changed, 83 insertions(+), 100 deletions(-)

You need to convert all boards to use fsl_pci_init before this patch.  =
Otherwise we'll end up with PCI getting initialized twice on boards.

- k=

On Aug 2, 2012, at 6:42 AM, Jia Hongtao wrote:

Remove the dependency on PCI initialization for SWIOTLB =

initialization.

So that PCI can be initialized at proper time.
=20
SWIOTLB is partly determined by PCI inbound/outbound map which is =

assigned

in PCI initialization. But swiotlb_init() should be done at the stage =

of

mem_init() which is much earlier than PCI initialization. So we =

reserve the

memory for SWIOTLB first and free it if not necessary.
=20
All boards are converted to fit this change.
=20
Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---

This doesn't seem like it addresses our issue w/regards to not being =
able to map all of memory from PCI.

arch/powerpc/include/asm/swiotlb.h       |    6 ++++++
arch/powerpc/kernel/dma-swiotlb.c        |   20 ++++++++++++++++++++
arch/powerpc/mm/mem.c                    |    3 +--
arch/powerpc/platforms/44x/currituck.c   |   10 ++--------
arch/powerpc/platforms/85xx/mpc85xx_ds.c |    1 +
arch/powerpc/platforms/85xx/qemu_e500.c  |    2 +-
arch/powerpc/sysdev/fsl_pci.c            |    5 +----
7 files changed, 32 insertions(+), 15 deletions(-)

Don't we also want to update all these:

arch/powerpc/platforms/85xx/corenet_ds.c:               =
ppc_swiotlb_enable =3D 1;
arch/powerpc/platforms/85xx/ge_imp3a.c:         ppc_swiotlb_enable =3D =
1;
arch/powerpc/platforms/85xx/mpc8536_ds.c:               =
ppc_swiotlb_enable =3D 1;
arch/powerpc/platforms/85xx/mpc85xx_mds.c:              =
ppc_swiotlb_enable =3D 1;
arch/powerpc/platforms/85xx/p1022_ds.c:         ppc_swiotlb_enable =3D =
1;
arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:             =
ppc_swiotlb_enable =3D 1;

b/arch/powerpc/include/asm/swiotlb.h

b/arch/powerpc/kernel/dma-swiotlb.c

bsssize;

b/arch/powerpc/platforms/44x/currituck.c

all of

b/arch/powerpc/platforms/85xx/mpc85xx_ds.c

b/arch/powerpc/platforms/85xx/qemu_e500.c

removing fsl_pci_init() seems wrong.

b/arch/powerpc/sysdev/fsl_pci.c

On 08/02/2012 07:23 AM, Kumar Gala wrote:

You need to convert all boards to use fsl_pci_init before this patch.  Otherwise we'll end up with PCI getting initialized twice on boards.

Alternatively, don't make fsl_pci_init an initcall until all boards have
been converted.

-Scott

On 08/02/2012 06:42 AM, Jia Hongtao wrote:

Why are you eliminating the uli lookup?  We don't want to call
uli_exclude_device on boards that don't have a uli.

-Scott

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogRnJpZGF5LCBBdWd1c3QgMDMsIDIwMTIgNDoxOSBBTQ0KPiBUbzogSmlhIEhv
bmd0YW8tQjM4OTUxDQo+IENjOiBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgZ2FsYWtA
a2VybmVsLmNyYXNoaW5nLm9yZzsgV29vZCBTY290dC0NCj4gQjA3NDIxOyBMaSBZYW5nLVI1ODQ3
Mg0KPiBTdWJqZWN0OiBSZTogW1BBVENIIFY0IDMvM10gcG93ZXJwYy9mc2wtcGNpOiBVbmlmeSBw
Y2kvcGNpZQ0KPiBpbml0aWFsaXphdGlvbiBjb2RlDQo+IA0KPiBPbiAwOC8wMi8yMDEyIDA2OjQy
IEFNLCBKaWEgSG9uZ3RhbyB3cm90ZToNCj4gPiBXZSB1bmlmaWVkIHRoZSBGcmVlc2NhbGUgcGNp
L3BjaWUgaW5pdGlhbGl6YXRpb24gYnkgY2hhbmdpbmcgdGhlDQo+IGZzbF9wY2kNCj4gPiB0byBh
IHBsYXRmb3JtIGRyaXZlci4gSW4gcHJldmlvdXMgUENJIGNvZGUgYXJjaGl0ZWN0dXJlIHRoZQ0K
PiBpbml0aWFsaXphdGlvbg0KPiA+IHJvdXRpbmUgaXMgY2FsbGVkIGF0IGJvYXJkX3NldHVwX2Fy
Y2ggc3RhZ2UuIE5vdyB0aGUgaW5pdGlhbGl6YXRpb24gaXMNCj4gZG9uZQ0KPiA+IGluIHByb2Jl
IGZ1bmN0aW9uIHdoaWNoIGlzIGFyY2hpdGVjdHVyYWwgYmV0dGVyLiBBbHNvIEl0J3MgY29udmVu
aWVudA0KPiBmb3INCj4gPiBhZGRpbmcgUE0gc3VwcG9ydCBmb3IgUENJIGNvbnRyb2xsZXIgaW4g
bGF0ZXIgcGF0Y2guDQo+ID4NCj4gPiBOb3cgd2UgcmVnaXN0ZXJlZCBwY2kgY29udHJvbGxlcnMg
YXMgcGxhdGZvcm0gZGV2aWNlcy4gU28gd2UgY29tYmluZQ0KPiB0d28NCj4gPiBpbml0aWFsaXph
dGlvbiBjb2RlIGFzIG9uZSBwbGF0Zm9ybSBkcml2ZXIuDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5
OiBKaWEgSG9uZ3RhbyA8QjM4OTUxQGZyZWVzY2FsZS5jb20+DQo+ID4gU2lnbmVkLW9mZi1ieTog
TGkgWWFuZyA8bGVvbGlAZnJlZXNjYWxlLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBDaHVuaGUg
TGFuIDxDaHVuaGUuTGFuQGZyZWVzY2FsZS5jb20+DQo+ID4gLS0tDQo+ID4gIGFyY2gvcG93ZXJw
Yy9wbGF0Zm9ybXMvODV4eC9tcGM4NXh4X2RzLmMgfCAgIDMyICsrLS0tLS0tLS0NCj4gPiAgYXJj
aC9wb3dlcnBjL3N5c2Rldi9mc2xfcGNpLmMgICAgICAgICAgICB8ICAxMDIgKysrKysrKysrKysr
KysrKysrLS0tLQ0KPiAtLS0tLS0tDQo+ID4gIGFyY2gvcG93ZXJwYy9zeXNkZXYvZnNsX3BjaS5o
ICAgICAgICAgICAgfCAgICA2ICstDQo+ID4gIGRyaXZlcnMvZWRhYy9tcGM4NXh4X2VkYWMuYyAg
ICAgICAgICAgICAgfCAgIDQzICsrKystLS0tLS0tLS0NCj4gPiAgNCBmaWxlcyBjaGFuZ2VkLCA4
MyBpbnNlcnRpb25zKCspLCAxMDAgZGVsZXRpb25zKC0pDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEv
YXJjaC9wb3dlcnBjL3BsYXRmb3Jtcy84NXh4L21wYzg1eHhfZHMuYw0KPiBiL2FyY2gvcG93ZXJw
Yy9wbGF0Zm9ybXMvODV4eC9tcGM4NXh4X2RzLmMNCj4gPiBpbmRleCA1NmY4YzhmLi5mMmM3YjFj
IDEwMDY0NA0KPiA+IC0tLSBhL2FyY2gvcG93ZXJwYy9wbGF0Zm9ybXMvODV4eC9tcGM4NXh4X2Rz
LmMNCj4gPiArKysgYi9hcmNoL3Bvd2VycGMvcGxhdGZvcm1zLzg1eHgvbXBjODV4eF9kcy5jDQo+
ID4gQEAgLTIwLDcgKzIwLDYgQEANCj4gPiAgI2luY2x1ZGUgPGxpbnV4L3NlcV9maWxlLmg+DQo+
ID4gICNpbmNsdWRlIDxsaW51eC9pbnRlcnJ1cHQuaD4NCj4gPiAgI2luY2x1ZGUgPGxpbnV4L29m
X3BsYXRmb3JtLmg+DQo+ID4gLSNpbmNsdWRlIDxsaW51eC9tZW1ibG9jay5oPg0KPiA+DQo+ID4g
ICNpbmNsdWRlIDxhc20vdGltZS5oPg0KPiA+ICAjaW5jbHVkZSA8YXNtL21hY2hkZXAuaD4NCj4g
PiBAQCAtMTE3LDQwICsxMTYsMTYgQEAgdm9pZCBfX2luaXQgbXBjODV4eF9kc19waWNfaW5pdCh2
b2lkKQ0KPiA+ICBleHRlcm4gaW50IHVsaV9leGNsdWRlX2RldmljZShzdHJ1Y3QgcGNpX2NvbnRy
b2xsZXIgKmhvc2UsDQo+ID4gIAkJCQl1X2NoYXIgYnVzLCB1X2NoYXIgZGV2Zm4pOw0KPiA+DQo+
ID4gLXN0YXRpYyBzdHJ1Y3QgZGV2aWNlX25vZGUgKnBjaV93aXRoX3VsaTsNCj4gPiAtDQo+ID4g
IHN0YXRpYyBpbnQgbXBjODV4eF9leGNsdWRlX2RldmljZShzdHJ1Y3QgcGNpX2NvbnRyb2xsZXIg
Kmhvc2UsDQo+ID4gIAkJCQkgICB1X2NoYXIgYnVzLCB1X2NoYXIgZGV2Zm4pDQo+ID4gIHsNCj4g
PiAtCWlmIChob3NlLT5kbiA9PSBwY2lfd2l0aF91bGkpDQo+ID4gKwlpZiAoaG9zZS0+ZG4gPT0g
ZnNsX3BjaV9wcmltYXJ5KQ0KPiA+ICAJCXJldHVybiB1bGlfZXhjbHVkZV9kZXZpY2UoaG9zZSwg
YnVzLCBkZXZmbik7DQo+ID4NCj4gPiAgCXJldHVybiBQQ0lCSU9TX1NVQ0NFU1NGVUw7DQo+ID4g
IH0NCj4gPiAgI2VuZGlmCS8qIENPTkZJR19QQ0kgKi8NCj4gPg0KPiA+IC1zdGF0aWMgdm9pZCBf
X2luaXQgbXBjODV4eF9kc19wY2lfaW5pdCh2b2lkKQ0KPiA+IC17DQo+ID4gLSNpZmRlZiBDT05G
SUdfUENJDQo+ID4gLQlzdHJ1Y3QgZGV2aWNlX25vZGUgKm5vZGU7DQo+ID4gLQ0KPiA+IC0JZnNs
X3BjaV9pbml0KCk7DQo+ID4gLQ0KPiA+IC0JLyogU2VlIGlmIHdlIGhhdmUgYSBVTEkgdW5kZXIg
dGhlIHByaW1hcnkgKi8NCj4gPiAtDQo+ID4gLQlub2RlID0gb2ZfZmluZF9ub2RlX2J5X25hbWUo
TlVMTCwgInVsaTE1NzUiKTsNCj4gPiAtCXdoaWxlICgocGNpX3dpdGhfdWxpID0gb2ZfZ2V0X3Bh
cmVudChub2RlKSkpIHsNCj4gPiAtCQlvZl9ub2RlX3B1dChub2RlKTsNCj4gPiAtCQlub2RlID0g
cGNpX3dpdGhfdWxpOw0KPiA+IC0NCj4gPiAtCQlpZiAocGNpX3dpdGhfdWxpID09IGZzbF9wY2lf
cHJpbWFyeSkgew0KPiA+IC0JCQlwcGNfbWQucGNpX2V4Y2x1ZGVfZGV2aWNlID0gbXBjODV4eF9l
eGNsdWRlX2RldmljZTsNCj4gPiAtCQkJYnJlYWs7DQo+ID4gLQkJfQ0KPiA+IC0JfQ0KPiA+IC0j
ZW5kaWYNCj4gPiAtfQ0KPiA+IC0NCj4gPiAgLyoNCj4gPiAgICogU2V0dXAgdGhlIGFyY2hpdGVj
dHVyZQ0KPiA+ICAgKi8NCj4gPiBAQCAtMTU5LDggKzEzNCwxMSBAQCBzdGF0aWMgdm9pZCBfX2lu
aXQgbXBjODV4eF9kc19zZXR1cF9hcmNoKHZvaWQpDQo+ID4gIAlpZiAocHBjX21kLnByb2dyZXNz
KQ0KPiA+ICAJCXBwY19tZC5wcm9ncmVzcygibXBjODV4eF9kc19zZXR1cF9hcmNoKCkiLCAwKTsN
Cj4gPg0KPiA+ICsjaWZkZWYgQ09ORklHX1BDSQ0KPiA+ICsJcHBjX21kLnBjaV9leGNsdWRlX2Rl
dmljZSA9IG1wYzg1eHhfZXhjbHVkZV9kZXZpY2U7DQo+ID4gKyNlbmRpZg0KPiANCj4gV2h5IGFy
ZSB5b3UgZWxpbWluYXRpbmcgdGhlIHVsaSBsb29rdXA/ICBXZSBkb24ndCB3YW50IHRvIGNhbGwN
Cj4gdWxpX2V4Y2x1ZGVfZGV2aWNlIG9uIGJvYXJkcyB0aGF0IGRvbid0IGhhdmUgYSB1bGkuDQo+
IA0KPiAtU2NvdHQNCg0KSSBmb3VuZCBvdXQgdGhhdCBhbGwgODV4eF9kcyBib2FyZHMgKG1wYzg1
NzJkcywgbXBjODU0NGRzLCBwMjAyMGRzKSBoYXZlDQpVTEkuIEFsc28gaW4gcGxhdGZvcm0gZHJp
dmVyIGZzbF9wY2lfcHJpbWFyeSBpcyBkZXRlcm1pbmVkIGF0IGFyY2hfaW5pdGNhbGwNCndoaWNo
IG1lYW5zIGF0IHRoZSBzdGFnZSBvZiBib2FyZF9zZXR1cF9hcmNoIGZzbF9wY2lfcHJpbWFyeSBp
cyBub3QgcmVhZHkuDQoNCi1Ib25ndGFvLg0KDQo=

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:55 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472
Subject: Re: [PATCH V4 2/3] powerpc/swiotlb: Enable at early stage and
disable if not necessary
=20
=20
On Aug 2, 2012, at 6:42 AM, Jia Hongtao wrote:
=20

quoted

Remove the dependency on PCI initialization for SWIOTLB initialization.
So that PCI can be initialized at proper time.

SWIOTLB is partly determined by PCI inbound/outbound map which is

assigned

quoted

in PCI initialization. But swiotlb_init() should be done at the stage

of

quoted

mem_init() which is much earlier than PCI initialization. So we reserve

the

quoted

memory for SWIOTLB first and free it if not necessary.

All boards are converted to fit this change.

Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---

=20
This doesn't seem like it addresses our issue w/regards to not being able
to map all of memory from PCI.

PCI init will determine ppc_swiotlb_enable due to PCI map. swiotlb_late_ini=
t
will handle all swiotlb things depend on the result of pci init.

=20
=20

quoted

arch/powerpc/include/asm/swiotlb.h       |    6 ++++++
arch/powerpc/kernel/dma-swiotlb.c        |   20 ++++++++++++++++++++
arch/powerpc/mm/mem.c                    |    3 +--
arch/powerpc/platforms/44x/currituck.c   |   10 ++--------
arch/powerpc/platforms/85xx/mpc85xx_ds.c |    1 +
arch/powerpc/platforms/85xx/qemu_e500.c  |    2 +-
arch/powerpc/sysdev/fsl_pci.c            |    5 +----
7 files changed, 32 insertions(+), 15 deletions(-)

=20
Don't we also want to update all these:
=20
arch/powerpc/platforms/85xx/corenet_ds.c:
ppc_swiotlb_enable =3D 1;
arch/powerpc/platforms/85xx/ge_imp3a.c:         ppc_swiotlb_enable =3D 1;
arch/powerpc/platforms/85xx/mpc8536_ds.c:
ppc_swiotlb_enable =3D 1;
arch/powerpc/platforms/85xx/mpc85xx_mds.c:
ppc_swiotlb_enable =3D 1;
arch/powerpc/platforms/85xx/p1022_ds.c:         ppc_swiotlb_enable =3D 1;
arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:
ppc_swiotlb_enable =3D 1;
=20

They are works fine at this point.

I will update all these boards after
[PATCH 3/3] Unify pci/pcie initialization code

=20

quoted

diff --git a/arch/powerpc/include/asm/swiotlb.h

b/arch/powerpc/include/asm/swiotlb.h

quoted

index 8979d4c..de99d6e 100644

--- a/arch/powerpc/include/asm/swiotlb.h
+++ b/arch/powerpc/include/asm/swiotlb.h

@@ -22,4 +22,10 @@ int __init swiotlb_setup_bus_notifier(void);

extern void pci_dma_dev_setup_swiotlb(struct pci_dev *pdev);

+#ifdef CONFIG_SWIOTLB
+void swiotlb_detect_4g(void);
+#else
+static inline void swiotlb_detect_4g(void) {}
+#endif
+
#endif /* __ASM_SWIOTLB_H */

diff --git a/arch/powerpc/kernel/dma-swiotlb.c

b/arch/powerpc/kernel/dma-swiotlb.c

quoted

index 4ab88da..aa85550 100644

--- a/arch/powerpc/kernel/dma-swiotlb.c
+++ b/arch/powerpc/kernel/dma-swiotlb.c

@@ -104,3 +104,23 @@ int __init swiotlb_setup_bus_notifier(void)

			      &ppc_swiotlb_plat_bus_notifier);
	return 0;
}
+
+void swiotlb_detect_4g(void)
+{
+	if ((memblock_end_of_DRAM() - 1) > 0xffffffff)
+		ppc_swiotlb_enable =3D 1;
+}
+
+static int __init swiotlb_late_init(void)
+{
+	if (ppc_swiotlb_enable) {
+		swiotlb_print_info();
+		set_pci_dma_ops(&swiotlb_dma_ops);
+		ppc_md.pci_dma_dev_setup =3D pci_dma_dev_setup_swiotlb;
+	} else {
+		swiotlb_free();
+	}
+
+	return 0;
+}
+subsys_initcall(swiotlb_late_init);

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index baaafde..f23c4e0 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c

@@ -300,8 +300,7 @@ void __init mem_init(void)

	unsigned long reservedpages =3D 0, codesize, initsize, datasize,

bsssize;

quoted

#ifdef CONFIG_SWIOTLB
-	if (ppc_swiotlb_enable)
-		swiotlb_init(1);
+	swiotlb_init(0);
#endif

	num_physpages =3D memblock_phys_mem_size() >> PAGE_SHIFT;

diff --git a/arch/powerpc/platforms/44x/currituck.c

b/arch/powerpc/platforms/44x/currituck.c

quoted

index 9f6c33d..6bd89a0 100644

--- a/arch/powerpc/platforms/44x/currituck.c
+++ b/arch/powerpc/platforms/44x/currituck.c

@@ -21,7 +21,6 @@
 */

#include <linux/init.h>
-#include <linux/memblock.h>
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/rtc.h>

@@ -159,13 +158,8 @@ static void __init ppc47x_setup_arch(void)

	/* No need to check the DMA config as we /know/ our windows are all

of

quoted

 	 * RAM.  Lets hope that doesn't change */
-#ifdef CONFIG_SWIOTLB
-	if ((memblock_end_of_DRAM() - 1) > 0xffffffff) {
-		ppc_swiotlb_enable =3D 1;
-		set_pci_dma_ops(&swiotlb_dma_ops);
-		ppc_md.pci_dma_dev_setup =3D pci_dma_dev_setup_swiotlb;
-	}
-#endif
+	swiotlb_detect_4g();
+
	ppc47x_smp_init();
}

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c

b/arch/powerpc/platforms/85xx/mpc85xx_ds.c

quoted

index 6d3265f..56f8c8f 100644

--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c

@@ -159,6 +159,7 @@ static void __init mpc85xx_ds_setup_arch(void)

	if (ppc_md.progress)
		ppc_md.progress("mpc85xx_ds_setup_arch()", 0);

+	swiotlb_detect_4g();
	mpc85xx_ds_pci_init();
	mpc85xx_smp_init();

diff --git a/arch/powerpc/platforms/85xx/qemu_e500.c

b/arch/powerpc/platforms/85xx/qemu_e500.c

quoted

index 95a2e53..04260cd 100644

--- a/arch/powerpc/platforms/85xx/qemu_e500.c
+++ b/arch/powerpc/platforms/85xx/qemu_e500.c

@@ -41,7 +41,7 @@ static void __init qemu_e500_setup_arch(void)

{
	ppc_md.progress("qemu_e500_setup_arch()", 0);

-	fsl_pci_init();
+	swiotlb_detect_4g();

=20
removing fsl_pci_init() seems wrong.

You are right. I will fix this.
Thanks.

-Hongtao.

=20

quoted

	mpc85xx_smp_init();
}

diff --git a/arch/powerpc/sysdev/fsl_pci.c

b/arch/powerpc/sysdev/fsl_pci.c

quoted

index 6938792..da7a3d7 100644

--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c

@@ -872,11 +872,8 @@ void __devinit fsl_pci_init(void)

	 * we need SWIOTLB to handle buffers located outside of
	 * dma capable memory region
	 */
-	if (memblock_end_of_DRAM() - 1 > max) {
+	if (memblock_end_of_DRAM() - 1 > max)
		ppc_swiotlb_enable =3D 1;
-		set_pci_dma_ops(&swiotlb_dma_ops);
-		ppc_md.pci_dma_dev_setup =3D pci_dma_dev_setup_swiotlb;
-	}
#endif
}
#endif
--
1.7.5.1

=20

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:24 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code
=20
=20
On Aug 2, 2012, at 6:42 AM, Jia Hongtao wrote:
=20

quoted

We unified the Freescale pci/pcie initialization by changing the

fsl_pci

quoted

to a platform driver. In previous PCI code architecture the

initialization

quoted

routine is called at board_setup_arch stage. Now the initialization is

done

quoted

in probe function which is architectural better. Also It's convenient

for

quoted

adding PM support for PCI controller in later patch.

Now we registered pci controllers as platform devices. So we combine

two

quoted

initialization code as one platform driver.

Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
Signed-off-by: Chunhe Lan <redacted>
---
arch/powerpc/platforms/85xx/mpc85xx_ds.c |   32 ++--------
arch/powerpc/sysdev/fsl_pci.c            |  102 ++++++++++++++++++-----

------

quoted

arch/powerpc/sysdev/fsl_pci.h            |    6 +-
drivers/edac/mpc85xx_edac.c              |   43 ++++---------
4 files changed, 83 insertions(+), 100 deletions(-)

=20
You need to convert all boards to use fsl_pci_init before this patch.
Otherwise we'll end up with PCI getting initialized twice on boards.
=20
- k

If we covert all boards with platform driver in this patch PCI will
be initialized only once without converting all boards to use
fsl_pci_init first.=20

If we convert all boards to use fsl_pci_init before this patch and
convert them to use platform driver again after this patch. Then
between this patch and next pci will be initialized twice too.

So I think convert all boards in this patch is the key not convert
all boards to use fsl_pci_init first before this patch.

-Hongtao.

On Aug 2, 2012, at 9:21 PM, Jia Hongtao-B38951 wrote:

quoted

=20
-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:55 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472
Subject: Re: [PATCH V4 2/3] powerpc/swiotlb: Enable at early stage =

and

quoted

disable if not necessary
=20
=20
On Aug 2, 2012, at 6:42 AM, Jia Hongtao wrote:
=20

quoted

Remove the dependency on PCI initialization for SWIOTLB =

initialization.

quoted

quoted

So that PCI can be initialized at proper time.
=20
SWIOTLB is partly determined by PCI inbound/outbound map which is

assigned

quoted

in PCI initialization. But swiotlb_init() should be done at the =

stage

quoted

of

quoted

mem_init() which is much earlier than PCI initialization. So we =

reserve

quoted

the

quoted

memory for SWIOTLB first and free it if not necessary.
=20
All boards are converted to fit this change.
=20
Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---

=20
This doesn't seem like it addresses our issue w/regards to not being =

able

quoted

to map all of memory from PCI.

=20
PCI init will determine ppc_swiotlb_enable due to PCI map. =

swiotlb_late_init

will handle all swiotlb things depend on the result of pci init.

Think about the case that we have 4095M of memory & 1G of PCI memory =
mapped space.  The old code would enable swiotlb for this case since we =
would NOT be able to DMA to all 4095M of memory.  The patch does not =
handle this case correctly.

- k

On Fri, Aug 3, 2012 at 8:38 PM, Kumar Gala [off-list ref] wrote:

On Aug 2, 2012, at 9:21 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:55 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472
Subject: Re: [PATCH V4 2/3] powerpc/swiotlb: Enable at early stage and
disable if not necessary


On Aug 2, 2012, at 6:42 AM, Jia Hongtao wrote:

quoted

Remove the dependency on PCI initialization for SWIOTLB initialization.
So that PCI can be initialized at proper time.

SWIOTLB is partly determined by PCI inbound/outbound map which is

assigned

quoted

in PCI initialization. But swiotlb_init() should be done at the stage

of

quoted

mem_init() which is much earlier than PCI initialization. So we reserve

the

quoted

memory for SWIOTLB first and free it if not necessary.

All boards are converted to fit this change.

Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---

This doesn't seem like it addresses our issue w/regards to not being able
to map all of memory from PCI.

PCI init will determine ppc_swiotlb_enable due to PCI map. swiotlb_late_init
will handle all swiotlb things depend on the result of pci init.

Think about the case that we have 4095M of memory & 1G of PCI memory mapped space.  The old code would enable swiotlb for this case since we would NOT be able to DMA to all 4095M of memory.  The patch does not handle this case correctly.

The patch can handle it.  The ppc_swiotlb_enable is still being set in
 fsl_pci_init() if there is 1G of PCI memory mapped space.  It is
after next patch that the ppc_swiotlb_enable is being set in the PCI
probe() routine.

Leo

On 08/02/2012 10:39 PM, Jia Hongtao-B38951 wrote:

quoted

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:24 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

You need to convert all boards to use fsl_pci_init before this patch.
Otherwise we'll end up with PCI getting initialized twice on boards.

- k

If we covert all boards with platform driver in this patch PCI will
be initialized only once without converting all boards to use
fsl_pci_init first. 

Then we'd have to pick apart core changes from board changes when reviewing.

If we convert all boards to use fsl_pci_init before this patch and
convert them to use platform driver again after this patch. Then
between this patch and next pci will be initialized twice too.

Why?  That one patch should both create the platform driver and remove
the init from fsl_pci_init() -- except things like primary bus detection
which has to happen globally.

-Scott

On 08/02/2012 09:20 PM, Jia Hongtao-B38951 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Friday, August 03, 2012 4:19 AM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood Scott-
B07421; Li Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

Why are you eliminating the uli lookup?  We don't want to call
uli_exclude_device on boards that don't have a uli.

-Scott

I found out that all 85xx_ds boards (mpc8572ds, mpc8544ds, p2020ds) have
ULI.

Perhaps all boards currently handled by this file do, but it's not true
for all mpc85xx_ds boards.  I think we could handle mpc8536ds here if we
didn't hardcode the uli assumption.  Not critical, but would be nice to
make this more device tree driven.

p1022ds OTOH is weird enough that it deserves its own board file.

Also in platform driver fsl_pci_primary is determined at arch_initcall
which means at the stage of board_setup_arch fsl_pci_primary is not ready.

You could export the primary detection function so that boards can call
it early if they want.

-Scott

On Aug 3, 2012, at 9:42 AM, Li Yang wrote:

On Fri, Aug 3, 2012 at 8:38 PM, Kumar Gala [off-list ref] =

wrote:

quoted

=20
On Aug 2, 2012, at 9:21 PM, Jia Hongtao-B38951 wrote:
=20

quoted

quoted

=20
-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:55 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li =

Yang-R58472

quoted

quoted

quoted

Subject: Re: [PATCH V4 2/3] powerpc/swiotlb: Enable at early stage =

and

quoted

quoted

quoted

disable if not necessary
=20
=20
On Aug 2, 2012, at 6:42 AM, Jia Hongtao wrote:
=20

quoted

Remove the dependency on PCI initialization for SWIOTLB =

initialization.

quoted

quoted

quoted

quoted

So that PCI can be initialized at proper time.
=20
SWIOTLB is partly determined by PCI inbound/outbound map which is

assigned

quoted

in PCI initialization. But swiotlb_init() should be done at the =

stage

quoted

quoted

quoted

of

quoted

mem_init() which is much earlier than PCI initialization. So we =

reserve

quoted

quoted

quoted

the

quoted

memory for SWIOTLB first and free it if not necessary.
=20
All boards are converted to fit this change.
=20
Signed-off-by: Jia Hongtao <redacted>
Signed-off-by: Li Yang <redacted>
---

=20
This doesn't seem like it addresses our issue w/regards to not =

being able

quoted

quoted

quoted

to map all of memory from PCI.

=20
PCI init will determine ppc_swiotlb_enable due to PCI map. =

swiotlb_late_init

quoted

quoted

will handle all swiotlb things depend on the result of pci init.

=20
Think about the case that we have 4095M of memory & 1G of PCI memory =

mapped space.  The old code would enable swiotlb for this case since we =
would NOT be able to DMA to all 4095M of memory.  The patch does not =
handle this case correctly.

=20
The patch can handle it.  The ppc_swiotlb_enable is still being set in
fsl_pci_init() if there is 1G of PCI memory mapped space.  It is
after next patch that the ppc_swiotlb_enable is being set in the PCI
probe() routine.
=20
Leo

Gotcha.  I was thinking the swiotlb_init(0) was:

if (ppc_swiotlb_enable)
	swiotlb_init(0)

Now I see, we call it unconditionally, than fixup in swiotlb_late_init

- k=

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogU2F0dXJkYXksIEF1Z3VzdCAwNCwgMjAxMiAxMjowNCBBTQ0KPiBUbzogSmlh
IEhvbmd0YW8tQjM4OTUxDQo+IENjOiBLdW1hciBHYWxhOyBsaW51eHBwYy1kZXZAbGlzdHMub3ps
YWJzLm9yZzsgV29vZCBTY290dC1CMDc0MjE7IExpDQo+IFlhbmctUjU4NDcyDQo+IFN1YmplY3Q6
IFJlOiBbUEFUQ0ggVjQgMy8zXSBwb3dlcnBjL2ZzbC1wY2k6IFVuaWZ5IHBjaS9wY2llDQo+IGlu
aXRpYWxpemF0aW9uIGNvZGUNCj4gDQo+IE9uIDA4LzAyLzIwMTIgMTA6MzkgUE0sIEppYSBIb25n
dGFvLUIzODk1MSB3cm90ZToNCj4gPg0KPiA+DQo+ID4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0t
LS0tDQo+ID4+IEZyb206IEt1bWFyIEdhbGEgW21haWx0bzpnYWxha0BrZXJuZWwuY3Jhc2hpbmcu
b3JnXQ0KPiA+PiBTZW50OiBUaHVyc2RheSwgQXVndXN0IDAyLCAyMDEyIDg6MjQgUE0NCj4gPj4g
VG86IEppYSBIb25ndGFvLUIzODk1MQ0KPiA+PiBDYzogbGludXhwcGMtZGV2QGxpc3RzLm96bGFi
cy5vcmc7IFdvb2QgU2NvdHQtQjA3NDIxOyBMaSBZYW5nLVI1ODQ3Mg0KPiA+PiBTdWJqZWN0OiBS
ZTogW1BBVENIIFY0IDMvM10gcG93ZXJwYy9mc2wtcGNpOiBVbmlmeSBwY2kvcGNpZQ0KPiA+PiBp
bml0aWFsaXphdGlvbiBjb2RlDQo+ID4+DQo+ID4+IFlvdSBuZWVkIHRvIGNvbnZlcnQgYWxsIGJv
YXJkcyB0byB1c2UgZnNsX3BjaV9pbml0IGJlZm9yZSB0aGlzIHBhdGNoLg0KPiA+PiBPdGhlcndp
c2Ugd2UnbGwgZW5kIHVwIHdpdGggUENJIGdldHRpbmcgaW5pdGlhbGl6ZWQgdHdpY2Ugb24gYm9h
cmRzLg0KPiA+Pg0KPiA+PiAtIGsNCj4gPg0KPiA+IElmIHdlIGNvdmVydCBhbGwgYm9hcmRzIHdp
dGggcGxhdGZvcm0gZHJpdmVyIGluIHRoaXMgcGF0Y2ggUENJIHdpbGwNCj4gPiBiZSBpbml0aWFs
aXplZCBvbmx5IG9uY2Ugd2l0aG91dCBjb252ZXJ0aW5nIGFsbCBib2FyZHMgdG8gdXNlDQo+ID4g
ZnNsX3BjaV9pbml0IGZpcnN0Lg0KPiANCj4gVGhlbiB3ZSdkIGhhdmUgdG8gcGljayBhcGFydCBj
b3JlIGNoYW5nZXMgZnJvbSBib2FyZCBjaGFuZ2VzIHdoZW4NCj4gcmV2aWV3aW5nLg0KPiANCj4g
PiBJZiB3ZSBjb252ZXJ0IGFsbCBib2FyZHMgdG8gdXNlIGZzbF9wY2lfaW5pdCBiZWZvcmUgdGhp
cyBwYXRjaCBhbmQNCj4gPiBjb252ZXJ0IHRoZW0gdG8gdXNlIHBsYXRmb3JtIGRyaXZlciBhZ2Fp
biBhZnRlciB0aGlzIHBhdGNoLiBUaGVuDQo+ID4gYmV0d2VlbiB0aGlzIHBhdGNoIGFuZCBuZXh0
IHBjaSB3aWxsIGJlIGluaXRpYWxpemVkIHR3aWNlIHRvby4NCj4gDQo+IFdoeT8gIFRoYXQgb25l
IHBhdGNoIHNob3VsZCBib3RoIGNyZWF0ZSB0aGUgcGxhdGZvcm0gZHJpdmVyIGFuZCByZW1vdmUN
Cj4gdGhlIGluaXQgZnJvbSBmc2xfcGNpX2luaXQoKSAtLSBleGNlcHQgdGhpbmdzIGxpa2UgcHJp
bWFyeSBidXMgZGV0ZWN0aW9uDQo+IHdoaWNoIGhhcyB0byBoYXBwZW4gZ2xvYmFsbHkuDQo+IA0K
PiAtU2NvdHQNCg0KIk9uZSBwYXRjaCBib3RoIGNyZWF0ZSB0aGUgcGxhdGZvcm0gZHJpdmVyIGFu
ZCByZW1vdmUgdGhlIGluaXQgZnJvbQ0KZnNsX3BjaV9pbml0KCkiIG1lYW5zIHdlIHNob3VsZCBj
cmVhdGUgcGxhdGZvcm0gZHJpdmVyIGFuZCBhcHBsaWVkIHRvDQphbGwgYm9hcmRzLiBJZiBzbyB3
aHkgbm90IGp1c3QgZGlyZWN0bHkgY29udmVydCBhbGwgYm9hcmRzIHVzaW5nIHBsYXRmb3JtDQpk
cml2ZXI/IE5vdyBpbiB0aGlzIG9uZSBwYXRjaCB3ZSBjcmVhdGUgcGxhdGZvcm0gZHJpdmVyIGFu
ZCByZW1vdmUNCmZzbF9wY2lfaW5pdCgpIGZyb20gdGhlIGJvYXJkcyB0aGF0IHVzaW5nIGl0IGFu
ZCBjb252ZXJ0IGFsbCBvdGhlciBib2FyZHMNCnRoYXQgc3RpbGwgdXNpbmcgb2xkIHdheXMuIEkg
dGhpbmsgaXQncyBjbGVhciBhbmQgcmlnaHQuDQoNCi1Ib25ndGFvLg0K

On Fri, Aug 3, 2012 at 11:09 AM, Scott Wood [off-list ref] wrote=
:

p1022ds OTOH is weird enough that it deserves its own board file.

What's so weird about the P1022DS?

Also, why do we need a default PCI bus if one isn't specified in the
device tree?

--=20
Timur Tabi
Linux kernel developer at Freescale=

On 08/05/2012 11:11 PM, Tabi Timur-B04825 wrote:

On Fri, Aug 3, 2012 at 11:09 AM, Scott Wood [off-list ref] wrote:

quoted

p1022ds OTOH is weird enough that it deserves its own board file.

What's so weird about the P1022DS?

The localbus muxing.

Also, why do we need a default PCI bus if one isn't specified in the
device tree?

Because there are bugs in the Linux PPC PCI code when you don't have a
primary bus -- see recent upstream linuxppc-dev discussion.

-Scott

On 08/05/2012 09:39 PM, Jia Hongtao-B38951 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, August 04, 2012 12:04 AM
To: Jia Hongtao-B38951
Cc: Kumar Gala; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/02/2012 10:39 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:24 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

You need to convert all boards to use fsl_pci_init before this patch.
Otherwise we'll end up with PCI getting initialized twice on boards.

- k

If we covert all boards with platform driver in this patch PCI will
be initialized only once without converting all boards to use
fsl_pci_init first.

Then we'd have to pick apart core changes from board changes when
reviewing.

quoted

If we convert all boards to use fsl_pci_init before this patch and
convert them to use platform driver again after this patch. Then
between this patch and next pci will be initialized twice too.

Why?  That one patch should both create the platform driver and remove
the init from fsl_pci_init() -- except things like primary bus detection
which has to happen globally.

-Scott

"One patch both create the platform driver and remove the init from
fsl_pci_init()" means we should create platform driver and applied to
all boards. If so why not just directly convert all boards using platform
driver?

Because it's harder to review when you have a bunch of board code in the
patch in addition to core changes.

Because you might want people to actually test on the boards in question
when converting, especially given the change in how primary buses are
determined, and that some boards may need to provide their own alternative.

-Scott

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0K
PiBTZW50OiBNb25kYXksIEF1Z3VzdCAwNiwgMjAxMiAxMToxNiBQTQ0KPiBUbzogSmlhIEhvbmd0
YW8tQjM4OTUxDQo+IENjOiBXb29kIFNjb3R0LUIwNzQyMTsgS3VtYXIgR2FsYTsgbGludXhwcGMt
ZGV2QGxpc3RzLm96bGFicy5vcmc7IExpDQo+IFlhbmctUjU4NDcyDQo+IFN1YmplY3Q6IFJlOiBb
UEFUQ0ggVjQgMy8zXSBwb3dlcnBjL2ZzbC1wY2k6IFVuaWZ5IHBjaS9wY2llDQo+IGluaXRpYWxp
emF0aW9uIGNvZGUNCj4gDQo+IE9uIDA4LzA1LzIwMTIgMDk6MzkgUE0sIEppYSBIb25ndGFvLUIz
ODk1MSB3cm90ZToNCj4gPg0KPiA+DQo+ID4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+
ID4+IEZyb206IFdvb2QgU2NvdHQtQjA3NDIxDQo+ID4+IFNlbnQ6IFNhdHVyZGF5LCBBdWd1c3Qg
MDQsIDIwMTIgMTI6MDQgQU0NCj4gPj4gVG86IEppYSBIb25ndGFvLUIzODk1MQ0KPiA+PiBDYzog
S3VtYXIgR2FsYTsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7IFdvb2QgU2NvdHQtQjA3
NDIxOyBMaQ0KPiA+PiBZYW5nLVI1ODQ3Mg0KPiA+PiBTdWJqZWN0OiBSZTogW1BBVENIIFY0IDMv
M10gcG93ZXJwYy9mc2wtcGNpOiBVbmlmeSBwY2kvcGNpZQ0KPiA+PiBpbml0aWFsaXphdGlvbiBj
b2RlDQo+ID4+DQo+ID4+IE9uIDA4LzAyLzIwMTIgMTA6MzkgUE0sIEppYSBIb25ndGFvLUIzODk1
MSB3cm90ZToNCj4gPj4+DQo+ID4+Pg0KPiA+Pj4+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0t
DQo+ID4+Pj4gRnJvbTogS3VtYXIgR2FsYSBbbWFpbHRvOmdhbGFrQGtlcm5lbC5jcmFzaGluZy5v
cmddDQo+ID4+Pj4gU2VudDogVGh1cnNkYXksIEF1Z3VzdCAwMiwgMjAxMiA4OjI0IFBNDQo+ID4+
Pj4gVG86IEppYSBIb25ndGFvLUIzODk1MQ0KPiA+Pj4+IENjOiBsaW51eHBwYy1kZXZAbGlzdHMu
b3psYWJzLm9yZzsgV29vZCBTY290dC1CMDc0MjE7IExpDQo+ID4+Pj4gWWFuZy1SNTg0NzINCj4g
Pj4+PiBTdWJqZWN0OiBSZTogW1BBVENIIFY0IDMvM10gcG93ZXJwYy9mc2wtcGNpOiBVbmlmeSBw
Y2kvcGNpZQ0KPiA+Pj4+IGluaXRpYWxpemF0aW9uIGNvZGUNCj4gPj4+Pg0KPiA+Pj4+IFlvdSBu
ZWVkIHRvIGNvbnZlcnQgYWxsIGJvYXJkcyB0byB1c2UgZnNsX3BjaV9pbml0IGJlZm9yZSB0aGlz
IHBhdGNoLg0KPiA+Pj4+IE90aGVyd2lzZSB3ZSdsbCBlbmQgdXAgd2l0aCBQQ0kgZ2V0dGluZyBp
bml0aWFsaXplZCB0d2ljZSBvbiBib2FyZHMuDQo+ID4+Pj4NCj4gPj4+PiAtIGsNCj4gPj4+DQo+
ID4+PiBJZiB3ZSBjb3ZlcnQgYWxsIGJvYXJkcyB3aXRoIHBsYXRmb3JtIGRyaXZlciBpbiB0aGlz
IHBhdGNoIFBDSSB3aWxsDQo+ID4+PiBiZSBpbml0aWFsaXplZCBvbmx5IG9uY2Ugd2l0aG91dCBj
b252ZXJ0aW5nIGFsbCBib2FyZHMgdG8gdXNlDQo+ID4+PiBmc2xfcGNpX2luaXQgZmlyc3QuDQo+
ID4+DQo+ID4+IFRoZW4gd2UnZCBoYXZlIHRvIHBpY2sgYXBhcnQgY29yZSBjaGFuZ2VzIGZyb20g
Ym9hcmQgY2hhbmdlcyB3aGVuDQo+ID4+IHJldmlld2luZy4NCj4gPj4NCj4gPj4+IElmIHdlIGNv
bnZlcnQgYWxsIGJvYXJkcyB0byB1c2UgZnNsX3BjaV9pbml0IGJlZm9yZSB0aGlzIHBhdGNoIGFu
ZA0KPiA+Pj4gY29udmVydCB0aGVtIHRvIHVzZSBwbGF0Zm9ybSBkcml2ZXIgYWdhaW4gYWZ0ZXIg
dGhpcyBwYXRjaC4gVGhlbg0KPiA+Pj4gYmV0d2VlbiB0aGlzIHBhdGNoIGFuZCBuZXh0IHBjaSB3
aWxsIGJlIGluaXRpYWxpemVkIHR3aWNlIHRvby4NCj4gPj4NCj4gPj4gV2h5PyAgVGhhdCBvbmUg
cGF0Y2ggc2hvdWxkIGJvdGggY3JlYXRlIHRoZSBwbGF0Zm9ybSBkcml2ZXIgYW5kDQo+ID4+IHJl
bW92ZSB0aGUgaW5pdCBmcm9tIGZzbF9wY2lfaW5pdCgpIC0tIGV4Y2VwdCB0aGluZ3MgbGlrZSBw
cmltYXJ5IGJ1cw0KPiA+PiBkZXRlY3Rpb24gd2hpY2ggaGFzIHRvIGhhcHBlbiBnbG9iYWxseS4N
Cj4gPj4NCj4gPj4gLVNjb3R0DQo+ID4NCj4gPiAiT25lIHBhdGNoIGJvdGggY3JlYXRlIHRoZSBw
bGF0Zm9ybSBkcml2ZXIgYW5kIHJlbW92ZSB0aGUgaW5pdCBmcm9tDQo+ID4gZnNsX3BjaV9pbml0
KCkiIG1lYW5zIHdlIHNob3VsZCBjcmVhdGUgcGxhdGZvcm0gZHJpdmVyIGFuZCBhcHBsaWVkIHRv
DQo+ID4gYWxsIGJvYXJkcy4gSWYgc28gd2h5IG5vdCBqdXN0IGRpcmVjdGx5IGNvbnZlcnQgYWxs
IGJvYXJkcyB1c2luZw0KPiA+IHBsYXRmb3JtIGRyaXZlcj8NCj4gDQo+IEJlY2F1c2UgaXQncyBo
YXJkZXIgdG8gcmV2aWV3IHdoZW4geW91IGhhdmUgYSBidW5jaCBvZiBib2FyZCBjb2RlIGluIHRo
ZQ0KPiBwYXRjaCBpbiBhZGRpdGlvbiB0byBjb3JlIGNoYW5nZXMuDQo+IA0KPiBCZWNhdXNlIHlv
dSBtaWdodCB3YW50IHBlb3BsZSB0byBhY3R1YWxseSB0ZXN0IG9uIHRoZSBib2FyZHMgaW4gcXVl
c3Rpb24NCj4gd2hlbiBjb252ZXJ0aW5nLCBlc3BlY2lhbGx5IGdpdmVuIHRoZSBjaGFuZ2UgaW4g
aG93IHByaW1hcnkgYnVzZXMgYXJlDQo+IGRldGVybWluZWQsIGFuZCB0aGF0IHNvbWUgYm9hcmRz
IG1heSBuZWVkIHRvIHByb3ZpZGUgdGhlaXIgb3duDQo+IGFsdGVybmF0aXZlLg0KPiANCj4gLVNj
b3R0DQoNCkJ1dCBpZiB3ZSBzZXBhcmF0ZSB0aGUgY29yZSBjaGFuZ2VzIGFuZCB0aGUgYm9hcmRz
IHVwZGF0ZSwgYmV0d2VlbiB0aGlzIHR3bw0KcGF0Y2hlcyBQQ0kgd2lsbCBiZSBpbml0aWFsaXpl
ZCB0d2ljZS4NCg0KLUhvbmd0YW8uDQoNCg==

On 08/07/2012 01:23 AM, Jia Hongtao-B38951 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Monday, August 06, 2012 11:16 PM
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421; Kumar Gala; linuxppc-dev@lists.ozlabs.org; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/05/2012 09:39 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, August 04, 2012 12:04 AM
To: Jia Hongtao-B38951
Cc: Kumar Gala; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/02/2012 10:39 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:24 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

You need to convert all boards to use fsl_pci_init before this patch.
Otherwise we'll end up with PCI getting initialized twice on boards.

- k

If we covert all boards with platform driver in this patch PCI will
be initialized only once without converting all boards to use
fsl_pci_init first.

Then we'd have to pick apart core changes from board changes when
reviewing.

quoted

If we convert all boards to use fsl_pci_init before this patch and
convert them to use platform driver again after this patch. Then
between this patch and next pci will be initialized twice too.

Why?  That one patch should both create the platform driver and
remove the init from fsl_pci_init() -- except things like primary bus
detection which has to happen globally.

-Scott

"One patch both create the platform driver and remove the init from
fsl_pci_init()" means we should create platform driver and applied to
all boards. If so why not just directly convert all boards using
platform driver?

Because it's harder to review when you have a bunch of board code in the
patch in addition to core changes.

Because you might want people to actually test on the boards in question
when converting, especially given the change in how primary buses are
determined, and that some boards may need to provide their own
alternative.

-Scott

But if we separate the core changes and the boards update, between this two
patches PCI will be initialized twice.

As I said earlier, you can remove the initcall and require boards to
manually call fsl_pci_init() until all boards are converted.

-Scott

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVHVlc2RheSwgQXVndXN0IDA3LCAyMDEyIDExOjIwIFBNDQo+IFRvOiBKaWEg
SG9uZ3Rhby1CMzg5NTENCj4gQ2M6IFdvb2QgU2NvdHQtQjA3NDIxOyBLdW1hciBHYWxhOyBsaW51
eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgTGkNCj4gWWFuZy1SNTg0NzINCj4gU3ViamVjdDog
UmU6IFtQQVRDSCBWNCAzLzNdIHBvd2VycGMvZnNsLXBjaTogVW5pZnkgcGNpL3BjaWUNCj4gaW5p
dGlhbGl6YXRpb24gY29kZQ0KPiANCj4gT24gMDgvMDcvMjAxMiAwMToyMyBBTSwgSmlhIEhvbmd0
YW8tQjM4OTUxIHdyb3RlOg0KPiA+PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+PiBG
cm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0KPiA+PiBTZW50OiBNb25kYXksIEF1Z3VzdCAwNiwgMjAx
MiAxMToxNiBQTQ0KPiA+PiBUbzogSmlhIEhvbmd0YW8tQjM4OTUxDQo+ID4+IENjOiBXb29kIFNj
b3R0LUIwNzQyMTsgS3VtYXIgR2FsYTsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7IExp
DQo+ID4+IFlhbmctUjU4NDcyDQo+ID4+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggVjQgMy8zXSBwb3dl
cnBjL2ZzbC1wY2k6IFVuaWZ5IHBjaS9wY2llDQo+ID4+IGluaXRpYWxpemF0aW9uIGNvZGUNCj4g
Pj4NCj4gPj4gT24gMDgvMDUvMjAxMiAwOTozOSBQTSwgSmlhIEhvbmd0YW8tQjM4OTUxIHdyb3Rl
Og0KPiA+Pj4NCj4gPj4+DQo+ID4+Pj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPj4+
PiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0KPiA+Pj4+IFNlbnQ6IFNhdHVyZGF5LCBBdWd1c3Qg
MDQsIDIwMTIgMTI6MDQgQU0NCj4gPj4+PiBUbzogSmlhIEhvbmd0YW8tQjM4OTUxDQo+ID4+Pj4g
Q2M6IEt1bWFyIEdhbGE7IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBXb29kIFNjb3R0
LUIwNzQyMTsgTGkNCj4gPj4+PiBZYW5nLVI1ODQ3Mg0KPiA+Pj4+IFN1YmplY3Q6IFJlOiBbUEFU
Q0ggVjQgMy8zXSBwb3dlcnBjL2ZzbC1wY2k6IFVuaWZ5IHBjaS9wY2llDQo+ID4+Pj4gaW5pdGlh
bGl6YXRpb24gY29kZQ0KPiA+Pj4+DQo+ID4+Pj4gT24gMDgvMDIvMjAxMiAxMDozOSBQTSwgSmlh
IEhvbmd0YW8tQjM4OTUxIHdyb3RlOg0KPiA+Pj4+Pg0KPiA+Pj4+Pg0KPiA+Pj4+Pj4gLS0tLS1P
cmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPj4+Pj4+IEZyb206IEt1bWFyIEdhbGEgW21haWx0bzpn
YWxha0BrZXJuZWwuY3Jhc2hpbmcub3JnXQ0KPiA+Pj4+Pj4gU2VudDogVGh1cnNkYXksIEF1Z3Vz
dCAwMiwgMjAxMiA4OjI0IFBNDQo+ID4+Pj4+PiBUbzogSmlhIEhvbmd0YW8tQjM4OTUxDQo+ID4+
Pj4+PiBDYzogbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7IFdvb2QgU2NvdHQtQjA3NDIx
OyBMaQ0KPiA+Pj4+Pj4gWWFuZy1SNTg0NzINCj4gPj4+Pj4+IFN1YmplY3Q6IFJlOiBbUEFUQ0gg
VjQgMy8zXSBwb3dlcnBjL2ZzbC1wY2k6IFVuaWZ5IHBjaS9wY2llDQo+ID4+Pj4+PiBpbml0aWFs
aXphdGlvbiBjb2RlDQo+ID4+Pj4+Pg0KPiA+Pj4+Pj4gWW91IG5lZWQgdG8gY29udmVydCBhbGwg
Ym9hcmRzIHRvIHVzZSBmc2xfcGNpX2luaXQgYmVmb3JlIHRoaXMNCj4gcGF0Y2guDQo+ID4+Pj4+
PiBPdGhlcndpc2Ugd2UnbGwgZW5kIHVwIHdpdGggUENJIGdldHRpbmcgaW5pdGlhbGl6ZWQgdHdp
Y2Ugb24NCj4gYm9hcmRzLg0KPiA+Pj4+Pj4NCj4gPj4+Pj4+IC0gaw0KPiA+Pj4+Pg0KPiA+Pj4+
PiBJZiB3ZSBjb3ZlcnQgYWxsIGJvYXJkcyB3aXRoIHBsYXRmb3JtIGRyaXZlciBpbiB0aGlzIHBh
dGNoIFBDSSB3aWxsDQo+ID4+Pj4+IGJlIGluaXRpYWxpemVkIG9ubHkgb25jZSB3aXRob3V0IGNv
bnZlcnRpbmcgYWxsIGJvYXJkcyB0byB1c2UNCj4gPj4+Pj4gZnNsX3BjaV9pbml0IGZpcnN0Lg0K
PiA+Pj4+DQo+ID4+Pj4gVGhlbiB3ZSdkIGhhdmUgdG8gcGljayBhcGFydCBjb3JlIGNoYW5nZXMg
ZnJvbSBib2FyZCBjaGFuZ2VzIHdoZW4NCj4gPj4+PiByZXZpZXdpbmcuDQo+ID4+Pj4NCj4gPj4+
Pj4gSWYgd2UgY29udmVydCBhbGwgYm9hcmRzIHRvIHVzZSBmc2xfcGNpX2luaXQgYmVmb3JlIHRo
aXMgcGF0Y2ggYW5kDQo+ID4+Pj4+IGNvbnZlcnQgdGhlbSB0byB1c2UgcGxhdGZvcm0gZHJpdmVy
IGFnYWluIGFmdGVyIHRoaXMgcGF0Y2guIFRoZW4NCj4gPj4+Pj4gYmV0d2VlbiB0aGlzIHBhdGNo
IGFuZCBuZXh0IHBjaSB3aWxsIGJlIGluaXRpYWxpemVkIHR3aWNlIHRvby4NCj4gPj4+Pg0KPiA+
Pj4+IFdoeT8gIFRoYXQgb25lIHBhdGNoIHNob3VsZCBib3RoIGNyZWF0ZSB0aGUgcGxhdGZvcm0g
ZHJpdmVyIGFuZA0KPiA+Pj4+IHJlbW92ZSB0aGUgaW5pdCBmcm9tIGZzbF9wY2lfaW5pdCgpIC0t
IGV4Y2VwdCB0aGluZ3MgbGlrZSBwcmltYXJ5DQo+IGJ1cw0KPiA+Pj4+IGRldGVjdGlvbiB3aGlj
aCBoYXMgdG8gaGFwcGVuIGdsb2JhbGx5Lg0KPiA+Pj4+DQo+ID4+Pj4gLVNjb3R0DQo+ID4+Pg0K
PiA+Pj4gIk9uZSBwYXRjaCBib3RoIGNyZWF0ZSB0aGUgcGxhdGZvcm0gZHJpdmVyIGFuZCByZW1v
dmUgdGhlIGluaXQgZnJvbQ0KPiA+Pj4gZnNsX3BjaV9pbml0KCkiIG1lYW5zIHdlIHNob3VsZCBj
cmVhdGUgcGxhdGZvcm0gZHJpdmVyIGFuZCBhcHBsaWVkIHRvDQo+ID4+PiBhbGwgYm9hcmRzLiBJ
ZiBzbyB3aHkgbm90IGp1c3QgZGlyZWN0bHkgY29udmVydCBhbGwgYm9hcmRzIHVzaW5nDQo+ID4+
PiBwbGF0Zm9ybSBkcml2ZXI/DQo+ID4+DQo+ID4+IEJlY2F1c2UgaXQncyBoYXJkZXIgdG8gcmV2
aWV3IHdoZW4geW91IGhhdmUgYSBidW5jaCBvZiBib2FyZCBjb2RlIGluDQo+IHRoZQ0KPiA+PiBw
YXRjaCBpbiBhZGRpdGlvbiB0byBjb3JlIGNoYW5nZXMuDQo+ID4+DQo+ID4+IEJlY2F1c2UgeW91
IG1pZ2h0IHdhbnQgcGVvcGxlIHRvIGFjdHVhbGx5IHRlc3Qgb24gdGhlIGJvYXJkcyBpbg0KPiBx
dWVzdGlvbg0KPiA+PiB3aGVuIGNvbnZlcnRpbmcsIGVzcGVjaWFsbHkgZ2l2ZW4gdGhlIGNoYW5n
ZSBpbiBob3cgcHJpbWFyeSBidXNlcyBhcmUNCj4gPj4gZGV0ZXJtaW5lZCwgYW5kIHRoYXQgc29t
ZSBib2FyZHMgbWF5IG5lZWQgdG8gcHJvdmlkZSB0aGVpciBvd24NCj4gPj4gYWx0ZXJuYXRpdmUu
DQo+ID4+DQo+ID4+IC1TY290dA0KPiA+DQo+ID4gQnV0IGlmIHdlIHNlcGFyYXRlIHRoZSBjb3Jl
IGNoYW5nZXMgYW5kIHRoZSBib2FyZHMgdXBkYXRlLCBiZXR3ZWVuIHRoaXMNCj4gdHdvDQo+ID4g
cGF0Y2hlcyBQQ0kgd2lsbCBiZSBpbml0aWFsaXplZCB0d2ljZS4NCj4gDQo+IEFzIEkgc2FpZCBl
YXJsaWVyLCB5b3UgY2FuIHJlbW92ZSB0aGUgaW5pdGNhbGwgYW5kIHJlcXVpcmUgYm9hcmRzIHRv
DQo+IG1hbnVhbGx5IGNhbGwgZnNsX3BjaV9pbml0KCkgdW50aWwgYWxsIGJvYXJkcyBhcmUgY29u
dmVydGVkLg0KPiANCj4gLVNjb3R0DQoNCkFzIEkgc2FpZCBlYXJsaWVyLCBJIGNhbiBkbyB0aGlz
IGJ1dCBpdCBkb2VzIG5vdCBzb2x2ZSB0aGUgdHdpY2UtaW5pdCBwcm9ibGVtLg0KSWYgSSBkbyB0
aGlzIGZpcnN0IGFuZCB0aGVuIGFkZCBwbGF0Zm9ybSBkcml2ZXIgd2UgYWxzbyBoYXZlIHRvIGNv
bnZlcnQgYWxsDQpib2FyZHMgdXNpbmcgcGxhdGZvcm0gZHJpdmVyIGluIHRoZSBzYW1lIHBhdGNo
Lg0KDQpXZSBmaW5hbGx5IHVzaW5nIHRoZSBwbGF0Zm9ybSBkcml2ZXIgc28gV2h5IGRvIHlvdSBr
ZWVwIGluc2lzdGluZyBvbiBjb252ZXJ0aW5nDQphbGwgYm9hcmRzIHVzaW5nIGZzbF9wY2lfaW5p
dCgpIGZpcnN0IGV2ZW4gaXQgZG9lcyBubyBpbXByb3ZlbWVudC4NCg0KLUhvbmd0YW8uDQoNCg==

On 08/07/2012 10:57 PM, Jia Hongtao-B38951 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Tuesday, August 07, 2012 11:20 PM
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421; Kumar Gala; linuxppc-dev@lists.ozlabs.org; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/07/2012 01:23 AM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Monday, August 06, 2012 11:16 PM
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421; Kumar Gala; linuxppc-dev@lists.ozlabs.org; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/05/2012 09:39 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, August 04, 2012 12:04 AM
To: Jia Hongtao-B38951
Cc: Kumar Gala; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/02/2012 10:39 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:24 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

You need to convert all boards to use fsl_pci_init before this

patch.

quoted

quoted

quoted

quoted

quoted

quoted

Otherwise we'll end up with PCI getting initialized twice on

boards.

quoted

quoted

quoted

quoted

quoted

quoted

- k

If we covert all boards with platform driver in this patch PCI will
be initialized only once without converting all boards to use
fsl_pci_init first.

Then we'd have to pick apart core changes from board changes when
reviewing.

quoted

If we convert all boards to use fsl_pci_init before this patch and
convert them to use platform driver again after this patch. Then
between this patch and next pci will be initialized twice too.

Why?  That one patch should both create the platform driver and
remove the init from fsl_pci_init() -- except things like primary

bus

quoted

quoted

quoted

quoted

detection which has to happen globally.

-Scott

"One patch both create the platform driver and remove the init from
fsl_pci_init()" means we should create platform driver and applied to
all boards. If so why not just directly convert all boards using
platform driver?

Because it's harder to review when you have a bunch of board code in

the

quoted

quoted

patch in addition to core changes.

Because you might want people to actually test on the boards in

question

quoted

quoted

when converting, especially given the change in how primary buses are
determined, and that some boards may need to provide their own
alternative.

-Scott

But if we separate the core changes and the boards update, between this

two

quoted

patches PCI will be initialized twice.

As I said earlier, you can remove the initcall and require boards to
manually call fsl_pci_init() until all boards are converted.

-Scott

As I said earlier, I can do this but it does not solve the twice-init problem.

I must have missed it.  Why does it not solve the problem?  If a board
doesn't call fsl_pci_init(), the platform driver doesn't get registered.

If I do this first and then add platform driver we also have to convert all
boards using platform driver in the same patch.

We finally using the platform driver so Why do you keep insisting on converting
all boards using fsl_pci_init() first even it does no improvement.

What we're asking for is bisectability (don't have any intermediate
stages where PCI gets initialized twice), and the ability to have a
smooth transition where boards can be converted as people are able to
test them and look into their individual needs regarding primary bus.

-Scott

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogV2VkbmVzZGF5LCBBdWd1c3QgMDgsIDIwMTIgMTE6NTQgUE0NCj4gVG86IEpp
YSBIb25ndGFvLUIzODk1MQ0KPiBDYzogV29vZCBTY290dC1CMDc0MjE7IEt1bWFyIEdhbGE7IGxp
bnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBMaQ0KPiBZYW5nLVI1ODQ3Mg0KPiBTdWJqZWN0
OiBSZTogW1BBVENIIFY0IDMvM10gcG93ZXJwYy9mc2wtcGNpOiBVbmlmeSBwY2kvcGNpZQ0KPiBp
bml0aWFsaXphdGlvbiBjb2RlDQo+IA0KPiBPbiAwOC8wNy8yMDEyIDEwOjU3IFBNLCBKaWEgSG9u
Z3Rhby1CMzg5NTEgd3JvdGU6DQo+ID4NCj4gPg0KPiA+PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2Ut
LS0tLQ0KPiA+PiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0KPiA+PiBTZW50OiBUdWVzZGF5LCBB
dWd1c3QgMDcsIDIwMTIgMTE6MjAgUE0NCj4gPj4gVG86IEppYSBIb25ndGFvLUIzODk1MQ0KPiA+
PiBDYzogV29vZCBTY290dC1CMDc0MjE7IEt1bWFyIEdhbGE7IGxpbnV4cHBjLWRldkBsaXN0cy5v
emxhYnMub3JnOyBMaQ0KPiA+PiBZYW5nLVI1ODQ3Mg0KPiA+PiBTdWJqZWN0OiBSZTogW1BBVENI
IFY0IDMvM10gcG93ZXJwYy9mc2wtcGNpOiBVbmlmeSBwY2kvcGNpZQ0KPiA+PiBpbml0aWFsaXph
dGlvbiBjb2RlDQo+ID4+DQo+ID4+IE9uIDA4LzA3LzIwMTIgMDE6MjMgQU0sIEppYSBIb25ndGFv
LUIzODk1MSB3cm90ZToNCj4gPj4+PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+Pj4+
IEZyb206IFdvb2QgU2NvdHQtQjA3NDIxDQo+ID4+Pj4gU2VudDogTW9uZGF5LCBBdWd1c3QgMDYs
IDIwMTIgMTE6MTYgUE0NCj4gPj4+PiBUbzogSmlhIEhvbmd0YW8tQjM4OTUxDQo+ID4+Pj4gQ2M6
IFdvb2QgU2NvdHQtQjA3NDIxOyBLdW1hciBHYWxhOyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJz
Lm9yZzsNCj4gPj4+PiBMaQ0KPiA+Pj4+IFlhbmctUjU4NDcyDQo+ID4+Pj4gU3ViamVjdDogUmU6
IFtQQVRDSCBWNCAzLzNdIHBvd2VycGMvZnNsLXBjaTogVW5pZnkgcGNpL3BjaWUNCj4gPj4+PiBp
bml0aWFsaXphdGlvbiBjb2RlDQo+ID4+Pj4NCj4gPj4+PiBPbiAwOC8wNS8yMDEyIDA5OjM5IFBN
LCBKaWEgSG9uZ3Rhby1CMzg5NTEgd3JvdGU6DQo+ID4+Pj4+DQo+ID4+Pj4+DQo+ID4+Pj4+PiAt
LS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+Pj4+Pj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gPj4+Pj4+IFNlbnQ6IFNhdHVyZGF5LCBBdWd1c3QgMDQsIDIwMTIgMTI6MDQgQU0NCj4g
Pj4+Pj4+IFRvOiBKaWEgSG9uZ3Rhby1CMzg5NTENCj4gPj4+Pj4+IENjOiBLdW1hciBHYWxhOyBs
aW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgV29vZCBTY290dC1CMDc0MjE7DQo+ID4+Pj4+
PiBMaQ0KPiA+Pj4+Pj4gWWFuZy1SNTg0NzINCj4gPj4+Pj4+IFN1YmplY3Q6IFJlOiBbUEFUQ0gg
VjQgMy8zXSBwb3dlcnBjL2ZzbC1wY2k6IFVuaWZ5IHBjaS9wY2llDQo+ID4+Pj4+PiBpbml0aWFs
aXphdGlvbiBjb2RlDQo+ID4+Pj4+Pg0KPiA+Pj4+Pj4gT24gMDgvMDIvMjAxMiAxMDozOSBQTSwg
SmlhIEhvbmd0YW8tQjM4OTUxIHdyb3RlOg0KPiA+Pj4+Pj4+DQo+ID4+Pj4+Pj4NCj4gPj4+Pj4+
Pj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPj4+Pj4+Pj4gRnJvbTogS3VtYXIgR2Fs
YSBbbWFpbHRvOmdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmddDQo+ID4+Pj4+Pj4+IFNlbnQ6IFRo
dXJzZGF5LCBBdWd1c3QgMDIsIDIwMTIgODoyNCBQTQ0KPiA+Pj4+Pj4+PiBUbzogSmlhIEhvbmd0
YW8tQjM4OTUxDQo+ID4+Pj4+Pj4+IENjOiBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsg
V29vZCBTY290dC1CMDc0MjE7IExpDQo+ID4+Pj4+Pj4+IFlhbmctUjU4NDcyDQo+ID4+Pj4+Pj4+
IFN1YmplY3Q6IFJlOiBbUEFUQ0ggVjQgMy8zXSBwb3dlcnBjL2ZzbC1wY2k6IFVuaWZ5IHBjaS9w
Y2llDQo+ID4+Pj4+Pj4+IGluaXRpYWxpemF0aW9uIGNvZGUNCj4gPj4+Pj4+Pj4NCj4gPj4+Pj4+
Pj4gWW91IG5lZWQgdG8gY29udmVydCBhbGwgYm9hcmRzIHRvIHVzZSBmc2xfcGNpX2luaXQgYmVm
b3JlIHRoaXMNCj4gPj4gcGF0Y2guDQo+ID4+Pj4+Pj4+IE90aGVyd2lzZSB3ZSdsbCBlbmQgdXAg
d2l0aCBQQ0kgZ2V0dGluZyBpbml0aWFsaXplZCB0d2ljZSBvbg0KPiA+PiBib2FyZHMuDQo+ID4+
Pj4+Pj4+DQo+ID4+Pj4+Pj4+IC0gaw0KPiA+Pj4+Pj4+DQo+ID4+Pj4+Pj4gSWYgd2UgY292ZXJ0
IGFsbCBib2FyZHMgd2l0aCBwbGF0Zm9ybSBkcml2ZXIgaW4gdGhpcyBwYXRjaCBQQ0kNCj4gPj4+
Pj4+PiB3aWxsIGJlIGluaXRpYWxpemVkIG9ubHkgb25jZSB3aXRob3V0IGNvbnZlcnRpbmcgYWxs
IGJvYXJkcyB0bw0KPiA+Pj4+Pj4+IHVzZSBmc2xfcGNpX2luaXQgZmlyc3QuDQo+ID4+Pj4+Pg0K
PiA+Pj4+Pj4gVGhlbiB3ZSdkIGhhdmUgdG8gcGljayBhcGFydCBjb3JlIGNoYW5nZXMgZnJvbSBi
b2FyZCBjaGFuZ2VzIHdoZW4NCj4gPj4+Pj4+IHJldmlld2luZy4NCj4gPj4+Pj4+DQo+ID4+Pj4+
Pj4gSWYgd2UgY29udmVydCBhbGwgYm9hcmRzIHRvIHVzZSBmc2xfcGNpX2luaXQgYmVmb3JlIHRo
aXMgcGF0Y2gNCj4gPj4+Pj4+PiBhbmQgY29udmVydCB0aGVtIHRvIHVzZSBwbGF0Zm9ybSBkcml2
ZXIgYWdhaW4gYWZ0ZXIgdGhpcyBwYXRjaC4NCj4gPj4+Pj4+PiBUaGVuIGJldHdlZW4gdGhpcyBw
YXRjaCBhbmQgbmV4dCBwY2kgd2lsbCBiZSBpbml0aWFsaXplZCB0d2ljZQ0KPiB0b28uDQo+ID4+
Pj4+Pg0KPiA+Pj4+Pj4gV2h5PyAgVGhhdCBvbmUgcGF0Y2ggc2hvdWxkIGJvdGggY3JlYXRlIHRo
ZSBwbGF0Zm9ybSBkcml2ZXIgYW5kDQo+ID4+Pj4+PiByZW1vdmUgdGhlIGluaXQgZnJvbSBmc2xf
cGNpX2luaXQoKSAtLSBleGNlcHQgdGhpbmdzIGxpa2UgcHJpbWFyeQ0KPiA+PiBidXMNCj4gPj4+
Pj4+IGRldGVjdGlvbiB3aGljaCBoYXMgdG8gaGFwcGVuIGdsb2JhbGx5Lg0KPiA+Pj4+Pj4NCj4g
Pj4+Pj4+IC1TY290dA0KPiA+Pj4+Pg0KPiA+Pj4+PiAiT25lIHBhdGNoIGJvdGggY3JlYXRlIHRo
ZSBwbGF0Zm9ybSBkcml2ZXIgYW5kIHJlbW92ZSB0aGUgaW5pdA0KPiA+Pj4+PiBmcm9tIGZzbF9w
Y2lfaW5pdCgpIiBtZWFucyB3ZSBzaG91bGQgY3JlYXRlIHBsYXRmb3JtIGRyaXZlciBhbmQNCj4g
Pj4+Pj4gYXBwbGllZCB0byBhbGwgYm9hcmRzLiBJZiBzbyB3aHkgbm90IGp1c3QgZGlyZWN0bHkg
Y29udmVydCBhbGwNCj4gPj4+Pj4gYm9hcmRzIHVzaW5nIHBsYXRmb3JtIGRyaXZlcj8NCj4gPj4+
Pg0KPiA+Pj4+IEJlY2F1c2UgaXQncyBoYXJkZXIgdG8gcmV2aWV3IHdoZW4geW91IGhhdmUgYSBi
dW5jaCBvZiBib2FyZCBjb2RlDQo+ID4+Pj4gaW4NCj4gPj4gdGhlDQo+ID4+Pj4gcGF0Y2ggaW4g
YWRkaXRpb24gdG8gY29yZSBjaGFuZ2VzLg0KPiA+Pj4+DQo+ID4+Pj4gQmVjYXVzZSB5b3UgbWln
aHQgd2FudCBwZW9wbGUgdG8gYWN0dWFsbHkgdGVzdCBvbiB0aGUgYm9hcmRzIGluDQo+ID4+IHF1
ZXN0aW9uDQo+ID4+Pj4gd2hlbiBjb252ZXJ0aW5nLCBlc3BlY2lhbGx5IGdpdmVuIHRoZSBjaGFu
Z2UgaW4gaG93IHByaW1hcnkgYnVzZXMNCj4gPj4+PiBhcmUgZGV0ZXJtaW5lZCwgYW5kIHRoYXQg
c29tZSBib2FyZHMgbWF5IG5lZWQgdG8gcHJvdmlkZSB0aGVpciBvd24NCj4gPj4+PiBhbHRlcm5h
dGl2ZS4NCj4gPj4+Pg0KPiA+Pj4+IC1TY290dA0KPiA+Pj4NCj4gPj4+IEJ1dCBpZiB3ZSBzZXBh
cmF0ZSB0aGUgY29yZSBjaGFuZ2VzIGFuZCB0aGUgYm9hcmRzIHVwZGF0ZSwgYmV0d2Vlbg0KPiA+
Pj4gdGhpcw0KPiA+PiB0d28NCj4gPj4+IHBhdGNoZXMgUENJIHdpbGwgYmUgaW5pdGlhbGl6ZWQg
dHdpY2UuDQo+ID4+DQo+ID4+IEFzIEkgc2FpZCBlYXJsaWVyLCB5b3UgY2FuIHJlbW92ZSB0aGUg
aW5pdGNhbGwgYW5kIHJlcXVpcmUgYm9hcmRzIHRvDQo+ID4+IG1hbnVhbGx5IGNhbGwgZnNsX3Bj
aV9pbml0KCkgdW50aWwgYWxsIGJvYXJkcyBhcmUgY29udmVydGVkLg0KPiA+Pg0KPiA+PiAtU2Nv
dHQNCj4gPg0KPiA+IEFzIEkgc2FpZCBlYXJsaWVyLCBJIGNhbiBkbyB0aGlzIGJ1dCBpdCBkb2Vz
IG5vdCBzb2x2ZSB0aGUgdHdpY2UtaW5pdA0KPiBwcm9ibGVtLg0KPiANCj4gSSBtdXN0IGhhdmUg
bWlzc2VkIGl0LiAgV2h5IGRvZXMgaXQgbm90IHNvbHZlIHRoZSBwcm9ibGVtPyAgSWYgYSBib2Fy
ZA0KPiBkb2Vzbid0IGNhbGwgZnNsX3BjaV9pbml0KCksIHRoZSBwbGF0Zm9ybSBkcml2ZXIgZG9l
c24ndCBnZXQgcmVnaXN0ZXJlZC4NCj4gDQo+ID4gSWYgSSBkbyB0aGlzIGZpcnN0IGFuZCB0aGVu
IGFkZCBwbGF0Zm9ybSBkcml2ZXIgd2UgYWxzbyBoYXZlIHRvDQo+ID4gY29udmVydCBhbGwgYm9h
cmRzIHVzaW5nIHBsYXRmb3JtIGRyaXZlciBpbiB0aGUgc2FtZSBwYXRjaC4NCj4gPg0KPiA+IFdl
IGZpbmFsbHkgdXNpbmcgdGhlIHBsYXRmb3JtIGRyaXZlciBzbyBXaHkgZG8geW91IGtlZXAgaW5z
aXN0aW5nIG9uDQo+ID4gY29udmVydGluZyBhbGwgYm9hcmRzIHVzaW5nIGZzbF9wY2lfaW5pdCgp
IGZpcnN0IGV2ZW4gaXQgZG9lcyBubw0KPiBpbXByb3ZlbWVudC4NCj4gDQo+IFdoYXQgd2UncmUg
YXNraW5nIGZvciBpcyBiaXNlY3RhYmlsaXR5IChkb24ndCBoYXZlIGFueSBpbnRlcm1lZGlhdGUN
Cj4gc3RhZ2VzIHdoZXJlIFBDSSBnZXRzIGluaXRpYWxpemVkIHR3aWNlKSwgYW5kIHRoZSBhYmls
aXR5IHRvIGhhdmUgYQ0KPiBzbW9vdGggdHJhbnNpdGlvbiB3aGVyZSBib2FyZHMgY2FuIGJlIGNv
bnZlcnRlZCBhcyBwZW9wbGUgYXJlIGFibGUgdG8NCj4gdGVzdCB0aGVtIGFuZCBsb29rIGludG8g
dGhlaXIgaW5kaXZpZHVhbCBuZWVkcyByZWdhcmRpbmcgcHJpbWFyeSBidXMuDQo+IA0KPiAtU2Nv
dHQNCg0KSW4gbXkgcGF0Y2ggdGhlcmUgaXMgbm8gYmlzZWN0YWJpbGl0eSBwcm9ibGVtLiBJZiB5
b3UgZG9uJ3QgdGhpbmsgc28gY291bGQNCnlvdSBwbGVhc2UgZ2l2ZSBtb3JlIGRldGFpbHM/DQoN
Ci1Ib25ndGFvLg0K

On 08/08/2012 10:52 PM, Jia Hongtao-B38951 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, August 08, 2012 11:54 PM
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421; Kumar Gala; linuxppc-dev@lists.ozlabs.org; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/07/2012 10:57 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Tuesday, August 07, 2012 11:20 PM
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421; Kumar Gala; linuxppc-dev@lists.ozlabs.org; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/07/2012 01:23 AM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Monday, August 06, 2012 11:16 PM
To: Jia Hongtao-B38951
Cc: Wood Scott-B07421; Kumar Gala; linuxppc-dev@lists.ozlabs.org;
Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/05/2012 09:39 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, August 04, 2012 12:04 AM
To: Jia Hongtao-B38951
Cc: Kumar Gala; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421;
Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

On 08/02/2012 10:39 PM, Jia Hongtao-B38951 wrote:

quoted

quoted

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Thursday, August 02, 2012 8:24 PM
To: Jia Hongtao-B38951
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Li
Yang-R58472
Subject: Re: [PATCH V4 3/3] powerpc/fsl-pci: Unify pci/pcie
initialization code

You need to convert all boards to use fsl_pci_init before this

patch.

quoted

quoted

quoted

quoted

quoted

quoted

Otherwise we'll end up with PCI getting initialized twice on

boards.

quoted

quoted

quoted

quoted

quoted

quoted

- k

If we covert all boards with platform driver in this patch PCI
will be initialized only once without converting all boards to
use fsl_pci_init first.

Then we'd have to pick apart core changes from board changes when
reviewing.

quoted

If we convert all boards to use fsl_pci_init before this patch
and convert them to use platform driver again after this patch.
Then between this patch and next pci will be initialized twice

too.

quoted

quoted

quoted

quoted

quoted

quoted

Why?  That one patch should both create the platform driver and
remove the init from fsl_pci_init() -- except things like primary

bus

quoted

quoted

quoted

quoted

detection which has to happen globally.

-Scott

"One patch both create the platform driver and remove the init
from fsl_pci_init()" means we should create platform driver and
applied to all boards. If so why not just directly convert all
boards using platform driver?

Because it's harder to review when you have a bunch of board code
in

the

quoted

quoted

patch in addition to core changes.

Because you might want people to actually test on the boards in

question

quoted

quoted

when converting, especially given the change in how primary buses
are determined, and that some boards may need to provide their own
alternative.

-Scott

But if we separate the core changes and the boards update, between
this

two

quoted

patches PCI will be initialized twice.

As I said earlier, you can remove the initcall and require boards to
manually call fsl_pci_init() until all boards are converted.

-Scott

As I said earlier, I can do this but it does not solve the twice-init

problem.

I must have missed it.  Why does it not solve the problem?  If a board
doesn't call fsl_pci_init(), the platform driver doesn't get registered.

quoted

If I do this first and then add platform driver we also have to
convert all boards using platform driver in the same patch.

We finally using the platform driver so Why do you keep insisting on
converting all boards using fsl_pci_init() first even it does no

improvement.

What we're asking for is bisectability (don't have any intermediate
stages where PCI gets initialized twice), and the ability to have a
smooth transition where boards can be converted as people are able to
test them and look into their individual needs regarding primary bus.

-Scott

In my patch there is no bisectability problem. If you don't think so could
you please give more details?

You are registering the PCI platform device with an initcall, but you
haven't updated the unconverted boards to not do the init themselves --
and your proposal to fix that breaks the "smooth transition" request.

-Scott