public inbox for linux-mediatek@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] PCI: mediatek: Add support for EcoNet SoCs
@ 2026-03-16 15:51 Caleb James DeLisle
  2026-03-16 15:51 ` [PATCH v2 1/3] dt-bindings: PCI: mediatek: Add support for EcoNet EN7528 Caleb James DeLisle
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Caleb James DeLisle @ 2026-03-16 15:51 UTC (permalink / raw)
  To: linux-pci
  Cc: linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel, Caleb James DeLisle

Add EcoNet EN7528 (and EN751221) PCIe support.

Changes from v1:
* mediatek-pcie.yaml slot0 needs device-type = "pci", fix dt_binding_check
Link: https://lore.kernel.org/linux-mips/177334026016.3889069.9474337544951486443.robh@kernel.org

This was split from a larger PCIe patchset which crossed multiple
subsystems. I'm not labeling this a v3 because it's a new patchset, but
I'm keeping the historical record anyway.

Changes from econet-pcie v2:
* mediatek-pcie.yaml add missing constraints to PCI node properties
* econet-pcie v2: https://lore.kernel.org/linux-mips/20260309131818.74467-1-cjd@cjdns.fr

Changes from econet-pcie v1:
* pcie-mediatek.c Exclude pcie_retrain_link() when building as a module
* econet-pcie v1: https://lore.kernel.org/linux-mips/20260303190948.694783-1-cjd@cjdns.fr/


Caleb James DeLisle (3):
  dt-bindings: PCI: mediatek: Add support for EcoNet EN7528
  PCI: mediatek: Add support for EcoNet EN7528 SoC
  PCI: Skip bridge window reads when window is not supported

 .../bindings/pci/mediatek-pcie.yaml           |  82 ++++++++++++
 drivers/pci/controller/Kconfig                |   2 +-
 drivers/pci/controller/pcie-mediatek.c        | 118 ++++++++++++++++++
 drivers/pci/probe.c                           |   6 +
 4 files changed, 207 insertions(+), 1 deletion(-)


base-commit: 3fa5e5702a82d259897bd7e209469bc06368bf31
-- 
2.39.5



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/3] dt-bindings: PCI: mediatek: Add support for EcoNet EN7528
  2026-03-16 15:51 [PATCH v2 0/3] PCI: mediatek: Add support for EcoNet SoCs Caleb James DeLisle
@ 2026-03-16 15:51 ` Caleb James DeLisle
  2026-03-17  7:26   ` Krzysztof Kozlowski
  2026-03-16 15:51 ` [PATCH v2 2/3] PCI: mediatek: Add support for EcoNet EN7528 SoC Caleb James DeLisle
  2026-03-16 15:51 ` [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported Caleb James DeLisle
  2 siblings, 1 reply; 13+ messages in thread
From: Caleb James DeLisle @ 2026-03-16 15:51 UTC (permalink / raw)
  To: linux-pci
  Cc: linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel, Caleb James DeLisle

Introduce EcoNet EN7528 SoC compatible in MediaTek PCIe controller
binding.

EcoNet PCIe controller has the same configuration model as
Mediatek v2 but is initiallized more similarly to an MT7621
PCIe.

Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 .../bindings/pci/mediatek-pcie.yaml           | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie.yaml
index 0b8c78ec4f91..bac6e3c84752 100644
--- a/Documentation/devicetree/bindings/pci/mediatek-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie.yaml
@@ -14,6 +14,7 @@ properties:
     oneOf:
       - enum:
           - airoha,an7583-pcie
+          - econet,en7528-pcie
           - mediatek,mt2712-pcie
           - mediatek,mt7622-pcie
           - mediatek,mt7629-pcie
@@ -226,6 +227,31 @@ allOf:
 
         mediatek,pbus-csr: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: econet,en7528-pcie
+    then:
+      properties:
+        clocks:
+          maxItems: 1
+
+        clock-names:
+          maxItems: 1
+
+        reset: false
+
+        reset-names: false
+
+        power-domain: false
+
+        mediatek,pbus-csr: false
+
+      required:
+        - phys
+        - phy-names
+
 unevaluatedProperties: false
 
 examples:
@@ -436,3 +462,59 @@ examples:
             };
         };
     };
+
+  # EN7528
+  - |
+    #include <dt-bindings/interrupt-controller/mips-gic.h>
+    #include <dt-bindings/clock/en7523-clk.h>
+    #include <dt-bindings/reset/airoha,en7523-reset.h>
+
+    soc_3 {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        pcie@1fb81000 {
+          compatible = "econet,en7528-pcie";
+          device_type = "pci";
+          linux,pci-domain = <0>;
+          #address-cells = <3>;
+          #size-cells = <2>;
+
+          reg = <0x1fb81000 0x1000>;
+          reg-names = "port0";
+
+          clocks = <&scuclk EN7523_CLK_PCIE>;
+          clock-names = "sys_ck0";
+
+          phys = <&pcie_phy0>;
+          phy-names = "pcie-phy0";
+
+          ranges = <0x01000000 0 0x00000000 0x1f600000 0 0x00010000>,
+                   <0x82000000 0 0x20000000 0x20000000 0 0x08000000>;
+
+          interrupt-parent = <&intc>;
+          interrupts = <23>;
+          interrupt-names = "pcie_irq";
+          bus-range = <0x00 0xff>;
+          #interrupt-cells = <1>;
+          interrupt-map-mask = <0 0 0 7>;
+          interrupt-map = <0 0 0 1 &pcie_intc 0>,
+              <0 0 0 2 &pcie_intc 1>,
+              <0 0 0 3 &pcie_intc 2>,
+              <0 0 0 4 &pcie_intc 3>;
+
+          pcie_intc: interrupt-controller {
+            interrupt-controller;
+            #address-cells = <0>;
+            #interrupt-cells = <1>;
+          };
+
+          slot0: pcie@0,0 {
+            device_type = "pci";
+            reg = <0x0000 0 0 0 0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+          };
+        };
+    };
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/3] PCI: mediatek: Add support for EcoNet EN7528 SoC
  2026-03-16 15:51 [PATCH v2 0/3] PCI: mediatek: Add support for EcoNet SoCs Caleb James DeLisle
  2026-03-16 15:51 ` [PATCH v2 1/3] dt-bindings: PCI: mediatek: Add support for EcoNet EN7528 Caleb James DeLisle
