Broadcom iProc PCIe fixes and outbound mapping support

PATCH Applied Ray Jui 5 participants 23 messages Sep 16, 2015 – Oct 16, 2015

Subsystems (4)

Device tree bindings PCI PCI native host bridge and endpoint drivers Broadcom iproc Arm architecture

lore

Share & tools

lore

Ray Jui

author maintainer

2015-09-16 00:39 UTC #0 lore raw

cover Broadcom iProc PCIe fixes and outbound mapping support

This patch series contains various fixes and outbound mapping support for the Broadcom iProc PCIe driver. Some of the critical fixes include 1) fix of PCIe core reset logic and therefore remove its dependency on the bootloader;
2) improved link detection logic that works for more iProc based SoCs.

This patch series also adds the outbound address mapping support. While outbound address mapping support is not required on chips like North Star, Cygnus, and etc., some of the newer iProc based chips like North Star 2 require this support to properly map the AXI address into the address used in the iProc PCIe core

This patch series is constructed based on Linux v4.3-rc1 (with workaround to disable calls to pci_read_bridge_bases in pci/probe.c that breaks some of the ARM based PCIe devices including iProc)

This patch series is tested on the following platforms with an Intel e1000e based PCIe x1 NIC card:
- ARM32 based Cygnus BCM958305K Wireless Audio board
- ARM64 based North Star 2 SVK board

code available at GITHUB:
https://github.com/Broadcom/cygnus-linux/tree/iproc-pcie-fix-v1

Ray Jui (8):
PCI: iproc: Fix code comment
PCI: iproc: Remove unused code
PCI: iproc: Remove ARCH specific flag
PCI: iproc: Fix PCIe reset logic
PCI: iproc: Improve link detection logic
PCI: iproc: Update iProc PCIe device tree bindings
PCI: iproc: Add outbound mapping support
PCI: iproc: Fix compile warnings

 .../devicetree/bindings/pci/brcm,iproc-pcie.txt    |  20 +++
 drivers/pci/host/pcie-iproc-platform.c             |  27 ++++
 drivers/pci/host/pcie-iproc.c                      | 159 +++++++++++++++++++--
 drivers/pci/host/pcie-iproc.h                      |  20 ++-
 4 files changed, 210 insertions(+), 16 deletions(-)

Signature · 1 line

1.9.1

Ray Jui

author maintainer

2015-09-16 00:39 UTC #1 lore raw

PATCH 1/8 PCI: iproc: Fix code comment

Fix code comment in pcie-iproc.h so it matches the code

Signed-off-by Ray Jui <[email protected]>

---
 drivers/pci/host/pcie-iproc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/pci/host/pcie-iproc.h +1 -1

index c9e4c10..4880b09 100644
@@ -20,11 +20,11 @@
  * iProc PCIe device
  * @dev: pointer to device data structure
  * @base: PCIe host controller I/O register base
- * @resources: linked list of all PCI resources
  * @sysdata: Per PCI controller data (ARM-specific)
  * @root_bus: pointer to root bus
  * @phy: optional PHY device that controls the Serdes
  * @irqs: interrupt IDs
+ * @map_irq: function callback to map interrupts
  */
 struct iproc_pcie {
 	struct device *dev;

Signature · 1 line

1.9.1

Ray Jui

author maintainer

2015-09-16 00:39 UTC #2 lore raw

PATCH 2/8 PCI: iproc: Remove unused code

In reply to Ray Jui

Remove unused struct iproc_pcie member irqs and #define IPROC_PCIE_MAX_NUM_IRQS

Signed-off-by Ray Jui <[email protected]>

---
 drivers/pci/host/pcie-iproc.h | 3 ---
 1 file changed, 3 deletions(-)

drivers/pci/host/pcie-iproc.h +0 -3

index 4880b09..ecaad57 100644
@@ -14,8 +14,6 @@
 #ifndef _PCIE_IPROC_H
 #define _PCIE_IPROC_H
 
-#define IPROC_PCIE_MAX_NUM_IRQS 6
-
 /**
  * iProc PCIe device
  * @dev: pointer to device data structure

@@ -34,7 +32,6 @@ struct iproc_pcie {
 #endif
 	struct pci_bus *root_bus;
 	struct phy *phy;
-	int irqs[IPROC_PCIE_MAX_NUM_IRQS];
 	int (*map_irq)(const struct pci_dev *, u8, u8);
 };

Signature · 1 line

1.9.1

Ray Jui

author maintainer

2015-09-16 00:39 UTC #3 lore raw

PATCH 3/8 PCI: iproc: Remove ARCH specific flag

In reply to Ray Jui

Now setup-irq.o is built for arm64, in 'commit 459a07721c11 ("PCI: Build setup-irq.o for arm64")' by Jayachandran, pci_fixup_irqs can be referenced. We no longer need the CONFIG_ARM flag to wrap around call to pci_fixup_irqs

Signed-off-by Ray Jui <[email protected]>

---
 drivers/pci/host/pcie-iproc.c | 2 --
 1 file changed, 2 deletions(-)

drivers/pci/host/pcie-iproc.c +0 -2

index fe2efb1..52e7ff2 100644
@@ -238,9 +238,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
 
 	pci_scan_child_bus(bus);
 	pci_assign_unassigned_bus_resources(bus);
-#ifdef CONFIG_ARM
 	pci_fixup_irqs(pci_common_swizzle, pcie->map_irq);
-#endif
 	pci_bus_add_devices(bus);
 
 	return 0;

Signature · 1 line

1.9.1

Ray Jui

author maintainer

2015-09-16 00:39 UTC #4 lore raw

PATCH 4/8 PCI: iproc: Fix PCIe reset logic

In reply to Ray Jui

The current iProc PCIe reset logic does not always properly reset the device. For example, in the case when the perst_b signal is already de-asserted in the bootloader, the current reset logic fails to trigger a proper asssert -> de-assert reset sequence. This patch fixes the issue by always triggering the proper reset sequence

This patch also explicitly selects the desired reset source, i.e., perst_b and reduces the wait time after the device comes out of reset from 250 ms to 100 ms, based on recommendation from the ASIC team

Signed-off-by Ray Jui <[email protected]>

Reviewed-by Vladimir Dreizin <[email protected]>

Reviewed-by Trac Hoang <[email protected]>

Reviewed-by Scott Branden <[email protected]>

Tested-by Vladimir Dreizin <[email protected]>

Tested-by Darren Edamura <[email protected]>

---
 drivers/pci/host/pcie-iproc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

drivers/pci/host/pcie-iproc.c +10 -5

index 52e7ff2..80e0541 100644
@@ -31,6 +31,8 @@
 #include "pcie-iproc.h"
 
 #define CLK_CONTROL_OFFSET           0x000
+#define EP_PERST_SOURCE_SELECT_SHIFT 2
+#define EP_PERST_SOURCE_SELECT       BIT(EP_PERST_SOURCE_SELECT_SHIFT)
 #define EP_MODE_SURVIVE_PERST_SHIFT  1
 #define EP_MODE_SURVIVE_PERST        BIT(EP_MODE_SURVIVE_PERST_SHIFT)
 #define RC_PCIE_RST_OUTPUT_SHIFT     0

@@ -119,15 +121,18 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
 	u32 val;
 
 	/*
-	 * Configure the PCIe controller as root complex and send a downstream
-	 * reset
+	 * Select perst_b signal as reset source. Put the device into reset,
+	 * and then bring it out of reset
 	 */
-	val = EP_MODE_SURVIVE_PERST | RC_PCIE_RST_OUTPUT;
+	val = readl(pcie->base + CLK_CONTROL_OFFSET);
+	val &= ~EP_PERST_SOURCE_SELECT & ~EP_MODE_SURVIVE_PERST &
+		~RC_PCIE_RST_OUTPUT;
 	writel(val, pcie->base + CLK_CONTROL_OFFSET);
 	udelay(250);
-	val &= ~EP_MODE_SURVIVE_PERST;
+
+	val |= RC_PCIE_RST_OUTPUT;
 	writel(val, pcie->base + CLK_CONTROL_OFFSET);
-	msleep(250);
+	msleep(100);
 }
 
 static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)

Signature · 1 line

1.9.1

Ray Jui

author maintainer

2015-09-16 00:39 UTC #5 lore raw

PATCH 5/8 PCI: iproc: Improve link detection logic

In reply to Ray Jui

This patch further improves the iProc PCIe link detection logic by explicitly querying the link status register to ensure link is active

It also forces class to PCI_CLASS_BRIDGE_PCI (0x0604) through the host configuration space register

Signed-off-by Ray Jui <[email protected]>

Reviewed-by Anup Patel <[email protected]>

Reviewed-by Scott Branden <[email protected]>

---
 drivers/pci/host/pcie-iproc.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

drivers/pci/host/pcie-iproc.c +23 -6

index 80e0541..62e8085 100644
@@ -60,6 +60,12 @@
 #define SYS_RC_INTX_EN               0x330
 #define SYS_RC_INTX_MASK             0xf
 
