diff mbox series

[v2] wifi: wilc1000: Keep slot powered on during suspend/resume

Message ID 20240926195113.2823392-1-marex@denx.de (mailing list archive)
State Accepted
Commit 98ca3178ad797a5ae9df50a69be49292d1734485
Delegated to: Kalle Valo
Headers show
Series [v2] wifi: wilc1000: Keep slot powered on during suspend/resume | expand

Commit Message

Marek Vasut Sept. 26, 2024, 7:50 p.m. UTC
The WILC3000 can suspend and enter low power state. According to local
measurements, the WILC3000 consumes the same amount of power if the slot
is powered up and WILC3000 is suspended, and if the WILC3000 is powered
off. Use the former option, keep the WILC3000 powered up as that allows
for things like WoWlan to work.

Note that this is tested on WILC3000 only, not on WILC1000 .

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Ajay Singh <ajay.kathat@microchip.com>
Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: Marek Vasut <marex@denx.de>
Cc: linux-wireless@vger.kernel.org
---
V2: Rebase on next-20240926
---
 drivers/net/wireless/microchip/wilc1000/sdio.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

Comments

Kalle Valo Sept. 28, 2024, 11:18 a.m. UTC | #1
Marek Vasut <marex@denx.de> writes:

> The WILC3000 can suspend and enter low power state. According to local
> measurements, the WILC3000 consumes the same amount of power if the slot
> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
> off. Use the former option, keep the WILC3000 powered up as that allows
> for things like WoWlan to work.
>
> Note that this is tested on WILC3000 only, not on WILC1000 .
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Ajay Singh <ajay.kathat@microchip.com>
> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Cc: Kalle Valo <kvalo@kernel.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: linux-wireless@vger.kernel.org
> ---
> V2: Rebase on next-20240926

BTW I recommend using wireless-next as the baseline for wireless
patches. For example, wireless-next is not pulled to linux-next during
merge windows or other patches in linux-next might create unnecessary
conflicts. Of course most of the cases using linux-next is fine.
Marek Vasut Sept. 29, 2024, 3:23 p.m. UTC | #2
On 9/28/24 1:18 PM, Kalle Valo wrote:
> Marek Vasut <marex@denx.de> writes:
> 
>> The WILC3000 can suspend and enter low power state. According to local
>> measurements, the WILC3000 consumes the same amount of power if the slot
>> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
>> off. Use the former option, keep the WILC3000 powered up as that allows
>> for things like WoWlan to work.
>>
>> Note that this is tested on WILC3000 only, not on WILC1000 .
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> Cc: Ajay Singh <ajay.kathat@microchip.com>
>> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
>> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>> Cc: Kalle Valo <kvalo@kernel.org>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: linux-wireless@vger.kernel.org
>> ---
>> V2: Rebase on next-20240926
> 
> BTW I recommend using wireless-next as the baseline for wireless
> patches. For example, wireless-next is not pulled to linux-next during
> merge windows or other patches in linux-next might create unnecessary
> conflicts. Of course most of the cases using linux-next is fine.
I didn't know there was one such tree, added to remotes, thanks !
Kalle Valo Sept. 30, 2024, 4:51 a.m. UTC | #3
Marek Vasut <marex@denx.de> writes:

> On 9/28/24 1:18 PM, Kalle Valo wrote:
>
>> Marek Vasut <marex@denx.de> writes:
>> 
>>> The WILC3000 can suspend and enter low power state. According to local
>>> measurements, the WILC3000 consumes the same amount of power if the slot
>>> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
>>> off. Use the former option, keep the WILC3000 powered up as that allows
>>> for things like WoWlan to work.
>>>
>>> Note that this is tested on WILC3000 only, not on WILC1000 .
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> ---
>>> Cc: Ajay Singh <ajay.kathat@microchip.com>
>>> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
>>> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>> Cc: Kalle Valo <kvalo@kernel.org>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: linux-wireless@vger.kernel.org
>>> ---
>>> V2: Rebase on next-20240926
>>
>> BTW I recommend using wireless-next as the baseline for wireless
>> patches. For example, wireless-next is not pulled to linux-next during
>> merge windows or other patches in linux-next might create unnecessary
>> conflicts. Of course most of the cases using linux-next is fine.
>
> I didn't know there was one such tree, added to remotes, thanks !

Another tip for the Monday :) get_maintainer script is a handy way to
find what tree should be used:

$ scripts/get_maintainer.pl -f --scm drivers/net/wireless/microchip/wilc1000/wlan.c
Ajay Singh <ajay.kathat@microchip.com> (supporter:MICROCHIP WILC1000 WIFI DRIVER)
Claudiu Beznea <claudiu.beznea@tuxon.dev> (supporter:MICROCHIP WILC1000 WIFI DRIVER)
Kalle Valo <kvalo@kernel.org> (maintainer:NETWORKING DRIVERS (WIRELESS))
linux-wireless@vger.kernel.org (open list:MICROCHIP WILC1000 WIFI DRIVER)
linux-kernel@vger.kernel.org (open list)
git git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git
git git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git
git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Alexis Lothoré (eBPF Foundation) Oct. 3, 2024, 8:31 a.m. UTC | #4
On 9/29/24 17:23, Marek Vasut wrote:
> On 9/28/24 1:18 PM, Kalle Valo wrote:
>> Marek Vasut <marex@denx.de> writes:
>>
>>> The WILC3000 can suspend and enter low power state. According to local
>>> measurements, the WILC3000 consumes the same amount of power if the slot
>>> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
>>> off. Use the former option, keep the WILC3000 powered up as that allows
>>> for things like WoWlan to work.
>>>
>>> Note that this is tested on WILC3000 only, not on WILC1000 .
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> ---
>>> Cc: Ajay Singh <ajay.kathat@microchip.com>
>>> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
>>> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>> Cc: Kalle Valo <kvalo@kernel.org>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: linux-wireless@vger.kernel.org
>>> ---
>>> V2: Rebase on next-20240926
>>
>> BTW I recommend using wireless-next as the baseline for wireless
>> patches. For example, wireless-next is not pulled to linux-next during
>> merge windows or other patches in linux-next might create unnecessary
>> conflicts. Of course most of the cases using linux-next is fine.
> I didn't know there was one such tree, added to remotes, thanks !

+1, as already mentioned in previous revisions, I would gladly test wilc3000
changes on both wilc3000 and wilc1000 on my platform, and having the series on
top of wireless-next would allow to do it on top of any change also affecting
the driver in wireless-next :)
Marek Vasut Oct. 3, 2024, 10:49 a.m. UTC | #5
On 10/3/24 10:31 AM, Alexis Lothoré wrote:
> On 9/29/24 17:23, Marek Vasut wrote:
>> On 9/28/24 1:18 PM, Kalle Valo wrote:
>>> Marek Vasut <marex@denx.de> writes:
>>>
>>>> The WILC3000 can suspend and enter low power state. According to local
>>>> measurements, the WILC3000 consumes the same amount of power if the slot
>>>> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
>>>> off. Use the former option, keep the WILC3000 powered up as that allows
>>>> for things like WoWlan to work.
>>>>
>>>> Note that this is tested on WILC3000 only, not on WILC1000 .
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> Cc: Ajay Singh <ajay.kathat@microchip.com>
>>>> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
>>>> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>>> Cc: Kalle Valo <kvalo@kernel.org>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>> Cc: linux-wireless@vger.kernel.org
>>>> ---
>>>> V2: Rebase on next-20240926
>>>
>>> BTW I recommend using wireless-next as the baseline for wireless
>>> patches. For example, wireless-next is not pulled to linux-next during
>>> merge windows or other patches in linux-next might create unnecessary
>>> conflicts. Of course most of the cases using linux-next is fine.
>> I didn't know there was one such tree, added to remotes, thanks !
> 
> +1, as already mentioned in previous revisions, I would gladly test wilc3000
> changes on both wilc3000 and wilc1000 on my platform, and having the series on
> top of wireless-next would allow to do it on top of any change also affecting
> the driver in wireless-next :)

