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

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

[Stanimir Varbanov]

Hello,

Here is a v2 of adding PCIe support for bcm2712 (RPi5), the fisrt
version can be found at [1].

v2 is based on linux-next plus latest changes in pcie-brcmstb driver
[2]. The changes recently made by Jim leaded to a simplified patchset
for bcm2712 enablement coparing with previous version of this series.

Noticeable changes are:

 - Use of msi-range property in the MIP MSI-X controller and DT which
 make possible to avoid few private DT properties. The other noticeable
 change is moving of msi-pci-addr private property to a second 'reg'
 region. I'll appreciate comments on this.

 - Now the PCIe DT nodes are on separate axi{} simple-bus because adding
 it on soc{} adds too much churn in the node (Florian).

 - Added 'quirks' field in pcie_cfg_data to work around an issue (hw bug?)
 with bridge_reset on bcm2712 SoC.

regards,
~Stan

[1] https://patchwork.kernel.org/project/linux-pci/cover/20240626104544.14233-1-svarbanov@suse.de/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/brcmstb

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: Restore CRS in RootCtl after prstn_n
  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
  arm64: dts: broadcom: bcm2712: Add PCIe DT nodes
  arm64: dts: broadcom: bcm2712-rpi-5-b: Enable PCIe DT nodes

 .../brcm,bcm2712-msix.yaml                    |  69 ++++
 .../bindings/pci/brcm,stb-pcie.yaml           |   5 +-
 .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   8 +
 arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 166 ++++++++++
 drivers/irqchip/Kconfig                       |  12 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-bcm2712-mip.c             | 310 ++++++++++++++++++
 drivers/pci/controller/pcie-brcmstb.c         | 172 +++++++---
 8 files changed, 694 insertions(+), 49 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
 create mode 100644 drivers/irqchip/irq-bcm2712-mip.c

-- 
2.35.3

[PATCH v2 -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>
---
 .../brcm,bcm2712-msix.yaml                    | 69 +++++++++++++++++++
 1 file changed, 69 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..2b53dfa7c25e
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
@@ -0,0 +1,69 @@
+# 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 registers address
+      - description: pcie message address
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  msi-controller: true
+
+  "#msi-cells":
+    enum: [0]
+
+  msi-ranges: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - "#interrupt-cells"
+  - 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>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+            msi-ranges = <&gicv2 GIC_SPI 128 IRQ_TYPE_EDGE_RISING 64>;
+        };
+    };
-- 
2.35.3

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

[Rob Herring]

On Tue, Sep 10, 2024 at 06:18:35PM +0300, Stanimir Varbanov wrote:
> Adds DT bindings for bcm2712 MSI-X interrupt peripheral controller.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
>  .../brcm,bcm2712-msix.yaml                    | 69 +++++++++++++++++++
>  1 file changed, 69 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..2b53dfa7c25e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
> @@ -0,0 +1,69 @@
> +# 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: >

Don't need '>' here.

> +  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 registers address
> +      - description: pcie message address
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2

What goes in these cells?

But really, what interrupts does an MSI controller handle? Or are we 
just putting "interrupt-controller" in here so that kernel handles this 
with IRQCHIP_DECLARE()?

> +
> +  msi-controller: true

Drop and use 'unevaluatedProperties'.

> +
> +  "#msi-cells":
> +    enum: [0]

const: 0

> +
> +  msi-ranges: true

Drop.

> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - 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>;
> +            interrupt-controller;
> +            #interrupt-cells = <2>;
> +            msi-ranges = <&gicv2 GIC_SPI 128 IRQ_TYPE_EDGE_RISING 64>;
> +        };
> +    };
> -- 
> 2.35.3
> 

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

[Stanimir Varbanov]

Hi Rob,

Thank you for the review comments!

On 9/11/24 19:56, Rob Herring wrote:
> On Tue, Sep 10, 2024 at 06:18:35PM +0300, Stanimir Varbanov wrote:
>> Adds DT bindings for bcm2712 MSI-X interrupt peripheral controller.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>> ---
>>  .../brcm,bcm2712-msix.yaml                    | 69 +++++++++++++++++++
>>  1 file changed, 69 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..2b53dfa7c25e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
>> @@ -0,0 +1,69 @@
>> +# 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: >
> 
> Don't need '>' here.

