Thread (9 messages) 9 messages, 5 authors, 2013-10-09

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help