> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, July 25, 2012 2:43 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev@lists.ozlabs.org; ga...@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.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