I just had a look at a diff between wireless-next/main and next/master 
20241003 for drivers/net/wireless/microchip , there are no changes to 
the driver between the two trees, so it should be possible to test this 
patch on either tree. Can you give it a try ? Ideally test this patch 
separately on WILC1000 across suspend/resume and check if it works. You 
might need the MMC controller fix which sets struct mmc_host .pm_caps |= 
MMC_PM_KEEP_POWER for your controller , unless this is already upstream.

The WILC3000 series depends on this patch.
Alexis Lothoré (eBPF Foundation) Oct. 3, 2024, 12:58 p.m. UTC | #6
On 10/3/24 12:49, Marek Vasut wrote:
> On 10/3/24 10:31 AM, Alexis Lothoré wrote:
>> On 9/29/24 17:23, Marek Vasut wrote:
>>> On 9/28/24 1:18 PM, Kalle Valo wrote:
>>>> Marek Vasut <marex@denx.de> writes:
>>>>
>>>>> The WILC3000 can suspend and enter low power state. According to local
>>>>> measurements, the WILC3000 consumes the same amount of power if the slot
>>>>> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
>>>>> off. Use the former option, keep the WILC3000 powered up as that allows
>>>>> for things like WoWlan to work.
>>>>>
>>>>> Note that this is tested on WILC3000 only, not on WILC1000 .
>>>>>
>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>> ---
>>>>> Cc: Ajay Singh <ajay.kathat@microchip.com>
>>>>> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
>>>>> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>>>> Cc: Kalle Valo <kvalo@kernel.org>
>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>> Cc: linux-wireless@vger.kernel.org
>>>>> ---
>>>>> V2: Rebase on next-20240926
>>>>
>>>> BTW I recommend using wireless-next as the baseline for wireless
>>>> patches. For example, wireless-next is not pulled to linux-next during
>>>> merge windows or other patches in linux-next might create unnecessary
>>>> conflicts. Of course most of the cases using linux-next is fine.
>>> I didn't know there was one such tree, added to remotes, thanks !
>>
>> +1, as already mentioned in previous revisions, I would gladly test wilc3000
>> changes on both wilc3000 and wilc1000 on my platform, and having the series on
>> top of wireless-next would allow to do it on top of any change also affecting
>> the driver in wireless-next :)
> 
> I just had a look at a diff between wireless-next/main and next/master 20241003
> for drivers/net/wireless/microchip , there are no changes to the driver between
> the two trees, so it should be possible to test this patch on either tree. Can
> you give it a try ? Ideally test this patch separately on WILC1000 across
> suspend/resume and check if it works. You might need the MMC controller fix
> which sets struct mmc_host .pm_caps |= MMC_PM_KEEP_POWER for your controller ,
> unless this is already upstream.
> 
> The WILC3000 series depends on this patch.
> 
Meh, you are right, I have read too fast your answer to my initial question
about used base branch, and omitted the suspend/resume patch (I assumed it
conflicted because of some other patches in wireless-next).

Thanks,

Alexis
Marek Vasut Oct. 3, 2024, 1:18 p.m. UTC | #7
On 10/3/24 2:58 PM, Alexis Lothoré wrote:
> On 10/3/24 12:49, Marek Vasut wrote:
>> On 10/3/24 10:31 AM, Alexis Lothoré wrote:
>>> On 9/29/24 17:23, Marek Vasut wrote:
>>>> On 9/28/24 1:18 PM, Kalle Valo wrote:
>>>>> Marek Vasut <marex@denx.de> writes:
>>>>>
>>>>>> The WILC3000 can suspend and enter low power state. According to local
>>>>>> measurements, the WILC3000 consumes the same amount of power if the slot
>>>>>> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
>>>>>> off. Use the former option, keep the WILC3000 powered up as that allows
>>>>>> for things like WoWlan to work.
>>>>>>
>>>>>> Note that this is tested on WILC3000 only, not on WILC1000 .
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>> ---
>>>>>> Cc: Ajay Singh <ajay.kathat@microchip.com>
>>>>>> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
>>>>>> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>>>>> Cc: Kalle Valo <kvalo@kernel.org>
>>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>>> Cc: linux-wireless@vger.kernel.org
>>>>>> ---
>>>>>> V2: Rebase on next-20240926
>>>>>
>>>>> BTW I recommend using wireless-next as the baseline for wireless
>>>>> patches. For example, wireless-next is not pulled to linux-next during
>>>>> merge windows or other patches in linux-next might create unnecessary
>>>>> conflicts. Of course most of the cases using linux-next is fine.
>>>> I didn't know there was one such tree, added to remotes, thanks !
>>>
>>> +1, as already mentioned in previous revisions, I would gladly test wilc3000
>>> changes on both wilc3000 and wilc1000 on my platform, and having the series on
>>> top of wireless-next would allow to do it on top of any change also affecting
>>> the driver in wireless-next :)
>>
>> I just had a look at a diff between wireless-next/main and next/master 20241003
>> for drivers/net/wireless/microchip , there are no changes to the driver between
>> the two trees, so it should be possible to test this patch on either tree. Can
>> you give it a try ? Ideally test this patch separately on WILC1000 across
>> suspend/resume and check if it works. You might need the MMC controller fix
>> which sets struct mmc_host .pm_caps |= MMC_PM_KEEP_POWER for your controller ,
>> unless this is already upstream.
>>
>> The WILC3000 series depends on this patch.
>>
> Meh, you are right, I have read too fast your answer to my initial question
> about used base branch, and omitted the suspend/resume patch (I assumed it
> conflicted because of some other patches in wireless-next).
Nope, it conflicted because I (again) didn't include cover letter with 
the WILC3000 series, which mentions this information. I really need to 
find some suitable tooling to manage the cover letters, branch 
description does not seem to cut it quite as I hoped it would.
Alexis Lothoré (eBPF Foundation) Oct. 3, 2024, 1:59 p.m. UTC | #8
On 10/3/24 15:18, Marek Vasut wrote:
> On 10/3/24 2:58 PM, Alexis Lothoré wrote:
>> On 10/3/24 12:49, Marek Vasut wrote:
>>> On 10/3/24 10:31 AM, Alexis Lothoré wrote:
>>>> On 9/29/24 17:23, Marek Vasut wrote:
>>>>> On 9/28/24 1:18 PM, Kalle Valo wrote:
>>>>>> Marek Vasut <marex@denx.de> writes:
>>>>>>
>>>>>>> The WILC3000 can suspend and enter low power state. According to local
>>>>>>> measurements, the WILC3000 consumes the same amount of power if the slot
>>>>>>> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
>>>>>>> off. Use the former option, keep the WILC3000 powered up as that allows
>>>>>>> for things like WoWlan to work.
>>>>>>>
>>>>>>> Note that this is tested on WILC3000 only, not on WILC1000 .
>>>>>>>
>>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>>> ---
>>>>>>> Cc: Ajay Singh <ajay.kathat@microchip.com>
>>>>>>> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
>>>>>>> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>>>>>> Cc: Kalle Valo <kvalo@kernel.org>
>>>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>>>> Cc: linux-wireless@vger.kernel.org
>>>>>>> ---
>>>>>>> V2: Rebase on next-20240926
>>>>>>
>>>>>> BTW I recommend using wireless-next as the baseline for wireless
>>>>>> patches. For example, wireless-next is not pulled to linux-next during
>>>>>> merge windows or other patches in linux-next might create unnecessary
>>>>>> conflicts. Of course most of the cases using linux-next is fine.
>>>>> I didn't know there was one such tree, added to remotes, thanks !
>>>>
>>>> +1, as already mentioned in previous revisions, I would gladly test wilc3000
>>>> changes on both wilc3000 and wilc1000 on my platform, and having the series on
>>>> top of wireless-next would allow to do it on top of any change also affecting
>>>> the driver in wireless-next :)
>>>
>>> I just had a look at a diff between wireless-next/main and next/master 20241003
>>> for drivers/net/wireless/microchip , there are no changes to the driver between
>>> the two trees, so it should be possible to test this patch on either tree. Can
>>> you give it a try ? Ideally test this patch separately on WILC1000 across
>>> suspend/resume and check if it works. You might need the MMC controller fix
>>> which sets struct mmc_host .pm_caps |= MMC_PM_KEEP_POWER for your controller ,
>>> unless this is already upstream.
>>>
>>> The WILC3000 series depends on this patch.
>>>
>> Meh, you are right, I have read too fast your answer to my initial question
>> about used base branch, and omitted the suspend/resume patch (I assumed it
>> conflicted because of some other patches in wireless-next).
> Nope, it conflicted because I (again) didn't include cover letter with the
> WILC3000 series, which mentions this information. I really need to find some
> suitable tooling to manage the cover letters, branch description does not seem
> to cut it quite as I hoped it would.

May I suggest b4 [0] for such task ? It really eases series submissions, taking
care of all the small details that can be easily forgotten :) Latest versions of
the tool will warn you during the sending phase if you forgot to create/update
your cover letter (which is stored by default as an empty commit, this can be
tuned). It is even able now to manage patch sets dependencies (but I still did
not have the opportunity to give this feature a try)

[0] https://b4.docs.kernel.org/en/latest/
Marek Vasut Oct. 3, 2024, 2:18 p.m. UTC | #9
On 10/3/24 3:59 PM, Alexis Lothoré wrote:
> On 10/3/24 15:18, Marek Vasut wrote:
>> On 10/3/24 2:58 PM, Alexis Lothoré wrote:
>>> On 10/3/24 12:49, Marek Vasut wrote:
>>>> On 10/3/24 10:31 AM, Alexis Lothoré wrote:
>>>>> On 9/29/24 17:23, Marek Vasut wrote:
>>>>>> On 9/28/24 1:18 PM, Kalle Valo wrote:
>>>>>>> Marek Vasut <marex@denx.de> writes:
>>>>>>>
>>>>>>>> The WILC3000 can suspend and enter low power state. According to local
>>>>>>>> measurements, the WILC3000 consumes the same amount of power if the slot
>>>>>>>> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
>>>>>>>> off. Use the former option, keep the WILC3000 powered up as that allows
>>>>>>>> for things like WoWlan to work.
>>>>>>>>
>>>>>>>> Note that this is tested on WILC3000 only, not on WILC1000 .
>>>>>>>>
>>>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>>>> ---
>>>>>>>> Cc: Ajay Singh <ajay.kathat@microchip.com>
>>>>>>>> Cc: Alexis Lothoré <alexis.lothore@bootlin.com>
>>>>>>>> Cc: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>>>>>>> Cc: Kalle Valo <kvalo@kernel.org>
>>>>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>>>>> Cc: linux-wireless@vger.kernel.org
>>>>>>>> ---
>>>>>>>> V2: Rebase on next-20240926
>>>>>>>
>>>>>>> BTW I recommend using wireless-next as the baseline for wireless
>>>>>>> patches. For example, wireless-next is not pulled to linux-next during
>>>>>>> merge windows or other patches in linux-next might create unnecessary
>>>>>>> conflicts. Of course most of the cases using linux-next is fine.
>>>>>> I didn't know there was one such tree, added to remotes, thanks !
>>>>>
>>>>> +1, as already mentioned in previous revisions, I would gladly test wilc3000
>>>>> changes on both wilc3000 and wilc1000 on my platform, and having the series on
>>>>> top of wireless-next would allow to do it on top of any change also affecting
>>>>> the driver in wireless-next :)
>>>>
>>>> I just had a look at a diff between wireless-next/main and next/master 20241003
>>>> for drivers/net/wireless/microchip , there are no changes to the driver between
>>>> the two trees, so it should be possible to test this patch on either tree. Can
>>>> you give it a try ? Ideally test this patch separately on WILC1000 across
>>>> suspend/resume and check if it works. You might need the MMC controller fix
>>>> which sets struct mmc_host .pm_caps |= MMC_PM_KEEP_POWER for your controller ,
>>>> unless this is already upstream.
>>>>
>>>> The WILC3000 series depends on this patch.
>>>>
>>> Meh, you are right, I have read too fast your answer to my initial question
>>> about used base branch, and omitted the suspend/resume patch (I assumed it
>>> conflicted because of some other patches in wireless-next).
>> Nope, it conflicted because I (again) didn't include cover letter with the
>> WILC3000 series, which mentions this information. I really need to find some
>> suitable tooling to manage the cover letters, branch description does not seem
>> to cut it quite as I hoped it would.
> 
> May I suggest b4 [0] for such task ? It really eases series submissions, taking
> care of all the small details that can be easily forgotten :) Latest versions of
> the tool will warn you during the sending phase if you forgot to create/update
> your cover letter (which is stored by default as an empty commit, this can be
> tuned). It is even able now to manage patch sets dependencies (but I still did
> not have the opportunity to give this feature a try)
> 
> [0] https://b4.docs.kernel.org/en/latest/
I haven't set up b4 yet, but empty commit might actually work in my 
workflow.
Alexis Lothoré (eBPF Foundation) Oct. 3, 2024, 3:51 p.m. UTC | #10
On 9/26/24 21:50, Marek Vasut wrote:
> The WILC3000 can suspend and enter low power state. According to local
> measurements, the WILC3000 consumes the same amount of power if the slot
> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
> off. Use the former option, keep the WILC3000 powered up as that allows
> for things like WoWlan to work.
> 
> Note that this is tested on WILC3000 only, not on WILC1000 .

So I have tested this change on wilc1000 over sdio (after enabling
MMC_PM_KEEP_POWER capability on my sdmmc controller), and LGTM, system properly
enters and leave suspend, and on resume module is working (ie in sta mode, chip
properly reconnects to configured AP).

The only concern I still have is about existing user who currently do not
declare MMC_PM_KEEP_POWER cap correctly on their platform (as it was the case
for me, and as Ajay eventually raised in the first revision):
- they can currently enter system suspend (and yes, for at least some Atmel
platforms, wilc chip still works on resume because it is "accidentally" kept
powered on)
- after this change, they will fail to do so (because of the failing call to
sdio_set_host_pm_flags, which makes the whole suspend method fail)

But I guess the issue is rather on the sdio host controller description on those
platforms, which should always have the MMC_PM_KEEP_POWER cap set ? If so:

Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>
Tested-on: WILC1000SD 07 SDIO WILC_WIFI_FW_REL_16_1_2
Kalle Valo Oct. 17, 2024, 4:47 p.m. UTC | #11
Marek Vasut <marex@denx.de> wrote:

> The WILC3000 can suspend and enter low power state. According to local
> measurements, the WILC3000 consumes the same amount of power if the slot
> is powered up and WILC3000 is suspended, and if the WILC3000 is powered
> off. Use the former option, keep the WILC3000 powered up as that allows
> for things like WoWlan to work.
> 
> Note that this is tested on WILC3000 only, not on WILC1000 .
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com>

Patch applied to wireless-next.git, thanks.

98ca3178ad79 wifi: wilc1000: Keep slot powered on during suspend/resume
diff mbox series

Patch

diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
index b4da05d5a498a..d67662b6b2a1a 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -969,7 +969,6 @@  static int wilc_sdio_suspend(struct device *dev)
 {
 	struct sdio_func *func = dev_to_sdio_func(dev);
 	struct wilc *wilc = sdio_get_drvdata(func);
-	int ret;
 
 	dev_info(dev, "sdio suspend\n");
 
@@ -983,13 +982,7 @@  static int wilc_sdio_suspend(struct device *dev)
 
 	wilc_sdio_disable_interrupt(wilc);
 
-	ret = wilc_sdio_reset(wilc);
-	if (ret) {
-		dev_err(&func->dev, "Fail reset sdio\n");
-		return ret;
-	}
-
-	return 0;
+	return sdio_set_host_pm_flags(func, MMC_PM_KEEP_POWER);
 }
 
 static int wilc_sdio_resume(struct device *dev)