Re: [PATCH v2 2/3] PCI/PM: Do not save the config space of an inaccessible device

["Francisco Beltrán Millalén" <fbeltranmillalen@gmail.com>]

Hi Bjorn,

Thanks for the review, and for suggesting a simpler way to do this.

On Thu, Oct 08, 2026 at 05:58:25PM -0500, Bjorn Helgaas wrote:
> Apparently this is a reproducible issue on MacBookPro14,3.  That makes
> me a little hesitant because we're not actually dealing with the fact
> that the Thunderbolt controller isn't responsive during suspend.
>
> It seems worthwhile to me to skip pci_save_state() if the device isn't
> accessible, but I don't think it's a real solution to whatever is
> going on with Thunderbolt, and I don't think we should mention it here
> as though it is.

You're right that this patch doesn't fix the Thunderbolt problem
itself; that is what the Alpine Ridge quirk is for.  This patch only
makes sure that, when a device stops responding, the PCI core doesn't
save garbage and write it back later.  I'll rewrite the commit message
in v3 to say just that, and mention the MacBook only as the machine
where I found it.

> Maybe we should remove the check in pci_dev_save_and_disable() and
> make it check the return value of pci_save_state()?  I don't think
> checking twice adds anything.

Yes, I'll change it that way in v3.  While looking at it I found one
small corner case worth mentioning: pci_save_state() can also fail when
the kernel couldn't allocate memory for part of the saved state, back
when the device was first found.  The device itself works fine then,
but with this change the reset would no longer disable it first.  It is
very unlikely to happen, so I don't think it matters much, but if you
prefer, I can make the reset stop only when the device isn't
responding.

> I bet we get 99% of the usefulness here by just adding the first
> accessibility check above.
>
> This second check only helps if the device becomes inaccessible during
> the tiny window while we're saving its state, and I'm not sure that
> the extra complexity here and being able to restore a valid config
> header with junk in the capabilities is really a benefit.

I'll drop it in v3, so pci_save_state() goes back to what it was, plus
the one check at the start.

I also wanted to see how your version behaves before sending it, so I
built it and tested it on the MacBook today.  It went through four
suspend/resume cycles, three of them with a USB disk attached, with no
warnings, and it skipped the controller that wasn't responding, just
like v2 did.  I also reset a USB controller by hand to try the
pci_dev_save_and_disable() change, and that worked as before too.

Thanks again,
Francisco