+#define PCIE_LINK_STATUS_OFFSET      0xf0c
+#define PCIE_PHYLINKUP_SHIFT         3
+#define PCIE_PHYLINKUP               BIT(PCIE_PHYLINKUP_SHIFT)
+#define PCIE_DL_ACTIVE_SHIFT         2
+#define PCIE_DL_ACTIVE               BIT(PCIE_DL_ACTIVE_SHIFT)
+
 static inline struct iproc_pcie *iproc_data(struct pci_bus *bus)
 {
 	struct iproc_pcie *pcie;

@@ -138,9 +144,15 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
 static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 {
 	u8 hdr_type;
-	u32 link_ctrl;
+	u32 link_ctrl, class, val;
 	u16 pos, link_status;
-	int link_is_active = 0;
+	bool link_is_active = false;
+
+	val = readl(pcie->base + PCIE_LINK_STATUS_OFFSET);
+	if (!(val & PCIE_PHYLINKUP) || !(val & PCIE_DL_ACTIVE)) {
+		dev_err(pcie->dev, "PHY or data link is INACTIVE!\n");
+		return -ENODEV;
+	}
 
 	/* make sure we are not in EP mode */
 	pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);

@@ -150,14 +162,19 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 	}
 
 	/* force class to PCI_CLASS_BRIDGE_PCI (0x0604) */
-	pci_bus_write_config_word(bus, 0, PCI_CLASS_DEVICE,
-				  PCI_CLASS_BRIDGE_PCI);
+#define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
+#define PCI_CLASS_BRIDGE_MASK      0xffff00
+#define PCI_CLASS_BRIDGE_SHIFT     8
+	pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+	class &= ~PCI_CLASS_BRIDGE_MASK;
+	class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
+	pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
 
 	/* check link status to see if link is active */
 	pos = pci_bus_find_capability(bus, 0, PCI_CAP_ID_EXP);
 	pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
 	if (link_status & PCI_EXP_LNKSTA_NLW)
-		link_is_active = 1;
+		link_is_active = true;
 
 	if (!link_is_active) {
 		/* try GEN 1 link speed */

@@ -181,7 +198,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 			pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
 						 &link_status);
 			if (link_status & PCI_EXP_LNKSTA_NLW)
-				link_is_active = 1;
+				link_is_active = true;
 		}
 	}

Signature · 1 line

1.9.1

Ray Jui

author maintainer

2015-09-16 00:39 UTC #6 lore raw

PATCH 6/8 PCI: iproc: Update iProc PCIe device tree bindings

In reply to Ray Jui

This patch updates the iProc PCIe device tree bindings with added support for outbound mapping configurations

Signed-off-by Ray Jui <[email protected]>

---
 .../devicetree/bindings/pci/brcm,iproc-pcie.txt      | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt +20 -0

index f7ce50e..45c2a80 100644
@@ -17,6 +17,21 @@ Optional properties:
 - phys: phandle of the PCIe PHY device
 - phy-names: must be "pcie-phy"
 