@ 2026-03-16 15:51 ` Caleb James DeLisle
  2026-03-16 15:51 ` [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported Caleb James DeLisle
  2 siblings, 0 replies; 13+ messages in thread
From: Caleb James DeLisle @ 2026-03-16 15:51 UTC (permalink / raw)
  To: linux-pci
  Cc: linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel, Caleb James DeLisle

Add support for the PCIe present on the EcoNet EN7528 (and EN751221) SoCs.

These SoCs have a mix of Gen1 and Gen2 capable ports, but the Gen2 ports
require re-training after startup.

Co-developed-by: Ahmed Naseef <naseefkm@gmail.com>
Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
Co-developed-by: Caleb James DeLisle <cjd@cjdns.fr>
Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/pci/controller/Kconfig         |   2 +-
 drivers/pci/controller/pcie-mediatek.c | 118 +++++++++++++++++++++++++
 2 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 5aaed8ac6e44..f6a5fcacb38d 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -209,7 +209,7 @@ config PCI_MVEBU
 
 config PCIE_MEDIATEK
 	tristate "MediaTek PCIe controller"
-	depends on ARCH_AIROHA || ARCH_MEDIATEK || COMPILE_TEST
+	depends on ARCH_AIROHA || ARCH_MEDIATEK || ECONET || COMPILE_TEST
 	depends on OF
 	depends on PCI_MSI
 	select IRQ_MSI_LIB
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 5defa5cc4c2b..84064061652a 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -14,6 +14,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqchip/irq-msi-lib.h>
 #include <linux/irqdomain.h>
+#include <linux/kconfig.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/msi.h>
@@ -77,6 +78,7 @@
 
 #define PCIE_CONF_VEND_ID	0x100
 #define PCIE_CONF_DEVICE_ID	0x102
+#define PCIE_CONF_REV_CLASS	0x104
 #define PCIE_CONF_CLASS_ID	0x106
 
 #define PCIE_INT_MASK		0x420
@@ -89,6 +91,11 @@
 #define MSI_MASK		BIT(23)
 #define MTK_MSI_IRQS_NUM	32
 
+#define EN7528_HOST_MODE	0x00804201
+#define EN7528_LINKUP_REG	0x50
+#define EN7528_RC0_LINKUP	BIT(1)
+#define EN7528_RC1_LINKUP	BIT(2)
+
 #define PCIE_AHB_TRANS_BASE0_L	0x438
 #define PCIE_AHB_TRANS_BASE0_H	0x43c
 #define AHB2PCIE_SIZE(x)	((x) & GENMASK(4, 0))
@@ -753,6 +760,86 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 	return 0;
 }
 
