Re: [PATCH v2] PCI: pciehp: Fix hotplug on Catlow Lake with unreliable PME status

[Kuppuswamy Sathyanarayanan]

On 2/14/2026 7:11 AM, Lukas Wunner wrote:
> On Sat, Feb 14, 2026 at 07:01:13AM +0100, Lukas Wunner wrote:
>> On Fri, Feb 13, 2026 at 03:14:28PM -0800, Kuppuswamy Sathyanarayanan wrote:
>>> On Intel Catlow Lake platforms, PCH PCIe root ports do not reliably
>>> update PME status registers (PME Status and PME Requester_ID in the
>>> Root Status register) during D3hot to D0 transitions, even though PME
>>> interrupts are delivered correctly.
>>
>> Hm, so in theory we could amend the PME driver to walk the bus below the
>> Root Port and see if anything has PME_Status set in the PMCSR register.
>>
>> But the PME interrupt is shared with hotplug, bandwidth control etc,
>> so we'd end up gratuitouly (and frequently) runtime resuming switches
>> below the Root Port to see if there's anything below which is requesting
>> wakeup.
>>
>> So just keeping the Root Port runtime resumed all the time, as this
>> patch does, is still a better approach IMO.
>>
>> I'm wondering though if this causes a power regression.  Does keeping
>> the Root Port in D0 prevent the Package from entering a lower power
>> state?  Or is this irrelevant because the PCH is a different chip
>> or tile?
> 
> I've just realized that pcie_disable_interrupt() isn't called from
> pciehp_suspend() if pme_is_native() is true.  Should disabling
> runtime PM cause a power regression, an alternative solution may be
> to make pcie_disable_interrupt() conditional on a new pme_is_broken()
> which checks for affected Catlow Lake PCH Root Ports.
> 
> The pm_runtime_disable() approach is slightly preferred because
> it keeps pciehp code clean.

I think pcie_disable_interrupt() is called from pciehp_suspend() when
pme_is_native() is true. Looking at the code:

static void pciehp_disable_interrupt(struct pcie_device *dev)
{
        /*
         * Disable hotplug interrupt so that it does not trigger
         * immediately when the downstream link goes down.
         */
        if (pme_is_native(dev))
                pcie_disable_interrupt(get_service_data(dev));
}

#ifdef CONFIG_PM_SLEEP
static int pciehp_suspend(struct pcie_device *dev)
{
        /*
         * If the port is already runtime suspended we can keep it that
         * way.
         */
        if (dev_pm_skip_suspend(&dev->port->dev))
                return 0;

        pciehp_disable_interrupt(dev);
        return 0;
}

pciehp_suspend() calls pciehp_disable_interrupt(), which in turn calls
pcie_disable_interrupt() only if pme_is_native() is true. So the interrupt
is disabled during suspend specifically when PME is native, not the other
way around. Did I misread your statement?

> 
> Thanks,
> 
> Lukas

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer