[PATCH v3 00/11] Add PCIe support for bcm2712

[PATCH v3 00/11] Add PCIe support for bcm2712

[Stanimir Varbanov]

Hello,

Here is v3 the series to add support for PCIe on bcm2712 SoC
used by RPi5. Previous v2 can be found at [1].

v2 -> v3 changes include:
 - Added Reviewed-by/Acked-by tags.
 - MIP MSI-X driver has been converted to MSI parent.
 - Added a new patch for PHY PLL adjustment need to succesfully
   enumerate PCIe endpoints on extension connector (tested with
   Pineboards AI Bundle + NVME SSD adapter card).
 - Re-introduced brcm,msi-offset DT private property for MIP
   interrupt-controller (without it I'm anable to use the interrupts
   of adapter cards on PCIe enxtension connector).

For more info check patches.

[1] https://patchwork.kernel.org/project/linux-pci/cover/20240910151845.17308-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: mip: Add Broadcom bcm2712 MSI-X interrupt controller
  PCI: brcmstb: Expand inbound size calculation helper
  PCI: brcmstb: Enable external MSI-X if available
  PCI: brcmstb: Avoid turn off of bridge reset
  PCI: brcmstb: Add bcm2712 support
  PCI: brcmstb: Reuse config structure
  PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
  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           |   5 +-
 .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   8 +
 arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 160 +++++++++
 drivers/irqchip/Kconfig                       |  16 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-bcm2712-mip.c             | 308 ++++++++++++++++++
 drivers/pci/controller/pcie-brcmstb.c         | 197 ++++++++---
 8 files changed, 707 insertions(+), 48 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
 create mode 100644 drivers/irqchip/irq-bcm2712-mip.c

-- 
2.43.0

