[PATCH] Cache nv to avoid request_firmware on resume path

[Eugene Krasnikov]

Did you enable wow on that interface? remove_interface should not be
called on suspend?

On 18 декабря 2013 г., 18:34, Jason Mobarak <jam at cozybit.com> wrote:
> Hello Pontus--
>
> Recently, I saw this oops on a wcn3660 device:
> https://gist.github.com/silverjam/434feac44e63eea624f1 ... with lots
> of warnings about request_firmware during resume... seems likely this
> patch will help with this, yes?
>
> The oops:
>
> [ 1895.207874] [<bf1356b4>] (wcn36xx_remove_interface+0x34/0x64
> [wcn36xx]) from [<bf0e3eac>] (__ieee80211_suspend+0x89c/0x9a0
> [mac80211])
> [ 1895.208331] [<bf0e3eac>] (__ieee80211_suspend+0x89c/0x9a0
> [mac80211]) from [<bf009f9c>] (wiphy_suspend+0x120/0x334 [cfg80211])
> [ 1895.208484] [<bf009f9c>] (wiphy_suspend+0x120/0x334 [cfg80211])
> from [<c03012b4>] (__device_suspend+0x1e4/0x32c)
> [ 1895.208514] [<c03012b4>] (__device_suspend+0x1e4/0x32c) from
> [<c0301ce0>] (dpm_suspend+0xa8/0x210)
> [ 1895.208576] [<c0301ce0>] (dpm_suspend+0xa8/0x210) from [<c00a5c18>]
> (suspend_devices_and_enter+0xd4/0x314)
> [ 1895.208606] [<c00a5c18>] (suspend_devices_and_enter+0xd4/0x314)
> from [<c00a5f78>] (pm_suspend+0x120/0x200)
> [ 1895.208667] [<c00a5f78>] (pm_suspend+0x120/0x200) from [<c00a6f20>]
> (suspend+0x68/0x180)
> [ 1895.208698] [<c00a6f20>] (suspend+0x68/0x180) from [<c0089198>]
> (process_one_work+0x280/0x488)
> [ 1895.208759] [<c0089198>] (process_one_work+0x280/0x488) from
> [<c00895b4>] (worker_thread+0x214/0x3b4)
> [ 1895.208789] [<c00895b4>] (worker_thread+0x214/0x3b4) from
> [<c008d4cc>] (kthread+0x84/0x90)
> [ 1895.208850] [<c008d4cc>] (kthread+0x84/0x90) from [<c000f028>]
> (kernel_thread_exit+0x0/0x8)
>
> Thanks,
> -Jason
>
>
> On Fri, Dec 13, 2013 at 12:38 AM, Pontus Fuchs <pontus.fuchs at gmail.com> wrote:
>> If wowlan if off mac80211 will stop / start the driver on suspend /
>> resume. This causes problems on resume since request_firmware is called
>> from start. Fix this by caching the nv.
>>
>> Signed-off-by: Pontus Fuchs <pontus.fuchs at gmail.com>
>> ---
>>  main.c    |  2 ++
>>  smd.c     | 22 ++++++++++------------
>>  wcn36xx.h |  2 ++
>>  3 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/main.c b/main.c
>> index 7ef1a91..288c282 100644
>> --- a/main.c
>> +++ b/main.c
>> @@ -18,6 +18,7 @@
>>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>
>>  #include <linux/module.h>
>> +#include <linux/firmware.h>
>>  #include <linux/platform_device.h>
>>  #include "wcn36xx.h"
>>
>> @@ -1052,6 +1053,7 @@ static int wcn36xx_remove(struct platform_device *pdev)
>>         struct wcn36xx *wcn = hw->priv;
>>         wcn36xx_dbg(WCN36XX_DBG_MAC, "platform remove\n");
>>
>> +       release_firmware(wcn->nv);
>>         mutex_destroy(&wcn->hal_mutex);
>>
>>         ieee80211_unregister_hw(hw);
>> diff --git a/smd.c b/smd.c
>> index 182c33c..36b8f03 100644
>> --- a/smd.c
>> +++ b/smd.c
>> @@ -247,21 +247,22 @@ static int wcn36xx_smd_rsp_status_check(void *buf, size_t len)
>>
>>  int wcn36xx_smd_load_nv(struct wcn36xx *wcn)
>>  {
>> -       const struct firmware *nv;
>>         struct nv_data *nv_d;
>>         struct wcn36xx_hal_nv_img_download_req_msg msg_body;
>>         int fw_bytes_left;
>>         int ret;
>>         u16 fm_offset = 0;
>>
>> -       ret = request_firmware(&nv, WLAN_NV_FILE, wcn->dev);
>> -       if (ret) {
>> -               wcn36xx_err("Failed to load nv file %s: %d\n",
>> -                             WLAN_NV_FILE, ret);
>> -               goto out_free_nv;
>> +       if (!wcn->nv) {
>> +               ret = request_firmware(&wcn->nv, WLAN_NV_FILE, wcn->dev);
>> +               if (ret) {
>> +                       wcn36xx_err("Failed to load nv file %s: %d\n",
>> +                                     WLAN_NV_FILE, ret);
>> +                       goto out;
>> +               }
>>         }
>>
>> -       nv_d = (struct nv_data *)nv->data;
>> +       nv_d = (struct nv_data *)wcn->nv->data;
>>         INIT_HAL_MSG(msg_body, WCN36XX_HAL_DOWNLOAD_NV_REQ);
>>
>>         msg_body.header.len += WCN36XX_NV_FRAGMENT_SIZE;
>> @@ -271,7 +272,7 @@ int wcn36xx_smd_load_nv(struct wcn36xx *wcn)
>>         mutex_lock(&wcn->hal_mutex);
>>
>>         do {
>> -               fw_bytes_left = nv->size - fm_offset - 4;
>> +               fw_bytes_left = wcn->nv->size - fm_offset - 4;
>>                 if (fw_bytes_left > WCN36XX_NV_FRAGMENT_SIZE) {
>>                         msg_body.last_fragment = 0;
>>                         msg_body.nv_img_buffer_size = WCN36XX_NV_FRAGMENT_SIZE;
>> @@ -309,10 +310,7 @@ int wcn36xx_smd_load_nv(struct wcn36xx *wcn)
>>
>>  out_unlock:
>>         mutex_unlock(&wcn->hal_mutex);
>> -out_free_nv:
>> -       release_firmware(nv);
>> -
>> -       return ret;
>> +out:   return ret;
>>  }
>>
>>  static int wcn36xx_smd_start_rsp(struct wcn36xx *wcn, void *buf, size_t len)
>> diff --git a/wcn36xx.h b/wcn36xx.h
>> index 9d217e5..38cf3a7 100644
>> --- a/wcn36xx.h
>> +++ b/wcn36xx.h
>> @@ -172,6 +172,8 @@ struct wcn36xx {
>>         struct device           *dev;
>>         struct list_head        vif_list;
>>
>> +       const struct firmware   *nv;
>> +
>>         u8                      fw_revision;
>>         u8                      fw_version;
>>         u8                      fw_minor;
>> --
>> 1.8.3.2
>>
>>
>> _______________________________________________
>> wcn36xx mailing list
>> wcn36xx at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/wcn36xx
>
> _______________________________________________
> wcn36xx mailing list
> wcn36xx at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/wcn36xx



-- 
Best regards,
Eugene