Re: [PATCH v8 4/4] PCI: qcom: Implement shutdown() callback

[Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>]

On 11/9/2026 7:59 PM, Konrad Dybcio wrote:
> On 11/7/26 2:45 AM, Krishna Chaitanya Chundru wrote:
>> From: Manivannan Sadhasivam <mani@kernel.org>
>>
>> PCIe host controllers should bring the link down cleanly before system
>> shutdown/reboot proceeds to remove power/clocks from the controller.
>> Without this, the link may still be up and endpoints still have
>> transactions in flight when power/clocks are cut, which can trip SMMU
>> translation faults or NoC protocol errors.
>>
>> Reuse dw_pcie_suspend_noirq() in the shutdown path to force the link
>> into L2, putting it into D3cold.
>>
>> device_shutdown() runs with interrupts enabled, unlike suspend_noirq().
>> Mask the chained MSI IRQ(s) and the Global IRQ before tearing down the
>> link and clocks/PHY, since a late/spurious interrupt could otherwise
>> reach a handler that touches now-unclocked PARF/DBI registers. Use
>> dw_pcie_mask_msi() rather than dw_pcie_free_msi() so the MSI IRQ domain
>> and the MSI mappings held by downstream devices (which device_shutdown()
>> does not remove) are left intact.
>>
>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>  drivers/pci/controller/dwc/pcie-qcom.c | 41 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 41 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index d39753f0e724..c5b06db3b92a 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -2046,6 +2046,9 @@ static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
>>  
>>  	dw_pcie_msi_init(pp);
>>  
>> +	/* Stash pci so qcom_pcie_shutdown() can mask the MSI IRQ(s) later */
>> +	platform_set_drvdata(to_platform_device(dev), pci);
>> +
>>  	return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
>>  }
>>  
>> @@ -2466,6 +2469,43 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>  	return ret;
>>  }
>>  
>> +static void qcom_pcie_shutdown(struct platform_device *pdev)
>> +{
>> +	const struct qcom_pcie_cfg *pcie_cfg = of_device_get_match_data(&pdev->dev);
>> +	struct qcom_pcie *pcie = NULL;
>> +	struct dw_pcie *pci;
>> +
>> +	if (pcie_cfg && pcie_cfg->firmware_managed) {
>> +		/*
>> +		 * Firmware owns the link teardown and clock/PHY shutdown in
>> +		 * this mode; Linux only owns the chained MSI IRQ(s), which
>> +		 * still need to be masked off before shutdown proceeds.
>> +		 */
>> +		pci = platform_get_drvdata(pdev);
> Revisiting my older comment, we can still stash `pci` in drvdata regardless
> of the flavor and do `struct qcom_pcie *pcie = to_qcom_pcie(pci);` in
> places where we need it
I am going with uniform drvdata but stashing struct qcom_pcie (not the bare
struct dw_pcie) in both paths — to_qcom_pcie() resolves through
dev_get_drvdata(), so stashing pci here would have returned pci again and
broken the existing dw_pcie_ops callbacks. The firmware/ECAM path now
allocates a thin qcom_pcie wrapper, letting shutdown() drop the branch and
recover everything (Global IRQ, cfg, dw_pcie) uniformly.

- Krishna Chaitanya.
> Konrad
>
>> +	} else {
>> +		pcie = platform_get_drvdata(pdev);
>> +		pci = pcie->pci;
>> +
>> +		if (pcie->global_irq)
>> +			disable_irq(pcie->global_irq);
>> +	}
>> +
>> +	if (pci->pp.use_imsi_rx)
>> +		dw_pcie_mask_msi(&pci->pp);
>> +
>> +	/*
>> +	 * In firmware-managed mode 'pcie' stays NULL and firmware owns the
>> +	 * link/clock teardown. Otherwise, skip if the controller is already
>> +	 * suspended (e.g. a prior resume failed), since dw_pcie_suspend_noirq()
>> +	 * would touch DBI registers with clocks/interconnects already off.
>> +	 */
>> +	if (pcie && !pcie->pci->suspended)
>> +		dw_pcie_suspend_noirq(pcie->pci, true);
>> +
>> +	pm_runtime_put_sync(&pdev->dev);
>> +	pm_runtime_disable(&pdev->dev);
>> +}
>> +
>>  static int qcom_pcie_suspend_noirq(struct device *dev)
>>  {
>>  	struct qcom_pcie *pcie;
>> @@ -2657,5 +2697,6 @@ static struct platform_driver qcom_pcie_driver = {
>>  		.pm = &qcom_pcie_pm_ops,
>>  		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>  	},
>> +	.shutdown = qcom_pcie_shutdown,
>>  };
>>  builtin_platform_driver(qcom_pcie_driver);
>>