[PATCH] PCI: brcmstb: implement BCM4908 support

Subsystems: broadcom bcm2711/bcm2835 arm architecture, broadcom bcm7xxx arm architecture, broadcom stb pcie driver, pci native host bridge and endpoint drivers, pci subsystem, the rest

STALE1844d

3 messages, 3 authors, 2021-08-23 · open the first message on its own page

[PATCH] PCI: brcmstb: implement BCM4908 support

From: Rafał Miłecki <zajec5@gmail.com>
Date: 2021-08-16 12:53:10 

From: Rafał Miłecki <rafal@milecki.pl>

BCM4908 is Broadcom's 64-bit platform with Broadcom's own Brahma-B53
CPU(s). It uses the same PCIe hardware block as STB (Set-Top-Box) family
but in a slightly different revision & setup.

Registers in BCM4908 variant are mostly the same but controller setup
differs a bit. It requires setting few extra registers and takes
slightly different bars setup.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pci/controller/pcie-brcmstb.c | 137 +++++++++++++++++++++++---
 1 file changed, 123 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index cc30215f5a43..24bc7efcfdd5 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -51,15 +51,20 @@
 #define PCIE_RC_DL_MDIO_RD_DATA				0x1108
 
 #define PCIE_MISC_MISC_CTRL				0x4008
+#define  PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE		0x00000080
+#define  PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE		0x00000400
+#define  PCIE_MISC_MISC_CTRL_PCIE_IN_WR_COMBINE		0x00000800
 #define  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK		0x1000
 #define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK	0x2000
 #define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK	0x300000
+#define  PCIE_MISC_MISC_CTRL_BURST_ALIGN_MASK		0x00080000
 
 #define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK		0xf8000000
 #define  PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK		0x07c00000
 #define  PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK		0x0000001f
 #define  SCB_SIZE_MASK(x) PCIE_MISC_MISC_CTRL_SCB ## x ## _SIZE_MASK
 
+
 #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO		0x400c
 #define PCIE_MEM_WIN0_LO(win)	\
 		PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + ((win) * 8)
@@ -115,6 +120,9 @@
 #define PCIE_MEM_WIN0_LIMIT_HI(win)	\
 		PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
 
+#define PCIE_MISC_UBUS_BAR1_CONFIG_REMAP_OFFSET		0x40ac
+#define  PCIE_MISC_UBUS_BAR_CONFIG_ACCESS_EN		0x00000001
+
 #define PCIE_MISC_HARD_PCIE_HARD_DEBUG					0x4204
 #define  PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK	0x2
 #define  PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK		0x08000000
@@ -131,6 +139,13 @@
 #define PCIE_EXT_CFG_DATA				0x8000
 #define PCIE_EXT_CFG_INDEX				0x9000
 
+#define PCIE_CPU_INTR1_INTR_MASK_CLEAR_OFFSET		0x940c
+#define  PCIE_CPU_INTR1_PCIE_INTA_CPU_INTR		0x00000002
+#define  PCIE_CPU_INTR1_PCIE_INTB_CPU_INTR		0x00000004
+#define  PCIE_CPU_INTR1_PCIE_INTC_CPU_INTR		0x00000008
+#define  PCIE_CPU_INTR1_PCIE_INTD_CPU_INTR		0x00000010
+#define  PCIE_CPU_INTR1_PCIE_INTR_CPU_INTR		0x00000020
+
 #define  PCIE_RGR1_SW_INIT_1_PERST_MASK			0x1
 #define  PCIE_RGR1_SW_INIT_1_PERST_SHIFT		0x0
 
@@ -746,13 +761,19 @@ static inline void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32
 
 static inline void brcm_pcie_perst_set_4908(struct brcm_pcie *pcie, u32 val)
 {
-	if (WARN_ONCE(!pcie->perst_reset, "missing PERST# reset controller\n"))
-		return;
+	if (pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
+		brcm_pcie_perst_set_7278(pcie, val);
+	} else {
+		if (WARN_ONCE(!pcie->perst_reset, "missing PERST# reset controller\n"))
+			return;
 
-	if (val)
-		reset_control_assert(pcie->perst_reset);
-	else
-		reset_control_deassert(pcie->perst_reset);
+		if (val)
+			reset_control_assert(pcie->perst_reset);
+		else
+			reset_control_deassert(pcie->perst_reset);
+	}
+
+	usleep_range(10000, 20000);
 }
 
 static inline void brcm_pcie_perst_set_7278(struct brcm_pcie *pcie, u32 val)
