Re: [PATCH v6 3/3] PCI: qcom: Implement shutdown() callback

[Konrad Dybcio]

On 9/9/26 2:24 PM, Krishna Chaitanya Chundru wrote:
> From: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> 
> 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.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
> ---

[...]

> +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;
> +	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);
> +	} else {
> +		pcie = platform_get_drvdata(pdev);

This is no less than an intriguing choice.. please store the same
element in both cases so we don't have to do this

> +		pci = pcie->pci;
> +
> +		if (pcie->global_irq)
> +			disable_irq(pcie->global_irq);

This changed the ordering of disabling things versus the previous
revision, although I don't think it should matter given this
IRQ currently only services the linkdown event


Konrad