OK.

> 
>> +  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 registers address
>> +      - description: pcie message address
>> +
>> +  interrupt-controller: true
>> +
>> +  "#interrupt-cells":
>> +    const: 2
> 
> What goes in these cells?
> 
> But really, what interrupts does an MSI controller handle? Or are we 
> just putting "interrupt-controller" in here so that kernel handles this 
> with IRQCHIP_DECLARE()?
> 

Yes, looks like interrupt-controller property is need by IRQCHIP_DECLARE().

I will drop interrupt-controller/cells and convert the driver to use
IRQCHIP_PLATFORM_DRIVER_BEGIN/END().

>> +
>> +  msi-controller: true
> 
> Drop and use 'unevaluatedProperties'.

OK.

> 
>> +
>> +  "#msi-cells":
>> +    enum: [0]
> 
> const: 0

OK.

> 
>> +
>> +  msi-ranges: true
> 
> Drop.

OK.

> 
>> +
>> +additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupt-controller
>> +  - "#interrupt-cells"
>> +  - 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>;
>> +            interrupt-controller;
>> +            #interrupt-cells = <2>;
>> +            msi-ranges = <&gicv2 GIC_SPI 128 IRQ_TYPE_EDGE_RISING 64>;
>> +        };
>> +    };
>> -- 
>> 2.35.3
>>