@@ -861,6 +882,86 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
 	return 0;
 }
 
+static int brcm_pcie_setup_bcm4908(struct brcm_pcie *pcie)
+{
+	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+	void __iomem *base = pcie->base;
+	struct device *dev = pcie->dev;
+	struct resource_entry *entry;
+	u32 burst_align;
+	u32 burst;
+	u32 tmp;
+	int win;
+
+	pcie->perst_set(pcie, 0);
+
+	msleep(500);
+
+	if (!brcm_pcie_link_up(pcie)) {
+		dev_err(dev, "link down\n");
+		return -ENODEV;
+	}
+
+	/* setup lgacy outband interrupts */
+	tmp = PCIE_CPU_INTR1_PCIE_INTA_CPU_INTR |
+	      PCIE_CPU_INTR1_PCIE_INTB_CPU_INTR |
+	      PCIE_CPU_INTR1_PCIE_INTC_CPU_INTR |
+	      PCIE_CPU_INTR1_PCIE_INTD_CPU_INTR;
+	writel(tmp, base + PCIE_CPU_INTR1_INTR_MASK_CLEAR_OFFSET);
+
+	win = 0;
+	resource_list_for_each_entry(entry, &bridge->windows) {
+		struct resource *res = entry->res;
+		u64 pcie_addr;
+
+		if (resource_type(res) != IORESOURCE_MEM)
+			continue;
+
+		if (win >= BRCM_NUM_PCIE_OUT_WINS) {
+			dev_err(pcie->dev, "too many outbound wins\n");
+			return -EINVAL;
+		}
+
+		tmp = 0;
+		u32p_replace_bits(&tmp, res->start / SZ_1M,
+				  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK);
+		u32p_replace_bits(&tmp, res->end / SZ_1M,
+				  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK);
+		writel(tmp, base + PCIE_MEM_WIN0_BASE_LIMIT(win));
+
+		pcie_addr = res->start - entry->offset;
+		writel(lower_32_bits(pcie_addr), pcie->base + PCIE_MEM_WIN0_LO(win));
+		writel(upper_32_bits(pcie_addr), pcie->base + PCIE_MEM_WIN0_HI(win));
+
+		win++;
+	}
+
+	writel(0xf, base + PCIE_MISC_RC_BAR1_CONFIG_LO);
+
+	tmp = PCIE_MISC_UBUS_BAR_CONFIG_ACCESS_EN;
+	writel(tmp, base + PCIE_MISC_UBUS_BAR1_CONFIG_REMAP_OFFSET);
+
+	tmp = readl(base + PCIE_RC_CFG_PRIV1_ID_VAL3);
+	u32p_replace_bits(&tmp, PCI_CLASS_BRIDGE_PCI << 8,
+			  PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK);
+	writel(tmp, base + PCIE_RC_CFG_PRIV1_ID_VAL3);
+
+	/* Burst */
+	burst = 0x1; /* 128 B */
+	burst_align = 1;
+
+	tmp = readl(base + PCIE_MISC_MISC_CTRL);
+	u32p_replace_bits(&tmp, burst_align, PCIE_MISC_MISC_CTRL_BURST_ALIGN_MASK);
+	u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_IN_WR_COMBINE);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE);
+	writel(tmp, base + PCIE_MISC_MISC_CTRL);
+
+	return 0;
+}
+
 static int brcm_pcie_setup(struct brcm_pcie *pcie)
 {
 	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
@@ -1284,6 +1385,13 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pcie->perst_reset);
 	}
 
+	if (pcie->type == BCM4908) {
+		/* On BCM4908 we can read rev early and perst_set needs it */
+		pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
+
+		pcie->perst_set(pcie, 1);
+	}
+
 	ret = reset_control_reset(pcie->rescal);
 	if (ret)
 		dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
@@ -1295,16 +1403,17 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = brcm_pcie_setup(pcie);
-	if (ret)
-		goto fail;
+	if (pcie->type == BCM4908) {
+		ret = brcm_pcie_setup_bcm4908(pcie);
+		if (ret)
+			goto fail;
+	} else {
+		ret = brcm_pcie_setup(pcie);
+		if (ret)
+			goto fail;
+	}
 
 	pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
-	if (pcie->type == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
-		dev_err(pcie->dev, "hardware revision with unsupported PERST# setup\n");
-		ret = -ENODEV;
-		goto fail;
-	}
 
 	msi_np = of_parse_phandle(pcie->np, "msi-parent", 0);
 	if (pci_msi_enabled() && msi_np == pcie->np) {
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] PCI: brcmstb: implement BCM4908 support

From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2021-08-22 09:10:05 

+JimQ,

On 8/16/2021 2:50 PM, Rafał Miłecki wrote:
quoted hunk
From: Rafał Miłecki <rafal@milecki.pl>

BCM4908 is Broadcom's 64-bit platform with Broadcom's own Brahma-B53
CPU(s). It uses the same PCIe hardware block as STB (Set-Top-Box) family
but in a slightly different revision & setup.

Registers in BCM4908 variant are mostly the same but controller setup
differs a bit. It requires setting few extra registers and takes
slightly different bars setup.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
  drivers/pci/controller/pcie-brcmstb.c | 137 +++++++++++++++++++++++---
  1 file changed, 123 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index cc30215f5a43..24bc7efcfdd5 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -51,15 +51,20 @@
  #define PCIE_RC_DL_MDIO_RD_DATA				0x1108
  
  #define PCIE_MISC_MISC_CTRL				0x4008
+#define  PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE		0x00000080
+#define  PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE		0x00000400
+#define  PCIE_MISC_MISC_CTRL_PCIE_IN_WR_COMBINE		0x00000800
  #define  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK		0x1000
  #define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK	0x2000
  #define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK	0x300000
+#define  PCIE_MISC_MISC_CTRL_BURST_ALIGN_MASK		0x00080000
  
  #define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK		0xf8000000
  #define  PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK		0x07c00000
  #define  PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK		0x0000001f
  #define  SCB_SIZE_MASK(x) PCIE_MISC_MISC_CTRL_SCB ## x ## _SIZE_MASK
  
+
  #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO		0x400c
  #define PCIE_MEM_WIN0_LO(win)	\
  		PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + ((win) * 8)
@@ -115,6 +120,9 @@
  #define PCIE_MEM_WIN0_LIMIT_HI(win)	\
  		PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
  
+#define PCIE_MISC_UBUS_BAR1_CONFIG_REMAP_OFFSET		0x40ac
+#define  PCIE_MISC_UBUS_BAR_CONFIG_ACCESS_EN		0x00000001
+
  #define PCIE_MISC_HARD_PCIE_HARD_DEBUG					0x4204
  #define  PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK	0x2
  #define  PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK		0x08000000
@@ -131,6 +139,13 @@
  #define PCIE_EXT_CFG_DATA				0x8000
  #define PCIE_EXT_CFG_INDEX				0x9000
  
+#define PCIE_CPU_INTR1_INTR_MASK_CLEAR_OFFSET		0x940c
+#define  PCIE_CPU_INTR1_PCIE_INTA_CPU_INTR		0x00000002
+#define  PCIE_CPU_INTR1_PCIE_INTB_CPU_INTR		0x00000004
+#define  PCIE_CPU_INTR1_PCIE_INTC_CPU_INTR		0x00000008
+#define  PCIE_CPU_INTR1_PCIE_INTD_CPU_INTR		0x00000010
+#define  PCIE_CPU_INTR1_PCIE_INTR_CPU_INTR		0x00000020
+
  #define  PCIE_RGR1_SW_INIT_1_PERST_MASK			0x1
  #define  PCIE_RGR1_SW_INIT_1_PERST_SHIFT		0x0
  
@@ -746,13 +761,19 @@ static inline void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32
  
  static inline void brcm_pcie_perst_set_4908(struct brcm_pcie *pcie, u32 val)
  {
-	if (WARN_ONCE(!pcie->perst_reset, "missing PERST# reset controller\n"))
-		return;
+	if (pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
+		brcm_pcie_perst_set_7278(pcie, val);
+	} else {
+		if (WARN_ONCE(!pcie->perst_reset, "missing PERST# reset controller\n"))
+			return;
  
-	if (val)
-		reset_control_assert(pcie->perst_reset);
-	else
-		reset_control_deassert(pcie->perst_reset);
+		if (val)
+			reset_control_assert(pcie->perst_reset);
+		else
+			reset_control_deassert(pcie->perst_reset);
+	}
+
+	usleep_range(10000, 20000);
This delay would warrant a comment.
quoted hunk
  }
  
  static inline void brcm_pcie_perst_set_7278(struct brcm_pcie *pcie, u32 val)
