Re: [PATCH v2] mmc: sdhci-pci-gli: GL975x: Mask rootport's replay timer timeout during suspend

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Jan 18, 2024 at 02:40:50PM +0800, Kai-Heng Feng wrote:
> On Sat, Jan 13, 2024 at 1:37 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Fri, Jan 12, 2024 at 01:14:42PM +0800, Kai-Heng Feng wrote:
> > > On Sat, Jan 6, 2024 at 5:19 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > On Thu, Dec 21, 2023 at 11:21:47AM +0800, Kai-Heng Feng wrote:
> > > > > Spamming `lspci -vv` can still observe the replay timer timeout error
> > > > > even after commit 015c9cbcf0ad ("mmc: sdhci-pci-gli: GL9750: Mask the
> > > > > replay timer timeout of AER"), albeit with a lower reproduce rate.
> > > >
> > > > I'm not sure what this is telling me.  By "spamming `lspci -vv`, do
> > > > you mean that if you run lspci continually, you still see Replay Timer
> > > > Timeout logged, e.g.,
> > > >
> > > >   CESta:        ... Timeout+
> > >
> > > Yes it's logged and the AER IRQ is raised.
> >
> > IIUC the AER IRQ is the important thing.
> >
> > Neither 015c9cbcf0ad nor this patch affects logging in
> > PCI_ERR_COR_STATUS, so the lspci output won't change and mentioning it
> > here doesn't add useful information.
> 
> You are right. That's just a way to access config space to reproduce
> the issue.

Oh, I think I completely misunderstood you!  I thought you were saying
that suspending the device caused the PCI_ERR_COR_REP_TIMER error, and
you happened to see that it was logged when you ran lspci.

But I guess you mean that running lspci actually *causes* the error?
I.e., lspci does a config access while we're suspending the device
causes the error, and the config access itself causes the error, which
causes the ERR_COR message and ultimately the AER interrupt, and that
interrupt prevents the system suspend.

If that's the case, I wonder if this is a generic problem that could
happen with *any* device, not just GL975x.

What power state do we put the GL975x in during system suspend?
D3hot?  D3cold?  Is there anything that prevents config access while
we suspend it?

We do have dev->block_cfg_access, and there's a comment that says
"we're required to prevent config accesses during D-state
transitions," but I don't see it being used during D-state
transitions.

Also, it doesn't seem suitable for preventing config accesses during
suspend because pci_wait_cfg() just busy-waits and never returns an
error.

Bjorn