+static int mtk_pcie_startup_port_en7528(struct mtk_pcie_port *port)
+{
+	struct mtk_pcie *pcie = port->pcie;
+	struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
+	struct resource *mem = NULL;
+	struct resource_entry *entry;
+	u32 val, link_mask;
+	int err;
+
+	entry = resource_list_first_type(&host->windows, IORESOURCE_MEM);
+	if (entry)
+		mem = entry->res;
+	if (!mem)
+		return -EINVAL;
+
+	if (!pcie->cfg) {
+		dev_err(pcie->dev, "EN7528: pciecfg syscon not available\n");
+		return -EINVAL;
+	}
+
+	/* Assert all reset signals */
+	writel(0, port->base + PCIE_RST_CTRL);
+
+	/*
+	 * Enable PCIe link down reset, if link status changed from link up to
+	 * link down, this will reset MAC control registers and configuration
+	 * space.
+	 */
+	writel(PCIE_LINKDOWN_RST_EN, port->base + PCIE_RST_CTRL);
+
+	/*
+	 * Described in PCIe CEM specification sections 2.2 (PERST# Signal) and
+	 * 2.2.1 (Initial Power-Up (G3 to S0)). The deassertion of PERST#
+	 * should be delayed 100ms (TPVPERL) for the power and clock to become
+	 * stable.
+	 */
+	msleep(100);
+
+	/* De-assert PHY, PE, PIPE, MAC and configuration reset */
+	val = readl(port->base + PCIE_RST_CTRL);
+	val |= PCIE_PHY_RSTB | PCIE_PERSTB | PCIE_PIPE_SRSTB |
+	       PCIE_MAC_SRSTB | PCIE_CRSTB;
+	writel(val, port->base + PCIE_RST_CTRL);
+
+	writel(PCIE_CLASS_CODE | PCIE_REVISION_ID,
+	       port->base + PCIE_CONF_REV_CLASS);
+	writel(EN7528_HOST_MODE, port->base);
+
+	link_mask = (port->slot == 0) ? EN7528_RC0_LINKUP : EN7528_RC1_LINKUP;
+
+	/* 100ms timeout value should be enough for Gen1/2 training */
+	err = regmap_read_poll_timeout(pcie->cfg, EN7528_LINKUP_REG, val,
+				       !!(val & link_mask), 20,
+				       100 * USEC_PER_MSEC);
+	if (err) {
+		dev_err(pcie->dev, "EN7528: port%d link timeout\n", port->slot);
+		return -ETIMEDOUT;
+	}
+
+	/* Set INTx mask */
+	val = readl(port->base + PCIE_INT_MASK);
+	val &= ~INTX_MASK;
+	writel(val, port->base + PCIE_INT_MASK);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		mtk_pcie_enable_msi(port);
+
+	/* Set AHB to PCIe translation windows */
+	val = lower_32_bits(mem->start) |
+	      AHB2PCIE_SIZE(fls(resource_size(mem)));
+	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
+
+	val = upper_32_bits(mem->start);
+	writel(val, port->base + PCIE_AHB_TRANS_BASE0_H);
+
+	writel(WIN_ENABLE, port->base + PCIE_AXI_WINDOW0);
+
+	return 0;
+}
+
 static void __iomem *mtk_pcie_map_bus(struct pci_bus *bus,
 				      unsigned int devfn, int where)
 {
@@ -1149,6 +1236,30 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	if (err)
 		goto put_resources;
 
+	/* Retrain Gen1 links to reach Gen2 where supported */
+	if (pcie->soc->startup == mtk_pcie_startup_port_en7528) {
+		struct pci_bus *bus = host->bus;
+		struct pci_dev *rc = NULL;
+
+		while ((rc = pci_get_class(PCI_CLASS_BRIDGE_PCI << 8, rc))) {
+			int ret = -EOPNOTSUPP;
+
+			if (rc->bus != bus)
+				continue;
+
+			#if IS_BUILTIN(CONFIG_PCIE_MEDIATEK)
+			ret = pcie_retrain_link(rc, true);
+			#endif
+
+			if (!ret)
+				dev_info(dev, "port%d link retrained\n",
+					 PCI_SLOT(rc->devfn));
+			else
+				dev_info(dev, "port%d failed to retrain %pe\n",
+					 PCI_SLOT(rc->devfn), ERR_PTR(ret));
+		}
+	}
+
 	return 0;
 
 put_resources:
@@ -1264,8 +1375,15 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt7629 = {
 	.quirks = MTK_PCIE_FIX_CLASS_ID | MTK_PCIE_FIX_DEVICE_ID,
 };
 
+static const struct mtk_pcie_soc mtk_pcie_soc_en7528 = {
+	.ops = &mtk_pcie_ops_v2,
+	.startup = mtk_pcie_startup_port_en7528,
+	.setup_irq = mtk_pcie_setup_irq,
+};
+
 static const struct of_device_id mtk_pcie_ids[] = {
 	{ .compatible = "airoha,an7583-pcie", .data = &mtk_pcie_soc_an7583 },
+	{ .compatible = "econet,en7528-pcie", .data = &mtk_pcie_soc_en7528 },
 	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
 	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
 	{ .compatible = "mediatek,mt2712-pcie", .data = &mtk_pcie_soc_mt2712 },
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
  2026-03-16 15:51 [PATCH v2 0/3] PCI: mediatek: Add support for EcoNet SoCs Caleb James DeLisle
  2026-03-16 15:51 ` [PATCH v2 1/3] dt-bindings: PCI: mediatek: Add support for EcoNet EN7528 Caleb James DeLisle
  2026-03-16 15:51 ` [PATCH v2 2/3] PCI: mediatek: Add support for EcoNet EN7528 SoC Caleb James DeLisle
@ 2026-03-16 15:51 ` Caleb James DeLisle
  2026-03-17 21:29   ` Bjorn Helgaas
  2026-03-18 21:58   ` Bjorn Helgaas
  2 siblings, 2 replies; 13+ messages in thread
From: Caleb James DeLisle @ 2026-03-16 15:51 UTC (permalink / raw)
  To: linux-pci
  Cc: linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel, Caleb James DeLisle, Bjorn Helgaas

pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
registers unconditionally. If the registers are hardwired to zero
(not implemented), both base and limit will be 0. Since (0 <= 0) is
true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
gets created.

pci_read_bridge_windows() already detects unsupported windows by
testing register writability and sets io_window/pref_window flags
accordingly. Check these flags at the start of pci_read_bridge_io()
and pci_read_bridge_mmio_pref() to skip reading registers when the
window is not supported.

Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/pci/probe.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index bccc7a4bdd79..4eacb741b4ec 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
 	unsigned long io_mask, io_granularity, base, limit;
 	struct pci_bus_region region;
 
+	if (!dev->io_window)
+		return;
+
 	io_mask = PCI_IO_RANGE_MASK;
 	io_granularity = 0x1000;
 	if (dev->io_window_1k) {
@@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
 	pci_bus_addr_t base, limit;
 	struct pci_bus_region region;
 
+	if (!dev->pref_window)
+		return;
+
 	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
 	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
 	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: PCI: mediatek: Add support for EcoNet EN7528
  2026-03-16 15:51 ` [PATCH v2 1/3] dt-bindings: PCI: mediatek: Add support for EcoNet EN7528 Caleb James DeLisle
@ 2026-03-17  7:26   ` Krzysztof Kozlowski
  2026-03-18 20:42     ` Caleb James DeLisle
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-17  7:26 UTC (permalink / raw)
  To: Caleb James DeLisle
  Cc: linux-pci, linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel

On Mon, Mar 16, 2026 at 03:51:55PM +0000, Caleb James DeLisle wrote:
>  
>          mediatek,pbus-csr: false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: econet,en7528-pcie
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +
> +        clock-names:
> +          maxItems: 1
> +
> +        reset: false
> +
> +        reset-names: false
> +
> +        power-domain: false

Wrong property.

> +
> +        mediatek,pbus-csr: false
> +
> +      required:
> +        - phys
> +        - phy-names
> +
>  unevaluatedProperties: false
>  
>  examples:
> @@ -436,3 +462,59 @@ examples:
>              };
>          };
>      };
> +
> +  # EN7528
> +  - |
> +    #include <dt-bindings/interrupt-controller/mips-gic.h>
> +    #include <dt-bindings/clock/en7523-clk.h>
> +    #include <dt-bindings/reset/airoha,en7523-reset.h>
> +
> +    soc_3 {

soc

Underscores are not allowed, but honestly neither soc node is needed nor
the example. There are already 3 examples, it's enough, especially that
there are no new properties here.


> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        pcie@1fb81000 {
> +          compatible = "econet,en7528-pcie";
> +          device_type = "pci";
> +          linux,pci-domain = <0>;
> +          #address-cells = <3>;
> +          #size-cells = <2>;
> +
> +          reg = <0x1fb81000 0x1000>;
> +          reg-names = "port0";
> +
> +          clocks = <&scuclk EN7523_CLK_PCIE>;
> +          clock-names = "sys_ck0";
> +
> +          phys = <&pcie_phy0>;
> +          phy-names = "pcie-phy0";
> +
> +          ranges = <0x01000000 0 0x00000000 0x1f600000 0 0x00010000>,
> +                   <0x82000000 0 0x20000000 0x20000000 0 0x08000000>;
> +
> +          interrupt-parent = <&intc>;
> +          interrupts = <23>;
> +          interrupt-names = "pcie_irq";
> +          bus-range = <0x00 0xff>;
> +          #interrupt-cells = <1>;
> +          interrupt-map-mask = <0 0 0 7>;
> +          interrupt-map = <0 0 0 1 &pcie_intc 0>,
> +              <0 0 0 2 &pcie_intc 1>,
> +              <0 0 0 3 &pcie_intc 2>,
> +              <0 0 0 4 &pcie_intc 3>;
> +
> +          pcie_intc: interrupt-controller {
> +            interrupt-controller;
> +            #address-cells = <0>;
> +            #interrupt-cells = <1>;
> +          };
> +
> +          slot0: pcie@0,0 {
> +            device_type = "pci";
> +            reg = <0x0000 0 0 0 0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            ranges;
> +          };
> +        };
> +    };
> -- 
> 2.39.5
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
  2026-03-16 15:51 ` [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported Caleb James DeLisle
@ 2026-03-17 21:29   ` Bjorn Helgaas
  2026-03-18  6:26     ` Ahmed Naseef
  2026-03-18 21:58   ` Bjorn Helgaas
  1 sibling, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2026-03-17 21:29 UTC (permalink / raw)
  To: Caleb James DeLisle
  Cc: linux-pci, linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel

On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> registers unconditionally. If the registers are hardwired to zero
> (not implemented), both base and limit will be 0. Since (0 <= 0) is
> true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> gets created.
> 
> pci_read_bridge_windows() already detects unsupported windows by
> testing register writability and sets io_window/pref_window flags
> accordingly. Check these flags at the start of pci_read_bridge_io()
> and pci_read_bridge_mmio_pref() to skip reading registers when the
> window is not supported.

The fundamental problem here is that assigned space to a bridge window
that isn't implemented.  I wish we understood the connection between
this "read window" path and the assignment path.

Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
with res->flags being NULL, and we set IORESOURCE_MEM |
IORESOURCE_PREFETCH again, which makes it look like we can assign
space for it?

If that's the case, I think it would improve the commit log to mention
the actual mechanism by which we avoid assigning space.

> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> ---
>  drivers/pci/probe.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..4eacb741b4ec 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
>  	unsigned long io_mask, io_granularity, base, limit;
>  	struct pci_bus_region region;
>  
> +	if (!dev->io_window)
> +		return;
> +
>  	io_mask = PCI_IO_RANGE_MASK;
>  	io_granularity = 0x1000;
>  	if (dev->io_window_1k) {
> @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
>  	pci_bus_addr_t base, limit;
>  	struct pci_bus_region region;
>  
> +	if (!dev->pref_window)
> +		return;
> +
>  	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
>  	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
>  	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
> -- 
> 2.39.5
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
  2026-03-17 21:29   ` Bjorn Helgaas
@ 2026-03-18  6:26     ` Ahmed Naseef
  2026-03-18 12:04       ` Ilpo Järvinen
  0 siblings, 1 reply; 13+ messages in thread
From: Ahmed Naseef @ 2026-03-18  6:26 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Caleb James DeLisle, linux-pci, linux-mips, ryder.lee, bhelgaas,
	lpieralisi, kwilczynski, mani, robh, krzk+dt, conor+dt,
	matthias.bgg, angelogioacchino.delregno, ansuelsmth,
	linux-mediatek, devicetree, linux-kernel

On Tue, Mar 17, 2026 at 04:29:08PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> > registers unconditionally. If the registers are hardwired to zero
> > (not implemented), both base and limit will be 0. Since (0 <= 0) is
> > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> > gets created.
> > 
> > pci_read_bridge_windows() already detects unsupported windows by
> > testing register writability and sets io_window/pref_window flags
> > accordingly. Check these flags at the start of pci_read_bridge_io()
> > and pci_read_bridge_mmio_pref() to skip reading registers when the
> > window is not supported.
> 
> The fundamental problem here is that assigned space to a bridge window
> that isn't implemented.  I wish we understood the connection between
> this "read window" path and the assignment path.
> 
> Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
> with res->flags being NULL, and we set IORESOURCE_MEM |
> IORESOURCE_PREFETCH again, which makes it look like we can assign
> space for it?

Yes, that's exactly right.

> 
> If that's the case, I think it would improve the commit log to mention
> the actual mechanism by which we avoid assigning space.
> 

How about this:

  pci_read_bridge_io() and pci_read_bridge_mmio_pref() read
  bridge window registers unconditionally. If the registers
  are hardwired to zero (not implemented), both base and limit
  will be 0. Since (0 <= 0) is true, these functions set
  IORESOURCE_IO or IORESOURCE_MEM | IORESOURCE_PREFETCH on
  the bridge resource. This causes the allocator to assign
  space for the window even though the hardware can't
  implement it.

  pci_read_bridge_windows() already detects unsupported windows
  by testing register writability and sets io_window/pref_window
  flags accordingly. Check these flags at the start of
  pci_read_bridge_io() and pci_read_bridge_mmio_pref() to skip
  reading registers when the window is not supported, so the
  resource flags remain clear and the allocator does not assign
  space for non-existent windows.


Ahmed Naseef

> > Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> > Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> > Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> > Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> > ---
> >  drivers/pci/probe.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index bccc7a4bdd79..4eacb741b4ec 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
> >  	unsigned long io_mask, io_granularity, base, limit;
> >  	struct pci_bus_region region;
> >  
> > +	if (!dev->io_window)
> > +		return;
> > +
> >  	io_mask = PCI_IO_RANGE_MASK;
> >  	io_granularity = 0x1000;
> >  	if (dev->io_window_1k) {
> > @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
> >  	pci_bus_addr_t base, limit;
> >  	struct pci_bus_region region;
> >  
> > +	if (!dev->pref_window)
> > +		return;
> > +
> >  	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
> >  	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
> >  	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
> > -- 
> > 2.39.5
> > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
  2026-03-18  6:26     ` Ahmed Naseef
@ 2026-03-18 12:04       ` Ilpo Järvinen
  2026-03-18 12:18         ` Ilpo Järvinen
  0 siblings, 1 reply; 13+ messages in thread
From: Ilpo Järvinen @ 2026-03-18 12:04 UTC (permalink / raw)
  To: Ahmed Naseef
  Cc: Bjorn Helgaas, Caleb James DeLisle, linux-pci, linux-mips,
	ryder.lee, bhelgaas, lpieralisi, kwilczynski, mani, robh, krzk+dt,
	conor+dt, matthias.bgg, angelogioacchino.delregno, ansuelsmth,
	linux-mediatek, devicetree, LKML

On Wed, 18 Mar 2026, Ahmed Naseef wrote:

> On Tue, Mar 17, 2026 at 04:29:08PM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> > > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> > > registers unconditionally. If the registers are hardwired to zero
> > > (not implemented), both base and limit will be 0. Since (0 <= 0) is
> > > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> > > gets created.
> > > 
> > > pci_read_bridge_windows() already detects unsupported windows by
> > > testing register writability and sets io_window/pref_window flags
> > > accordingly. Check these flags at the start of pci_read_bridge_io()
> > > and pci_read_bridge_mmio_pref() to skip reading registers when the
> > > window is not supported.
> > 
> > The fundamental problem here is that assigned space to a bridge window
> > that isn't implemented.  I wish we understood the connection between
> > this "read window" path and the assignment path.
> > 
> > Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
> > with res->flags being NULL, and we set IORESOURCE_MEM |
> > IORESOURCE_PREFETCH again, which makes it look like we can assign
> > space for it?
> 
> Yes, that's exactly right.
> 
> > 
> > If that's the case, I think it would improve the commit log to mention
> > the actual mechanism by which we avoid assigning space.
> > 
> 
> How about this:
> 
>   pci_read_bridge_io() and pci_read_bridge_mmio_pref() read
>   bridge window registers unconditionally. If the registers
>   are hardwired to zero (not implemented), both base and limit
>   will be 0. Since (0 <= 0) is true, these functions set
>   IORESOURCE_IO or IORESOURCE_MEM | IORESOURCE_PREFETCH on
>   the bridge resource. This causes the allocator to assign
>   space for the window even though the hardware can't
>   implement it.
> 
>   pci_read_bridge_windows() already detects unsupported windows
>   by testing register writability and sets io_window/pref_window
>   flags accordingly. Check these flags at the start of
>   pci_read_bridge_io() and pci_read_bridge_mmio_pref() to skip
>   reading registers when the window is not supported, so the
>   resource flags remain clear and the allocator does not assign
>   space for non-existent windows.

At least to me the proposed text reads much better than the original.
The original text required reading between the lines to connect the dots, 
whereas this new one clearly explains what causes what.

--
 i.

> Ahmed Naseef
> 
> > > Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> > > Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> > > Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> > > Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> > > ---
> > >  drivers/pci/probe.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > index bccc7a4bdd79..4eacb741b4ec 100644
> > > --- a/drivers/pci/probe.c
> > > +++ b/drivers/pci/probe.c
> > > @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
> > >  	unsigned long io_mask, io_granularity, base, limit;
> > >  	struct pci_bus_region region;
> > >  
> > > +	if (!dev->io_window)
> > > +		return;
> > > +
> > >  	io_mask = PCI_IO_RANGE_MASK;
> > >  	io_granularity = 0x1000;
> > >  	if (dev->io_window_1k) {
> > > @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
> > >  	pci_bus_addr_t base, limit;
> > >  	struct pci_bus_region region;
> > >  
> > > +	if (!dev->pref_window)
> > > +		return;
> > > +
> > >  	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
> > >  	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
> > >  	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
> > > -- 
> > > 2.39.5
> > > 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
  2026-03-18 12:04       ` Ilpo Järvinen
@ 2026-03-18 12:18         ` Ilpo Järvinen
  2026-03-18 13:12           ` Ahmed Naseef
  0 siblings, 1 reply; 13+ messages in thread
From: Ilpo Järvinen @ 2026-03-18 12:18 UTC (permalink / raw)
  To: Ahmed Naseef
  Cc: Bjorn Helgaas, Caleb James DeLisle, linux-pci, linux-mips,
	ryder.lee, bhelgaas, lpieralisi, kwilczynski, mani, robh, krzk+dt,
	conor+dt, matthias.bgg, angelogioacchino.delregno, ansuelsmth,
	linux-mediatek, devicetree, LKML

[-- Attachment #1: Type: text/plain, Size: 3377 bytes --]

On Wed, 18 Mar 2026, Ilpo Järvinen wrote:

> On Wed, 18 Mar 2026, Ahmed Naseef wrote:
> 
> > On Tue, Mar 17, 2026 at 04:29:08PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> > > > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> > > > registers unconditionally. If the registers are hardwired to zero
> > > > (not implemented), both base and limit will be 0. Since (0 <= 0) is
> > > > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> > > > gets created.
> > > > 
> > > > pci_read_bridge_windows() already detects unsupported windows by
> > > > testing register writability and sets io_window/pref_window flags
> > > > accordingly. Check these flags at the start of pci_read_bridge_io()
> > > > and pci_read_bridge_mmio_pref() to skip reading registers when the
> > > > window is not supported.
> > > 
> > > The fundamental problem here is that assigned space to a bridge window
> > > that isn't implemented.  I wish we understood the connection between
> > > this "read window" path and the assignment path.
> > > 
> > > Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
> > > with res->flags being NULL, and we set IORESOURCE_MEM |
> > > IORESOURCE_PREFETCH again, which makes it look like we can assign
> > > space for it?
> > 
> > Yes, that's exactly right.
> > 
> > > 
> > > If that's the case, I think it would improve the commit log to mention
> > > the actual mechanism by which we avoid assigning space.
> > > 
> > 
> > How about this:
> > 
> >   pci_read_bridge_io() and pci_read_bridge_mmio_pref() read
> >   bridge window registers unconditionally. If the registers
> >   are hardwired to zero (not implemented), both base and limit
> >   will be 0. Since (0 <= 0) is true, these functions set
> >   IORESOURCE_IO or IORESOURCE_MEM | IORESOURCE_PREFETCH on
> >   the bridge resource. This causes the allocator to assign
> >   space for the window even though the hardware can't
> >   implement it.
> > 
> >   pci_read_bridge_windows() already detects unsupported windows
> >   by testing register writability and sets io_window/pref_window
> >   flags accordingly. Check these flags at the start of
> >   pci_read_bridge_io() and pci_read_bridge_mmio_pref() to skip
> >   reading registers when the window is not supported, so the
> >   resource flags remain clear and the allocator does not assign
> >   space for non-existent windows.
> 
> At least to me the proposed text reads much better than the original.
> The original text required reading between the lines to connect the dots, 
> whereas this new one clearly explains what causes what.

Hi again,

Reading the code I think the entire 0 <= 0 part is a red herring 
when it comes to the current code, the flags are always set by the 
functions!

The code would only add IORESOURCE_UNSET | IORESOURCE_DISABLED if the 
base <= limit check fails but that's still wrong because it says to the 
resource allocation code that it can enable that bridge window if it needs 
to.

Prior to the commit 8278c6914306 ("PCI: Preserve bridge window resource 
type flags") the base <= limit check did play some role (maybe the 
original commit message was based on some older tree than the most current 
one).

-- 
 i.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
  2026-03-18 12:18         ` Ilpo Järvinen
@ 2026-03-18 13:12           ` Ahmed Naseef
  0 siblings, 0 replies; 13+ messages in thread
From: Ahmed Naseef @ 2026-03-18 13:12 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Bjorn Helgaas, Caleb James DeLisle, linux-pci, linux-mips,
	ryder.lee, bhelgaas, lpieralisi, kwilczynski, mani, robh, krzk+dt,
	conor+dt, matthias.bgg, angelogioacchino.delregno, ansuelsmth,
	linux-mediatek, devicetree, LKML

On Wed, Mar 18, 2026 at 02:18:22PM +0200, Ilpo Järvinen wrote:
> On Wed, 18 Mar 2026, Ilpo Järvinen wrote:
> 
> > On Wed, 18 Mar 2026, Ahmed Naseef wrote:
> > 
> > > On Tue, Mar 17, 2026 at 04:29:08PM -0500, Bjorn Helgaas wrote:
> > > > On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> > > > > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> > > > > registers unconditionally. If the registers are hardwired to zero
> > > > > (not implemented), both base and limit will be 0. Since (0 <= 0) is
> > > > > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> > > > > gets created.
> > > > > 
> > > > > pci_read_bridge_windows() already detects unsupported windows by
> > > > > testing register writability and sets io_window/pref_window flags
> > > > > accordingly. Check these flags at the start of pci_read_bridge_io()
> > > > > and pci_read_bridge_mmio_pref() to skip reading registers when the
> > > > > window is not supported.
> > > > 
> > > > The fundamental problem here is that assigned space to a bridge window
> > > > that isn't implemented.  I wish we understood the connection between
> > > > this "read window" path and the assignment path.
> > > > 
> > > > Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
> > > > with res->flags being NULL, and we set IORESOURCE_MEM |
> > > > IORESOURCE_PREFETCH again, which makes it look like we can assign
> > > > space for it?
> > > 
> > > Yes, that's exactly right.
> > > 
> > > > 
> > > > If that's the case, I think it would improve the commit log to mention
> > > > the actual mechanism by which we avoid assigning space.
> > > > 
> > > 
> > > How about this:
> > > 
> > >   pci_read_bridge_io() and pci_read_bridge_mmio_pref() read
> > >   bridge window registers unconditionally. If the registers
> > >   are hardwired to zero (not implemented), both base and limit
> > >   will be 0. Since (0 <= 0) is true, these functions set
> > >   IORESOURCE_IO or IORESOURCE_MEM | IORESOURCE_PREFETCH on
> > >   the bridge resource. This causes the allocator to assign
> > >   space for the window even though the hardware can't
> > >   implement it.
> > > 
> > >   pci_read_bridge_windows() already detects unsupported windows
> > >   by testing register writability and sets io_window/pref_window
> > >   flags accordingly. Check these flags at the start of
> > >   pci_read_bridge_io() and pci_read_bridge_mmio_pref() to skip
> > >   reading registers when the window is not supported, so the
> > >   resource flags remain clear and the allocator does not assign
> > >   space for non-existent windows.
> > 
> > At least to me the proposed text reads much better than the original.
> > The original text required reading between the lines to connect the dots, 
> > whereas this new one clearly explains what causes what.
> 
> Hi again,
> 
> Reading the code I think the entire 0 <= 0 part is a red herring 
> when it comes to the current code, the flags are always set by the 
> functions!
> 
> The code would only add IORESOURCE_UNSET | IORESOURCE_DISABLED if the 
> base <= limit check fails but that's still wrong because it says to the 
> resource allocation code that it can enable that bridge window if it needs 
> to.
> 
> Prior to the commit 8278c6914306 ("PCI: Preserve bridge window resource 
> type flags") the base <= limit check did play some role (maybe the 
> original commit message was based on some older tree than the most current 
> one).

Thank you for catching that. We were testing on LTS kernel
6.12 (downstream OpenWrt) where the commit 8278c6914306
("PCI: Preserve bridge window resource type flags") is not
present . In that tree the flags are still only set inside
the base <= limit check, which is why the commit message
focused on that path.
  
For current mainline, how about this for the commit message:

  pci_read_bridge_io() and pci_read_bridge_mmio_pref()
  unconditionally set resource type flags (IORESOURCE_IO
  or IORESOURCE_MEM | IORESOURCE_PREFETCH) when reading
  bridge window registers. For windows that are not
  implemented in hardware, this causes the allocator to
  assign space for a window that doesn't exist.

  pci_read_bridge_windows() already detects unsupported
  windows by testing register writability and sets
  io_window/pref_window flags accordingly. Check these
  flags at the start of pci_read_bridge_io() and
  pci_read_bridge_mmio_pref() to skip them entirely when
  the window is not supported, so the resource flags
  remain clear and the allocator does not assign space
  for non-existent windows.


Ahmed Naseef

> 
> -- 
>  i.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: PCI: mediatek: Add support for EcoNet EN7528
  2026-03-17  7:26   ` Krzysztof Kozlowski
@ 2026-03-18 20:42     ` Caleb James DeLisle
  0 siblings, 0 replies; 13+ messages in thread
From: Caleb James DeLisle @ 2026-03-18 20:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-pci, linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel


On 17/03/2026 08:26, Krzysztof Kozlowski wrote:
> On Mon, Mar 16, 2026 at 03:51:55PM +0000, Caleb James DeLisle wrote:
>>   
>>           mediatek,pbus-csr: false
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: econet,en7528-pcie
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 1
>> +
>> +        clock-names:
>> +          maxItems: 1
>> +
>> +        reset: false
>> +
>> +        reset-names: false
>> +
>> +        power-domain: false
> Wrong property.
>
>> +
>> +        mediatek,pbus-csr: false
>> +
>> +      required:
>> +        - phys
>> +        - phy-names
>> +
>>   unevaluatedProperties: false
>>   
>>   examples:
>> @@ -436,3 +462,59 @@ examples:
>>               };
>>           };
>>       };
>> +
>> +  # EN7528
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/mips-gic.h>
>> +    #include <dt-bindings/clock/en7523-clk.h>
>> +    #include <dt-bindings/reset/airoha,en7523-reset.h>
>> +
>> +    soc_3 {
> soc
>
> Underscores are not allowed, but honestly neither soc node is needed nor
> the example. There are already 3 examples, it's enough, especially that
> there are no new properties here.

Ok, will remove the example then.

Thanks,

Caleb


>
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +
>> +        pcie@1fb81000 {
>> +          compatible = "econet,en7528-pcie";
>> +          device_type = "pci";
>> +          linux,pci-domain = <0>;
>> +          #address-cells = <3>;
>> +          #size-cells = <2>;
>> +
>> +          reg = <0x1fb81000 0x1000>;
>> +          reg-names = "port0";
>> +
>> +          clocks = <&scuclk EN7523_CLK_PCIE>;
>> +          clock-names = "sys_ck0";
>> +
>> +          phys = <&pcie_phy0>;
>> +          phy-names = "pcie-phy0";
>> +
>> +          ranges = <0x01000000 0 0x00000000 0x1f600000 0 0x00010000>,
>> +                   <0x82000000 0 0x20000000 0x20000000 0 0x08000000>;
>> +
>> +          interrupt-parent = <&intc>;
>> +          interrupts = <23>;
>> +          interrupt-names = "pcie_irq";
>> +          bus-range = <0x00 0xff>;
>> +          #interrupt-cells = <1>;
>> +          interrupt-map-mask = <0 0 0 7>;
>> +          interrupt-map = <0 0 0 1 &pcie_intc 0>,
>> +              <0 0 0 2 &pcie_intc 1>,
>> +              <0 0 0 3 &pcie_intc 2>,
>> +              <0 0 0 4 &pcie_intc 3>;
>> +
>> +          pcie_intc: interrupt-controller {
>> +            interrupt-controller;
>> +            #address-cells = <0>;
>> +            #interrupt-cells = <1>;
>> +          };
>> +
>> +          slot0: pcie@0,0 {
>> +            device_type = "pci";
>> +            reg = <0x0000 0 0 0 0>;
>> +            #address-cells = <3>;
>> +            #size-cells = <2>;
>> +            ranges;
>> +          };
>> +        };
>> +    };
>> -- 
>> 2.39.5
>>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
  2026-03-16 15:51 ` [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported Caleb James DeLisle
  2026-03-17 21:29   ` Bjorn Helgaas
@ 2026-03-18 21:58   ` Bjorn Helgaas
  2026-03-18 22:18     ` Caleb James DeLisle
  1 sibling, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2026-03-18 21:58 UTC (permalink / raw)
  To: Caleb James DeLisle
  Cc: linux-pci, linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel

On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> registers unconditionally. If the registers are hardwired to zero
> (not implemented), both base and limit will be 0. Since (0 <= 0) is
> true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> gets created.
> 
> pci_read_bridge_windows() already detects unsupported windows by
> testing register writability and sets io_window/pref_window flags
> accordingly. Check these flags at the start of pci_read_bridge_io()
> and pci_read_bridge_mmio_pref() to skip reading registers when the
> window is not supported.
> 
> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>

I applied this to pci/resource for v7.1 with the following commit log:

  PCI: Prevent assignment to unsupported bridge windows

  Previously, pci_read_bridge_io() and pci_read_bridge_mmio_pref()
  unconditionally set resource type flags (IORESOURCE_IO or IORESOURCE_MEM |
  IORESOURCE_PREFETCH) when reading bridge window registers. For windows that
  are not implemented in hardware, this may cause the allocator to assign
  space for a window that doesn't exist.

  For example, the EcoNET EN7528 SoC Root Port doesn't support the
  prefetchable window, but since a downstream device had a prefetchable BAR,
  the allocator mistakenly assigned a prefetchable window:

    pci 0001:00:01.0: [14c3:0811] type 01 class 0x060400 PCIe Root Port
    pci 0001:00:01.0: PCI bridge to [bus 01-ff]
    pci 0001:00:01.0: bridge window [mem 0x28000000-0x280fffff]: assigned
    pci 0001:00:01.0: bridge window [mem 0x28100000-0x282fffff pref]: assigned
    pci 0001:01:00.0: BAR 0 [mem 0x28100000-0x281fffff 64bit pref]: assigned

  pci_read_bridge_windows() already detects unsupported windows by testing
  register writability and sets dev->io_window/pref_window accordingly.

  Check dev->io_window/pref_window so we don't set the resource flags for
  unsupported windows, which prevents the allocator from assigning space to
  them.

  After this commit, the prefetchable BAR is correctly allocated from the
  non-prefetchable window:

    pci 0001:00:01.0: bridge window [mem 0x28000000-0x281fffff]: assigned
    pci 0001:01:00.0: BAR 0 [mem 0x28000000-0x280fffff 64bit pref]: assigned

I also set the author to "Ahmed Naseef <naseefkm@gmail.com>" per
https://lore.kernel.org/all/abRQYM1If/6Vv/tI@DESKTOP-TIT0J8O.localdomain

You can make this work correctly next time by including a
"From: Ahmed Naseef <naseefkm@gmail.com>" line as the very first line
in the message body; see
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.19#n723

> ---
>  drivers/pci/probe.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..4eacb741b4ec 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
>  	unsigned long io_mask, io_granularity, base, limit;
>  	struct pci_bus_region region;
>  
> +	if (!dev->io_window)
> +		return;
> +
>  	io_mask = PCI_IO_RANGE_MASK;
>  	io_granularity = 0x1000;
>  	if (dev->io_window_1k) {
> @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
>  	pci_bus_addr_t base, limit;
>  	struct pci_bus_region region;
>  
> +	if (!dev->pref_window)
> +		return;
> +
>  	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
>  	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
>  	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
> -- 
> 2.39.5
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
  2026-03-18 21:58   ` Bjorn Helgaas
@ 2026-03-18 22:18     ` Caleb James DeLisle
  0 siblings, 0 replies; 13+ messages in thread
From: Caleb James DeLisle @ 2026-03-18 22:18 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-mips, naseefkm, ryder.lee, bhelgaas, lpieralisi,
	kwilczynski, mani, robh, krzk+dt, conor+dt, matthias.bgg,
	angelogioacchino.delregno, ansuelsmth, linux-mediatek, devicetree,
	linux-kernel


On 18/03/2026 22:58, Bjorn Helgaas wrote:
> On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
>> pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
>> registers unconditionally. If the registers are hardwired to zero
>> (not implemented), both base and limit will be 0. Since (0 <= 0) is
>> true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
>> gets created.
>>
>> pci_read_bridge_windows() already detects unsupported windows by
>> testing register writability and sets io_window/pref_window flags
>> accordingly. Check these flags at the start of pci_read_bridge_io()
>> and pci_read_bridge_mmio_pref() to skip reading registers when the
>> window is not supported.
>>
>> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
>> Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
>> Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
>> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> I applied this to pci/resource for v7.1 with the following commit log:
>
>    PCI: Prevent assignment to unsupported bridge windows
>
>    Previously, pci_read_bridge_io() and pci_read_bridge_mmio_pref()
>    unconditionally set resource type flags (IORESOURCE_IO or IORESOURCE_MEM |
>    IORESOURCE_PREFETCH) when reading bridge window registers. For windows that
>    are not implemented in hardware, this may cause the allocator to assign
>    space for a window that doesn't exist.
>
>    For example, the EcoNET EN7528 SoC Root Port doesn't support the
>    prefetchable window, but since a downstream device had a prefetchable BAR,
>    the allocator mistakenly assigned a prefetchable window:
>
>      pci 0001:00:01.0: [14c3:0811] type 01 class 0x060400 PCIe Root Port
>      pci 0001:00:01.0: PCI bridge to [bus 01-ff]
>      pci 0001:00:01.0: bridge window [mem 0x28000000-0x280fffff]: assigned
>      pci 0001:00:01.0: bridge window [mem 0x28100000-0x282fffff pref]: assigned
>      pci 0001:01:00.0: BAR 0 [mem 0x28100000-0x281fffff 64bit pref]: assigned
>
>    pci_read_bridge_windows() already detects unsupported windows by testing
>    register writability and sets dev->io_window/pref_window accordingly.
>
>    Check dev->io_window/pref_window so we don't set the resource flags for
>    unsupported windows, which prevents the allocator from assigning space to
>    them.
>
>    After this commit, the prefetchable BAR is correctly allocated from the
>    non-prefetchable window:
>
>      pci 0001:00:01.0: bridge window [mem 0x28000000-0x281fffff]: assigned
>      pci 0001:01:00.0: BAR 0 [mem 0x28000000-0x280fffff 64bit pref]: assigned
>
> I also set the author to "Ahmed Naseef <naseefkm@gmail.com>" per
> https://lore.kernel.org/all/abRQYM1If/6Vv/tI@DESKTOP-TIT0J8O.localdomain
>
> You can make this work correctly next time by including a
> "From: Ahmed Naseef <naseefkm@gmail.com>" line as the very first line
> in the message body; see
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.19#n723

Thank you very much. I wasn't quite sure how to submit this so thanks 
and I will keep that in mind.

Thanks,

Caleb

>> ---
>>   drivers/pci/probe.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index bccc7a4bdd79..4eacb741b4ec 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
>>   	unsigned long io_mask, io_granularity, base, limit;
>>   	struct pci_bus_region region;
>>   
>> +	if (!dev->io_window)
>> +		return;
>> +
>>   	io_mask = PCI_IO_RANGE_MASK;
>>   	io_granularity = 0x1000;
>>   	if (dev->io_window_1k) {
>> @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
>>   	pci_bus_addr_t base, limit;
>>   	struct pci_bus_region region;
>>   
>> +	if (!dev->pref_window)
>> +		return;
>> +
>>   	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
>>   	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
>>   	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
>> -- 
>> 2.39.5
>>


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-03-18 22:18 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-16 15:51 [PATCH v2 0/3] PCI: mediatek: Add support for EcoNet SoCs Caleb James DeLisle
2026-03-16 15:51 ` [PATCH v2 1/3] dt-bindings: PCI: mediatek: Add support for EcoNet EN7528 Caleb James DeLisle
2026-03-17  7:26   ` Krzysztof Kozlowski
2026-03-18 20:42     ` Caleb James DeLisle
2026-03-16 15:51 ` [PATCH v2 2/3] PCI: mediatek: Add support for EcoNet EN7528 SoC Caleb James DeLisle
2026-03-16 15:51 ` [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported Caleb James DeLisle
2026-03-17 21:29   ` Bjorn Helgaas
2026-03-18  6:26     ` Ahmed Naseef
2026-03-18 12:04       ` Ilpo Järvinen
2026-03-18 12:18         ` Ilpo Järvinen
2026-03-18 13:12           ` Ahmed Naseef
2026-03-18 21:58   ` Bjorn Helgaas
2026-03-18 22:18     ` Caleb James DeLisle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox