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

From
Scott Wood <hidden>
Date
2012-07-25 17:23:58
Reply to
Jia Hongtao-B38951, 2012-07-25
On 07/24/2012 09:35 PM, Jia Hongtao-B38951 wrote:
quoted
-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, July 25, 2012 2:43 AM
To: Jia Hongtao-B38951
Cc: <redacted>; <redacted>; 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:
quoted
We unified the Freescale pci/pcie initialization by changing the
fsl_pci
quoted
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?
quoted
+#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.
quoted
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
Thread (36 messages) 36 messages, 4 authors, last reply 2012-08-10
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help