[PATCH v3 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>
---
v2 -> v3:
 - dropped '>' from the description entry (Rob)
 - dropped interrupt-controller and interrupt-cells properties (Rob)
 - dropped msi-controller and use 'unevaluatedProperties' (Rob)
 - use const: 0 in msi-cells (Rob)
 - dropped msi-ranges property (Rob)
 - re-introduce brcm,msi-offset private property, 
   which looks unavoidable at that time

.../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.43.0

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

[Rob Herring (Arm)]

On Mon, 14 Oct 2024 16:07:00 +0300, Stanimir Varbanov wrote:
> Adds DT bindings for bcm2712 MSI-X interrupt peripheral controller.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v2 -> v3:
>  - dropped '>' from the description entry (Rob)
>  - dropped interrupt-controller and interrupt-cells properties (Rob)
>  - dropped msi-controller and use 'unevaluatedProperties' (Rob)
>  - use const: 0 in msi-cells (Rob)
>  - dropped msi-ranges property (Rob)
>  - re-introduce brcm,msi-offset private property,
>    which looks unavoidable at that time
> 
> .../brcm,bcm2712-msix.yaml                    | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
> 

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

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

[Stanimir Varbanov]

Update brcmstb PCIe controller bindings with bcm2712 compatible
and add new resets.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
---
v2 -> v3:
 - Added Reviewed-by/Acked-by tags.

Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 0925c520195a..8517dd9510ef 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
@@ -158,7 +159,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: brcm,bcm7712-pcie
+            enum:
+              - brcm,bcm7712-pcie
+              - brcm,bcm2712-pcie
     then:
       properties:
         resets:
-- 
2.43.0

[PATCH v3 03/11] irqchip: mip: 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>
---
v2 -> v3:
 - converted to MSI-parent.
 - parse re-introduced brcm,msi-offset DT property.
 - use of IRQCHIP_PLATFORM_DRIVER macros to avoid interrupt-controller
   from DT node.

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

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 341cd9ca5a05..c9bd0a4f6871 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -116,6 +116,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 e3679ec2b9f7..a11307b1b610 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -62,6 +62,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..14f0369a7d10
--- /dev/null
+++ b/drivers/irqchip/irq-bcm2712-mip.c
@@ -0,0 +1,308 @@
+// 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)
+ */
+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,
+			   unsigned int *hwirq)
+{
+	int bit;
+
+	spin_lock(&mip->lock);
+	bit = bitmap_find_free_region(mip->bitmap, mip->num_msis,
+				      ilog2(nr_irqs));
+	spin_unlock(&mip->lock);
+
+	if (bit < 0)
+		return bit;
+
+	if (hwirq)
+		*hwirq = bit;
+
+	return 0;
+}
+
+static void mip_free_hwirq(struct mip_priv *mip, unsigned int hwirq,
+			   unsigned int nr_irqs)
+{
+	spin_lock(&mip->lock);
+	bitmap_release_region(mip->bitmap, hwirq, ilog2(nr_irqs));
+	spin_unlock(&mip->lock);
+}
+
+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};
+	struct irq_data *irqd;
+	unsigned int hwirq, irq, i;
+	int ret;
+
+	ret = mip_alloc_hwirq(mip, nr_irqs, &irq);
+	if (ret < 0)
+		return ret;
+
+	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) {
+		mip_free_hwirq(mip, irq, nr_irqs);
+		return ret;
+	}
+
+	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);
+	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 |	\
+				 MSI_FLAG_PCI_MSIX)
+
+#define MIP_MSI_FLAGS_SUPPORTED	(MSI_GENERIC_FLAGS_MASK |	\
+				 MSI_FLAG_PCI_MSIX |		\
+				 IRQ_DOMAIN_FLAG_MSI_PARENT)
+
+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_PCI_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_PCI_MSI);
+	middle->dev = mip->dev;
+	middle->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
+	middle->msi_parent_ops = &mip_msi_parent_ops;
+
+	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;
+	}
+
+	/*
+	 * All MSI-X masked in for the host, masked out 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);
+
+	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 interrupt controller");
+MODULE_LICENSE("GPL");
-- 
2.43.0

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

[Thomas Gleixner]

> Subject: irqchip: mip:

is not a valid prefix

Just make it: irqchip: Add Broadcom  .....

> +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};
> +	struct irq_data *irqd;
> +	unsigned int hwirq, irq, i;

	unsigned int hwirq, irq, i;
	struct irq_data *irqd;

> +
> +#define MIP_MSI_FLAGS_REQUIRED	(MSI_FLAG_USE_DEF_DOM_OPS |	\
> +				 MSI_FLAG_USE_DEF_CHIP_OPS |	\
> +				 MSI_FLAG_PCI_MSI_MASK_PARENT |	\
> +				 MSI_FLAG_PCI_MSIX)

Why are you requiring MSI_FLAG_PCI_MSIX here? That's a supported flag,
not a required one.

> +#define MIP_MSI_FLAGS_SUPPORTED	(MSI_GENERIC_FLAGS_MASK |	\
> +				 MSI_FLAG_PCI_MSIX |		\

So this does not support multi MSI, but your allocation function looks
like it supports it (nr_irqs is not range checked).

> +				 IRQ_DOMAIN_FLAG_MSI_PARENT)

This is not a MSI flag and has no place here.

> +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_PCI_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_PCI_MSI);

That's the wrong token. DOMAIN_BUS_PCI_MSI is what the v2 global PCI/MSI
domain uses. But that's not what this is about. This is the parent
domain for PCI/MSI. DOMAIN_BUS_GENERIC_MSI or DOMAIN_BUS_NEXUS is what
you want here.

> +	middle->dev = mip->dev;
> +	middle->flags |= IRQ_DOMAIN_FLAG_MSI_PARENT;
> +	middle->msi_parent_ops = &mip_msi_parent_ops;
> +

Other than this, this looks good now.

Thanks,

        tglx

[PATCH v3 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Stanimir Varbanov]

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

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
v2 -> v3:
 - Added Reviewed-by tags.
 - Improved patch description (Florian).

 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 9321280f6edb..b0ef2f31914d 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -309,8 +309,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.43.0

Re: [PATCH v3 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Bjorn Helgaas]

On Mon, Oct 14, 2024 at 04:07:03PM +0300, Stanimir Varbanov wrote:
> BCM2712 memory map can supports up to 64GB of system
> memory, thus expand the inbound size calculation in
> helper function up to 64GB.

The fact that the calculation is done in a helper isn't important
here.  Can you make the subject line say something about supporting
DMA for up to 64GB of system memory?

This is being done specifically for BCM2712, but I assume it's safe
for *all* brcmstb devices, right?

s/can supports/can support/

Rewrap commit log to fill 75 columns.

> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> v2 -> v3:
>  - Added Reviewed-by tags.
>  - Improved patch description (Florian).
> 
>  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 9321280f6edb..b0ef2f31914d 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -309,8 +309,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.43.0
> 

Re: [PATCH v3 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Florian Fainelli]

On 10/14/24 09:57, Bjorn Helgaas wrote:
> On Mon, Oct 14, 2024 at 04:07:03PM +0300, Stanimir Varbanov wrote:
>> BCM2712 memory map can supports up to 64GB of system
>> memory, thus expand the inbound size calculation in
>> helper function up to 64GB.
> 
> The fact that the calculation is done in a helper isn't important
> here.  Can you make the subject line say something about supporting
> DMA for up to 64GB of system memory?
> 
> This is being done specifically for BCM2712, but I assume it's safe
> for *all* brcmstb devices, right?

It is safe in the sense that all brcmstb devices with this PCIe 
controller will adopt the same encoding of the size, all of the 
currently supported brcmstb devices have a variety of limitations when 
it comes to the amount of addressable DRAM however. Typically we have a 
hard limit at 4GB of DRAM per memory controller, some devices can do 2GB 
x3, 4GB x2, or 4GB x1.

Does that answer your question?
-- 
Florian

Re: [PATCH v3 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Bjorn Helgaas]

On Mon, Oct 14, 2024 at 10:10:11AM -0700, Florian Fainelli wrote:
> On 10/14/24 09:57, Bjorn Helgaas wrote:
> > On Mon, Oct 14, 2024 at 04:07:03PM +0300, Stanimir Varbanov wrote:
> > > BCM2712 memory map can supports up to 64GB of system
> > > memory, thus expand the inbound size calculation in
> > > helper function up to 64GB.
> > 
> > The fact that the calculation is done in a helper isn't important
> > here.  Can you make the subject line say something about supporting
> > DMA for up to 64GB of system memory?
> > 
> > This is being done specifically for BCM2712, but I assume it's safe
> > for *all* brcmstb devices, right?
> 
> It is safe in the sense that all brcmstb devices with this PCIe controller
> will adopt the same encoding of the size, all of the currently supported
> brcmstb devices have a variety of limitations when it comes to the amount of
> addressable DRAM however. Typically we have a hard limit at 4GB of DRAM per
> memory controller, some devices can do 2GB x3, 4GB x2, or 4GB x1.
> 
> Does that answer your question?

I'd like something in the commit log to the effect that while we're
doing this to support more system memory on BCM2712, this change is
safe for other SoCs that don't support as much system memory.

Re: [PATCH v3 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Jim Quinlan]

On Mon, Oct 14, 2024 at 1:25 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Oct 14, 2024 at 10:10:11AM -0700, Florian Fainelli wrote:
> > On 10/14/24 09:57, Bjorn Helgaas wrote:
> > > On Mon, Oct 14, 2024 at 04:07:03PM +0300, Stanimir Varbanov wrote:
> > > > BCM2712 memory map can supports up to 64GB of system
> > > > memory, thus expand the inbound size calculation in
> > > > helper function up to 64GB.
> > >
> > > The fact that the calculation is done in a helper isn't important
> > > here.  Can you make the subject line say something about supporting
> > > DMA for up to 64GB of system memory?
> > >
> > > This is being done specifically for BCM2712, but I assume it's safe
> > > for *all* brcmstb devices, right?
> >
> > It is safe in the sense that all brcmstb devices with this PCIe controller
> > will adopt the same encoding of the size, all of the currently supported
> > brcmstb devices have a variety of limitations when it comes to the amount of
> > addressable DRAM however. Typically we have a hard limit at 4GB of DRAM per
> > memory controller, some devices can do 2GB x3, 4GB x2, or 4GB x1.
> >
> > Does that answer your question?
>
> I'd like something in the commit log to the effect that while we're
> doing this to support more system memory on BCM2712, this change is
> safe for other SoCs that don't support as much system memory.

Hello,

This setting configures the size of an RC's inbound window to system
memory.  Any inbound access outside of all of the
inbound windows will be discarded.

Some existing SoCs cannot support the 64GB size.  Configuring such an
SoC to 64GB
will effectively disable the entire window.

Regards,
Jim Quinlan
Broadcom STB/CM

Re: [PATCH v3 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Bjorn Helgaas]

On Wed, Oct 16, 2024 at 01:09:00PM -0400, Jim Quinlan wrote:
> On Mon, Oct 14, 2024 at 1:25 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Oct 14, 2024 at 10:10:11AM -0700, Florian Fainelli wrote:
> > > On 10/14/24 09:57, Bjorn Helgaas wrote:
> > > > On Mon, Oct 14, 2024 at 04:07:03PM +0300, Stanimir Varbanov wrote:
> > > > > BCM2712 memory map can supports up to 64GB of system
> > > > > memory, thus expand the inbound size calculation in
> > > > > helper function up to 64GB.
> > > >
> > > > The fact that the calculation is done in a helper isn't important
> > > > here.  Can you make the subject line say something about supporting
> > > > DMA for up to 64GB of system memory?
> > > >
> > > > This is being done specifically for BCM2712, but I assume it's safe
> > > > for *all* brcmstb devices, right?
> > >
> > > It is safe in the sense that all brcmstb devices with this PCIe
> > > controller will adopt the same encoding of the size, all of the
> > > currently supported brcmstb devices have a variety of
> > > limitations when it comes to the amount of addressable DRAM
> > > however. Typically we have a hard limit at 4GB of DRAM per
> > > memory controller, some devices can do 2GB x3, 4GB x2, or 4GB
> > > x1.
> > >
> > > Does that answer your question?
> >
> > I'd like something in the commit log to the effect that while
> > we're doing this to support more system memory on BCM2712, this
> > change is safe for other SoCs that don't support as much system
> > memory.
> 
> This setting configures the size of an RC's inbound window to system
> memory.  Any inbound access outside of all of the inbound windows
> will be discarded.
> 
> Some existing SoCs cannot support the 64GB size.  Configuring such
> an SoC to 64GB will effectively disable the entire window.

So I *think* you're saying that this patch will break existing SoCs
that don't support the 64GB size, right?

Bjorn

Re: [PATCH v3 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Stanimir Varbanov]

Hi Bjorn,

On 10/16/24 22:38, Bjorn Helgaas wrote:
> On Wed, Oct 16, 2024 at 01:09:00PM -0400, Jim Quinlan wrote:
>> On Mon, Oct 14, 2024 at 1:25 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>>> On Mon, Oct 14, 2024 at 10:10:11AM -0700, Florian Fainelli wrote:
>>>> On 10/14/24 09:57, Bjorn Helgaas wrote:
>>>>> On Mon, Oct 14, 2024 at 04:07:03PM +0300, Stanimir Varbanov wrote:
>>>>>> BCM2712 memory map can supports up to 64GB of system
>>>>>> memory, thus expand the inbound size calculation in
>>>>>> helper function up to 64GB.
>>>>>
>>>>> The fact that the calculation is done in a helper isn't important
>>>>> here.  Can you make the subject line say something about supporting
>>>>> DMA for up to 64GB of system memory?
>>>>>
>>>>> This is being done specifically for BCM2712, but I assume it's safe
>>>>> for *all* brcmstb devices, right?
>>>>
>>>> It is safe in the sense that all brcmstb devices with this PCIe
>>>> controller will adopt the same encoding of the size, all of the
>>>> currently supported brcmstb devices have a variety of
>>>> limitations when it comes to the amount of addressable DRAM
>>>> however. Typically we have a hard limit at 4GB of DRAM per
>>>> memory controller, some devices can do 2GB x3, 4GB x2, or 4GB
>>>> x1.
>>>>
>>>> Does that answer your question?
>>>
>>> I'd like something in the commit log to the effect that while
>>> we're doing this to support more system memory on BCM2712, this
>>> change is safe for other SoCs that don't support as much system
>>> memory.
>>
>> This setting configures the size of an RC's inbound window to system
>> memory.  Any inbound access outside of all of the inbound windows
>> will be discarded.
>>
>> Some existing SoCs cannot support the 64GB size.  Configuring such
>> an SoC to 64GB will effectively disable the entire window.
> 
> So I *think* you're saying that this patch will break existing SoCs
> that don't support the 64GB size, right?

Existing SoCs will not be impacted. It could be theoretically possible
to break inbound window translations only if you wrongly populate window
sizes in DT.

~Stan

Re: [PATCH v3 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Bjorn Helgaas]

On Thu, Oct 17, 2024 at 11:02:33AM +0300, Stanimir Varbanov wrote:
> On 10/16/24 22:38, Bjorn Helgaas wrote:
> > On Wed, Oct 16, 2024 at 01:09:00PM -0400, Jim Quinlan wrote:
> >> On Mon, Oct 14, 2024 at 1:25 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >>> On Mon, Oct 14, 2024 at 10:10:11AM -0700, Florian Fainelli wrote:
> >>>> On 10/14/24 09:57, Bjorn Helgaas wrote:
> >>>>> On Mon, Oct 14, 2024 at 04:07:03PM +0300, Stanimir Varbanov wrote:
> >>>>>> BCM2712 memory map can supports up to 64GB of system
> >>>>>> memory, thus expand the inbound size calculation in
> >>>>>> helper function up to 64GB.
> >>>>>
> >>>>> The fact that the calculation is done in a helper isn't important
> >>>>> here.  Can you make the subject line say something about supporting
> >>>>> DMA for up to 64GB of system memory?
> >>>>>
> >>>>> This is being done specifically for BCM2712, but I assume it's safe
> >>>>> for *all* brcmstb devices, right?
> >>>>
> >>>> It is safe in the sense that all brcmstb devices with this PCIe
> >>>> controller will adopt the same encoding of the size, all of the
> >>>> currently supported brcmstb devices have a variety of
> >>>> limitations when it comes to the amount of addressable DRAM
> >>>> however. Typically we have a hard limit at 4GB of DRAM per
> >>>> memory controller, some devices can do 2GB x3, 4GB x2, or 4GB
> >>>> x1.
> >>>>
> >>>> Does that answer your question?
> >>>
> >>> I'd like something in the commit log to the effect that while
> >>> we're doing this to support more system memory on BCM2712, this
> >>> change is safe for other SoCs that don't support as much system
> >>> memory.
> >>
> >> This setting configures the size of an RC's inbound window to system
> >> memory.  Any inbound access outside of all of the inbound windows
> >> will be discarded.
> >>
> >> Some existing SoCs cannot support the 64GB size.  Configuring such
> >> an SoC to 64GB will effectively disable the entire window.
> > 
> > So I *think* you're saying that this patch will break existing SoCs
> > that don't support the 64GB size, right?
> 
> Existing SoCs will not be impacted. It could be theoretically possible
> to break inbound window translations only if you wrongly populate window
> sizes in DT.

I guess this is the part that I missed -- the inbound window sizes
come from DT (via bridge->dma_ranges, IIUC), and the patch merely
supports encoding of larger windows than previously.

Bjorn

[PATCH v3 05/11] PCI: brcmstb: Enable external MSI-X if available

[Stanimir Varbanov]

On RPi5 there is an external MIP MSI-X interrupt controller
which can handle up to 64 interrupts.

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

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

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index b0ef2f31914d..b76c16287f37 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1323,6 +1323,52 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
 	return 0;
 }
 
+static int brcm_pcie_enable_external_msix(struct brcm_pcie *pcie,
+					  struct device_node *msi_np)
+{
+	struct inbound_win inbound_wins[PCIE_BRCM_MAX_INBOUND_WINS];
+	u64 msi_pci_addr, msi_phys_addr;
+	struct resource r;
+	int mip_bar, ret;
+	u32 val, reg;
+
+	ret = of_property_read_reg(msi_np, 1, &msi_pci_addr, NULL);
+	if (ret)
+		return ret;
+
+	ret = of_address_to_resource(msi_np, 0, &r);
+	if (ret)
+		return ret;
+
+	msi_phys_addr = r.start;
+
+	/* Find free inbound window for MIP access */
+	mip_bar = brcm_pcie_get_inbound_wins(pcie, inbound_wins);
+	if (mip_bar < 0)
+		return mip_bar;
+
+	mip_bar += 1;
+	reg = brcm_bar_reg_offset(mip_bar);
+
+	val = lower_32_bits(msi_pci_addr);
+	val |= brcm_pcie_encode_ibar_size(SZ_4K);
+	writel(val, pcie->base + reg);
+
+	val = upper_32_bits(msi_pci_addr);
+	writel(val, pcie->base + reg + 4);
+
+	reg = brcm_ubus_reg_offset(mip_bar);
+
+	val = lower_32_bits(msi_phys_addr);
+	val |= PCIE_MISC_UBUS_BAR1_CONFIG_REMAP_ACCESS_EN_MASK;
+	writel(val, pcie->base + reg);
+
+	val = upper_32_bits(msi_phys_addr);
+	writel(val, pcie->base + reg + 4);
+
+	return 0;
+}
+
 static const char * const supplies[] = {
 	"vpcie3v3",
 	"vpcie3v3aux",
@@ -1888,11 +1934,20 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	msi_np = of_parse_phandle(pcie->np, "msi-parent", 0);
-	if (pci_msi_enabled() && msi_np == pcie->np) {
-		ret = brcm_pcie_enable_msi(pcie);
+	if (pci_msi_enabled()) {
+		msi_np = of_parse_phandle(pcie->np, "msi-parent", 0);
+		const char *str;
+
+		if (msi_np == pcie->np) {
+			str = "internal MSI";
+			ret = brcm_pcie_enable_msi(pcie);
+		} else {
+			str = "external MSI-X";
+			ret = brcm_pcie_enable_external_msix(pcie, msi_np);
+		}
+
 		if (ret) {
-			dev_err(pcie->dev, "probe of internal MSI failed");
+			dev_err(pcie->dev, "enable of %s failed\n", str);
 			goto fail;
 		}
 	}
-- 
2.43.0

[PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset

[Stanimir Varbanov]

On PCIe turn off avoid shutdown of bridge reset,
by introducing a quirk flag.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v2 -> v3:
 - Added more descriptive comment on CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN quirk.

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

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index b76c16287f37..757a1646d53c 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 not 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);
@@ -290,6 +300,7 @@ struct brcm_pcie {
 	struct subdev_regulators *sr;
 	bool			ep_wakeup_capable;
 	bool			has_phy;
+	u32			quirks;
 	u8			num_inbound_wins;
 };
 
@@ -1539,8 +1550,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->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
+		/* Shutdown PCIe bridge */
+		ret = pcie->bridge_sw_init_set(pcie, 1);
 
 	return ret;
 }
@@ -1854,6 +1866,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 	pcie->perst_set = data->perst_set;
 	pcie->bridge_sw_init_set = data->bridge_sw_init_set;
 	pcie->has_phy = data->has_phy;
+	pcie->quirks = data->quirks;
 	pcie->num_inbound_wins = data->num_inbound_wins;
 
 	pcie->base = devm_platform_ioremap_resource(pdev, 0);
-- 
2.43.0

Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset

[Bjorn Helgaas]

On Mon, Oct 14, 2024 at 04:07:05PM +0300, Stanimir Varbanov wrote:
> On PCIe turn off avoid shutdown of bridge reset,
> by introducing a quirk flag.

Can you include something here about *why* we need this change?  I
think the RESCAL comment below would be a good start.

I think this should be squashed with the next commit that adds the use
of CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN.  Otherwise this commit doesn't
have an obvious reason.

> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v2 -> v3:
>  - Added more descriptive comment on CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN quirk.
> 
>  drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index b76c16287f37..757a1646d53c 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 not other controller can access this register
> + * space, and depending upon the bus fabric we may get a timeout (UBUS/GISB),
> + * or a hang (AXI).

s/not other/no other/

> + */
> +#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);
> @@ -290,6 +300,7 @@ struct brcm_pcie {
>  	struct subdev_regulators *sr;
>  	bool			ep_wakeup_capable;
>  	bool			has_phy;
> +	u32			quirks;
>  	u8			num_inbound_wins;
>  };
>  
> @@ -1539,8 +1550,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->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
> +		/* Shutdown PCIe bridge */
> +		ret = pcie->bridge_sw_init_set(pcie, 1);
>  
>  	return ret;
>  }
> @@ -1854,6 +1866,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>  	pcie->perst_set = data->perst_set;
>  	pcie->bridge_sw_init_set = data->bridge_sw_init_set;
>  	pcie->has_phy = data->has_phy;
> +	pcie->quirks = data->quirks;
>  	pcie->num_inbound_wins = data->num_inbound_wins;
>  
>  	pcie->base = devm_platform_ioremap_resource(pdev, 0);
> -- 
> 2.43.0
> 

Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset

[Florian Fainelli]

On 10/14/24 10:01, Bjorn Helgaas wrote:
> On Mon, Oct 14, 2024 at 04:07:05PM +0300, Stanimir Varbanov wrote:
>> On PCIe turn off avoid shutdown of bridge reset,
>> by introducing a quirk flag.
> 
> Can you include something here about *why* we need this change?  I
> think the RESCAL comment below would be a good start.
> 
> I think this should be squashed with the next commit that adds the use
> of CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN.  Otherwise this commit doesn't
> have an obvious reason.

Agreed.
-- 
Florian

Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset

[Stanimir Varbanov]

Hi,

On 10/14/24 20:02, Florian Fainelli wrote:
> On 10/14/24 10:01, Bjorn Helgaas wrote:
>> On Mon, Oct 14, 2024 at 04:07:05PM +0300, Stanimir Varbanov wrote:
>>> On PCIe turn off avoid shutdown of bridge reset,
>>> by introducing a quirk flag.
>>
>> Can you include something here about *why* we need this change?  I
>> think the RESCAL comment below would be a good start.
>>
>> I think this should be squashed with the next commit that adds the use
>> of CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN.  Otherwise this commit doesn't
>> have an obvious reason.
> 
> Agreed.

OK, will do. Thank you for the review!

~Stan

Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset

[Jim Quinlan]

On Mon, Oct 14, 2024 at 9:07 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> On PCIe turn off avoid shutdown of bridge reset,
> by introducing a quirk flag.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v2 -> v3:
>  - Added more descriptive comment on CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN quirk.
>
>  drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index b76c16287f37..757a1646d53c 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 not other controller can access this register

s/no/not/

I assume that the quirks is specific to 2712 as the 7712 does not need
this since it only has PCIe1
(I'll probably seethis as I read more of your commits).

-- Jim

> + * 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);
> @@ -290,6 +300,7 @@ struct brcm_pcie {
>         struct subdev_regulators *sr;
>         bool                    ep_wakeup_capable;
>         bool                    has_phy;
> +       u32                     quirks;
>         u8                      num_inbound_wins;
>  };
>
> @@ -1539,8 +1550,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->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
> +               /* Shutdown PCIe bridge */
> +               ret = pcie->bridge_sw_init_set(pcie, 1);
>
>         return ret;
>  }
> @@ -1854,6 +1866,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>         pcie->perst_set = data->perst_set;
>         pcie->bridge_sw_init_set = data->bridge_sw_init_set;
>         pcie->has_phy = data->has_phy;
> +       pcie->quirks = data->quirks;
>         pcie->num_inbound_wins = data->num_inbound_wins;
>
>         pcie->base = devm_platform_ioremap_resource(pdev, 0);
> --
> 2.43.0
>

