[PATCH 00/13] PCI: brcmstb: Stop using chip numbers for conditions

[PATCH 00/13] PCI: brcmstb: Stop using chip numbers for conditions

[Jim Quinlan]

Much of the brcmstb PCIe driver code does (essentially) this:

        if (pcie->chip == 0x1234 || pcie->chip == 0x5678) {
                /* ... */
        }

when it should instead do this:

        if (behavior_xyz(pcie)) {
                /* ... */
        }

This series does the above.  In addition, some other minor improvements are
added in the hopes of making the code more readable.

Jim Quinlan (13):
  PCI: brcmstb: Remove redundant const specifier for struct fields
  PCI: brcmstb: Use flags u32 instead of bools
  PCI: brcmstb: Add Broadcom quirks macro
  PCI: brcmstb: Declare and assign quirk OB_WIN_32BIT_ADDR
  PCI: brcmstb: Declare and assign quirk OB_WIN_MAXSZ_128MB
  PCI: brcmstb: Declare and assign flag IS_BMIPS
  PCI: brcmstb: Declare and assign quirk 32BIT_PCI_OPS
  PCI: brcmstb: Declare and assign quirk NO_RGR1_TIMER
  PCI: brcmstb: Declare and assign quirk EARLY_PERST_ASSERT
  PCI: brcmstb: Declare and assign quirk PERST_PCIE_REV_CUTOFF
  PCI: brcmstb: Put max_burst_size setting in cfg_data
  PCI: brcmstb: Use order-0 indexing for inbound BAR window array
  PCI: brcmstb: Split up complicated function into two variants

 drivers/pci/controller/pcie-brcmstb.c | 267 +++++++++++++++-----------
 1 file changed, 158 insertions(+), 109 deletions(-)


base-commit: bc35965f6940a9bf834d54187b6088b8eb09206d
-- 
2.34.1

[PATCH 01/13] PCI: brcmstb: Remove redundant const specifier for struct fields

[Jim Quinlan]