[PATCH v2 -next 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>
---
 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.35.3

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

[Rob Herring (Arm)]

On Tue, 10 Sep 2024 18:18:36 +0300, Stanimir Varbanov wrote:
> Update brcmstb PCIe controller bindings with bcm2712 compatible
> and add new resets.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
>  Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

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

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

[Florian Fainelli]

On 9/10/24 08:18, Stanimir Varbanov wrote:
> 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>
-- 
Florian

[PATCH v2 -next 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>
---
 drivers/irqchip/Kconfig           |  12 ++
 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-bcm2712-mip.c | 310 ++++++++++++++++++++++++++++++
 3 files changed, 323 insertions(+)
 create mode 100644 drivers/irqchip/irq-bcm2712-mip.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 341cd9ca5a05..49b18da4d237 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -116,6 +116,18 @@ config I8259
 	bool
 	select IRQ_DOMAIN
 
+config BCM2712_MIP
+	bool "Broadcom BCM2712 MSI-X Interrupt Peripheral support"
+	depends on ARCH_BRCMSTB
+	default ARCH_BRCMSTB
+	depends on ARM_GIC
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN
+	help
+	  Enable support for the Broadcom BCM2712 MSI-X target peripheral
+	  (MIP) needed by PCIe brcmstb to handle MSI-X interrupts on
+	  Raspberry Pi 5.
+
 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..51323e8cbe97
--- /dev/null
+++ b/drivers/irqchip/irq-bcm2712-mip.c
@@ -0,0 +1,310 @@
+// 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.h>
+#include <linux/of_address.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 {
+	/* used to protect bitmap alloc/free */
+	spinlock_t lock;
+	void __iomem *base;
+	u64 msg_addr;
+	u32 msi_base;
+	u32 num_msis;
+	unsigned long *bitmap;
+	struct irq_domain *parent;
+};
+
+static void mip_mask_msi_irq(struct irq_data *d)
+{
+	pci_msi_mask_irq(d);
+	irq_chip_mask_parent(d);
+}
+
+static void mip_unmask_msi_irq(struct irq_data *d)
+{
+	pci_msi_unmask_irq(d);
+	irq_chip_unmask_parent(d);
+}
+
+static struct irq_chip mip_msi_irq_chip = {
+	.name			= "MIP-MSI",
+	.irq_unmask		= mip_unmask_msi_irq,
+	.irq_mask		= mip_mask_msi_irq,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+};
+
+static struct msi_domain_info mip_msi_domain_info = {
+	.flags	= (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
+		   MSI_FLAG_PCI_MSIX),
+	.chip	= &mip_msi_irq_chip,
+};
+
+static void mip_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
+{
+	struct mip_priv *priv = irq_data_get_irq_chip_data(d);
+
+	msg->address_hi = upper_32_bits(priv->msg_addr);
+	msg->address_lo = lower_32_bits(priv->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 *priv, unsigned int nr_irqs,
+			   unsigned int *hwirq)
+{
+	int bit;
+
+	spin_lock(&priv->lock);
+	bit = bitmap_find_free_region(priv->bitmap, priv->num_msis,
+				      ilog2(nr_irqs));
+	spin_unlock(&priv->lock);
+
+	if (bit < 0)
+		return bit;
+
+	if (hwirq)
+		*hwirq = bit + priv->msi_base;
+
+	return 0;
+}
+
+static void mip_free_hwirq(struct mip_priv *priv, unsigned int hwirq,
+			   unsigned int nr_irqs)
+{
+	unsigned int irq = hwirq - priv->msi_base;
+
+	if (hwirq < priv->msi_base) {
+		pr_err("MIP: hwirq must be greater than %u\n", priv->msi_base);
+		return;
+	}
+
+	spin_lock(&priv->lock);
+	bitmap_release_region(priv->bitmap, irq, ilog2(nr_irqs));
+	spin_unlock(&priv->lock);
+}
+
+static int mip_parent_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				   int hwirq)
+{
+	struct irq_fwspec fwspec = {0};
+	struct irq_data *irqd;
+	int ret;
+
+	if (!is_of_node(domain->parent->fwnode))
+		return -EINVAL;
+
+	fwspec.fwnode = domain->parent->fwnode;
+	fwspec.param_count = 3;
+	fwspec.param[0] = 0;
+	fwspec.param[1] = hwirq;
+	fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
+	if (ret)
+		return ret;
+
+	irqd = irq_domain_get_irq_data(domain->parent, virq);
+	if (irqd)
+		irqd->chip->irq_set_type(irqd, IRQ_TYPE_EDGE_RISING);
+
+	return 0;
+}
+
+static int mip_middle_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				   unsigned int nr_irqs, void *arg)
+{
+	struct mip_priv *priv = domain->host_data;
+	struct irq_data *irqd;
+	unsigned int hwirq, i;
+	int ret;
+
+	ret = mip_alloc_hwirq(priv, nr_irqs, &hwirq);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < nr_irqs; i++) {
+		ret = mip_parent_domain_alloc(domain, virq + i, hwirq + i);
+		if (ret)
+			goto err_free;
+
+		ret = irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+						    &mip_middle_irq_chip, priv);
+		if (ret)
+			goto err_free;
+
+		irqd = irq_domain_get_irq_data(domain->parent, virq + i);
+		if (irqd) {
+			irqd_set_single_target(irqd);
+			irqd_set_affinity_on_activate(irqd);
+		}
+	}
+
+	return 0;
+
+err_free:
+	irq_domain_free_irqs_parent(domain, virq, i);
+	mip_free_hwirq(priv, hwirq, 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 *priv;
+
+	if (!irqd)
+		return;
+
+	priv = irq_data_get_irq_chip_data(irqd);
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+	mip_free_hwirq(priv, irqd->hwirq, nr_irqs);
+}
+
+static const struct irq_domain_ops mip_middle_domain_ops = {
+	.alloc	= mip_middle_domain_alloc,
+	.free	= mip_middle_domain_free,
+};
+
+static int mip_init_domains(struct mip_priv *priv, struct device_node *np)
+{
+	struct irq_domain *middle_domain, *msi_domain;
+
+	middle_domain = irq_domain_add_hierarchy(priv->parent, 0,
+						 priv->num_msis, np,
+						 &mip_middle_domain_ops,
+						 priv);
+	if (!middle_domain)
+		return -ENOMEM;
+
+	msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(np),
+					       &mip_msi_domain_info,
+					       middle_domain);
+	if (!msi_domain) {
+		irq_domain_remove(middle_domain);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int mip_parse_dt(struct mip_priv *priv, struct device_node *np)
+{
+	struct of_phandle_args args;
+	u64 size;
+	int ret;
+
+	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,
+					 &priv->num_msis);
+	if (ret)
+		goto err_put;
+
+	ret = of_property_read_reg(np, 1, &priv->msg_addr, &size);
+	if (ret)
+		goto err_put;
+
+	priv->msi_base = args.args[1];
+
+	priv->parent = irq_find_host(args.np);
+	if (!priv->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 mip_priv *priv;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	spin_lock_init(&priv->lock);
+
+	ret = mip_parse_dt(priv, node);
+	if (ret)
+		goto err_priv;
+
+	priv->base = of_iomap(node, 0);
+	if (!priv->base) {
+		ret = -ENXIO;
+		goto err_priv;
+	}
+
+	priv->bitmap = bitmap_zalloc(priv->num_msis, GFP_KERNEL);
+	if (!priv->bitmap) {
+		ret = -ENOMEM;
+		goto err_base;
+	}
+
+	/*
+	 * All MSI-X masked in for the host, masked out for the
+	 * VPU, and edge-triggered.
+	 */
+	writel(0, priv->base + MIP_INT_MASKL_HOST);
+	writel(0, priv->base + MIP_INT_MASKH_HOST);
+	writel(~0, priv->base + MIP_INT_MASKL_VPU);
+	writel(~0, priv->base + MIP_INT_MASKH_VPU);
+	writel(~0, priv->base + MIP_INT_CFGL_HOST);
+	writel(~0, priv->base + MIP_INT_CFGH_HOST);
+
+	ret = mip_init_domains(priv, node);
+	if (ret)
+		goto err_map;
+
+	pr_info("MIP: registered %u MSI-X, starting at %u\n", priv->num_msis,
+		priv->msi_base);
+
+	return 0;
+
+err_map:
+	bitmap_free(priv->bitmap);
+err_base:
+	iounmap(priv->base);
+err_priv:
+	kfree(priv);
+	return ret;
+}
+
+IRQCHIP_DECLARE(bcm_mip, "brcm,bcm2712-mip", mip_of_msi_init);
-- 
2.35.3

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

[Thomas Gleixner]

On Tue, Sep 10 2024 at 18:18, Stanimir Varbanov wrote:
> +
> +struct mip_priv {
> +	/* used to protect bitmap alloc/free */
> +	spinlock_t lock;
> +	void __iomem *base;
> +	u64 msg_addr;
> +	u32 msi_base;
> +	u32 num_msis;
> +	unsigned long *bitmap;
> +	struct irq_domain *parent;

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers

And please read the rest of the document too.

> +};
> +
> +static void mip_mask_msi_irq(struct irq_data *d)
> +{
> +	pci_msi_mask_irq(d);
> +	irq_chip_mask_parent(d);
> +}
> +
> +static void mip_unmask_msi_irq(struct irq_data *d)
> +{
> +	pci_msi_unmask_irq(d);
> +	irq_chip_unmask_parent(d);

This is asymmetric vs. mask(), but that's just the usual copy & pasta
problem.

> +}
> +static int mip_init_domains(struct mip_priv *priv, struct device_node *np)
> +{
> +	struct irq_domain *middle_domain, *msi_domain;
> +
> +	middle_domain = irq_domain_add_hierarchy(priv->parent, 0,
> +						 priv->num_msis, np,
> +						 &mip_middle_domain_ops,
> +						 priv);
> +	if (!middle_domain)
> +		return -ENOMEM;
> +
> +	msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(np),
> +					       &mip_msi_domain_info,
> +					       middle_domain);
> +	if (!msi_domain) {
> +		irq_domain_remove(middle_domain);
> +		return -ENOMEM;
> +	}

This is not much different. Please convert this to a proper MSI parent
domain and let the PCI/MSI core handle the PCI/MSI part.

See

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=irq-msi-2024-07-22

for reference.

Thanks,

        tglx

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

[Stanimir Varbanov]

Hi Thomas,

Thank you for the comments!

On 9/10/24 18:58, Thomas Gleixner wrote:
> On Tue, Sep 10 2024 at 18:18, Stanimir Varbanov wrote:
>> +
>> +struct mip_priv {
>> +	/* used to protect bitmap alloc/free */
>> +	spinlock_t lock;
>> +	void __iomem *base;
>> +	u64 msg_addr;
>> +	u32 msi_base;
>> +	u32 num_msis;
>> +	unsigned long *bitmap;
>> +	struct irq_domain *parent;
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers
> 
> And please read the rest of the document too.

Sure.

> 
>> +};
>> +
>> +static void mip_mask_msi_irq(struct irq_data *d)
>> +{
>> +	pci_msi_mask_irq(d);
>> +	irq_chip_mask_parent(d);
>> +}
>> +
>> +static void mip_unmask_msi_irq(struct irq_data *d)
>> +{
>> +	pci_msi_unmask_irq(d);
>> +	irq_chip_unmask_parent(d);
> 
> This is asymmetric vs. mask(), but that's just the usual copy & pasta
> problem.

Correct, but this will disappear when convert to MSI parent.

> 
>> +}
>> +static int mip_init_domains(struct mip_priv *priv, struct device_node *np)
>> +{
>> +	struct irq_domain *middle_domain, *msi_domain;
>> +
>> +	middle_domain = irq_domain_add_hierarchy(priv->parent, 0,
>> +						 priv->num_msis, np,
>> +						 &mip_middle_domain_ops,
>> +						 priv);
>> +	if (!middle_domain)
>> +		return -ENOMEM;
>> +
>> +	msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(np),
>> +					       &mip_msi_domain_info,
>> +					       middle_domain);
>> +	if (!msi_domain) {
>> +		irq_domain_remove(middle_domain);
>> +		return -ENOMEM;
>> +	}
> 
> This is not much different. Please convert this to a proper MSI parent
> domain and let the PCI/MSI core handle the PCI/MSI part.

I apologize, but I wasn't able to make it work on time. The good news is
that I made it now and will send it in next version of the series.

regards,
~Stan

[PATCH v2 -next 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Stanimir Varbanov]

Expand the inbound size calculation in helper function
up to 64GB.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
 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 29ad04f8d3f3..7bd85566c242 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.35.3

Re: [PATCH v2 -next 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Florian Fainelli]

On 9/10/24 08:18, Stanimir Varbanov wrote:
> Expand the inbound size calculation in helper function
> up to 64GB.

Nit, we could explain why, which is that BCM2712's memory map supports 
up to 64GB of DRAM. With that:

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

Re: [PATCH v2 -next 04/11] PCI: brcmstb: Expand inbound size calculation helper

[Stanimir Varbanov]

On 9/10/24 19:59, Florian Fainelli wrote:
> On 9/10/24 08:18, Stanimir Varbanov wrote:
>> Expand the inbound size calculation in helper function
>> up to 64GB.
> 
> Nit, we could explain why, which is that BCM2712's memory map supports
> up to 64GB of DRAM. With that:

Sure.

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

Thank you!

regards,
~Stan

[PATCH v2 -next 05/11] PCI: brcmstb: Restore CRS in RootCtl after prstn_n

[Stanimir Varbanov]

RootCtl bits might reset by perst_n during probe, re-enable
CRS SVE here in pcie_start_link.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
 drivers/pci/controller/pcie-brcmstb.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 7bd85566c242..f2a7a8e93a74 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1271,7 +1271,7 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
 	void __iomem *base = pcie->base;
-	u16 nlw, cls, lnksta;
+	u16 nlw, cls, lnksta, tmp16;
 	bool ssc_good = false;
 	int ret, i;
 
@@ -1319,6 +1319,17 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
 		 pci_speed_string(pcie_link_speed[cls]), nlw,
 		 ssc_good ? "(SSC)" : "(!SSC)");
 
