Re: [PATCH] PCI: mediatek-gen3: Do full device power down on removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Asserting PERST# and disabling clocks before deregistering the interrupt handler introduces a race condition that can cause a bus fault (kernel panic).
--

commit 392cdae7adde07f3a5a5cd47d941a62a1d778a2b
Author: Chen-Yu Tsai <wenst@chromium.org>

PCI: mediatek-gen3: Do full device power down on removal

This commit updates the driver removal path to follow a proper power down
sequence for downstream devices. It calls mtk_pcie_devices_power_down()
on driver removal so that PERST# is asserted.

> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index b0accd8285892..e6acca7e79a0f 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -1260,7 +1260,7 @@ static void mtk_pcie_remove(struct platform_device *pdev)
>  	pci_remove_root_bus(host->bus);
>  	pci_unlock_rescan_remove();
>  
> -	pci_pwrctrl_power_off_devices(pcie->dev);
> +	mtk_pcie_devices_power_down(pcie);
>  	mtk_pcie_power_down(pcie);

Could this sequence cause a regression by introducing a race condition
with the interrupt handler?

Calling mtk_pcie_devices_power_down() asserts the PERST# pin, which forces
the downstream PCIe link down. This can trigger a link-down or MAC error
interrupt at the controller level.

Immediately following this, mtk_pcie_power_down() disables the transaction
layer clocks via clk_bulk_disable_unprepare().

If an interrupt fires during this window, the top-level chained interrupt
handler will execute because mtk_pcie_irq_teardown() hasn't been called yet.

If the handler attempts to read controller registers while its clocks are
disabled:

mtk_pcie_irq_handler()
	status = readl_relaxed(pcie->base + PCIE_INT_STATUS_REG);

Could this cause a synchronous external abort and crash the kernel?

>  	pci_pwrctrl_destroy_devices(pcie->dev);
>  	mtk_pcie_irq_teardown(pcie);

Should the interrupt handler be deregistered or masked before the clocks
are disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505105918.1823170-1-wenst@chromium.org?part=1