[PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host

Scott Wood scottwood at freescale.com
Wed Oct 9 10:20:40 EST 2013


On Tue, 2013-10-08 at 17:09 -0600, Bjorn Helgaas wrote:
> On Tue, Oct 8, 2013 at 4:46 PM, Scott Wood <scottwood at freescale.com> wrote:
> > On Tue, 2013-10-08 at 13:13 -0600, Bjorn Helgaas wrote:
> >> [+cc Ben, Paul, linuxppc-dev]
> >>
> >> On Mon, Sep 30, 2013 at 04:52:54PM +0800, Minghuan Lian wrote:
> >> > 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 <Minghuan.Lian at freescale.com>
> >>
> >> I cc'd the powerpc maintainers so we can work out which tree this
> >> should go through.
> >>
> >> > ---
> >> > 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





More information about the Linuxppc-dev mailing list