All of the variables using this particular structure are declared as const.
There is no reason to declare its individual fields as being const.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 8a0c353d2abf..89aaa19d508a 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -288,9 +288,9 @@ struct inbound_win {
 
 struct pcie_cfg_data {
 	const int *offsets;
-	const enum pcie_soc_base soc_base;
-	const bool has_phy;
-	const u32 quirks;
+	enum pcie_soc_base soc_base;
+	bool has_phy;
+	u32 quirks;
 	u8 num_inbound_wins;
 	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
 	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
-- 
2.34.1

[PATCH 02/13] PCI: brcmstb: Use flags u32 instead of bools

[Jim Quinlan]

Instead of having separate 'bool' fields for each SoC configuration, use a
small bitvector (u32).  Doing this uses less memory and provides a uniform
method to assign flags for a specific SoC.  The expectation is that more
flags will be added as we replace conditions like "if (chipA || chipB)"
with "if (behavior_X)".

A helper macro "BFLAG()" is used to shorten the determination of the flag
value.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 89aaa19d508a..47862569937e 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -286,16 +286,24 @@ struct inbound_win {
  */
 #define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN		BIT(0)
 
+/* FLAGS */
+#define BFLAG(pcie, flag)			((pcie)->cfg->flags & CFG_FLG_ ## flag)
+
+/* The PCIe HW registers have phy control of a "RESCAL" reset block */
+#define CFG_FLG_HAS_PHY				BIT(0)
+/* The SoC PCIe HW can dump to the console PCIe error info */
+#define CFG_FLG_HAS_ERR_REPORT			BIT(1)
+
+
 struct pcie_cfg_data {
 	const int *offsets;
 	enum pcie_soc_base soc_base;
-	bool has_phy;
+	u32 flags;
 	u32 quirks;
 	u8 num_inbound_wins;
 	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
 	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
 	int (*post_setup)(struct brcm_pcie *pcie);
-	bool has_err_report;
 };
 
 struct subdev_regulators {
@@ -356,14 +364,14 @@ static int brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 val)
 	unsigned long flags;
 	int ret;
 
-	if (pcie->cfg->has_err_report)
+	if (BFLAG(pcie, HAS_ERR_REPORT))
 		spin_lock_irqsave(&pcie->bridge_lock, flags);
 
 	ret = pcie->cfg->bridge_sw_init_set(pcie, val);
 	/* If we fail, assume the bridge is in reset (off) */
 	pcie->bridge_in_reset = ret ? true : val;
 
-	if (pcie->cfg->has_err_report)
+	if (BFLAG(pcie, HAS_ERR_REPORT))
 		spin_unlock_irqrestore(&pcie->bridge_lock, flags);
 
 	return ret;
@@ -1589,12 +1597,12 @@ static int brcm_phy_cntl(struct brcm_pcie *pcie, const int start)
 
 static inline int brcm_phy_start(struct brcm_pcie *pcie)
 {
-	return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
+	return BFLAG(pcie, HAS_PHY) ? brcm_phy_cntl(pcie, 1) : 0;
 }
 
 static inline int brcm_phy_stop(struct brcm_pcie *pcie)
 {
-	return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
+	return BFLAG(pcie, HAS_PHY) ? brcm_phy_cntl(pcie, 0) : 0;
 }
 
 static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
@@ -1898,7 +1906,7 @@ static void brcm_pcie_remove(struct platform_device *pdev)
 	pci_stop_root_bus(bridge->bus);
 	pci_remove_root_bus(bridge->bus);
 	pci_unlock_rescan_remove();
-	if (pcie->cfg->has_err_report)
+	if (BFLAG(pcie, HAS_ERR_REPORT))
 		brcm_unregister_die_notifiers(pcie);
 
 	__brcm_pcie_remove(pcie);
@@ -1999,9 +2007,8 @@ static const struct pcie_cfg_data bcm7216_cfg = {
 	.soc_base	= BCM7278,
 	.perst_set	= brcm_pcie_perst_set_7278,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_7278,
-	.has_phy	= true,
+	.flags		= CFG_FLG_HAS_PHY | CFG_FLG_HAS_ERR_REPORT,
 	.num_inbound_wins = 3,
-	.has_err_report = true,
 };
 
 static const struct pcie_cfg_data bcm7712_cfg = {
@@ -2179,7 +2186,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (pcie->cfg->has_err_report) {
+	if (BFLAG(pcie, HAS_ERR_REPORT)) {
 		spin_lock_init(&pcie->bridge_lock);
 		brcm_register_die_notifiers(pcie);
 	}
-- 
2.34.1

[PATCH 03/13] PCI: brcmstb: Add Broadcom quirks macro

[Jim Quinlan]

Introduce a 'BQUIRK()' macro to shorten the calculation of a specific
quirk's value.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 47862569937e..bf002bcf6bd5 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -277,6 +277,9 @@ struct inbound_win {
 	u64 cpu_addr;
 };
 
+/* QUIRKS */
+#define BQUIRK(pcie, quirk)			((pcie)->cfg->quirks & CFG_QUIRK_ ## quirk)
+
 /*
  * The RESCAL block is tied to PCIe controller #1, regardless of the number of
  * controllers, and turning off PCIe controller #1 prevents access to the RESCAL
@@ -1627,7 +1630,7 @@ static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK);
 	writel(tmp, base + HARD_DEBUG(pcie));
 
-	if (!(pcie->cfg->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
+	if (!(BQUIRK(pcie, AVOID_BRIDGE_SHUTDOWN)))
 		/* Shutdown PCIe bridge */
 		ret = brcm_pcie_bridge_sw_init_set(pcie, 1);
 
-- 
2.34.1

[PATCH 04/13] PCI: brcmstb: Declare and assign quirk OB_WIN_32BIT_ADDR

[Jim Quinlan]

Even though only BMIPs chips have this quirk, it is more uniform and
readable to define this specific quirk in which the outbound region must
reside in the lower 4GB region.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index bf002bcf6bd5..71ddd8a760af 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -288,6 +288,8 @@ struct inbound_win {
  * or a hang (AXI).
  */
 #define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN		BIT(0)
+/* The outbound windows must be within the 0-4GB region */
+#define CFG_QUIRK_OB_WIN_32BIT_ADDR		BIT(1)
 
 /* FLAGS */
 #define BFLAG(pcie, flag)			((pcie)->cfg->flags & CFG_FLG_ ## flag)
@@ -519,7 +521,7 @@ static void brcm_pcie_set_outbound_win(struct brcm_pcie *pcie,
 			  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK);
 	writel(tmp, pcie->base + PCIE_MEM_WIN0_BASE_LIMIT(win));
 
-	if (is_bmips(pcie))
+	if (BQUIRK(pcie, OB_WIN_32BIT_ADDR))
 		return;
 
 	/* Write the cpu & limit addr upper bits */
@@ -1995,6 +1997,7 @@ static const struct pcie_cfg_data bcm7425_cfg = {
 	.perst_set	= brcm_pcie_perst_set_generic,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
+	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR,
 };
 
 static const struct pcie_cfg_data bcm7435_cfg = {
@@ -2003,6 +2006,7 @@ static const struct pcie_cfg_data bcm7435_cfg = {
 	.perst_set	= brcm_pcie_perst_set_generic,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
+	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR,
 };
 
 static const struct pcie_cfg_data bcm7216_cfg = {
-- 
2.34.1

[PATCH 05/13] PCI: brcmstb: Declare and assign quirk OB_WIN_MAXSZ_128MB

[Jim Quinlan]

Even though only BMIPs chips have this quirk, it is more uniform and
readable to define this specific quirk where each outbound window is must
be <= SZ_128M.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 71ddd8a760af..401898d04d52 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -290,6 +290,8 @@ struct inbound_win {
 #define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN		BIT(0)
 /* The outbound windows must be within the 0-4GB region */
 #define CFG_QUIRK_OB_WIN_32BIT_ADDR		BIT(1)
+/* Each of the outbound windows size must be <= SZ_128M */
+#define CFG_QUIRK_OB_WIN_MAXSZ_128MB		BIT(2)
 
 /* FLAGS */
 #define BFLAG(pcie, flag)			((pcie)->cfg->flags & CFG_FLG_ ## flag)
@@ -1294,11 +1296,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 			return -EINVAL;
 		}
 
-		if (is_bmips(pcie)) {
+		if (BQUIRK(pcie, OB_WIN_MAXSZ_128MB)) {
 			u64 start = res->start;
 			unsigned int j, nwins = resource_size(res) / SZ_128M;
 
-			/* bmips PCIe outbound windows have a 128MB max size */
+			/* PCIe outbound windows have a 128MB max size */
 			if (nwins > BRCM_NUM_PCIE_OUT_WINS)
 				nwins = BRCM_NUM_PCIE_OUT_WINS;
 			for (j = 0; j < nwins; j++, start += SZ_128M)
@@ -1997,7 +1999,8 @@ static const struct pcie_cfg_data bcm7425_cfg = {
 	.perst_set	= brcm_pcie_perst_set_generic,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
-	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR,
+	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR
+		| CFG_QUIRK_OB_WIN_MAXSZ_128MB,
 };
 
 static const struct pcie_cfg_data bcm7435_cfg = {
@@ -2006,7 +2009,8 @@ static const struct pcie_cfg_data bcm7435_cfg = {
 	.perst_set	= brcm_pcie_perst_set_generic,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
-	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR,
+	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR
+		| CFG_QUIRK_OB_WIN_MAXSZ_128MB,
 };
 
 static const struct pcie_cfg_data bcm7216_cfg = {
-- 
2.34.1

[PATCH 06/13] PCI: brcmstb: Declare and assign flag IS_BMIPS

[Jim Quinlan]

Eliminate a function that evaluates SoC chip IDs and instead declare and
use a flag that indicates the SoC employs a CPU with a variant of the
MIPS-1 ISA.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 401898d04d52..8927d1d97f4a 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -300,7 +300,8 @@ struct inbound_win {
 #define CFG_FLG_HAS_PHY				BIT(0)
 /* The SoC PCIe HW can dump to the console PCIe error info */
 #define CFG_FLG_HAS_ERR_REPORT			BIT(1)
-
+/* SoC uses a Broadcom variant of the MIPS-1 ISA */
+#define CFG_FLG_IS_BMIPS			BIT(2)
 
 struct pcie_cfg_data {
 	const int *offsets;
@@ -361,11 +362,6 @@ struct brcm_pcie {
 	spinlock_t		bridge_lock;
 };
 
-static inline bool is_bmips(const struct brcm_pcie *pcie)
-{
-	return pcie->cfg->soc_base == BCM7435 || pcie->cfg->soc_base == BCM7425;
-}
-
 static int brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 val)
 {
 	unsigned long flags;
@@ -1176,7 +1172,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 		return ret;
 
 	tmp = readl(base + HARD_DEBUG(pcie));
-	if (is_bmips(pcie))
+	if (BFLAG(pcie, IS_BMIPS))
 		tmp &= ~PCIE_BMIPS_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK;
 	else
 		tmp &= ~PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK;
@@ -1189,7 +1185,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	 * is encoded as 0=128, 1=256, 2=512, 3=Rsvd, for BCM7278 it
 	 * is encoded as 0=Rsvd, 1=128, 2=256, 3=512.
 	 */
-	if (is_bmips(pcie))
+	if (BFLAG(pcie, IS_BMIPS))
 		burst = 0x1; /* 256 bytes */
 	else if (pcie->cfg->soc_base == BCM2711)
 		burst = 0x0; /* 128 bytes */
@@ -2001,6 +1997,7 @@ static const struct pcie_cfg_data bcm7425_cfg = {
 	.num_inbound_wins = 3,
 	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR
 		| CFG_QUIRK_OB_WIN_MAXSZ_128MB,
+	.flags		= CFG_FLG_IS_BMIPS,
 };
 
 static const struct pcie_cfg_data bcm7435_cfg = {
@@ -2011,6 +2008,7 @@ static const struct pcie_cfg_data bcm7435_cfg = {
 	.num_inbound_wins = 3,
 	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR
 		| CFG_QUIRK_OB_WIN_MAXSZ_128MB,
+	.flags		= CFG_FLG_IS_BMIPS,
 };
 
 static const struct pcie_cfg_data bcm7216_cfg = {
-- 
2.34.1

[PATCH 07/13] PCI: brcmstb: Declare and assign quirk 32BIT_PCI_OPS

[Jim Quinlan]

Declare a new quirk that indicates SoCs where the config-space accesses are
restricted to 32bit R/W.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 8927d1d97f4a..be414d346855 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -292,6 +292,8 @@ struct inbound_win {
 #define CFG_QUIRK_OB_WIN_32BIT_ADDR		BIT(1)
 /* Each of the outbound windows size must be <= SZ_128M */
 #define CFG_QUIRK_OB_WIN_MAXSZ_128MB		BIT(2)
+/* PCIe HW can only access config space with 32bit R/W */
+#define CFG_QUIRK_32BIT_PCI_OPS			BIT(3)
 
 /* FLAGS */
 #define BFLAG(pcie, flag)			((pcie)->cfg->flags & CFG_FLG_ ## flag)
@@ -1996,7 +1998,7 @@ static const struct pcie_cfg_data bcm7425_cfg = {
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
 	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR
-		| CFG_QUIRK_OB_WIN_MAXSZ_128MB,
+		| CFG_QUIRK_OB_WIN_MAXSZ_128MB | CFG_QUIRK_32BIT_PCI_OPS,
 	.flags		= CFG_FLG_IS_BMIPS,
 };
 
@@ -2050,7 +2052,7 @@ static struct pci_ops brcm_pcie_ops = {
 	.remove_bus = brcm_pcie_remove_bus,
 };
 
-static struct pci_ops brcm7425_pcie_ops = {
+static struct pci_ops brcm_pcie_ops32 = {
 	.map_bus = brcm7425_pcie_map_bus,
 	.read = pci_generic_config_read32,
 	.write = pci_generic_config_write32,
@@ -2180,8 +2182,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		}
 	}
 
-	bridge->ops = pcie->cfg->soc_base == BCM7425 ?
-				&brcm7425_pcie_ops : &brcm_pcie_ops;
+	bridge->ops = BQUIRK(pcie, 32BIT_PCI_OPS)
+		? &brcm_pcie_ops32 : &brcm_pcie_ops;
 	bridge->sysdata = pcie;
 
 	platform_set_drvdata(pdev, pcie);
-- 
2.34.1

[PATCH 08/13] PCI: brcmstb: Declare and assign quirk NO_RGR1_TIMER

[Jim Quinlan]

Very few SoCs do not have an internal bus timer whose timeout value may be
adusted for L1SS operation.  Declare this as a quirk and assign it
accordingly.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index be414d346855..e2284e114241 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -294,6 +294,8 @@ struct inbound_win {
 #define CFG_QUIRK_OB_WIN_MAXSZ_128MB		BIT(2)
 /* PCIe HW can only access config space with 32bit R/W */
 #define CFG_QUIRK_32BIT_PCI_OPS			BIT(3)
+/* PCIe HW does not have an internal bus timer */
+#define CFG_QUIRK_NO_RGR1_TIMER			BIT(4)
 
 /* FLAGS */
 #define BFLAG(pcie, flag)			((pcie)->cfg->flags & CFG_FLG_ ## flag)
@@ -1339,8 +1341,8 @@ static void brcm_extend_rbus_timeout(struct brcm_pcie *pcie)
 	const unsigned int REG_OFFSET = PCIE_RGR1_SW_INIT_1(pcie) - 8;
 	u32 timeout_us = 4000000; /* 4 seconds, our setting for L1SS */
 
-	/* 7712 does not have this (RGR1) timer */
-	if (pcie->cfg->soc_base == BCM7712)
+	/* Don't do access if there is no timer */
+	if (BQUIRK(pcie, NO_RGR1_TIMER))
 		return;
 
 	/* Each unit in timeout register is 1/216,000,000 seconds */
@@ -1971,7 +1973,8 @@ static const struct pcie_cfg_data bcm2712_cfg = {
 	.perst_set	= brcm_pcie_perst_set_7278,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.post_setup	= brcm_pcie_post_setup_bcm2712,
-	.quirks		= CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN,
+	.quirks		= CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN |
+		CFG_QUIRK_NO_RGR1_TIMER,
 	.num_inbound_wins = 10,
 };
 
@@ -2028,6 +2031,7 @@ static const struct pcie_cfg_data bcm7712_cfg = {
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.soc_base	= BCM7712,
 	.num_inbound_wins = 10,
+	.quirks		= CFG_QUIRK_NO_RGR1_TIMER,
 };
 
 static const struct of_device_id brcm_pcie_match[] = {
-- 
2.34.1

[PATCH 09/13] PCI: brcmstb: Declare and assign quirk EARLY_PERST_ASSERT

[Jim Quinlan]

Declare and assign a quirk where PERST# must be asserted prior to an
internal bus being turned on.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index e2284e114241..dfd703571091 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -296,6 +296,8 @@ struct inbound_win {
 #define CFG_QUIRK_32BIT_PCI_OPS			BIT(3)
 /* PCIe HW does not have an internal bus timer */
 #define CFG_QUIRK_NO_RGR1_TIMER			BIT(4)
+/* PCIe PERST# must be asserted before internal bridge turned on */
+#define CFG_QUIRK_EARLY_PERST_ASSERT		BIT(5)
 
 /* FLAGS */
 #define BFLAG(pcie, flag)			((pcie)->cfg->flags & CFG_FLG_ ## flag)
@@ -1159,8 +1161,8 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	if (ret)
 		return ret;
 
-	/* Ensure that PERST# is asserted; some bootloaders may deassert it. */
-	if (pcie->cfg->soc_base == BCM2711) {
+	if (BQUIRK(pcie, EARLY_PERST_ASSERT)) {
+		/* Ensure that PERST# is asserted; some bootloaders may deassert it. */
 		ret = pcie->cfg->perst_set(pcie, 1);
 		if (ret) {
 			pcie->cfg->bridge_sw_init_set(pcie, 0);
@@ -1965,6 +1967,7 @@ static const struct pcie_cfg_data bcm2711_cfg = {
 	.perst_set	= brcm_pcie_perst_set_generic,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
+	.quirks		= CFG_QUIRK_EARLY_PERST_ASSERT,
 };
 
 static const struct pcie_cfg_data bcm2712_cfg = {
-- 
2.34.1

[PATCH 10/13] PCI: brcmstb: Declare and assign quirk PERST_PCIE_REV_CUTOFF

[Jim Quinlan]

The SW behavior for asserting PERST# for some SoCs only works up to a
specific PCIe HW version.  Declare this as a quirk and assign it
accordingly.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index dfd703571091..047783e33ae4 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -298,6 +298,8 @@ struct inbound_win {
 #define CFG_QUIRK_NO_RGR1_TIMER			BIT(4)
 /* PCIe PERST# must be asserted before internal bridge turned on */
 #define CFG_QUIRK_EARLY_PERST_ASSERT		BIT(5)
+/* PCIe SW PERST behavior only works up to a specific HW version */
+#define CFG_QUIRK_PERST_PCIE_REV_CUTOFF		BIT(6)
 
 /* FLAGS */
 #define BFLAG(pcie, flag)			((pcie)->cfg->flags & CFG_FLG_ ## flag)
@@ -1987,6 +1989,7 @@ static const struct pcie_cfg_data bcm4908_cfg = {
 	.perst_set	= brcm_pcie_perst_set_4908,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
+	.quirks		= CFG_QUIRK_PERST_PCIE_REV_CUTOFF,
 };
 
 static const struct pcie_cfg_data bcm7278_cfg = {
@@ -2168,7 +2171,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		goto fail;
 
 	pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
-	if (pcie->cfg->soc_base == BCM4908 &&
+	if (BQUIRK(pcie, PERST_PCIE_REV_CUTOFF) &&
 	    pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
 		dev_err(pcie->dev, "hardware revision with unsupported PERST# setup\n");
 		ret = -ENODEV;
-- 
2.34.1

[PATCH 11/13] PCI: brcmstb: Put max_burst_size setting in cfg_data

[Jim Quinlan]

Rather than ascertaining the max_burst_size setting by comparing different
SoC IDs, specify the setting in the config_data structure for each SoC.
Also add a comment on each SoC's encoding of this field.

Note: Previously, the max burst setting used for the 4908 Soc was 2.
However, it appears that the setting of 2 is illegal for this chip.  Set it
to 0 as (a) this is the safest (smallest) choice and (b) the 2 was probably
interpreted by the HW as a 0 anyway.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 29 ++++++++++++---------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 047783e33ae4..8bca98e6f5fa 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -317,6 +317,7 @@ struct pcie_cfg_data {
 	u32 flags;
 	u32 quirks;
 	u8 num_inbound_wins;
+	u8 burst_setting;
 	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
 	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
 	int (*post_setup)(struct brcm_pcie *pcie);
@@ -1153,7 +1154,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	void __iomem *base = pcie->base;
 	struct pci_host_bridge *bridge;
 	struct resource_entry *entry;
-	u32 tmp, burst, num_lanes, num_lanes_cap;
+	u32 tmp, num_lanes, num_lanes_cap;
 	u8 num_out_wins = 0;
 	int num_inbound_wins = 0;
 	int memc, ret;
@@ -1188,20 +1189,6 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	/* Wait for SerDes to be stable */
 	usleep_range(100, 200);
 
-	/*
-	 * SCB_MAX_BURST_SIZE is a two bit field.  For GENERIC chips it
-	 * is encoded as 0=128, 1=256, 2=512, 3=Rsvd, for BCM7278 it
-	 * is encoded as 0=Rsvd, 1=128, 2=256, 3=512.
-	 */
-	if (BFLAG(pcie, IS_BMIPS))
-		burst = 0x1; /* 256 bytes */
-	else if (pcie->cfg->soc_base == BCM2711)
-		burst = 0x0; /* 128 bytes */
-	else if (pcie->cfg->soc_base == BCM7278)
-		burst = 0x3; /* 512 bytes */
-	else
-		burst = 0x2; /* 512 bytes */
-
 	/*
 	 * Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN,
 	 * RCB_MPS_MODE, RCB_64B_MODE
@@ -1209,7 +1196,8 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	tmp = readl(base + PCIE_MISC_MISC_CTRL);
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
-	u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
+	u32p_replace_bits(&tmp, pcie->cfg->burst_setting,
+			  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE_MASK);
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK);
 	writel(tmp, base + PCIE_MISC_MISC_CTRL);
@@ -1961,6 +1949,7 @@ static const struct pcie_cfg_data generic_cfg = {
 	.perst_set	= brcm_pcie_perst_set_generic,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
+	.burst_setting	= 0x2, /* 0=128B, 1=256B, 2=512B, 3=Rsvd */
 };
 
 static const struct pcie_cfg_data bcm2711_cfg = {
@@ -1970,6 +1959,7 @@ static const struct pcie_cfg_data bcm2711_cfg = {
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
 	.quirks		= CFG_QUIRK_EARLY_PERST_ASSERT,
+	.burst_setting	= 0x0, /* 0=128B, 1=256B, 2=512B, 3=Rsvd */
 };
 
 static const struct pcie_cfg_data bcm2712_cfg = {
@@ -1981,6 +1971,7 @@ static const struct pcie_cfg_data bcm2712_cfg = {
 	.quirks		= CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN |
 		CFG_QUIRK_NO_RGR1_TIMER,
 	.num_inbound_wins = 10,
+	.burst_setting	= 0x2, /* 0=64B, 1=128B, 2=256B, 3=Rsvd */
 };
 
 static const struct pcie_cfg_data bcm4908_cfg = {
@@ -1990,6 +1981,7 @@ static const struct pcie_cfg_data bcm4908_cfg = {
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
 	.quirks		= CFG_QUIRK_PERST_PCIE_REV_CUTOFF,
+	.burst_setting	= 0x0, /* 0=64B, 1=128B, 2=Rsvd, 3=Rsvd */
 };
 
 static const struct pcie_cfg_data bcm7278_cfg = {
@@ -1998,6 +1990,7 @@ static const struct pcie_cfg_data bcm7278_cfg = {
 	.perst_set	= brcm_pcie_perst_set_7278,
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_7278,
 	.num_inbound_wins = 3,
+	.burst_setting	= 0x3, /* 0=Resv, 1=128B, 2=256B, 3=512B */
 };
 
 static const struct pcie_cfg_data bcm7425_cfg = {
@@ -2009,6 +2002,7 @@ static const struct pcie_cfg_data bcm7425_cfg = {
 	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR
 		| CFG_QUIRK_OB_WIN_MAXSZ_128MB | CFG_QUIRK_32BIT_PCI_OPS,
 	.flags		= CFG_FLG_IS_BMIPS,
+	.burst_setting = 1, /* 0=128B, 1=256B, 2=Rsvd, 3=Rsvd */
 };
 
 static const struct pcie_cfg_data bcm7435_cfg = {
@@ -2020,6 +2014,7 @@ static const struct pcie_cfg_data bcm7435_cfg = {
 	.quirks		= CFG_QUIRK_OB_WIN_32BIT_ADDR
 		| CFG_QUIRK_OB_WIN_MAXSZ_128MB,
 	.flags		= CFG_FLG_IS_BMIPS,
+	.burst_setting = 1, /* 0=128B, 1=256B, 2=Rsvd, 3=Rsvd */
 };
 
 static const struct pcie_cfg_data bcm7216_cfg = {
@@ -2029,6 +2024,7 @@ static const struct pcie_cfg_data bcm7216_cfg = {
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_7278,
 	.flags		= CFG_FLG_HAS_PHY | CFG_FLG_HAS_ERR_REPORT,
 	.num_inbound_wins = 3,
+	.burst_setting	= 0x3, /* 0=Resv, 1=128B, 2=256B, 3=512B */
 };
 
 static const struct pcie_cfg_data bcm7712_cfg = {
@@ -2038,6 +2034,7 @@ static const struct pcie_cfg_data bcm7712_cfg = {
 	.soc_base	= BCM7712,
 	.num_inbound_wins = 10,
 	.quirks		= CFG_QUIRK_NO_RGR1_TIMER,
+	.burst_setting	= 0x2, /* 0=64B, 1=128B, 2=256B, 3=Resv */
 };
 
 static const struct of_device_id brcm_pcie_match[] = {
-- 
2.34.1

[PATCH 12/13] PCI: brcmstb: Use order-0 indexing for inbound BAR window array

[Jim Quinlan]

The register names for the inbound BAR registers use order-1 indexing,
e.g. the first is BAR1.  The code followed this nomenclature by not using
the 0th element of the inbound array.

Undo this approach and use order-0 indexing.  Also use a struct pointer
variable in two functions instead of array indexing.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 49 ++++++++++++---------------
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 8bca98e6f5fa..53984cfa2c96 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -954,8 +954,8 @@ static void add_inbound_win(struct inbound_win *b, u8 *count, u64 size,
 	(*count)++;
 }
 
-static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
-				      struct inbound_win inbound_wins[])
+static int brcm_pcie_get_ib_wins(struct brcm_pcie *pcie,
+				 struct inbound_win *ib_win)
 {
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
 	u64 pci_offset, cpu_addr, size = 0, tot_size = 0;
@@ -965,13 +965,6 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
 	int ret, i = 0;
 	u8 n = 0;
 
-	/*
-	 * The HW registers (and PCIe) use order-1 numbering for BARs.  As such,
-	 * we have inbound_wins[0] unused and BAR1 starts at inbound_wins[1].
-	 */
-	struct inbound_win *b_begin = &inbound_wins[1];
-	struct inbound_win *b = b_begin;
-
 	/*
 	 * STB chips beside 7712 disable the first inbound window default.
 	 * Rather being mapped to system memory it is mapped to the
@@ -980,7 +973,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
 	 * SoCs.
 	 */
 	if (pcie->cfg->soc_base != BCM7712)
-		add_inbound_win(b++, &n, 0, 0, 0);
+		add_inbound_win(ib_win++, &n, 0, 0, 0);
 
 	resource_list_for_each_entry(entry, &bridge->dma_ranges) {
 		u64 pcie_start = entry->res->start - entry->offset;
@@ -997,7 +990,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
 		 * two.
 		 */
 		if (pcie->cfg->soc_base == BCM7712)
-			add_inbound_win(b++, &n, size, cpu_start, pcie_start);
+			add_inbound_win(ib_win++, &n, size, cpu_start, pcie_start);
 
 		if (n > pcie->cfg->num_inbound_wins)
 			break;
@@ -1081,44 +1074,44 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
 	}
 
 	/* Enable inbound window 2, the main inbound window for STB chips */
-	add_inbound_win(b++, &n, size, cpu_addr, pci_offset);
+	add_inbound_win(ib_win++, &n, size, cpu_addr, pci_offset);
 
 	/*
 	 * Disable inbound window 3.  On some chips presents the same
 	 * window as #2 but the data appears in a settable endianness.
 	 */
-	add_inbound_win(b++, &n, 0, 0, 0);
+	add_inbound_win(ib_win++, &n, 0, 0, 0);
 
 	return n;
 }
 
 static u32 brcm_bar_reg_offset(int bar)
 {
-	if (bar <= 3)
-		return PCIE_MISC_RC_BAR1_CONFIG_LO + 8 * (bar - 1);
+	if (bar <= 2)
+		return PCIE_MISC_RC_BAR1_CONFIG_LO + 8 * bar;
 	else
-		return PCIE_MISC_RC_BAR4_CONFIG_LO + 8 * (bar - 4);
+		return PCIE_MISC_RC_BAR4_CONFIG_LO + 8 * (bar - 3);
 }
 
 static u32 brcm_ubus_reg_offset(int bar)
 {
-	if (bar <= 3)
-		return PCIE_MISC_UBUS_BAR1_CONFIG_REMAP + 8 * (bar - 1);
+	if (bar <= 2)
+		return PCIE_MISC_UBUS_BAR1_CONFIG_REMAP + 8 * bar;
 	else
-		return PCIE_MISC_UBUS_BAR4_CONFIG_REMAP + 8 * (bar - 4);
+		return PCIE_MISC_UBUS_BAR4_CONFIG_REMAP + 8 * (bar - 3);
 }
 
-static void set_inbound_win_registers(struct brcm_pcie *pcie,
-				      const struct inbound_win *inbound_wins,
-				      u8 num_inbound_wins)
+static void brcm_pcie_set_ib_win_registers(struct brcm_pcie *pcie,
+					   const struct inbound_win *ib_win,
+					   u8 num_inbound_wins)
 {
 	void __iomem *base = pcie->base;
 	int i;
 
-	for (i = 1; i <= num_inbound_wins; i++) {
-		u64 pci_offset = inbound_wins[i].pci_offset;
-		u64 cpu_addr = inbound_wins[i].cpu_addr;
-		u64 size = inbound_wins[i].size;
+	for (i = 0; i < num_inbound_wins; i++, ib_win++) {
+		u64 pci_offset = ib_win->pci_offset;
+		u64 cpu_addr = ib_win->cpu_addr;
+		u64 size = ib_win->size;
 		u32 reg_offset = brcm_bar_reg_offset(i);
 		u32 tmp = lower_32_bits(pci_offset);
 
@@ -1202,11 +1195,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK);
 	writel(tmp, base + PCIE_MISC_MISC_CTRL);
 
-	num_inbound_wins = brcm_pcie_get_inbound_wins(pcie, inbound_wins);
+	num_inbound_wins = brcm_pcie_get_ib_wins(pcie, inbound_wins);
 	if (num_inbound_wins < 0)
 		return num_inbound_wins;
 
-	set_inbound_win_registers(pcie, inbound_wins, num_inbound_wins);
+	brcm_pcie_set_ib_win_registers(pcie, inbound_wins, num_inbound_wins);
 
 	if (!brcm_pcie_rc_mode(pcie)) {
 		dev_err(pcie->dev, "PCIe RC controller misconfigured as Endpoint\n");
-- 
2.34.1

[PATCH 13/13] PCI: brcmstb: Split up complicated function into two variants

[Jim Quinlan]

The function brcm_pcie_get_inbound_wins() is abstruse.  Most of
its code is used exclusively by older chips, where the PCIe HW employed a
baroque internal inbound window mapping scheme which was an artifact for
when the PCIe would be configured as an endpoint.

Create two variants of this function, one for the newer chips and one for
the older chips, and assign them using the config_data structures.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 105 +++++++++++++++++---------
 1 file changed, 68 insertions(+), 37 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 53984cfa2c96..45e8a9e030d4 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -321,6 +321,9 @@ struct pcie_cfg_data {
 	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
 	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
 	int (*post_setup)(struct brcm_pcie *pcie);
+	int (*get_ib_wins)(struct brcm_pcie *pcie, struct inbound_win
+			   *inbound_wins);
+
 };
 
 struct subdev_regulators {
@@ -954,8 +957,58 @@ static void add_inbound_win(struct inbound_win *b, u8 *count, u64 size,
 	(*count)++;
 }
 
+/*
+ * This is used by newer SoCs. It configures the inbound mapping windows
+ * in accordance to the values of the dma-ranges properties.
+ */
 static int brcm_pcie_get_ib_wins(struct brcm_pcie *pcie,
 				 struct inbound_win *ib_win)
+{
+	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+	struct resource_entry *entry;
+	u64 size;
+	u8 n = 0;
+
+	resource_list_for_each_entry(entry, &bridge->dma_ranges) {
+		u64 pcie_start = entry->res->start - entry->offset;
+		u64 cpu_start = entry->res->start;
+
+		size = resource_size(entry->res);
+		add_inbound_win(ib_win++, &n, size, cpu_start, pcie_start);
+		if (n > pcie->cfg->num_inbound_wins)
+			break;
+	}
+
+	if (!n) {
+		dev_err(pcie->dev, "DT node has no dma-ranges\n");
+		return -EINVAL;
+	}
+
+	return n;
+}
+
+/*
+ * Originally, the Broadcom STB PCIe HW played the endpoint (EP) role.  As
+ * an EP, one of its goals was to present system memory as a single
+ * contigous PCIe BAR.  So if there was two regions of system memory, say
+ * 1GB@0GB and 2GB@2GB, these two regions would be presented as a
+ * contiguous BAR that was 3GB in size and started at a PCIe offset that
+ * was configured by SW.
+ *
+ * Then the same PCIe HW was modified to also play the Root Complex (RC)
+ * role and the same internal mapping strategy was employed.  For any SoC
+ * that uses this scheme, each "BAR" is an inbound window and the PCIe HW
+ * is internally mapped and hard-wired to system memory regions.  Even
+ * though the code of the function below uses the dma-ranges properties, it
+ * is unable to configure the CPU region that is covered, but it can set
+ * the size and offset of the PCIe side of the window.
+ *
+ * Newer SoCs use the brcm_pcie_get_ib_wins() function have the freedom to
+ * configure mapping windows from any CPU region to any PCIe region,
+ * provided they follow the rules on offset alignment and size.
+ */
+static int brcm_pcie_get_ib_wins_internal_map(struct brcm_pcie *pcie,
+					      struct inbound_win *ib_win)
 {
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
 	u64 pci_offset, cpu_addr, size = 0, tot_size = 0;
@@ -965,33 +1018,15 @@ static int brcm_pcie_get_ib_wins(struct brcm_pcie *pcie,
 	int ret, i = 0;
 	u8 n = 0;
 
-	/*
-	 * STB chips beside 7712 disable the first inbound window default.
-	 * Rather being mapped to system memory it is mapped to the
-	 * internal registers of the SoC.  This feature is deprecated, has
-	 * security considerations, and is not implemented in our modern
-	 * SoCs.
-	 */
-	if (pcie->cfg->soc_base != BCM7712)
-		add_inbound_win(ib_win++, &n, 0, 0, 0);
+	/* By default, disable the first inbound window */
+	add_inbound_win(ib_win++, &n, 0, 0, 0);
 
 	resource_list_for_each_entry(entry, &bridge->dma_ranges) {
 		u64 pcie_start = entry->res->start - entry->offset;
-		u64 cpu_start = entry->res->start;
 
-		size = resource_size(entry->res);
-		tot_size += size;
+		tot_size += resource_size(entry->res);
 		if (pcie_start < lowest_pcie_addr)
 			lowest_pcie_addr = pcie_start;
-		/*
-		 * 7712 and newer chips may have many BARs, with each
-		 * offering a non-overlapping viewport to system memory.
-		 * That being said, each BARs size must still be a power of
-		 * two.
-		 */
-		if (pcie->cfg->soc_base == BCM7712)
-			add_inbound_win(ib_win++, &n, size, cpu_start, pcie_start);
-
 		if (n > pcie->cfg->num_inbound_wins)
 			break;
 	}
@@ -1001,14 +1036,6 @@ static int brcm_pcie_get_ib_wins(struct brcm_pcie *pcie,
 		return -EINVAL;
 	}
 
-	/*
-	 * 7712 and newer chips do not have an internal memory mapping system
-	 * that enables multiple memory controllers.  As such, it can return
-	 * now w/o doing special configuration.
-	 */
-	if (pcie->cfg->soc_base == BCM7712)
-		return n;
-
 	ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
 						  PCIE_BRCM_MAX_MEMC);
 	if (ret <= 0) {
@@ -1123,13 +1150,8 @@ static void brcm_pcie_set_ib_win_registers(struct brcm_pcie *pcie,
 		/* Write high */
 		writel_relaxed(upper_32_bits(pci_offset), base + reg_offset + 4);
 
-		/*
-		 * Most STB chips:
-		 *     Do nothing.
-		 * 7712:
-		 *     All of their BARs need to be set.
-		 */
-		if (pcie->cfg->soc_base == BCM7712) {
+		/* SoCs w/o fixed internal mapping can remap the cpu_addr */
+		if (pcie->cfg->get_ib_wins == brcm_pcie_get_ib_wins) {
 			/* BUS remap register settings */
 			reg_offset = brcm_ubus_reg_offset(i);
 			tmp = lower_32_bits(cpu_addr) & ~0xfff;
@@ -1195,7 +1217,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK);
 	writel(tmp, base + PCIE_MISC_MISC_CTRL);
 
-	num_inbound_wins = brcm_pcie_get_ib_wins(pcie, inbound_wins);
+	num_inbound_wins = pcie->cfg->get_ib_wins(pcie, inbound_wins);
 	if (num_inbound_wins < 0)
 		return num_inbound_wins;
 
@@ -1943,6 +1965,7 @@ static const struct pcie_cfg_data generic_cfg = {
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
 	.num_inbound_wins = 3,
 	.burst_setting	= 0x2, /* 0=128B, 1=256B, 2=512B, 3=Rsvd */
+	.get_ib_wins	= brcm_pcie_get_ib_wins_internal_map,
 };
 
 static const struct pcie_cfg_data bcm2711_cfg = {
@@ -1953,6 +1976,7 @@ static const struct pcie_cfg_data bcm2711_cfg = {
 	.num_inbound_wins = 3,
 	.quirks		= CFG_QUIRK_EARLY_PERST_ASSERT,
 	.burst_setting	= 0x0, /* 0=128B, 1=256B, 2=512B, 3=Rsvd */
+	.get_ib_wins	= brcm_pcie_get_ib_wins_internal_map,
 };
 
 static const struct pcie_cfg_data bcm2712_cfg = {
@@ -1965,6 +1989,7 @@ static const struct pcie_cfg_data bcm2712_cfg = {
 		CFG_QUIRK_NO_RGR1_TIMER,
 	.num_inbound_wins = 10,
 	.burst_setting	= 0x2, /* 0=64B, 1=128B, 2=256B, 3=Rsvd */
+	.get_ib_wins	= brcm_pcie_get_ib_wins,
 };
 
 static const struct pcie_cfg_data bcm4908_cfg = {
@@ -1975,6 +2000,7 @@ static const struct pcie_cfg_data bcm4908_cfg = {
 	.num_inbound_wins = 3,
 	.quirks		= CFG_QUIRK_PERST_PCIE_REV_CUTOFF,
 	.burst_setting	= 0x0, /* 0=64B, 1=128B, 2=Rsvd, 3=Rsvd */
+	.get_ib_wins	= brcm_pcie_get_ib_wins_internal_map,
 };
 
 static const struct pcie_cfg_data bcm7278_cfg = {
@@ -1984,6 +2010,7 @@ static const struct pcie_cfg_data bcm7278_cfg = {
 	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_7278,
 	.num_inbound_wins = 3,
 	.burst_setting	= 0x3, /* 0=Resv, 1=128B, 2=256B, 3=512B */
+	.get_ib_wins	= brcm_pcie_get_ib_wins_internal_map,
 };
 
 static const struct pcie_cfg_data bcm7425_cfg = {
@@ -1996,6 +2023,7 @@ static const struct pcie_cfg_data bcm7425_cfg = {
 		| CFG_QUIRK_OB_WIN_MAXSZ_128MB | CFG_QUIRK_32BIT_PCI_OPS,
 	.flags		= CFG_FLG_IS_BMIPS,
 	.burst_setting = 1, /* 0=128B, 1=256B, 2=Rsvd, 3=Rsvd */
+	.get_ib_wins	= brcm_pcie_get_ib_wins_internal_map,
 };
 
 static const struct pcie_cfg_data bcm7435_cfg = {
@@ -2008,6 +2036,7 @@ static const struct pcie_cfg_data bcm7435_cfg = {
 		| CFG_QUIRK_OB_WIN_MAXSZ_128MB,
 	.flags		= CFG_FLG_IS_BMIPS,
 	.burst_setting = 1, /* 0=128B, 1=256B, 2=Rsvd, 3=Rsvd */
+	.get_ib_wins	= brcm_pcie_get_ib_wins_internal_map,
 };
 
 static const struct pcie_cfg_data bcm7216_cfg = {
@@ -2018,6 +2047,7 @@ static const struct pcie_cfg_data bcm7216_cfg = {
 	.flags		= CFG_FLG_HAS_PHY | CFG_FLG_HAS_ERR_REPORT,
 	.num_inbound_wins = 3,
 	.burst_setting	= 0x3, /* 0=Resv, 1=128B, 2=256B, 3=512B */
+	.get_ib_wins	= brcm_pcie_get_ib_wins_internal_map,
 };
 
 static const struct pcie_cfg_data bcm7712_cfg = {
@@ -2028,6 +2058,7 @@ static const struct pcie_cfg_data bcm7712_cfg = {
 	.num_inbound_wins = 10,
 	.quirks		= CFG_QUIRK_NO_RGR1_TIMER,
 	.burst_setting	= 0x2, /* 0=64B, 1=128B, 2=256B, 3=Resv */
+	.get_ib_wins	= brcm_pcie_get_ib_wins,
 };
 
 static const struct of_device_id brcm_pcie_match[] = {
-- 
2.34.1

Re: [PATCH 01/13] PCI: brcmstb: Remove redundant const specifier for struct fields

[sashiko-bot]

> All of the variables using this particular structure are declared as const.
> There is no reason to declare its individual fields as being const.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=1

Re: [PATCH 03/13] PCI: brcmstb: Add Broadcom quirks macro

[sashiko-bot]

> Introduce a 'BQUIRK()' macro to shorten the calculation of a specific
> quirk's value.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=3

Re: [PATCH 06/13] PCI: brcmstb: Declare and assign flag IS_BMIPS

[sashiko-bot]

> Eliminate a function that evaluates SoC chip IDs and instead declare and
> use a flag that indicates the SoC employs a CPU with a variant of the
> MIPS-1 ISA.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=6

Re: [PATCH 11/13] PCI: brcmstb: Put max_burst_size setting in cfg_data

[sashiko-bot]

> Rather than ascertaining the max_burst_size setting by comparing different
> SoC IDs, specify the setting in the config_data structure for each SoC.
> Also add a comment on each SoC's encoding of this field.
> 
> Note: Previously, the max burst setting used for the 4908 Soc was 2.
> However, it appears that the setting of 2 is illegal for this chip.  Set it
> to 0 as (a) this is the safest (smallest) choice and (b) the 2 was probably
> interpreted by the HW as a 0 anyway.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=11

Re: [PATCH 09/13] PCI: brcmstb: Declare and assign quirk EARLY_PERST_ASSERT

[sashiko-bot]

> Declare and assign a quirk where PERST# must be asserted prior to an
> internal bus being turned on.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=9

Re: [PATCH 04/13] PCI: brcmstb: Declare and assign quirk OB_WIN_32BIT_ADDR

[sashiko-bot]

> Even though only BMIPs chips have this quirk, it is more uniform and
> readable to define this specific quirk in which the outbound region must
> reside in the lower 4GB region.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=4

Re: [PATCH 08/13] PCI: brcmstb: Declare and assign quirk NO_RGR1_TIMER

[sashiko-bot]

> Very few SoCs do not have an internal bus timer whose timeout value may be
> adusted for L1SS operation.  Declare this as a quirk and assign it
> accordingly.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=8

Re: [PATCH 07/13] PCI: brcmstb: Declare and assign quirk 32BIT_PCI_OPS

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional teardown in brcm_pcie_remove on pci_host_probe failure causes UAF or NULL pointer dereference.
--

commit d3ea9196e71b4fdf62dbbe06f00b5b226f406396
Author: Jim Quinlan <james.quinlan@broadcom.com>

PCI: brcmstb: Declare and assign quirk 32BIT_PCI_OPS
    
Declare a new quirk that indicates SoCs where the config-space accesses are
restricted to 32bit R/W.

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 8927d1d97f4a8..be414d3468555 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c

[ ... ]

> @@ -2180,8 +2182,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	bridge->ops = pcie->cfg->soc_base == BCM7425 ?
> -				&brcm7425_pcie_ops : &brcm_pcie_ops;
> +	bridge->ops = BQUIRK(pcie, 32BIT_PCI_OPS)
> +		? &brcm_pcie_ops32 : &brcm_pcie_ops;
>  	bridge->sysdata = pcie;
>  
>  	platform_set_drvdata(pdev, pcie);

[Severity: High]
This isn't a bug introduced by this patch, but does this probe error path have
a use-after-free?

Right after the modified code in brcm_pcie_probe(), we have this error path:

	ret = pci_host_probe(bridge);
	if (!ret && !brcm_pcie_link_up(pcie))
		ret = -ENODEV;

	if (ret) {
		brcm_pcie_remove(pdev);
		return ret;
	}

If pci_host_probe() returns an error, it has already cleaned up and freed
bridge->bus.

When we then call brcm_pcie_remove(pdev), it unconditionally accesses the
freed pointer:

	pci_lock_rescan_remove();
	pci_stop_root_bus(bridge->bus);
	pci_remove_root_bus(bridge->bus);
	pci_unlock_rescan_remove();

Can this cause a kernel panic on device probe failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=7

Re: [PATCH 02/13] PCI: brcmstb: Use flags u32 instead of bools

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The newly introduced BFLAG macro lacks outer parentheses around the bitwise AND expression, introducing a dangerous operator precedence pitfall.

Pre-existing issues:
- [High] Uninitialized spinlock `bridge_lock` is acquired during early probe before being initialized.
--

commit c6e2c757989753d5008cfee8a289e1111796b0bf
Author: Jim Quinlan <james.quinlan@broadcom.com>

    PCI: brcmstb: Use flags u32 instead of bools

    This commit replaces separate boolean fields for SoC configurations with a
    single u32 bitvector. It introduces the BFLAG macro to provide a uniform
    way to assign and check these flags.

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 89aaa19d508a7..47862569937eb 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -286,16 +286,24 @@ struct inbound_win {
>   */
>  #define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN		BIT(0)
>  
> +/* FLAGS */
> +#define BFLAG(pcie, flag)			((pcie)->cfg->flags & CFG_FLG_ ## flag)

[Severity: Medium]
Will this macro evaluate correctly if used in a logical NOT expression like
!BFLAG(pcie, HAS_PHY)?

Without outer parentheses around the bitwise AND expression, an expression
like !BFLAG(...) expands to !(pcie)->cfg->flags & flag. This applies the
logical NOT before the bitwise AND, which could lead to incorrect evaluation.

Could outer parentheses be added to prevent operator precedence issues?

[ ... ]

> @@ -356,14 +364,14 @@ static int brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 val)
>  	unsigned long flags;
>  	int ret;
>  
> -	if (pcie->cfg->has_err_report)
> +	if (BFLAG(pcie, HAS_ERR_REPORT))
>  		spin_lock_irqsave(&pcie->bridge_lock, flags);

[Severity: High]
This is a pre-existing issue, but does this acquire an uninitialized spinlock
during early probe?

During device probe, brcm_pcie_probe() calls brcm_pcie_bridge_sw_init_set()
early in the sequence:

brcm_pcie_probe() {
    ...
    ret = brcm_pcie_bridge_sw_init_set(pcie, 0);
    if (ret)
        return dev_err_probe(&pdev->dev, ret,
                             "could not de-assert bridge reset\n");
    ...
}

If the SoC configuration has the HAS_ERR_REPORT flag set, this attempts to
acquire pcie->bridge_lock.

However, the spinlock isn't initialized until much later in probe:

[ ... ]

> @@ -2179,7 +2186,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	if (pcie->cfg->has_err_report) {
> +	if (BFLAG(pcie, HAS_ERR_REPORT)) {
>  		spin_lock_init(&pcie->bridge_lock);
>  		brcm_register_die_notifiers(pcie);
>  	}

As shown here at the end of brcm_pcie_probe(), spin_lock_init() happens
long after the first call to brcm_pcie_bridge_sw_init_set().

Could this cause lockdep splats or kernel crashes when PREEMPT_RT or lock
debugging is enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=2

Re: [PATCH 12/13] PCI: brcmstb: Use order-0 indexing for inbound BAR window array

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The shift from 1-based to 0-based array indexing for `inbound_wins` broke the MSI target address calculation. `brcm_pcie_setup()` was not updated and continues to evaluate the hardcoded index 2, which now points to a 0-sized disabled window (or uninitialized memory), forcing an incorrect MSI target address.
--

commit cc21218055610569186c9058228a2b11645c3501
Author: Jim Quinlan <james.quinlan@broadcom.com>

PCI: brcmstb: Use order-0 indexing for inbound BAR window array

The register names for the inbound BAR registers use order-1 indexing,
e.g. the first is BAR1. The code followed this nomenclature by not using
the 0th element of the inbound array. This commit undoes this approach
and uses order-0 indexing.

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 8bca98e6f5fa4..53984cfa2c96f 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
[ ... ]
> @@ -1202,11 +1195,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>  	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK);
>  	writel(tmp, base + PCIE_MISC_MISC_CTRL);
>  
> -	num_inbound_wins = brcm_pcie_get_inbound_wins(pcie, inbound_wins);
> +	num_inbound_wins = brcm_pcie_get_ib_wins(pcie, inbound_wins);

[Severity: High]
By changing the inbound_wins array indexing from 1-based to 0-based, the main
inbound window for non-7712 chips has shifted from index 2 to index 1.

Does this break the MSI target address configuration further down in
brcm_pcie_setup()?

Later in brcm_pcie_setup(), the code still unconditionally evaluates index 2:

	if (inbound_wins[2].pci_offset >= SZ_4G ||
	    (inbound_wins[2].size + inbound_wins[2].pci_offset) < SZ_4G)
		pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_LT_4GB;
	else
		pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_GT_4GB;

For non-7712 chips, inbound_wins[2] will now point to the disabled zero-size
window, forcing the condition to always evaluate to true. This incorrectly
forces BRCM_MSI_TARGET_ADDR_LT_4GB and breaks >4GB MSI allocation.

Additionally, for BCM7712 chips with fewer than three dma-ranges,
inbound_wins[2] is never initialized and will result in an uninitialized stack
memory read. This issue remains unfixed in subsequent patches of the series.

>  	if (num_inbound_wins < 0)
>  		return num_inbound_wins;
>  
> -	set_inbound_win_registers(pcie, inbound_wins, num_inbound_wins);
> +	brcm_pcie_set_ib_win_registers(pcie, inbound_wins, num_inbound_wins);
>  
>  	if (!brcm_pcie_rc_mode(pcie)) {
>  		dev_err(pcie->dev, "PCIe RC controller misconfigured as Endpoint\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=12

Re: [PATCH 05/13] PCI: brcmstb: Declare and assign quirk OB_WIN_MAXSZ_128MB

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The OB_WIN_MAXSZ_128MB quirk handler silently truncates outbound memory resources and completely ignores subsequent memory windows due to an unconditional early exit.
--

commit 6fb01c14c0bdf317322f341660866644991a983a
Author: Jim Quinlan <james.quinlan@broadcom.com>

PCI: brcmstb: Declare and assign quirk OB_WIN_MAXSZ_128MB
    
Even though only BMIPs chips have this quirk, it is more uniform and
readable to define this specific quirk where each outbound window is must
be <= SZ_128M.

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 71ddd8a760af9..401898d04d521 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c

[ ... ]

> @@ -1294,11 +1296,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>  			return -EINVAL;
>  		}
>  
> -		if (is_bmips(pcie)) {
> +		if (BQUIRK(pcie, OB_WIN_MAXSZ_128MB)) {
>  			u64 start = res->start;
>  			unsigned int j, nwins = resource_size(res) / SZ_128M;
>  
> -			/* bmips PCIe outbound windows have a 128MB max size */
> +			/* PCIe outbound windows have a 128MB max size */
>  			if (nwins > BRCM_NUM_PCIE_OUT_WINS)
>  				nwins = BRCM_NUM_PCIE_OUT_WINS;

[Severity: High]
This is a pre-existing issue, but does this logic leave resources smaller than
128MB unmapped due to integer division dropping the remainder?

Also, instead of returning an error like the non-quirk path does when the
window count is exceeded, oversized resources seem to be silently capped here.

Could this lead to leaving some required memory regions unmapped?

>  			for (j = 0; j < nwins; j++, start += SZ_128M)

[Severity: High]
This is also a pre-existing issue inherited from the earlier is_bmips()
implementation, but looking at brcm_pcie_setup() just past this loop:

			for (j = 0; j < nwins; j++, start += SZ_128M)
				brcm_pcie_set_outbound_win(pcie, j, start,
							   start - entry->offset,
							   SZ_128M);
			break;

Does this unconditional break force an early exit from the resource iteration
loop over bridge->windows?

If platforms define multiple memory regions in their device tree (such as the
four 128MB PCIe memory regions in bcm7425.dtsi), wouldn't this break cause the
handler to map only the first resource and ignore the subsequent memory windows?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=5

Re: [PATCH 10/13] PCI: brcmstb: Declare and assign quirk PERST_PCIE_REV_CUTOFF

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free / NULL pointer dereference in error path when pci_host_probe() fails.
- [High] Use of uninitialized spinlock `pcie->bridge_lock`.
--

commit 8a2eb9e850a817eb0d5eb7cbe7b4bf1dbe6785e6
Author: Jim Quinlan <james.quinlan@broadcom.com>

PCI: brcmstb: Declare and assign quirk PERST_PCIE_REV_CUTOFF

This commit declares a new quirk for SoCs where the SW behavior for asserting
PERST# only works up to a specific PCIe HW version, and assigns it to the
bcm4908 configuration.

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index dfd7035710918..047783e33ae42 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c

[ ... ]

> @@ -2168,7 +2171,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>  		goto fail;
>  
>  	pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
> -	if (pcie->cfg->soc_base == BCM4908 &&
> +	if (BQUIRK(pcie, PERST_PCIE_REV_CUTOFF) &&
>  	    pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
>  		dev_err(pcie->dev, "hardware revision with unsupported PERST# setup\n");
>  		ret = -ENODEV;

[Severity: High]
This is a pre-existing issue, but looking at the error path later in
brcm_pcie_probe(), is it safe to call brcm_pcie_remove() when
pci_host_probe() fails?

If pci_host_probe(bridge) fails (for example, due to a memory allocation
failure inside pci_alloc_bus()), the bridge's bus structure might be freed
or remain NULL.

brcm_pcie_probe() {
    ...
    ret = pci_host_probe(bridge);
    if (ret) {
        brcm_pcie_remove(pdev);
        return ret;
    }
}

brcm_pcie_remove() {
    ...
    pci_stop_root_bus(bridge->bus);
}

Could this pci_stop_root_bus() call dereference a dangling or NULL
bridge->bus pointer, causing a crash instead of a graceful failure? Should
this path jump to the fail label instead to clean up local resources?

[Severity: High]
This is also a pre-existing issue, but does brcm_pcie_probe() use
pcie->bridge_lock before it is initialized?

Early in brcm_pcie_probe(), brcm_pcie_bridge_sw_init_set(pcie, 0) is called.
If the controller config has the HAS_ERR_REPORT flag set, this acquires
pcie->bridge_lock:

brcm_pcie_bridge_sw_init_set() {
    if (BFLAG(pcie, HAS_ERR_REPORT))
        spin_lock_irqsave(&pcie->bridge_lock, flags);
}

However, spin_lock_init(&pcie->bridge_lock) is not called until the very end
of brcm_pcie_probe(), right before it returns success.

Can this lead to an uninitialized spinlock usage on platforms with
HAS_ERR_REPORT enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=10

Re: [PATCH 13/13] PCI: brcmstb: Split up complicated function into two variants

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] An off-by-one error in the inbound window limit check allows one more window than supported by hardware to be processed, leading to out-of-bounds register writes.
- [High] The MSI target address calculation incorrectly hardcodes the array index `inbound_wins[2]`, evaluating either a disabled zero-sized window or uninitialized stack memory instead of the main inbound window.
--

commit 2fa82ffee79e487108d802c77822d346d0a063c9
Author: Jim Quinlan <james.quinlan@broadcom.com>

PCI: brcmstb: Split up complicated function into two variants

This patch refactors brcm_pcie_get_inbound_wins into two variants: one for
newer chips and one for older chips. It updates the configuration data
structures to assign the appropriate function pointer to get_ib_wins.

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 53984cfa2c96f..45e8a9e030d4a 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
[ ... ]
> @@ -954,8 +957,58 @@ static void add_inbound_win(struct inbound_win *b, u8 *count, u64 size,
>  	(*count)++;
>  }
>  
> +/*
> + * This is used by newer SoCs. It configures the inbound mapping windows
> + * in accordance to the values of the dma-ranges properties.
> + */
>  static int brcm_pcie_get_ib_wins(struct brcm_pcie *pcie,
>  				 struct inbound_win *ib_win)
> +{
> +	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
> +	struct resource_entry *entry;
> +	u64 size;
> +	u8 n = 0;
> +
> +	resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> +		u64 pcie_start = entry->res->start - entry->offset;
> +		u64 cpu_start = entry->res->start;
> +
> +		size = resource_size(entry->res);
> +		add_inbound_win(ib_win++, &n, size, cpu_start, pcie_start);
> +		if (n > pcie->cfg->num_inbound_wins)
> +			break;

[Severity: High]
This is a pre-existing issue, but does this off-by-one check allow n to
exceed the maximum hardware window count?

Because add_inbound_win() post-increments n, if a device tree provides more
dma-ranges than the maximum supported hardware windows, n can exceed
num_inbound_wins before the break occurs. The function will then return
this oversized window count.

If this happens, when brcm_pcie_set_ib_win_registers() subsequently iterates
over the returned count, could it fetch an out-of-bounds register offset
and overwrite unrelated hardware configuration space?

[ ... ]
> @@ -1195,7 +1217,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>  	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK);
>  	writel(tmp, base + PCIE_MISC_MISC_CTRL);
>  
> -	num_inbound_wins = brcm_pcie_get_ib_wins(pcie, inbound_wins);
> +	num_inbound_wins = pcie->cfg->get_ib_wins(pcie, inbound_wins);
>  	if (num_inbound_wins < 0)
>  		return num_inbound_wins;

[Severity: High]
This is a pre-existing issue, but does the setup logic later in this
function rely on inbound_wins[2] being initialized and valid?

In brcm_pcie_setup(), the inbound_wins array is allocated on the stack.
Later in the function, it calculates the MSI target address like this:

    if (inbound_wins[2].pci_offset >= SZ_4G ||
        (inbound_wins[2].size + inbound_wins[2].pci_offset) < SZ_4G)
            pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_LT_4GB;
    else
            pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_GT_4GB;

For newer SoCs using brcm_pcie_get_ib_wins(), if the device tree provides
fewer than 3 dma-ranges entries, could this logic read uninitialized stack
memory?

For older SoCs using brcm_pcie_get_ib_wins_internal_map(), inbound_wins[2]
is deliberately populated as a zero-sized, disabled window. Does evaluating
this zeroed window result in misconfiguring the MSI target address,
potentially causing PCIe device interrupts to fail?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=13