[PATCH 00/16] PCI generic configuration space accessors

[PATCH 00/16] PCI generic configuration space accessors

[Rob Herring]

This series adds common accessor functions for PCI configuration space
accesses. This supports most PCI hosts with memory mapped configuration
space like ECAM or hosts with memory mapped address/data registers. ECAM
is not generically supported by this series, but could be added on top
of this. While some hosts have standard address decoding which could be 
common as well, the various checks on bus numbers and device numbers are 
quite varied. It is unclear how much of that is really necessary or 
could be common. 

The first 4 patches are preparatory cleanup. Patch 5 introduces the
common accessors. The remaining patches convert several PCI host
controllers. This is in no way a complete list of host controllers. The
conversion of more hosts should be possible. The Designware controller
in particular should be able to be converted, but its config space
accessors are a mess of override-able functions that I've not gotten my
head around.

This series is available here [1].

Rob

[1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git pci-config-access

Rob Herring (16):
  frv: add struct pci_ops member names to initialization
  mips: add struct pci_ops member names to initialization
  mn10300: add struct pci_ops member names to initialization
  powerpc: add struct pci_ops member names to initialization
  pci: introduce common pci config space accessors
  ARM: cns3xxx: convert PCI to use generic config accesses
  ARM: integrator: convert PCI to use generic config accesses
  ARM: sa1100: convert PCI to use generic config accesses
  ARM: ks8695: convert PCI to use generic config accesses
  powerpc: fsl_pci: convert PCI to use generic config accesses
  powerpc: powermac: convert PCI to use generic config accesses
  pci/host: generic: convert to use generic config accesses
  pci/host: rcar-gen2: convert to use generic config accesses
  pci/host: tegra: convert to use generic config accesses
  pci/host: xgene: convert to use generic config accesses
  pci/host: xilinx: convert to use generic config accesses

 arch/arm/mach-cns3xxx/pcie.c                   |  52 ++----
 arch/arm/mach-integrator/pci_v3.c              |  61 +-------
 arch/arm/mach-ks8695/pci.c                     |  77 +--------
 arch/arm/mach-sa1100/pci-nanoengine.c          |  94 +----------
 arch/frv/mb93090-mb00/pci-vdk.c                |   4 +-
 arch/mips/pci/pci-bcm1480.c                    |   4 +-
 arch/mips/pci/pci-octeon.c                     |   4 +-
 arch/mips/pci/pcie-octeon.c                    |  12 +-
 arch/mn10300/unit-asb2305/pci.c                |   4 +-
 arch/powerpc/platforms/cell/celleb_scc_pciex.c |   4 +-
 arch/powerpc/platforms/powermac/pci.c          | 209 +++++--------------------
 arch/powerpc/sysdev/fsl_pci.c                  |  46 +-----
 drivers/pci/access.c                           |  87 ++++++++++
 drivers/pci/host/pci-host-generic.c            |  51 +-----
 drivers/pci/host/pci-rcar-gen2.c               |  51 +-----
 drivers/pci/host/pci-tegra.c                   |  55 +------
 drivers/pci/host/pci-xgene.c                   | 150 ++----------------
 drivers/pci/host/pcie-xilinx.c                 |  88 ++---------
 include/linux/pci.h                            |  11 ++
 19 files changed, 212 insertions(+), 852 deletions(-)

-- 
2.1.0

[PATCH 01/16] frv: add struct pci_ops member names to initialization

[Rob Herring]

Some instances of pci_ops initialization rely on the read/write members'
location in the struct. This is fragile and may break when adding new
members to the beginning of the struct.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: David Howells <dhowells@redhat.com>
---
 arch/frv/mb93090-mb00/pci-vdk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index efa5d65..b073f4d 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -168,8 +168,8 @@ static int pci_frv_write_config(struct pci_bus *bus, unsigned int devfn, int whe
 }
 
 static struct pci_ops pci_direct_frv = {
-	pci_frv_read_config,
-	pci_frv_write_config,
+	.read = pci_frv_read_config,
+	.write = pci_frv_write_config,
 };
 
 /*
-- 
2.1.0

[PATCH 02/16] mips: add struct pci_ops member names to initialization

[Rob Herring]

Some instances of pci_ops initialization rely on the read/write members'
location in the struct. This is fragile and may break when adding new
members to the beginning of the struct.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/pci/pci-bcm1480.c |  4 ++--
 arch/mips/pci/pci-octeon.c  |  4 ++--
 arch/mips/pci/pcie-octeon.c | 12 ++++++------
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/mips/pci/pci-bcm1480.c b/arch/mips/pci/pci-bcm1480.c
index 5ec2a7b..f2355e3 100644
--- a/arch/mips/pci/pci-bcm1480.c
+++ b/arch/mips/pci/pci-bcm1480.c
@@ -173,8 +173,8 @@ static int bcm1480_pcibios_write(struct pci_bus *bus, unsigned int devfn,
 }
 
 struct pci_ops bcm1480_pci_ops = {
-	bcm1480_pcibios_read,
-	bcm1480_pcibios_write,
+	.read = bcm1480_pcibios_read,
+	.write = bcm1480_pcibios_write,
 };
 
 static struct resource bcm1480_mem_resource = {
diff --git a/arch/mips/pci/pci-octeon.c b/arch/mips/pci/pci-octeon.c
index d07e041..bedb72b 100644
--- a/arch/mips/pci/pci-octeon.c
+++ b/arch/mips/pci/pci-octeon.c
@@ -327,8 +327,8 @@ static int octeon_write_config(struct pci_bus *bus, unsigned int devfn,
 
 
 static struct pci_ops octeon_pci_ops = {
-	octeon_read_config,
-	octeon_write_config,
+	.read = octeon_read_config,
+	.write = octeon_write_config,
 };
 
 static struct resource octeon_pci_mem_resource = {
diff --git a/arch/mips/pci/pcie-octeon.c b/arch/mips/pci/pcie-octeon.c
index 5e36c33..eb4a17b 100644
--- a/arch/mips/pci/pcie-octeon.c
+++ b/arch/mips/pci/pcie-octeon.c
@@ -1792,8 +1792,8 @@ static int octeon_dummy_write_config(struct pci_bus *bus, unsigned int devfn,
 }
 
 static struct pci_ops octeon_pcie0_ops = {
-	octeon_pcie0_read_config,
-	octeon_pcie0_write_config,
+	.read = octeon_pcie0_read_config,
+	.write = octeon_pcie0_write_config,
 };
 
 static struct resource octeon_pcie0_mem_resource = {
@@ -1813,8 +1813,8 @@ static struct pci_controller octeon_pcie0_controller = {
 };
 
 static struct pci_ops octeon_pcie1_ops = {
-	octeon_pcie1_read_config,
-	octeon_pcie1_write_config,
+	.read = octeon_pcie1_read_config,
+	.write = octeon_pcie1_write_config,
 };
 
 static struct resource octeon_pcie1_mem_resource = {
@@ -1834,8 +1834,8 @@ static struct pci_controller octeon_pcie1_controller = {
 };
 
 static struct pci_ops octeon_dummy_ops = {
-	octeon_dummy_read_config,
-	octeon_dummy_write_config,
+	.read = octeon_dummy_read_config,
+	.write = octeon_dummy_write_config,
 };
 
 static struct resource octeon_dummy_mem_resource = {
-- 
2.1.0

[PATCH 03/16] mn10300: add struct pci_ops member names to initialization

[Rob Herring]

Some instances of pci_ops initialization rely on the read/write members'
location in the struct. This is fragile and may break when adding new
members to the beginning of the struct.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Koichi Yasutake <yasutake.koichi@jp.panasonic.com>
Cc: linux-am33-list@redhat.com
---
 arch/mn10300/unit-asb2305/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
index 6b4339f..406cbe0 100644
--- a/arch/mn10300/unit-asb2305/pci.c
+++ b/arch/mn10300/unit-asb2305/pci.c
@@ -228,8 +228,8 @@ static int pci_ampci_write_config(struct pci_bus *bus, unsigned int devfn,
 }
 
 static struct pci_ops pci_direct_ampci = {
-	pci_ampci_read_config,
-	pci_ampci_write_config,
+	.read = pci_ampci_read_config,
+	.write = pci_ampci_write_config,
 };
 
 /*
-- 
2.1.0

[PATCH 04/16] powerpc: add struct pci_ops member names to initialization

[Rob Herring]

Some instances of pci_ops initialization rely on the read/write members'
location in the struct. This is fragile and may break when adding new
members to the beginning of the struct.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: cbe-oss-dev@lists.ozlabs.org
---
 arch/powerpc/platforms/cell/celleb_scc_pciex.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/celleb_scc_pciex.c b/arch/powerpc/platforms/cell/celleb_scc_pciex.c
index f223875..94170e4 100644
--- a/arch/powerpc/platforms/cell/celleb_scc_pciex.c
+++ b/arch/powerpc/platforms/cell/celleb_scc_pciex.c
@@ -399,8 +399,8 @@ static int scc_pciex_write_config(struct pci_bus *bus, unsigned int devfn,
 }
 
 static struct pci_ops scc_pciex_pci_ops = {
-	scc_pciex_read_config,
-	scc_pciex_write_config,
+	.read = scc_pciex_read_config,
+	.write = scc_pciex_write_config,
 };
 
 static void pciex_clear_intr_all(unsigned int __iomem *base)
-- 
2.1.0

[PATCH 05/16] pci: introduce common pci config space accessors

[Rob Herring]

Many PCI controllers' configuration space accesses are memory mapped
varying only in address calculation and access checks. There are 2 main
access methods: a decoded address space such as ECAM or a single address
and data register similar to x86. This implementation can support both
cases as well as be used in cases that need additional pre or post access
handling.

A new pci_ops member map_bus is introduced which can do access checks and
any necessary setup. It returns the address to use for the configuration
space access. The access types supported are 32-bit only accesses or
correct byte, word, or dword sized accesses.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/pci/access.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h  | 11 +++++++
 2 files changed, 98 insertions(+)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 49dd766..d9b64a1 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -67,6 +67,93 @@ EXPORT_SYMBOL(pci_bus_write_config_byte);
 EXPORT_SYMBOL(pci_bus_write_config_word);
 EXPORT_SYMBOL(pci_bus_write_config_dword);
 
+int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
+			    int where, int size, u32 *val)
+{
+	void __iomem *addr;
+
+	addr = bus->ops->map_bus(bus, devfn, where);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	if (size == 1)
+		*val = readb(addr);
+	else if (size == 2)
+		*val = readw(addr);
+	else
+		*val = readl(addr);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+EXPORT_SYMBOL_GPL(pci_generic_config_read);
+
+int pci_generic_config_write(struct pci_bus *bus, unsigned int devfn,
+			     int where, int size, u32 val)
+{
+	void __iomem *addr;
+
+	addr = bus->ops->map_bus(bus, devfn, where);
+	if (!addr)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (size == 1)
+		writeb(val, addr);
+	else if (size == 2)
+		writew(val, addr);
+	else
+		writel(val, addr);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+EXPORT_SYMBOL_GPL(pci_generic_config_write);
+
+int pci_generic_config_read32(struct pci_bus *bus, unsigned int devfn,
+			      int where, int size, u32 *val)
+{
+	void __iomem *addr;
+
+	addr = bus->ops->map_bus(bus, devfn, where & ~0x3);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	*val = readl(addr);
+
+	if (size <= 2)
+		*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+EXPORT_SYMBOL_GPL(pci_generic_config_read32);
+
+int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn,
+			       int where, int size, u32 val)
+{
+	void __iomem *addr;
+	u32 mask, tmp;
+
+	addr = bus->ops->map_bus(bus, devfn, where & ~0x3);
+	if (!addr)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (size == 4) {
+		writel(val, addr);
+		return PCIBIOS_SUCCESSFUL;
+	} else {
+		mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+	}
+
+	tmp = readl(addr) & mask;
+	tmp |= val << ((where & 0x3) * 8);
+	writel(tmp, addr);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+EXPORT_SYMBOL_GPL(pci_generic_config_write32);
+
 /**
  * pci_bus_set_ops - Set raw operations of pci bus
  * @bus:	pci bus struct
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 360a966..e7fd519 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -560,6 +560,7 @@ static inline int pcibios_err_to_errno(int err)
 /* Low-level architecture-dependent routines */
 
 struct pci_ops {
+	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
 	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
 	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
 };
@@ -857,6 +858,16 @@ int pci_bus_write_config_word(struct pci_bus *bus, unsigned int devfn,
 			      int where, u16 val);
 int pci_bus_write_config_dword(struct pci_bus *bus, unsigned int devfn,
 			       int where, u32 val);
+
+int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,
+			    int where, int size, u32 *val);
+int pci_generic_config_write(struct pci_bus *bus, unsigned int devfn,
+			    int where, int size, u32 val);
+int pci_generic_config_read32(struct pci_bus *bus, unsigned int devfn,
+			      int where, int size, u32 *val);
+int pci_generic_config_write32(struct pci_bus *bus, unsigned int devfn,
+			       int where, int size, u32 val);
+
 struct pci_ops *pci_bus_set_ops(struct pci_bus *bus, struct pci_ops *ops);
 
 static inline int pci_read_config_byte(const struct pci_dev *dev, int where, u8 *val)
-- 
2.1.0

Re: [PATCH 05/16] pci: introduce common pci config space accessors

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

On Fri, Jan 09, 2015 at 08:34:39PM -0600, Rob Herring wrote:
> Many PCI controllers' configuration space accesses are memory mapped
> varying only in address calculation and access checks. There are 2 main
> access methods: a decoded address space such as ECAM or a single address
> and data register similar to x86. This implementation can support both
> cases as well as be used in cases that need additional pre or post access
> handling.
> 
> A new pci_ops member map_bus is introduced which can do access checks and
> any necessary setup. It returns the address to use for the configuration
> space access. The access types supported are 32-bit only accesses or
> correct byte, word, or dword sized accesses.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> ---
>  drivers/pci/access.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h  | 11 +++++++
>  2 files changed, 98 insertions(+)

Reviewed-by: Thierry Reding <treding@nvidia.com>
Tested-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 05/16] pci: introduce common pci config space accessors

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

On Mon, Jan 12, 2015 at 11:01:33AM +0100, Thierry Reding wrote:
> On Fri, Jan 09, 2015 at 08:34:39PM -0600, Rob Herring wrote:
> > Many PCI controllers' configuration space accesses are memory mapped
> > varying only in address calculation and access checks. There are 2 main
> > access methods: a decoded address space such as ECAM or a single address
> > and data register similar to x86. This implementation can support both
> > cases as well as be used in cases that need additional pre or post access
> > handling.
> > 
> > A new pci_ops member map_bus is introduced which can do access checks and
> > any necessary setup. It returns the address to use for the configuration
> > space access. The access types supported are 32-bit only accesses or
> > correct byte, word, or dword sized accesses.
> > 
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-pci@vger.kernel.org
> > ---
> >  drivers/pci/access.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/pci.h  | 11 +++++++
> >  2 files changed, 98 insertions(+)
> 
> Reviewed-by: Thierry Reding <treding@nvidia.com>
> Tested-by: Thierry Reding <treding@nvidia.com>

Although the subject could use the proper casing: "... common PCI ...".

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 06/16] ARM: cns3xxx: convert PCI to use generic config accesses

[Rob Herring]

Convert the cns3xxx PCI driver to use the generic config access functions.

This changes accesses from __raw_readl/__raw_writel to readl/writel.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Krzysztof Halasa <khalasa@piap.pl>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-cns3xxx/pcie.c | 52 +++++++++-----------------------------------
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
index 45d6bd0..19b4e20 100644
--- a/arch/arm/mach-cns3xxx/pcie.c
+++ b/arch/arm/mach-cns3xxx/pcie.c
@@ -54,8 +54,8 @@ static struct cns3xxx_pcie *pbus_to_cnspci(struct pci_bus *bus)
 	return sysdata_to_cnspci(bus->sysdata);
 }
 
-static void __iomem *cns3xxx_pci_cfg_base(struct pci_bus *bus,
-				  unsigned int devfn, int where)
+static void __iomem *cns3xxx_pci_map_bus(struct pci_bus *bus,
+					 unsigned int devfn, int where)
 {
 	struct cns3xxx_pcie *cnspci = pbus_to_cnspci(bus);
 	int busno = bus->number;
@@ -91,55 +91,22 @@ static void __iomem *cns3xxx_pci_cfg_base(struct pci_bus *bus,
 static int cns3xxx_pci_read_config(struct pci_bus *bus, unsigned int devfn,
 				   int where, int size, u32 *val)
 {
-	u32 v;
-	void __iomem *base;
+	int ret;
 	u32 mask = (0x1ull << (size * 8)) - 1;
 	int shift = (where % 4) * 8;
 
-	base = cns3xxx_pci_cfg_base(bus, devfn, where);
-	if (!base) {
-		*val = 0xffffffff;
-		return PCIBIOS_SUCCESSFUL;
-	}
-
-	v = __raw_readl(base);
+	ret = pci_generic_config_read32(bus, devfn, where, size, val);
 
-	if (bus->number == 0 && devfn == 0 &&
-			(where & 0xffc) == PCI_CLASS_REVISION) {
+	if (ret == PCIBIOS_SUCCESSFUL && !bus->number && !devfn &&
+	    (where & 0xffc) == PCI_CLASS_REVISION)
 		/*
 		 * RC's class is 0xb, but Linux PCI driver needs 0x604
 		 * for a PCIe bridge. So we must fixup the class code
 		 * to 0x604 here.
 		 */
-		v &= 0xff;
-		v |= 0x604 << 16;
-	}
-
-	*val = (v >> shift) & mask;
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int cns3xxx_pci_write_config(struct pci_bus *bus, unsigned int devfn,
-				    int where, int size, u32 val)
-{
-	u32 v;
-	void __iomem *base;
-	u32 mask = (0x1ull << (size * 8)) - 1;
-	int shift = (where % 4) * 8;
-
-	base = cns3xxx_pci_cfg_base(bus, devfn, where);
-	if (!base)
-		return PCIBIOS_SUCCESSFUL;
-
-	v = __raw_readl(base);
-
-	v &= ~(mask << shift);
-	v |= (val & mask) << shift;
-
-	__raw_writel(v, base);
+		*val = (((((*val << shift) & 0xff) | (0x604 << 16)) >> shift) & mask;
 
-	return PCIBIOS_SUCCESSFUL;
+	return ret;
 }
 
 static int cns3xxx_pci_setup(int nr, struct pci_sys_data *sys)
@@ -158,8 +125,9 @@ static int cns3xxx_pci_setup(int nr, struct pci_sys_data *sys)
 }
 
 static struct pci_ops cns3xxx_pcie_ops = {
+	.map_bus = cns3xxx_pci_map_bus,
 	.read = cns3xxx_pci_read_config,
-	.write = cns3xxx_pci_write_config,
+	.write = pci_generic_config_write,
 };
 
 static int cns3xxx_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-- 
2.1.0

Re: [PATCH 06/16] ARM: cns3xxx: convert PCI to use generic config accesses

[Krzysztof Hałasa]

Rob Herring <robh@kernel.org> writes:

> Convert the cns3xxx PCI driver to use the generic config access functions.
>
> This changes accesses from __raw_readl/__raw_writel to readl/writel.
>
>  arch/arm/mach-cns3xxx/pcie.c | 52 +++++++++-----------------------------------

This looks fine:
Acked-by: Krzysztof Hałasa <khalasa@piap.pl>

-- 
Krzysztof Halasa

Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: [PATCH 06/16] ARM: cns3xxx: convert PCI to use generic config accesses

[Bjorn Helgaas]

On Thu, Jan 29, 2015 at 12:16 AM, Krzysztof Hałasa <khalasa@piap.pl> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> Convert the cns3xxx PCI driver to use the generic config access functions.
>>
>> This changes accesses from __raw_readl/__raw_writel to readl/writel.
>>
>>  arch/arm/mach-cns3xxx/pcie.c | 52 +++++++++-----------------------------------
>
> This looks fine:
> Acked-by: Krzysztof Hałasa <khalasa@piap.pl>

Thanks, I added your ack.

[PATCH 07/16] ARM: integrator: convert PCI to use generic config accesses

[Rob Herring]

Convert the integrator PCI driver to use the generic config access
functions.

This changes accesses from __raw_readX/__raw_writeX to readX/writeX
variants. The spinlock is removed because it is unnecessary. The config
read and write functions are already protected with a spinlock and no
access can occur during the .pre_init function.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-integrator/pci_v3.c | 61 ++++-----------------------------------
 1 file changed, 5 insertions(+), 56 deletions(-)

diff --git a/arch/arm/mach-integrator/pci_v3.c b/arch/arm/mach-integrator/pci_v3.c
index c186a17..dc7782f 100644
--- a/arch/arm/mach-integrator/pci_v3.c
+++ b/arch/arm/mach-integrator/pci_v3.c
@@ -356,7 +356,6 @@ static u64 pre_mem_pci_sz;
  *	 7:2	register number
  *
  */
-static DEFINE_RAW_SPINLOCK(v3_lock);
 
 #undef V3_LB_BASE_PREFETCH
 #define V3_LB_BASE_PREFETCH 0
@@ -457,67 +456,21 @@ static void v3_close_config_window(void)
 static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
 			  int size, u32 *val)
 {
-	void __iomem *addr;
-	unsigned long flags;
-	u32 v;
-
-	raw_spin_lock_irqsave(&v3_lock, flags);
-	addr = v3_open_config_window(bus, devfn, where);
-
-	switch (size) {
-	case 1:
-		v = __raw_readb(addr);
-		break;
-
-	case 2:
-		v = __raw_readw(addr);
-		break;
-
-	default:
-		v = __raw_readl(addr);
-		break;
-	}
-
+	int ret = pci_generic_config_read(bus, devfn, where, size, val);
 	v3_close_config_window();
-	raw_spin_unlock_irqrestore(&v3_lock, flags);
-
-	*val = v;
-	return PCIBIOS_SUCCESSFUL;
+	return ret;
 }
 
 static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
 			   int size, u32 val)
 {
-	void __iomem *addr;
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&v3_lock, flags);
-	addr = v3_open_config_window(bus, devfn, where);
-
-	switch (size) {
-	case 1:
-		__raw_writeb((u8)val, addr);
-		__raw_readb(addr);
-		break;
-
-	case 2:
-		__raw_writew((u16)val, addr);
-		__raw_readw(addr);
-		break;
-
-	case 4:
-		__raw_writel(val, addr);
-		__raw_readl(addr);
-		break;
-	}
-
+	int ret = pci_generic_config_write(bus, devfn, where, size, val);
 	v3_close_config_window();
-	raw_spin_unlock_irqrestore(&v3_lock, flags);
-
-	return PCIBIOS_SUCCESSFUL;
+	return ret;
 }
 
 static struct pci_ops pci_v3_ops = {
+	.map_bus = v3_open_config_window,
 	.read	= v3_read_config,
 	.write	= v3_write_config,
 };
@@ -672,8 +625,6 @@ static void __init pci_v3_preinit(void)
 	hook_fault_code(8, v3_pci_fault, SIGBUS, 0, "external abort on non-linefetch");
 	hook_fault_code(10, v3_pci_fault, SIGBUS, 0, "external abort on non-linefetch");
 
-	raw_spin_lock_irqsave(&v3_lock, flags);
-
 	/*
 	 * Unlock V3 registers, but only if they were previously locked.
 	 */
@@ -736,8 +687,6 @@ static void __init pci_v3_preinit(void)
 	v3_writew(V3_LB_CFG, v3_readw(V3_LB_CFG) | (1 << 10));
 	v3_writeb(V3_LB_IMASK, 0x28);
 	__raw_writel(3, ap_syscon_base + INTEGRATOR_SC_PCIENABLE_OFFSET);
-
-	raw_spin_unlock_irqrestore(&v3_lock, flags);
 }
 
 static void __init pci_v3_postinit(void)
-- 
2.1.0

Re: [PATCH 07/16] ARM: integrator: convert PCI to use generic config accesses

[Linus Walleij]

On Sat, Jan 10, 2015 at 3:34 AM, Rob Herring <robh@kernel.org> wrote:

> Convert the integrator PCI driver to use the generic config access
> functions.
>
> This changes accesses from __raw_readX/__raw_writeX to readX/writeX
> variants.

Just as good.

> The spinlock is removed because it is unnecessary. The config
> read and write functions are already protected with a spinlock and no
> access can occur during the .pre_init function.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org

I trust that you do the right thing I guess...
Acked-by: Linus Walleij <linus.walleij@linaro.org>

>  static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
>                           int size, u32 *val)
>  {
> -       addr = v3_open_config_window(bus, devfn, where);
> +       int ret = pci_generic_config_read(bus, devfn, where, size, val);
>         v3_close_config_window();
> +       return ret;
>  }
>
>  static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
>                            int size, u32 val)
>  {
> +       int ret = pci_generic_config_write(bus, devfn, where, size, val);
>         v3_close_config_window();
> -       raw_spin_unlock_irqrestore(&v3_lock, flags);
> +       return ret;
>  }
>
>  static struct pci_ops pci_v3_ops = {
> +       .map_bus = v3_open_config_window,
>         .read   = v3_read_config,
>         .write  = v3_write_config,

So .map_bus is called before every .read/.write operation I take it.

Wouldn't it be proper to call the v3_close_config_window() from a
matching .unmap_bus() callback for symmetry?

Yours,
Linus Walleij

Re: [PATCH 07/16] ARM: integrator: convert PCI to use generic config accesses

[Arnd Bergmann]

On Saturday 10 January 2015 22:40:22 Linus Walleij wrote:
> >  static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
> >                           int size, u32 *val)
> >  {
> > -       addr = v3_open_config_window(bus, devfn, where);
> > +       int ret = pci_generic_config_read(bus, devfn, where, size, val);
> >         v3_close_config_window();
> > +       return ret;
> >  }
> >
> >  static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
> >                            int size, u32 val)
> >  {
> > +       int ret = pci_generic_config_write(bus, devfn, where, size, val);
> >         v3_close_config_window();
> > -       raw_spin_unlock_irqrestore(&v3_lock, flags);
> > +       return ret;
> >  }
> >
> >  static struct pci_ops pci_v3_ops = {
> > +       .map_bus = v3_open_config_window,
> >         .read   = v3_read_config,
> >         .write  = v3_write_config,
> 
> So .map_bus is called before every .read/.write operation I take it.
> 
> Wouldn't it be proper to call the v3_close_config_window() from a
> matching .unmap_bus() callback for symmetry?

It would be nicer for integrator but useless for anything else, so
I'd vote for leaving it the way Rob posted.

	Arnd

Re: [PATCH 07/16] ARM: integrator: convert PCI to use generic config accesses

[Linus Walleij]

On Sat, Jan 10, 2015 at 10:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 10 January 2015 22:40:22 Linus Walleij wrote:
>> >  static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
>> >                           int size, u32 *val)
>> >  {
>> > -       addr = v3_open_config_window(bus, devfn, where);
>> > +       int ret = pci_generic_config_read(bus, devfn, where, size, val);
>> >         v3_close_config_window();
>> > +       return ret;
>> >  }
>> >
>> >  static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
>> >                            int size, u32 val)
>> >  {
>> > +       int ret = pci_generic_config_write(bus, devfn, where, size, val);
>> >         v3_close_config_window();
>> > -       raw_spin_unlock_irqrestore(&v3_lock, flags);
>> > +       return ret;
>> >  }
>> >
>> >  static struct pci_ops pci_v3_ops = {
>> > +       .map_bus = v3_open_config_window,
>> >         .read   = v3_read_config,
>> >         .write  = v3_write_config,
>>
>> So .map_bus is called before every .read/.write operation I take it.
>>
>> Wouldn't it be proper to call the v3_close_config_window() from a
>> matching .unmap_bus() callback for symmetry?
>
> It would be nicer for integrator but useless for anything else, so
> I'd vote for leaving it the way Rob posted.

OK I buy that, throw in a comment about it in the code if there
is some time for iterating the patch.

Yours,
Linus Walleij

Re: [PATCH 07/16] ARM: integrator: convert PCI to use generic config accesses

[Bjorn Helgaas]

On Mon, Jan 12, 2015 at 01:05:12AM +0100, Linus Walleij wrote:
> On Sat, Jan 10, 2015 at 10:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Saturday 10 January 2015 22:40:22 Linus Walleij wrote:
> >> >  static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
> >> >                           int size, u32 *val)
> >> >  {
> >> > -       addr = v3_open_config_window(bus, devfn, where);
> >> > +       int ret = pci_generic_config_read(bus, devfn, where, size, val);
> >> >         v3_close_config_window();
> >> > +       return ret;
> >> >  }
> >> >
> >> >  static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
> >> >                            int size, u32 val)
> >> >  {
> >> > +       int ret = pci_generic_config_write(bus, devfn, where, size, val);
> >> >         v3_close_config_window();
> >> > -       raw_spin_unlock_irqrestore(&v3_lock, flags);
> >> > +       return ret;
> >> >  }
> >> >
> >> >  static struct pci_ops pci_v3_ops = {
> >> > +       .map_bus = v3_open_config_window,
> >> >         .read   = v3_read_config,
> >> >         .write  = v3_write_config,
> >>
> >> So .map_bus is called before every .read/.write operation I take it.
> >>
> >> Wouldn't it be proper to call the v3_close_config_window() from a
> >> matching .unmap_bus() callback for symmetry?
> >
> > It would be nicer for integrator but useless for anything else, so
> > I'd vote for leaving it the way Rob posted.
> 
> OK I buy that, throw in a comment about it in the code if there
> is some time for iterating the patch.

Would you prefer something like the following instead?  It keeps the
v3_open/close symmetry, but it does break apart and duplicate some of the
logic from v3_open_config_window().

I can't build or test this.


commit 3e0c269a560968002298d22ac47bde4bd45aea8e
Author: Rob Herring <robh@kernel.org>
Date:   Fri Jan 9 20:34:41 2015 -0600

    ARM: integrator: Convert PCI to use generic config accessors
    
    Convert the integrator PCI driver to use the generic config access
    functions.
    
    This changes accesses from __raw_readX/__raw_writeX to readX/writeX
    variants.  The spinlock is removed because it is unnecessary.  The config
    read and write functions are already protected with a spinlock and no
    access can occur during the .pre_init function.
    
    Signed-off-by: Rob Herring <robh@kernel.org>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Acked-by: Linus Walleij <linus.walleij@linaro.org>
    CC: Russell King <linux@arm.linux.org.uk>
    CC: linux-arm-kernel@lists.infradead.org

diff --git a/arch/arm/mach-integrator/pci_v3.c b/arch/arm/mach-integrator/pci_v3.c
index c186a17c2cff..c1098729cf8a 100644
--- a/arch/arm/mach-integrator/pci_v3.c
+++ b/arch/arm/mach-integrator/pci_v3.c
@@ -324,7 +324,7 @@ static u64 pre_mem_pci_sz;
  * configuration address space, you present the V3 with the following pattern
  * (which is very nearly a type 1 (except that the lower two bits are 00 and
  * not 01).   In order for this mapping to work you need to set up one of
- * the local to PCI aperatures to 16Mbytes in length translating to
+ * the local to PCI apertures to 16Mbytes in length translating to
  * PCI configuration space starting at 0x0000.0000.
  *
  * PCI configuration cycles look like this:
@@ -356,15 +356,14 @@ static u64 pre_mem_pci_sz;
  *	 7:2	register number
  *
  */
-static DEFINE_RAW_SPINLOCK(v3_lock);
 
 #undef V3_LB_BASE_PREFETCH
 #define V3_LB_BASE_PREFETCH 0
 
-static void __iomem *v3_open_config_window(struct pci_bus *bus,
-					   unsigned int devfn, int offset)
+static void v3_open_config_window(struct pci_bus *bus, unsigned int devfn,
+				  int offset)
 {
-	unsigned int address, mapaddress, busnr;
+	unsigned int mapaddress, busnr;
 
 	busnr = bus->number;
 
@@ -381,14 +380,10 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus,
 		/*
 		 * local bus segment so need a type 0 config cycle
 		 *
-		 * build the PCI configuration "address" with one-hot in
-		 * A31-A11
-		 *
 		 * mapaddress:
 		 *  3:1 = config cycle (101)
 		 *  0   = PCI A1 & A0 are 0 (0)
 		 */
-		address = PCI_FUNC(devfn) << 8;
 		mapaddress = V3_LB_MAP_TYPE_CONFIG;
 
 		if (slot > 12)
@@ -396,26 +391,15 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus,
 			 * high order bits are handled by the MAP register
 			 */
 			mapaddress |= 1 << (slot - 5);
-		else
-			/*
-			 * low order bits handled directly in the address
-			 */
-			address |= 1 << (slot + 11);
 	} else {
         	/*
 		 * not the local bus segment so need a type 1 config cycle
 		 *
-		 * address:
-		 *  23:16 = bus number
-		 *  15:11 = slot number (7:3 of devfn)
-		 *  10:8  = func number (2:0 of devfn)
-		 *
 		 * mapaddress:
 		 *  3:1 = config cycle (101)
 		 *  0   = PCI A1 & A0 from host bus (1)
 		 */
 		mapaddress = V3_LB_MAP_TYPE_CONFIG | V3_LB_MAP_AD_LOW_EN;
-		address = (busnr << 16) | (devfn << 8);
 	}
 
 	/*
@@ -432,8 +416,6 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus,
 	v3_writel(V3_LB_BASE1, v3_addr_to_lb_base(conf_mem.start) |
 			V3_LB_BASE_ADR_SIZE_16MB | V3_LB_BASE_ENABLE);
 	v3_writew(V3_LB_MAP1, mapaddress);
-
-	return PCI_CONFIG_VADDR + address + offset;
 }
 
 static void v3_close_config_window(void)
@@ -454,70 +436,66 @@ static void v3_close_config_window(void)
 			V3_LB_BASE_ADR_SIZE_256MB | V3_LB_BASE_ENABLE);
 }
 
-static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
-			  int size, u32 *val)
+static void __iomem *v3_map_bus(struct pci_bus *bus,
+					   unsigned int devfn, int offset)
 {
-	void __iomem *addr;
-	unsigned long flags;
-	u32 v;
+	unsigned int busnr = bus->number;
+	unsigned int address;
 
-	raw_spin_lock_irqsave(&v3_lock, flags);
-	addr = v3_open_config_window(bus, devfn, where);
-
-	switch (size) {
-	case 1:
-		v = __raw_readb(addr);
-		break;
+	if (busnr == 0) {
+		int slot = PCI_SLOT(devfn);
 
-	case 2:
-		v = __raw_readw(addr);
-		break;
+		/*
+		 * build the PCI configuration "address" with function in
+		 * A10-A8 and device one-hot in A31-A11
+		 */
+		address = PCI_FUNC(devfn) << 8;
 
-	default:
-		v = __raw_readl(addr);
-		break;
+		/*
+		 * high order bits are handled by the MAP register;
+		 * low order bits handled directly in the address
+		 */
+		if (slot <= 12)
+			address |= 1 << (slot + 11);
+	} else {
+		/*
+		 * not the local bus segment so need a type 1 config cycle
+		 *
+		 * address:
+		 *  23:16 = bus number
+		 *  15:11 = slot number (7:3 of devfn)
+		 *  10:8  = func number (2:0 of devfn)
+		 */
+		address = (busnr << 16) | (devfn << 8);
 	}
 
-	v3_close_config_window();
-	raw_spin_unlock_irqrestore(&v3_lock, flags);
+	return PCI_CONFIG_VADDR + address + offset;
+}
+
+static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
+			  int size, u32 *val)
+{
+	int ret;
 
-	*val = v;
-	return PCIBIOS_SUCCESSFUL;
+	v3_open_config_window();
+	ret = pci_generic_config_read(bus, devfn, where, size, val);
+	v3_close_config_window();
+	return ret;
 }
 
 static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
 			   int size, u32 val)
 {
-	void __iomem *addr;
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&v3_lock, flags);
-	addr = v3_open_config_window(bus, devfn, where);
-
-	switch (size) {
-	case 1:
-		__raw_writeb((u8)val, addr);
-		__raw_readb(addr);
-		break;
-
-	case 2:
-		__raw_writew((u16)val, addr);
-		__raw_readw(addr);
-		break;
-
-	case 4:
-		__raw_writel(val, addr);
-		__raw_readl(addr);
-		break;
-	}
+	int ret;
 
+	v3_open_config_window();
+	ret = pci_generic_config_write(bus, devfn, where, size, val);
 	v3_close_config_window();
-	raw_spin_unlock_irqrestore(&v3_lock, flags);
-
-	return PCIBIOS_SUCCESSFUL;
+	return ret;
 }
 
 static struct pci_ops pci_v3_ops = {
+	.map_bus = v3_map_bus,
 	.read	= v3_read_config,
 	.write	= v3_write_config,
 };
@@ -672,8 +650,6 @@ static void __init pci_v3_preinit(void)
 	hook_fault_code(8, v3_pci_fault, SIGBUS, 0, "external abort on non-linefetch");
 	hook_fault_code(10, v3_pci_fault, SIGBUS, 0, "external abort on non-linefetch");
 
-	raw_spin_lock_irqsave(&v3_lock, flags);
-
 	/*
 	 * Unlock V3 registers, but only if they were previously locked.
 	 */
@@ -736,8 +712,6 @@ static void __init pci_v3_preinit(void)
 	v3_writew(V3_LB_CFG, v3_readw(V3_LB_CFG) | (1 << 10));
 	v3_writeb(V3_LB_IMASK, 0x28);
 	__raw_writel(3, ap_syscon_base + INTEGRATOR_SC_PCIENABLE_OFFSET);
-
-	raw_spin_unlock_irqrestore(&v3_lock, flags);
 }
 
 static void __init pci_v3_postinit(void)

Re: [PATCH 07/16] ARM: integrator: convert PCI to use generic config accesses

[Bjorn Helgaas]

