[PATCH 1/6] powerpc/fsl-pci: Unify pci/pcie initialization code

Jia Hongtao-B38951 B38951 at freescale.com
Wed Jul 25 12:35:23 EST 2012



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, July 25, 2012 2:43 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev at lists.ozlabs.org; galak at 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. 


More information about the Linuxppc-dev mailing list