Re: [PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host
From: Scott Wood <hidden>
Date: 2013-10-08 23:20:55
Also in:
linux-pci
On Tue, 2013-10-08 at 17:09 -0600, Bjorn Helgaas wrote:
On Tue, Oct 8, 2013 at 4:46 PM, Scott Wood [off-list ref] wrote:quoted
On Tue, 2013-10-08 at 13:13 -0600, Bjorn Helgaas wrote:quoted
[+cc Ben, Paul, linuxppc-dev] On Mon, Sep 30, 2013 at 04:52:54PM +0800, Minghuan Lian wrote:quoted
The Freescale's Layerscape series processors will use ARM cores. The LS1's PCIe controllers is the same as T4240's. So it's better the PCIe controller driver can support PowerPC and ARM simultaneously. This patch is for this purpose. It derives the common functions from arch/powerpc/sysdev/fsl_pci.c to drivers/pci/host/pci-fsl-common.c and leaves the architecture specific functions which should be implemented in arch related files. Signed-off-by: Minghuan Lian <redacted>I cc'd the powerpc maintainers so we can work out which tree this should go through.quoted
--- change log: v1-v2: 1. rename pci.h to pci-common.h 2. rename pci-fsl.c to pci-fsl-common.c Based on upstream master. Based on the discussion of RFC version here http://patchwork.ozlabs.org/patch/274487/ arch/powerpc/sysdev/fsl_pci.c | 521 +----------------- arch/powerpc/sysdev/fsl_pci.h | 89 ---- .../fsl_pci.c => drivers/pci/host/pci-fsl-common.c | 591 +-------------------- .../fsl_pci.h => include/linux/fsl/pci-common.h | 45 +-Is there any way to avoid putting this file in include/linux? I know you want to share it beyond PowerPC, and I know there are similar examples there already, but this is all arch-specific or chipset-specific stuff that seems like it should be in some not-so-public place. It doesn't seem scalable to add an include/linux subdirectory for every chipset that might be shared across architectures.What specifically is the problem with it, as long as it's properly namespaced?Well, as I said above, it doesn't seem scalable,
I'm not sure what scaling problems you're picturing, assuming proper namespacing and organization within include/linux/.
and it doesn't seem to be the common existing practice. Possibly this is just because sharing chipsets across arches isn't very common yet. I hadn't noticed that include/linux/fsl exists already; I thought you were adding it. Given that it *does* exist already, I guess I'm OK with putting more stuff in it.
I see other existing practice as well. Besides plenty of "include/linux/fsl*" that ought to be moved to "include/linux/fsl/", I see things like include/linux/amba/, include/linux/scx200*, include/linux/clksrc-dbx500-prcmu.h, include/linux/com202020.h, etc. These are just a few random examples out of many.
So I'll apply these given an ack from the powerpc folks.
ACK this patch. The second one I'd like to see broken up into digestible chunks so I can better review it. -Scott