Re: [PATCH v3 06/11] PCI: brcmstb: Avoid turn off of bridge reset

[Stanimir Varbanov]

Hi Jim,

On 10/16/24 20:17, Jim Quinlan wrote:
> On Mon, Oct 14, 2024 at 9:07 AM Stanimir Varbanov <svarbanov@suse.de> wrote:
>>
>> On PCIe turn off avoid shutdown of bridge reset,
>> by introducing a quirk flag.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>> ---
>> v2 -> v3:
>>  - Added more descriptive comment on CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN quirk.
>>
>>  drivers/pci/controller/pcie-brcmstb.c | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
>> index b76c16287f37..757a1646d53c 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 not other controller can access this register
> 
> s/no/not/
> 
> I assume that the quirks is specific to 2712 as the 7712 does not need
> this since it only has PCIe1
> (I'll probably seethis as I read more of your commits).

Yes, the .post_setup op is implemented for 2712 only. Look into next
patch in the series.

~Stan

[PATCH v3 07/11] PCI: brcmstb: Add bcm2712 support

[Stanimir Varbanov]

Add bare minimum amount of changes in order to support
PCIe RC hardware IP found in RPi5.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
v2 -> v3:
 - Added Reviewed-by tag.

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

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 757a1646d53c..85dd328fa090 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1762,6 +1762,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,
@@ -1813,6 +1822,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.43.0

[PATCH v3 08/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>
---
v2 -> v3:
 - Added Reviewed-by tag.

drivers/pci/controller/pcie-brcmstb.c | 76 ++++++++++++---------------
 1 file changed, 33 insertions(+), 43 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 85dd328fa090..407343a30439 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
@@ -286,8 +286,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;
@@ -295,18 +293,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;
-	u32			quirks;
-	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;
 }
 
 /*
@@ -866,7 +860,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) {
@@ -883,10 +877,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;
 	}
 
@@ -900,7 +894,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,
@@ -1023,7 +1017,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;
@@ -1047,15 +1041,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;
 		}
 	}
@@ -1063,7 +1057,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;
 
@@ -1083,9 +1077,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 */
@@ -1210,7 +1204,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 */
@@ -1288,7 +1282,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;
 
@@ -1520,12 +1514,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)
@@ -1536,7 +1530,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;
 
@@ -1550,9 +1544,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));
 
-	if (!(pcie->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
+	if (!(pcie->cfg->quirks & CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN))
 		/* Shutdown PCIe bridge */
-		ret = pcie->bridge_sw_init_set(pcie, 1);
+		ret = pcie->cfg->bridge_sw_init_set(pcie, 1);
 
 	return ret;
 }
@@ -1640,7 +1634,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));
@@ -1871,13 +1865,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->quirks = data->quirks;
-	pcie->num_inbound_wins = data->num_inbound_wins;
+	pcie->cfg = data;
 
 	pcie->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pcie->base))
@@ -1912,7 +1900,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);
@@ -1951,7 +1939,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;
@@ -1975,7 +1964,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.43.0

Re: [PATCH v3 08/11] PCI: brcmstb: Reuse config structure

[Bjorn Helgaas]

On Mon, Oct 14, 2024 at 04:07:07PM +0300, Stanimir Varbanov wrote:
> Instead of copying fields from pcie_cfg_data structure to
> brcm_pcie reference it directly.

This seems good.  I would consider moving it earlier in the series
so you don't have to touch the CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN stuff
twice.

Bjorn

Re: [PATCH v3 08/11] PCI: brcmstb: Reuse config structure

[Stanimir Varbanov]

Hi Bjorn,

On 10/14/24 20:03, Bjorn Helgaas wrote:
> On Mon, Oct 14, 2024 at 04:07:07PM +0300, Stanimir Varbanov wrote:
>> Instead of copying fields from pcie_cfg_data structure to
>> brcm_pcie reference it directly.
> 
> This seems good.  I would consider moving it earlier in the series
> so you don't have to touch the CFG_QUIRK_AVOID_BRIDGE_SHUTDOWN stuff
> twice.

Sure, will do it. Thank you for the review!

[PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Stanimir Varbanov]

Use canned MDIO writes from Broadcom that switch the ref_clk output
pair to run from the internal fractional PLL, and set the internal
PLL to expect a 54MHz input reference clock.

Without this RPi5 PCIe cannot enumerate endpoint devices on
extension connector.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
v2 -> v3:
 - New patch.

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

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 407343a30439..12591e292c0c 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,32 @@ 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 };
+	u32 tmp;
+	int i;
+
+	/* Allow a 54MHz (xosc) refclk source */
+
+	brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, SET_ADDR_OFFSET, 0x1600);
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++)
+		brcm_pcie_mdio_write(pcie->base, MDIO_PORT0, regs[i], data[i]);
+
+	usleep_range(100, 200);
+
+	/* Fix for L1SS errata */
+	tmp = readl(pcie->base + PCIE_RC_PL_PHY_CTL_15);
+	tmp &= ~PCIE_RC_PL_PHY_CTL_15_PM_CLK_PERIOD_MASK;
+	/* PM clock period is 18.52ns (round down) */
+	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 +1220,9 @@ 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)
+		pcie->cfg->post_setup(pcie);
+
 	return 0;
 }
 
@@ -1761,6 +1795,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.43.0

Re: [PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Florian Fainelli]

On 10/14/24 06:07, Stanimir Varbanov wrote:
> Use canned MDIO writes from Broadcom that switch the ref_clk output
> pair to run from the internal fractional PLL, and set the internal
> PLL to expect a 54MHz input reference clock.
> 
> Without this RPi5 PCIe cannot enumerate endpoint devices on
> extension connector.

You could say that the default reference clock for the PLL is 100MHz, 
except for some devices, where it is 54MHz, like 2712d0. AFAIR, 2712c1 
might have been 100MHz as well, so whether we need to support that 
revision of the chip or not might be TBD.

> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
> v2 -> v3:
>   - New patch.
> 
>   drivers/pci/controller/pcie-brcmstb.c | 35 +++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 407343a30439..12591e292c0c 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,32 @@ 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 };
> +	u32 tmp;
> +	int i;
> +
> +	/* Allow a 54MHz (xosc) refclk source */
> +

This newline is not necessary. Other than that:

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

Re: [PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Stanimir Varbanov]

Hi Florian,

On 10/14/24 20:07, Florian Fainelli wrote:
> On 10/14/24 06:07, Stanimir Varbanov wrote:
>> Use canned MDIO writes from Broadcom that switch the ref_clk output
>> pair to run from the internal fractional PLL, and set the internal
>> PLL to expect a 54MHz input reference clock.
>>
>> Without this RPi5 PCIe cannot enumerate endpoint devices on
>> extension connector.
> 
> You could say that the default reference clock for the PLL is 100MHz,
> except for some devices, where it is 54MHz, like 2712d0. AFAIR, 2712c1
> might have been 100MHz as well, so whether we need to support that
> revision of the chip or not might be TBD.

I'm confused now, according to [1] :

BCM2712C1 - 4GB and 8GB RPi5 models
BCM2712D0 - 2GB RPi5 models

My device is 4GB RPi5 model so I would expect it is BCM2712C1, thus
according to your comment the PLL PHY adjustment is not needed. But I
see that the PCIex1 RC cannot enumerate devices on ext PCI connector
because of link training failure. Implementing PLL adjustment fixes the
failure.


~Stan

[1]
https://www.raspberrypi.com/documentation/computers/processors.html#bcm2712

Re: [PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Jonathan Bell]

On Thu, 17 Oct 2024 at 15:42, Stanimir Varbanov <svarbanov@suse.de> wrote:
>
> Hi Florian,
>
> On 10/14/24 20:07, Florian Fainelli wrote:
> > On 10/14/24 06:07, Stanimir Varbanov wrote:
> >> Use canned MDIO writes from Broadcom that switch the ref_clk output
> >> pair to run from the internal fractional PLL, and set the internal
> >> PLL to expect a 54MHz input reference clock.
> >>
> >> Without this RPi5 PCIe cannot enumerate endpoint devices on
> >> extension connector.
> >
> > You could say that the default reference clock for the PLL is 100MHz,
> > except for some devices, where it is 54MHz, like 2712d0. AFAIR, 2712c1
> > might have been 100MHz as well, so whether we need to support that
> > revision of the chip or not might be TBD.
>
> I'm confused now, according to [1] :
>
> BCM2712C1 - 4GB and 8GB RPi5 models
> BCM2712D0 - 2GB RPi5 models
>
> My device is 4GB RPi5 model so I would expect it is BCM2712C1, thus
> according to your comment the PLL PHY adjustment is not needed. But I
> see that the PCIex1 RC cannot enumerate devices on ext PCI connector
> because of link training failure. Implementing PLL adjustment fixes the
> failure.
>
>
> ~Stan
>
> [1]
> https://www.raspberrypi.com/documentation/computers/processors.html#bcm2712

The MDIO writes for 2712C1 are required because platform firmware
arranges for the reference input clock to be 54MHz.
2712D0 can't generate a 100MHz reference input, it's 54MHz only. The
MDIO register defaults are also changed to suit, but there's no harm
in applying the writes anyway.
Both steppings need to behave identically for compliance and interop reasons.
RP1 is very tolerant of out-of-spec reference clocks, which is why
only the expansion connector appears to be affected.

Regards
Jonathan

Re: [PATCH v3 09/11] PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk

[Stanimir Varbanov]

Hi,

On 10/21/24 15:56, Jonathan Bell wrote:
> On Thu, 17 Oct 2024 at 15:42, Stanimir Varbanov <svarbanov@suse.de> wrote:
>>
>> Hi Florian,
>>
>> On 10/14/24 20:07, Florian Fainelli wrote:
>>> On 10/14/24 06:07, Stanimir Varbanov wrote:
>>>> Use canned MDIO writes from Broadcom that switch the ref_clk output
>>>> pair to run from the internal fractional PLL, and set the internal
>>>> PLL to expect a 54MHz input reference clock.
>>>>
>>>> Without this RPi5 PCIe cannot enumerate endpoint devices on
>>>> extension connector.
>>>
>>> You could say that the default reference clock for the PLL is 100MHz,
>>> except for some devices, where it is 54MHz, like 2712d0. AFAIR, 2712c1
>>> might have been 100MHz as well, so whether we need to support that
>>> revision of the chip or not might be TBD.
>>
>> I'm confused now, according to [1] :
>>
>> BCM2712C1 - 4GB and 8GB RPi5 models
>> BCM2712D0 - 2GB RPi5 models
>>
>> My device is 4GB RPi5 model so I would expect it is BCM2712C1, thus
>> according to your comment the PLL PHY adjustment is not needed. But I
>> see that the PCIex1 RC cannot enumerate devices on ext PCI connector
>> because of link training failure. Implementing PLL adjustment fixes the
>> failure.
>>
>>
>> ~Stan
>>
>> [1]
>> https://www.raspberrypi.com/documentation/computers/processors.html#bcm2712
> 

Thanks for jumping in, Jon.

> The MDIO writes for 2712C1 are required because platform firmware
> arranges for the reference input clock to be 54MHz.
> 2712D0 can't generate a 100MHz reference input, it's 54MHz only. The
> MDIO register defaults are also changed to suit, but there's no harm

I see that MDIO register defaults for pcie2 (where RP1 is connected) are
changed to suit to 54Mhz but this is not true for pcie1 (expansion
connector). And that could explain why the link training is failing on
pcie1.

> in applying the writes anyway.
> Both steppings need to behave identically for compliance and interop reasons.

Yes, for sure.

> RP1 is very tolerant of out-of-spec reference clocks, which is why
> only the expansion connector appears to be affected.

Thank you for clarifications.

~Stan

[1] Firmware version: RPi: BOOTSYS release VERSION:790da7ef DATE:
2024/07/30 TIME: 15:25:46

[PATCH v3 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>
---
v2 -> v3:
- Added comments on ranges and dma-ranges properties (Florian)

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

diff --git a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
index 6e5a984c1d4e..a83de23856e3 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>;
@@ -267,6 +279,154 @@ gicv2: interrupt-controller@7fff9000 {
 		};
 	};
 
+	axi@1000000000 {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+
+		ranges = <0x00 0x00000000 0x10 0x00000000 0x01 0x00000000>,
+			 <0x14 0x00000000 0x14 0x00000000 0x04 0x00000000>,
+			 <0x18 0x00000000 0x18 0x00000000 0x04 0x00000000>,
+			 <0x1c 0x00000000 0x1c 0x00000000 0x04 0x00000000>;
+
+		dma-ranges = <0x00 0x00000000 0x00 0x00000000 0x10 0x00000000>,
+			     <0x14 0x00000000 0x14 0x00000000 0x04 0x00000000>,
+			     <0x18 0x00000000 0x18 0x00000000 0x04 0x00000000>,
+			     <0x1c 0x00000000 0x1c 0x00000000 0x04 0x00000000>;
+
+		pcie0: pcie@100000 {
+			compatible = "brcm,bcm2712-pcie";
+			reg = <0x00 0x00100000 0x00 0x9310>;
+			device_type = "pci";
+			linux,pci-domain = <0>;
+			max-link-speed = <2>;
+			bus-range = <0x00 0xff>;
+			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 = <&bcm_reset 42>, <&pcie_rescal>;
+			reset-names = "bridge", "rescal";
+			msi-controller;
+			msi-parent = <&pcie0>;
+
+			ranges =
+				/* ~4GB, 32-bit, non-prefetchable at PCIe 00_0000_0000 */
+				<0x02000000 0x00 0x00000000 0x17 0x00000000 0x00 0xfffffffc>,
+				/* 12GB, 64-bit, prefetchable at PCIe 04_0000_0000 */
+				<0x43000000 0x04 0x00000000 0x14 0x00000000 0x03 0x00000000>;
+
+			dma-ranges =
+				/* 64GB, 64-bit, prefetchable at PCIe 10_0000_0000 */
+				<0x43000000 0x10 0x00000000 0x00 0x00000000 0x10 0x00000000>;
+
+			status = "disabled";
+		};
+
+		pcie1: pcie@110000 {
+			compatible = "brcm,bcm2712-pcie";
+			reg = <0x00 0x00110000 0x00 0x9310>;
+			device_type = "pci";
+			linux,pci-domain = <1>;
+			max-link-speed = <2>;
+			bus-range = <0x00 0xff>;
+			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 = <&bcm_reset 43>, <&pcie_rescal>;
+			reset-names = "bridge", "rescal";
+			msi-parent = <&mip1>;
+
+			ranges =
+				/* ~4GB, 32-bit, non-prefetchable at PCIe 00_0000_0000 */
+				<0x02000000 0x00 0x00000000 0x1b 0x00000000 0x00 0xfffffffc>,
+				/* 12GB, 64-bit, prefetchable at PCIe 04_0000_0000 */
+				<0x43000000 0x04 0x00000000 0x18 0x00000000 0x03 0x00000000>;
+
+			dma-ranges =
+				/* 64GB, 64-bit, non-prefetchable at PCIe 10_0000_0000 */
+				<0x03000000 0x10 0x00000000 0x00 0x00000000 0x10 0x00000000>;
+
+			status = "disabled";
+		};
+
+		pcie2: pcie@120000 {
+			compatible = "brcm,bcm2712-pcie";
+			reg = <0x00 0x00120000 0x00 0x9310>;
+			device_type = "pci";
+			linux,pci-domain = <2>;
+			max-link-speed = <2>;
+			bus-range = <0x00 0xff>;
+			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 = <&bcm_reset 44>, <&pcie_rescal>;
+			reset-names = "bridge", "rescal";
+			msi-parent = <&mip0>;
+
+			ranges =
+				/* ~4GB, 32-bit, non-prefetchable at PCIe 00_0000_0000 */
+				<0x02000000 0x00 0x00000000 0x1f 0x00000000 0x00 0xfffffffc>,
+				/* 12GB, 64-bit, prefetchable at PCIe 04_0000_0000 */
+				<0x43000000 0x04 0x00000000 0x1c 0x00000000 0x03 0x00000000>;
+
+			dma-ranges =
+				/* 4MB, 32-bit, non-prefetchable at PCIe 00_0000_0000 */
+				<0x02000000 0x00 0x00000000 0x1f 0x00000000 0x00 0x00400000>,
+				/* 64GB, 64-bit, prefetchable at PCIe 10_0000_0000 */
+				<0x43000000 0x10 0x00000000 0x00 0x00000000 0x10 0x00000000>;
+
+			status = "disabled";
+		};
+
+		mip0: msi-controller@130000 {
+			compatible = "brcm,bcm2712-mip";
+			reg = <0x00 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@131000 {
+			compatible = "brcm,bcm2712-mip";
+			reg = <0x00 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 {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
-- 
2.43.0

[PATCH v3 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>
---
v2 -> v3:
 - 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 2bdbb6780242..e970a6013c6f 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
@@ -62,3 +62,11 @@ &sdio1 {
 	sd-uhs-ddr50;
 	sd-uhs-sdr104;
 };
+
+&pcie1 {
+	status = "okay";
+};
+
+&pcie2 {
+	status = "okay";
+};
-- 
2.43.0

Re: [PATCH v3 00/11] Add PCIe support for bcm2712

[Rob Herring (Arm)]

On Mon, 14 Oct 2024 16:06:59 +0300, Stanimir Varbanov wrote:
> Hello,
> 
> Here is v3 the series to add support for PCIe on bcm2712 SoC
> used by RPi5. Previous v2 can be found at [1].
> 
> v2 -> v3 changes include:
>  - Added Reviewed-by/Acked-by tags.
>  - MIP MSI-X driver has been converted to MSI parent.
>  - Added a new patch for PHY PLL adjustment need to succesfully
>    enumerate PCIe endpoints on extension connector (tested with
>    Pineboards AI Bundle + NVME SSD adapter card).
>  - Re-introduced brcm,msi-offset DT private property for MIP
>    interrupt-controller (without it I'm anable to use the interrupts
>    of adapter cards on PCIe enxtension connector).
> 
> For more info check patches.
> 
> [1] https://patchwork.kernel.org/project/linux-pci/cover/20240910151845.17308-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: mip: Add Broadcom bcm2712 MSI-X interrupt controller
>   PCI: brcmstb: Expand inbound size calculation helper
>   PCI: brcmstb: Enable external MSI-X if available
>   PCI: brcmstb: Avoid turn off of bridge reset
>   PCI: brcmstb: Add bcm2712 support
>   PCI: brcmstb: Reuse config structure
>   PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
>   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           |   5 +-
>  .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   8 +
>  arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 160 +++++++++
>  drivers/irqchip/Kconfig                       |  16 +
>  drivers/irqchip/Makefile                      |   1 +
>  drivers/irqchip/irq-bcm2712-mip.c             | 308 ++++++++++++++++++
>  drivers/pci/controller/pcie-brcmstb.c         | 197 ++++++++---
>  8 files changed, 707 insertions(+), 48 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
>  create mode 100644 drivers/irqchip/irq-bcm2712-mip.c
> 
> --
> 2.43.0
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y broadcom/bcm2712-rpi-5-b.dtb' for 20241014130710.413-1-svarbanov@suse.de:

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@100000: resets: [[12, 42], [13]] is too short
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@100000: reset-names:0: 'rescal' was expected
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@100000: reset-names:1: 'bridge' was expected
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@100000: reset-names: ['bridge', 'rescal'] is too short
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: 'msi-controller' is a required property
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: resets: [[12, 43], [13]] is too short
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: reset-names:0: 'rescal' was expected
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: reset-names:1: 'bridge' was expected
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: reset-names: ['bridge', 'rescal'] is too short
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: 'msi-controller' is a required property
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: resets: [[12, 44], [13]] is too short
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: reset-names:0: 'rescal' was expected
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: reset-names:1: 'bridge' was expected
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: reset-names: ['bridge', 'rescal'] is too short
	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#

Re: [PATCH v3 00/11] Add PCIe support for bcm2712

[Stanimir Varbanov]

On 10/14/24 17:05, Rob Herring (Arm) wrote:
> 
> On Mon, 14 Oct 2024 16:06:59 +0300, Stanimir Varbanov wrote:
>> Hello,
>>
>> Here is v3 the series to add support for PCIe on bcm2712 SoC
>> used by RPi5. Previous v2 can be found at [1].
>>
>> v2 -> v3 changes include:
>>  - Added Reviewed-by/Acked-by tags.
>>  - MIP MSI-X driver has been converted to MSI parent.
>>  - Added a new patch for PHY PLL adjustment need to succesfully
>>    enumerate PCIe endpoints on extension connector (tested with
>>    Pineboards AI Bundle + NVME SSD adapter card).
>>  - Re-introduced brcm,msi-offset DT private property for MIP
>>    interrupt-controller (without it I'm anable to use the interrupts
>>    of adapter cards on PCIe enxtension connector).
>>
>> For more info check patches.
>>
>> [1] https://patchwork.kernel.org/project/linux-pci/cover/20240910151845.17308-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: mip: Add Broadcom bcm2712 MSI-X interrupt controller
>>   PCI: brcmstb: Expand inbound size calculation helper
>>   PCI: brcmstb: Enable external MSI-X if available
>>   PCI: brcmstb: Avoid turn off of bridge reset
>>   PCI: brcmstb: Add bcm2712 support
>>   PCI: brcmstb: Reuse config structure
>>   PCI: brcmstb: Adjust PHY PLL setup to use a 54MHz input refclk
>>   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           |   5 +-
>>  .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   8 +
>>  arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 160 +++++++++
>>  drivers/irqchip/Kconfig                       |  16 +
>>  drivers/irqchip/Makefile                      |   1 +
>>  drivers/irqchip/irq-bcm2712-mip.c             | 308 ++++++++++++++++++
>>  drivers/pci/controller/pcie-brcmstb.c         | 197 ++++++++---
>>  8 files changed, 707 insertions(+), 48 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
>>  create mode 100644 drivers/irqchip/irq-bcm2712-mip.c
>>
>> --
>> 2.43.0
>>
>>
>>
> 
> 
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>   pip3 install dtschema --upgrade
> 
> 
> New warnings running 'make CHECK_DTBS=y broadcom/bcm2712-rpi-5-b.dtb' for 20241014130710.413-1-svarbanov@suse.de:

Sorry about that. I forgot to update brcm,stb-pcie.yaml schema for
number of resets.

~Stan

> 
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@100000: resets: [[12, 42], [13]] is too short
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@100000: reset-names:0: 'rescal' was expected
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@100000: reset-names:1: 'bridge' was expected
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@100000: reset-names: ['bridge', 'rescal'] is too short
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: 'msi-controller' is a required property
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: resets: [[12, 43], [13]] is too short
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: reset-names:0: 'rescal' was expected
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: reset-names:1: 'bridge' was expected
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@110000: reset-names: ['bridge', 'rescal'] is too short
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: 'msi-controller' is a required property
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: resets: [[12, 44], [13]] is too short
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: reset-names:0: 'rescal' was expected
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: reset-names:1: 'bridge' was expected
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: pcie@120000: reset-names: ['bridge', 'rescal'] is too short
> 	from schema $id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
> 
> 
> 
> 
>