+- brcm,pcie-ob: Some iProc SoCs do not have the outbound address mapping done
+by the ASIC after power on reset. In this case, SW needs to configure it
+
+If the brcm,pcie-ob property is present, the following properties become
+effective:
+
+Required:
+- brcm,pcie-ob-axi-offset: The offset from the AXI address to the internal
+address used by the iProc PCIe core (not the PCIe address)
+- brcm,pcie-ob-window-size: The outbound address mapping window size (in MB)
+
+Optional:
+- brcm,pcie-ob-oarr-size: Some iProc SoCs need the OARR size bit to be set to
+increase the outbound window size
+
 Example:
 	pcie0: pcie at 18012000 {
 		compatible = "brcm,iproc-pcie";

@@ -38,6 +53,11 @@ Example:
 
 		phys = <&phy 0 5>;
 		phy-names = "pcie-phy";
+
+		brcm,pcie-ob;
+		brcm,pcie-ob-oarr-size;
+		brcm,pcie-ob-axi-offset = <0x00000000>;
+		brcm,pcie-ob-window-size = <256>;
 	};
 
 	pcie1: pcie at 18013000 {

Signature · 1 line

1.9.1

Ray Jui

author maintainer

2015-09-16 00:39 UTC #7 lore raw

PATCH 7/8 PCI: iproc: Add outbound mapping support

In reply to Ray Jui

Certain iProc SoCs require the PCIe outbound mapping to be configured in SW. This patch adds support for those chips

Signed-off-by Ray Jui <[email protected]>

---
 drivers/pci/host/pcie-iproc-platform.c |  27 ++++++++
 drivers/pci/host/pcie-iproc.c          | 113 +++++++++++++++++++++++++++++++++
 drivers/pci/host/pcie-iproc.h          |  17 +++++
 3 files changed, 157 insertions(+)

drivers/pci/host/pcie-iproc-platform.c +27 -0

index 9aedc8e..c9550dc 100644
@@ -54,6 +54,33 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	if (of_property_read_bool(np, "brcm,pcie-ob")) {
+		u32 val;
+
+		ret = of_property_read_u32(np, "brcm,pcie-ob-axi-offset",
+					   &val);
+		if (ret) {
+			dev_err(pcie->dev,
+				"missing brcm,pcie-ob-axi-offset property\n");
+			return ret;
+		}
+		pcie->ob.axi_offset = val;
+
+		ret = of_property_read_u32(np, "brcm,pcie-ob-window-size",
+					   &val);
+		if (ret) {
+			dev_err(pcie->dev,
+				"missing brcm,pcie-ob-window-size property\n");
+			return ret;
+		}
+		pcie->ob.window_size = (resource_size_t)val * SZ_1M;
+
+		if (of_property_read_bool(np, "brcm,pcie-ob-oarr-size"))
+			pcie->ob.set_oarr_size = true;
+
+		pcie->need_ob_cfg = true;
+	}
+
 	/* PHY use is optional */
 	pcie->phy = devm_phy_get(&pdev->dev, "pcie-phy");
 	if (IS_ERR(pcie->phy)) {

drivers/pci/host/pcie-iproc.c +113 -0

131 lines not on this page · raw message

drivers/pci/host/pcie-iproc.h +17 -0

index ecaad57..d3dc940 100644
@@ -15,6 +15,19 @@
 #define _PCIE_IPROC_H
 
 /**
+ * iProc PCIe outbound mapping
+ * @set_oarr_size: indicates the OARR size bit needs to be set
+ * @axi_offset: offset from the AXI address to the internal address used by
+ * the iProc PCIe core
+ * @window_size: outbound window size
+ */
+struct iproc_pcie_ob {
+	bool set_oarr_size;
+	resource_size_t axi_offset;
+	resource_size_t window_size;
+};
+
+/**
  * iProc PCIe device
  * @dev: pointer to device data structure
  * @base: PCIe host controller I/O register base

@@ -23,6 +36,8 @@
  * @phy: optional PHY device that controls the Serdes
  * @irqs: interrupt IDs
  * @map_irq: function callback to map interrupts
+ * @need_ob_cfg: indidates SW needs to configure the outbound mapping window
+ * @ob: outbound mapping parameters
  */
 struct iproc_pcie {
 	struct device *dev;

@@ -33,6 +48,8 @@ struct iproc_pcie {
 	struct pci_bus *root_bus;
 	struct phy *phy;
 	int (*map_irq)(const struct pci_dev *, u8, u8);
+	bool need_ob_cfg;
+	struct iproc_pcie_ob ob;
 };
 
 int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res);

Signature · 1 line

1.9.1

Kevin Hilman

2015-10-13 17:58 UTC #8 lore raw

Re: [PATCH 7/8] PCI: iproc: Add outbound mapping support

Quoted 4 lines · Ray Jui

Certain iProc SoCs require the PCIe outbound mapping to be configured in SW. This patch adds support for those chips
Signed-off-by Ray Jui <[email protected]>

kernelci.org bot found a new build failure in linux-next[1] and it was bisected down to this patch, which landed in linux-next in the form of 90e4032eeaf2 PCI: iproc: Add outbound mapping support.

The build fails on arm[1] with multi_v7_defconfig and CONFIG_ARM_LPAE=y with:

drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x6d874): undefined reference to `__aeabi_uldivmod'

Kevin

[1] http://storage.kernelci.org/next/next-20151013/arm-multi_v7_defconfig+CONFIG_ARM_LPAE=y/build.log

Arnd Bergmann

2015-10-13 20:02 UTC #9 lore raw

PATCH 1/1 PCI: iproc: fix 32-bit build error

The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:

drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'

This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.

Signed-off-by Arnd Bergmann <[email protected]>

Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")

drivers/pci/host/pcie-iproc.c +4 -2

index f3481ddff344..abcb4be2ee19 100644
@@ -235,12 +235,13 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
  *
  * axi_addr -> iproc_pcie_address -> OARR -> OMAP -> pci_address
  */
-static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
+int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 			       u64 pci_addr, resource_size_t size)
 {
 	struct iproc_pcie_ob *ob = &pcie->ob;
 	unsigned i;
 	u64 max_size = (u64)ob->window_size * MAX_NUM_OB_WINDOWS;
+	u64 remainder;
 
 	if (size > max_size) {
 		dev_err(pcie->dev,

@@ -249,7 +250,8 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 		return -EINVAL;
 	}
 
-	if (size % ob->window_size) {
+	div64_u64_rem(size, ob->window_size, &remainder);
+	if (remainder) {
 		dev_err(pcie->dev,
 			"res size %pap needs to be multiple of window size %pap\n",
 			&size, &ob->window_size);

Hauke Mehrtens

2015-10-13 20:06 UTC #10 lore raw

Re: [PATCH] PCI: iproc: fix 32-bit build error

Quoted 20 lines · Arnd Bergmann

The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:
drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'
This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.
Signed-off-by Arnd Bergmann <[email protected]>
Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")
drivers/pci/host/pcie-iproc.c +0 -0
index f3481ddff344..abcb4be2ee19 100644

drivers/pci/host/pcie-iproc.c static void iproc_pcie_enable(struct iproc_pcie *pcie) · line 238

Quoted 5 lines

drivers/pci/host/pcie-iproc.c +1 -1

@@ -235,12 +235,13 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
  *
  * axi_addr -> iproc_pcie_address -> OARR -> OMAP -> pci_address
  */
-static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
+int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,

Why are you removing the static here?

Quoted 20 lines

drivers/pci/host/pcie-iproc.c +3 -1

 			       u64 pci_addr, resource_size_t size)
 {
 	struct iproc_pcie_ob *ob = &pcie->ob;
 	unsigned i;
 	u64 max_size = (u64)ob->window_size * MAX_NUM_OB_WINDOWS;
+	u64 remainder;
 
 	if (size > max_size) {
 		dev_err(pcie->dev,

@@ -249,7 +250,8 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 		return -EINVAL;
 	}
 
-	if (size % ob->window_size) {
+	div64_u64_rem(size, ob->window_size, &remainder);
+	if (remainder) {
 		dev_err(pcie->dev,
 			"res size %pap needs to be multiple of window size %pap\n",
 			&size, &ob->window_size);

Ray Jui

author maintainer

2015-10-13 20:11 UTC #11 lore raw

Re: [PATCH] PCI: iproc: fix 32-bit build error

Quoted 5 lines · Hauke Mehrtens

Quoted 4 lines · Arnd Bergmann
The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:

Quoted 8 lines

Quoted 8 lines
drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'
This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.

Thanks, Arnd. Guess I did not see this issue when running on ARM32 based platforms like Cygnus, because LAPE was not enabled by default for multi_v7_defconfig. When LAPE is enabled, resource_size_t becomes 64-bit.

Quoted 16 lines

drivers/pci/host/pcie-iproc.c static void iproc_pcie_enable(struct iproc_pcie *pcie) · line 238
Quoted 13 lines
Signed-off-by Arnd Bergmann <[email protected]>
Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")
drivers/pci/host/pcie-iproc.c +1 -1
index f3481ddff344..abcb4be2ee19 100644
@@ -235,12 +235,13 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
  *
  * axi_addr -> iproc_pcie_address -> OARR -> OMAP -> pci_address
  */
-static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
+int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
Why are you removing the static here?

Yeah we should keep this function as static. The rest of this change looks good!

Quoted 13 lines

drivers/pci/host/pcie-iproc.c +1 -0

 			       u64 pci_addr, resource_size_t size)
 {
 	struct iproc_pcie_ob *ob = &pcie->ob;
 	unsigned i;
 	u64 max_size = (u64)ob->window_size * MAX_NUM_OB_WINDOWS;
+	u64 remainder;
 
 	if (size > max_size) {
 		dev_err(pcie->dev,

@@ -249,7 +250,8 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 		return -EINVAL;
 	}

Quoted 8 lines

drivers/pci/host/pcie-iproc.c static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr, · line 257
Quoted 7 lines
drivers/pci/host/pcie-iproc.c +2 -1
-	if (size % ob->window_size) {
+	div64_u64_rem(size, ob->window_size, &remainder);
+	if (remainder) {
 		dev_err(pcie->dev,
 			"res size %pap needs to be multiple of window size %pap\n",
 			&size, &ob->window_size);

Thanks,

Ray

Arnd Bergmann

2015-10-13 21:00 UTC #12 lore raw

Re: [PATCH] PCI: iproc: fix 32-bit build error

Quoted 14 lines · On Tuesday 13 October 2015 13:11:34 Ray Jui wrote:

Quoted 4 lines
Quoted 4 lines
drivers/pci/host/pcie-iproc.c +0 -0
index f3481ddff344..abcb4be2ee19 100644
Quoted 9 lines
drivers/pci/host/pcie-iproc.c static void iproc_pcie_enable(struct iproc_pcie *pcie) · line 238
Quoted 6 lines
drivers/pci/host/pcie-iproc.c +1 -1
@@ -235,12 +235,13 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
  *
  * axi_addr -> iproc_pcie_address -> OARR -> OMAP -> pci_address
  */
-static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
+int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
Why are you removing the static here?

Quoted 3 lines

Yeah we should keep this function as static. The rest of this change looks good!

Sorry, this accidentally ended up in the patch when I marked the function as global to track down how the division got called.

I'll send a new version.

Arnd

Bjorn Helgaas

2015-10-15 16:00 UTC #13 lore raw

Re: [PATCH] PCI: iproc: fix 32-bit build error

Quoted 15 lines · Arnd Bergmann

Quoted 14 lines · On Tuesday 13 October 2015 13:11:34 Ray Jui wrote:
Quoted 13 lines
drivers/pci/host/pcie-iproc.c static void iproc_pcie_enable(struct iproc_pcie *pcie) · line 238
Quoted 10 lines
drivers/pci/host/pcie-iproc.c +1 -1
index f3481ddff344..abcb4be2ee19 100644
@@ -235,12 +235,13 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
  *
  * axi_addr -> iproc_pcie_address -> OARR -> OMAP -> pci_address
  */
-static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
+int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
Why are you removing the static here?

Quoted 8 lines

Quoted 3 lines
Yeah we should keep this function as static. The rest of this change looks good!
Sorry, this accidentally ended up in the patch when I marked the function as global to track down how the division got called.
I'll send a new version.

Ping; I can easily fix up the static, but it'd be nice to have real acks from Ray and Hauke.

Bjorn

Ray Jui

author maintainer Acked-by

2015-10-15 16:19 UTC #14 lore raw

Re: [PATCH] PCI: iproc: fix 32-bit build error

Quoted 23 lines · Bjorn Helgaas

Quoted 22 lines · Arnd Bergmann
Quoted 14 lines · On Tuesday 13 October 2015 13:11:34 Ray Jui wrote:
Quoted 4 lines
Quoted 4 lines
drivers/pci/host/pcie-iproc.c +0 -0
index f3481ddff344..abcb4be2ee19 100644
Quoted 9 lines
drivers/pci/host/pcie-iproc.c static void iproc_pcie_enable(struct iproc_pcie *pcie) · line 238
Quoted 6 lines
drivers/pci/host/pcie-iproc.c +1 -1
@@ -235,12 +235,13 @@ static void iproc_pcie_enable(struct iproc_pcie *pcie)
  *
  * axi_addr -> iproc_pcie_address -> OARR -> OMAP -> pci_address
  */
-static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
+int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
Why are you removing the static here?
Quoted 3 lines
Yeah we should keep this function as static. The rest of this change looks good!
Sorry, this accidentally ended up in the patch when I marked the function as global to track down how the division got called.

Quoted 7 lines

Quoted 1 line
I'll send a new version.
Ping; I can easily fix up the static, but it'd be nice to have real acks from Ray and Hauke.
Bjorn

The current patch looks good to me except that iproc_pcie_setup_ob should be kept static.

With static added back:

Acked-by Ray Jui <[email protected]>

Thanks,

Ray

Arnd Bergmann

2015-10-15 19:40 UTC #15 lore raw

PATCH v2 1/1 PCI: iproc: fix 32-bit build error

The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:

drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'

This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.

Signed-off-by Arnd Bergmann <[email protected]>

Acked-by Ray Jui <[email protected]>

Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")

---

Quoted 4 lines · On Thursday 15 October 2015 09:19:27 Ray Jui wrote:

Quoted 3 lines · Bjorn Helgaas
Ping; I can easily fix up the static, but it'd be nice to have real acks from Ray and Hauke.

Argh, I knew there was one patch from my build fixes that I needed to redo, I just couldn't find it again. Thanks for the reminder.

drivers/pci/host/pcie-iproc.c +3 -1

index f3481ddff344..9193951ae861 100644
@@ -241,6 +241,7 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 	struct iproc_pcie_ob *ob = &pcie->ob;
 	unsigned i;
 	u64 max_size = (u64)ob->window_size * MAX_NUM_OB_WINDOWS;
+	u64 remainder;
 
 	if (size > max_size) {
 		dev_err(pcie->dev,

@@ -249,7 +250,8 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 		return -EINVAL;
 	}
 
-	if (size % ob->window_size) {
+	div64_u64_rem(size, ob->window_size, &remainder);
+	if (remainder) {
 		dev_err(pcie->dev,
 			"res size %pap needs to be multiple of window size %pap\n",
 			&size, &ob->window_size);

Hauke Mehrtens

Acked-by on v2 1/1

2015-10-15 20:57 UTC #16 lore raw

Re: [PATCH v2] PCI: iproc: fix 32-bit build error

Quoted 7 lines · Arnd Bergmann

The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:
drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'

Quoted 7 lines

This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.
Signed-off-by Arnd Bergmann <[email protected]>
Acked-by Ray Jui <[email protected]>

Acked-by Hauke Mehrtens <[email protected]>

Quoted 37 lines

Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")
---
Quoted 4 lines · On Thursday 15 October 2015 09:19:27 Ray Jui wrote:
Quoted 3 lines · Bjorn Helgaas
Ping; I can easily fix up the static, but it'd be nice to have real acks from Ray and Hauke.
Argh, I knew there was one patch from my build fixes that I needed to redo, I just couldn't find it again. Thanks for the reminder.
drivers/pci/host/pcie-iproc.c +3 -1
index f3481ddff344..9193951ae861 100644
@@ -241,6 +241,7 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 	struct iproc_pcie_ob *ob = &pcie->ob;
 	unsigned i;
 	u64 max_size = (u64)ob->window_size * MAX_NUM_OB_WINDOWS;
+	u64 remainder;
 
 	if (size > max_size) {
 		dev_err(pcie->dev,
@@ -249,7 +250,8 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 		return -EINVAL;
 	}
 
-	if (size % ob->window_size) {
+	div64_u64_rem(size, ob->window_size, &remainder);
+	if (remainder) {
 		dev_err(pcie->dev,
 			"res size %pap needs to be multiple of window size %pap\n",
 			&size, &ob->window_size);

Bjorn Helgaas

2015-10-15 21:01 UTC #17 lore raw

Re: [PATCH v2] PCI: iproc: fix 32-bit build error

In reply to Arnd Bergmann

Quoted 7 lines · Arnd Bergmann

The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:
drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'

Quoted 8 lines

This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.
Signed-off-by Arnd Bergmann <[email protected]>
Acked-by Ray Jui <[email protected]>
Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")

This *looks* like it should be fixing 90e4032eeaf2 ("PCI: iproc: Add outbound mapping support"), not 0ab99ca8eb6c.

90e4032eeaf2 added the "if (size % ob->window_size)".

Is this more subtle than that?

Quoted 39 lines

---
Quoted 4 lines · On Thursday 15 October 2015 09:19:27 Ray Jui wrote:
Quoted 3 lines · Bjorn Helgaas
Ping; I can easily fix up the static, but it'd be nice to have real acks from Ray and Hauke.
Argh, I knew there was one patch from my build fixes that I needed to redo, I just couldn't find it again. Thanks for the reminder.
drivers/pci/host/pcie-iproc.c +3 -2
index f3481ddff344..9193951ae861 100644
@@ -241,6 +241,7 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 	struct iproc_pcie_ob *ob = &pcie->ob;
 	unsigned i;
 	u64 max_size = (u64)ob->window_size * MAX_NUM_OB_WINDOWS;
+	u64 remainder;
 
 	if (size > max_size) {
 		dev_err(pcie->dev,
@@ -249,7 +250,8 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 		return -EINVAL;
 	}
 
-	if (size % ob->window_size) {
+	div64_u64_rem(size, ob->window_size, &remainder);
+	if (remainder) {
 		dev_err(pcie->dev,
 			"res size %pap needs to be multiple of window size %pap\n",
 			&size, &ob->window_size);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Arnd Bergmann

2015-10-16 09:47 UTC #18 lore raw

Re: [PATCH v2] PCI: iproc: fix 32-bit build error

Quoted 17 lines · On Thursday 15 October 2015 16:01:13 Bjorn Helgaas wrote:

Quoted 7 lines · Arnd Bergmann
The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:
drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'
Quoted 8 lines
This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.
Signed-off-by Arnd Bergmann <[email protected]>
Acked-by Ray Jui <[email protected]>
Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")

Quoted 8 lines

This *looks* like it should be fixing 90e4032eeaf2 ("PCI: iproc: Add outbound mapping support"), not 0ab99ca8eb6c.
90e4032eeaf2 added the "if (size % ob->window_size)".
Is this more subtle than that?

You are absolutely right, I must have copy-pasted the wrong entry. Do you want me to resend this? I guess you can just remove the line as this commit has not appeared in any release anyway.

Arnd

Bjorn Helgaas

2015-10-16 13:23 UTC #19 lore raw

Re: [PATCH v2] PCI: iproc: fix 32-bit build error

Quoted 21 lines · Arnd Bergmann

Quoted 20 lines · On Thursday 15 October 2015 16:01:13 Bjorn Helgaas wrote:
Quoted 7 lines · Arnd Bergmann
The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:
drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'
Quoted 8 lines
This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.
Signed-off-by Arnd Bergmann <[email protected]>
Acked-by Ray Jui <[email protected]>
Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")
This *looks* like it should be fixing 90e4032eeaf2 ("PCI: iproc: Add outbound mapping support"), not 0ab99ca8eb6c.

Quoted 7 lines

Quoted 3 lines
90e4032eeaf2 added the "if (size % ob->window_size)".
Is this more subtle than that?
You are absolutely right, I must have copy-pasted the wrong entry. Do you want me to resend this? I guess you can just remove the line as this commit has not appeared in any release anyway.

I squashed this fix and Jon's %pap fix into Ray's original "Add outbound mapping support" patch and re-did the merge. Thanks!

Bjorn

Kevin Hilman

Tested-by on v1 1/1

2015-10-13 21:21 UTC #20 lore raw

Re: [PATCH] PCI: iproc: fix 32-bit build error

In reply to Arnd Bergmann

Quoted 7 lines · Arnd Bergmann

The iproc PCI driver tries to figure out whether the MMIO window has a valid size, but does this using a 64-bit modulo operation, which is not allowed on 32-bit kernels and leads to a link error:
drivers/built-in.o: In function `iproc_pcie_setup':
:(.text+0x4b798): undefined reference to `__aeabi_uldivmod'

Quoted 7 lines

This works around that error by using the div64_u64() function provided by the kernel. While this is an expensive operation, it is harmless because we only call it during the probing of the driver.
Signed-off-by Arnd Bergmann <[email protected]>
Fixes 0ab99ca8eb6c ("PCI: iproc: Fix compile warnings")

Tested-by Kevin Hilman <[email protected]>

Verified that this fixes the build failure on arm multi_v7_defconfig + CONFIG_ARM_LPAE=y on top of next-20151013.

Kevin

Ray Jui

author maintainer

2015-09-16 00:39 UTC #21 lore raw

PATCH 8/8 PCI: iproc: Fix compile warnings

In reply to Ray Jui

There are several compile warnings in pcie-iproc.c related to the printing of a size_t value. This is a 32bit value on arm, and 64bit on arm64. However, the printks are for 64bit values (thus the warning). Using the %pap printk for these values (per Documentation/printk-formats.txt) corrects the issue.

Signed-off-by Jon Mason <[email protected]>

---
 drivers/pci/host/pcie-iproc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/pci/host/pcie-iproc.c +4 -4

index 2ba3c4b..f3481dd 100644
@@ -244,15 +244,15 @@ static int iproc_pcie_setup_ob(struct iproc_pcie *pcie, u64 axi_addr,
 
 	if (size > max_size) {
 		dev_err(pcie->dev,
-			"res size 0x%llx exceeds max supported size 0x%llx\n",
-			(u64)size, max_size);
+			"res size 0x%pap exceeds max supported size 0x%llx\n",
+			&size, max_size);
 		return -EINVAL;
 	}
 
 	if (size % ob->window_size) {
 		dev_err(pcie->dev,
-			"res size 0x%llx needs to be multiple of "
-			"window size 0x%llx\n", (u64)size, ob->window_size);
+			"res size %pap needs to be multiple of window size %pap\n",
+			&size, &ob->window_size);
 		return -EINVAL;
 	}

Signature · 1 line

1.9.1

Bjorn Helgaas

Applied

2015-09-25 23:21 UTC #22 lore raw

In reply to Ray Jui

Quoted 33 lines · Ray Jui

This patch series contains various fixes and outbound mapping support for the Broadcom iProc PCIe driver. Some of the critical fixes include 1) fix of PCIe core reset logic and therefore remove its dependency on the bootloader;
2) improved link detection logic that works for more iProc based SoCs.
This patch series also adds the outbound address mapping support. While outbound address mapping support is not required on chips like North Star, Cygnus, and etc., some of the newer iProc based chips like North Star 2 require this support to properly map the AXI address into the address used in the iProc PCIe core
This patch series is constructed based on Linux v4.3-rc1 (with workaround to disable calls to pci_read_bridge_bases in pci/probe.c that breaks some of the ARM based PCIe devices including iProc)
This patch series is tested on the following platforms with an Intel e1000e based PCIe x1 NIC card:
- ARM32 based Cygnus BCM958305K Wireless Audio board
- ARM64 based North Star 2 SVK board
code available at GITHUB:
https://github.com/Broadcom/cygnus-linux/tree/iproc-pcie-fix-v1
Ray Jui (8):
PCI: iproc: Fix code comment
PCI: iproc: Remove unused code
PCI: iproc: Remove ARCH specific flag
PCI: iproc: Fix PCIe reset logic
PCI: iproc: Improve link detection logic
PCI: iproc: Update iProc PCIe device tree bindings
PCI: iproc: Add outbound mapping support
PCI: iproc: Fix compile warnings

Quoted 5 lines

 .../devicetree/bindings/pci/brcm,iproc-pcie.txt    |  20 +++
 drivers/pci/host/pcie-iproc-platform.c             |  27 ++++
 drivers/pci/host/pcie-iproc.c                      | 159 +++++++++++++++++++--
 drivers/pci/host/pcie-iproc.h                      |  20 ++-
 4 files changed, 210 insertions(+), 16 deletions(-)

Applied to pci/host-iproc for v4.4, thanks!