On 8/31/24 14:18, Marek Vasut wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On 8/31/24 7:22 AM, Ajay.Kathat@xxxxxxxxxxxxx wrote: >> Hi Marek, > > Hi, > >>>> 2. With this patch, does the ping to the station work during the suspend >>>> state? >>> >>> I haven't tested this, but that's unlikely because the host is >>> suspended. Still, that's not really the point here, the point is that >>> the whole WILC gets powered off during suspend/resume without this >>> patch. At least on STM32MP15xx it is, maybe on the Atmel controller it >>> is not, but we cannot depend on that. >> >> Indeed, you are right. It seems, the test results have dependency on the host >> controller power management behavior. In my setup, the card keep the power, >> when the host is suspended. > > Right, currently the driver depends on the controller behavior and it > works by accident with the atmel controller. This patch fixes that and > assures this will work on other controllers too. > >>>> AFAIR, during host suspend, the firmware continues to run without passing >>>> frames to the host unless 'wowlan' is enabled. >>>> >>>> There is another scenario. Let's assume a host that wants to go to suspend >>>> (power save mode) without caring about the WiFi status, i.e., it is okay to >>>> reconnect with the AP if required (anyway, the AP may disconnect the station >>>> based on inactivity timeout) or have to re-trigger the DHCP request again. >>>> But >>>> with this change, the driver would block the host from entering suspend mode. >>>> >>>> How about adding an 'if' check for host pm_caps before calling >>>> sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER)? In that case, it will only >>>> request when configured by the host platform. >>> >>> Since this driver does not reload the firmware into the card on resume, >>> the card has to be kept powered on during suspend/resume cycle. The card >>> can NOT be powered off during suspend/resume cycle, otherwise the >>> firmware is lost. >>> >>> Without this flag, the card may be powered off during suspend/resume >>> cycle. It possibly does not happen on the Atmel controller, but it does >>> on the STM32MP15xx ARM MMCI one. >>> >> yes, in the Atmel controller, it is not necessary to re-program the firmware >> on resume. > > It is if the slot gets powered off across suspend/resume . It could be > that this does not happen on the hardware you use for testing, but the > driver cannot depend on that. > > [...] > >>>> I think, the wilc firmware should resume but the connection with AP may get >>>> closed. Additional commands to scan and reconnect with AP may be required >>>> that >>>> should work without downloading the firmware to wilc chip again. >>> >>> Currently, the slot may get powered off and then there is no firmware. >> >> Currently, driver doesn't support re-programming of the chip when the host >> resumes. > > Therefore, the slot MUST stay powered on across suspend/resume. >> Having that support would allow both types of hosts to suspend/resume. >> Added changes to download the firmware asynchronously on resume and it is >> included in the attached patch. It is based on the latest 'for-next' branch. I >> was not able to test it for the suspend scenario without powering the card. If >> possible, could you please try this patch in your setup. >> >> Incase, there is no improvement with the attached patch then IMO, your patch >> looks safe to keep the MMC_PM_KEEP_POWER state since host resume may not work >> without the card powered-on anyway. > > I think the attached patch is a new feature, this patch is a fix for > existing bug. Please submit the attached patch properly, so it can get > proper review on the ML. I believe your patch addresses the host suspend/resume scenario until the driver includes support to re-download the firmware. Before submitting the new patch to ML for re-download on resume, I must test it with the exact setup to confirm if it really resolves the issue. I tested the patch by forcing the re-download in resume, but it was not an exact scenario because the wilc chip was not entirely powered down during host suspend in my setup. I will check if a similar setup can be arranged to test this scenario before submitting it to ML. Regards, Ajay
