(You really should re-send your patches, as they didn't make it to the list. But while you're here:) On Thu, May 30, 2019 at 3:01 AM Ganapathi Bhat <gbhat@xxxxxxxxxxx> wrote: > > From: Sharvari Harisangam <sharvari@xxxxxxxxxxx> > > > > PCIE WAKE# is asserted by firmware, when WoWLAN conditions are > > matched. Current driver does not enable PME bit needed for WAKE# > > assertion, causing host to remain in sleep even after WoWLAN conditions are > > matched. This commit fixes it by enabling wakeup (PME bit) in suspend > > handler. Are you sure said devices actually have their 'wakeup' attribute set to 'enabled' (e.g., in sysfs)? I think the PCI core should probably be taking care of this for you already. See below. > > Signed-off-by: Sharvari Harisangam <sharvari@xxxxxxxxxxx> > > Signed-off-by: Ganapathi Bhat <gbhat@xxxxxxxxxxx> > > --- > > drivers/net/wireless/marvell/mwifiex/pcie.c | 27 +++++++++++++++------------ > > 1 file changed, 15 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c > > b/drivers/net/wireless/marvell/mwifiex/pcie.c > > index 3fe81b2..0bd81d4 100644 > > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c > > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c ... > > @@ -181,6 +180,10 @@ static int mwifiex_pcie_suspend(struct device *dev) > > set_bit(MWIFIEX_IS_SUSPENDED, &adapter->work_flags); > > clear_bit(MWIFIEX_IS_HS_ENABLING, &adapter->work_flags); > > > > + pci_enable_wake(pdev, pci_choose_state(pdev, state), 1); > > + pci_save_state(pdev); > > + pci_set_power_state(pdev, pci_choose_state(pdev, state)); >From Documentation/power/pci.txt: """ 3.1.2. suspend() ... This callback is expected to quiesce the device and prepare it to be put into a low-power state by the PCI subsystem. It is not required (in fact it even is not recommended) that a PCI driver's suspend() callback save the standard configuration registers of the device, prepare it for waking up the system, or put it into a low-power state. All of these operations can very well be taken care of by the PCI subsystem, without the driver's participation. However, in some rare case it is convenient to carry out these operations in a PCI driver. [...] """ I think you need to do a little more work to justify why you are one of those rare cases. On a related note: some of the existing "configure wakeup" stuff we do here should probably be gated behind a call to device_may_wakeup(). That would help make it more obvious that such configuration is futile, if the device was marked as wake-disabled. Brian > > + > > return 0; > > } > >
