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;
> ga...@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; 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.
>
> 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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
