Re: [PATCH v8 4/6] PCI: tegra: Add Tegra264 support

[Manivannan Sadhasivam]

On Tue, Jul 28, 2026 at 09:25:17AM +0530, Manikanta Maddireddy wrote:
> 
> 
> On 27/07/26 9:14 pm, Thierry Reding wrote:
> > > > +static int tegra264_pcie_suspend(struct device *dev)
> > > > +{
> > > > +	/*
> > > > +	 * No need to do anything device-specific here, BPMP handles the
> > > > +	 * details of taking the link into L2.
> > > > +	 */
> > > > +
> > > > +	return pinctrl_pm_select_sleep_state(dev);
> > > > +}
> > > > +
> > > > +static int tegra264_pcie_resume(struct device *dev)
> > > > +{
> > > > +	struct tegra264_pcie *pcie = dev_get_drvdata(dev);
> > > > +	int err;
> > > > +
> > > > +	err = pinctrl_pm_select_default_state(dev);
> > > > +	if (err < 0) {
> > > > +		dev_err(dev, "failed to configure sideband pins: %pe\n",
> > > > +			ERR_PTR(err));
> > > > +		return err;
> > > > +	}
> > > > +
> > > > +	tegra264_pcie_init(pcie);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static DEFINE_SIMPLE_DEV_PM_OPS(tegra264_pcie_pm_ops,
> > > > +				tegra264_pcie_suspend,
> > > > +				tegra264_pcie_resume);
> > > PCI subsystem registered pm_noirq callbacks. So, controller should also
> > > register to pm_noirq callbacks.
> > Unfortunately that causes illegal uses of msleep() in no-IRQ context.
> > Sashiko complains about it and is right. I don't observe any runtime
> > issues with switching to the _noirq variants, but it doesn't seem right
> > to stick to it.
> > 
> > Is there any reason why this needs to be _noirq, or any benefits if it
> > was?
> > 
> > Thierry
> 
> The PCI subsystem registers pci_pm_resume_noirq() as a noirq resume
> callback. This function accesses PCI configuration space to transition the
> device from D3hot to D0. Therefore, the controller driver must first bring
> the PCIe controller out of reset and make its configuration space accessible
> to the PCI subsystem.
> 
> Many other controller drivers in drivers/pci/controller/ are registering
> noirq callback.
> 

Since the PCI client drivers use noirq callbacks, they would expect the device
to be accessible by the time the callbacks get executed. But if the controller
driver disables the resources or puts the devices into D3Cold in regular suspend
callback, then it will cause the client driver to fail or even crash.

This is why it is recommended to use noirq callbacks in controller drivers.

- Mani

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