+	/*
+	 * RootCtl bits are reset by perst_n, which undoes pci_enable_crs()
+	 * called prior to pci_add_new_bus() during probe. Re-enable here.
+	 */
+	tmp16 = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_RTCAP);
+	if (tmp16 & PCI_EXP_RTCAP_CRSVIS) {
+		tmp16 = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_RTCTL);
+		u16p_replace_bits(&tmp16, 1, PCI_EXP_RTCTL_CRSSVE);
+		writew(tmp16, base + BRCM_PCIE_CAP_REGS + PCI_EXP_RTCTL);
+	}
+
 	return 0;
 }
 
-- 
2.35.3

Re: [PATCH v2 -next 05/11] PCI: brcmstb: Restore CRS in RootCtl after prstn_n

[Florian Fainelli]

On 9/10/24 08:18, Stanimir Varbanov wrote:
> RootCtl bits might reset by perst_n during probe, re-enable
> CRS SVE here in pcie_start_link.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>

This looks like a bug fix, and we should explain what is the user 
visible effect of that, if any.
-- 
Florian

Re: [PATCH v2 -next 05/11] PCI: brcmstb: Restore CRS in RootCtl after prstn_n

[Stanimir Varbanov]

Hi Florian,

On 9/10/24 19:59, Florian Fainelli wrote:
> On 9/10/24 08:18, Stanimir Varbanov wrote:
>> RootCtl bits might reset by perst_n during probe, re-enable
>> CRS SVE here in pcie_start_link.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> 
> This looks like a bug fix, and we should explain what is the user
> visible effect of that, if any.

It is definitely a bugfix. Otherwise, CRS Software Visibility is
important feature from pcie1.1. Not enabling it on Root Port could lead
to infinite configuration retry cycles when enumerate endpoints which
supports CRS. For more information [1] and [2].

I spent some time debugging it and found that this is not the proper
solution.  I think the issue comes from wrongly implemented .add_bus
pci_ops. Looks like .add_bus op shouldn't call brcm_pcie_start_link()
but invoke before pci_host_probe(), then the issue will fix by itself.

What I observed is that pci_enable_crs() is setting CSR Software
Visibility Enable bit but the controller is ignoring it without error
(reading the Root Control register returns zero). This means that the
controller is not ready to accept configuration write requests at that
time, that's why I tried the following diff which seems to work:

 static struct pci_ops brcm_pcie_ops = {
        .map_bus = brcm_pcie_map_bus,
        .read = pci_generic_config_read,
        .write = pci_generic_config_write,
-       .add_bus = brcm_pcie_add_bus,
-       .remove_bus = brcm_pcie_remove_bus,
 };

 static struct pci_ops brcm7425_pcie_ops = {
@@ -1983,6 +2018,9 @@ static int brcm_pcie_probe(struct platform_device
*pdev)

        platform_set_drvdata(pdev, pcie);

+       //TODO: check for error
+       brcm_pcie_start_link(pcie);
+
        ret = pci_host_probe(bridge);
        if (!ret && !brcm_pcie_link_up(pcie))
                ret = -ENODEV;

Of course this change would work on RPi5 because there are no regulators.

I will drop the patch from the series for now and work on a proper solution.

regards,
~Stan

[1]
https://patchwork.kernel.org/project/linux-pci/patch/53FFA54D.9000907@gmail.com/
[2]
https://blog.linuxplumbersconf.org/2017/ocw/system/presentations/4732/original/crs.pdf

[PATCH v2 -next 06/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>
---
 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 f2a7a8e93a74..d78f33b33884 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1333,6 +1333,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",
@@ -1898,11 +1944,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.35.3

[PATCH v2 -next 07/11] PCI: brcmstb: Avoid turn off of bridge reset

[Stanimir Varbanov]

On brcm_pcie_turn_off avoid shutdown of bridge reset.

Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
---
 drivers/pci/controller/pcie-brcmstb.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index d78f33b33884..185ccf7fe86a 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -234,10 +234,17 @@ struct inbound_win {
 	u64 cpu_addr;
 };
 
+/*
+ * Shutting down this bridge on pcie1 means accesses to rescal block
+ * will hang the chip if another RC wants to assert/deassert rescal.
+ */
+#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 +297,7 @@ struct brcm_pcie {
 	struct subdev_regulators *sr;
 	bool			ep_wakeup_capable;
 	bool			has_phy;
+	u32			quirks;
 	u8			num_inbound_wins;
 };
 
@@ -1549,8 +1557,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;
 }
