* [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver
@ 2026-09-22 2:36 Shawn Lin
2026-09-22 2:37 ` [PATCH v3 1/3] PCI: dw-rockchip: Move the INTx irq setup to probe Shawn Lin
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Shawn Lin @ 2026-09-22 2:36 UTC (permalink / raw)
To: Manivannan Sadhasivam, Bjorn Helgaas
Cc: linux-rockchip, linux-pci, Niklas Cassel, Shawn Lin
This short series fixes the INTx handling around the newly introduced
.reset_root_port() (b376b3ff9cb0), and is split in three patches per
Niklas' suggestion:
Patch 1 stops .reset_root_port() from recreating the INTx irq domain
on every root port reset, which leaked the old domain and silently
broke INTx delivery afterwards, the downstream devices' virqs were
allocated in the previous domain and were never re-mapped. By moving
the of_irq_get_byname() lookup, the INTx irq domain creation and the
chained handler installation into rockchip_pcie_configure_rc(), right
after dw_pcie_host_init(). This leaves .init() with nothing but
idempotent register programming, so it can safely be re-run by
.reset_root_port() and dw_pcie_resume_noirq().
Patch 2 makes the irq domain and the chained handler devm-managed, so
that they are released with the device instead of leaking, which also
addresses the probe failure leak/use-after-free flagged by the Sashiko
review.
Patch 3 keeps the INTx IRQ masked while .reset_root_port() gates the
controller clocks, so the chained handler cannot read the unclocked
APB bus and raise a synchronous external abort.
Changes in v3:
- split devm-managed part into a seperate patch
Changes in v2:
- Moved the of_irq_get_byname() lookup, the INTx irq domain creation
and the chained handler installation out of the host ops .init()
callback into rockchip_pcie_configure_rc(), right after
dw_pcie_host_init(), as suggested by Niklas Cassel. This supersedes
v1 patch 1/2, as .init() no longer creates the irq domain, and
removes the rockchip_pcie_host_hw_init() helper from v1.
- Made the INTx irq domain devm-managed with
devm_irq_domain_instantiate() and uninstall the chained handler
through a devres action, addressing the probe failure leak and
use-after-free flagged by the Sashiko review.
- keep the INTx IRQ masked while .reset_root_port()
gates the controller clocks, responding to the Sashiko review
finding about accessing the unclocked APB bus.
Shawn Lin (3):
PCI: dw-rockchip: Move the INTx irq setup to probe
PCI: dw-rockchip: Make the INTx irq setup devm-managed
PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are
gated
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 77 ++++++++++++++++++++-------
1 file changed, 57 insertions(+), 20 deletions(-)
--
2.7.4
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/3] PCI: dw-rockchip: Move the INTx irq setup to probe
2026-09-22 2:36 [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Shawn Lin
@ 2026-09-22 2:37 ` Shawn Lin
2026-09-22 9:56 ` Niklas Cassel
2026-09-22 2:37 ` [PATCH v3 2/3] PCI: dw-rockchip: Make the INTx irq setup devm-managed Shawn Lin
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Shawn Lin @ 2026-09-22 2:37 UTC (permalink / raw)
To: Manivannan Sadhasivam, Bjorn Helgaas
Cc: linux-rockchip, linux-pci, Niklas Cassel, Shawn Lin
Since commit b376b3ff9cb0 ("PCI: dw-rockchip: Implement .reset_root_port()
and use for link down"), .reset_root_port() re-runs the host ops .init()
callback to reprogram the Root Complex after a controller reset. That
works for the register programming, but .init() is not re-entrant: it
also creates the INTx irq domain and installs the chained INTx handler.
Every root port reset therefore ends up with a second irq domain
registered for the same fwnode: the previous one is leaked, as it is
never removed, and worse, the INTx virqs of the downstream PCI devices
were allocated in the previous irq domain and are never re-mapped, while
the chained handler now looks up virqs in the new, empty domain. After a
link down recovery, INTx interrupts are silently lost.
Fix it by moving the of_irq_get_byname() lookup, the INTx irq domain
creation and the chained handler installation out of .init() and into
rockchip_pcie_configure_rc(), right after dw_pcie_host_init(). This
mirrors how the qcom driver requests its global IRQ, and leaves .init()
with nothing but idempotent register programming, so both
.reset_root_port() and dw_pcie_resume_noirq() can safely re-run it.
Re-running of_irq_get_byname() on every resume is also gone.
rockchip_pcie_configure_rc() can abort probe, so propagate the irq
domain creation error instead of only logging it as .init() used to do.
Fixes: b376b3ff9cb0 ("PCI: dw-rockchip: Implement .reset_root_port() and use for link down")
Suggested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v3:
- split devm-managed part into a seperate patch
Changes in v2:
- Moved the of_irq_get_byname() lookup, the INTx irq domain creation
and the chained handler installation out of the host ops .init()
callback into rockchip_pcie_configure_rc(), right after
dw_pcie_host_init(), as suggested by Niklas Cassel. This supersedes
v1 patch 1/2, as .init() no longer creates the irq domain, and
removes the rockchip_pcie_host_hw_init() helper from v1.
- Made the INTx irq domain devm-managed with
devm_irq_domain_instantiate() and uninstall the chained handler
through a devres action, addressing the probe failure leak and
use-after-free flagged by the Sashiko review.
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 32 +++++++++++++++------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 1497686..8788a10 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -422,23 +422,9 @@ static void rockchip_pcie_stop_link(struct dw_pcie *pci)
static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
- struct device *dev = rockchip->pci.dev;
- int irq, ret;
-
- irq = of_irq_get_byname(dev->of_node, "legacy");
- if (irq < 0)
- return irq;
pci->dbi_base2 = pci->dbi_base + PCIE_TYPE0_HDR_DBI2_OFFSET;
- ret = rockchip_pcie_init_irq_domain(rockchip);
- if (ret < 0)
- dev_err(dev, "failed to init irq domain\n");
-
- irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
- rockchip);
-
rockchip_pcie_configure_l1ss(pci);
rockchip_pcie_enable_l0s(pci);
pp->bridge->reset_root_port = rockchip_pcie_rc_reset_root_port;
@@ -740,6 +726,24 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
return ret;
}
+ /*
+ * This is done here instead of in the host ops .init() callback,
+ * which is also re-run by .reset_root_port(), so that the INTx irq
+ * domain is only created once, at probe time.
+ */
+ irq = of_irq_get_byname(dev->of_node, "legacy");
+ if (irq < 0)
+ return irq;
+
+ ret = rockchip_pcie_init_irq_domain(rockchip);
+ if (ret < 0) {
+ dev_err(dev, "failed to init irq domain\n");
+ return ret;
+ }
+
+ irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
+ rockchip);
+
/* unmask hot reset/link-down reset */
val = FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_MISC);
--
2.7.4
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] PCI: dw-rockchip: Make the INTx irq setup devm-managed
2026-09-22 2:36 [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Shawn Lin
2026-09-22 2:37 ` [PATCH v3 1/3] PCI: dw-rockchip: Move the INTx irq setup to probe Shawn Lin
@ 2026-09-22 2:37 ` Shawn Lin
2026-09-22 9:58 ` Niklas Cassel
2026-09-22 2:37 ` [PATCH v3 3/3] PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are gated Shawn Lin
2026-09-23 9:13 ` [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Diederik de Haas
3 siblings, 1 reply; 10+ messages in thread
From: Shawn Lin @ 2026-09-22 2:37 UTC (permalink / raw)
To: Manivannan Sadhasivam, Bjorn Helgaas
Cc: linux-rockchip, linux-pci, Niklas Cassel, Shawn Lin
The irq domain and the chained handler set up by the previous commit
are resources owned by the device, but nothing releases them if probing
ever fails after they are set up, and they are not tied to the device
lifetime in any other way either.
Tie their lifetime to the device with devres: create the irq domain
with devm_irq_domain_instantiate() and uninstall the chained handler
through the rockchip_pcie_intx_chained_release() devres action. The
driver is builtin and cannot be unbound (suppress_bind_attrs), so probe
failure is the only path that ever needs this cleanup, and devres takes
care of it without sprinkling it over every error path.
Since the irq setup is the last step of rockchip_pcie_configure_rc(),
the only failure point left after the chained handler is installed is
devm_add_action_or_reset() itself, whose failure mode runs the action,
so the handler can never run against the devm-freed rockchip structure.
devres also unwinds in reverse registration order, so the handler is
always uninstalled before the domain is removed. There is no devm API
for chained handlers, hence the small devres action wrapper.
While at it, drop the now unused rockchip variable from
rockchip_pcie_host_init().
Suggested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
This patch didn't find a suitable fix tag as it fixes an issue along with
patch 1/3, then patch 3/3 depends on it. So it might go with the whole series
into a fix branch.
Changes in v3: None
Changes in v2: None
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 46 ++++++++++++++++++++-------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 8788a10..f395a66 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -114,6 +114,7 @@ struct rockchip_pcie {
struct reset_control *rst;
struct gpio_desc *rst_gpio;
struct irq_domain *irq_domain;
+ int intx_irq;
const struct rockchip_pcie_of_data *data;
bool supports_clkreq;
struct delayed_work trace_work;
@@ -188,9 +189,16 @@ static const struct irq_domain_ops intx_domain_ops = {
.map = rockchip_pcie_intx_map,
};
-static int rockchip_pcie_init_irq_domain(struct rockchip_pcie *rockchip)
+static void rockchip_pcie_intx_chained_release(void *data)
+{
+ struct rockchip_pcie *rockchip = data;
+
+ irq_set_chained_handler_and_data(rockchip->intx_irq, NULL, NULL);
+}
+
+static int rockchip_pcie_init_irq_domain(struct device *dev,
+ struct rockchip_pcie *rockchip)
{
- struct device *dev = rockchip->pci.dev;
struct device_node *intc;
intc = of_get_child_by_name(dev->of_node, "legacy-interrupt-controller");
@@ -199,12 +207,17 @@ static int rockchip_pcie_init_irq_domain(struct rockchip_pcie *rockchip)
return -EINVAL;
}
- rockchip->irq_domain = irq_domain_create_linear(of_fwnode_handle(intc), PCI_NUM_INTX,
- &intx_domain_ops, rockchip);
+ rockchip->irq_domain = devm_irq_domain_instantiate(dev,
+ &(struct irq_domain_info){
+ .fwnode = of_fwnode_handle(intc),
+ .size = PCI_NUM_INTX,
+ .ops = &intx_domain_ops,
+ .host_data = rockchip,
+ });
of_node_put(intc);
- if (!rockchip->irq_domain) {
+ if (IS_ERR(rockchip->irq_domain)) {
dev_err(dev, "failed to get a INTx IRQ domain\n");
- return -EINVAL;
+ return PTR_ERR(rockchip->irq_domain);
}
return 0;
@@ -731,18 +744,27 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
* which is also re-run by .reset_root_port(), so that the INTx irq
* domain is only created once, at probe time.
*/
- irq = of_irq_get_byname(dev->of_node, "legacy");
- if (irq < 0)
- return irq;
+ rockchip->intx_irq = of_irq_get_byname(dev->of_node, "legacy");
+ if (rockchip->intx_irq < 0)
+ return rockchip->intx_irq;
- ret = rockchip_pcie_init_irq_domain(rockchip);
+ ret = rockchip_pcie_init_irq_domain(dev, rockchip);
if (ret < 0) {
dev_err(dev, "failed to init irq domain\n");
return ret;
}
- irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
- rockchip);
+ irq_set_chained_handler_and_data(rockchip->intx_irq,
+ rockchip_pcie_intx_handler, rockchip);
+
+ /*
+ * Uninstall the chained handler on probe failure, so that it can
+ * never run against the devm-freed rockchip structure.
+ */
+ ret = devm_add_action_or_reset(dev, rockchip_pcie_intx_chained_release,
+ rockchip);
+ if (ret)
+ return ret;
/* unmask hot reset/link-down reset */
val = FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
--
2.7.4
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are gated
2026-09-22 2:36 [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Shawn Lin
2026-09-22 2:37 ` [PATCH v3 1/3] PCI: dw-rockchip: Move the INTx irq setup to probe Shawn Lin
2026-09-22 2:37 ` [PATCH v3 2/3] PCI: dw-rockchip: Make the INTx irq setup devm-managed Shawn Lin
@ 2026-09-22 2:37 ` Shawn Lin
2026-09-23 9:13 ` [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Diederik de Haas
3 siblings, 0 replies; 10+ messages in thread
From: Shawn Lin @ 2026-09-22 2:37 UTC (permalink / raw)
To: Manivannan Sadhasivam, Bjorn Helgaas
Cc: linux-rockchip, linux-pci, Niklas Cassel, Shawn Lin
.reset_root_port() gates the controller clocks and PHY before
re-initializing the hardware. Devices on the bus may keep their INTx
line asserted across this window, so the chained INTx handler can run
and read the APB registers of the unclocked controller, raising a
synchronous external abort.
Mask the INTx IRQ before turning the clocks off and re-enable it once
the clocks are running again. On the error paths the clocks stay
gated, so the IRQ is deliberately left masked there.
Fixes: b376b3ff9cb0 ("PCI: dw-rockchip: Implement .reset_root_port() and use for link down")
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v3: None
Changes in v2:
- keep the INTx IRQ masked while .reset_root_port()
gates the controller clocks, responding to the Sashiko review
finding about accessing the unclocked APB bus.
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index f395a66..fd5cc6f 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -925,6 +925,16 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
u32 val;
int ret;
+ /*
+ * Devices may keep their INTx line asserted across the reset. Mask
+ * the INTx IRQ so that the chained handler does not touch the
+ * unclocked APB bus, which would raise a synchronous external abort.
+ * The IRQ is re-enabled once the clocks are restored, and is
+ * deliberately left masked on the error paths where the controller
+ * remains unclocked.
+ */
+ disable_irq(rockchip->intx_irq);
+
dw_pcie_stop_link(pci);
clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
rockchip_pcie_phy_deinit(rockchip);
@@ -975,6 +985,7 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
/* Ignore errors, the link may come up later */
dw_pcie_wait_for_link(pci);
+ enable_irq(rockchip->intx_irq);
dev_dbg(dev, "Root Port reset completed\n");
return ret;
--
2.7.4
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] PCI: dw-rockchip: Move the INTx irq setup to probe
2026-09-22 2:37 ` [PATCH v3 1/3] PCI: dw-rockchip: Move the INTx irq setup to probe Shawn Lin
@ 2026-09-22 9:56 ` Niklas Cassel
0 siblings, 0 replies; 10+ messages in thread
From: Niklas Cassel @ 2026-09-22 9:56 UTC (permalink / raw)
To: Shawn Lin; +Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci
Hello Shawn,
On Tue, Sep 22, 2026 at 10:37:00AM +0800, Shawn Lin wrote:
> Since commit b376b3ff9cb0 ("PCI: dw-rockchip: Implement .reset_root_port()
> and use for link down"), .reset_root_port() re-runs the host ops .init()
> callback to reprogram the Root Complex after a controller reset. That
> works for the register programming, but .init() is not re-entrant: it
> also creates the INTx irq domain and installs the chained INTx handler.
> Every root port reset therefore ends up with a second irq domain
> registered for the same fwnode: the previous one is leaked, as it is
> never removed, and worse, the INTx virqs of the downstream PCI devices
> were allocated in the previous irq domain and are never re-mapped, while
> the chained handler now looks up virqs in the new, empty domain. After a
> link down recovery, INTx interrupts are silently lost.
>
> Fix it by moving the of_irq_get_byname() lookup, the INTx irq domain
> creation and the chained handler installation out of .init() and into
> rockchip_pcie_configure_rc(), right after dw_pcie_host_init(). This
> mirrors how the qcom driver requests its global IRQ, and leaves .init()
> with nothing but idempotent register programming, so both
> .reset_root_port() and dw_pcie_resume_noirq() can safely re-run it.
> Re-running of_irq_get_byname() on every resume is also gone.
>
> rockchip_pcie_configure_rc() can abort probe, so propagate the irq
> domain creation error instead of only logging it as .init() used to do.
>
> Fixes: b376b3ff9cb0 ("PCI: dw-rockchip: Implement .reset_root_port() and use for link down")
> Suggested-by: Niklas Cassel <cassel@kernel.org>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
> Changes in v3:
> - split devm-managed part into a seperate patch
>
> Changes in v2:
> - Moved the of_irq_get_byname() lookup, the INTx irq domain creation
> and the chained handler installation out of the host ops .init()
> callback into rockchip_pcie_configure_rc(), right after
> dw_pcie_host_init(), as suggested by Niklas Cassel. This supersedes
> v1 patch 1/2, as .init() no longer creates the irq domain, and
> removes the rockchip_pcie_host_hw_init() helper from v1.
> - Made the INTx irq domain devm-managed with
> devm_irq_domain_instantiate() and uninstall the chained handler
> through a devres action, addressing the probe failure leak and
> use-after-free flagged by the Sashiko review.
>
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 32 +++++++++++++++------------
> 1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 1497686..8788a10 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -422,23 +422,9 @@ static void rockchip_pcie_stop_link(struct dw_pcie *pci)
> static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> - struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> - struct device *dev = rockchip->pci.dev;
> - int irq, ret;
> -
> - irq = of_irq_get_byname(dev->of_node, "legacy");
> - if (irq < 0)
> - return irq;
>
> pci->dbi_base2 = pci->dbi_base + PCIE_TYPE0_HDR_DBI2_OFFSET;
>
> - ret = rockchip_pcie_init_irq_domain(rockchip);
> - if (ret < 0)
> - dev_err(dev, "failed to init irq domain\n");
> -
> - irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
> - rockchip);
> -
> rockchip_pcie_configure_l1ss(pci);
> rockchip_pcie_enable_l0s(pci);
> pp->bridge->reset_root_port = rockchip_pcie_rc_reset_root_port;
> @@ -740,6 +726,24 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
> return ret;
> }
>
> + /*
> + * This is done here instead of in the host ops .init() callback,
> + * which is also re-run by .reset_root_port(), so that the INTx irq
> + * domain is only created once, at probe time.
> + */
I don't think a code comment is needed here.
But with or without the code comment:
Reviewed-by: Niklas Cassel <cassel@kernel.org>
> + irq = of_irq_get_byname(dev->of_node, "legacy");
> + if (irq < 0)
> + return irq;
> +
> + ret = rockchip_pcie_init_irq_domain(rockchip);
> + if (ret < 0) {
> + dev_err(dev, "failed to init irq domain\n");
> + return ret;
> + }
> +
> + irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
> + rockchip);
> +
> /* unmask hot reset/link-down reset */
> val = FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
> rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_MISC);
> --
> 2.7.4
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] PCI: dw-rockchip: Make the INTx irq setup devm-managed
2026-09-22 2:37 ` [PATCH v3 2/3] PCI: dw-rockchip: Make the INTx irq setup devm-managed Shawn Lin
@ 2026-09-22 9:58 ` Niklas Cassel
0 siblings, 0 replies; 10+ messages in thread
From: Niklas Cassel @ 2026-09-22 9:58 UTC (permalink / raw)
To: Shawn Lin; +Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci
On Tue, Sep 22, 2026 at 10:37:01AM +0800, Shawn Lin wrote:
> The irq domain and the chained handler set up by the previous commit
> are resources owned by the device, but nothing releases them if probing
> ever fails after they are set up, and they are not tied to the device
> lifetime in any other way either.
>
> Tie their lifetime to the device with devres: create the irq domain
> with devm_irq_domain_instantiate() and uninstall the chained handler
> through the rockchip_pcie_intx_chained_release() devres action. The
> driver is builtin and cannot be unbound (suppress_bind_attrs), so probe
> failure is the only path that ever needs this cleanup, and devres takes
> care of it without sprinkling it over every error path.
>
> Since the irq setup is the last step of rockchip_pcie_configure_rc(),
> the only failure point left after the chained handler is installed is
> devm_add_action_or_reset() itself, whose failure mode runs the action,
> so the handler can never run against the devm-freed rockchip structure.
> devres also unwinds in reverse registration order, so the handler is
> always uninstalled before the domain is removed. There is no devm API
> for chained handlers, hence the small devres action wrapper.
>
> While at it, drop the now unused rockchip variable from
> rockchip_pcie_host_init().
>
> Suggested-by: Niklas Cassel <cassel@kernel.org>
Thank you for giving me credit :)
Unfortunately, I don't think that I was the one who suggested this change.
I think you came up with the idea :)
Kind regards,
Niklas
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver
2026-09-22 2:36 [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Shawn Lin
` (2 preceding siblings ...)
2026-09-22 2:37 ` [PATCH v3 3/3] PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are gated Shawn Lin
@ 2026-09-23 9:13 ` Diederik de Haas
2026-09-23 9:21 ` Shawn Lin
3 siblings, 1 reply; 10+ messages in thread
From: Diederik de Haas @ 2026-09-23 9:13 UTC (permalink / raw)
To: Shawn Lin, Manivannan Sadhasivam, Bjorn Helgaas
Cc: linux-rockchip, linux-pci, Niklas Cassel
Hi,
On Tue Sep 22, 2026 at 4:36 AM CEST, Shawn Lin wrote:
>
> This short series fixes the INTx handling around the newly introduced
> .reset_root_port() (b376b3ff9cb0), and is split in three patches per
> Niklas' suggestion:
>
> Patch 1 stops .reset_root_port() from recreating the INTx irq domain
> on every root port reset, which leaked the old domain and silently
> broke INTx delivery afterwards, the downstream devices' virqs were
> allocated in the previous domain and were never re-mapped. By moving
> the of_irq_get_byname() lookup, the INTx irq domain creation and the
> chained handler installation into rockchip_pcie_configure_rc(), right
> after dw_pcie_host_init(). This leaves .init() with nothing but
> idempotent register programming, so it can safely be re-run by
> .reset_root_port() and dw_pcie_resume_noirq().
>
> Patch 2 makes the irq domain and the chained handler devm-managed, so
> that they are released with the device instead of leaking, which also
> addresses the probe failure leak/use-after-free flagged by the Sashiko
> review.
>
> Patch 3 keeps the INTx IRQ masked while .reset_root_port() gates the
> controller clocks, so the chained handler cannot read the unclocked
> APB bus and raise a synchronous external abort.
I build a kernel with this patch set (7.3~rc4-2) and got several warnings
like this, shortly followed by a stack trace:
irq: no irq domain found for legacy-interrupt-controller !
Then I build another kernel (7.3~rc4-3) where I disabled this patch set
and then I did not get the warnings/stack traces.
I was able to reproduce this on the following devices:
1) NanoPC-T6 Plus (RK3588)
2) Rock 5B (RK3588)
3) NanoPi-R5S (RK3568)
ad 3) Possibly unrelated, but I sometimes get this error:
gpio-keys gpio-keys: error -ENXIO: Unable to get irq number for GPIO
But that is *not* dependent on this patch set; I'm not sure if I've
seen it with this patch set. Could be because I haven't booted enough
with the 7.3~rc4-2 kernel. Or maybe this patch set fixed it?
The (only) correlation is that it has to do with IRQs.
I don't know if this patch set caused the warning/stack traces or just
brought an underlying issue to surface, but hopefully you do.
Warnings/stack trace on NanoPC-T6 Plus (because it's the most extensive):
```
root@nanopc-t6-plus:~# dmesg --level 4
[ 2.684054] pci 0003:30:00.0: Primary bus is hard wired to 0
[ 2.691234] irq: no irq domain found for legacy-interrupt-controller !
[ 3.043257] irq: no irq domain found for legacy-interrupt-controller !
[ 3.133600] ------------[ cut here ]------------
[ 3.133619] error: hwirq 0x0 is too large for :pcie@fe150000:legacy-interrupt-controller
[ 3.133639] WARNING: kernel/irq/irqdomain.c:676 at irq_domain_associate_locked+0x118/0x1a0, CPU#7: kworker/u32:5/60
[ 3.133662] Modules linked in: nvme rk808_regulator nvme_core nvme_keyring nvme_auth fusb302 tcpm rockchipdrm fan53555 aux_hpd_bridge dw_hdmi_qp rtc_hym8563 dw_mipi_dsi dw_hdmi analogix_dp drm_dp_aux_bus drm_display_helper rockchip_saradc fixed sdhci_of_dwcmshc cec phy_rockchip_usbdp sdhci_pltfm industrialio_triggered_buffer phy_rockchip_naneng_combphy dw_mmc_rockchip sdhci rc_core typec dw_mmc_pltfm gpio_rockchip phy_rockchip_samsung_hdptx display_connector phy_rockchip_snps_pcie3 kfifo_buf nvmem_rockchip_otp drm_client_lib cqhci ohci_platform spi_rockchip_sfc dw_mmc dw_wdt spi_rockchip rockchip_dfi pl330 drm_dma_helper ehci_platform drm_kms_helper dwc3 ehci_hcd drm ohci_hcd udc_core adc_keys usbcore i2c_rk3x phy_rockchip_inno_usb2 industrialio pwm_rockchip ulpi usb_common
[ 3.133815] CPU: 7 UID: 0 PID: 60 Comm: kworker/u32:5 Not tainted 7.3-rc4+unreleased-arm64-cknow #1 PREEMPTLAZY Debian 7.3~rc4-2
[ 3.133831] Hardware name: FriendlyElec NanoPC-T6 Plus (DT)
[ 3.133838] Workqueue: async async_run_entry_fn
[ 3.133852] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 3.133862] pc : irq_domain_associate_locked+0x118/0x1a0
[ 3.133872] lr : irq_domain_associate_locked+0x118/0x1a0
[ 3.133881] sp : ffff80008043b9c0
[ 3.133887] x29: ffff80008043b9c0 x28: 0000000000000000 x27: 0000000000000000
[ 3.133900] x26: ffff000100038c00 x25: 00000000fffffef7 x24: ffff0001095393e8
[ 3.133913] x23: 0000000000000000 x22: 0000000000000073 x21: 0000000000000000
[ 3.133925] x20: ffff0001196c9e30 x19: ffff000108e8d500 x18: 000000000000000a
[ 3.133937] x17: 7075727265746e69 x16: 2d79636167656c3a x15: 0720072007200720
[ 3.133949] x14: 0720072007200720 x13: 0720072007200720 x12: 000000000006ff90
[ 3.133961] x11: ffffc87b581950d0 x10: ffffc87b5810cf08 x9 : ffffc87b55db6444
[ 3.133974] x8 : ffffc87b5817d0e8 x7 : ffffffffffffefff x6 : 0000000000000001
[ 3.133985] x5 : ffffc87b5817d078 x4 : 0000000000000000 x3 : 0000000000000000
[ 3.133997] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000100b7a580
[ 3.134010] Call trace:
[ 3.134016] irq_domain_associate_locked+0x118/0x1a0 (P)
[ 3.134028] irq_create_mapping_affinity_locked+0x98/0x1b8
[ 3.134039] irq_create_fwspec_mapping+0x320/0x3e0
[ 3.134049] irq_create_of_mapping+0x74/0xb0
[ 3.134059] of_irq_parse_and_map_pci+0xf8/0x1f8
[ 3.134072] pci_assign_irq+0x9c/0x180
[ 3.134082] pci_device_probe+0x68/0x170
[ 3.134091] really_probe+0xc8/0x3f8
[ 3.134102] __driver_probe_device+0x168/0x1c8
[ 3.134110] driver_probe_device+0x44/0x128
[ 3.134119] __driver_attach_async_helper+0x58/0xf8
[ 3.134128] async_run_entry_fn+0x40/0x1a0
[ 3.134138] process_one_work+0x1cc/0x550
[ 3.134150] worker_thread+0x18c/0x2f0
[ 3.134161] kthread+0x134/0x150
[ 3.134171] ret_from_fork+0x10/0x20
[ 3.134183] ---[ end trace 0000000000000000 ]---
[ 3.331933] pci 0004:40:00.0: Primary bus is hard wired to 0
[ 3.338953] irq: no irq domain found for legacy-interrupt-controller !
[ 3.492147] ------------[ cut here ]------------
[ 3.492161] error: hwirq 0x0 is too large for :pcie@fe190000:legacy-interrupt-controller
[ 3.492182] WARNING: kernel/irq/irqdomain.c:676 at irq_domain_associate_locked+0x118/0x1a0, CPU#6: (udev-worker)/187
[ 3.492205] Modules linked in: r8169(+) realtek phy_package mdio_devres of_mdio fixed_phy fwnode_mdio libphy mdio_bus xhci_plat_hcd xhci_hcd nvme rk808_regulator nvme_core nvme_keyring nvme_auth fusb302 tcpm rockchipdrm fan53555 aux_hpd_bridge dw_hdmi_qp rtc_hym8563 dw_mipi_dsi dw_hdmi analogix_dp drm_dp_aux_bus drm_display_helper rockchip_saradc fixed sdhci_of_dwcmshc cec phy_rockchip_usbdp sdhci_pltfm industrialio_triggered_buffer phy_rockchip_naneng_combphy dw_mmc_rockchip sdhci rc_core typec dw_mmc_pltfm gpio_rockchip phy_rockchip_samsung_hdptx display_connector phy_rockchip_snps_pcie3 kfifo_buf nvmem_rockchip_otp drm_client_lib cqhci ohci_platform spi_rockchip_sfc dw_mmc dw_wdt spi_rockchip rockchip_dfi pl330 drm_dma_helper ehci_platform drm_kms_helper dwc3 ehci_hcd drm ohci_hcd udc_core adc_keys usbcore i2c_rk3x phy_rockchip_inno_usb2 industrialio pwm_rockchip ulpi usb_common
[ 3.492388] CPU: 6 UID: 0 PID: 187 Comm: (udev-worker) Tainted: G W 7.3-rc4+unreleased-arm64-cknow #1 PREEMPTLAZY Debian 7.3~rc4-2
[ 3.492404] Tainted: [W]=WARN
[ 3.492410] Hardware name: FriendlyElec NanoPC-T6 Plus (DT)
[ 3.492417] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 3.492427] pc : irq_domain_associate_locked+0x118/0x1a0
[ 3.492437] lr : irq_domain_associate_locked+0x118/0x1a0
[ 3.492446] sp : ffff8000818335e0
[ 3.492451] x29: ffff8000818335e0 x28: ffffc87b5865ccb8 x27: ffffc87ae59d7818
[ 3.492465] x26: 000000000000000c x25: ffff000103534310 x24: ffff00010a50cde8
[ 3.492477] x23: 0000000000000000 x22: 0000000000000088 x21: 0000000000000000
[ 3.492490] x20: ffff000168464630 x19: ffff000108fb2a00 x18: 000000000000000a
[ 3.492502] x17: 7075727265746e69 x16: 2d79636167656c3a x15: 0720072007200720
[ 3.492514] x14: 0720072007200720 x13: 0720072007200720 x12: 000000000006ff90
[ 3.492526] x11: ffffc87b581950d0 x10: ffffc87b5810cf08 x9 : ffffc87b55db6444
[ 3.492538] x8 : ffffc87b5817d0e8 x7 : ffffffffffffefff x6 : 0000000000000001
[ 3.492550] x5 : ffffc87b5817d078 x4 : 0000000000000000 x3 : 0000000000000000
[ 3.492562] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0001097192c0
[ 3.492574] Call trace:
[ 3.492579] irq_domain_associate_locked+0x118/0x1a0 (P)
[ 3.492591] irq_create_mapping_affinity_locked+0x98/0x1b8
[ 3.492602] irq_create_fwspec_mapping+0x320/0x3e0
[ 3.492613] irq_create_of_mapping+0x74/0xb0
[ 3.492623] of_irq_parse_and_map_pci+0xf8/0x1f8
[ 3.492635] pci_assign_irq+0x9c/0x180
[ 3.492646] pci_device_probe+0x68/0x170
[ 3.492656] really_probe+0xc8/0x3f8
[ 3.492666] __driver_probe_device+0x168/0x1c8
[ 3.492675] driver_probe_device+0x44/0x128
[ 3.492683] __driver_attach+0xd0/0x228
[ 3.492692] bus_for_each_dev+0x84/0xf0
[ 3.492704] driver_attach+0x2c/0x40
[ 3.492712] bus_add_driver+0x124/0x280
[ 3.492720] driver_register+0x70/0x138
[ 3.492729] __pci_register_driver+0x48/0x60
[ 3.492742] rtl8169_pci_driver_init+0x30/0xfd0 [r8169]
[ 3.492768] do_one_initcall+0x5c/0x458
[ 3.492778] do_init_module+0x5c/0x280
[ 3.492788] load_module+0x1cc0/0x25b8
[ 3.492796] init_module_from_file+0xe8/0x158
[ 3.492805] __arm64_sys_finit_module+0x208/0x360
[ 3.492814] invoke_syscall.constprop.0+0xac/0x110
[ 3.492829] el0_svc_common.constprop.0+0x40/0xf0
[ 3.492842] do_el0_svc+0x24/0x40
[ 3.492854] el0_svc+0x40/0x260
[ 3.492867] el0t_64_sync_handler+0xa0/0xe8
[ 3.492879] el0t_64_sync+0x198/0x1a0
[ 3.492888] ---[ end trace 0000000000000000 ]---
[ 3.554818] pci 0002:20:00.0: Primary bus is hard wired to 0
[ 3.563276] irq: no irq domain found for legacy-interrupt-controller !
[ 3.569263] irq: no irq domain found for legacy-interrupt-controller !
[ 8.679178] panthor fb000000.gpu: [drm] Firmware protected mode entry is not supported, ignoring
[ 8.871598] rockchip-i2s-tdm fddf0000.i2s: using zero-initialized flat cache, this may cause unexpected behavior
[ 9.004753] ------------[ cut here ]------------
[ 9.004768] error: hwirq 0x0 is too large for :pcie@fe180000:legacy-interrupt-controller
[ 9.004779] WARNING: kernel/irq/irqdomain.c:676 at irq_domain_associate_locked+0x118/0x1a0, CPU#5: (udev-worker)/399
[ 9.004791] Modules linked in: mt7925e(+) aes_ce_blk mt7925_common snd_soc_audio_graph_card(+) ghash_ce gf128mul mt792x_lib pwm_fan mt76_connac_lib btusb leds_gpio snd_soc_simple_card gpio_ir_recv btrtl snd_soc_simple_card_utils mt76 btintel rk805_pwrkey btbcm snd_soc_hdmi_codec ofpart snd_soc_rockchip_i2s_tdm snd_soc_es8389 mac80211 btmtk snd_soc_core rockchip_thermal bluetooth rockchip_vdec synopsys_hdmirx spi_nor v4l2_vp9 rockchip_rga snd_compress mtd v4l2_dv_timings v4l2_h264 ecdh_generic snd_pcm_dmaengine videobuf2_dma_contig v4l2_mem2mem rockchip_rng videobuf2_dma_sg snd_pcm videobuf2_memops cfg80211 videobuf2_v4l2 videodev snd_timer panthor snd rocket drm_gpuvm videobuf2_common rfkill soundcore gpu_sched libarc4 vsi_iommu mc drm_shmem_helper drm_exec cpufreq_dt evdev pkcs8_key_parser nvme_fabrics efi_pstore configfs autofs4 ext4 crc16 mbcache jbd2 onboard_usb_dev r8169 realtek phy_package mdio_devres of_mdio fixed_phy fwnode_mdio libphy mdio_bus xhci_plat_hcd xhci_hcd nvme rk808_regulator nvme_core nvme_keyring
[ 9.004872] nvme_auth fusb302 tcpm rockchipdrm fan53555 aux_hpd_bridge dw_hdmi_qp rtc_hym8563 dw_mipi_dsi dw_hdmi analogix_dp drm_dp_aux_bus drm_display_helper rockchip_saradc fixed sdhci_of_dwcmshc cec phy_rockchip_usbdp sdhci_pltfm industrialio_triggered_buffer phy_rockchip_naneng_combphy dw_mmc_rockchip sdhci rc_core typec dw_mmc_pltfm gpio_rockchip phy_rockchip_samsung_hdptx display_connector phy_rockchip_snps_pcie3 kfifo_buf nvmem_rockchip_otp drm_client_lib cqhci ohci_platform spi_rockchip_sfc dw_mmc dw_wdt spi_rockchip rockchip_dfi pl330 drm_dma_helper ehci_platform drm_kms_helper dwc3 ehci_hcd drm ohci_hcd udc_core adc_keys usbcore i2c_rk3x phy_rockchip_inno_usb2 industrialio pwm_rockchip ulpi usb_common
[ 9.004964] CPU: 5 UID: 0 PID: 399 Comm: (udev-worker) Tainted: G W 7.3-rc4+unreleased-arm64-cknow #1 PREEMPTLAZY Debian 7.3~rc4-2
[ 9.004971] Tainted: [W]=WARN
[ 9.004974] Hardware name: FriendlyElec NanoPC-T6 Plus (DT)
[ 9.004977] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 9.004982] pc : irq_domain_associate_locked+0x118/0x1a0
[ 9.004986] lr : irq_domain_associate_locked+0x118/0x1a0
[ 9.004989] sp : ffff800083c734d0
[ 9.004992] x29: ffff800083c734d0 x28: ffffc87b5865ccb8 x27: ffffc87ae61951d8
[ 9.004998] x26: 000000000000000c x25: ffff0001035340b0 x24: ffff000107ec6ae8
[ 9.005003] x23: 0000000000000000 x22: 00000000000000a3 x21: 0000000000000000
[ 9.005008] x20: ffff0001180b0030 x19: ffff000108e26200 x18: 000000000000000a
[ 9.005013] x17: 7075727265746e69 x16: 2d79636167656c3a x15: 0720072007200720
[ 9.005018] x14: 0720072007200720 x13: 0720072007200720 x12: 000000000006ff90
[ 9.005023] x11: ffffc87b581950d0 x10: ffffc87b5810cf08 x9 : ffffc87b55db6444
[ 9.005028] x8 : ffffc87b5817d0e8 x7 : ffffffffffffefff x6 : 0000000000000001
[ 9.005033] x5 : ffff0005fdeff208 x4 : ffff378aa5f0e000 x3 : ffff000168385dc0
[ 9.005038] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000168385dc0
[ 9.005044] Call trace:
[ 9.005046] irq_domain_associate_locked+0x118/0x1a0 (P)
[ 9.005052] irq_create_mapping_affinity_locked+0x98/0x1b8
[ 9.005056] irq_create_fwspec_mapping+0x320/0x3e0
[ 9.005061] irq_create_of_mapping+0x74/0xb0
[ 9.005065] of_irq_parse_and_map_pci+0xf8/0x1f8
[ 9.005071] pci_assign_irq+0x9c/0x180
[ 9.005076] pci_device_probe+0x68/0x170
[ 9.005080] really_probe+0xc8/0x3f8
[ 9.005085] __driver_probe_device+0x168/0x1c8
[ 9.005088] driver_probe_device+0x44/0x128
[ 9.005092] __driver_attach+0xd0/0x228
[ 9.005095] bus_for_each_dev+0x84/0xf0
[ 9.005100] driver_attach+0x2c/0x40
[ 9.005103] bus_add_driver+0x124/0x280
[ 9.005106] driver_register+0x70/0x138
[ 9.005110] __pci_register_driver+0x48/0x60
[ 9.005116] mt7925_pci_driver_init+0x30/0xfd0 [mt7925e]
[ 9.005125] do_one_initcall+0x5c/0x458
[ 9.005129] do_init_module+0x5c/0x280
[ 9.005134] load_module+0x1cc0/0x25b8
[ 9.005137] init_module_from_file+0xe8/0x158
[ 9.005141] __arm64_sys_finit_module+0x208/0x360
[ 9.005144] invoke_syscall.constprop.0+0xac/0x110
[ 9.005151] el0_svc_common.constprop.0+0xc0/0xf0
[ 9.005156] do_el0_svc+0x24/0x40
[ 9.005161] el0_svc+0x40/0x260
[ 9.005167] el0t_64_sync_handler+0xa0/0xe8
[ 9.005171] el0t_64_sync+0x198/0x1a0
[ 9.005176] ---[ end trace 0000000000000000 ]---
[ 10.568235] Bluetooth: hci0: HCI Enhanced Setup Synchronous Connection command is advertised, but not supported.
```
The ``mt7925`` one is from my M.2 Wi-Fi+BT card plugged into the system.
rk3588-nanopc-t6-plus-intx-fixes-stacktraces-dmesg.txt:
https://paste.sr.ht/~diederik/85ee576b76934754139a496488d451ef89f88db0
rk3588-rock5b-intx-fixes-stacktraces-dmesg.txt:
https://paste.sr.ht/~diederik/c6d46b2e5e0de22316bd31ad63152afed1a2dc70
rk3568-nanopi-r5s-intx-fixes-stacktraces-dmesg.txt:
https://paste.sr.ht/~diederik/9b83b304646df3823687b75e5e07c683658d1024
Cheers,
Diederik
> Changes in v3:
> - split devm-managed part into a seperate patch
>
> Changes in v2:
> - Moved the of_irq_get_byname() lookup, the INTx irq domain creation
> and the chained handler installation out of the host ops .init()
> callback into rockchip_pcie_configure_rc(), right after
> dw_pcie_host_init(), as suggested by Niklas Cassel. This supersedes
> v1 patch 1/2, as .init() no longer creates the irq domain, and
> removes the rockchip_pcie_host_hw_init() helper from v1.
> - Made the INTx irq domain devm-managed with
> devm_irq_domain_instantiate() and uninstall the chained handler
> through a devres action, addressing the probe failure leak and
> use-after-free flagged by the Sashiko review.
> - keep the INTx IRQ masked while .reset_root_port()
> gates the controller clocks, responding to the Sashiko review
> finding about accessing the unclocked APB bus.
>
> Shawn Lin (3):
> PCI: dw-rockchip: Move the INTx irq setup to probe
> PCI: dw-rockchip: Make the INTx irq setup devm-managed
> PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are
> gated
>
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 77 ++++++++++++++++++++-------
> 1 file changed, 57 insertions(+), 20 deletions(-)
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver
2026-09-23 9:13 ` [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Diederik de Haas
@ 2026-09-23 9:21 ` Shawn Lin
2026-09-23 15:25 ` Niklas Cassel
0 siblings, 1 reply; 10+ messages in thread
From: Shawn Lin @ 2026-09-23 9:21 UTC (permalink / raw)
To: Diederik de Haas
Cc: shawn.lin, linux-rockchip, linux-pci, Niklas Cassel,
Manivannan Sadhasivam, Bjorn Helgaas
在 2026/09/23 星期三 17:13, Diederik de Haas 写道:
> Hi,
>
> On Tue Sep 22, 2026 at 4:36 AM CEST, Shawn Lin wrote:
>>
>> This short series fixes the INTx handling around the newly introduced
>> .reset_root_port() (b376b3ff9cb0), and is split in three patches per
>> Niklas' suggestion:
>>
>> Patch 1 stops .reset_root_port() from recreating the INTx irq domain
>> on every root port reset, which leaked the old domain and silently
>> broke INTx delivery afterwards, the downstream devices' virqs were
>> allocated in the previous domain and were never re-mapped. By moving
>> the of_irq_get_byname() lookup, the INTx irq domain creation and the
>> chained handler installation into rockchip_pcie_configure_rc(), right
>> after dw_pcie_host_init(). This leaves .init() with nothing but
>> idempotent register programming, so it can safely be re-run by
>> .reset_root_port() and dw_pcie_resume_noirq().
>>
>> Patch 2 makes the irq domain and the chained handler devm-managed, so
>> that they are released with the device instead of leaking, which also
>> addresses the probe failure leak/use-after-free flagged by the Sashiko
>> review.
>>
>> Patch 3 keeps the INTx IRQ masked while .reset_root_port() gates the
>> controller clocks, so the chained handler cannot read the unclocked
>> APB bus and raise a synchronous external abort.
>
> I build a kernel with this patch set (7.3~rc4-2) and got several warnings
> like this, shortly followed by a stack trace:
Sashiko already reportted some valid concern around this, and I'll plan
to rework this series a bit later. Thanks for reporting this.
>
> irq: no irq domain found for legacy-interrupt-controller !
>
> Then I build another kernel (7.3~rc4-3) where I disabled this patch set
> and then I did not get the warnings/stack traces.
>
> I was able to reproduce this on the following devices:
> 1) NanoPC-T6 Plus (RK3588)
> 2) Rock 5B (RK3588)
> 3) NanoPi-R5S (RK3568)
>
> ad 3) Possibly unrelated, but I sometimes get this error:
>
> gpio-keys gpio-keys: error -ENXIO: Unable to get irq number for GPIO
>
> But that is *not* dependent on this patch set; I'm not sure if I've
> seen it with this patch set. Could be because I haven't booted enough
> with the 7.3~rc4-2 kernel. Or maybe this patch set fixed it?
> The (only) correlation is that it has to do with IRQs.
>
> I don't know if this patch set caused the warning/stack traces or just
> brought an underlying issue to surface, but hopefully you do.
>
> Warnings/stack trace on NanoPC-T6 Plus (because it's the most extensive):
>
> ```
> root@nanopc-t6-plus:~# dmesg --level 4
> [ 2.684054] pci 0003:30:00.0: Primary bus is hard wired to 0
> [ 2.691234] irq: no irq domain found for legacy-interrupt-controller !
> [ 3.043257] irq: no irq domain found for legacy-interrupt-controller !
> [ 3.133600] ------------[ cut here ]------------
> [ 3.133619] error: hwirq 0x0 is too large for :pcie@fe150000:legacy-interrupt-controller
> [ 3.133639] WARNING: kernel/irq/irqdomain.c:676 at irq_domain_associate_locked+0x118/0x1a0, CPU#7: kworker/u32:5/60
> [ 3.133662] Modules linked in: nvme rk808_regulator nvme_core nvme_keyring nvme_auth fusb302 tcpm rockchipdrm fan53555 aux_hpd_bridge dw_hdmi_qp rtc_hym8563 dw_mipi_dsi dw_hdmi analogix_dp drm_dp_aux_bus drm_display_helper rockchip_saradc fixed sdhci_of_dwcmshc cec phy_rockchip_usbdp sdhci_pltfm industrialio_triggered_buffer phy_rockchip_naneng_combphy dw_mmc_rockchip sdhci rc_core typec dw_mmc_pltfm gpio_rockchip phy_rockchip_samsung_hdptx display_connector phy_rockchip_snps_pcie3 kfifo_buf nvmem_rockchip_otp drm_client_lib cqhci ohci_platform spi_rockchip_sfc dw_mmc dw_wdt spi_rockchip rockchip_dfi pl330 drm_dma_helper ehci_platform drm_kms_helper dwc3 ehci_hcd drm ohci_hcd udc_core adc_keys usbcore i2c_rk3x phy_rockchip_inno_usb2 industrialio pwm_rockchip ulpi usb_common
> [ 3.133815] CPU: 7 UID: 0 PID: 60 Comm: kworker/u32:5 Not tainted 7.3-rc4+unreleased-arm64-cknow #1 PREEMPTLAZY Debian 7.3~rc4-2
> [ 3.133831] Hardware name: FriendlyElec NanoPC-T6 Plus (DT)
> [ 3.133838] Workqueue: async async_run_entry_fn
> [ 3.133852] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 3.133862] pc : irq_domain_associate_locked+0x118/0x1a0
> [ 3.133872] lr : irq_domain_associate_locked+0x118/0x1a0
> [ 3.133881] sp : ffff80008043b9c0
> [ 3.133887] x29: ffff80008043b9c0 x28: 0000000000000000 x27: 0000000000000000
> [ 3.133900] x26: ffff000100038c00 x25: 00000000fffffef7 x24: ffff0001095393e8
> [ 3.133913] x23: 0000000000000000 x22: 0000000000000073 x21: 0000000000000000
> [ 3.133925] x20: ffff0001196c9e30 x19: ffff000108e8d500 x18: 000000000000000a
> [ 3.133937] x17: 7075727265746e69 x16: 2d79636167656c3a x15: 0720072007200720
> [ 3.133949] x14: 0720072007200720 x13: 0720072007200720 x12: 000000000006ff90
> [ 3.133961] x11: ffffc87b581950d0 x10: ffffc87b5810cf08 x9 : ffffc87b55db6444
> [ 3.133974] x8 : ffffc87b5817d0e8 x7 : ffffffffffffefff x6 : 0000000000000001
> [ 3.133985] x5 : ffffc87b5817d078 x4 : 0000000000000000 x3 : 0000000000000000
> [ 3.133997] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000100b7a580
> [ 3.134010] Call trace:
> [ 3.134016] irq_domain_associate_locked+0x118/0x1a0 (P)
> [ 3.134028] irq_create_mapping_affinity_locked+0x98/0x1b8
> [ 3.134039] irq_create_fwspec_mapping+0x320/0x3e0
> [ 3.134049] irq_create_of_mapping+0x74/0xb0
> [ 3.134059] of_irq_parse_and_map_pci+0xf8/0x1f8
> [ 3.134072] pci_assign_irq+0x9c/0x180
> [ 3.134082] pci_device_probe+0x68/0x170
> [ 3.134091] really_probe+0xc8/0x3f8
> [ 3.134102] __driver_probe_device+0x168/0x1c8
> [ 3.134110] driver_probe_device+0x44/0x128
> [ 3.134119] __driver_attach_async_helper+0x58/0xf8
> [ 3.134128] async_run_entry_fn+0x40/0x1a0
> [ 3.134138] process_one_work+0x1cc/0x550
> [ 3.134150] worker_thread+0x18c/0x2f0
> [ 3.134161] kthread+0x134/0x150
> [ 3.134171] ret_from_fork+0x10/0x20
> [ 3.134183] ---[ end trace 0000000000000000 ]---
> [ 3.331933] pci 0004:40:00.0: Primary bus is hard wired to 0
> [ 3.338953] irq: no irq domain found for legacy-interrupt-controller !
> [ 3.492147] ------------[ cut here ]------------
> [ 3.492161] error: hwirq 0x0 is too large for :pcie@fe190000:legacy-interrupt-controller
> [ 3.492182] WARNING: kernel/irq/irqdomain.c:676 at irq_domain_associate_locked+0x118/0x1a0, CPU#6: (udev-worker)/187
> [ 3.492205] Modules linked in: r8169(+) realtek phy_package mdio_devres of_mdio fixed_phy fwnode_mdio libphy mdio_bus xhci_plat_hcd xhci_hcd nvme rk808_regulator nvme_core nvme_keyring nvme_auth fusb302 tcpm rockchipdrm fan53555 aux_hpd_bridge dw_hdmi_qp rtc_hym8563 dw_mipi_dsi dw_hdmi analogix_dp drm_dp_aux_bus drm_display_helper rockchip_saradc fixed sdhci_of_dwcmshc cec phy_rockchip_usbdp sdhci_pltfm industrialio_triggered_buffer phy_rockchip_naneng_combphy dw_mmc_rockchip sdhci rc_core typec dw_mmc_pltfm gpio_rockchip phy_rockchip_samsung_hdptx display_connector phy_rockchip_snps_pcie3 kfifo_buf nvmem_rockchip_otp drm_client_lib cqhci ohci_platform spi_rockchip_sfc dw_mmc dw_wdt spi_rockchip rockchip_dfi pl330 drm_dma_helper ehci_platform drm_kms_helper dwc3 ehci_hcd drm ohci_hcd udc_core adc_keys usbcore i2c_rk3x phy_rockchip_inno_usb2 industrialio pwm_rockchip ulpi usb_common
> [ 3.492388] CPU: 6 UID: 0 PID: 187 Comm: (udev-worker) Tainted: G W 7.3-rc4+unreleased-arm64-cknow #1 PREEMPTLAZY Debian 7.3~rc4-2
> [ 3.492404] Tainted: [W]=WARN
> [ 3.492410] Hardware name: FriendlyElec NanoPC-T6 Plus (DT)
> [ 3.492417] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 3.492427] pc : irq_domain_associate_locked+0x118/0x1a0
> [ 3.492437] lr : irq_domain_associate_locked+0x118/0x1a0
> [ 3.492446] sp : ffff8000818335e0
> [ 3.492451] x29: ffff8000818335e0 x28: ffffc87b5865ccb8 x27: ffffc87ae59d7818
> [ 3.492465] x26: 000000000000000c x25: ffff000103534310 x24: ffff00010a50cde8
> [ 3.492477] x23: 0000000000000000 x22: 0000000000000088 x21: 0000000000000000
> [ 3.492490] x20: ffff000168464630 x19: ffff000108fb2a00 x18: 000000000000000a
> [ 3.492502] x17: 7075727265746e69 x16: 2d79636167656c3a x15: 0720072007200720
> [ 3.492514] x14: 0720072007200720 x13: 0720072007200720 x12: 000000000006ff90
> [ 3.492526] x11: ffffc87b581950d0 x10: ffffc87b5810cf08 x9 : ffffc87b55db6444
> [ 3.492538] x8 : ffffc87b5817d0e8 x7 : ffffffffffffefff x6 : 0000000000000001
> [ 3.492550] x5 : ffffc87b5817d078 x4 : 0000000000000000 x3 : 0000000000000000
> [ 3.492562] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0001097192c0
> [ 3.492574] Call trace:
> [ 3.492579] irq_domain_associate_locked+0x118/0x1a0 (P)
> [ 3.492591] irq_create_mapping_affinity_locked+0x98/0x1b8
> [ 3.492602] irq_create_fwspec_mapping+0x320/0x3e0
> [ 3.492613] irq_create_of_mapping+0x74/0xb0
> [ 3.492623] of_irq_parse_and_map_pci+0xf8/0x1f8
> [ 3.492635] pci_assign_irq+0x9c/0x180
> [ 3.492646] pci_device_probe+0x68/0x170
> [ 3.492656] really_probe+0xc8/0x3f8
> [ 3.492666] __driver_probe_device+0x168/0x1c8
> [ 3.492675] driver_probe_device+0x44/0x128
> [ 3.492683] __driver_attach+0xd0/0x228
> [ 3.492692] bus_for_each_dev+0x84/0xf0
> [ 3.492704] driver_attach+0x2c/0x40
> [ 3.492712] bus_add_driver+0x124/0x280
> [ 3.492720] driver_register+0x70/0x138
> [ 3.492729] __pci_register_driver+0x48/0x60
> [ 3.492742] rtl8169_pci_driver_init+0x30/0xfd0 [r8169]
> [ 3.492768] do_one_initcall+0x5c/0x458
> [ 3.492778] do_init_module+0x5c/0x280
> [ 3.492788] load_module+0x1cc0/0x25b8
> [ 3.492796] init_module_from_file+0xe8/0x158
> [ 3.492805] __arm64_sys_finit_module+0x208/0x360
> [ 3.492814] invoke_syscall.constprop.0+0xac/0x110
> [ 3.492829] el0_svc_common.constprop.0+0x40/0xf0
> [ 3.492842] do_el0_svc+0x24/0x40
> [ 3.492854] el0_svc+0x40/0x260
> [ 3.492867] el0t_64_sync_handler+0xa0/0xe8
> [ 3.492879] el0t_64_sync+0x198/0x1a0
> [ 3.492888] ---[ end trace 0000000000000000 ]---
> [ 3.554818] pci 0002:20:00.0: Primary bus is hard wired to 0
> [ 3.563276] irq: no irq domain found for legacy-interrupt-controller !
> [ 3.569263] irq: no irq domain found for legacy-interrupt-controller !
> [ 8.679178] panthor fb000000.gpu: [drm] Firmware protected mode entry is not supported, ignoring
> [ 8.871598] rockchip-i2s-tdm fddf0000.i2s: using zero-initialized flat cache, this may cause unexpected behavior
> [ 9.004753] ------------[ cut here ]------------
> [ 9.004768] error: hwirq 0x0 is too large for :pcie@fe180000:legacy-interrupt-controller
> [ 9.004779] WARNING: kernel/irq/irqdomain.c:676 at irq_domain_associate_locked+0x118/0x1a0, CPU#5: (udev-worker)/399
> [ 9.004791] Modules linked in: mt7925e(+) aes_ce_blk mt7925_common snd_soc_audio_graph_card(+) ghash_ce gf128mul mt792x_lib pwm_fan mt76_connac_lib btusb leds_gpio snd_soc_simple_card gpio_ir_recv btrtl snd_soc_simple_card_utils mt76 btintel rk805_pwrkey btbcm snd_soc_hdmi_codec ofpart snd_soc_rockchip_i2s_tdm snd_soc_es8389 mac80211 btmtk snd_soc_core rockchip_thermal bluetooth rockchip_vdec synopsys_hdmirx spi_nor v4l2_vp9 rockchip_rga snd_compress mtd v4l2_dv_timings v4l2_h264 ecdh_generic snd_pcm_dmaengine videobuf2_dma_contig v4l2_mem2mem rockchip_rng videobuf2_dma_sg snd_pcm videobuf2_memops cfg80211 videobuf2_v4l2 videodev snd_timer panthor snd rocket drm_gpuvm videobuf2_common rfkill soundcore gpu_sched libarc4 vsi_iommu mc drm_shmem_helper drm_exec cpufreq_dt evdev pkcs8_key_parser nvme_fabrics efi_pstore configfs autofs4 ext4 crc16 mbcache jbd2 onboard_usb_dev r8169 realtek phy_package mdio_devres of_mdio fixed_phy fwnode_mdio libphy mdio_bus xhci_plat_hcd xhci_hcd nvme rk808_regulator nvme_core nvme_keyring
> [ 9.004872] nvme_auth fusb302 tcpm rockchipdrm fan53555 aux_hpd_bridge dw_hdmi_qp rtc_hym8563 dw_mipi_dsi dw_hdmi analogix_dp drm_dp_aux_bus drm_display_helper rockchip_saradc fixed sdhci_of_dwcmshc cec phy_rockchip_usbdp sdhci_pltfm industrialio_triggered_buffer phy_rockchip_naneng_combphy dw_mmc_rockchip sdhci rc_core typec dw_mmc_pltfm gpio_rockchip phy_rockchip_samsung_hdptx display_connector phy_rockchip_snps_pcie3 kfifo_buf nvmem_rockchip_otp drm_client_lib cqhci ohci_platform spi_rockchip_sfc dw_mmc dw_wdt spi_rockchip rockchip_dfi pl330 drm_dma_helper ehci_platform drm_kms_helper dwc3 ehci_hcd drm ohci_hcd udc_core adc_keys usbcore i2c_rk3x phy_rockchip_inno_usb2 industrialio pwm_rockchip ulpi usb_common
> [ 9.004964] CPU: 5 UID: 0 PID: 399 Comm: (udev-worker) Tainted: G W 7.3-rc4+unreleased-arm64-cknow #1 PREEMPTLAZY Debian 7.3~rc4-2
> [ 9.004971] Tainted: [W]=WARN
> [ 9.004974] Hardware name: FriendlyElec NanoPC-T6 Plus (DT)
> [ 9.004977] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 9.004982] pc : irq_domain_associate_locked+0x118/0x1a0
> [ 9.004986] lr : irq_domain_associate_locked+0x118/0x1a0
> [ 9.004989] sp : ffff800083c734d0
> [ 9.004992] x29: ffff800083c734d0 x28: ffffc87b5865ccb8 x27: ffffc87ae61951d8
> [ 9.004998] x26: 000000000000000c x25: ffff0001035340b0 x24: ffff000107ec6ae8
> [ 9.005003] x23: 0000000000000000 x22: 00000000000000a3 x21: 0000000000000000
> [ 9.005008] x20: ffff0001180b0030 x19: ffff000108e26200 x18: 000000000000000a
> [ 9.005013] x17: 7075727265746e69 x16: 2d79636167656c3a x15: 0720072007200720
> [ 9.005018] x14: 0720072007200720 x13: 0720072007200720 x12: 000000000006ff90
> [ 9.005023] x11: ffffc87b581950d0 x10: ffffc87b5810cf08 x9 : ffffc87b55db6444
> [ 9.005028] x8 : ffffc87b5817d0e8 x7 : ffffffffffffefff x6 : 0000000000000001
> [ 9.005033] x5 : ffff0005fdeff208 x4 : ffff378aa5f0e000 x3 : ffff000168385dc0
> [ 9.005038] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000168385dc0
> [ 9.005044] Call trace:
> [ 9.005046] irq_domain_associate_locked+0x118/0x1a0 (P)
> [ 9.005052] irq_create_mapping_affinity_locked+0x98/0x1b8
> [ 9.005056] irq_create_fwspec_mapping+0x320/0x3e0
> [ 9.005061] irq_create_of_mapping+0x74/0xb0
> [ 9.005065] of_irq_parse_and_map_pci+0xf8/0x1f8
> [ 9.005071] pci_assign_irq+0x9c/0x180
> [ 9.005076] pci_device_probe+0x68/0x170
> [ 9.005080] really_probe+0xc8/0x3f8
> [ 9.005085] __driver_probe_device+0x168/0x1c8
> [ 9.005088] driver_probe_device+0x44/0x128
> [ 9.005092] __driver_attach+0xd0/0x228
> [ 9.005095] bus_for_each_dev+0x84/0xf0
> [ 9.005100] driver_attach+0x2c/0x40
> [ 9.005103] bus_add_driver+0x124/0x280
> [ 9.005106] driver_register+0x70/0x138
> [ 9.005110] __pci_register_driver+0x48/0x60
> [ 9.005116] mt7925_pci_driver_init+0x30/0xfd0 [mt7925e]
> [ 9.005125] do_one_initcall+0x5c/0x458
> [ 9.005129] do_init_module+0x5c/0x280
> [ 9.005134] load_module+0x1cc0/0x25b8
> [ 9.005137] init_module_from_file+0xe8/0x158
> [ 9.005141] __arm64_sys_finit_module+0x208/0x360
> [ 9.005144] invoke_syscall.constprop.0+0xac/0x110
> [ 9.005151] el0_svc_common.constprop.0+0xc0/0xf0
> [ 9.005156] do_el0_svc+0x24/0x40
> [ 9.005161] el0_svc+0x40/0x260
> [ 9.005167] el0t_64_sync_handler+0xa0/0xe8
> [ 9.005171] el0t_64_sync+0x198/0x1a0
> [ 9.005176] ---[ end trace 0000000000000000 ]---
> [ 10.568235] Bluetooth: hci0: HCI Enhanced Setup Synchronous Connection command is advertised, but not supported.
> ```
>
> The ``mt7925`` one is from my M.2 Wi-Fi+BT card plugged into the system.
>
> rk3588-nanopc-t6-plus-intx-fixes-stacktraces-dmesg.txt:
> https://paste.sr.ht/~diederik/85ee576b76934754139a496488d451ef89f88db0
>
> rk3588-rock5b-intx-fixes-stacktraces-dmesg.txt:
> https://paste.sr.ht/~diederik/c6d46b2e5e0de22316bd31ad63152afed1a2dc70
>
> rk3568-nanopi-r5s-intx-fixes-stacktraces-dmesg.txt:
> https://paste.sr.ht/~diederik/9b83b304646df3823687b75e5e07c683658d1024
>
> Cheers,
> Diederik
>
>> Changes in v3:
>> - split devm-managed part into a seperate patch
>>
>> Changes in v2:
>> - Moved the of_irq_get_byname() lookup, the INTx irq domain creation
>> and the chained handler installation out of the host ops .init()
>> callback into rockchip_pcie_configure_rc(), right after
>> dw_pcie_host_init(), as suggested by Niklas Cassel. This supersedes
>> v1 patch 1/2, as .init() no longer creates the irq domain, and
>> removes the rockchip_pcie_host_hw_init() helper from v1.
>> - Made the INTx irq domain devm-managed with
>> devm_irq_domain_instantiate() and uninstall the chained handler
>> through a devres action, addressing the probe failure leak and
>> use-after-free flagged by the Sashiko review.
>> - keep the INTx IRQ masked while .reset_root_port()
>> gates the controller clocks, responding to the Sashiko review
>> finding about accessing the unclocked APB bus.
>>
>> Shawn Lin (3):
>> PCI: dw-rockchip: Move the INTx irq setup to probe
>> PCI: dw-rockchip: Make the INTx irq setup devm-managed
>> PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are
>> gated
>>
>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 77 ++++++++++++++++++++-------
>> 1 file changed, 57 insertions(+), 20 deletions(-)
>
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver
2026-09-23 9:21 ` Shawn Lin
@ 2026-09-23 15:25 ` Niklas Cassel
2026-09-24 3:27 ` Shawn Lin
0 siblings, 1 reply; 10+ messages in thread
From: Niklas Cassel @ 2026-09-23 15:25 UTC (permalink / raw)
To: Shawn Lin
Cc: Diederik de Haas, linux-rockchip, linux-pci,
Manivannan Sadhasivam, Bjorn Helgaas
Hello Shawn, Diederik,
On Wed, Sep 23, 2026 at 05:21:59PM +0800, Shawn Lin wrote:
> >
> > I build a kernel with this patch set (7.3~rc4-2) and got several warnings
> > like this, shortly followed by a stack trace:
>
> Sashiko already reportted some valid concern around this, and I'll plan
> to rework this series a bit later. Thanks for reporting this.
Just thinking out loud:
Patch 1/3 in this series fixes a regression, so it should be picked up as
soon as possible.
Patch 2/3 is converting the resources to be device managed.
However, after patch 1/3 (if the moved code is called _before_
dw_pcie_host_init()) the only thing left after dw_pcie_host_init() is the
PCIE_CLIENT_INTR_MASK_MISC write.
I.e. there is no error return remains that would need a dw_pcie_host_deinit().
So there is no strict need to convert to devm_().
You can do so, but that should be in a separate series IMO.
In fact, Diederik's later warnings are a result of Patch 2/3 which replaced
irq_domain_create_linear() with devm_irq_domain_instantiate():
error: hwirq 0x0 is too large for :pcie@fe150000:legacy-interrupt-controller
WARNING: kernel/irq/irqdomain.c:676 at irq_domain_associate_locked+0x118/0x1a0
The fix seems to be to add .hwirq_max = PCI_NUM_INTX, to
rockchip_pcie_init_irq_domain():
The fix is one line in rockchip_pcie_init_irq_domain() :
.size = PCI_NUM_INTX,
.hwirq_max = PCI_NUM_INTX,
Patch 3/3 looks like a theoretical problem that Sashiko found.
Is the underlying problem real? It's plausible, but nobody has reproduced it.
For it to happen, the controller's legacy IRQ output has to be asserted while
clocks are gated. Two ways that could happen:
- The output was high when clk_bulk_disable_unprepare() froze the logic.
- A device was still asserting INTx when an AER- or sysfs-triggered reset
stopped the link.
Patch 3/3 is not enough though:
disable_irq() never masks this chained IRQ in hardware, so
rockchip_pcie_intx_handler() can still run and read the unclocked APB.
This is the kind of case IRQ_DISABLE_UNLAZY exists for, according to the
comment above irq_disable().
Minimal amend of patch 3/3 as it currently looks:
turn off lazy disable for this line when the chained handler is installed:
irq_set_status_flags(rockchip->intx_irq, IRQ_DISABLE_UNLAZY);
irq_set_chained_handler_and_data(rockchip->intx_irq,
rockchip_pcie_intx_handler, rockchip);
Such that irq_disable() masks the chained IRQ.
Do we want patch 3/3? Probably.. but this race seems very small...
probably most imporent to get patch 1 (with the code move _before_
dw_pcie_host_init()) accepted ASAP, as it currently is causing problems
for Diederik.
Kind regards,
Niklas
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver
2026-09-23 15:25 ` Niklas Cassel
@ 2026-09-24 3:27 ` Shawn Lin
0 siblings, 0 replies; 10+ messages in thread
From: Shawn Lin @ 2026-09-24 3:27 UTC (permalink / raw)
To: Niklas Cassel
Cc: shawn.lin, Diederik de Haas, linux-rockchip, linux-pci,
Manivannan Sadhasivam, Bjorn Helgaas
Hi Niklas
在 2026/09/23 星期三 23:25, Niklas Cassel 写道:
> Hello Shawn, Diederik,
>
> On Wed, Sep 23, 2026 at 05:21:59PM +0800, Shawn Lin wrote:
>>>
>>> I build a kernel with this patch set (7.3~rc4-2) and got several warnings
>>> like this, shortly followed by a stack trace:
>>
>> Sashiko already reportted some valid concern around this, and I'll plan
>> to rework this series a bit later. Thanks for reporting this.
>
> Just thinking out loud:
>
> Patch 1/3 in this series fixes a regression, so it should be picked up as
> soon as possible.
>
Thank you for your detailed analysis and valuable suggestions.
My original intention was to provide a minimal fix for the regression
introduced during the merge window. However, after Sashiko's review,
some existing problems (even if only theoretical) were pointed out, so
the series grew into three patches.
I agree with your reasoning: patch 1 fixes a real regression and should
be picked up as soon as possible, while patches 2 and 3 can be handled
separately. Given that we are approaching -rc5 and I will have two
consecutive national holidays in the coming weeks, I may be unable to
access my development equipment for about two weeks. Therefore, I plan
to revise patch 1 based on your suggestion (moving the code before
dw_pcie_host_init()) and send it out promptly.
> Patch 2/3 is converting the resources to be device managed.
> However, after patch 1/3 (if the moved code is called _before_
> dw_pcie_host_init()) the only thing left after dw_pcie_host_init() is the
> PCIE_CLIENT_INTR_MASK_MISC write.
>
> I.e. there is no error return remains that would need a dw_pcie_host_deinit().
>
> So there is no strict need to convert to devm_().
> You can do so, but that should be in a separate series IMO.
>
>
> In fact, Diederik's later warnings are a result of Patch 2/3 which replaced
> irq_domain_create_linear() with devm_irq_domain_instantiate():
>
> error: hwirq 0x0 is too large for :pcie@fe150000:legacy-interrupt-controller
> WARNING: kernel/irq/irqdomain.c:676 at irq_domain_associate_locked+0x118/0x1a0
>
>
> The fix seems to be to add .hwirq_max = PCI_NUM_INTX, to
> rockchip_pcie_init_irq_domain():
>
> The fix is one line in rockchip_pcie_init_irq_domain() :
> .size = PCI_NUM_INTX,
> .hwirq_max = PCI_NUM_INTX,
>
>
>
> Patch 3/3 looks like a theoretical problem that Sashiko found.
>
> Is the underlying problem real? It's plausible, but nobody has reproduced it.
> For it to happen, the controller's legacy IRQ output has to be asserted while
> clocks are gated. Two ways that could happen:
>
> - The output was high when clk_bulk_disable_unprepare() froze the logic.
> - A device was still asserting INTx when an AER- or sysfs-triggered reset
> stopped the link.
>
> Patch 3/3 is not enough though:
>
> disable_irq() never masks this chained IRQ in hardware, so
> rockchip_pcie_intx_handler() can still run and read the unclocked APB.
>
> This is the kind of case IRQ_DISABLE_UNLAZY exists for, according to the
> comment above irq_disable().
>
> Minimal amend of patch 3/3 as it currently looks:
>
> turn off lazy disable for this line when the chained handler is installed:
> irq_set_status_flags(rockchip->intx_irq, IRQ_DISABLE_UNLAZY);
> irq_set_chained_handler_and_data(rockchip->intx_irq,
> rockchip_pcie_intx_handler, rockchip);
>
> Such that irq_disable() masks the chained IRQ.
>
> Do we want patch 3/3? Probably.. but this race seems very small...
> probably most imporent to get patch 1 (with the code move _before_
> dw_pcie_host_init()) accepted ASAP, as it currently is causing problems
> for Diederik.
>
>
> Kind regards,
> Niklas
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-24 3:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-22 2:36 [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Shawn Lin
2026-09-22 2:37 ` [PATCH v3 1/3] PCI: dw-rockchip: Move the INTx irq setup to probe Shawn Lin
2026-09-22 9:56 ` Niklas Cassel
2026-09-22 2:37 ` [PATCH v3 2/3] PCI: dw-rockchip: Make the INTx irq setup devm-managed Shawn Lin
2026-09-22 9:58 ` Niklas Cassel
2026-09-22 2:37 ` [PATCH v3 3/3] PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are gated Shawn Lin
2026-09-23 9:13 ` [PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver Diederik de Haas
2026-09-23 9:21 ` Shawn Lin
2026-09-23 15:25 ` Niklas Cassel
2026-09-24 3:27 ` Shawn Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox