On Mon, Jan 3, 2022 at 11:28 PM Kumar, M Chetan <m.chetan.kumar@xxxxxxxxx> wrote: > > > -----Original Message----- > > From: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > > Sent: Thursday, December 30, 2021 6:31 AM > > To: Bjorn Helgaas <helgaas@xxxxxxxxxx> > > Cc: Kumar, M Chetan <m.chetan.kumar@xxxxxxxxx>; linuxwwan > > <linuxwwan@xxxxxxxxx>; linux-pci@xxxxxxxxxxxxxxx; linux- > > pm@xxxxxxxxxxxxxxx; Loic Poulain <loic.poulain@xxxxxxxxxx>; Sergey > > Ryazanov <ryazanov.s.a@xxxxxxxxx>; Johannes Berg > > <johannes@xxxxxxxxxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; > > Jakub Kicinski <kuba@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux- > > kernel@xxxxxxxxxxxxxxx; Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>; Vaibhav > > Gupta <vaibhavgupta40@xxxxxxxxx> > > Subject: Re: [PATCH 2/2] net: wwan: iosm: Keep device at D0 for s2idle case > > > > On Thu, Dec 30, 2021 at 4:18 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > > > > > [+cc Rafael, Vaibhav] > > > > > > On Fri, Dec 24, 2021 at 04:19:14PM +0800, Kai-Heng Feng wrote: > > > > We are seeing spurious wakeup caused by Intel 7560 WWAN on AMD > > laptops. > > > > This prevent those laptops to stay in s2idle state. > > > > > > > > From what I can understand, the intention of ipc_pcie_suspend() is > > > > to put the device to D3cold, and ipc_pcie_suspend_s2idle() is to > > > > keep the device at D0. However, the device can still be put to > > > > D3hot/D3cold by PCI core. > > > > > > > > So explicitly let PCI core know this device should stay at D0, to > > > > solve the spurious wakeup. > > Did you get a chance to check the cause of spurious wakeup ? Was there any > information device is trying to send while platform is entering suspend/ > host sw missed to unsubscribe certain notifications which resulted in wake event. Can you please let me know how to check it? > > In our internal test (x86 platform) we had not noticed such spurious wakeup but would > like to cross check by running few more tests. Sure, let me know what tests you want me to run. > > > > > > > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > > > > --- > > > > drivers/net/wwan/iosm/iosm_ipc_pcie.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/net/wwan/iosm/iosm_ipc_pcie.c > > > > b/drivers/net/wwan/iosm/iosm_ipc_pcie.c > > > > index d73894e2a84ed..af1d0e837fe99 100644 > > > > --- a/drivers/net/wwan/iosm/iosm_ipc_pcie.c > > > > +++ b/drivers/net/wwan/iosm/iosm_ipc_pcie.c > > > > @@ -340,6 +340,9 @@ static int __maybe_unused > > > > ipc_pcie_suspend_s2idle(struct iosm_pcie *ipc_pcie) > > > > > > > > ipc_imem_pm_s2idle_sleep(ipc_pcie->imem, true); > > > > > > > > + /* Let PCI core know this device should stay at D0 */ > > > > + pci_save_state(ipc_pcie->pci); > > > > > > This is a weird and non-obvious way to say "this device should stay at > > > D0". It's also fairly expensive since pci_save_state() does a lot of > > > slow PCI config reads. > > > > Yes, so I was waiting for feedback from IOSM devs what's the expected PCI > > state for the s2idle case. > > D3 is the expected state. Is it D3hot or D3cold? Kai-Heng > > > Dave, can you drop it from netdev until IOSM devs confirm this patch is > > correct? > > Dave, please drop this patch from netdev.