@@ -1864,6 +1873,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.35.3

Re: [PATCH v2 -next 07/11] PCI: brcmstb: Avoid turn off of bridge reset

[Florian Fainelli]

On 9/10/24 08:18, Stanimir Varbanov wrote:
> On brcm_pcie_turn_off avoid shutdown of bridge reset.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
>   drivers/pci/controller/pcie-brcmstb.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index d78f33b33884..185ccf7fe86a 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -234,10 +234,17 @@ struct inbound_win {
>   	u64 cpu_addr;
>   };
>   
> +/*
> + * Shutting down this bridge on pcie1 means accesses to rescal block
> + * will hang the chip if another RC wants to assert/deassert rescal.
> + */

Maybe a slightly more detailed comment saying that 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).

Thanks!
-- 
Florian

Re: [PATCH v2 -next 07/11] PCI: brcmstb: Avoid turn off of bridge reset

[Stanimir Varbanov]

On 9/10/24 20:03, Florian Fainelli wrote:
> On 9/10/24 08:18, Stanimir Varbanov wrote:
>> On brcm_pcie_turn_off avoid shutdown of bridge reset.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
>> ---
>>   drivers/pci/controller/pcie-brcmstb.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pcie-brcmstb.c
>> b/drivers/pci/controller/pcie-brcmstb.c
>> index d78f33b33884..185ccf7fe86a 100644
>> --- a/drivers/pci/controller/pcie-brcmstb.c
>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>> @@ -234,10 +234,17 @@ struct inbound_win {
>>       u64 cpu_addr;
>>   };
>>   +/*
>> + * Shutting down this bridge on pcie1 means accesses to rescal block
>> + * will hang the chip if another RC wants to assert/deassert rescal.
>> + */
> 
> Maybe a slightly more detailed comment saying that 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).

