netdev - Re: [PATCH 2/2] net: wwan: iosm: Keep device at D0 for s2idle case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 30 Dec 2021 09:00:30 +0800
From:   Kai-Heng Feng <kai.heng.feng@...onical.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     m.chetan.kumar@...el.com, linuxwwan@...el.com,
        linux-pci@...r.kernel.org, linux-pm@...r.kernel.org,
        Loic Poulain <loic.poulain@...aro.org>,
        Sergey Ryazanov <ryazanov.s.a@...il.com>,
        Johannes Berg <johannes@...solutions.net>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Vaibhav Gupta <vaibhavgupta40@...il.com>
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@...nel.org> 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.
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
> > ---
> >  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.

Dave, can you drop it from netdev until IOSM devs confirm this patch is correct?

Kai-Heng

>
> >       return 0;
> >  }
> >
> > --
> > 2.33.1
> >