On Thu, Jan 22, 2015 at 02:33:36PM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 12, 2015 at 01:05:12AM +0100, Linus Walleij wrote:
> > On Sat, Jan 10, 2015 at 10:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Saturday 10 January 2015 22:40:22 Linus Walleij wrote:
> > >> >  static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
> > >> >                           int size, u32 *val)
> > >> >  {
> > >> > -       addr = v3_open_config_window(bus, devfn, where);
> > >> > +       int ret = pci_generic_config_read(bus, devfn, where, size, val);
> > >> >         v3_close_config_window();
> > >> > +       return ret;
> > >> >  }
> > >> >
> > >> >  static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
> > >> >                            int size, u32 val)
> > >> >  {
> > >> > +       int ret = pci_generic_config_write(bus, devfn, where, size, val);
> > >> >         v3_close_config_window();
> > >> > -       raw_spin_unlock_irqrestore(&v3_lock, flags);
> > >> > +       return ret;
> > >> >  }
> > >> >
> > >> >  static struct pci_ops pci_v3_ops = {
> > >> > +       .map_bus = v3_open_config_window,
> > >> >         .read   = v3_read_config,
> > >> >         .write  = v3_write_config,
> > >>
> > >> So .map_bus is called before every .read/.write operation I take it.
> > >>
> > >> Wouldn't it be proper to call the v3_close_config_window() from a
> > >> matching .unmap_bus() callback for symmetry?
> > >
> > > It would be nicer for integrator but useless for anything else, so
> > > I'd vote for leaving it the way Rob posted.
> > 
> > OK I buy that, throw in a comment about it in the code if there
> > is some time for iterating the patch.
> 
> Would you prefer something like the following instead?  It keeps the
> v3_open/close symmetry, but it does break apart and duplicate some of the
> logic from v3_open_config_window().

Hearing nothing, I kept Rob's initial version.  It can be tweaked later if
anybody wants to.  I just want to get all this into v3.20.

Bjorn

> commit 3e0c269a560968002298d22ac47bde4bd45aea8e
> Author: Rob Herring <robh@kernel.org>
> Date:   Fri Jan 9 20:34:41 2015 -0600
> 
>     ARM: integrator: Convert PCI to use generic config accessors
>     
>     Convert the integrator PCI driver to use the generic config access
>     functions.
>     
>     This changes accesses from __raw_readX/__raw_writeX to readX/writeX
>     variants.  The spinlock is removed because it is unnecessary.  The config
>     read and write functions are already protected with a spinlock and no
>     access can occur during the .pre_init function.
>     
>     Signed-off-by: Rob Herring <robh@kernel.org>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>     Acked-by: Linus Walleij <linus.walleij@linaro.org>
>     CC: Russell King <linux@arm.linux.org.uk>
>     CC: linux-arm-kernel@lists.infradead.org
> 
> diff --git a/arch/arm/mach-integrator/pci_v3.c b/arch/arm/mach-integrator/pci_v3.c
> index c186a17c2cff..c1098729cf8a 100644
> --- a/arch/arm/mach-integrator/pci_v3.c
> +++ b/arch/arm/mach-integrator/pci_v3.c
> @@ -324,7 +324,7 @@ static u64 pre_mem_pci_sz;
>   * configuration address space, you present the V3 with the following pattern
>   * (which is very nearly a type 1 (except that the lower two bits are 00 and
>   * not 01).   In order for this mapping to work you need to set up one of
> - * the local to PCI aperatures to 16Mbytes in length translating to
> + * the local to PCI apertures to 16Mbytes in length translating to
>   * PCI configuration space starting at 0x0000.0000.
>   *
>   * PCI configuration cycles look like this:
> @@ -356,15 +356,14 @@ static u64 pre_mem_pci_sz;
>   *	 7:2	register number
>   *
>   */
> -static DEFINE_RAW_SPINLOCK(v3_lock);
>  
>  #undef V3_LB_BASE_PREFETCH
>  #define V3_LB_BASE_PREFETCH 0
>  
> -static void __iomem *v3_open_config_window(struct pci_bus *bus,
> -					   unsigned int devfn, int offset)
> +static void v3_open_config_window(struct pci_bus *bus, unsigned int devfn,
> +				  int offset)
>  {
> -	unsigned int address, mapaddress, busnr;
> +	unsigned int mapaddress, busnr;
>  
>  	busnr = bus->number;
>  
> @@ -381,14 +380,10 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus,
>  		/*
>  		 * local bus segment so need a type 0 config cycle
>  		 *
> -		 * build the PCI configuration "address" with one-hot in
> -		 * A31-A11
> -		 *
>  		 * mapaddress:
>  		 *  3:1 = config cycle (101)
>  		 *  0   = PCI A1 & A0 are 0 (0)
>  		 */
> -		address = PCI_FUNC(devfn) << 8;
>  		mapaddress = V3_LB_MAP_TYPE_CONFIG;
>  
>  		if (slot > 12)
> @@ -396,26 +391,15 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus,
>  			 * high order bits are handled by the MAP register
>  			 */
>  			mapaddress |= 1 << (slot - 5);
> -		else
> -			/*
> -			 * low order bits handled directly in the address
> -			 */
> -			address |= 1 << (slot + 11);
>  	} else {
>          	/*
>  		 * not the local bus segment so need a type 1 config cycle
>  		 *
> -		 * address:
> -		 *  23:16 = bus number
> -		 *  15:11 = slot number (7:3 of devfn)
> -		 *  10:8  = func number (2:0 of devfn)
> -		 *
>  		 * mapaddress:
>  		 *  3:1 = config cycle (101)
>  		 *  0   = PCI A1 & A0 from host bus (1)
>  		 */
>  		mapaddress = V3_LB_MAP_TYPE_CONFIG | V3_LB_MAP_AD_LOW_EN;
> -		address = (busnr << 16) | (devfn << 8);
>  	}
>  
>  	/*
> @@ -432,8 +416,6 @@ static void __iomem *v3_open_config_window(struct pci_bus *bus,
>  	v3_writel(V3_LB_BASE1, v3_addr_to_lb_base(conf_mem.start) |
>  			V3_LB_BASE_ADR_SIZE_16MB | V3_LB_BASE_ENABLE);
>  	v3_writew(V3_LB_MAP1, mapaddress);
> -
> -	return PCI_CONFIG_VADDR + address + offset;
>  }
>  
>  static void v3_close_config_window(void)
> @@ -454,70 +436,66 @@ static void v3_close_config_window(void)
>  			V3_LB_BASE_ADR_SIZE_256MB | V3_LB_BASE_ENABLE);
>  }
>  
> -static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
> -			  int size, u32 *val)
> +static void __iomem *v3_map_bus(struct pci_bus *bus,
> +					   unsigned int devfn, int offset)
>  {
> -	void __iomem *addr;
> -	unsigned long flags;
> -	u32 v;
> +	unsigned int busnr = bus->number;
> +	unsigned int address;
>  
> -	raw_spin_lock_irqsave(&v3_lock, flags);
> -	addr = v3_open_config_window(bus, devfn, where);
> -
> -	switch (size) {
> -	case 1:
> -		v = __raw_readb(addr);
> -		break;
> +	if (busnr == 0) {
> +		int slot = PCI_SLOT(devfn);
>  
> -	case 2:
> -		v = __raw_readw(addr);
> -		break;
> +		/*
> +		 * build the PCI configuration "address" with function in
> +		 * A10-A8 and device one-hot in A31-A11
> +		 */
> +		address = PCI_FUNC(devfn) << 8;
>  
> -	default:
> -		v = __raw_readl(addr);
> -		break;
> +		/*
> +		 * high order bits are handled by the MAP register;
> +		 * low order bits handled directly in the address
> +		 */
> +		if (slot <= 12)
> +			address |= 1 << (slot + 11);
> +	} else {
> +		/*
> +		 * not the local bus segment so need a type 1 config cycle
> +		 *
> +		 * address:
> +		 *  23:16 = bus number
> +		 *  15:11 = slot number (7:3 of devfn)
> +		 *  10:8  = func number (2:0 of devfn)
> +		 */
> +		address = (busnr << 16) | (devfn << 8);
>  	}
>  
> -	v3_close_config_window();
> -	raw_spin_unlock_irqrestore(&v3_lock, flags);
> +	return PCI_CONFIG_VADDR + address + offset;
> +}
> +
> +static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
> +			  int size, u32 *val)
> +{
> +	int ret;
>  
> -	*val = v;
> -	return PCIBIOS_SUCCESSFUL;
> +	v3_open_config_window();
> +	ret = pci_generic_config_read(bus, devfn, where, size, val);
> +	v3_close_config_window();
> +	return ret;
>  }
>  
>  static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
>  			   int size, u32 val)
>  {
> -	void __iomem *addr;
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&v3_lock, flags);
> -	addr = v3_open_config_window(bus, devfn, where);
> -
> -	switch (size) {
> -	case 1:
> -		__raw_writeb((u8)val, addr);
> -		__raw_readb(addr);
> -		break;
> -
> -	case 2:
> -		__raw_writew((u16)val, addr);
> -		__raw_readw(addr);
> -		break;
> -
> -	case 4:
> -		__raw_writel(val, addr);
> -		__raw_readl(addr);
> -		break;
> -	}
> +	int ret;
>  
> +	v3_open_config_window();
> +	ret = pci_generic_config_write(bus, devfn, where, size, val);
>  	v3_close_config_window();
> -	raw_spin_unlock_irqrestore(&v3_lock, flags);
> -
> -	return PCIBIOS_SUCCESSFUL;
> +	return ret;
>  }
>  
>  static struct pci_ops pci_v3_ops = {
> +	.map_bus = v3_map_bus,
>  	.read	= v3_read_config,
>  	.write	= v3_write_config,
>  };
> @@ -672,8 +650,6 @@ static void __init pci_v3_preinit(void)
>  	hook_fault_code(8, v3_pci_fault, SIGBUS, 0, "external abort on non-linefetch");
>  	hook_fault_code(10, v3_pci_fault, SIGBUS, 0, "external abort on non-linefetch");
>  
> -	raw_spin_lock_irqsave(&v3_lock, flags);
> -
>  	/*
>  	 * Unlock V3 registers, but only if they were previously locked.
>  	 */
> @@ -736,8 +712,6 @@ static void __init pci_v3_preinit(void)
>  	v3_writew(V3_LB_CFG, v3_readw(V3_LB_CFG) | (1 << 10));
>  	v3_writeb(V3_LB_IMASK, 0x28);
>  	__raw_writel(3, ap_syscon_base + INTEGRATOR_SC_PCIENABLE_OFFSET);
> -
> -	raw_spin_unlock_irqrestore(&v3_lock, flags);
>  }
>  
>  static void __init pci_v3_postinit(void)

Re: [PATCH 07/16] ARM: integrator: convert PCI to use generic config accesses

[Linus Walleij]

On Mon, Jan 26, 2015 at 7:22 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Jan 22, 2015 at 02:33:36PM -0600, Bjorn Helgaas wrote:
>> On Mon, Jan 12, 2015 at 01:05:12AM +0100, Linus Walleij wrote:
>> > On Sat, Jan 10, 2015 at 10:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > > On Saturday 10 January 2015 22:40:22 Linus Walleij wrote:
>> > >> >  static int v3_read_config(struct pci_bus *bus, unsigned int devfn, int where,
>> > >> >                           int size, u32 *val)
>> > >> >  {
>> > >> > -       addr = v3_open_config_window(bus, devfn, where);
>> > >> > +       int ret = pci_generic_config_read(bus, devfn, where, size, val);
>> > >> >         v3_close_config_window();
>> > >> > +       return ret;
>> > >> >  }
>> > >> >
>> > >> >  static int v3_write_config(struct pci_bus *bus, unsigned int devfn, int where,
>> > >> >                            int size, u32 val)
>> > >> >  {
>> > >> > +       int ret = pci_generic_config_write(bus, devfn, where, size, val);
>> > >> >         v3_close_config_window();
>> > >> > -       raw_spin_unlock_irqrestore(&v3_lock, flags);
>> > >> > +       return ret;
>> > >> >  }
>> > >> >
>> > >> >  static struct pci_ops pci_v3_ops = {
>> > >> > +       .map_bus = v3_open_config_window,
>> > >> >         .read   = v3_read_config,
>> > >> >         .write  = v3_write_config,
>> > >>
>> > >> So .map_bus is called before every .read/.write operation I take it.
>> > >>
>> > >> Wouldn't it be proper to call the v3_close_config_window() from a
>> > >> matching .unmap_bus() callback for symmetry?
>> > >
>> > > It would be nicer for integrator but useless for anything else, so
>> > > I'd vote for leaving it the way Rob posted.
>> >
>> > OK I buy that, throw in a comment about it in the code if there
>> > is some time for iterating the patch.
>>
>> Would you prefer something like the following instead?  It keeps the
>> v3_open/close symmetry, but it does break apart and duplicate some of the
>> logic from v3_open_config_window().
>
> Hearing nothing, I kept Rob's initial version.  It can be tweaked later if
> anybody wants to.  I just want to get all this into v3.20.

Sorry, I'm OK with either version, this latest iteration looks very nice.
Acked-by.

Yours,
Linus Walleij

[PATCH 08/16] ARM: sa1100: convert PCI to use generic config accesses

[Rob Herring]

Convert the sa1100 nanoengine PCI driver to use the generic config access
functions.

This changes accesses from __raw_readX/__raw_writeX to readX/writeX
variants. The spinlock is removed because it is unnecessary. The config
read and write functions are already protected with a spinlock.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-sa1100/pci-nanoengine.c | 94 +++--------------------------------
 1 file changed, 8 insertions(+), 86 deletions(-)

diff --git a/arch/arm/mach-sa1100/pci-nanoengine.c b/arch/arm/mach-sa1100/pci-nanoengine.c
index b704433..d7ae8d5 100644
--- a/arch/arm/mach-sa1100/pci-nanoengine.c
+++ b/arch/arm/mach-sa1100/pci-nanoengine.c
@@ -22,7 +22,6 @@
 #include <linux/kernel.h>
 #include <linux/irq.h>
 #include <linux/pci.h>
-#include <linux/spinlock.h>
 
 #include <asm/mach/pci.h>
 #include <asm/mach-types.h>
@@ -30,97 +29,20 @@
 #include <mach/nanoengine.h>
 #include <mach/hardware.h>
 
-static DEFINE_SPINLOCK(nano_lock);
-
-static int nanoengine_get_pci_address(struct pci_bus *bus,
-	unsigned int devfn, int where, void __iomem **address)
+static void __iomem *nanoengine_pci_map_bus(struct pci_bus *bus,
+					    unsigned int devfn, int where)
 {
-	int ret = PCIBIOS_DEVICE_NOT_FOUND;
-	unsigned int busnr = bus->number;
+	if (bus->number != 0 || (devfn >> 3) != 0)
+		return NULL;
 
-	*address = (void __iomem *)NANO_PCI_CONFIG_SPACE_VIRT +
+	return (void __iomem *)NANO_PCI_CONFIG_SPACE_VIRT +
 		((bus->number << 16) | (devfn << 8) | (where & ~3));
-
-	ret = (busnr > 255 || devfn > 255 || where > 255) ?
-		PCIBIOS_DEVICE_NOT_FOUND : PCIBIOS_SUCCESSFUL;
-
-	return ret;
-}
-
-static int nanoengine_read_config(struct pci_bus *bus, unsigned int devfn, int where,
-	int size, u32 *val)
-{
-	int ret;
-	void __iomem *address;
-	unsigned long flags;
-	u32 v;
-
-	/* nanoEngine PCI bridge does not return -1 for a non-existing
-	 * device. We must fake the answer. We know that the only valid
-	 * device is device zero at bus 0, which is the network chip. */
-	if (bus->number != 0 || (devfn >> 3) != 0) {
-		v = -1;
-		nanoengine_get_pci_address(bus, devfn, where, &address);
-		goto exit_function;
-	}
-
-	spin_lock_irqsave(&nano_lock, flags);
-
-	ret = nanoengine_get_pci_address(bus, devfn, where, &address);
-	if (ret != PCIBIOS_SUCCESSFUL)
-		return ret;
-	v = __raw_readl(address);
-
-	spin_unlock_irqrestore(&nano_lock, flags);
-
-	v >>= ((where & 3) * 8);
-	v &= (unsigned long)(-1) >> ((4 - size) * 8);
-
-exit_function:
-	*val = v;
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int nanoengine_write_config(struct pci_bus *bus, unsigned int devfn, int where,
-	int size, u32 val)
-{
-	int ret;
-	void __iomem *address;
-	unsigned long flags;
-	unsigned shift;
-	u32 v;
-
-	shift = (where & 3) * 8;
-
-	spin_lock_irqsave(&nano_lock, flags);
-
-	ret = nanoengine_get_pci_address(bus, devfn, where, &address);
-	if (ret != PCIBIOS_SUCCESSFUL)
-		return ret;
-	v = __raw_readl(address);
-	switch (size) {
-	case 1:
-		v &= ~(0xFF << shift);
-		v |= val << shift;
-		break;
-	case 2:
-		v &= ~(0xFFFF << shift);
-		v |= val << shift;
-		break;
-	case 4:
-		v = val;
-		break;
-	}
-	__raw_writel(v, address);
-
-	spin_unlock_irqrestore(&nano_lock, flags);
-
-	return PCIBIOS_SUCCESSFUL;
 }
 
 static struct pci_ops pci_nano_ops = {
-	.read	= nanoengine_read_config,
-	.write	= nanoengine_write_config,
+	.map_bus = nanoengine_pci_map_bus,
+	.read	= pci_generic_config_read32,
+	.write	= pci_generic_config_write32,
 };
 
 static int __init pci_nanoengine_map_irq(const struct pci_dev *dev, u8 slot,
-- 
2.1.0

[PATCH 09/16] ARM: ks8695: convert PCI to use generic config accesses

[Rob Herring]

Convert the ks8695 PCI driver to use the generic config access functions.

This changes accesses from __raw_readX/__raw_writeX to readX/writeX
variants.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Greg Ungerer <gerg@uclinux.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm/mach-ks8695/pci.c | 77 ++++------------------------------------------
 1 file changed, 6 insertions(+), 71 deletions(-)

diff --git a/arch/arm/mach-ks8695/pci.c b/arch/arm/mach-ks8695/pci.c
index bb18193..c1bc4c3 100644
--- a/arch/arm/mach-ks8695/pci.c
+++ b/arch/arm/mach-ks8695/pci.c
@@ -38,8 +38,6 @@
 
 
 static int pci_dbg;
-static int pci_cfg_dbg;
-
 
 static void ks8695_pci_setupconfig(unsigned int bus_nr, unsigned int devfn, unsigned int where)
 {
@@ -59,75 +57,11 @@ static void ks8695_pci_setupconfig(unsigned int bus_nr, unsigned int devfn, unsi
 	}
 }
 
-
-/*
- * The KS8695 datasheet prohibits anything other than 32bit accesses
- * to the IO registers, so all our configuration must be done with
- * 32bit operations, and the correct bit masking and shifting.
- */
-
-static int ks8695_pci_readconfig(struct pci_bus *bus,
-			unsigned int devfn, int where, int size, u32 *value)
-{
-	ks8695_pci_setupconfig(bus->number, devfn, where);
-
-	*value = __raw_readl(KS8695_PCI_VA +  KS8695_PBCD);
-
-	switch (size) {
-		case 4:
-			break;
-		case 2:
-			*value = *value >> ((where & 2) * 8);
-			*value &= 0xffff;
-			break;
-		case 1:
-			*value = *value >> ((where & 3) * 8);
-			*value &= 0xff;
-			break;
-	}
-
-	if (pci_cfg_dbg) {
-		printk("read: %d,%08x,%02x,%d: %08x (%08x)\n",
-			bus->number, devfn, where, size, *value,
-			__raw_readl(KS8695_PCI_VA +  KS8695_PBCD));
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int ks8695_pci_writeconfig(struct pci_bus *bus,
-			unsigned int devfn, int where, int size, u32 value)
+static void __iomem *ks8695_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
+					int where)
 {
-	unsigned long tmp;
-
-	if (pci_cfg_dbg) {
-		printk("write: %d,%08x,%02x,%d: %08x\n",
-			bus->number, devfn, where, size, value);
-	}
-
 	ks8695_pci_setupconfig(bus->number, devfn, where);
-
-	switch (size) {
-		case 4:
-			__raw_writel(value, KS8695_PCI_VA +  KS8695_PBCD);
-			break;
-		case 2:
-			tmp = __raw_readl(KS8695_PCI_VA +  KS8695_PBCD);
-			tmp &= ~(0xffff << ((where & 2) * 8));
-			tmp |= value << ((where & 2) * 8);
-
-			__raw_writel(tmp, KS8695_PCI_VA +  KS8695_PBCD);
-			break;
-		case 1:
-			tmp = __raw_readl(KS8695_PCI_VA +  KS8695_PBCD);
-			tmp &= ~(0xff << ((where & 3) * 8));
-			tmp |= value << ((where & 3) * 8);
-
-			__raw_writel(tmp, KS8695_PCI_VA +  KS8695_PBCD);
-			break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
+	return KS8695_PCI_VA +  KS8695_PBCD;
 }
 
 static void ks8695_local_writeconfig(int where, u32 value)
@@ -137,8 +71,9 @@ static void ks8695_local_writeconfig(int where, u32 value)
 }
 
 static struct pci_ops ks8695_pci_ops = {
-	.read	= ks8695_pci_readconfig,
-	.write	= ks8695_pci_writeconfig,
+	.map_bus = ks8695_pci_map_bus,
+	.read	= pci_generic_config_read32,
+	.write	= pci_generic_config_write32,
 };
 
 static struct resource pci_mem = {
-- 
2.1.0

Re: [PATCH 09/16] ARM: ks8695: convert PCI to use generic config accesses

[Greg Ungerer]

Hi Rob,

On 10/01/15 12:34, Rob Herring wrote:
> Convert the ks8695 PCI driver to use the generic config access functions.
>
> This changes accesses from __raw_readX/__raw_writeX to readX/writeX
> variants.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Greg Ungerer <gerg@uclinux.org>

I wasn't CC'ed on the generic implementation patch ("pci: introduce
common pci config space accessors"), but I assume this is it:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg800405.html

That all looks ok to me. I have not tested it, but:

Acked-by: Greg Ungerer <gerg@uclinux.org>

Regards
Greg


> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>   arch/arm/mach-ks8695/pci.c | 77 ++++------------------------------------------
>   1 file changed, 6 insertions(+), 71 deletions(-)
>
> diff --git a/arch/arm/mach-ks8695/pci.c b/arch/arm/mach-ks8695/pci.c
> index bb18193..c1bc4c3 100644
> --- a/arch/arm/mach-ks8695/pci.c
> +++ b/arch/arm/mach-ks8695/pci.c
> @@ -38,8 +38,6 @@
>   
>   
>   static int pci_dbg;
> -static int pci_cfg_dbg;
> -
>   
>   static void ks8695_pci_setupconfig(unsigned int bus_nr, unsigned int devfn, unsigned int where)
>   {
> @@ -59,75 +57,11 @@ static void ks8695_pci_setupconfig(unsigned int bus_nr, unsigned int devfn, unsi
>   	}
>   }
>   
> -
> -/*
> - * The KS8695 datasheet prohibits anything other than 32bit accesses
> - * to the IO registers, so all our configuration must be done with
> - * 32bit operations, and the correct bit masking and shifting.
> - */
> -
> -static int ks8695_pci_readconfig(struct pci_bus *bus,
> -			unsigned int devfn, int where, int size, u32 *value)
> -{
> -	ks8695_pci_setupconfig(bus->number, devfn, where);
> -
> -	*value = __raw_readl(KS8695_PCI_VA +  KS8695_PBCD);
> -
> -	switch (size) {
> -		case 4:
> -			break;
> -		case 2:
> -			*value = *value >> ((where & 2) * 8);
> -			*value &= 0xffff;
> -			break;
> -		case 1:
> -			*value = *value >> ((where & 3) * 8);
> -			*value &= 0xff;
> -			break;
> -	}
> -
> -	if (pci_cfg_dbg) {
> -		printk("read: %d,%08x,%02x,%d: %08x (%08x)\n",
> -			bus->number, devfn, where, size, *value,
> -			__raw_readl(KS8695_PCI_VA +  KS8695_PBCD));
> -	}
> -
> -	return PCIBIOS_SUCCESSFUL;
> -}
> -
> -static int ks8695_pci_writeconfig(struct pci_bus *bus,
> -			unsigned int devfn, int where, int size, u32 value)
> +static void __iomem *ks8695_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
> +					int where)
>   {
> -	unsigned long tmp;
> -
> -	if (pci_cfg_dbg) {
> -		printk("write: %d,%08x,%02x,%d: %08x\n",
> -			bus->number, devfn, where, size, value);
> -	}
> -
>   	ks8695_pci_setupconfig(bus->number, devfn, where);
> -
> -	switch (size) {
> -		case 4:
> -			__raw_writel(value, KS8695_PCI_VA +  KS8695_PBCD);
> -			break;
> -		case 2:
> -			tmp = __raw_readl(KS8695_PCI_VA +  KS8695_PBCD);
> -			tmp &= ~(0xffff << ((where & 2) * 8));
> -			tmp |= value << ((where & 2) * 8);
> -
> -			__raw_writel(tmp, KS8695_PCI_VA +  KS8695_PBCD);
> -			break;
> -		case 1:
> -			tmp = __raw_readl(KS8695_PCI_VA +  KS8695_PBCD);
> -			tmp &= ~(0xff << ((where & 3) * 8));
> -			tmp |= value << ((where & 3) * 8);
> -
> -			__raw_writel(tmp, KS8695_PCI_VA +  KS8695_PBCD);
> -			break;
> -	}
> -
> -	return PCIBIOS_SUCCESSFUL;
> +	return KS8695_PCI_VA +  KS8695_PBCD;
>   }
>   
>   static void ks8695_local_writeconfig(int where, u32 value)
> @@ -137,8 +71,9 @@ static void ks8695_local_writeconfig(int where, u32 value)
>   }
>   
>   static struct pci_ops ks8695_pci_ops = {
> -	.read	= ks8695_pci_readconfig,
> -	.write	= ks8695_pci_writeconfig,
> +	.map_bus = ks8695_pci_map_bus,
> +	.read	= pci_generic_config_read32,
> +	.write	= pci_generic_config_write32,
>   };
>   
>   static struct resource pci_mem = {

[PATCH 10/16] powerpc: fsl_pci: convert PCI to use generic config accesses

[Rob Herring]

Convert the fsl_pci driver to use the generic config access functions.

This changes accesses from (in|out)_(8|le16|le32) to readX/writeX
variants. I believe these should be equivalent for PCI config space
accesses, but confirmation would be nice.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/sysdev/fsl_pci.c | 46 +++----------------------------------------
 1 file changed, 3 insertions(+), 43 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 6455c1e..271b67e 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -645,61 +645,21 @@ mapped:
 	return pcie->cfg_type1 + offset;
 }
 
-static int mpc83xx_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
-				    int offset, int len, u32 *val)
-{
-	void __iomem *cfg_addr;
-
-	cfg_addr = mpc83xx_pcie_remap_cfg(bus, devfn, offset);
-	if (!cfg_addr)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	switch (len) {
-	case 1:
-		*val = in_8(cfg_addr);
-		break;
-	case 2:
-		*val = in_le16(cfg_addr);
-		break;
-	default:
-		*val = in_le32(cfg_addr);
-		break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
 static int mpc83xx_pcie_write_config(struct pci_bus *bus, unsigned int devfn,
 				     int offset, int len, u32 val)
 {
 	struct pci_controller *hose = pci_bus_to_host(bus);
-	void __iomem *cfg_addr;
-
-	cfg_addr = mpc83xx_pcie_remap_cfg(bus, devfn, offset);
-	if (!cfg_addr)
-		return PCIBIOS_DEVICE_NOT_FOUND;
 
 	/* PPC_INDIRECT_TYPE_SURPRESS_PRIMARY_BUS */
 	if (offset == PCI_PRIMARY_BUS && bus->number == hose->first_busno)
 		val &= 0xffffff00;
 
-	switch (len) {
-	case 1:
-		out_8(cfg_addr, val);
-		break;
-	case 2:
-		out_le16(cfg_addr, val);
-		break;
-	default:
-		out_le32(cfg_addr, val);
-		break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
+	return pci_generic_config_write(bus, devfn, offset, len, val);
 }
 
 static struct pci_ops mpc83xx_pcie_ops = {
-	.read = mpc83xx_pcie_read_config,
+	.map_bus = mpc83xx_pcie_remap_cfg,
+	.read = pci_generic_config_read,
 	.write = mpc83xx_pcie_write_config,
 };
 
-- 
2.1.0

[PATCH 11/16] powerpc: powermac: convert PCI to use generic config accesses

[Rob Herring]

Convert the powermac PCI driver to use the generic config access functions.

This changes accesses from (in|out)_(8|le16|le32) to readX/writeX
variants. I believe these should be equivalent for PCI config space
accesses, but confirmation would be nice.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/platforms/powermac/pci.c | 209 +++++++---------------------------
 1 file changed, 39 insertions(+), 170 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/pci.c b/arch/powerpc/platforms/powermac/pci.c
index 04702db..f4071a6 100644
--- a/arch/powerpc/platforms/powermac/pci.c
+++ b/arch/powerpc/platforms/powermac/pci.c
@@ -133,17 +133,23 @@ static void __init fixup_bus_range(struct device_node *bridge)
 	|(((unsigned int)(off)) & 0xFCUL) \
 	|1UL)
 
-static volatile void __iomem *macrisc_cfg_access(struct pci_controller* hose,
-					       u8 bus, u8 dev_fn, u8 offset)
+static void __iomem *macrisc_cfg_map_bus(struct pci_bus *bus,
+					 unsigned int dev_fn,
+					 int offset)
 {
 	unsigned int caddr;
+	struct pci_controller *hose;
 
-	if (bus == hose->first_busno) {
+	hose = pci_bus_to_host(bus);
+	if (hose == NULL)
+		return NULL;
+
+	if (bus->number == hose->first_busno) {
 		if (dev_fn < (11 << 3))
 			return NULL;
 		caddr = MACRISC_CFA0(dev_fn, offset);
 	} else
-		caddr = MACRISC_CFA1(bus, dev_fn, offset);
+		caddr = MACRISC_CFA1(bus->number, dev_fn, offset);
 
 	/* Uninorth will return garbage if we don't read back the value ! */
 	do {
@@ -154,129 +160,46 @@ static volatile void __iomem *macrisc_cfg_access(struct pci_controller* hose,
 	return hose->cfg_data + offset;
 }
 
-static int macrisc_read_config(struct pci_bus *bus, unsigned int devfn,
-				      int offset, int len, u32 *val)
-{
-	struct pci_controller *hose;
-	volatile void __iomem *addr;
-
-	hose = pci_bus_to_host(bus);
-	if (hose == NULL)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	if (offset >= 0x100)
-		return  PCIBIOS_BAD_REGISTER_NUMBER;
-	addr = macrisc_cfg_access(hose, bus->number, devfn, offset);
-	if (!addr)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	/*
-	 * Note: the caller has already checked that offset is
-	 * suitably aligned and that len is 1, 2 or 4.
-	 */
-	switch (len) {
-	case 1:
-		*val = in_8(addr);
-		break;
-	case 2:
-		*val = in_le16(addr);
-		break;
-	default:
-		*val = in_le32(addr);
-		break;
-	}
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int macrisc_write_config(struct pci_bus *bus, unsigned int devfn,
-				       int offset, int len, u32 val)
-{
-	struct pci_controller *hose;
-	volatile void __iomem *addr;
-
-	hose = pci_bus_to_host(bus);
-	if (hose == NULL)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	if (offset >= 0x100)
-		return  PCIBIOS_BAD_REGISTER_NUMBER;
-	addr = macrisc_cfg_access(hose, bus->number, devfn, offset);
-	if (!addr)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	/*
-	 * Note: the caller has already checked that offset is
-	 * suitably aligned and that len is 1, 2 or 4.
-	 */
-	switch (len) {
-	case 1:
-		out_8(addr, val);
-		break;
-	case 2:
-		out_le16(addr, val);
-		break;
-	default:
-		out_le32(addr, val);
-		break;
-	}
-	return PCIBIOS_SUCCESSFUL;
-}
-
 static struct pci_ops macrisc_pci_ops =
 {
-	.read = macrisc_read_config,
-	.write = macrisc_write_config,
+	.map_bus = macrisc_cfg_map_bus,
+	.read = pci_generic_config_read,
+	.write = pci_generic_config_write,
 };
 
 #ifdef CONFIG_PPC32
 /*
  * Verify that a specific (bus, dev_fn) exists on chaos
  */
-static int chaos_validate_dev(struct pci_bus *bus, int devfn, int offset)
+static void __iomem *chaos_map_bus(struct pci_bus *bus, unsigned int devfn,
+				   int offset)
 {
 	struct device_node *np;
 	const u32 *vendor, *device;
 
 	if (offset >= 0x100)
-		return  PCIBIOS_BAD_REGISTER_NUMBER;
+		return NULL;
 	np = of_pci_find_child_device(bus->dev.of_node, devfn);
 	if (np == NULL)
-		return PCIBIOS_DEVICE_NOT_FOUND;
+		return NULL;
 
 	vendor = of_get_property(np, "vendor-id", NULL);
 	device = of_get_property(np, "device-id", NULL);
 	if (vendor == NULL || device == NULL)
-		return PCIBIOS_DEVICE_NOT_FOUND;
+		return NULL;
 
 	if ((*vendor == 0x106b) && (*device == 3) && (offset >= 0x10)
 	    && (offset != 0x14) && (offset != 0x18) && (offset <= 0x24))
-		return PCIBIOS_BAD_REGISTER_NUMBER;
-
-	return PCIBIOS_SUCCESSFUL;
-}
+		return NULL;
 
-static int
-chaos_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
-		  int len, u32 *val)
-{
-	int result = chaos_validate_dev(bus, devfn, offset);
-	if (result == PCIBIOS_BAD_REGISTER_NUMBER)
-		*val = ~0U;
-	if (result != PCIBIOS_SUCCESSFUL)
-		return result;
-	return macrisc_read_config(bus, devfn, offset, len, val);
-}
-
-static int
-chaos_write_config(struct pci_bus *bus, unsigned int devfn, int offset,
-		   int len, u32 val)
-{
-	int result = chaos_validate_dev(bus, devfn, offset);
-	if (result != PCIBIOS_SUCCESSFUL)
-		return result;
-	return macrisc_write_config(bus, devfn, offset, len, val);
+	return macrisc_cfg_map_bus(bus, devfn, offset);
 }
 
 static struct pci_ops chaos_pci_ops =
 {
-	.read = chaos_read_config,
-	.write = chaos_write_config,
+	.map_bus = chaos_map_bus,
+	.read = pci_generic_config_read,
+	.write = pci_generic_config_write,
 };
 
 static void __init setup_chaos(struct pci_controller *hose,
@@ -471,15 +394,24 @@ static struct pci_ops u3_ht_pci_ops =
 	 |(((unsigned int)(off)) & 0xfcU)	\
 	 |1UL)
 
-static volatile void __iomem *u4_pcie_cfg_access(struct pci_controller* hose,
-					u8 bus, u8 dev_fn, int offset)
+static void __iomem *u4_pcie_cfg_map_bus(struct pci_bus *bus,
+					 unsigned int dev_fn,
+					 int offset)
 {
+	struct pci_controller *hose;
 	unsigned int caddr;
 
-	if (bus == hose->first_busno) {
+	if (offset >= 0x1000)
+		return NULL;
+
+	hose = pci_bus_to_host(bus);
+	if (!hose)
+		return NULL;
+
+	if (bus->number == hose->first_busno) {
 		caddr = U4_PCIE_CFA0(dev_fn, offset);
 	} else
-		caddr = U4_PCIE_CFA1(bus, dev_fn, offset);
+		caddr = U4_PCIE_CFA1(bus->number, dev_fn, offset);
 
 	/* Uninorth will return garbage if we don't read back the value ! */
 	do {
@@ -490,74 +422,11 @@ static volatile void __iomem *u4_pcie_cfg_access(struct pci_controller* hose,
 	return hose->cfg_data + offset;
 }
 
-static int u4_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
-			       int offset, int len, u32 *val)
-{
-	struct pci_controller *hose;
-	volatile void __iomem *addr;
-
-	hose = pci_bus_to_host(bus);
-	if (hose == NULL)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	if (offset >= 0x1000)
-		return  PCIBIOS_BAD_REGISTER_NUMBER;
-	addr = u4_pcie_cfg_access(hose, bus->number, devfn, offset);
-	if (!addr)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	/*
-	 * Note: the caller has already checked that offset is
-	 * suitably aligned and that len is 1, 2 or 4.
-	 */
-	switch (len) {
-	case 1:
-		*val = in_8(addr);
-		break;
-	case 2:
-		*val = in_le16(addr);
-		break;
-	default:
-		*val = in_le32(addr);
-		break;
-	}
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int u4_pcie_write_config(struct pci_bus *bus, unsigned int devfn,
-				int offset, int len, u32 val)
-{
-	struct pci_controller *hose;
-	volatile void __iomem *addr;
-
-	hose = pci_bus_to_host(bus);
-	if (hose == NULL)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	if (offset >= 0x1000)
-		return  PCIBIOS_BAD_REGISTER_NUMBER;
-	addr = u4_pcie_cfg_access(hose, bus->number, devfn, offset);
-	if (!addr)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	/*
-	 * Note: the caller has already checked that offset is
-	 * suitably aligned and that len is 1, 2 or 4.
-	 */
-	switch (len) {
-	case 1:
-		out_8(addr, val);
-		break;
-	case 2:
-		out_le16(addr, val);
-		break;
-	default:
-		out_le32(addr, val);
-		break;
-	}
-	return PCIBIOS_SUCCESSFUL;
-}
-
 static struct pci_ops u4_pcie_pci_ops =
 {
-	.read = u4_pcie_read_config,
-	.write = u4_pcie_write_config,
+	.map_bus = u4_pcie_cfg_map_bus,
+	.read = pci_generic_config_read,
+	.write = pci_generic_config_write,
 };
 
 static void pmac_pci_fixup_u4_of_node(struct pci_dev *dev)
-- 
2.1.0

[PATCH 12/16] pci/host: generic: convert to use generic config accesses

[Rob Herring]

Convert the generic host PCI driver to use the generic config access
functions.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/pci/host/pci-host-generic.c | 51 +++----------------------------------
 1 file changed, 3 insertions(+), 48 deletions(-)

diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 6eb1aa7..925e29e 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -76,55 +76,9 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = {
 	.map_bus	= gen_pci_map_cfg_bus_ecam,
 };
 
-static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn,
-				int where, int size, u32 *val)
-{
-	void __iomem *addr;
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
-
-	addr = pci->cfg.ops->map_bus(bus, devfn, where);
-
-	switch (size) {
-	case 1:
-		*val = readb(addr);
-		break;
-	case 2:
-		*val = readw(addr);
-		break;
-	default:
-		*val = readl(addr);
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
-				 int where, int size, u32 val)
-{
-	void __iomem *addr;
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
-
-	addr = pci->cfg.ops->map_bus(bus, devfn, where);
-
-	switch (size) {
-	case 1:
-		writeb(val, addr);
-		break;
-	case 2:
-		writew(val, addr);
-		break;
-	default:
-		writel(val, addr);
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
 static struct pci_ops gen_pci_ops = {
-	.read	= gen_pci_config_read,
-	.write	= gen_pci_config_write,
+	.read	= pci_generic_config_read,
+	.write	= pci_generic_config_write,
 };
 
 static const struct of_device_id gen_pci_of_match[] = {
@@ -287,6 +241,7 @@ static int gen_pci_probe(struct platform_device *pdev)
 
 	of_id = of_match_node(gen_pci_of_match, np);
 	pci->cfg.ops = of_id->data;
+	gen_pci_ops.map_bus = pci->cfg.ops->map_bus;
 	pci->host.dev.parent = dev;
 	INIT_LIST_HEAD(&pci->host.windows);
 	INIT_LIST_HEAD(&pci->resources);
-- 
2.1.0

Re: [PATCH 12/16] pci/host: generic: convert to use generic config accesses

[Will Deacon]

On Sat, Jan 10, 2015 at 02:34:46AM +0000, Rob Herring wrote:
> Convert the generic host PCI driver to use the generic config access
> functions.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/pci/host/pci-host-generic.c | 51 +++----------------------------------
>  1 file changed, 3 insertions(+), 48 deletions(-)

Looks like a fairly straightforward conversion for this driver!

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 6eb1aa7..925e29e 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -76,55 +76,9 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = {
>  	.map_bus	= gen_pci_map_cfg_bus_ecam,
>  };
>  
> -static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn,
> -				int where, int size, u32 *val)
> -{
> -	void __iomem *addr;
> -	struct pci_sys_data *sys = bus->sysdata;
> -	struct gen_pci *pci = sys->private_data;
> -
> -	addr = pci->cfg.ops->map_bus(bus, devfn, where);
> -
> -	switch (size) {
> -	case 1:
> -		*val = readb(addr);
> -		break;
> -	case 2:
> -		*val = readw(addr);
> -		break;
> -	default:
> -		*val = readl(addr);
> -	}
> -
> -	return PCIBIOS_SUCCESSFUL;
> -}
> -
> -static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
> -				 int where, int size, u32 val)
> -{
> -	void __iomem *addr;
> -	struct pci_sys_data *sys = bus->sysdata;
> -	struct gen_pci *pci = sys->private_data;
> -
> -	addr = pci->cfg.ops->map_bus(bus, devfn, where);
> -
> -	switch (size) {
> -	case 1:
> -		writeb(val, addr);
> -		break;
> -	case 2:
> -		writew(val, addr);
> -		break;
> -	default:
> -		writel(val, addr);
> -	}
> -
> -	return PCIBIOS_SUCCESSFUL;
> -}
> -
>  static struct pci_ops gen_pci_ops = {
> -	.read	= gen_pci_config_read,
> -	.write	= gen_pci_config_write,
> +	.read	= pci_generic_config_read,
> +	.write	= pci_generic_config_write,
>  };
>  
>  static const struct of_device_id gen_pci_of_match[] = {
> @@ -287,6 +241,7 @@ static int gen_pci_probe(struct platform_device *pdev)
>  
>  	of_id = of_match_node(gen_pci_of_match, np);
>  	pci->cfg.ops = of_id->data;
> +	gen_pci_ops.map_bus = pci->cfg.ops->map_bus;
>  	pci->host.dev.parent = dev;
>  	INIT_LIST_HEAD(&pci->host.windows);
>  	INIT_LIST_HEAD(&pci->resources);
> -- 
> 2.1.0
> 
> 

[PATCH 13/16] pci/host: rcar-gen2: convert to use generic config accesses

[Rob Herring]

Convert the rcar-gen2 host PCI driver to use the generic config access
functions.

This changes the i/o accessors from io(read|write)X to readX/writeX
variants which are equivalent on ARM.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Simon Horman <horms@verge.net.au>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-sh@vger.kernel.org
---
 drivers/pci/host/pci-rcar-gen2.c | 51 +++-------------------------------------
 1 file changed, 3 insertions(+), 48 deletions(-)

diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
index d9c042f..dd6b84e 100644
--- a/drivers/pci/host/pci-rcar-gen2.c
+++ b/drivers/pci/host/pci-rcar-gen2.c
@@ -131,52 +131,6 @@ static void __iomem *rcar_pci_cfg_base(struct pci_bus *bus, unsigned int devfn,
 	return priv->reg + (slot >> 1) * 0x100 + where;
 }
 
-static int rcar_pci_read_config(struct pci_bus *bus, unsigned int devfn,
-				int where, int size, u32 *val)
-{
-	void __iomem *reg = rcar_pci_cfg_base(bus, devfn, where);
-
-	if (!reg)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	switch (size) {
-	case 1:
-		*val = ioread8(reg);
-		break;
-	case 2:
-		*val = ioread16(reg);
-		break;
-	default:
-		*val = ioread32(reg);
-		break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int rcar_pci_write_config(struct pci_bus *bus, unsigned int devfn,
-				 int where, int size, u32 val)
-{
-	void __iomem *reg = rcar_pci_cfg_base(bus, devfn, where);
-
-	if (!reg)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	switch (size) {
-	case 1:
-		iowrite8(val, reg);
-		break;
-	case 2:
-		iowrite16(val, reg);
-		break;
-	default:
-		iowrite32(val, reg);
-		break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
 /* PCI interrupt mapping */
 static int rcar_pci_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 {
@@ -325,8 +279,9 @@ static int rcar_pci_setup(int nr, struct pci_sys_data *sys)
 }
 
 static struct pci_ops rcar_pci_ops = {
-	.read	= rcar_pci_read_config,
-	.write	= rcar_pci_write_config,
+	.map_bus = rcar_pci_cfg_base,
+	.read	= pci_generic_config_read,
+	.write	= pci_generic_config_write,
 };
 
 static int rcar_pci_probe(struct platform_device *pdev)
-- 
2.1.0

Re: [PATCH 13/16] pci/host: rcar-gen2: convert to use generic config accesses

[Geert Uytterhoeven]

On Sat, Jan 10, 2015 at 3:34 AM, Rob Herring <robh@kernel.org> wrote:
> Convert the rcar-gen2 host PCI driver to use the generic config access
> functions.
>
> This changes the i/o accessors from io(read|write)X to readX/writeX
> variants which are equivalent on ARM.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-sh@vger.kernel.org

Thanks, look OK to me.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 14/16] pci/host: tegra: convert to use generic config accesses

[Rob Herring]

Convert the tegra host PCI driver to use the generic config access
functions.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: linux-tegra@vger.kernel.org
Cc: linux-pci@vger.kernel.org
---
 drivers/pci/host/pci-tegra.c | 55 +++-----------------------------------------
 1 file changed, 3 insertions(+), 52 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index a800ae9..44fd31b 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -480,59 +480,10 @@ static void __iomem *tegra_pcie_conf_address(struct pci_bus *bus,
 	return addr;
 }
 
-static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
-				int where, int size, u32 *value)
-{
-	void __iomem *addr;
-
-	addr = tegra_pcie_conf_address(bus, devfn, where);
-	if (!addr) {
-		*value = 0xffffffff;
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	}
-
-	*value = readl(addr);
-
-	if (size == 1)
-		*value = (*value >> (8 * (where & 3))) & 0xff;
-	else if (size == 2)
-		*value = (*value >> (8 * (where & 3))) & 0xffff;
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int tegra_pcie_write_conf(struct pci_bus *bus, unsigned int devfn,
-				 int where, int size, u32 value)
-{
-	void __iomem *addr;
-	u32 mask, tmp;
-
-	addr = tegra_pcie_conf_address(bus, devfn, where);
-	if (!addr)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	if (size == 4) {
-		writel(value, addr);
-		return PCIBIOS_SUCCESSFUL;
-	}
-
-	if (size == 2)
-		mask = ~(0xffff << ((where & 0x3) * 8));
-	else if (size == 1)
-		mask = ~(0xff << ((where & 0x3) * 8));
-	else
-		return PCIBIOS_BAD_REGISTER_NUMBER;
-
-	tmp = readl(addr) & mask;
-	tmp |= value << ((where & 0x3) * 8);
-	writel(tmp, addr);
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
 static struct pci_ops tegra_pcie_ops = {
-	.read = tegra_pcie_read_conf,
-	.write = tegra_pcie_write_conf,
+	.map_bus = tegra_pcie_conf_address,
+	.read = pci_generic_config_read32,
+	.write = pci_generic_config_write32,
 };
 
 static unsigned long tegra_pcie_port_get_pex_ctrl(struct tegra_pcie_port *port)
-- 
2.1.0

Re: [PATCH 14/16] pci/host: tegra: convert to use generic config accesses

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

On Fri, Jan 09, 2015 at 08:34:48PM -0600, Rob Herring wrote:
> Convert the tegra host PCI driver to use the generic config access

Nit: s/tegra/Tegra/

Other than than looks like great cleanup:

Tested-by: Thierry Reding <treding@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 15/16] pci/host: xgene: convert to use generic config accesses

[Rob Herring]

Convert the xgene host PCI driver to use the generic config access
functions.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Tanmay Inamdar <tinamdar@apm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/pci/host/pci-xgene.c | 150 +++----------------------------------------
 1 file changed, 9 insertions(+), 141 deletions(-)

diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index b1d0596..ee6a3d3 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -74,92 +74,6 @@ static inline u32 pcie_bar_low_val(u32 addr, u32 flags)
 	return (addr & PCI_BASE_ADDRESS_MEM_MASK) | flags;
 }
 
-/* PCIe Configuration Out/In */
-static inline void xgene_pcie_cfg_out32(void __iomem *addr, int offset, u32 val)
-{
-	writel(val, addr + offset);
-}
-
-static inline void xgene_pcie_cfg_out16(void __iomem *addr, int offset, u16 val)
-{
-	u32 val32 = readl(addr + (offset & ~0x3));
-
-	switch (offset & 0x3) {
-	case 2:
-		val32 &= ~0xFFFF0000;
-		val32 |= (u32)val << 16;
-		break;
-	case 0:
-	default:
-		val32 &= ~0xFFFF;
-		val32 |= val;
-		break;
-	}
-	writel(val32, addr + (offset & ~0x3));
-}
-
-static inline void xgene_pcie_cfg_out8(void __iomem *addr, int offset, u8 val)
-{
-	u32 val32 = readl(addr + (offset & ~0x3));
-
-	switch (offset & 0x3) {
-	case 0:
-		val32 &= ~0xFF;
-		val32 |= val;
-		break;
-	case 1:
-		val32 &= ~0xFF00;
-		val32 |= (u32)val << 8;
-		break;
-	case 2:
-		val32 &= ~0xFF0000;
-		val32 |= (u32)val << 16;
-		break;
-	case 3:
-	default:
-		val32 &= ~0xFF000000;
-		val32 |= (u32)val << 24;
-		break;
-	}
-	writel(val32, addr + (offset & ~0x3));
-}
-
-static inline void xgene_pcie_cfg_in32(void __iomem *addr, int offset, u32 *val)
-{
-	*val = readl(addr + offset);
-}
-
-static inline void xgene_pcie_cfg_in16(void __iomem *addr, int offset, u32 *val)
-{
-	*val = readl(addr + (offset & ~0x3));
-
-	switch (offset & 0x3) {
-	case 2:
-		*val >>= 16;
-		break;
-	}
-
-	*val &= 0xFFFF;
-}
-
-static inline void xgene_pcie_cfg_in8(void __iomem *addr, int offset, u32 *val)
-{
-	*val = readl(addr + (offset & ~0x3));
-
-	switch (offset & 0x3) {
-	case 3:
-		*val = *val >> 24;
-		break;
-	case 2:
-		*val = *val >> 16;
-		break;
-	case 1:
-		*val = *val >> 8;
-		break;
-	}
-	*val &= 0xFF;
-}
-
 /*
  * When the address bit [17:16] is 2'b01, the Configuration access will be
  * treated as Type 1 and it will be forwarded to external PCIe device.
@@ -213,69 +127,23 @@ static bool xgene_pcie_hide_rc_bars(struct pci_bus *bus, int offset)
 	return false;
 }
 
-static int xgene_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
-				  int offset, int len, u32 *val)
-{
-	struct xgene_pcie_port *port = bus->sysdata;
-	void __iomem *addr;
-
-	if ((pci_is_root_bus(bus) && devfn != 0) || !port->link_up)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	if (xgene_pcie_hide_rc_bars(bus, offset)) {
-		*val = 0;
-		return PCIBIOS_SUCCESSFUL;
-	}
-
-	xgene_pcie_set_rtdid_reg(bus, devfn);
-	addr = xgene_pcie_get_cfg_base(bus);
-	switch (len) {
-	case 1:
-		xgene_pcie_cfg_in8(addr, offset, val);
-		break;
-	case 2:
-		xgene_pcie_cfg_in16(addr, offset, val);
-		break;
-	default:
-		xgene_pcie_cfg_in32(addr, offset, val);
-		break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
-static int xgene_pcie_write_config(struct pci_bus *bus, unsigned int devfn,
-				   int offset, int len, u32 val)
+static int xgene_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
+			      int offset)
 {
 	struct xgene_pcie_port *port = bus->sysdata;
-	void __iomem *addr;
 
-	if ((pci_is_root_bus(bus) && devfn != 0) || !port->link_up)
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	if (xgene_pcie_hide_rc_bars(bus, offset))
-		return PCIBIOS_SUCCESSFUL;
+	if ((pci_is_root_bus(bus) && devfn != 0) || !port->link_up ||
+	    xgene_pcie_hide_rc_bars(bus, offset))
+		return NULL;
 
 	xgene_pcie_set_rtdid_reg(bus, devfn);
-	addr = xgene_pcie_get_cfg_base(bus);
-	switch (len) {
-	case 1:
-		xgene_pcie_cfg_out8(addr, offset, (u8)val);
-		break;
-	case 2:
-		xgene_pcie_cfg_out16(addr, offset, (u16)val);
-		break;
-	default:
-		xgene_pcie_cfg_out32(addr, offset, val);
-		break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
+	return xgene_pcie_get_cfg_base(bus);
 }
 
 static struct pci_ops xgene_pcie_ops = {
-	.read = xgene_pcie_read_config,
-	.write = xgene_pcie_write_config
+	.map_bus = xgene_pcie_map_bus,
+	.read = pci_generic_config_read32,
+	.write = pci_generic_config_write32,
 };
 
 static u64 xgene_pcie_set_ib_mask(void __iomem *csr_base, u32 addr,
-- 
2.1.0

[PATCH 16/16] pci/host: xilinx: convert to use generic config accesses

[Rob Herring]

Convert the Xilinx host PCI driver to use the generic config access
functions.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: "Sören Brinkmann" <soren.brinkmann@xilinx.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/pci/host/pcie-xilinx.c | 88 +++++-------------------------------------
 1 file changed, 9 insertions(+), 79 deletions(-)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index ef3ebaf..f67d036 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -189,7 +189,7 @@ static bool xilinx_pcie_valid_device(struct pci_bus *bus, unsigned int devfn)
 }
 
 /**
- * xilinx_pcie_config_base - Get configuration base
+ * xilinx_pcie_map_bus - Get configuration base
  * @bus: PCI Bus structure
  * @devfn: Device/function
  * @where: Offset from base
@@ -197,96 +197,26 @@ static bool xilinx_pcie_valid_device(struct pci_bus *bus, unsigned int devfn)
  * Return: Base address of the configuration space needed to be
  *	   accessed.
  */
-static void __iomem *xilinx_pcie_config_base(struct pci_bus *bus,
-					     unsigned int devfn, int where)
+static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
+					 unsigned int devfn, int where)
 {
 	struct xilinx_pcie_port *port = sys_to_pcie(bus->sysdata);
 	int relbus;
 
+	if (!xilinx_pcie_valid_device(bus, devfn))
+		return NULL;
+
 	relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
 		 (devfn << ECAM_DEV_NUM_SHIFT);
 
 	return port->reg_base + relbus + where;
 }
 
-/**
- * xilinx_pcie_read_config - Read configuration space
- * @bus: PCI Bus structure
- * @devfn: Device/function
- * @where: Offset from base
- * @size: Byte/word/dword
- * @val: Value to be read
- *
- * Return: PCIBIOS_SUCCESSFUL on success
- *	   PCIBIOS_DEVICE_NOT_FOUND on failure
- */
-static int xilinx_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
-				   int where, int size, u32 *val)
-{
-	void __iomem *addr;
-
-	if (!xilinx_pcie_valid_device(bus, devfn)) {
-		*val = 0xFFFFFFFF;
-		return PCIBIOS_DEVICE_NOT_FOUND;
-	}
-
-	addr = xilinx_pcie_config_base(bus, devfn, where);
-
-	switch (size) {
-	case 1:
-		*val = readb(addr);
-		break;
-	case 2:
-		*val = readw(addr);
-		break;
-	default:
-		*val = readl(addr);
-		break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
-/**
- * xilinx_pcie_write_config - Write configuration space
- * @bus: PCI Bus structure
- * @devfn: Device/function
- * @where: Offset from base
- * @size: Byte/word/dword
- * @val: Value to be written to device
- *
- * Return: PCIBIOS_SUCCESSFUL on success
- *	   PCIBIOS_DEVICE_NOT_FOUND on failure
- */
-static int xilinx_pcie_write_config(struct pci_bus *bus, unsigned int devfn,
-				    int where, int size, u32 val)
-{
-	void __iomem *addr;
-
-	if (!xilinx_pcie_valid_device(bus, devfn))
-		return PCIBIOS_DEVICE_NOT_FOUND;
-
-	addr = xilinx_pcie_config_base(bus, devfn, where);
-
-	switch (size) {
-	case 1:
-		writeb(val, addr);
-		break;
-	case 2:
-		writew(val, addr);
-		break;
-	default:
-		writel(val, addr);
-		break;
-	}
-
-	return PCIBIOS_SUCCESSFUL;
-}
-
 /* PCIe operations */
 static struct pci_ops xilinx_pcie_ops = {
-	.read  = xilinx_pcie_read_config,
-	.write = xilinx_pcie_write_config,
+	.map_bus = xilinx_pcie_map_bus,
+	.read	= pci_generic_config_read,
+	.write	= pci_generic_config_write,
 };
 
 /* MSI functions */
-- 
2.1.0

Re: [PATCH 00/16] PCI generic configuration space accessors

[Bjorn Helgaas]

On Fri, Jan 09, 2015 at 08:34:34PM -0600, Rob Herring wrote:
> This series adds common accessor functions for PCI configuration space
> accesses. This supports most PCI hosts with memory mapped configuration
> space like ECAM or hosts with memory mapped address/data registers. ECAM
> is not generically supported by this series, but could be added on top
> of this. While some hosts have standard address decoding which could be 
> common as well, the various checks on bus numbers and device numbers are 
> quite varied. It is unclear how much of that is really necessary or 
> could be common. 
> 
> The first 4 patches are preparatory cleanup. Patch 5 introduces the
> common accessors. The remaining patches convert several PCI host
> controllers. This is in no way a complete list of host controllers. The
> conversion of more hosts should be possible. The Designware controller
> in particular should be able to be converted, but its config space
> accessors are a mess of override-able functions that I've not gotten my
> head around.
> 
> This series is available here [1].
> 
> Rob
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git pci-config-access
> 
> Rob Herring (16):
>   frv: add struct pci_ops member names to initialization
>   mips: add struct pci_ops member names to initialization
>   mn10300: add struct pci_ops member names to initialization
>   powerpc: add struct pci_ops member names to initialization
>   pci: introduce common pci config space accessors
>   ARM: cns3xxx: convert PCI to use generic config accesses
>   ARM: integrator: convert PCI to use generic config accesses
>   ARM: sa1100: convert PCI to use generic config accesses
>   ARM: ks8695: convert PCI to use generic config accesses
>   powerpc: fsl_pci: convert PCI to use generic config accesses
>   powerpc: powermac: convert PCI to use generic config accesses
>   pci/host: generic: convert to use generic config accesses
>   pci/host: rcar-gen2: convert to use generic config accesses
>   pci/host: tegra: convert to use generic config accesses
>   pci/host: xgene: convert to use generic config accesses
>   pci/host: xilinx: convert to use generic config accesses
> 
>  arch/arm/mach-cns3xxx/pcie.c                   |  52 ++----
>  arch/arm/mach-integrator/pci_v3.c              |  61 +-------
>  arch/arm/mach-ks8695/pci.c                     |  77 +--------
>  arch/arm/mach-sa1100/pci-nanoengine.c          |  94 +----------
>  arch/frv/mb93090-mb00/pci-vdk.c                |   4 +-
>  arch/mips/pci/pci-bcm1480.c                    |   4 +-
>  arch/mips/pci/pci-octeon.c                     |   4 +-
>  arch/mips/pci/pcie-octeon.c                    |  12 +-
>  arch/mn10300/unit-asb2305/pci.c                |   4 +-
>  arch/powerpc/platforms/cell/celleb_scc_pciex.c |   4 +-
>  arch/powerpc/platforms/powermac/pci.c          | 209 +++++--------------------
>  arch/powerpc/sysdev/fsl_pci.c                  |  46 +-----
>  drivers/pci/access.c                           |  87 ++++++++++
>  drivers/pci/host/pci-host-generic.c            |  51 +-----
>  drivers/pci/host/pci-rcar-gen2.c               |  51 +-----
>  drivers/pci/host/pci-tegra.c                   |  55 +------
>  drivers/pci/host/pci-xgene.c                   | 150 ++----------------
>  drivers/pci/host/pcie-xilinx.c                 |  88 ++---------
>  include/linux/pci.h                            |  11 ++
>  19 files changed, 212 insertions(+), 852 deletions(-)

Really nice cleanups.  I added these with the acks so far to a pci/config
branch for v3.20.  I'll update it with more acks if they trickle in.

You've structured it nicely so I can also just drop individual arch pieces
if necessary.  The pieces that haven't been acked yet (hint, hint):

    arch/arm/mach-cns3xxx/pcie.c
    arch/arm/mach-sa1100/pci-nanoengine.c
    arch/mn10300/unit-asb2305/pci.c
    arch/powerpc

In addition, nobody has acked the frv and mips parts, but they're trivial
(they only add struct member names) that I can apply them without worrying.

Bjorn

Re: [PATCH 00/16] PCI generic configuration space accessors

[Rob Herring]

On Thu, Jan 22, 2015 at 3:03 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Jan 09, 2015 at 08:34:34PM -0600, Rob Herring wrote:
>> This series adds common accessor functions for PCI configuration space
>> accesses. This supports most PCI hosts with memory mapped configuration
>> space like ECAM or hosts with memory mapped address/data registers. ECAM
>> is not generically supported by this series, but could be added on top
>> of this. While some hosts have standard address decoding which could be
>> common as well, the various checks on bus numbers and device numbers are
>> quite varied. It is unclear how much of that is really necessary or
>> could be common.
>>
>> The first 4 patches are preparatory cleanup. Patch 5 introduces the
>> common accessors. The remaining patches convert several PCI host
>> controllers. This is in no way a complete list of host controllers. The
>> conversion of more hosts should be possible. The Designware controller
>> in particular should be able to be converted, but its config space
>> accessors are a mess of override-able functions that I've not gotten my
>> head around.

[...]

> Really nice cleanups.  I added these with the acks so far to a pci/config
> branch for v3.20.  I'll update it with more acks if they trickle in.

Thanks.

> You've structured it nicely so I can also just drop individual arch pieces
> if necessary.  The pieces that haven't been acked yet (hint, hint):
>
>     arch/arm/mach-cns3xxx/pcie.c
>     arch/arm/mach-sa1100/pci-nanoengine.c

Some ARM sub-arch maintainers tend to not respond on things. These are
platforms which aren't very active and aren't going to move to
drivers/pci/host/ anytime soon. Maybe Arnd wants to ack them.

>     arch/mn10300/unit-asb2305/pci.c
>     arch/powerpc
>
> In addition, nobody has acked the frv and mips parts, but they're trivial
> (they only add struct member names) that I can apply them without worrying.

You must pick-up the 4 clean-up ones or the build will break for those
platforms. Or perhaps that will encourage some acks.

I've also got some actual conversions for some MIPS platforms in my
tree I haven't sent out yet. MIPS is fun with all the variety of
endianness and h/w swapping capability or not.

Rob