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

[Manivannan Sadhasivam]

On Thu, Sep 10, 2026 at 08:56:38AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 9/9/2026 6:48 PM, Konrad Dybcio wrote:
> > On 9/9/26 2:40 PM, Krishna Chaitanya Chundru wrote:
> >>
> >> On 9/9/2026 6:06 PM, Konrad Dybcio wrote:
> >>> 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
> >> for firmware_managed solutions struct qcom_pcie *pcie is not being
> >> used/allocated at all. it will waste of memory if we allocate memory for just
> >> this use case. - Krishna Chaitanya.
> > Should we move it to a separate file/driver then, perhaps?
> > We could share the ECAM ops in e.g. pcie-qcom-common.c
> I will let Mani to comment on this.
> 

pcie-qcom-common.c is only for sharing common code between RC and EP controller
drivers. So it doesn't make sense to move ECAM ops which is just RC specific to
the common driver.

- Mani

-- 
மணிவண்ணன் சதாசிவம்