Ack. Thank you!

regards,
~Stan

[PATCH v2 -next 08/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>
---
 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 185ccf7fe86a..43d071d12201 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1769,6 +1769,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,
@@ -1820,6 +1829,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.35.3

Re: [PATCH v2 -next 08/11] PCI: brcmstb: Add bcm2712 support

[Florian Fainelli]

On 9/10/24 08:18, Stanimir Varbanov wrote:
> 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>
-- 
Florian

[PATCH v2 -next 09/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>
---
 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 43d071d12201..caf2b8b63f75 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
@@ -283,8 +283,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;
@@ -292,18 +290,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;
 }
 
 /*
@@ -863,7 +857,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) {
@@ -880,10 +874,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;
 	}
 
@@ -897,7 +891,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,
@@ -1020,7 +1014,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;
@@ -1043,15 +1037,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;
 		}
 	}
@@ -1059,7 +1053,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;
 
@@ -1079,9 +1073,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 */
@@ -1206,7 +1200,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 */
@@ -1284,7 +1278,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;
 
@@ -1527,12 +1521,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)
@@ -1543,7 +1537,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;
 
@@ -1557,9 +1551,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;
 }
@@ -1647,7 +1641,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));
@@ -1878,13 +1872,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))
@@ -1919,7 +1907,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);
@@ -1958,7 +1946,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;
@@ -1982,7 +1971,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.35.3

Re: [PATCH v2 -next 09/11] PCI: brcmstb: Reuse config structure

[Florian Fainelli]

On 9/10/24 08:18, Stanimir Varbanov 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>
-- 
Florian

