[PATCH v5 -next 00/11] Add PCIe support for bcm2712

[PATCH v5 -next 00/11] Add PCIe support for bcm2712

[Stanimir Varbanov]

Here is v5 of the series which aims to add support for PCIe on bcm2712 SoC
used by RPi5. Previous v4 can be found at [1].

Based the series on linux-next because of vc4 gpu node in bcm2712.dtsi.

v4 -> v5 changes include:
 - Addressed comments to interrupt-controller driver. (Thomas)
 - Fixed DTB warnings  broadcom/bcm2712-rpi-5-b.dtb.
 - New patch in the series to fix missing of_node_put.
 - New patch to make a softdep to a MIP MSI-X driver.
 - Dropped the patch which adds MSI-X support in pcie-brcmstb driver,
   and instead use DT dma-ranges to pass the needed information. (Jim)

For more detailed info check patches.

Comments are welcome!
~Stan

[1] https://patchwork.kernel.org/project/linux-pci/cover/20241025124515.14066-1-svarbanov@suse.de/

Stanimir Varbanov (11):
  dt-bindings: interrupt-controller: Add bcm2712 MSI-X DT bindings
  dt-bindings: PCI: brcmstb: Update bindings for PCIe on bcm2712
  irqchip: Add Broadcom bcm2712 MSI-X interrupt controller
  PCI: brcmstb: Reuse config structure
  PCI: brcmstb: Expand inbound window size up to 64GB
  PCI: brcmstb: Add bcm2712 support
  PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
  PCI: brcmstb: Adding a softdep to MIP MSI-X driver
  PCI: brcmstb: Fix for missing of_node_put
  arm64: dts: broadcom: bcm2712: Add PCIe DT nodes
  arm64: dts: broadcom: bcm2712-rpi-5-b: Enable PCIe DT nodes

 .../brcm,bcm2712-msix.yaml                    |  60 ++++
 .../bindings/pci/brcm,stb-pcie.yaml           |   6 +-
 .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   8 +
 arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 147 +++++++++
 drivers/irqchip/Kconfig                       |  16 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-bcm2712-mip.c             | 292 ++++++++++++++++++
 drivers/pci/controller/pcie-brcmstb.c         | 147 ++++++---
 8 files changed, 632 insertions(+), 45 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
 create mode 100644 drivers/irqchip/irq-bcm2712-mip.c

-- 
2.47.0

[PATCH v5 -next 01/11] dt-bindings: interrupt-controller: Add bcm2712 MSI-X DT bindings

[Stanimir Varbanov]

Adds DT bindings for bcm2712 MSI-X interrupt peripheral controller.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
v4 -> v5:
 - No changes.

 .../brcm,bcm2712-msix.yaml                    | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
new file mode 100644
index 000000000000..c84614663b5d
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/brcm,bcm2712-msix.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom bcm2712 MSI-X Interrupt Peripheral support
+
+maintainers:
+  - Stanimir Varbanov <svarbanov@suse.de>
+
+description:
+  This interrupt controller is used to provide interrupt vectors to the
+  generic interrupt controller (GIC) on bcm2712. It will be used as
+  external MSI-X controller for PCIe root complex.
+
+allOf:
+  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
+
+properties:
+  compatible:
+    const: brcm,bcm2712-mip
+
+  reg:
+    items:
+      - description: Base register address
+      - description: PCIe message address
+
+  "#msi-cells":
+    const: 0
+
+  brcm,msi-offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Shift the allocated MSI's.
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - msi-controller
+  - msi-ranges
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    axi {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        msi-controller@1000130000 {
+            compatible = "brcm,bcm2712-mip";
+            reg = <0x10 0x00130000 0x00 0xc0>,
+                  <0xff 0xfffff000 0x00 0x1000>;
+            msi-controller;
+            #msi-cells = <0>;
+            msi-ranges = <&gicv2 GIC_SPI 128 IRQ_TYPE_EDGE_RISING 64>;
+        };
+    };
-- 
2.47.0

Re: [PATCH v5 -next 01/11] dt-bindings: interrupt-controller: Add bcm2712 MSI-X DT bindings

[Florian Fainelli]

On 1/20/25 05:01, Stanimir Varbanov wrote:
> Adds DT bindings for bcm2712 MSI-X interrupt peripheral controller.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[PATCH v5 -next 02/11] dt-bindings: PCI: brcmstb: Update bindings for PCIe on bcm2712

[Stanimir Varbanov]

Update brcmstb PCIe controller bindings with bcm2712 compatible.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
 Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 2ad1652c2584..29f0e1eb5096 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -14,6 +14,7 @@ properties:
     items:
       - enum:
           - brcm,bcm2711-pcie # The Raspberry Pi 4
+          - brcm,bcm2712-pcie # Raspberry Pi 5
           - brcm,bcm4908-pcie
           - brcm,bcm7211-pcie # Broadcom STB version of RPi4
           - brcm,bcm7216-pcie # Broadcom 7216 Arm
@@ -101,7 +102,10 @@ properties:
 
   reset-names:
     minItems: 1
-    maxItems: 3
+    items:
+      - enum: [perst, rescal]
+      - const: bridge
+      - const: swinit
 
 required:
   - compatible
-- 
2.47.0

Re: [PATCH v5 -next 02/11] dt-bindings: PCI: brcmstb: Update bindings for PCIe on bcm2712

[Florian Fainelli]

On 1/20/25 05:01, Stanimir Varbanov wrote:
> Update brcmstb PCIe controller bindings with bcm2712 compatible.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH v5 -next 02/11] dt-bindings: PCI: brcmstb: Update bindings for PCIe on bcm2712

[Rob Herring (Arm)]

On Mon, 20 Jan 2025 15:01:10 +0200, Stanimir Varbanov wrote:
> Update brcmstb PCIe controller bindings with bcm2712 compatible.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH v5 -next 03/11] irqchip: Add Broadcom bcm2712 MSI-X interrupt controller

[Stanimir Varbanov]

Add an interrupt controller driver for MSI-X Interrupt Peripheral (MIP)
hardware block found in bcm2712. The interrupt controller is used to
handle MSI-X interrupts from peripherials behind PCIe endpoints like
RP1 south bridge found in RPi5.

There are two MIPs on bcm2712, the first has 64 consecutive SPIs
assigned to 64 output vectors, and the second has 17 SPIs, but only
8 of them are consecutive starting at the 8th output vector.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v4 -> v5:
 - Use scoped guard in mip_alloc_hwirq()/mip_free_hwirq() (Thomas)
 - Removed last function argument of mip_alloc_hwirq() (Thomas)
 - Moved initialization register writes in mip_init_domains()
   and clear the comments (Thomas)

 drivers/irqchip/Kconfig           |  16 ++
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-bcm2712-mip.c | 292 ++++++++++++++++++++++++++++++
 3 files changed, 309 insertions(+)
 create mode 100644 drivers/irqchip/irq-bcm2712-mip.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 9bee02db1643..380fd04563f6 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -109,6 +109,22 @@ config I8259
 	bool
 	select IRQ_DOMAIN
 
+config BCM2712_MIP
+	tristate "Broadcom BCM2712 MSI-X Interrupt Peripheral support"
+	depends on ARCH_BRCMSTB || COMPILE_TEST
+	default m if ARCH_BRCMSTB
+	depends on ARM_GIC
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN_HIERARCHY
+	select GENERIC_MSI_IRQ
+	select IRQ_MSI_LIB
+	help
+	  Enable support for the Broadcom BCM2712 MSI-X target peripheral
+	  (MIP) needed by brcmstb PCIe to handle MSI-X interrupts on
+	  Raspberry Pi 5.
+
+	  If unsure say n.
+
 config BCM6345_L1_IRQ
 	bool
 	select GENERIC_IRQ_CHIP
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 25e9ad29b8c4..411385c4f3ad 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_XTENSA_MX)			+= irq-xtensa-mx.o
 obj-$(CONFIG_XILINX_INTC)		+= irq-xilinx-intc.o
 obj-$(CONFIG_IRQ_CROSSBAR)		+= irq-crossbar.o
 obj-$(CONFIG_SOC_VF610)			+= irq-vf610-mscm-ir.o
+obj-$(CONFIG_BCM2712_MIP)               += irq-bcm2712-mip.o
 obj-$(CONFIG_BCM6345_L1_IRQ)		+= irq-bcm6345-l1.o
 obj-$(CONFIG_BCM7038_L1_IRQ)		+= irq-bcm7038-l1.o
 obj-$(CONFIG_BCM7120_L2_IRQ)		+= irq-bcm7120-l2.o
diff --git a/drivers/irqchip/irq-bcm2712-mip.c b/drivers/irqchip/irq-bcm2712-mip.c
new file mode 100644
index 000000000000..49a19db2d1e1
--- /dev/null
+++ b/drivers/irqchip/irq-bcm2712-mip.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Raspberry Pi Ltd., All Rights Reserved.
+ * Copyright (c) 2024 SUSE
+ */
+
+#include <linux/bitmap.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/msi.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+
+#include "irq-msi-lib.h"
+
+#define MIP_INT_RAISE		0x00
+#define MIP_INT_CLEAR		0x10
+#define MIP_INT_CFGL_HOST	0x20
+#define MIP_INT_CFGH_HOST	0x30
+#define MIP_INT_MASKL_HOST	0x40
+#define MIP_INT_MASKH_HOST	0x50
+#define MIP_INT_MASKL_VPU	0x60
+#define MIP_INT_MASKH_VPU	0x70
+#define MIP_INT_STATUSL_HOST	0x80
+#define MIP_INT_STATUSH_HOST	0x90
+#define MIP_INT_STATUSL_VPU	0xa0
+#define MIP_INT_STATUSH_VPU	0xb0
+
+/**
+ * struct mip_priv - MSI-X interrupt controller data
+ * @lock:	Used to protect bitmap alloc/free
+ * @base:	Base address of MMIO area
+ * @msg_addr:	PCIe MSI-X address
+ * @msi_base:	MSI base
+ * @num_msis:	Count of MSIs
+ * @msi_offset:	MSI offset
+ * @bitmap:	A bitmap for hwirqs
+ * @parent:	Parent domain (GIC)
+ * @dev:	A device pointer
+ */
+struct mip_priv {
+	spinlock_t		lock;
+	void __iomem		*base;
+	u64			msg_addr;
+	u32			msi_base;
+	u32			num_msis;
+	u32			msi_offset;
+	unsigned long		*bitmap;
+	struct irq_domain	*parent;
+	struct device		*dev;
+};
+
+static void mip_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
+{
+	struct mip_priv *mip = irq_data_get_irq_chip_data(d);
+
+	msg->address_hi = upper_32_bits(mip->msg_addr);
+	msg->address_lo = lower_32_bits(mip->msg_addr);
+	msg->data = d->hwirq;
+}
+
+static struct irq_chip mip_middle_irq_chip = {
+	.name			= "MIP",
+	.irq_mask		= irq_chip_mask_parent,
+	.irq_unmask		= irq_chip_unmask_parent,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+	.irq_set_type		= irq_chip_set_type_parent,
+	.irq_compose_msi_msg	= mip_compose_msi_msg,
+};
+
+static int mip_alloc_hwirq(struct mip_priv *mip, unsigned int nr_irqs)
+{
+	guard(spinlock)(&mip->lock);
+	return bitmap_find_free_region(mip->bitmap, mip->num_msis, ilog2(nr_irqs));
+}
+
+static void mip_free_hwirq(struct mip_priv *mip, unsigned int hwirq,
+			   unsigned int nr_irqs)
+{
+	guard(spinlock)(&mip->lock);
+	bitmap_release_region(mip->bitmap, hwirq, ilog2(nr_irqs));
+}
+
+static int mip_middle_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				   unsigned int nr_irqs, void *arg)
+{
+	struct mip_priv *mip = domain->host_data;
+	struct irq_fwspec fwspec = {0};
+	unsigned int hwirq, i;
+	struct irq_data *irqd;
+	int irq, ret;
+
+	irq = mip_alloc_hwirq(mip, nr_irqs);
+	if (irq < 0)
+		return irq;
+
+	hwirq = irq + mip->msi_offset;
+
+	fwspec.fwnode = domain->parent->fwnode;
+	fwspec.param_count = 3;
+	fwspec.param[0] = 0;
+	fwspec.param[1] = hwirq + mip->msi_base;
+	fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &fwspec);
+	if (ret)
+		goto err_free_hwirq;
+
+	for (i = 0; i < nr_irqs; i++) {
+		irqd = irq_domain_get_irq_data(domain->parent, virq + i);
+		irqd->chip->irq_set_type(irqd, IRQ_TYPE_EDGE_RISING);
+
+		ret = irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+						    &mip_middle_irq_chip, mip);
+		if (ret)
+			goto err_free;
+
+		irqd = irq_get_irq_data(virq + i);
+		irqd_set_single_target(irqd);
+		irqd_set_affinity_on_activate(irqd);
+	}
+
+	return 0;
+
+err_free:
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+err_free_hwirq:
+	mip_free_hwirq(mip, irq, nr_irqs);
+	return ret;
+}
+
+static void mip_middle_domain_free(struct irq_domain *domain, unsigned int virq,
+				   unsigned int nr_irqs)
+{
+	struct irq_data *irqd = irq_domain_get_irq_data(domain, virq);
+	struct mip_priv *mip;
+	unsigned int hwirq;
+
+	if (!irqd)
+		return;
+
+	mip = irq_data_get_irq_chip_data(irqd);
+	hwirq = irqd_to_hwirq(irqd);
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+	mip_free_hwirq(mip, hwirq - mip->msi_offset, nr_irqs);
+}
+
+static const struct irq_domain_ops mip_middle_domain_ops = {
+	.select		= msi_lib_irq_domain_select,
+	.alloc		= mip_middle_domain_alloc,
+	.free		= mip_middle_domain_free,
+};
+
+#define MIP_MSI_FLAGS_REQUIRED	(MSI_FLAG_USE_DEF_DOM_OPS |	\
+				 MSI_FLAG_USE_DEF_CHIP_OPS |	\
+				 MSI_FLAG_PCI_MSI_MASK_PARENT)
+
+#define MIP_MSI_FLAGS_SUPPORTED	(MSI_GENERIC_FLAGS_MASK |	\
+				 MSI_FLAG_MULTI_PCI_MSI |	\
+				 MSI_FLAG_PCI_MSIX)
+
+static const struct msi_parent_ops mip_msi_parent_ops = {
+	.supported_flags	= MIP_MSI_FLAGS_SUPPORTED,
+	.required_flags		= MIP_MSI_FLAGS_REQUIRED,
+	.bus_select_token       = DOMAIN_BUS_GENERIC_MSI,
+	.bus_select_mask	= MATCH_PCI_MSI,
+	.prefix			= "MIP-MSI-",
+	.init_dev_msi_info	= msi_lib_init_dev_msi_info,
+};
+
+static int mip_init_domains(struct mip_priv *mip, struct device_node *np)
+{
+	struct irq_domain *middle;
+
+	middle = irq_domain_add_hierarchy(mip->parent, 0, mip->num_msis, np,
+					  &mip_middle_domain_ops, mip);
+	if (!middle)
+		return -ENOMEM;
+
+	irq_domain_update_bus_token(middle, DOMAIN_BUS_GENERIC_MSI);
+	middle->dev = mip->dev;
+	middle->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
+	middle->msi_parent_ops = &mip_msi_parent_ops;
+
+	/*
+	 * All MSI-X unmasked for the host, masked for the VPU, and edge-triggered.
+	 */
+	writel(0, mip->base + MIP_INT_MASKL_HOST);
+	writel(0, mip->base + MIP_INT_MASKH_HOST);
+	writel(~0, mip->base + MIP_INT_MASKL_VPU);
+	writel(~0, mip->base + MIP_INT_MASKH_VPU);
+	writel(~0, mip->base + MIP_INT_CFGL_HOST);
+	writel(~0, mip->base + MIP_INT_CFGH_HOST);
+
+	return 0;
+}
+
+static int mip_parse_dt(struct mip_priv *mip, struct device_node *np)
+{
+	struct of_phandle_args args;
+	u64 size;
+	int ret;
+
+	ret = of_property_read_u32(np, "brcm,msi-offset", &mip->msi_offset);
+	if (ret)
+		mip->msi_offset = 0;
+
+	ret = of_parse_phandle_with_args(np, "msi-ranges", "#interrupt-cells",
+					 0, &args);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32_index(np, "msi-ranges", args.args_count + 1,
+					 &mip->num_msis);
+	if (ret)
+		goto err_put;
+
+	ret = of_property_read_reg(np, 1, &mip->msg_addr, &size);
+	if (ret)
+		goto err_put;
+
+	mip->msi_base = args.args[1];
+
+	mip->parent = irq_find_host(args.np);
+	if (!mip->parent)
+		ret = -EINVAL;
+
+err_put:
+	of_node_put(args.np);
+	return ret;
+}
+
+static int __init mip_of_msi_init(struct device_node *node, struct device_node *parent)
+{
+	struct platform_device *pdev;
+	struct mip_priv *mip;
+	int ret;
+
+	pdev = of_find_device_by_node(node);
+	of_node_put(node);
+	if (!pdev)
+		return -EPROBE_DEFER;
+
+	mip = kzalloc(sizeof(*mip), GFP_KERNEL);
+	if (!mip)
+		return -ENOMEM;
+
+	spin_lock_init(&mip->lock);
+	mip->dev = &pdev->dev;
+
+	ret = mip_parse_dt(mip, node);
+	if (ret)
+		goto err_priv;
+
+	mip->base = of_iomap(node, 0);
+	if (!mip->base) {
+		ret = -ENXIO;
+		goto err_priv;
+	}
+
+	mip->bitmap = bitmap_zalloc(mip->num_msis, GFP_KERNEL);
+	if (!mip->bitmap) {
+		ret = -ENOMEM;
+		goto err_base;
+	}
+
+	ret = mip_init_domains(mip, node);
+	if (ret)
+		goto err_map;
+
+	dev_dbg(&pdev->dev, "MIP: MSI-X count: %u, base: %u, offset: %u, msg_addr: %llx\n",
+		mip->num_msis, mip->msi_base, mip->msi_offset, mip->msg_addr);
+
+	return 0;
+
+err_map:
+	bitmap_free(mip->bitmap);
+err_base:
+	iounmap(mip->base);
+err_priv:
+	kfree(mip);
+	return ret;
+}
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(mip_msi)
+IRQCHIP_MATCH("brcm,bcm2712-mip", mip_of_msi_init)
+IRQCHIP_PLATFORM_DRIVER_END(mip_msi)
+MODULE_DESCRIPTION("Broadcom BCM2712 MSI-X interrupt controller");
+MODULE_AUTHOR("Phil Elwell <phil@raspberrypi.com>");
+MODULE_AUTHOR("Stanimir Varbanov <svarbanov@suse.de>");
+MODULE_LICENSE("GPL");
-- 
2.47.0

