On 8/29/24 3:45 AM, Ajay.Kathat@xxxxxxxxxxxxx wrote:
Hi,
Hi,
This change breaks suspend/resume on my wilc1000 setup (sama5d2 wlsom evk +
wilc1000 sd):
# echo mem > /sys/power/state
PM: suspend entry (deep)
Filesystems sync: 0.055 seconds
Freezing user space processes
Freezing user space processes completed (elapsed 0.018 seconds)
OOM killer disabled.
Freezing remaining freezable tasks
Freezing remaining freezable tasks completed (elapsed 0.006 seconds)
wilc1000_sdio mmc0:0001:1: sdio suspend
wilc1000_sdio mmc0:0001:1: PM: dpm_run_callback(): pm_generic_suspend returns -22
wilc1000_sdio mmc0:0001:1: PM: failed to suspend async: error -22
PM: Some devices failed to suspend, or early wake event detected
OOM killer enabled.
Restarting tasks ... done.
random: crng reseeded on system resumption
PM: suspend exit
sh: write error: Invalid argument
But I have to dig more to really understand the root cause.
Does your MMC controller struct mmc_host set .pm_caps |= MMC_PM_KEEP_POWER ?
Maybe that's the problem, that the controller does not set these PM caps ?
It looks like you are right, my sdmmc controller was missing some
keep-power-in-suspend flag in my device tree, preventing your change to work on
my platform. So it behaves correctly for me with both wilc1000/wilc3000, thanks.
Looks ok for me, but others may have a more informed opinion than me about the
impact of this change.
When the host suspend is triggered, the WILC power consumption should be
reduced since it is controlled via chip_allow_sleep() sequence which is
irrespective of MMC_PM_KEEP_POWER flag state of the host. So I'm not sure why
there was no difference observed in Marek's setup.
I think you misunderstood the patch description, there is no measurable
power consumption difference with/without this patch. The chip does
either get powered off (without this patch) or enter some sort of low
power state (with this patch), which I can see on the drop in power
consumption during suspend/resume.
This patch assures the chip does not get power-cycled during
suspend/resume cycle, which makes it lose state (and firmware), so the
chip is unusable after resume without this patch.
It may be that when the
power consumption was measured,the WILC suspend state is not enabled because
of MMC controller pm_caps in the test setup.
I think it is better to have a generic patch for any host which has
MMC_PM_KEEP_POWER capabilities defined or not. With proposed patch, driver
will not allow the host to go into the suspend state when MMC_PM_KEEP_POWER is
not set in PM caps. I think, sdio_set_host_pm_flags() should only be called if
MMC_PM_KEEP_POWER is defined in the host.
To retain firmware in the chip, the chip must not be powered off during
suspend/resume, which is what this patch assures. Without this patch,
the controller may power off the slot during suspend/resume and the WILC
will lose firmware and be unusable after resume.
Actually, WILC can support suspend mode with or without this host
capabilities. For SDIO, the host can be wake-up using the external IRQ GPIO
(uses out-of-band, instead of in-band interrupt) on WILC.
To test wake-on-wlan(wowlan) in suspend mode, the IRQ pin from WILC should be
connected with the host that will help to interrupt/wake the host when any
WiFi packet arrives. Without 'wowlan' enabled in suspend mode, the host should
be allowed to go into suspend mode but it can't be wake-up by WiFi packets.
All the packets will be dropped in the firmware in suspend state.
WILC supports only ANY option for wowlan. So, after connecting the IRQ line
with host, the below commands can be used to test "wowlan" in suspend state.
# iw phy <phyname> wowlan enable any
# echo mem > /sys/power/state
If the WILC is powered off because the slot is powered off, WILC cannot
resume anything.