| Message ID | 1343125210-16720-1-git-send-email-B38951@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kumar Gala |
| Headers | show |
On 07/24/2012 05:20 AM, Jia Hongtao wrote: > We unified the Freescale pci/pcie initialization by changing the fsl_pci > to a platform driver. > > In previous version pci/pcie initialization is in platform code which > Initialize pci bridge base on EP/RC or host/agent settings. The previous version of what? This patch, or the PCI code? What changed in this patch since the last time you sent it, and where is the version number? > +#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) > +static const struct of_device_id pci_ids[] = { > + { .compatible = "fsl,mpc8540-pci", }, > + { .compatible = "fsl,mpc8548-pcie", }, > + { .compatible = "fsl,mpc8641-pcie", }, > + { .compatible = "fsl,p1022-pcie", }, > + { .compatible = "fsl,p1010-pcie", }, > + { .compatible = "fsl,p1023-pcie", }, > + { .compatible = "fsl,p4080-pcie", }, > + { .compatible = "fsl,qoriq-pcie-v2.3", }, > + { .compatible = "fsl,qoriq-pcie-v2.2", }, > + {}, > +}; Again, please base this on the latest tree, which has my PCI patches. This table already exists in this file. And you're still missing fsl,mpc8610-pci. > +int primary_phb_addr; > +static int __devinit fsl_pci_probe(struct platform_device *pdev) > +{ > + struct pci_controller *hose; > + bool is_primary; > + > + if (of_match_node(pci_ids, pdev->dev.of_node)) { > + struct resource rsrc; > + of_address_to_resource(pdev->dev.of_node, 0, &rsrc); > + is_primary = ((rsrc.start & 0xfffff) == primary_phb_addr); > + fsl_add_bridge(pdev->dev.of_node, is_primary); > + > +#ifdef CONFIG_SWIOTLB > + 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() > hose->dma_window_base_cur > + + hose->dma_window_size) { > + ppc_swiotlb_enable = 1; > + set_pci_dma_ops(&swiotlb_dma_ops); > + ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb; > + } > +#endif > + } It's too late for swiotlb here. Again, please don't break something in one patch and then fix it in a later patch. Use "git rebase -i" to edit your patchset into a reviewable, bisectable form. -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Wednesday, July 25, 2012 2:43 AM > To: Jia Hongtao-B38951 > Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood Scott- > B07421; Li Yang-R58472 > Subject: Re: [PATCH 1/6] powerpc/fsl-pci: Unify pci/pcie initialization > code > > On 07/24/2012 05:20 AM, Jia Hongtao wrote: > > We unified the Freescale pci/pcie initialization by changing the > fsl_pci > > to a platform driver. > > > > In previous version pci/pcie initialization is in platform code which > > Initialize pci bridge base on EP/RC or host/agent settings. > > The previous version of what? This patch, or the PCI code? What > changed in this patch since the last time you sent it, and where is the > version number? > > > +#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) > > +static const struct of_device_id pci_ids[] = { > > + { .compatible = "fsl,mpc8540-pci", }, > > + { .compatible = "fsl,mpc8548-pcie", }, > > + { .compatible = "fsl,mpc8641-pcie", }, > > + { .compatible = "fsl,p1022-pcie", }, > > + { .compatible = "fsl,p1010-pcie", }, > > + { .compatible = "fsl,p1023-pcie", }, > > + { .compatible = "fsl,p4080-pcie", }, > > + { .compatible = "fsl,qoriq-pcie-v2.3", }, > > + { .compatible = "fsl,qoriq-pcie-v2.2", }, > > + {}, > > +}; > > Again, please base this on the latest tree, which has my PCI patches. > This table already exists in this file. And you're still missing > fsl,mpc8610-pci. Sorry fsl,mpc8610-pci will be added. > > > +int primary_phb_addr; > > +static int __devinit fsl_pci_probe(struct platform_device *pdev) > > +{ > > + struct pci_controller *hose; > > + bool is_primary; > > + > > + if (of_match_node(pci_ids, pdev->dev.of_node)) { > > + struct resource rsrc; > > + of_address_to_resource(pdev->dev.of_node, 0, &rsrc); > > + is_primary = ((rsrc.start & 0xfffff) == primary_phb_addr); > > + fsl_add_bridge(pdev->dev.of_node, is_primary); > > + > > +#ifdef CONFIG_SWIOTLB > > + 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() > hose->dma_window_base_cur > > + + hose->dma_window_size) { > > + ppc_swiotlb_enable = 1; > > + set_pci_dma_ops(&swiotlb_dma_ops); > > + ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb; > > + } > > +#endif > > + } > > It's too late for swiotlb here. Again, please don't break something in > one patch and then fix it in a later patch. Use "git rebase -i" to edit > your patchset into a reviewable, bisectable form. > > -Scott Yes, bisectable requirement is sort of reasonable. But I check the SubmittingPatches Doc and it says "If one patch depends on another patch in order for a change to be complete, that is OK. Simply note 'this patch depends on patch X' in your patch description". In my opinion swiotlb is a whole functional patch so I separate them. Maybe I should add depends description in the next patch. About all this patch set Leo and I insist to make it as a platform driver which is architectural better. I didn't base this patch set on the latest tree and it's unapplicable just because I want to show the whole idea of this patchset. If the idea is ok for upstream I will rebase the patch set. -Hongtao.
On 07/24/2012 09:35 PM, Jia Hongtao-B38951 wrote: > > >> -----Original Message----- >> From: Wood Scott-B07421 >> Sent: Wednesday, July 25, 2012 2:43 AM >> To: Jia Hongtao-B38951 >> Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood Scott- >> B07421; Li Yang-R58472 >> Subject: Re: [PATCH 1/6] powerpc/fsl-pci: Unify pci/pcie initialization >> code >> >> On 07/24/2012 05:20 AM, Jia Hongtao wrote: >>> We unified the Freescale pci/pcie initialization by changing the >> fsl_pci >>> to a platform driver. >>> >>> In previous version pci/pcie initialization is in platform code which >>> Initialize pci bridge base on EP/RC or host/agent settings. >> >> The previous version of what? This patch, or the PCI code? What >> changed in this patch since the last time you sent it, and where is the >> version number? >> >>> +#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) >>> +static const struct of_device_id pci_ids[] = { >>> + { .compatible = "fsl,mpc8540-pci", }, >>> + { .compatible = "fsl,mpc8548-pcie", }, >>> + { .compatible = "fsl,mpc8641-pcie", }, >>> + { .compatible = "fsl,p1022-pcie", }, >>> + { .compatible = "fsl,p1010-pcie", }, >>> + { .compatible = "fsl,p1023-pcie", }, >>> + { .compatible = "fsl,p4080-pcie", }, >>> + { .compatible = "fsl,qoriq-pcie-v2.3", }, >>> + { .compatible = "fsl,qoriq-pcie-v2.2", }, >>> + {}, >>> +}; >> >> Again, please base this on the latest tree, which has my PCI patches. >> This table already exists in this file. And you're still missing >> fsl,mpc8610-pci. > > Sorry fsl,mpc8610-pci will be added. To what? The table is already there in Linus's tree, with fsl,mpc8610-pci. You don't need to add it again. >> It's too late for swiotlb here. Again, please don't break something in >> one patch and then fix it in a later patch. Use "git rebase -i" to edit >> your patchset into a reviewable, bisectable form. >> >> -Scott > > Yes, bisectable requirement is sort of reasonable. > > But I check the SubmittingPatches Doc and it says "If one patch depends on > another patch in order for a change to be complete, that is OK. Simply > note 'this patch depends on patch X' in your patch description". In my > opinion swiotlb is a whole functional patch so I separate them. Maybe > I should add depends description in the next patch. That's not what that means. What it means is that if someone else has already posted a patch, and your patch is supposed to go on top of that patch, you should mention that. > About all this patch set Leo and I insist to make it as a platform driver > which is architectural better. I didn't base this patch set on the latest > tree and it's unapplicable just because I want to show the whole idea of > this patchset. If the idea is ok for upstream I will rebase the patch set. If that's the case, you should label it as an [RFC PATCH] (stands for Request For Comments), and mention under the --- line any known issues, such as that it doesn't apply to the current tree. But it would be a lot easier to comment on it if it were based on the current code, rather than having to speculate what you'd do when you rebase. -Scott
Thanks for all your comments. I submit the V2 of this patch set which is based on the latest tree. Please have a review. Thanks -Hongtao. > -----Original Message----- > From: Wood Scott-B07421 > Sent: Thursday, July 26, 2012 1:24 AM > To: Jia Hongtao-B38951 > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; > galak@kernel.crashing.org; Li Yang-R58472 > Subject: Re: [PATCH 1/6] powerpc/fsl-pci: Unify pci/pcie initialization > code > > On 07/24/2012 09:35 PM, Jia Hongtao-B38951 wrote: > > > > > >> -----Original Message----- > >> From: Wood Scott-B07421 > >> Sent: Wednesday, July 25, 2012 2:43 AM > >> To: Jia Hongtao-B38951 > >> Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood > Scott- > >> B07421; Li Yang-R58472 > >> Subject: Re: [PATCH 1/6] powerpc/fsl-pci: Unify pci/pcie > initialization > >> code > >> > >> On 07/24/2012 05:20 AM, Jia Hongtao wrote: > >>> We unified the Freescale pci/pcie initialization by changing the > >> fsl_pci > >>> to a platform driver. > >>> > >>> In previous version pci/pcie initialization is in platform code which > >>> Initialize pci bridge base on EP/RC or host/agent settings. > >> > >> The previous version of what? This patch, or the PCI code? What > >> changed in this patch since the last time you sent it, and where is > the > >> version number? > >> > >>> +#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) > >>> +static const struct of_device_id pci_ids[] = { > >>> + { .compatible = "fsl,mpc8540-pci", }, > >>> + { .compatible = "fsl,mpc8548-pcie", }, > >>> + { .compatible = "fsl,mpc8641-pcie", }, > >>> + { .compatible = "fsl,p1022-pcie", }, > >>> + { .compatible = "fsl,p1010-pcie", }, > >>> + { .compatible = "fsl,p1023-pcie", }, > >>> + { .compatible = "fsl,p4080-pcie", }, > >>> + { .compatible = "fsl,qoriq-pcie-v2.3", }, > >>> + { .compatible = "fsl,qoriq-pcie-v2.2", }, > >>> + {}, > >>> +}; > >> > >> Again, please base this on the latest tree, which has my PCI patches. > >> This table already exists in this file. And you're still missing > >> fsl,mpc8610-pci. > > > > Sorry fsl,mpc8610-pci will be added. > > To what? The table is already there in Linus's tree, with > fsl,mpc8610-pci. You don't need to add it again. > > >> It's too late for swiotlb here. Again, please don't break something > in > >> one patch and then fix it in a later patch. Use "git rebase -i" to > edit > >> your patchset into a reviewable, bisectable form. > >> > >> -Scott > > > > Yes, bisectable requirement is sort of reasonable. > > > > But I check the SubmittingPatches Doc and it says "If one patch depends > on > > another patch in order for a change to be complete, that is OK. Simply > > note 'this patch depends on patch X' in your patch description". In my > > opinion swiotlb is a whole functional patch so I separate them. Maybe > > I should add depends description in the next patch. > > That's not what that means. What it means is that if someone else has > already posted a patch, and your patch is supposed to go on top of that > patch, you should mention that. > > > About all this patch set Leo and I insist to make it as a platform > driver > > which is architectural better. I didn't base this patch set on the > latest > > tree and it's unapplicable just because I want to show the whole idea > of > > this patchset. If the idea is ok for upstream I will rebase the patch > set. > > If that's the case, you should label it as an [RFC PATCH] (stands for > Request For Comments), and mention under the --- line any known issues, > such as that it doesn't apply to the current tree. > > But it would be a lot easier to comment on it if it were based on the > current code, rather than having to speculate what you'd do when you > rebase. > > -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Thursday, July 26, 2012 1:24 AM > To: Jia Hongtao-B38951 > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; > galak@kernel.crashing.org; Li Yang-R58472 > Subject: Re: [PATCH 1/6] powerpc/fsl-pci: Unify pci/pcie initialization > code > > On 07/24/2012 09:35 PM, Jia Hongtao-B38951 wrote: > > > > > >> -----Original Message----- > >> From: Wood Scott-B07421 > >> Sent: Wednesday, July 25, 2012 2:43 AM > >> To: Jia Hongtao-B38951 > >> Cc: linuxppc-dev@lists.ozlabs.org; galak@kernel.crashing.org; Wood > Scott- > >> B07421; Li Yang-R58472 > >> Subject: Re: [PATCH 1/6] powerpc/fsl-pci: Unify pci/pcie > initialization > >> code > >> > >> On 07/24/2012 05:20 AM, Jia Hongtao wrote: > >>> We unified the Freescale pci/pcie initialization by changing the > >> fsl_pci > >>> to a platform driver. > >>> > >>> In previous version pci/pcie initialization is in platform code which > >>> Initialize pci bridge base on EP/RC or host/agent settings. > >> > >> The previous version of what? This patch, or the PCI code? What > >> changed in this patch since the last time you sent it, and where is > the > >> version number? > >> > >>> +#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) > >>> +static const struct of_device_id pci_ids[] = { > >>> + { .compatible = "fsl,mpc8540-pci", }, > >>> + { .compatible = "fsl,mpc8548-pcie", }, > >>> + { .compatible = "fsl,mpc8641-pcie", }, > >>> + { .compatible = "fsl,p1022-pcie", }, > >>> + { .compatible = "fsl,p1010-pcie", }, > >>> + { .compatible = "fsl,p1023-pcie", }, > >>> + { .compatible = "fsl,p4080-pcie", }, > >>> + { .compatible = "fsl,qoriq-pcie-v2.3", }, > >>> + { .compatible = "fsl,qoriq-pcie-v2.2", }, > >>> + {}, > >>> +}; > >> > >> Again, please base this on the latest tree, which has my PCI patches. > >> This table already exists in this file. And you're still missing > >> fsl,mpc8610-pci. > > > > Sorry fsl,mpc8610-pci will be added. > > To what? The table is already there in Linus's tree, with > fsl,mpc8610-pci. You don't need to add it again. > > >> It's too late for swiotlb here. Again, please don't break something > in > >> one patch and then fix it in a later patch. Use "git rebase -i" to > edit > >> your patchset into a reviewable, bisectable form. > >> > >> -Scott > > > > Yes, bisectable requirement is sort of reasonable. > > > > But I check the SubmittingPatches Doc and it says "If one patch depends > on > > another patch in order for a change to be complete, that is OK. Simply > > note 'this patch depends on patch X' in your patch description". In my > > opinion swiotlb is a whole functional patch so I separate them. Maybe > > I should add depends description in the next patch. > > That's not what that means. What it means is that if someone else has > already posted a patch, and your patch is supposed to go on top of that > patch, you should mention that. > > > About all this patch set Leo and I insist to make it as a platform > driver > > which is architectural better. I didn't base this patch set on the > latest > > tree and it's unapplicable just because I want to show the whole idea > of > > this patchset. If the idea is ok for upstream I will rebase the patch > set. > > If that's the case, you should label it as an [RFC PATCH] (stands for > Request For Comments), and mention under the --- line any known issues, > such as that it doesn't apply to the current tree. > I will discuss with Leo and resubmit the patch if necessary. Thanks. -Hongtao.
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index edbf794..feed364 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -807,3 +807,63 @@ u64 fsl_pci_immrbar_base(struct pci_controller *hose) return 0; } + +#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) +static const struct of_device_id pci_ids[] = { + { .compatible = "fsl,mpc8540-pci", }, + { .compatible = "fsl,mpc8548-pcie", }, + { .compatible = "fsl,mpc8641-pcie", }, + { .compatible = "fsl,p1022-pcie", }, + { .compatible = "fsl,p1010-pcie", }, + { .compatible = "fsl,p1023-pcie", }, + { .compatible = "fsl,p4080-pcie", }, + { .compatible = "fsl,qoriq-pcie-v2.3", }, + { .compatible = "fsl,qoriq-pcie-v2.2", }, + {}, +}; + +int primary_phb_addr; +static int __devinit fsl_pci_probe(struct platform_device *pdev) +{ + struct pci_controller *hose; + bool is_primary; + + if (of_match_node(pci_ids, pdev->dev.of_node)) { + struct resource rsrc; + of_address_to_resource(pdev->dev.of_node, 0, &rsrc); + is_primary = ((rsrc.start & 0xfffff) == primary_phb_addr); + fsl_add_bridge(pdev->dev.of_node, is_primary); + +#ifdef CONFIG_SWIOTLB + 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() > hose->dma_window_base_cur + + hose->dma_window_size) { + ppc_swiotlb_enable = 1; + set_pci_dma_ops(&swiotlb_dma_ops); + ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb; + } +#endif + } + + 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 a39ed5c..df9fc44 100644 --- a/arch/powerpc/sysdev/fsl_pci.h +++ b/arch/powerpc/sysdev/fsl_pci.h @@ -88,6 +88,7 @@ struct ccsr_pci { __be32 pex_err_cap_r3; /* 0x.e34 - PCIE error capture register 0 */ }; +extern int primary_phb_addr; extern int fsl_add_bridge(struct device_node *dev, int is_primary); extern void fsl_pcibios_fixup_bus(struct pci_bus *bus); extern int mpc83xx_add_bridge(struct device_node *dev);