Re: [PATCH v5 -next 03/11] irqchip: Add Broadcom bcm2712 MSI-X interrupt controller

[Thomas Gleixner]

On Mon, Jan 20 2025 at 15:01, Stanimir Varbanov wrote:

> Add an interrupt controller driver for MSI-X Interrupt Peripheral (MIP)
> hardware block found in bcm2712. The interrupt controller is used to
> handle MSI-X interrupts from peripherials behind PCIe endpoints like
> RP1 south bridge found in RPi5.
>
> There are two MIPs on bcm2712, the first has 64 consecutive SPIs
> assigned to 64 output vectors, and the second has 17 SPIs, but only
> 8 of them are consecutive starting at the 8th output vector.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

As this is a new controller and required for the actual PCI muck, I
think the best way is to take it through the PCI tree, unless someone
wants me to pick the whole lot up.

Thanks,

        tglx

Re: [PATCH v5 -next 03/11] irqchip: Add Broadcom bcm2712 MSI-X interrupt controller

[Florian Fainelli]

On 1/27/25 10:10, Thomas Gleixner wrote:
> On Mon, Jan 20 2025 at 15:01, Stanimir Varbanov wrote:
> 
>> Add an interrupt controller driver for MSI-X Interrupt Peripheral (MIP)
>> hardware block found in bcm2712. The interrupt controller is used to
>> handle MSI-X interrupts from peripherials behind PCIe endpoints like
>> RP1 south bridge found in RPi5.
>>
>> There are two MIPs on bcm2712, the first has 64 consecutive SPIs
>> assigned to 64 output vectors, and the second has 17 SPIs, but only
>> 8 of them are consecutive starting at the 8th output vector.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> 
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> 
> As this is a new controller and required for the actual PCI muck, I
> think the best way is to take it through the PCI tree, unless someone
> wants me to pick the whole lot up.

Agreed, the PCI maintainers should take patches 1 through 9 inclusive, 
and I will take patches 10-11 through the Broadcom ARM SoC tree, Bjorn, 
KW, does that work?
-- 
Florian

Re: [PATCH v5 -next 03/11] irqchip: Add Broadcom bcm2712 MSI-X interrupt controller

[Stanimir Varbanov]

Hi,

On 1/28/25 7:55 PM, Florian Fainelli wrote:
> On 1/27/25 10:10, Thomas Gleixner wrote:
>> On Mon, Jan 20 2025 at 15:01, Stanimir Varbanov wrote:
>>
>>> Add an interrupt controller driver for MSI-X Interrupt Peripheral (MIP)
>>> hardware block found in bcm2712. The interrupt controller is used to
>>> handle MSI-X interrupts from peripherials behind PCIe endpoints like
>>> RP1 south bridge found in RPi5.
>>>
>>> There are two MIPs on bcm2712, the first has 64 consecutive SPIs
>>> assigned to 64 output vectors, and the second has 17 SPIs, but only
>>> 8 of them are consecutive starting at the 8th output vector.
>>>
>>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>>
>> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
>>
>> As this is a new controller and required for the actual PCI muck, I
>> think the best way is to take it through the PCI tree, unless someone
>> wants me to pick the whole lot up.
> 
> Agreed, the PCI maintainers should take patches 1 through 9 inclusive,

Just small correction, patch 09/11 [1] has a new v2 at [2]. And I think
PCI maintainer have to take v2.

> and I will take patches 10-11 through the Broadcom ARM SoC tree, Bjorn,
> KW, does that work?

~Stan

[1] [PATCH v5 -next 09/11] PCI: brcmstb: Fix for missing of_node_put

[2]
https://lore.kernel.org/lkml/20250122222955.1752778-1-svarbanov@suse.de/T/

Re: [PATCH v5 -next 03/11] irqchip: Add Broadcom bcm2712 MSI-X interrupt controller

[Krzysztof Wilczyński]

Hello,

[...]
> >> As this is a new controller and required for the actual PCI muck, I
> >> think the best way is to take it through the PCI tree, unless someone
> >> wants me to pick the whole lot up.
> > 
> > Agreed, the PCI maintainers should take patches 1 through 9 inclusive,
> 
> Just small correction, patch 09/11 [1] has a new v2 at [2]. And I think
> PCI maintainer have to take v2.

No problem.  I dropped #9 when applying.  Thank you!

	Krzysztof

Re: [PATCH v5 -next 03/11] irqchip: Add Broadcom bcm2712 MSI-X interrupt controller

[Krzysztof Wilczyński]

Hello,

[...]
> > As this is a new controller and required for the actual PCI muck, I
> > think the best way is to take it through the PCI tree, unless someone
> > wants me to pick the whole lot up.
> 
> Agreed, the PCI maintainers should take patches 1 through 9 inclusive, and I
> will take patches 10-11 through the Broadcom ARM SoC tree, Bjorn, KW, does
> that work?

No problem.

As such, when applying, I took all the patches from #1 through #8 and
dropped #9 as it has been superseded.

	Krzysztof

[PATCH v5 -next 04/11] PCI: brcmstb: Reuse config structure

[Stanimir Varbanov]

Instead of copying fields from pcie_cfg_data structure to
brcm_pcie reference it directly.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Reviewed-by: Florian Fainelil <florian.fainelli@broadcom.com>
---
v4 -> v5:
 - No changes.

 drivers/pci/controller/pcie-brcmstb.c | 70 ++++++++++++---------------
 1 file changed, 31 insertions(+), 39 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index e733a27dc8df..48b2747d8c98 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -191,11 +191,11 @@
 #define SSC_STATUS_PLL_LOCK_MASK	0x800
 #define PCIE_BRCM_MAX_MEMC		3
 
-#define IDX_ADDR(pcie)			((pcie)->reg_offsets[EXT_CFG_INDEX])
-#define DATA_ADDR(pcie)			((pcie)->reg_offsets[EXT_CFG_DATA])
-#define PCIE_RGR1_SW_INIT_1(pcie)	((pcie)->reg_offsets[RGR1_SW_INIT_1])
-#define HARD_DEBUG(pcie)		((pcie)->reg_offsets[PCIE_HARD_DEBUG])
-#define INTR2_CPU_BASE(pcie)		((pcie)->reg_offsets[PCIE_INTR2_CPU_BASE])
+#define IDX_ADDR(pcie)			((pcie)->cfg->offsets[EXT_CFG_INDEX])
+#define DATA_ADDR(pcie)			((pcie)->cfg->offsets[EXT_CFG_DATA])
+#define PCIE_RGR1_SW_INIT_1(pcie)	((pcie)->cfg->offsets[RGR1_SW_INIT_1])
+#define HARD_DEBUG(pcie)		((pcie)->cfg->offsets[PCIE_HARD_DEBUG])
+#define INTR2_CPU_BASE(pcie)		((pcie)->cfg->offsets[PCIE_INTR2_CPU_BASE])
 
 /* Rescal registers */
 #define PCIE_DVT_PMU_PCIE_PHY_CTRL				0xc700
@@ -276,8 +276,6 @@ struct brcm_pcie {
 	int			gen;
 	u64			msi_target_addr;
 	struct brcm_msi		*msi;
-	const int		*reg_offsets;
-	enum pcie_soc_base	soc_base;
 	struct reset_control	*rescal;
 	struct reset_control	*perst_reset;
 	struct reset_control	*bridge_reset;
@@ -285,17 +283,14 @@ struct brcm_pcie {
 	int			num_memc;
 	u64			memc_size[PCIE_BRCM_MAX_MEMC];
 	u32			hw_rev;
-	int			(*perst_set)(struct brcm_pcie *pcie, u32 val);
-	int			(*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
 	struct subdev_regulators *sr;
 	bool			ep_wakeup_capable;
-	bool			has_phy;
-	u8			num_inbound_wins;
+	const struct pcie_cfg_data	*cfg;
 };
 
 static inline bool is_bmips(const struct brcm_pcie *pcie)
 {
-	return pcie->soc_base == BCM7435 || pcie->soc_base == BCM7425;
+	return pcie->cfg->soc_base == BCM7435 || pcie->cfg->soc_base == BCM7425;
 }
 
 /*
@@ -855,7 +850,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
 	 * security considerations, and is not implemented in our modern
 	 * SoCs.
 	 */
-	if (pcie->soc_base != BCM7712)
+	if (pcie->cfg->soc_base != BCM7712)
 		add_inbound_win(b++, &n, 0, 0, 0);
 
 	resource_list_for_each_entry(entry, &bridge->dma_ranges) {
@@ -872,10 +867,10 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
 		 * That being said, each BARs size must still be a power of
 		 * two.
 		 */
-		if (pcie->soc_base == BCM7712)
+		if (pcie->cfg->soc_base == BCM7712)
 			add_inbound_win(b++, &n, size, cpu_start, pcie_start);
 
-		if (n > pcie->num_inbound_wins)
+		if (n > pcie->cfg->num_inbound_wins)
 			break;
 	}
 
@@ -889,7 +884,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
 	 * that enables multiple memory controllers.  As such, it can return
 	 * now w/o doing special configuration.
 	 */
-	if (pcie->soc_base == BCM7712)
+	if (pcie->cfg->soc_base == BCM7712)
 		return n;
 
 	ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
@@ -1012,7 +1007,7 @@ static void set_inbound_win_registers(struct brcm_pcie *pcie,
 		 * 7712:
 		 *     All of their BARs need to be set.
 		 */
-		if (pcie->soc_base == BCM7712) {
+		if (pcie->cfg->soc_base == BCM7712) {
 			/* BUS remap register settings */
 			reg_offset = brcm_ubus_reg_offset(i);
 			tmp = lower_32_bits(cpu_addr) & ~0xfff;
@@ -1036,15 +1031,15 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	int memc, ret;
 
 	/* Reset the bridge */
-	ret = pcie->bridge_sw_init_set(pcie, 1);
+	ret = pcie->cfg->bridge_sw_init_set(pcie, 1);
 	if (ret)
 		return ret;
 
 	/* Ensure that PERST# is asserted; some bootloaders may deassert it. */
-	if (pcie->soc_base == BCM2711) {
-		ret = pcie->perst_set(pcie, 1);
+	if (pcie->cfg->soc_base == BCM2711) {
+		ret = pcie->cfg->perst_set(pcie, 1);
 		if (ret) {
-			pcie->bridge_sw_init_set(pcie, 0);
+			pcie->cfg->bridge_sw_init_set(pcie, 0);
 			return ret;
 		}
 	}
@@ -1052,7 +1047,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	usleep_range(100, 200);
 
 	/* Take the bridge out of reset */
-	ret = pcie->bridge_sw_init_set(pcie, 0);
+	ret = pcie->cfg->bridge_sw_init_set(pcie, 0);
 	if (ret)
 		return ret;
 
@@ -1072,9 +1067,9 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 	 */
 	if (is_bmips(pcie))
 		burst = 0x1; /* 256 bytes */
-	else if (pcie->soc_base == BCM2711)
+	else if (pcie->cfg->soc_base == BCM2711)
 		burst = 0x0; /* 128 bytes */
-	else if (pcie->soc_base == BCM7278)
+	else if (pcie->cfg->soc_base == BCM7278)
 		burst = 0x3; /* 512 bytes */
 	else
 		burst = 0x2; /* 512 bytes */
@@ -1199,7 +1194,7 @@ static void brcm_extend_rbus_timeout(struct brcm_pcie *pcie)
 	u32 timeout_us = 4000000; /* 4 seconds, our setting for L1SS */
 
 	/* 7712 does not have this (RGR1) timer */
-	if (pcie->soc_base == BCM7712)
+	if (pcie->cfg->soc_base == BCM7712)
 		return;
 
 	/* Each unit in timeout register is 1/216,000,000 seconds */
@@ -1277,7 +1272,7 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
 	int ret, i;
 
 	/* Unassert the fundamental reset */
-	ret = pcie->perst_set(pcie, 0);
+	ret = pcie->cfg->perst_set(pcie, 0);
 	if (ret)
 		return ret;
 
@@ -1463,12 +1458,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->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
+	return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
 }
 
 static inline int brcm_phy_stop(struct brcm_pcie *pcie)
 {
-	return pcie->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
+	return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
 }
 
 static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
@@ -1479,7 +1474,7 @@ static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
 	if (brcm_pcie_link_up(pcie))
 		brcm_pcie_enter_l23(pcie);
 	/* Assert fundamental reset */
-	ret = pcie->perst_set(pcie, 1);
+	ret = pcie->cfg->perst_set(pcie, 1);
 	if (ret)
 		return ret;
 
@@ -1582,7 +1577,7 @@ static int brcm_pcie_resume_noirq(struct device *dev)
 		goto err_reset;
 
 	/* Take bridge out of reset so we can access the SERDES reg */
-	pcie->bridge_sw_init_set(pcie, 0);
+	pcie->cfg->bridge_sw_init_set(pcie, 0);
 
 	/* SERDES_IDDQ = 0 */
 	tmp = readl(base + HARD_DEBUG(pcie));
@@ -1803,12 +1798,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 	pcie = pci_host_bridge_priv(bridge);
 	pcie->dev = &pdev->dev;
 	pcie->np = np;
-	pcie->reg_offsets = data->offsets;
-	pcie->soc_base = data->soc_base;
-	pcie->perst_set = data->perst_set;
-	pcie->bridge_sw_init_set = data->bridge_sw_init_set;
-	pcie->has_phy = data->has_phy;
-	pcie->num_inbound_wins = data->num_inbound_wins;
+	pcie->cfg = data;
 
 	pcie->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pcie->base))
@@ -1843,7 +1833,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "could not enable clock\n");
 
-	pcie->bridge_sw_init_set(pcie, 0);
+	pcie->cfg->bridge_sw_init_set(pcie, 0);
 
 	if (pcie->swinit_reset) {
 		ret = reset_control_assert(pcie->swinit_reset);
@@ -1882,7 +1872,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		goto fail;
 
 	pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
-	if (pcie->soc_base == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
+	if (pcie->cfg->soc_base == 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;
@@ -1897,7 +1888,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		}
 	}
 
-	bridge->ops = pcie->soc_base == BCM7425 ? &brcm7425_pcie_ops : &brcm_pcie_ops;
+	bridge->ops = pcie->cfg->soc_base == BCM7425 ?
+				&brcm7425_pcie_ops : &brcm_pcie_ops;
 	bridge->sysdata = pcie;
 
 	platform_set_drvdata(pdev, pcie);
-- 
2.47.0

Re: [PATCH v5 -next 04/11] PCI: brcmstb: Reuse config structure

[Jim Quinlan]

On Mon, Jan 20, 2025 at 8:01 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> Instead of copying fields from pcie_cfg_data structure to
> brcm_pcie reference it directly.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Reviewed-by: Florian Fainelil <florian.fainelli@broadcom.com>
> ---
> v4 -> v5:
>  - No changes.
>
>  drivers/pci/controller/pcie-brcmstb.c | 70 ++++++++++++---------------
>  1 file changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index e733a27dc8df..48b2747d8c98 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -191,11 +191,11 @@
>  #define SSC_STATUS_PLL_LOCK_MASK       0x800
>  #define PCIE_BRCM_MAX_MEMC             3
>
> -#define IDX_ADDR(pcie)                 ((pcie)->reg_offsets[EXT_CFG_INDEX])
> -#define DATA_ADDR(pcie)                        ((pcie)->reg_offsets[EXT_CFG_DATA])
> -#define PCIE_RGR1_SW_INIT_1(pcie)      ((pcie)->reg_offsets[RGR1_SW_INIT_1])
> -#define HARD_DEBUG(pcie)               ((pcie)->reg_offsets[PCIE_HARD_DEBUG])
> -#define INTR2_CPU_BASE(pcie)           ((pcie)->reg_offsets[PCIE_INTR2_CPU_BASE])
> +#define IDX_ADDR(pcie)                 ((pcie)->cfg->offsets[EXT_CFG_INDEX])
> +#define DATA_ADDR(pcie)                        ((pcie)->cfg->offsets[EXT_CFG_DATA])
> +#define PCIE_RGR1_SW_INIT_1(pcie)      ((pcie)->cfg->offsets[RGR1_SW_INIT_1])
> +#define HARD_DEBUG(pcie)               ((pcie)->cfg->offsets[PCIE_HARD_DEBUG])
> +#define INTR2_CPU_BASE(pcie)           ((pcie)->cfg->offsets[PCIE_INTR2_CPU_BASE])
>
>  /* Rescal registers */
>  #define PCIE_DVT_PMU_PCIE_PHY_CTRL                             0xc700
> @@ -276,8 +276,6 @@ struct brcm_pcie {
>         int                     gen;
>         u64                     msi_target_addr;
>         struct brcm_msi         *msi;
> -       const int               *reg_offsets;
> -       enum pcie_soc_base      soc_base;
>         struct reset_control    *rescal;
>         struct reset_control    *perst_reset;
>         struct reset_control    *bridge_reset;
> @@ -285,17 +283,14 @@ struct brcm_pcie {
>         int                     num_memc;
>         u64                     memc_size[PCIE_BRCM_MAX_MEMC];
>         u32                     hw_rev;
> -       int                     (*perst_set)(struct brcm_pcie *pcie, u32 val);
> -       int                     (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
>         struct subdev_regulators *sr;
>         bool                    ep_wakeup_capable;
> -       bool                    has_phy;
> -       u8                      num_inbound_wins;
> +       const struct pcie_cfg_data      *cfg;
>  };
>
>  static inline bool is_bmips(const struct brcm_pcie *pcie)
>  {
> -       return pcie->soc_base == BCM7435 || pcie->soc_base == BCM7425;
> +       return pcie->cfg->soc_base == BCM7435 || pcie->cfg->soc_base == BCM7425;
>  }
>
>  /*
> @@ -855,7 +850,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
>          * security considerations, and is not implemented in our modern
>          * SoCs.
>          */
> -       if (pcie->soc_base != BCM7712)
> +       if (pcie->cfg->soc_base != BCM7712)
>                 add_inbound_win(b++, &n, 0, 0, 0);
>
>         resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> @@ -872,10 +867,10 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
>                  * That being said, each BARs size must still be a power of
>                  * two.
>                  */
> -               if (pcie->soc_base == BCM7712)
> +               if (pcie->cfg->soc_base == BCM7712)
>                         add_inbound_win(b++, &n, size, cpu_start, pcie_start);
>
> -               if (n > pcie->num_inbound_wins)
> +               if (n > pcie->cfg->num_inbound_wins)
>                         break;
>         }
>
> @@ -889,7 +884,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
>          * that enables multiple memory controllers.  As such, it can return
>          * now w/o doing special configuration.
>          */
> -       if (pcie->soc_base == BCM7712)
> +       if (pcie->cfg->soc_base == BCM7712)
>                 return n;
>
>         ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
> @@ -1012,7 +1007,7 @@ static void set_inbound_win_registers(struct brcm_pcie *pcie,
>                  * 7712:
>                  *     All of their BARs need to be set.
>                  */
> -               if (pcie->soc_base == BCM7712) {
> +               if (pcie->cfg->soc_base == BCM7712) {
>                         /* BUS remap register settings */
>                         reg_offset = brcm_ubus_reg_offset(i);
>                         tmp = lower_32_bits(cpu_addr) & ~0xfff;
> @@ -1036,15 +1031,15 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>         int memc, ret;
>
>         /* Reset the bridge */
> -       ret = pcie->bridge_sw_init_set(pcie, 1);
> +       ret = pcie->cfg->bridge_sw_init_set(pcie, 1);
>         if (ret)
>                 return ret;
>
>         /* Ensure that PERST# is asserted; some bootloaders may deassert it. */
> -       if (pcie->soc_base == BCM2711) {
> -               ret = pcie->perst_set(pcie, 1);
> +       if (pcie->cfg->soc_base == BCM2711) {
> +               ret = pcie->cfg->perst_set(pcie, 1);
>                 if (ret) {
> -                       pcie->bridge_sw_init_set(pcie, 0);
> +                       pcie->cfg->bridge_sw_init_set(pcie, 0);
>                         return ret;
>                 }
>         }
> @@ -1052,7 +1047,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>         usleep_range(100, 200);
>
>         /* Take the bridge out of reset */
> -       ret = pcie->bridge_sw_init_set(pcie, 0);
> +       ret = pcie->cfg->bridge_sw_init_set(pcie, 0);
>         if (ret)
>                 return ret;
>
> @@ -1072,9 +1067,9 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>          */
>         if (is_bmips(pcie))
>                 burst = 0x1; /* 256 bytes */
> -       else if (pcie->soc_base == BCM2711)
> +       else if (pcie->cfg->soc_base == BCM2711)
>                 burst = 0x0; /* 128 bytes */
> -       else if (pcie->soc_base == BCM7278)
> +       else if (pcie->cfg->soc_base == BCM7278)
>                 burst = 0x3; /* 512 bytes */
>         else
>                 burst = 0x2; /* 512 bytes */
> @@ -1199,7 +1194,7 @@ static void brcm_extend_rbus_timeout(struct brcm_pcie *pcie)
>         u32 timeout_us = 4000000; /* 4 seconds, our setting for L1SS */
>
>         /* 7712 does not have this (RGR1) timer */
> -       if (pcie->soc_base == BCM7712)
> +       if (pcie->cfg->soc_base == BCM7712)
>                 return;
>
>         /* Each unit in timeout register is 1/216,000,000 seconds */
> @@ -1277,7 +1272,7 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
>         int ret, i;
>
>         /* Unassert the fundamental reset */
> -       ret = pcie->perst_set(pcie, 0);
> +       ret = pcie->cfg->perst_set(pcie, 0);
>         if (ret)
>                 return ret;
>
> @@ -1463,12 +1458,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->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
> +       return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
>  }
>
>  static inline int brcm_phy_stop(struct brcm_pcie *pcie)
>  {
> -       return pcie->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
> +       return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
>  }
>
>  static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
> @@ -1479,7 +1474,7 @@ static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
>         if (brcm_pcie_link_up(pcie))
>                 brcm_pcie_enter_l23(pcie);
>         /* Assert fundamental reset */
> -       ret = pcie->perst_set(pcie, 1);
> +       ret = pcie->cfg->perst_set(pcie, 1);
>         if (ret)
>                 return ret;
>
> @@ -1582,7 +1577,7 @@ static int brcm_pcie_resume_noirq(struct device *dev)
>                 goto err_reset;
>
>         /* Take bridge out of reset so we can access the SERDES reg */
> -       pcie->bridge_sw_init_set(pcie, 0);
> +       pcie->cfg->bridge_sw_init_set(pcie, 0);
>
>         /* SERDES_IDDQ = 0 */
>         tmp = readl(base + HARD_DEBUG(pcie));
> @@ -1803,12 +1798,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>         pcie = pci_host_bridge_priv(bridge);
>         pcie->dev = &pdev->dev;
>         pcie->np = np;
> -       pcie->reg_offsets = data->offsets;
> -       pcie->soc_base = data->soc_base;
> -       pcie->perst_set = data->perst_set;
> -       pcie->bridge_sw_init_set = data->bridge_sw_init_set;
> -       pcie->has_phy = data->has_phy;
> -       pcie->num_inbound_wins = data->num_inbound_wins;
> +       pcie->cfg = data;
>
>         pcie->base = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(pcie->base))
> @@ -1843,7 +1833,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>         if (ret)
>                 return dev_err_probe(&pdev->dev, ret, "could not enable clock\n");
>
> -       pcie->bridge_sw_init_set(pcie, 0);
> +       pcie->cfg->bridge_sw_init_set(pcie, 0);
>
>         if (pcie->swinit_reset) {
>                 ret = reset_control_assert(pcie->swinit_reset);
> @@ -1882,7 +1872,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>                 goto fail;
>
>         pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
> -       if (pcie->soc_base == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
> +       if (pcie->cfg->soc_base == 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;
> @@ -1897,7 +1888,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>                 }
>         }
>
> -       bridge->ops = pcie->soc_base == BCM7425 ? &brcm7425_pcie_ops : &brcm_pcie_ops;
> +       bridge->ops = pcie->cfg->soc_base == BCM7425 ?
> +                               &brcm7425_pcie_ops : &brcm_pcie_ops;
>         bridge->sysdata = pcie;
>
>         platform_set_drvdata(pdev, pcie);

Reviewed-by: Jim Quinlan <james.quinlan@broadcom.com>
> --
> 2.47.0
>

Re: [PATCH v5 -next 04/11] PCI: brcmstb: Reuse config structure

[Jim Quinlan]

On Fri, Jan 31, 2025 at 11:10 AM Jim Quinlan <jim2101024@gmail.com> wrote:
>
> On Mon, Jan 20, 2025 at 8:01 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
> >
> > Instead of copying fields from pcie_cfg_data structure to
> > brcm_pcie reference it directly.
> >
> > Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> > Reviewed-by: Florian Fainelil <florian.fainelli@broadcom.com>
> > ---
> > v4 -> v5:
> >  - No changes.
> >
> >  drivers/pci/controller/pcie-brcmstb.c | 70 ++++++++++++---------------
> >  1 file changed, 31 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > index e733a27dc8df..48b2747d8c98 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -191,11 +191,11 @@
> >  #define SSC_STATUS_PLL_LOCK_MASK       0x800
> >  #define PCIE_BRCM_MAX_MEMC             3
> >
> > -#define IDX_ADDR(pcie)                 ((pcie)->reg_offsets[EXT_CFG_INDEX])
> > -#define DATA_ADDR(pcie)                        ((pcie)->reg_offsets[EXT_CFG_DATA])
> > -#define PCIE_RGR1_SW_INIT_1(pcie)      ((pcie)->reg_offsets[RGR1_SW_INIT_1])
> > -#define HARD_DEBUG(pcie)               ((pcie)->reg_offsets[PCIE_HARD_DEBUG])
> > -#define INTR2_CPU_BASE(pcie)           ((pcie)->reg_offsets[PCIE_INTR2_CPU_BASE])
> > +#define IDX_ADDR(pcie)                 ((pcie)->cfg->offsets[EXT_CFG_INDEX])
> > +#define DATA_ADDR(pcie)                        ((pcie)->cfg->offsets[EXT_CFG_DATA])
> > +#define PCIE_RGR1_SW_INIT_1(pcie)      ((pcie)->cfg->offsets[RGR1_SW_INIT_1])
> > +#define HARD_DEBUG(pcie)               ((pcie)->cfg->offsets[PCIE_HARD_DEBUG])
> > +#define INTR2_CPU_BASE(pcie)           ((pcie)->cfg->offsets[PCIE_INTR2_CPU_BASE])
> >
> >  /* Rescal registers */
> >  #define PCIE_DVT_PMU_PCIE_PHY_CTRL                             0xc700
> > @@ -276,8 +276,6 @@ struct brcm_pcie {
> >         int                     gen;
> >         u64                     msi_target_addr;
> >         struct brcm_msi         *msi;
> > -       const int               *reg_offsets;
> > -       enum pcie_soc_base      soc_base;
> >         struct reset_control    *rescal;
> >         struct reset_control    *perst_reset;
> >         struct reset_control    *bridge_reset;
> > @@ -285,17 +283,14 @@ struct brcm_pcie {
> >         int                     num_memc;
> >         u64                     memc_size[PCIE_BRCM_MAX_MEMC];
> >         u32                     hw_rev;
> > -       int                     (*perst_set)(struct brcm_pcie *pcie, u32 val);
> > -       int                     (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
> >         struct subdev_regulators *sr;
> >         bool                    ep_wakeup_capable;
> > -       bool                    has_phy;
> > -       u8                      num_inbound_wins;
> > +       const struct pcie_cfg_data      *cfg;
> >  };
> >
> >  static inline bool is_bmips(const struct brcm_pcie *pcie)
> >  {
> > -       return pcie->soc_base == BCM7435 || pcie->soc_base == BCM7425;
> > +       return pcie->cfg->soc_base == BCM7435 || pcie->cfg->soc_base == BCM7425;
> >  }
> >
> >  /*
> > @@ -855,7 +850,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
> >          * security considerations, and is not implemented in our modern
> >          * SoCs.
> >          */
> > -       if (pcie->soc_base != BCM7712)
> > +       if (pcie->cfg->soc_base != BCM7712)
> >                 add_inbound_win(b++, &n, 0, 0, 0);
> >
> >         resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> > @@ -872,10 +867,10 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
> >                  * That being said, each BARs size must still be a power of
> >                  * two.
> >                  */
> > -               if (pcie->soc_base == BCM7712)
> > +               if (pcie->cfg->soc_base == BCM7712)
> >                         add_inbound_win(b++, &n, size, cpu_start, pcie_start);
> >
> > -               if (n > pcie->num_inbound_wins)
> > +               if (n > pcie->cfg->num_inbound_wins)
> >                         break;
> >         }
> >
> > @@ -889,7 +884,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
> >          * that enables multiple memory controllers.  As such, it can return
> >          * now w/o doing special configuration.
> >          */
> > -       if (pcie->soc_base == BCM7712)
> > +       if (pcie->cfg->soc_base == BCM7712)
> >                 return n;
> >
> >         ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
> > @@ -1012,7 +1007,7 @@ static void set_inbound_win_registers(struct brcm_pcie *pcie,
> >                  * 7712:
> >                  *     All of their BARs need to be set.
> >                  */
> > -               if (pcie->soc_base == BCM7712) {
> > +               if (pcie->cfg->soc_base == BCM7712) {
> >                         /* BUS remap register settings */
> >                         reg_offset = brcm_ubus_reg_offset(i);
> >                         tmp = lower_32_bits(cpu_addr) & ~0xfff;
> > @@ -1036,15 +1031,15 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> >         int memc, ret;
> >
> >         /* Reset the bridge */
> > -       ret = pcie->bridge_sw_init_set(pcie, 1);
> > +       ret = pcie->cfg->bridge_sw_init_set(pcie, 1);
> >         if (ret)
> >                 return ret;
> >
> >         /* Ensure that PERST# is asserted; some bootloaders may deassert it. */
> > -       if (pcie->soc_base == BCM2711) {
> > -               ret = pcie->perst_set(pcie, 1);
> > +       if (pcie->cfg->soc_base == BCM2711) {
> > +               ret = pcie->cfg->perst_set(pcie, 1);
> >                 if (ret) {
> > -                       pcie->bridge_sw_init_set(pcie, 0);
> > +                       pcie->cfg->bridge_sw_init_set(pcie, 0);
> >                         return ret;
> >                 }
> >         }
> > @@ -1052,7 +1047,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> >         usleep_range(100, 200);
> >
> >         /* Take the bridge out of reset */
> > -       ret = pcie->bridge_sw_init_set(pcie, 0);
> > +       ret = pcie->cfg->bridge_sw_init_set(pcie, 0);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1072,9 +1067,9 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> >          */
> >         if (is_bmips(pcie))
> >                 burst = 0x1; /* 256 bytes */
> > -       else if (pcie->soc_base == BCM2711)
> > +       else if (pcie->cfg->soc_base == BCM2711)
> >                 burst = 0x0; /* 128 bytes */
> > -       else if (pcie->soc_base == BCM7278)
> > +       else if (pcie->cfg->soc_base == BCM7278)
> >                 burst = 0x3; /* 512 bytes */
> >         else
> >                 burst = 0x2; /* 512 bytes */
> > @@ -1199,7 +1194,7 @@ static void brcm_extend_rbus_timeout(struct brcm_pcie *pcie)
> >         u32 timeout_us = 4000000; /* 4 seconds, our setting for L1SS */
> >
> >         /* 7712 does not have this (RGR1) timer */
> > -       if (pcie->soc_base == BCM7712)
> > +       if (pcie->cfg->soc_base == BCM7712)
> >                 return;
> >
> >         /* Each unit in timeout register is 1/216,000,000 seconds */
> > @@ -1277,7 +1272,7 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
> >         int ret, i;
> >
> >         /* Unassert the fundamental reset */
> > -       ret = pcie->perst_set(pcie, 0);
> > +       ret = pcie->cfg->perst_set(pcie, 0);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1463,12 +1458,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->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
> > +       return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
> >  }
> >
> >  static inline int brcm_phy_stop(struct brcm_pcie *pcie)
> >  {
> > -       return pcie->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
> > +       return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
> >  }
> >
> >  static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
> > @@ -1479,7 +1474,7 @@ static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
> >         if (brcm_pcie_link_up(pcie))
> >                 brcm_pcie_enter_l23(pcie);
> >         /* Assert fundamental reset */
> > -       ret = pcie->perst_set(pcie, 1);
> > +       ret = pcie->cfg->perst_set(pcie, 1);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1582,7 +1577,7 @@ static int brcm_pcie_resume_noirq(struct device *dev)
> >                 goto err_reset;
> >
> >         /* Take bridge out of reset so we can access the SERDES reg */
> > -       pcie->bridge_sw_init_set(pcie, 0);
> > +       pcie->cfg->bridge_sw_init_set(pcie, 0);
> >
> >         /* SERDES_IDDQ = 0 */
> >         tmp = readl(base + HARD_DEBUG(pcie));
> > @@ -1803,12 +1798,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> >         pcie = pci_host_bridge_priv(bridge);
> >         pcie->dev = &pdev->dev;
> >         pcie->np = np;
> > -       pcie->reg_offsets = data->offsets;
> > -       pcie->soc_base = data->soc_base;
> > -       pcie->perst_set = data->perst_set;
> > -       pcie->bridge_sw_init_set = data->bridge_sw_init_set;
> > -       pcie->has_phy = data->has_phy;
> > -       pcie->num_inbound_wins = data->num_inbound_wins;
> > +       pcie->cfg = data;
> >
> >         pcie->base = devm_platform_ioremap_resource(pdev, 0);
> >         if (IS_ERR(pcie->base))
> > @@ -1843,7 +1833,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> >         if (ret)
> >                 return dev_err_probe(&pdev->dev, ret, "could not enable clock\n");
> >
> > -       pcie->bridge_sw_init_set(pcie, 0);
> > +       pcie->cfg->bridge_sw_init_set(pcie, 0);
> >
> >         if (pcie->swinit_reset) {
> >                 ret = reset_control_assert(pcie->swinit_reset);
> > @@ -1882,7 +1872,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> >                 goto fail;
> >
> >         pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
> > -       if (pcie->soc_base == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
> > +       if (pcie->cfg->soc_base == 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;
> > @@ -1897,7 +1888,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> >                 }
> >         }
> >
> > -       bridge->ops = pcie->soc_base == BCM7425 ? &brcm7425_pcie_ops : &brcm_pcie_ops;
> > +       bridge->ops = pcie->cfg->soc_base == BCM7425 ?
> > +                               &brcm7425_pcie_ops : &brcm_pcie_ops;
> >         bridge->sysdata = pcie;
> >
> >         platform_set_drvdata(pdev, pcie);
>
> Reviewed-by: Jim Quinlan <james.quinlan@broadcom.com>

Hi Stan,

Sorry for the late notice but I get a compilation error on this commit:

drivers/pci/controller/pcie-brcmstb.c: In function 'brcm_pcie_turn_off':
drivers/pci/controller/pcie-brcmstb.c:1492:14: error: 'struct
brcm_pcie' has no member named 'bridge_sw_init_set'; did you mean
'bridge_reset'?
  ret = pcie->bridge_sw_init_set(pcie, 1);
              ^~~~~~~~~~~~~~~~~~
              bridge_reset
make[5]: *** [scripts/Makefile.build:194:
drivers/pci/controller/pcie-brcmstb.o] Error 1

It appears to be fixed with the subsequent commit "PCI: brcmstb: Add
bcm2712 support".

Can you please look into this and see if you get the same results?

Regards,
Jim Quinlan
Broadcom STB/CM


> > --
> > 2.47.0
> >

Re: [PATCH v5 -next 04/11] PCI: brcmstb: Reuse config structure

[Krzysztof Wilczyński]

Hello,

> > > Instead of copying fields from pcie_cfg_data structure to
> > > brcm_pcie reference it directly.
[...]
> Sorry for the late notice but I get a compilation error on this commit:
> 
> drivers/pci/controller/pcie-brcmstb.c: In function 'brcm_pcie_turn_off':
> drivers/pci/controller/pcie-brcmstb.c:1492:14: error: 'struct
> brcm_pcie' has no member named 'bridge_sw_init_set'; did you mean
> 'bridge_reset'?
>   ret = pcie->bridge_sw_init_set(pcie, 1);
>               ^~~~~~~~~~~~~~~~~~
>               bridge_reset
> make[5]: *** [scripts/Makefile.build:194:
> drivers/pci/controller/pcie-brcmstb.o] Error 1
> 
> It appears to be fixed with the subsequent commit "PCI: brcmstb: Add
> bcm2712 support".
> 
> Can you please look into this and see if you get the same results?

I applied this series to controller/brcmstb recently. As such, we can fix
the issue directly on the branch, there will be no need to send a new
version.

That said, I can wait for Stanimir to see about a fix or look into this
myself if needed.

The 0-day bot will pick this now, and I expect it to have the same build
issue.

	Krzysztof

Re: [PATCH v5 -next 04/11] PCI: brcmstb: Reuse config structure

[Stanimir Varbanov]

Hi Jim,

On 2/21/25 5:36 PM, Jim Quinlan wrote:
> On Fri, Jan 31, 2025 at 11:10 AM Jim Quinlan <jim2101024@gmail.com> wrote:
>>
>> On Mon, Jan 20, 2025 at 8:01 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>>>
>>> Instead of copying fields from pcie_cfg_data structure to
>>> brcm_pcie reference it directly.
>>>
>>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>>> Reviewed-by: Florian Fainelil <florian.fainelli@broadcom.com>
>>> ---
>>> v4 -> v5:
>>>  - No changes.
>>>
>>>  drivers/pci/controller/pcie-brcmstb.c | 70 ++++++++++++---------------
>>>  1 file changed, 31 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
>>> index e733a27dc8df..48b2747d8c98 100644
>>> --- a/drivers/pci/controller/pcie-brcmstb.c
>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>>> @@ -191,11 +191,11 @@
>>>  #define SSC_STATUS_PLL_LOCK_MASK       0x800
>>>  #define PCIE_BRCM_MAX_MEMC             3
>>>
>>> -#define IDX_ADDR(pcie)                 ((pcie)->reg_offsets[EXT_CFG_INDEX])
>>> -#define DATA_ADDR(pcie)                        ((pcie)->reg_offsets[EXT_CFG_DATA])
>>> -#define PCIE_RGR1_SW_INIT_1(pcie)      ((pcie)->reg_offsets[RGR1_SW_INIT_1])
>>> -#define HARD_DEBUG(pcie)               ((pcie)->reg_offsets[PCIE_HARD_DEBUG])
>>> -#define INTR2_CPU_BASE(pcie)           ((pcie)->reg_offsets[PCIE_INTR2_CPU_BASE])
>>> +#define IDX_ADDR(pcie)                 ((pcie)->cfg->offsets[EXT_CFG_INDEX])
>>> +#define DATA_ADDR(pcie)                        ((pcie)->cfg->offsets[EXT_CFG_DATA])
>>> +#define PCIE_RGR1_SW_INIT_1(pcie)      ((pcie)->cfg->offsets[RGR1_SW_INIT_1])
>>> +#define HARD_DEBUG(pcie)               ((pcie)->cfg->offsets[PCIE_HARD_DEBUG])
>>> +#define INTR2_CPU_BASE(pcie)           ((pcie)->cfg->offsets[PCIE_INTR2_CPU_BASE])
>>>
>>>  /* Rescal registers */
>>>  #define PCIE_DVT_PMU_PCIE_PHY_CTRL                             0xc700
>>> @@ -276,8 +276,6 @@ struct brcm_pcie {
>>>         int                     gen;
>>>         u64                     msi_target_addr;
>>>         struct brcm_msi         *msi;
>>> -       const int               *reg_offsets;
>>> -       enum pcie_soc_base      soc_base;
>>>         struct reset_control    *rescal;
>>>         struct reset_control    *perst_reset;
>>>         struct reset_control    *bridge_reset;
>>> @@ -285,17 +283,14 @@ struct brcm_pcie {
>>>         int                     num_memc;
>>>         u64                     memc_size[PCIE_BRCM_MAX_MEMC];
>>>         u32                     hw_rev;
>>> -       int                     (*perst_set)(struct brcm_pcie *pcie, u32 val);
>>> -       int                     (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
>>>         struct subdev_regulators *sr;
>>>         bool                    ep_wakeup_capable;
>>> -       bool                    has_phy;
>>> -       u8                      num_inbound_wins;
>>> +       const struct pcie_cfg_data      *cfg;
>>>  };
>>>
>>>  static inline bool is_bmips(const struct brcm_pcie *pcie)
>>>  {
>>> -       return pcie->soc_base == BCM7435 || pcie->soc_base == BCM7425;
>>> +       return pcie->cfg->soc_base == BCM7435 || pcie->cfg->soc_base == BCM7425;
>>>  }
>>>
>>>  /*
>>> @@ -855,7 +850,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
>>>          * security considerations, and is not implemented in our modern
>>>          * SoCs.
>>>          */
>>> -       if (pcie->soc_base != BCM7712)
>>> +       if (pcie->cfg->soc_base != BCM7712)
>>>                 add_inbound_win(b++, &n, 0, 0, 0);
>>>
>>>         resource_list_for_each_entry(entry, &bridge->dma_ranges) {
>>> @@ -872,10 +867,10 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
>>>                  * That being said, each BARs size must still be a power of
>>>                  * two.
>>>                  */
>>> -               if (pcie->soc_base == BCM7712)
>>> +               if (pcie->cfg->soc_base == BCM7712)
>>>                         add_inbound_win(b++, &n, size, cpu_start, pcie_start);
>>>
>>> -               if (n > pcie->num_inbound_wins)
>>> +               if (n > pcie->cfg->num_inbound_wins)
>>>                         break;
>>>         }
>>>
>>> @@ -889,7 +884,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
>>>          * that enables multiple memory controllers.  As such, it can return
>>>          * now w/o doing special configuration.
>>>          */
>>> -       if (pcie->soc_base == BCM7712)
>>> +       if (pcie->cfg->soc_base == BCM7712)
>>>                 return n;
>>>
>>>         ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
>>> @@ -1012,7 +1007,7 @@ static void set_inbound_win_registers(struct brcm_pcie *pcie,
>>>                  * 7712:
>>>                  *     All of their BARs need to be set.
>>>                  */
>>> -               if (pcie->soc_base == BCM7712) {
>>> +               if (pcie->cfg->soc_base == BCM7712) {
>>>                         /* BUS remap register settings */
>>>                         reg_offset = brcm_ubus_reg_offset(i);
>>>                         tmp = lower_32_bits(cpu_addr) & ~0xfff;
>>> @@ -1036,15 +1031,15 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>>>         int memc, ret;
>>>
>>>         /* Reset the bridge */
>>> -       ret = pcie->bridge_sw_init_set(pcie, 1);
>>> +       ret = pcie->cfg->bridge_sw_init_set(pcie, 1);
>>>         if (ret)
>>>                 return ret;
>>>
>>>         /* Ensure that PERST# is asserted; some bootloaders may deassert it. */
>>> -       if (pcie->soc_base == BCM2711) {
>>> -               ret = pcie->perst_set(pcie, 1);
>>> +       if (pcie->cfg->soc_base == BCM2711) {
>>> +               ret = pcie->cfg->perst_set(pcie, 1);
>>>                 if (ret) {
>>> -                       pcie->bridge_sw_init_set(pcie, 0);
>>> +                       pcie->cfg->bridge_sw_init_set(pcie, 0);
>>>                         return ret;
>>>                 }
>>>         }
>>> @@ -1052,7 +1047,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>>>         usleep_range(100, 200);
>>>
>>>         /* Take the bridge out of reset */
>>> -       ret = pcie->bridge_sw_init_set(pcie, 0);
>>> +       ret = pcie->cfg->bridge_sw_init_set(pcie, 0);
>>>         if (ret)
>>>                 return ret;
>>>
>>> @@ -1072,9 +1067,9 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>>>          */
>>>         if (is_bmips(pcie))
>>>                 burst = 0x1; /* 256 bytes */
>>> -       else if (pcie->soc_base == BCM2711)
>>> +       else if (pcie->cfg->soc_base == BCM2711)
>>>                 burst = 0x0; /* 128 bytes */
>>> -       else if (pcie->soc_base == BCM7278)
>>> +       else if (pcie->cfg->soc_base == BCM7278)
>>>                 burst = 0x3; /* 512 bytes */
>>>         else
>>>                 burst = 0x2; /* 512 bytes */
>>> @@ -1199,7 +1194,7 @@ static void brcm_extend_rbus_timeout(struct brcm_pcie *pcie)
>>>         u32 timeout_us = 4000000; /* 4 seconds, our setting for L1SS */
>>>
>>>         /* 7712 does not have this (RGR1) timer */
>>> -       if (pcie->soc_base == BCM7712)
>>> +       if (pcie->cfg->soc_base == BCM7712)
>>>                 return;
>>>
>>>         /* Each unit in timeout register is 1/216,000,000 seconds */
>>> @@ -1277,7 +1272,7 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
>>>         int ret, i;
>>>
>>>         /* Unassert the fundamental reset */
>>> -       ret = pcie->perst_set(pcie, 0);
>>> +       ret = pcie->cfg->perst_set(pcie, 0);
>>>         if (ret)
>>>                 return ret;
>>>
>>> @@ -1463,12 +1458,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->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
>>> +       return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
>>>  }
>>>
>>>  static inline int brcm_phy_stop(struct brcm_pcie *pcie)
>>>  {
>>> -       return pcie->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
>>> +       return pcie->cfg->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
>>>  }
>>>
>>>  static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
>>> @@ -1479,7 +1474,7 @@ static int brcm_pcie_turn_off(struct brcm_pcie *pcie)
>>>         if (brcm_pcie_link_up(pcie))
>>>                 brcm_pcie_enter_l23(pcie);
>>>         /* Assert fundamental reset */
>>> -       ret = pcie->perst_set(pcie, 1);
>>> +       ret = pcie->cfg->perst_set(pcie, 1);
>>>         if (ret)
>>>                 return ret;
>>>
>>> @@ -1582,7 +1577,7 @@ static int brcm_pcie_resume_noirq(struct device *dev)
>>>                 goto err_reset;
>>>
>>>         /* Take bridge out of reset so we can access the SERDES reg */
>>> -       pcie->bridge_sw_init_set(pcie, 0);
>>> +       pcie->cfg->bridge_sw_init_set(pcie, 0);
>>>
>>>         /* SERDES_IDDQ = 0 */
>>>         tmp = readl(base + HARD_DEBUG(pcie));
>>> @@ -1803,12 +1798,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>>>         pcie = pci_host_bridge_priv(bridge);
>>>         pcie->dev = &pdev->dev;
>>>         pcie->np = np;
>>> -       pcie->reg_offsets = data->offsets;
>>> -       pcie->soc_base = data->soc_base;
>>> -       pcie->perst_set = data->perst_set;
>>> -       pcie->bridge_sw_init_set = data->bridge_sw_init_set;
>>> -       pcie->has_phy = data->has_phy;
>>> -       pcie->num_inbound_wins = data->num_inbound_wins;
>>> +       pcie->cfg = data;
>>>
>>>         pcie->base = devm_platform_ioremap_resource(pdev, 0);
>>>         if (IS_ERR(pcie->base))
>>> @@ -1843,7 +1833,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>>>         if (ret)
>>>                 return dev_err_probe(&pdev->dev, ret, "could not enable clock\n");
>>>
>>> -       pcie->bridge_sw_init_set(pcie, 0);
>>> +       pcie->cfg->bridge_sw_init_set(pcie, 0);
>>>
>>>         if (pcie->swinit_reset) {
>>>                 ret = reset_control_assert(pcie->swinit_reset);
>>> @@ -1882,7 +1872,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>>>                 goto fail;
>>>
>>>         pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
>>> -       if (pcie->soc_base == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
>>> +       if (pcie->cfg->soc_base == 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;
>>> @@ -1897,7 +1888,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>>>                 }
>>>         }
>>>
>>> -       bridge->ops = pcie->soc_base == BCM7425 ? &brcm7425_pcie_ops : &brcm_pcie_ops;
>>> +       bridge->ops = pcie->cfg->soc_base == BCM7425 ?
>>> +                               &brcm7425_pcie_ops : &brcm_pcie_ops;
>>>         bridge->sysdata = pcie;
>>>
>>>         platform_set_drvdata(pdev, pcie);
>>
>> Reviewed-by: Jim Quinlan <james.quinlan@broadcom.com>
> 
> Hi Stan,
> 
> Sorry for the late notice but I get a compilation error on this commit:
> 
> drivers/pci/controller/pcie-brcmstb.c: In function 'brcm_pcie_turn_off':
> drivers/pci/controller/pcie-brcmstb.c:1492:14: error: 'struct
> brcm_pcie' has no member named 'bridge_sw_init_set'; did you mean
> 'bridge_reset'?
>   ret = pcie->bridge_sw_init_set(pcie, 1);
>               ^~~~~~~~~~~~~~~~~~
>               bridge_reset
> make[5]: *** [scripts/Makefile.build:194:
> drivers/pci/controller/pcie-brcmstb.o] Error 1
> 
> It appears to be fixed with the subsequent commit "PCI: brcmstb: Add
> bcm2712 support".
> 
> Can you please look into this and see if you get the same results?

Ah, it is my fault. Thanks for spotting this. This must have happened
when moving this patch earlier in the series.

Krzystof,

I could send a new version of the series or the other option could be to
rework those two patches in controller/brcmstb?

I will post later the fixes here if you choose the second option.

~Stan

Re: [PATCH v5 -next 04/11] PCI: brcmstb: Reuse config structure

[Krzysztof Wilczyński]

Hello,

> > Sorry for the late notice but I get a compilation error on this commit:
> > 
> > drivers/pci/controller/pcie-brcmstb.c: In function 'brcm_pcie_turn_off':
> > drivers/pci/controller/pcie-brcmstb.c:1492:14: error: 'struct
> > brcm_pcie' has no member named 'bridge_sw_init_set'; did you mean
> > 'bridge_reset'?
> >   ret = pcie->bridge_sw_init_set(pcie, 1);
> >               ^~~~~~~~~~~~~~~~~~
> >               bridge_reset
> > make[5]: *** [scripts/Makefile.build:194:
> > drivers/pci/controller/pcie-brcmstb.o] Error 1
> > 
> > It appears to be fixed with the subsequent commit "PCI: brcmstb: Add
> > bcm2712 support".
> > 
> > Can you please look into this and see if you get the same results?
> 
> Ah, it is my fault. Thanks for spotting this. This must have happened
> when moving this patch earlier in the series.

No worries.  Things happen.  We can fix everything. :)

> Krzystof,
> 
> I could send a new version of the series or the other option could be to
> rework those two patches in controller/brcmstb?

Happy to modify things on the branch directly.  But if it's easier to send
a new version, then feel free.  Either approach would work. :)

> I will post later the fixes here if you choose the second option.

Thank you, sir, for a prompt reply!  Appreciated.

	Krzysztof

[PATCH v5 -next 05/11] PCI: brcmstb: Expand inbound window size up to 64GB

[Stanimir Varbanov]

BCM2712 memory map can support up to 64GB of system memory, thus expand
the inbound window size in calculation helper function.

The change is save for the currently supported SoCs that has smaller
inbound window sizes.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
v4 -> v5:
 - No changes.

 drivers/pci/controller/pcie-brcmstb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 48b2747d8c98..59190d8be0fb 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -304,8 +304,8 @@ static int brcm_pcie_encode_ibar_size(u64 size)
 	if (log2_in >= 12 && log2_in <= 15)
 		/* Covers 4KB to 32KB (inclusive) */
 		return (log2_in - 12) + 0x1c;
-	else if (log2_in >= 16 && log2_in <= 35)
-		/* Covers 64KB to 32GB, (inclusive) */
+	else if (log2_in >= 16 && log2_in <= 36)
+		/* Covers 64KB to 64GB, (inclusive) */
 		return log2_in - 15;
 	/* Something is awry so disable */
 	return 0;
-- 
2.47.0

Re: [PATCH v5 -next 05/11] PCI: brcmstb: Expand inbound window size up to 64GB

[Jim Quinlan]

On Mon, Jan 20, 2025 at 8:01 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> BCM2712 memory map can support up to 64GB of system memory, thus expand
> the inbound window size in calculation helper function.
>
> The change is save for the currently supported SoCs that has smaller
> inbound window sizes.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Reviewed-by: Jim Quinlan <james.quinlan@broadcom.com>

> ---
> v4 -> v5:
>  - No changes.
>
>  drivers/pci/controller/pcie-brcmstb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 48b2747d8c98..59190d8be0fb 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -304,8 +304,8 @@ static int brcm_pcie_encode_ibar_size(u64 size)
>         if (log2_in >= 12 && log2_in <= 15)
>                 /* Covers 4KB to 32KB (inclusive) */
>                 return (log2_in - 12) + 0x1c;
> -       else if (log2_in >= 16 && log2_in <= 35)
> -               /* Covers 64KB to 32GB, (inclusive) */
> +       else if (log2_in >= 16 && log2_in <= 36)
> +               /* Covers 64KB to 64GB, (inclusive) */
>                 return log2_in - 15;
>         /* Something is awry so disable */
>         return 0;
> --
> 2.47.0
>

Re: [PATCH v5 -next 05/11] PCI: brcmstb: Expand inbound window size up to 64GB

[Bjorn Helgaas]

On Mon, Jan 20, 2025 at 03:01:13PM +0200, Stanimir Varbanov wrote:
> BCM2712 memory map can support up to 64GB of system memory, thus expand
> the inbound window size in calculation helper function.
> 
> The change is save for the currently supported SoCs that has smaller
> inbound window sizes.

If you repost:

s/save/safe/
s/that has/that have/

Otherwise we can fix these when merging.

> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> v4 -> v5:
>  - No changes.
> 
>  drivers/pci/controller/pcie-brcmstb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 48b2747d8c98..59190d8be0fb 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -304,8 +304,8 @@ static int brcm_pcie_encode_ibar_size(u64 size)
>  	if (log2_in >= 12 && log2_in <= 15)
>  		/* Covers 4KB to 32KB (inclusive) */
>  		return (log2_in - 12) + 0x1c;
> -	else if (log2_in >= 16 && log2_in <= 35)
> -		/* Covers 64KB to 32GB, (inclusive) */
> +	else if (log2_in >= 16 && log2_in <= 36)
> +		/* Covers 64KB to 64GB, (inclusive) */
>  		return log2_in - 15;
>  	/* Something is awry so disable */
>  	return 0;
> -- 
> 2.47.0
> 

Re: [PATCH v5 -next 05/11] PCI: brcmstb: Expand inbound window size up to 64GB

[Krzysztof Wilczyński]

Hello,

> > BCM2712 memory map can support up to 64GB of system memory, thus expand
> > the inbound window size in calculation helper function.
> > 
> > The change is save for the currently supported SoCs that has smaller
> > inbound window sizes.
> 
> If you repost:
> 
> s/save/safe/
> s/that has/that have/
> 
> Otherwise we can fix these when merging.

Updated directly on the branch.  Thank you!

	Krzysztof

[PATCH v5 -next 06/11] PCI: brcmstb: Add bcm2712 support

[Stanimir Varbanov]

Add bare minimum amount of changes in order to support PCIe RC hardware
IP found on RPi5. The PCIe controller on bcm2712 is based on bcm7712 and
as such it inherits register offsets, perst, bridge_reset ops and inbound
windows count.
Although, the implementation for bcm2712 needs a workaround related to the
control of the bridge_reset where turning off of the root port must not
shutdown the bridge_reset and this must be avoided. To implement this
workaround a quirks field is introduced in pcie_cfg_data struct.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
v4 -> v5:
 - No changes.

 drivers/pci/controller/pcie-brcmstb.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 59190d8be0fb..50607df34a66 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -234,10 +234,20 @@ struct inbound_win {
 	u64 cpu_addr;
 };
 
+/*
+ * 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
+ * register blocks, therefore no other controller can access this register
+ * space, and depending upon the bus fabric we may get a timeout (UBUS/GISB),
+ * or a hang (AXI).
+ */
+#define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN		BIT(0)
+
 struct pcie_cfg_data {
 	const int *offsets;
 	const enum pcie_soc_base soc_base;
 	const bool has_phy;
+	const 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);
@@ -1488,8 +1498,9 @@ 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));
 
-	/* Shutdown PCIe bridge */
-	ret = pcie->bridge_sw_init_set(pcie, 1);
+	if (!(pcie->cfg->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
+		/* Shutdown PCIe bridge */
+		ret = pcie->cfg->bridge_sw_init_set(pcie, 1);
 
 	return ret;
 }
@@ -1699,6 +1710,15 @@ static const struct pcie_cfg_data bcm2711_cfg = {
 	.num_inbound_wins = 3,
 };
 
+static const struct pcie_cfg_data bcm2712_cfg = {
+	.offsets	= pcie_offsets_bcm7712,
+	.soc_base	= BCM7712,
+	.perst_set	= brcm_pcie_perst_set_7278,
+	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
+	.quirks		= CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN,
+	.num_inbound_wins = 10,
+};
+
 static const struct pcie_cfg_data bcm4908_cfg = {
 	.offsets	= pcie_offsets,
 	.soc_base	= BCM4908,
@@ -1750,6 +1770,7 @@ static const struct pcie_cfg_data bcm7712_cfg = {
 
 static const struct of_device_id brcm_pcie_match[] = {
 	{ .compatible = "brcm,bcm2711-pcie", .data = &bcm2711_cfg },
+	{ .compatible = "brcm,bcm2712-pcie", .data = &bcm2712_cfg },
 	{ .compatible = "brcm,bcm4908-pcie", .data = &bcm4908_cfg },
 	{ .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg },
 	{ .compatible = "brcm,bcm7216-pcie", .data = &bcm7216_cfg },
-- 
2.47.0

Re: [PATCH v5 -next 06/11] PCI: brcmstb: Add bcm2712 support

[Jim Quinlan]

On Mon, Jan 20, 2025 at 8:01 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> Add bare minimum amount of changes in order to support PCIe RC hardware
> IP found on RPi5. The PCIe controller on bcm2712 is based on bcm7712 and
> as such it inherits register offsets, perst, bridge_reset ops and inbound
> windows count.
> Although, the implementation for bcm2712 needs a workaround related to the
> control of the bridge_reset where turning off of the root port must not
> shutdown the bridge_reset and this must be avoided. To implement this
> workaround a quirks field is introduced in pcie_cfg_data struct.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> v4 -> v5:
>  - No changes.
>
>  drivers/pci/controller/pcie-brcmstb.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 59190d8be0fb..50607df34a66 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -234,10 +234,20 @@ struct inbound_win {
>         u64 cpu_addr;
>  };
>
> +/*
> + * 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
> + * register blocks, therefore no other controller can access this register
> + * space, and depending upon the bus fabric we may get a timeout (UBUS/GISB),
> + * or a hang (AXI).
> + */
> +#define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN                BIT(0)
> +
>  struct pcie_cfg_data {
>         const int *offsets;
>         const enum pcie_soc_base soc_base;
>         const bool has_phy;
> +       const 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);
> @@ -1488,8 +1498,9 @@ 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));
>
> -       /* Shutdown PCIe bridge */
> -       ret = pcie->bridge_sw_init_set(pcie, 1);
> +       if (!(pcie->cfg->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
> +               /* Shutdown PCIe bridge */
> +               ret = pcie->cfg->bridge_sw_init_set(pcie, 1);
>
>         return ret;
>  }
> @@ -1699,6 +1710,15 @@ static const struct pcie_cfg_data bcm2711_cfg = {
>         .num_inbound_wins = 3,
>  };
>
> +static const struct pcie_cfg_data bcm2712_cfg = {
> +       .offsets        = pcie_offsets_bcm7712,
> +       .soc_base       = BCM7712,
> +       .perst_set      = brcm_pcie_perst_set_7278,
> +       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
> +       .quirks         = CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN,
> +       .num_inbound_wins = 10,
> +};
> +
>  static const struct pcie_cfg_data bcm4908_cfg = {
>         .offsets        = pcie_offsets,
>         .soc_base       = BCM4908,
> @@ -1750,6 +1770,7 @@ static const struct pcie_cfg_data bcm7712_cfg = {
>
>  static const struct of_device_id brcm_pcie_match[] = {
>         { .compatible = "brcm,bcm2711-pcie", .data = &bcm2711_cfg },
> +       { .compatible = "brcm,bcm2712-pcie", .data = &bcm2712_cfg },
>         { .compatible = "brcm,bcm4908-pcie", .data = &bcm4908_cfg },
>         { .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg },
>         { .compatible = "brcm,bcm7216-pcie", .data = &bcm7216_cfg },
Reviewed-by: Jim Quinlan <james.quinlan@broadcom.com>
> --
> 2.47.0
>

Re: [PATCH v5 -next 06/11] PCI: brcmstb: Add bcm2712 support

[Bjorn Helgaas]

On Mon, Jan 20, 2025 at 03:01:14PM +0200, Stanimir Varbanov wrote:
> Add bare minimum amount of changes in order to support PCIe RC hardware
> IP found on RPi5. The PCIe controller on bcm2712 is based on bcm7712 and
> as such it inherits register offsets, perst, bridge_reset ops and inbound
> windows count.

Add blank line between paragraphs.  We can fix when merging if you
don't repost for other reasons.

> Although, the implementation for bcm2712 needs a workaround related to the
> control of the bridge_reset where turning off of the root port must not
> shutdown the bridge_reset and this must be avoided. To implement this
> workaround a quirks field is introduced in pcie_cfg_data struct.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> v4 -> v5:
>  - No changes.
> 
>  drivers/pci/controller/pcie-brcmstb.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 59190d8be0fb..50607df34a66 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -234,10 +234,20 @@ struct inbound_win {
>  	u64 cpu_addr;
>  };
>  
> +/*
> + * 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
> + * register blocks, therefore no other controller can access this register
> + * space, and depending upon the bus fabric we may get a timeout (UBUS/GISB),
> + * or a hang (AXI).
> + */
> +#define CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN		BIT(0)
> +
>  struct pcie_cfg_data {
>  	const int *offsets;
>  	const enum pcie_soc_base soc_base;
>  	const bool has_phy;
> +	const 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);
> @@ -1488,8 +1498,9 @@ 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));
>  
> -	/* Shutdown PCIe bridge */
> -	ret = pcie->bridge_sw_init_set(pcie, 1);
> +	if (!(pcie->cfg->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
> +		/* Shutdown PCIe bridge */
> +		ret = pcie->cfg->bridge_sw_init_set(pcie, 1);
>  
>  	return ret;
>  }
> @@ -1699,6 +1710,15 @@ static const struct pcie_cfg_data bcm2711_cfg = {
>  	.num_inbound_wins = 3,
>  };
>  
> +static const struct pcie_cfg_data bcm2712_cfg = {
> +	.offsets	= pcie_offsets_bcm7712,
> +	.soc_base	= BCM7712,
> +	.perst_set	= brcm_pcie_perst_set_7278,
> +	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
> +	.quirks		= CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN,
> +	.num_inbound_wins = 10,
> +};
> +
>  static const struct pcie_cfg_data bcm4908_cfg = {
>  	.offsets	= pcie_offsets,
>  	.soc_base	= BCM4908,
> @@ -1750,6 +1770,7 @@ static const struct pcie_cfg_data bcm7712_cfg = {
>  
>  static const struct of_device_id brcm_pcie_match[] = {
>  	{ .compatible = "brcm,bcm2711-pcie", .data = &bcm2711_cfg },
> +	{ .compatible = "brcm,bcm2712-pcie", .data = &bcm2712_cfg },
>  	{ .compatible = "brcm,bcm4908-pcie", .data = &bcm4908_cfg },
>  	{ .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg },
>  	{ .compatible = "brcm,bcm7216-pcie", .data = &bcm7216_cfg },
> -- 
> 2.47.0
> 

Re: [PATCH v5 -next 06/11] PCI: brcmstb: Add bcm2712 support

[Krzysztof Wilczyński]

Hello,

> > Add bare minimum amount of changes in order to support PCIe RC hardware
> > IP found on RPi5. The PCIe controller on bcm2712 is based on bcm7712 and
> > as such it inherits register offsets, perst, bridge_reset ops and inbound
> > windows count.
> 
> Add blank line between paragraphs.  We can fix when merging if you
> don't repost for other reasons.

Updated directly on the branch.  Thank you!

	Krzysztof

[PATCH v5 -next 07/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Stanimir Varbanov]

The default input reference clock for the PHY PLL is 100Mhz, except for
some devices where it is 54Mhz like bcm2712C1 and bcm2712D0.

To implement this adjustments introduce a new .post_setup op in
pcie_cfg_data and call it at the end of brcm_pcie_setup function.

The bcm2712 .post_setup callback implements the required MDIO writes that
switch the PLL refclk and also change PHY PM clock period.

Without this RPi5 PCIex1 is unable to enumerate endpoint devices on
the expansion connector.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
v4 -> v5:
 - Updated a comment (Jim).

 drivers/pci/controller/pcie-brcmstb.c | 44 +++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 50607df34a66..03396a9d97be 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -55,6 +55,10 @@
 #define PCIE_RC_DL_MDIO_WR_DATA				0x1104
 #define PCIE_RC_DL_MDIO_RD_DATA				0x1108
 
+#define PCIE_RC_PL_PHY_CTL_15				0x184c
+#define  PCIE_RC_PL_PHY_CTL_15_DIS_PLL_PD_MASK		0x400000
+#define  PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK	0xff
+
 #define PCIE_MISC_MISC_CTRL				0x4008
 #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK	0x80
 #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE_MASK	0x400
@@ -251,6 +255,7 @@ struct pcie_cfg_data {
 	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);
 };
 
 struct subdev_regulators {
@@ -826,6 +831,38 @@ static int brcm_pcie_perst_set_generic(struct brcm_pcie *pcie, u32 val)
 	return 0;
 }
 
+static int brcm_pcie_post_setup_bcm2712(struct brcm_pcie *pcie)
+{
+	const u16 data[] = { 0x50b9, 0xbda1, 0x0094, 0x97b4, 0x5030, 0x5030, 0x0007 };
+	const u8 regs[] = { 0x16, 0x17, 0x18, 0x19, 0x1b, 0x1c, 0x1e };
+	int ret, i;
+	u32 tmp;
+
+	/* Allow a 54MHz (xosc) refclk source */
+	ret = brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, SET_ADDR_OFFSET, 0x1600);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++) {
+		ret = brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, regs[i], data[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	usleep_range(100, 200);
+
+	/*
+	 * Set L1SS sub-state timers to avoid lengthy state transitions,
+	 * PM clock period is 18.52ns (1/54MHz, round down).
+	 */
+	tmp = readl(pcie->base + PCIE_RC_PL_PHY_CTL_15);
+	tmp &= ~PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK;
+	tmp |= 0x12;
+	writel(tmp, pcie->base + PCIE_RC_PL_PHY_CTL_15);
+
+	return 0;
+}
+
 static void add_inbound_win(struct inbound_win *b, u8 *count, u64 size,
 			    u64 cpu_addr, u64 pci_offset)
 {
@@ -1189,6 +1226,12 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
 		PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK);
 	writel(tmp, base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
 
+	if (pcie->cfg->post_setup) {
+		ret = pcie->cfg->post_setup(pcie);
+		if (ret < 0)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -1715,6 +1758,7 @@ static const struct pcie_cfg_data bcm2712_cfg = {
 	.soc_base	= BCM7712,
 	.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,
 	.num_inbound_wins = 10,
 };
-- 
2.47.0

Re: [PATCH v5 -next 07/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Jim Quinlan]

On Mon, Jan 20, 2025 at 8:01 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> The default input reference clock for the PHY PLL is 100Mhz, except for
> some devices where it is 54Mhz like bcm2712C1 and bcm2712D0.
>
> To implement this adjustments introduce a new .post_setup op in
> pcie_cfg_data and call it at the end of brcm_pcie_setup function.
>
> The bcm2712 .post_setup callback implements the required MDIO writes that
> switch the PLL refclk and also change PHY PM clock period.
>
> Without this RPi5 PCIex1 is unable to enumerate endpoint devices on
> the expansion connector.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> v4 -> v5:
>  - Updated a comment (Jim).
>
>  drivers/pci/controller/pcie-brcmstb.c | 44 +++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 50607df34a66..03396a9d97be 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -55,6 +55,10 @@
>  #define PCIE_RC_DL_MDIO_WR_DATA                                0x1104
>  #define PCIE_RC_DL_MDIO_RD_DATA                                0x1108
>
> +#define PCIE_RC_PL_PHY_CTL_15                          0x184c
> +#define  PCIE_RC_PL_PHY_CTL_15_DIS_PLL_PD_MASK         0x400000
> +#define  PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK      0xff
> +
>  #define PCIE_MISC_MISC_CTRL                            0x4008
>  #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK    0x80
>  #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE_MASK    0x400
> @@ -251,6 +255,7 @@ struct pcie_cfg_data {
>         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);
>  };
>
>  struct subdev_regulators {
> @@ -826,6 +831,38 @@ static int brcm_pcie_perst_set_generic(struct brcm_pcie *pcie, u32 val)
>         return 0;
>  }
>
> +static int brcm_pcie_post_setup_bcm2712(struct brcm_pcie *pcie)
> +{
> +       const u16 data[] = { 0x50b9, 0xbda1, 0x0094, 0x97b4, 0x5030, 0x5030, 0x0007 };
> +       const u8 regs[] = { 0x16, 0x17, 0x18, 0x19, 0x1b, 0x1c, 0x1e };
> +       int ret, i;
> +       u32 tmp;
> +
> +       /* Allow a 54MHz (xosc) refclk source */
> +       ret = brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, SET_ADDR_OFFSET, 0x1600);
> +       if (ret < 0)
> +               return ret;
> +
> +       for (i = 0; i < ARRAY_SIZE(regs); i++) {
> +               ret = brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, regs[i], data[i]);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
> +       usleep_range(100, 200);
> +
> +       /*
> +        * Set L1SS sub-state timers to avoid lengthy state transitions,
> +        * PM clock period is 18.52ns (1/54MHz, round down).
> +        */
> +       tmp = readl(pcie->base + PCIE_RC_PL_PHY_CTL_15);
> +       tmp &= ~PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK;
> +       tmp |= 0x12;
> +       writel(tmp, pcie->base + PCIE_RC_PL_PHY_CTL_15);
> +
> +       return 0;
> +}
> +
>  static void add_inbound_win(struct inbound_win *b, u8 *count, u64 size,
>                             u64 cpu_addr, u64 pci_offset)
>  {
> @@ -1189,6 +1226,12 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>                 PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK);
>         writel(tmp, base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
>
> +       if (pcie->cfg->post_setup) {
> +               ret = pcie->cfg->post_setup(pcie);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
>         return 0;
>  }
>
> @@ -1715,6 +1758,7 @@ static const struct pcie_cfg_data bcm2712_cfg = {
>         .soc_base       = BCM7712,
>         .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,
>         .num_inbound_wins = 10,
>  };
> --
> 2.47.0
>
Hi Stan,
Any reason you didn't make this a quirk like the other commit?
Reviewed-by: Jim Quinlan <james.quinlan@broadcom.com>

Re: [PATCH v5 -next 07/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Stanimir Varbanov]

Hi Jim,

On 1/31/25 6:08 PM, Jim Quinlan wrote:
> On Mon, Jan 20, 2025 at 8:01 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>>
>> The default input reference clock for the PHY PLL is 100Mhz, except for
>> some devices where it is 54Mhz like bcm2712C1 and bcm2712D0.
>>
>> To implement this adjustments introduce a new .post_setup op in
>> pcie_cfg_data and call it at the end of brcm_pcie_setup function.
>>
>> The bcm2712 .post_setup callback implements the required MDIO writes that
>> switch the PLL refclk and also change PHY PM clock period.
>>
>> Without this RPi5 PCIex1 is unable to enumerate endpoint devices on
>> the expansion connector.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
>> ---
>> v4 -> v5:
>>  - Updated a comment (Jim).
>>
>>  drivers/pci/controller/pcie-brcmstb.c | 44 +++++++++++++++++++++++++++
>>  1 file changed, 44 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
>> index 50607df34a66..03396a9d97be 100644
>> --- a/drivers/pci/controller/pcie-brcmstb.c
>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>> @@ -55,6 +55,10 @@
>>  #define PCIE_RC_DL_MDIO_WR_DATA                                0x1104
>>  #define PCIE_RC_DL_MDIO_RD_DATA                                0x1108
>>
>> +#define PCIE_RC_PL_PHY_CTL_15                          0x184c
>> +#define  PCIE_RC_PL_PHY_CTL_15_DIS_PLL_PD_MASK         0x400000
>> +#define  PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK      0xff
>> +
>>  #define PCIE_MISC_MISC_CTRL                            0x4008
>>  #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK    0x80
>>  #define  PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE_MASK    0x400
>> @@ -251,6 +255,7 @@ struct pcie_cfg_data {
>>         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);
>>  };
>>
>>  struct subdev_regulators {
>> @@ -826,6 +831,38 @@ static int brcm_pcie_perst_set_generic(struct brcm_pcie *pcie, u32 val)
>>         return 0;
>>  }
>>
>> +static int brcm_pcie_post_setup_bcm2712(struct brcm_pcie *pcie)
>> +{
>> +       const u16 data[] = { 0x50b9, 0xbda1, 0x0094, 0x97b4, 0x5030, 0x5030, 0x0007 };
>> +       const u8 regs[] = { 0x16, 0x17, 0x18, 0x19, 0x1b, 0x1c, 0x1e };
>> +       int ret, i;
>> +       u32 tmp;
>> +
>> +       /* Allow a 54MHz (xosc) refclk source */
>> +       ret = brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, SET_ADDR_OFFSET, 0x1600);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(regs); i++) {
>> +               ret = brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, regs[i], data[i]);
>> +               if (ret < 0)
>> +                       return ret;
>> +       }
>> +
>> +       usleep_range(100, 200);
>> +
>> +       /*
>> +        * Set L1SS sub-state timers to avoid lengthy state transitions,
>> +        * PM clock period is 18.52ns (1/54MHz, round down).
>> +        */
>> +       tmp = readl(pcie->base + PCIE_RC_PL_PHY_CTL_15);
>> +       tmp &= ~PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK;
>> +       tmp |= 0x12;
>> +       writel(tmp, pcie->base + PCIE_RC_PL_PHY_CTL_15);
>> +
>> +       return 0;
>> +}
>> +
>>  static void add_inbound_win(struct inbound_win *b, u8 *count, u64 size,
>>                             u64 cpu_addr, u64 pci_offset)
>>  {
>> @@ -1189,6 +1226,12 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>>                 PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK);
>>         writel(tmp, base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
>>
>> +       if (pcie->cfg->post_setup) {
>> +               ret = pcie->cfg->post_setup(pcie);
>> +               if (ret < 0)
>> +                       return ret;
>> +       }
>> +
>>         return 0;
>>  }
>>
>> @@ -1715,6 +1758,7 @@ static const struct pcie_cfg_data bcm2712_cfg = {
>>         .soc_base       = BCM7712,
>>         .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,
>>         .num_inbound_wins = 10,
>>  };
>> --
>> 2.47.0
>>
> Hi Stan,
> Any reason you didn't make this a quirk like the other commit?

Not a special reason. I think that the code is more readable with
separate ops function which is executed after the generic .setup
function. The other SoC/variants could also benefit of such op.

> Reviewed-by: Jim Quinlan <james.quinlan@broadcom.com>

Thanks!

~Stan

Re: [PATCH v5 -next 07/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Bjorn Helgaas]

On Mon, Jan 20, 2025 at 03:01:15PM +0200, Stanimir Varbanov wrote:
> The default input reference clock for the PHY PLL is 100Mhz, except for
> some devices where it is 54Mhz like bcm2712C1 and bcm2712D0.
> 
> To implement this adjustments introduce a new .post_setup op in
> pcie_cfg_data and call it at the end of brcm_pcie_setup function.
> 
> The bcm2712 .post_setup callback implements the required MDIO writes that
> switch the PLL refclk and also change PHY PM clock period.
> 
> Without this RPi5 PCIex1 is unable to enumerate endpoint devices on
> the expansion connector.

This makes it sound like this patch should be reordered before "[PATCH
v5 -next 06/11] PCI: brcmstb: Add bcm2712 support".

We don't really want a driver to claim a bcm2712 controller before
it's able to enumerate devices, because that would break bisection
through this.

Bjorn

Re: [PATCH v5 -next 07/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Stanimir Varbanov]

Hi Bjorn,

On 2/21/25 11:33 PM, Bjorn Helgaas wrote:
> On Mon, Jan 20, 2025 at 03:01:15PM +0200, Stanimir Varbanov wrote:
>> The default input reference clock for the PHY PLL is 100Mhz, except for
>> some devices where it is 54Mhz like bcm2712C1 and bcm2712D0.
>>
>> To implement this adjustments introduce a new .post_setup op in
>> pcie_cfg_data and call it at the end of brcm_pcie_setup function.
>>
>> The bcm2712 .post_setup callback implements the required MDIO writes that
>> switch the PLL refclk and also change PHY PM clock period.
>>
>> Without this RPi5 PCIex1 is unable to enumerate endpoint devices on
>> the expansion connector.
> 
> This makes it sound like this patch should be reordered before "[PATCH
> v5 -next 06/11] PCI: brcmstb: Add bcm2712 support".
> 
> We don't really want a driver to claim a bcm2712 controller before
> it's able to enumerate devices, because that would break bisection
> through this.

I absolutely agree with you in regards to bisect-ability. But to satisfy
this I have to squash "Adjust PHY PLL ..." into "PCI: brcmstb: Add
bcm2712 support" to avoid a warning about not used function. If that
works I'll send a new rebased controller/brcmstb version (v6).

~Stan

[PATCH v5 -next 08/11] PCI: brcmstb: Adding a softdep to MIP MSI-X driver

[Stanimir Varbanov]

In case brcmstb PCIe driver and MIP MSI-X interrupt controller
drivers are built as modules there could be a race in probing.
To avoid this add a softdep to MIP driver to guarantee that MIP
driver will be load first.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v4 -> v5:
 - New patch in the series.

 drivers/pci/controller/pcie-brcmstb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 03396a9d97be..744fe1a4cf9c 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1997,3 +1997,4 @@ module_platform_driver(brcm_pcie_driver);
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Broadcom STB PCIe RC driver");
 MODULE_AUTHOR("Broadcom");
+MODULE_SOFTDEP("pre: irq_bcm2712_mip");
-- 
2.47.0

Re: [PATCH v5 -next 08/11] PCI: brcmstb: Adding a softdep to MIP MSI-X driver

[Florian Fainelli]

On 1/20/25 05:01, Stanimir Varbanov wrote:
> In case brcmstb PCIe driver and MIP MSI-X interrupt controller
> drivers are built as modules there could be a race in probing.
> To avoid this add a softdep to MIP driver to guarantee that MIP
> driver will be load first.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

Re: [PATCH v5 -next 08/11] PCI: brcmstb: Adding a softdep to MIP MSI-X driver

[Bjorn Helgaas]

On Mon, Jan 20, 2025 at 03:01:16PM +0200, Stanimir Varbanov wrote:
> In case brcmstb PCIe driver and MIP MSI-X interrupt controller
> drivers are built as modules there could be a race in probing.
> To avoid this add a softdep to MIP driver to guarantee that MIP
> driver will be load first.

Maybe this one too?  Should this be moved to after the irq_bcm2712_mip
driver is added, but before brcmstb will claim bcm2712?  I just want
to avoid bisection problems when possible, and it sounds like if we
lose the race, interrupts might not work as expected?
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v4 -> v5:
>  - New patch in the series.
> 
>  drivers/pci/controller/pcie-brcmstb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 03396a9d97be..744fe1a4cf9c 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -1997,3 +1997,4 @@ module_platform_driver(brcm_pcie_driver);
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("Broadcom STB PCIe RC driver");
>  MODULE_AUTHOR("Broadcom");
> +MODULE_SOFTDEP("pre: irq_bcm2712_mip");
> -- 
> 2.47.0
> 

Re: [PATCH v5 -next 08/11] PCI: brcmstb: Adding a softdep to MIP MSI-X driver

[Stanimir Varbanov]

Hi Bjorn,

On 2/21/25 11:40 PM, Bjorn Helgaas wrote:
> On Mon, Jan 20, 2025 at 03:01:16PM +0200, Stanimir Varbanov wrote:
>> In case brcmstb PCIe driver and MIP MSI-X interrupt controller
>> drivers are built as modules there could be a race in probing.
>> To avoid this add a softdep to MIP driver to guarantee that MIP
>> driver will be load first.
> 
> Maybe this one too?  Should this be moved to after the irq_bcm2712_mip
> driver is added, but before brcmstb will claim bcm2712?  I just want
> to avoid bisection problems when possible, and it sounds like if we
> lose the race, interrupts might not work as expected?

Makes sense, thank you.

~Stan

[PATCH v5 -next 09/11] PCI: brcmstb: Fix for missing of_node_put

[Stanimir Varbanov]

A call to of_parse_phandle() increments refcount, of_node_put must be
called when done the work on it. Fix missing of_node_put() on the
msi_np device node by using scope based of_node_put() cleanups.

Cc: stable@vger.kernel.org # v5.10+
Fixes: 40ca1bf580ef ("PCI: brcmstb: Add MSI support")
Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v4 -> v5:
 - New patch in the series.

 drivers/pci/controller/pcie-brcmstb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 744fe1a4cf9c..546056f7f0d3 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1844,7 +1844,8 @@ static struct pci_ops brcm7425_pcie_ops = {
 
 static int brcm_pcie_probe(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node, *msi_np;
+	struct device_node *msi_np __free(device_node) = NULL;
+	struct device_node *np = pdev->dev.of_node;
 	struct pci_host_bridge *bridge;
 	const struct pcie_cfg_data *data;
 	struct brcm_pcie *pcie;
-- 
2.47.0

Re: [PATCH v5 -next 09/11] PCI: brcmstb: Fix for missing of_node_put

[Florian Fainelli]

On 1/20/25 05:01, Stanimir Varbanov wrote:
> A call to of_parse_phandle() increments refcount, of_node_put must be
> called when done the work on it. Fix missing of_node_put() on the
> msi_np device node by using scope based of_node_put() cleanups.
> 
> Cc: stable@vger.kernel.org # v5.10+
> Fixes: 40ca1bf580ef ("PCI: brcmstb: Add MSI support")
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v4 -> v5:
>   - New patch in the series.
> 
>   drivers/pci/controller/pcie-brcmstb.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 744fe1a4cf9c..546056f7f0d3 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -1844,7 +1844,8 @@ static struct pci_ops brcm7425_pcie_ops = {
>   
>   static int brcm_pcie_probe(struct platform_device *pdev)
>   {
> -	struct device_node *np = pdev->dev.of_node, *msi_np;
> +	struct device_node *msi_np __free(device_node) = NULL;

In the interest of making this a straight back port to 5.10 that does 
not have all of the __free() goodies, I would just add the missing 
of_node_put() where necessary.

Also, since this is a bug fix, you should probably make it appear 
earlier in the patch series, or even sent it as a separate fix entirely.
-- 
Florian

Re: [PATCH v5 -next 09/11] PCI: brcmstb: Fix for missing of_node_put

[Stanimir Varbanov]

Hi Florian,

On 1/21/25 8:32 PM, Florian Fainelli wrote:
> On 1/20/25 05:01, Stanimir Varbanov wrote:
>> A call to of_parse_phandle() increments refcount, of_node_put must be
>> called when done the work on it. Fix missing of_node_put() on the
>> msi_np device node by using scope based of_node_put() cleanups.
>>
>> Cc: stable@vger.kernel.org # v5.10+
>> Fixes: 40ca1bf580ef ("PCI: brcmstb: Add MSI support")
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>> ---
>> v4 -> v5:
>>   - New patch in the series.
>>
>>   drivers/pci/controller/pcie-brcmstb.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/
>> controller/pcie-brcmstb.c
>> index 744fe1a4cf9c..546056f7f0d3 100644
>> --- a/drivers/pci/controller/pcie-brcmstb.c
>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>> @@ -1844,7 +1844,8 @@ static struct pci_ops brcm7425_pcie_ops = {
>>     static int brcm_pcie_probe(struct platform_device *pdev)
>>   {
>> -    struct device_node *np = pdev->dev.of_node, *msi_np;
>> +    struct device_node *msi_np __free(device_node) = NULL;
> 
> In the interest of making this a straight back port to 5.10 that does
> not have all of the __free() goodies, I would just add the missing
> of_node_put() where necessary.

Good point. Thank you.

> 
> Also, since this is a bug fix, you should probably make it appear
> earlier in the patch series, or even sent it as a separate fix entirely.

OK, will send it as a standalone patch (as v2 with your comment addressed).

~Stan

Re: [PATCH v5 -next 09/11] PCI: brcmstb: Fix for missing of_node_put

[Stanimir Varbanov]

Hi,

On 1/22/25 6:20 PM, Stanimir Varbanov wrote:
> Hi Florian,
> 
> On 1/21/25 8:32 PM, Florian Fainelli wrote:
>> On 1/20/25 05:01, Stanimir Varbanov wrote:
>>> A call to of_parse_phandle() increments refcount, of_node_put must be
>>> called when done the work on it. Fix missing of_node_put() on the
>>> msi_np device node by using scope based of_node_put() cleanups.
>>>
>>> Cc: stable@vger.kernel.org # v5.10+
>>> Fixes: 40ca1bf580ef ("PCI: brcmstb: Add MSI support")
>>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>>> ---
>>> v4 -> v5:
>>>   - New patch in the series.
>>>
>>>   drivers/pci/controller/pcie-brcmstb.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/
>>> controller/pcie-brcmstb.c
>>> index 744fe1a4cf9c..546056f7f0d3 100644
>>> --- a/drivers/pci/controller/pcie-brcmstb.c
>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>>> @@ -1844,7 +1844,8 @@ static struct pci_ops brcm7425_pcie_ops = {
>>>     static int brcm_pcie_probe(struct platform_device *pdev)
>>>   {
>>> -    struct device_node *np = pdev->dev.of_node, *msi_np;
>>> +    struct device_node *msi_np __free(device_node) = NULL;
>>
>> In the interest of making this a straight back port to 5.10 that does
>> not have all of the __free() goodies, I would just add the missing
>> of_node_put() where necessary.
> 
> Good point. Thank you.
> 
>>
>> Also, since this is a bug fix, you should probably make it appear
>> earlier in the patch series, or even sent it as a separate fix entirely.
> 
> OK, will send it as a standalone patch (as v2 with your comment addressed).

Sent here [1], now separate from this series.

~Stan

[1]
https://lore.kernel.org/lkml/20250122222955.1752778-1-svarbanov@suse.de/T/

[PATCH v5 -next 10/11] arm64: dts: broadcom: bcm2712: Add PCIe DT nodes

[Stanimir Varbanov]

Add PCIe devicetree nodes, plus needed reset and mip MSI-X
controllers.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v4 -> v5:
 - Reorder reset-names.
 - Dropped bus-range property.
 - Added MIP0(pcie2) and MIP1(pcie1) dma-ranges properties to access MIP MSI-X
   interrupt controllers.

 arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 147 ++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
index 39305e0869ec..a7019327dd12 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
@@ -192,6 +192,12 @@ soc: soc@107c000000 {
 		#address-cells = <1>;
 		#size-cells = <1>;
 
+		pcie_rescal: reset-controller@119500 {
+			compatible = "brcm,bcm7216-pcie-sata-rescal";
+			reg = <0x00119500 0x10>;
+			#reset-cells = <0>;
+		};
+
 		sdio1: mmc@fff000 {
 			compatible = "brcm,bcm2712-sdhci",
 				     "brcm,sdhci-brcmstb";
@@ -204,6 +210,12 @@ sdio1: mmc@fff000 {
 			mmc-ddr-3_3v;
 		};
 
+		bcm_reset: reset-controller@1504318 {
+			compatible = "brcm,brcmstb-reset";
+			reg = <0x01504318 0x30>;
+			#reset-cells = <1>;
+		};
+
 		system_timer: timer@7c003000 {
 			compatible = "brcm,bcm2835-system-timer";
 			reg = <0x7c003000 0x1000>;
@@ -431,6 +443,141 @@ axi: axi {
 		vc4: gpu {
 			compatible = "brcm,bcm2712-vc6";
 		};
+
+		pcie0: pcie@1000100000 {
+			compatible = "brcm,bcm2712-pcie";
+			reg = <0x10 0x00100000 0x00 0x9310>;
+			device_type = "pci";
+			linux,pci-domain = <0>;
+			max-link-speed = <2>;
+			num-lanes = <1>;
+			#address-cells = <3>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			interrupt-parent = <&gicv2>;
+			interrupts = <GIC_SPI 213 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 214 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "pcie", "msi";
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0 0 0 1 &gicv2 GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &gicv2 GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &gicv2 GIC_SPI 211 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &gicv2 GIC_SPI 212 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&pcie_rescal>, <&bcm_reset 42>;
+			reset-names = "rescal", "bridge";
+			msi-controller;
+			msi-parent = <&pcie0>;
+
+			ranges =
+				/* ~4GiB, 32-bit, non-prefetchable at PCIe 00_0000_0000 */
+				<0x02000000 0x00 0x00000000 0x17 0x00000000 0x00 0xfffffffc>,
+				/* 12GiB, 64-bit, prefetchable at PCIe 04_0000_0000 */
+				<0x43000000 0x04 0x00000000 0x14 0x00000000 0x03 0x00000000>;
+
+			dma-ranges =
+				/* 64GiB, 64-bit, prefetchable at PCIe 10_0000_0000 */
+				<0x43000000 0x10 0x00000000 0x00 0x00000000 0x10 0x00000000>;
+
+			status = "disabled";
+		};
+
+		pcie1: pcie@1000110000 {
+			compatible = "brcm,bcm2712-pcie";
+			reg = <0x10 0x00110000 0x00 0x9310>;
+			device_type = "pci";
+			linux,pci-domain = <1>;
+			max-link-speed = <2>;
+			num-lanes = <1>;
+			#address-cells = <3>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			interrupt-parent = <&gicv2>;
+			interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "pcie", "msi";
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0 0 0 1 &gicv2 GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &gicv2 GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &gicv2 GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &gicv2 GIC_SPI 222 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&pcie_rescal>, <&bcm_reset 43>;
+			reset-names = "rescal", "bridge";
+			msi-controller;
+			msi-parent = <&mip1>;
+
+			ranges =
+				/* ~4GiB, 32-bit, non-prefetchable at PCIe 00_0000_0000 */
+				<0x02000000 0x00 0x00000000 0x1b 0x00000000 0x00 0xfffffffc>,
+				/* 12GiB, 64-bit, prefetchable at PCIe 04_0000_0000 */
+				<0x43000000 0x04 0x00000000 0x18 0x00000000 0x03 0x00000000>;
+
+			dma-ranges =
+				/* 64GiB, 64-bit, non-prefetchable at PCIe 10_0000_0000 */
+				<0x03000000 0x10 0x00000000 0x00 0x00000000 0x10 0x00000000>,
+				/* 4KiB, 64-bit, non-prefetchable at PCIe ff_ffff_f000 MIP1 */
+				<0x03000000 0xff 0xfffff000 0x10 0x00131000 0x00 0x00001000>;
+
+			status = "disabled";
+		};
+
+		pcie2: pcie@1000120000 {
+			compatible = "brcm,bcm2712-pcie";
+			reg = <0x10 0x00120000 0x00 0x9310>;
+			device_type = "pci";
+			linux,pci-domain = <2>;
+			max-link-speed = <2>;
+			num-lanes = <4>;
+			#address-cells = <3>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			interrupt-parent = <&gicv2>;
+			interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 234 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "pcie", "msi";
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0 0 0 1 &gicv2 GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &gicv2 GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &gicv2 GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &gicv2 GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&pcie_rescal>, <&bcm_reset 44>;
+			reset-names = "rescal", "bridge";
+			msi-controller;
+			msi-parent = <&mip0>;
+
+			ranges =
+				/* ~4GiB, 32-bit, non-prefetchable at PCIe 00_0000_0000 */
+				<0x02000000 0x00 0x00000000 0x1f 0x00000000 0x00 0xfffffffc>,
+				/* 12GiB, 64-bit, prefetchable at PCIe 04_0000_0000 */
+				<0x43000000 0x04 0x00000000 0x1c 0x00000000 0x03 0x00000000>;
+
+			dma-ranges =
+				/* 4MiB, 32-bit, non-prefetchable at PCIe 00_0000_0000 */
+				<0x02000000 0x00 0x00000000 0x1f 0x00000000 0x00 0x00400000>,
+				/* 64GiB, 64-bit, prefetchable at PCIe 10_0000_0000 */
+				<0x43000000 0x10 0x00000000 0x00 0x00000000 0x10 0x00000000>,
+				/* 4KiB, 64-bit, non-prefetchable at PCIe ff_ffff_f000 MIP0 */
+				<0x03000000 0xff 0xfffff000 0x10 0x00130000 0x00 0x00001000>;
+
+			status = "disabled";
+		};
+
+		mip0: msi-controller@1000130000 {
+			compatible = "brcm,bcm2712-mip";
+			reg = <0x10 0x00130000 0x00 0xc0>,
+			      <0xff 0xfffff000 0x00 0x1000>;
+			msi-controller;
+			msi-ranges = <&gicv2 GIC_SPI 128 IRQ_TYPE_EDGE_RISING 64>;
+			brcm,msi-offset = <0>;
+		};
+
+		mip1: msi-controller@1000131000 {
+			compatible = "brcm,bcm2712-mip";
+			reg = <0x10 0x00131000 0x00 0xc0>,
+			      <0xff 0xfffff000 0x00 0x1000>;
+			msi-controller;
+			msi-ranges = <&gicv2 GIC_SPI 247 IRQ_TYPE_EDGE_RISING 8>;
+			brcm,msi-offset = <8>;
+		};
 	};
 
 	timer {
-- 
2.47.0

Re: [PATCH v5 -next 10/11] arm64: dts: broadcom: bcm2712: Add PCIe DT nodes

[Florian Fainelli]

From: Florian Fainelli <f.fainelli@gmail.com>

On Mon, 20 Jan 2025 15:01:18 +0200, Stanimir Varbanov <svarbanov@suse.de> wrote:
> Add PCIe devicetree nodes, plus needed reset and mip MSI-X
> controllers.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---

Applied to https://github.com/Broadcom/stblinux/commits/devicetree-arm64/next, thanks!
--
Florian

Re: [PATCH v5 -next 10/11] arm64: dts: broadcom: bcm2712: Add PCIe DT nodes

[Stanimir Varbanov]

Hi,

On 1/28/25 11:52 PM, Florian Fainelli wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> 
> On Mon, 20 Jan 2025 15:01:18 +0200, Stanimir Varbanov <svarbanov@suse.de> wrote:
>> Add PCIe devicetree nodes, plus needed reset and mip MSI-X
>> controllers.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>> ---
> 
> Applied to https://github.com/Broadcom/stblinux/commits/devicetree-arm64/next, thanks!

Florian, this somehow missed v6.15?

> --
> Florian

Re: [PATCH v5 -next 10/11] arm64: dts: broadcom: bcm2712: Add PCIe DT nodes

[Florian Fainelli]

On 4/23/2025 11:13 AM, Stanimir Varbanov wrote:
> Hi,
> 
> On 1/28/25 11:52 PM, Florian Fainelli wrote:
>> From: Florian Fainelli <f.fainelli@gmail.com>
>>
>> On Mon, 20 Jan 2025 15:01:18 +0200, Stanimir Varbanov <svarbanov@suse.de> wrote:
>>> Add PCIe devicetree nodes, plus needed reset and mip MSI-X
>>> controllers.
>>>
>>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>>> ---
>>
>> Applied to https://github.com/Broadcom/stblinux/commits/devicetree-arm64/next, thanks!
> 
> Florian, this somehow missed v6.15?

Yes, I was busy and did not have time to send the pull requests for 
6.15, this will be sent out for 6.16.
-- 
Florian

[PATCH v5 -next 11/11] arm64: dts: broadcom: bcm2712-rpi-5-b: Enable PCIe DT nodes

[Stanimir Varbanov]

Enable pcie1 and pcie2 DT nodes. Pcie1 is used for the extension
connector and pcie2 is used for RP1 south-bridge.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v4 -> v5:
 - No changes.

 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
index 1850a575e708..6ea3c102e0d6 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
@@ -111,3 +111,11 @@ &hdmi1 {
 	clocks = <&firmware_clocks 13>, <&firmware_clocks 14>, <&dvp 1>, <&clk_27MHz>;
 	clock-names = "hdmi", "bvb", "audio", "cec";
 };
+
+&pcie1 {
+	status = "okay";
+};
+
+&pcie2 {
+	status = "okay";
+};
-- 
2.47.0

Re: [PATCH v5 -next 11/11] arm64: dts: broadcom: bcm2712-rpi-5-b: Enable PCIe DT nodes

[Florian Fainelli]

From: Florian Fainelli <f.fainelli@gmail.com>

On Mon, 20 Jan 2025 15:01:19 +0200, Stanimir Varbanov <svarbanov@suse.de> wrote:
> Enable pcie1 and pcie2 DT nodes. Pcie1 is used for the extension
> connector and pcie2 is used for RP1 south-bridge.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---

Applied to https://github.com/Broadcom/stblinux/commits/devicetree-arm64/next, thanks!
--
Florian

Re: [PATCH v5 -next 00/11] Add PCIe support for bcm2712

[Ivan T. Ivanov]

Hi Stan,

On 01-20 15:01, Stanimir Varbanov wrote:
> 
> Here is v5 of the series which aims to add support for PCIe on bcm2712 SoC
> used by RPi5. Previous v4 can be found at [1].
> 
> Based the series on linux-next because of vc4 gpu node in bcm2712.dtsi.
> 
> v4 -> v5 changes include:
>  - Addressed comments to interrupt-controller driver. (Thomas)
>  - Fixed DTB warnings  broadcom/bcm2712-rpi-5-b.dtb.
>  - New patch in the series to fix missing of_node_put.
>  - New patch to make a softdep to a MIP MSI-X driver.
>  - Dropped the patch which adds MSI-X support in pcie-brcmstb driver,
>    and instead use DT dma-ranges to pass the needed information. (Jim)
> 
> For more detailed info check patches.
> 
> Comments are welcome!
> ~Stan
> 
> [1] https://patchwork.kernel.org/project/linux-pci/cover/20241025124515.14066-1-svarbanov@suse.de/
> 
> Stanimir Varbanov (11):
>   dt-bindings: interrupt-controller: Add bcm2712 MSI-X DT bindings
>   dt-bindings: PCI: brcmstb: Update bindings for PCIe on bcm2712
>   irqchip: Add Broadcom bcm2712 MSI-X interrupt controller
>   PCI: brcmstb: Reuse config structure
>   PCI: brcmstb: Expand inbound window size up to 64GB
>   PCI: brcmstb: Add bcm2712 support
>   PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
>   PCI: brcmstb: Adding a softdep to MIP MSI-X driver
>   PCI: brcmstb: Fix for missing of_node_put
>   arm64: dts: broadcom: bcm2712: Add PCIe DT nodes
>   arm64: dts: broadcom: bcm2712-rpi-5-b: Enable PCIe DT nodes
> 
>  .../brcm,bcm2712-msix.yaml                    |  60 ++++
>  .../bindings/pci/brcm,stb-pcie.yaml           |   6 +-
>  .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   8 +
>  arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 147 +++++++++
>  drivers/irqchip/Kconfig                       |  16 +
>  drivers/irqchip/Makefile                      |   1 +
>  drivers/irqchip/irq-bcm2712-mip.c             | 292 ++++++++++++++++++
>  drivers/pci/controller/pcie-brcmstb.c         | 147 ++++++---
>  8 files changed, 632 insertions(+), 45 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
>  create mode 100644 drivers/irqchip/irq-bcm2712-mip.c

Thanks! This works fine.

Tested-by: Ivan T. Ivanov <iivanov@suse.de>

Re: [PATCH v5 -next 00/11] Add PCIe support for bcm2712

[Stanimir Varbanov]

Hi Bjorn,

Do I need to send a new version with the collected Acked/Reviewed tags?

Thanks,
~Stan

On 1/20/25 3:01 PM, Stanimir Varbanov wrote:
> Here is v5 of the series which aims to add support for PCIe on bcm2712 SoC
> used by RPi5. Previous v4 can be found at [1].
> 
> Based the series on linux-next because of vc4 gpu node in bcm2712.dtsi.
> 
> v4 -> v5 changes include:
>  - Addressed comments to interrupt-controller driver. (Thomas)
>  - Fixed DTB warnings  broadcom/bcm2712-rpi-5-b.dtb.
>  - New patch in the series to fix missing of_node_put.
>  - New patch to make a softdep to a MIP MSI-X driver.
>  - Dropped the patch which adds MSI-X support in pcie-brcmstb driver,
>    and instead use DT dma-ranges to pass the needed information. (Jim)
> 
> For more detailed info check patches.
> 
> Comments are welcome!
> ~Stan
> 
> [1] https://patchwork.kernel.org/project/linux-pci/cover/20241025124515.14066-1-svarbanov@suse.de/
> 
> Stanimir Varbanov (11):
>   dt-bindings: interrupt-controller: Add bcm2712 MSI-X DT bindings
>   dt-bindings: PCI: brcmstb: Update bindings for PCIe on bcm2712
>   irqchip: Add Broadcom bcm2712 MSI-X interrupt controller
>   PCI: brcmstb: Reuse config structure
>   PCI: brcmstb: Expand inbound window size up to 64GB
>   PCI: brcmstb: Add bcm2712 support
>   PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
>   PCI: brcmstb: Adding a softdep to MIP MSI-X driver
>   PCI: brcmstb: Fix for missing of_node_put
>   arm64: dts: broadcom: bcm2712: Add PCIe DT nodes
>   arm64: dts: broadcom: bcm2712-rpi-5-b: Enable PCIe DT nodes
> 
>  .../brcm,bcm2712-msix.yaml                    |  60 ++++
>  .../bindings/pci/brcm,stb-pcie.yaml           |   6 +-
>  .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   8 +
>  arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 147 +++++++++
>  drivers/irqchip/Kconfig                       |  16 +
>  drivers/irqchip/Makefile                      |   1 +
>  drivers/irqchip/irq-bcm2712-mip.c             | 292 ++++++++++++++++++
>  drivers/pci/controller/pcie-brcmstb.c         | 147 ++++++---
>  8 files changed, 632 insertions(+), 45 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
>  create mode 100644 drivers/irqchip/irq-bcm2712-mip.c
> 

Re: [PATCH v5 -next 00/11] Add PCIe support for bcm2712

[Bjorn Helgaas]

On Tue, Feb 11, 2025 at 03:30:22PM +0200, Stanimir Varbanov wrote:
> Hi Bjorn,
> 
> Do I need to send a new version with the collected Acked/Reviewed tags?

No need to resend unless you change the code.

Bjorn

Re: [PATCH v5 -next 00/11] Add PCIe support for bcm2712

[Stanimir Varbanov]

Hi Bjorn,

On 2/12/25 8:04 PM, Bjorn Helgaas wrote:
> On Tue, Feb 11, 2025 at 03:30:22PM +0200, Stanimir Varbanov wrote:
>> Hi Bjorn,
>>
>> Do I need to send a new version with the collected Acked/Reviewed tags?
> 
> No need to resend unless you change the code.

At this point I have no plans to change the code.

> 
> Bjorn

Thank you!

~Stan

Re: [PATCH v5 -next 00/11] Add PCIe support for bcm2712

[Krzysztof Wilczyński]

Hello,

> Here is v5 of the series which aims to add support for PCIe on bcm2712 SoC
> used by RPi5. Previous v4 can be found at [1].
> 
> Based the series on linux-next because of vc4 gpu node in bcm2712.dtsi.

Applied to controller/brcmstb, thank you!

	Krzysztof