@@ -861,6 +882,86 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
  	return 0;
  }
  
+static int brcm_pcie_setup_bcm4908(struct brcm_pcie *pcie)
+{
+	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+	void __iomem *base = pcie->base;
+	struct device *dev = pcie->dev;
+	struct resource_entry *entry;
+	u32 burst_align;
+	u32 burst;
+	u32 tmp;
+	int win;
+
+	pcie->perst_set(pcie, 0);
+
+	msleep(500);
+
+	if (!brcm_pcie_link_up(pcie)) {
+		dev_err(dev, "link down\n");
+		return -ENODEV;
+	}
+
+	/* setup lgacy outband interrupts */
s/lgacy/legacy/

+	tmp = PCIE_CPU_INTR1_PCIE_INTA_CPU_INTR |
+	      PCIE_CPU_INTR1_PCIE_INTB_CPU_INTR |
+	      PCIE_CPU_INTR1_PCIE_INTC_CPU_INTR |
+	      PCIE_CPU_INTR1_PCIE_INTD_CPU_INTR;
+	writel(tmp, base + PCIE_CPU_INTR1_INTR_MASK_CLEAR_OFFSET);
+
+	win = 0;
+	resource_list_for_each_entry(entry, &bridge->windows) {
+		struct resource *res = entry->res;
+		u64 pcie_addr;
+
+		if (resource_type(res) != IORESOURCE_MEM)
+			continue;
+
+		if (win >= BRCM_NUM_PCIE_OUT_WINS) {
+			dev_err(pcie->dev, "too many outbound wins\n");
+			return -EINVAL;
+		}
+
+		tmp = 0;
+		u32p_replace_bits(&tmp, res->start / SZ_1M,
+				  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK);
+		u32p_replace_bits(&tmp, res->end / SZ_1M,
+				  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK);
+		writel(tmp, base + PCIE_MEM_WIN0_BASE_LIMIT(win));
+
+		pcie_addr = res->start - entry->offset;
+		writel(lower_32_bits(pcie_addr), pcie->base + PCIE_MEM_WIN0_LO(win));
+		writel(upper_32_bits(pcie_addr), pcie->base + PCIE_MEM_WIN0_HI(win));
+
+		win++;
+	}
+
+	writel(0xf, base + PCIE_MISC_RC_BAR1_CONFIG_LO);
+
+	tmp = PCIE_MISC_UBUS_BAR_CONFIG_ACCESS_EN;
+	writel(tmp, base + PCIE_MISC_UBUS_BAR1_CONFIG_REMAP_OFFSET);
+
+	tmp = readl(base + PCIE_RC_CFG_PRIV1_ID_VAL3);
+	u32p_replace_bits(&tmp, PCI_CLASS_BRIDGE_PCI << 8,
+			  PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK);
+	writel(tmp, base + PCIE_RC_CFG_PRIV1_ID_VAL3);
+
+	/* Burst */
+	burst = 0x1; /* 128 B */
+	burst_align = 1;
+
+	tmp = readl(base + PCIE_MISC_MISC_CTRL);
+	u32p_replace_bits(&tmp, burst_align, PCIE_MISC_MISC_CTRL_BURST_ALIGN_MASK);
+	u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_IN_WR_COMBINE);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE);
+	writel(tmp, base + PCIE_MISC_MISC_CTRL);
I see why you have made the 4908 bridge setup completely indepdent from 
the STB version of brcm_pcie_setup() however in the process there is a 
number of possibly equally relevant initialization that is not done and 
may assume firmware initialized or hardware default values (those would 
not survive suspend/resume states). Can you see about making minimal 
changes to brcm_pcie_setup() such that it supports the way 4908 should 
be initialized?
quoted hunk
+
+	return 0;
+}
+
  static int brcm_pcie_setup(struct brcm_pcie *pcie)
  {
  	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
@@ -1284,6 +1385,13 @@ static int brcm_pcie_probe(struct platform_device *pdev)
  		return PTR_ERR(pcie->perst_reset);
  	}
  
+	if (pcie->type == BCM4908) {
+		/* On BCM4908 we can read rev early and perst_set needs it */
+		pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
+
+		pcie->perst_set(pcie, 1);
+	}
+
  	ret = reset_control_reset(pcie->rescal);
  	if (ret)
  		dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