[PATCH v2 -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>
---
 arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 166 ++++++++++++++++++++++
 1 file changed, 166 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
index 6e5a984c1d4e..9dd127d4c9a2 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,160 @@ 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 = <0x02000000 0x00 0x00000000
+				  0x17 0x00000000
+				  0x00 0xfffffffc>,
+				 <0x43000000 0x04 0x00000000
+				  0x14 0x00000000
+				  0x3 0x00000000>;
+
+			dma-ranges = <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 = <0x02000000 0x00 0x00000000
+				  0x1b 0x00000000
+				  0x00 0xfffffffc>,
+				 <0x43000000 0x04 0x00000000
+				  0x18 0x00000000
+				  0x03 0x00000000>;
+
+			dma-ranges = <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 = <0x02000000 0x00 0x00000000
+				  0x1f 0x00000000
+				  0x00 0xfffffffc>,
+				 <0x43000000 0x04 0x00000000
+				  0x1c 0x00000000
+				  0x03 0x00000000>;
+
+			dma-ranges = <0x02000000 0x00 0x00000000
+				      0x1f 0x00000000
+				      0x00 0x00400000>,
+				     <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;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			msi-ranges = <&gicv2 GIC_SPI 128 IRQ_TYPE_EDGE_RISING 64>;
+		};
+
+		mip1: msi-controller@131000 {
+			compatible = "brcm,bcm2712-mip";
+			reg = <0x00 0x00131000 0x00 0xc0>,
+			      <0xff 0xffffe000 0x00 0x1000>;
+			msi-controller;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			msi-ranges = <&gicv2 GIC_SPI 255 IRQ_TYPE_EDGE_RISING 8>;
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
-- 
2.35.3

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

[Florian Fainelli]

On 9/10/24 08:18, Stanimir Varbanov wrote:
> Add PCIe devicetree nodes, plus needed reset and mip MSI-X
> controllers.
> 
> Signed-off-by: Stanimir Varbanov <svarbanov@suse.de>
> ---
>   arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 166 ++++++++++++++++++++++
>   1 file changed, 166 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
> index 6e5a984c1d4e..9dd127d4c9a2 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,160 @@ 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 = <0x02000000 0x00 0x00000000
> +				  0x17 0x00000000
> +				  0x00 0xfffffffc>,
> +				 <0x43000000 0x04 0x00000000
> +				  0x14 0x00000000
> +				  0x3 0x00000000>;

Maybe a comment would help in figuring out what these ranges describe, 
and we could probably do a single line cell:

> +
> +			dma-ranges = <0x43000000 0x10 0x00000000

Likewise

> +				      0x00 0x00000000
> +				      0x10 0x00000000>;
> +
> +			status = "disabled";

Other than that, this LGTM, assume the dt-bindings maintainer are also 
OK with that.
-- 
Florian

[PATCH v2 -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>
---
 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.35.3

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

[Rob Herring (Arm)]

On Tue, 10 Sep 2024 18:18:34 +0300, Stanimir Varbanov wrote:
> Hello,
> 
> Here is a v2 of adding PCIe support for bcm2712 (RPi5), the fisrt
> version can be found at [1].
> 
> v2 is based on linux-next plus latest changes in pcie-brcmstb driver
> [2]. The changes recently made by Jim leaded to a simplified patchset
> for bcm2712 enablement coparing with previous version of this series.
> 
> Noticeable changes are:
> 
>  - Use of msi-range property in the MIP MSI-X controller and DT which
>  make possible to avoid few private DT properties. The other noticeable
>  change is moving of msi-pci-addr private property to a second 'reg'
>  region. I'll appreciate comments on this.
> 
>  - Now the PCIe DT nodes are on separate axi{} simple-bus because adding
>  it on soc{} adds too much churn in the node (Florian).
> 
>  - Added 'quirks' field in pcie_cfg_data to work around an issue (hw bug?)
>  with bridge_reset on bcm2712 SoC.
> 
> regards,
> ~Stan
> 
> [1] https://patchwork.kernel.org/project/linux-pci/cover/20240626104544.14233-1-svarbanov@suse.de/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/brcmstb
> 
> 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: Restore CRS in RootCtl after prstn_n
>   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
>   arm64: dts: broadcom: bcm2712: Add PCIe DT nodes
>   arm64: dts: broadcom: bcm2712-rpi-5-b: Enable PCIe DT nodes
> 
>  .../brcm,bcm2712-msix.yaml                    |  69 ++++
>  .../bindings/pci/brcm,stb-pcie.yaml           |   5 +-
>  .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     |   8 +
>  arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 166 ++++++++++
>  drivers/irqchip/Kconfig                       |  12 +
>  drivers/irqchip/Makefile                      |   1 +
>  drivers/irqchip/irq-bcm2712-mip.c             | 310 ++++++++++++++++++
>  drivers/pci/controller/pcie-brcmstb.c         | 172 +++++++---
>  8 files changed, 694 insertions(+), 49 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2712-msix.yaml
>  create mode 100644 drivers/irqchip/irq-bcm2712-mip.c
> 
> --
> 2.35.3
> 
> 
> 


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 20240910151845.17308-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#