Received-SPF: pass (google.com: domain of linux-kernel+bounces-109881-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <CAAd53p6qE76QjJmjr5ei0mU8xcSNE32hJMOE9Frwz-BuC3gDkA@mail.gmail.com>
 <20240119224053.GA187501@bhelgaas>
In-Reply-To: <20240119224053.GA187501@bhelgaas>
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
Date: Thu, 21 Mar 2024 18:05:33 +0800
Message-ID: <CAAd53p52fi_wr3Js9Rqct+i1D3rjrnVZ6tBN=uHqThM7UvzXQA@mail.gmail.com>
Subject: Re: [PATCH v2] mmc: sdhci-pci-gli: GL975x: Mask rootport's replay
 timer timeout during suspend
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: adrian.hunter@intel.com, ulf.hansson@linaro.org, 
	Victor Shih <victor.shih@genesyslogic.com.tw>, Ben Chuang <benchuanggli@gmail.com>, 
	linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Bjorn,

Sorry for the belated response.

On Sat, Jan 20, 2024 at 6:41=E2=80=AFAM Bjorn Helgaas <helgaas@kernel.org> =
wrote:
>
> On Thu, Jan 18, 2024 at 02:40:50PM +0800, Kai-Heng Feng wrote:
> > On Sat, Jan 13, 2024 at 1:37=E2=80=AFAM Bjorn Helgaas <helgaas@kernel.o=
rg> wrote:
> > > On Fri, Jan 12, 2024 at 01:14:42PM +0800, Kai-Heng Feng wrote:
> > > > On Sat, Jan 6, 2024 at 5:19=E2=80=AFAM Bjorn Helgaas <helgaas@kerne=
l.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: Ma=
sk the
> > > > > > replay timer timeout of AER"), albeit with a lower reproduce ra=
te.
> > > > >
> > > > > 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 i=
t
> > > 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.

Both running lspci and suspending the device can observe the error,
because both are accessing the config space.

>
> 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.

My point was that any kind of PCI config access can cause the error.
Using lspci is just make the error more easier to reproduce.

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

For now, it's 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?

The target device state is D3hot.
However, the issue happens when the devices is in D0, when the PCI
core is saving the device's config space.

So I think the issue isn't related to the device state.

>
> 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.

Yes, there isn't any D-state change happens here.

Kai-Heng

>
> 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