@@ -1295,16 +1403,17 @@ static int brcm_pcie_probe(struct platform_device *pdev)
  		return ret;
  	}
  
-	ret = brcm_pcie_setup(pcie);
-	if (ret)
-		goto fail;
+	if (pcie->type == BCM4908) {
+		ret = brcm_pcie_setup_bcm4908(pcie);
+		if (ret)
+			goto fail;
+	} else {
+		ret = brcm_pcie_setup(pcie);
+		if (ret)
+			goto fail;
+	}
  
  	pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
-	if (pcie->type == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
-		dev_err(pcie->dev, "hardware revision with unsupported PERST# setup\n");
-		ret = -ENODEV;
-		goto fail;
-	}
  
  	msi_np = of_parse_phandle(pcie->np, "msi-parent", 0);
  	if (pci_msi_enabled() && msi_np == pcie->np) {
-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] PCI: brcmstb: implement BCM4908 support

From: Jim Quinlan <hidden>
Date: 2021-08-23 13:47:16 

Hello,
Some comments below...


On Mon, Aug 16, 2021 at 8:51 AM Rafał Miłecki [off-list ref] wrote:
quoted hunk
From: Rafał Miłecki <rafal@milecki.pl>

BCM4908 is Broadcom's 64-bit platform with Broadcom's own Brahma-B53
CPU(s). It uses the same PCIe hardware block as STB (Set-Top-Box) family
but in a slightly different revision & setup.

Registers in BCM4908 variant are mostly the same but controller setup
differs a bit. It requires setting few extra registers and takes
slightly different bars setup.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pci/controller/pcie-brcmstb.c | 137 +++++++++++++++++++++++---
 1 file changed, 123 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index cc30215f5a43..24bc7efcfdd5 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -51,15 +51,20 @@
 #define PCIE_RC_DL_MDIO_RD_DATA                                0x1108

 #define PCIE_MISC_MISC_CTRL                            0x4008
+#define  PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE         0x00000080
+#define  PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE         0x00000400
+#define  PCIE_MISC_MISC_CTRL_PCIE_IN_WR_COMBINE                0x00000800
 #define  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK                0x1000
 #define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK     0x2000
 #define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK       0x300000
+#define  PCIE_MISC_MISC_CTRL_BURST_ALIGN_MASK          0x00080000

 #define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK            0xf8000000
 #define  PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK            0x07c00000
 #define  PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK            0x0000001f
 #define  SCB_SIZE_MASK(x) PCIE_MISC_MISC_CTRL_SCB ## x ## _SIZE_MASK

+
Extra WS
quoted hunk
 #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO               0x400c
 #define PCIE_MEM_WIN0_LO(win)  \
                PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + ((win) * 8)
@@ -115,6 +120,9 @@
 #define PCIE_MEM_WIN0_LIMIT_HI(win)    \
                PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)

+#define PCIE_MISC_UBUS_BAR1_CONFIG_REMAP_OFFSET                0x40ac
+#define  PCIE_MISC_UBUS_BAR_CONFIG_ACCESS_EN           0x00000001
+
 #define PCIE_MISC_HARD_PCIE_HARD_DEBUG                                 0x4204
 #define  PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK       0x2
 #define  PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK               0x08000000
@@ -131,6 +139,13 @@
 #define PCIE_EXT_CFG_DATA                              0x8000
 #define PCIE_EXT_CFG_INDEX                             0x9000

+#define PCIE_CPU_INTR1_INTR_MASK_CLEAR_OFFSET          0x940c
+#define  PCIE_CPU_INTR1_PCIE_INTA_CPU_INTR             0x00000002
+#define  PCIE_CPU_INTR1_PCIE_INTB_CPU_INTR             0x00000004
+#define  PCIE_CPU_INTR1_PCIE_INTC_CPU_INTR             0x00000008
+#define  PCIE_CPU_INTR1_PCIE_INTD_CPU_INTR             0x00000010
+#define  PCIE_CPU_INTR1_PCIE_INTR_CPU_INTR             0x00000020
+
 #define  PCIE_RGR1_SW_INIT_1_PERST_MASK                        0x1
 #define  PCIE_RGR1_SW_INIT_1_PERST_SHIFT               0x0

@@ -746,13 +761,19 @@ static inline void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32

 static inline void brcm_pcie_perst_set_4908(struct brcm_pcie *pcie, u32 val)
 {
-       if (WARN_ONCE(!pcie->perst_reset, "missing PERST# reset controller\n"))
-               return;
+       if (pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
+               brcm_pcie_perst_set_7278(pcie, val);
+       } else {
+               if (WARN_ONCE(!pcie->perst_reset, "missing PERST# reset controller\n"))
+                       return;
Is this necessary?  Better to just assume it is there and call the
assert/deassrt routines always as they work on NULL.
-       if (val)
-               reset_control_assert(pcie->perst_reset);
-       else
-               reset_control_deassert(pcie->perst_reset);
+               if (val)
+                       reset_control_assert(pcie->perst_reset);
+               else
+                       reset_control_deassert(pcie->perst_reset);
+       }
+
+       usleep_range(10000, 20000);
Why so long?  And does it need to be long for both directions (assert/deassert)?
quoted hunk
 }

 static inline void brcm_pcie_perst_set_7278(struct brcm_pcie *pcie, u32 val)
@@ -861,6 +882,86 @@ static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
        return 0;
 }

+static int brcm_pcie_setup_bcm4908(struct brcm_pcie *pcie)
+{
+       struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+       void __iomem *base = pcie->base;
+       struct device *dev = pcie->dev;
+       struct resource_entry *entry;
+       u32 burst_align;
+       u32 burst;
+       u32 tmp;
+       int win;
+
+       pcie->perst_set(pcie, 0);
+
+       msleep(500);
+
+       if (!brcm_pcie_link_up(pcie)) {
+               dev_err(dev, "link down\n");
+               return -ENODEV;
+       }
+
+       /* setup lgacy outband interrupts */
+       tmp = PCIE_CPU_INTR1_PCIE_INTA_CPU_INTR |
+             PCIE_CPU_INTR1_PCIE_INTB_CPU_INTR |
+             PCIE_CPU_INTR1_PCIE_INTC_CPU_INTR |
+             PCIE_CPU_INTR1_PCIE_INTD_CPU_INTR;
+       writel(tmp, base + PCIE_CPU_INTR1_INTR_MASK_CLEAR_OFFSET);
Shouldn't this be taken care of by the interrupt-map and interrupt-map
mask properties of the PCIe DT node?  For example, most of our STB
chips use something like this:

interrupt-map-mask = <0x0 0x0 0x0 0x7>;
interrupt-map = <
0 0 0 1 &intc 0 44 4
0 0 0 2 &intc 0 45 4
0 0 0 3 &intc 0 46 4
0 0 0 4 &intc 0 47 4>;

If you  don't have an interrupt controller DT node/driver, try looking
at the existing BRCM ones or you  may have to craft your own.

+
+       win = 0;
+       resource_list_for_each_entry(entry, &bridge->windows) {
+               struct resource *res = entry->res;
+               u64 pcie_addr;
+
+               if (resource_type(res) != IORESOURCE_MEM)
+                       continue;
+
+               if (win >= BRCM_NUM_PCIE_OUT_WINS) {
+                       dev_err(pcie->dev, "too many outbound wins\n");
+                       return -EINVAL;
+               }
+
+               tmp = 0;
+               u32p_replace_bits(&tmp, res->start / SZ_1M,
+                                 PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK);
+               u32p_replace_bits(&tmp, res->end / SZ_1M,
+                                 PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK);
+               writel(tmp, base + PCIE_MEM_WIN0_BASE_LIMIT(win));
+
+               pcie_addr = res->start - entry->offset;
+               writel(lower_32_bits(pcie_addr), pcie->base + PCIE_MEM_WIN0_LO(win));
+               writel(upper_32_bits(pcie_addr), pcie->base + PCIE_MEM_WIN0_HI(win));
Again, can you try using the PCIe DT ranges/dma-ranges node properties
instead?

+
+               win++;
+       }
+
+       writel(0xf, base + PCIE_MISC_RC_BAR1_CONFIG_LO);
+
+       tmp = PCIE_MISC_UBUS_BAR_CONFIG_ACCESS_EN;
+       writel(tmp, base + PCIE_MISC_UBUS_BAR1_CONFIG_REMAP_OFFSET);
+
+       tmp = readl(base + PCIE_RC_CFG_PRIV1_ID_VAL3);
+       u32p_replace_bits(&tmp, PCI_CLASS_BRIDGE_PCI << 8,
+                         PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK);
+       writel(tmp, base + PCIE_RC_CFG_PRIV1_ID_VAL3);
Isn't this already in the driver?  If it is, you should at least make
it a function and call it -- the same code should not exist in
multiple places.

+
+       /* Burst */
+       burst = 0x1; /* 128 B */
+       burst_align = 1;
+
+       tmp = readl(base + PCIE_MISC_MISC_CTRL);if (
+       u32p_replace_bits(&tmp, burst_align, PCIE_MISC_MISC_CTRL_BURST_ALIGN_MASK);
+       u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
+       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
+       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_IN_WR_COMBINE);
+       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE);
+       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE);
It would be better if you could use the vanilla brcm_pcie_setup()
function and do something like
if (pcie->type == BCM4908)
    do_this();
else
    /* ... */
quoted hunk
+       writel(tmp, base + PCIE_MISC_MISC_CTRL);
+
+       return 0;
+}
+
 static int brcm_pcie_setup(struct brcm_pcie *pcie)
 {
        struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
@@ -1284,6 +1385,13 @@ static int brcm_pcie_probe(struct platform_device *pdev)
                return PTR_ERR(pcie->perst_reset);
        }

+       if (pcie->type == BCM4908) {
+               /* On BCM4908 we can read rev early and perst_set needs it */
+               pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
We might as well do this in all cases, no need for the 4908 qualifier.

+
+               pcie->perst_set(pcie, 1); +               writel(lower_32_bits(pcie_addr), pcie->base + PCIE_MEM_WIN0_LO(win));
+               writel(upper_32_bits(pcie_addr), pcie->base + PCIE_MEM_WIN0_HI(win));
Again, can you try using the PCIe DT ranges/dma-ranges node properties instead?

+
+               win++;
+       }
+
+       writel(0xf, base + PCIE_MISC_RC_BAR1_CONFIG_LO);
+
+       tmp = PCIE_MISC_UBUS_BAR_CONFIG_ACCESS_EN;
+       writel(tmp, base + PCIE_MISC_UBUS_BAR1_CONFIG_REMAP_OFFSET);
+
+       tmp = readl(base + PCIE_RC_CFG_PRIV1_ID_VAL3);
+       u32p_replace_bits(&tmp, PCI_CLASS_BRIDGE_PCI << 8,
+                         PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK);
+       writel(tmp, base + PCIE_RC_CFG_PRIV1_ID_VAL3);
Isn't this already in the driver?  If it is, it you should at least
make it a function and call it -- the same code should not exist in
multiple places.

+
+       /* Burst */
+       burst = 0x1; /* 128 B */
+       burst_align = 1;
+
+       tmp = readl(base + PCIE_MISC_MISC_CTRL);if (
+       u32p_replace_bits(&tmp, burst_align, PCIE_MISC_MISC_CTRL_BURST_ALIGN_MASK);
+       u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
+       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
+       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_IN_WR_COMBINE);
+       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE);
+       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE);
It would be better if you could use the vanilla brcm_pcie_setup()
function and do something like
if (pcie->type == BCM4908)
    do_this();
else
    /* ... */

+       writel(tmp, base + PCIE_MISC_MISC_CTRL);
+
b
quoted hunk
+       }
+
        ret = reset_control_reset(pcie->rescal);
        if (ret)
                dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
@@ -1295,16 +1403,17 @@ static int brcm_pcie_probe(struct platform_device *pdev)
                return ret;
        }

-       ret = brcm_pcie_setup(pcie);
-       if (ret)
-               goto fail;
+       if (pcie->type == BCM4908) {
+               ret = brcm_pcie_setup_bcm4908(pcie);
+               if (ret)
+                       goto fail;
+       } else {
+               ret = brcm_pcie_setup(pcie);
+               if (ret)
+                       goto fail;
+       }
As Florian said, try to use the existing routine with clauses for
4908-specific code.

Regards,
Jim Quinlan, Broadcom STB
        pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
-       if (pcie->type == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
-               dev_err(pcie->dev, "hardware revision with unsupported PERST# setup\n");
-               ret = -ENODEV;
-               goto fail;
-       }

        msi_np = of_parse_phandle(pcie->np, "msi-parent", 0);
        if (pci_msi_enabled() && msi_np == pcie->np) {
--
2.26.2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help