[PATCH v3 0/3] Small INTx fixes for Rockchip's dwc based PCIe controller driver

[Shawn Lin]

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 at 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
>