[PATCH] Remove firmware allocation from wcn struct

Olof Johansson dev at skyshaper.net
Fri Jul 12 09:42:34 EDT 2013


Mac addresses are kept on the file system. Then kernel needs
request_firmware to read stuff from the file system like this.
https://github.com/KrasnikovEugene/wcn36xx/blob/master/main.c#L848

Cheerio
--
Olof

On Fri, Jul 12, 2013 at 3:40 PM, Eugene Krasnikov <k.eugene.e at gmail.com> wrote:
> Sorry but do not see any connection between firmware.h and mac
> addresses. Could you please explain in more details which line in
> main.c requires which line from firmware.h?:)
>
> 2013/7/12 Olof Johansson <dev at skyshaper.net>:
>> Yes. Mac addresses.
>>
>> Did not see them belonging in the smd parts of the code so I left that
>> code where it was.
>> But I didn't see any reason to keep the include in wcn36xx.h when it
>> was only needed in two files.
>>
>> Cheers
>> --
>> Olof
>>
>> On Fri, Jul 12, 2013 at 3:34 PM, Eugene Krasnikov <k.eugene.e at gmail.com> wrote:
>>> Was about to merege this patch but noticed this line in main.c:
>>> +#include <linux/firmware.h>
>>>
>>> Do we need it for something?
>>>
>>> 2013/7/12 Eugene Krasnikov <k.eugene.e at gmail.com>:
>>>> Looks good to me.
>>>>
>>>> 2013/7/12 Olof Johansson <dev at skyshaper.net>:
>>>>> And pull request: https://github.com/KrasnikovEugene/wcn36xx/pull/79
>>>>>
>>>>> On Fri, Jul 12, 2013 at 9:17 AM, Olof Johansson <dev at skyshaper.net> wrote:
>>>>>> Remove the nv pointer from the wcn struct and keep it inside the
>>>>>> smd_load_nv function as local. This way we don't keep it allocated
>>>>>> throught the life of the module and won't waste memory.
>>>>>>
>>>>>> This commit also includes minor cleanup of unused variables in the
>>>>>> smd_load_nv function.
>>>>>>
>>>>>> Signed-off-by: Olof Johansson <dev at skyshaper.net>
>>>>>> ---
>>>>>>  main.c    | 13 ++-----------
>>>>>>  smd.c     | 19 ++++++++++++++-----
>>>>>>  wcn36xx.h |  2 --
>>>>>>  3 files changed, 16 insertions(+), 18 deletions(-)
>>>>>>
>>>>>> diff --git a/main.c b/main.c
>>>>>> index 6800b17..d9ea7bc 100644
>>>>>> --- a/main.c
>>>>>> +++ b/main.c
>>>>>> @@ -16,6 +16,7 @@
>>>>>>
>>>>>>  #include <linux/module.h>
>>>>>>  #include <linux/wcnss_wlan.h>
>>>>>> +#include <linux/firmware.h>
>>>>>>  #include "wcn36xx.h"
>>>>>>
>>>>>>  unsigned int debug_mask;
>>>>>> @@ -961,24 +962,15 @@ static int __init wcn36xx_init(void)
>>>>>>         private_hw = hw;
>>>>>>         wcn->beacon_enable = false;
>>>>>>
>>>>>> -       ret = request_firmware(&wcn->nv, WLAN_NV_FILE, wcn->dev);
>>>>>> -       if (ret) {
>>>>>> -               wcn36xx_error("Failed to load nv file %s: %d", WLAN_NV_FILE,
>>>>>> -                             ret);
>>>>>> -               goto out_unmap;
>>>>>> -       }
>>>>>> -
>>>>>>         wcn36xx_read_mac_addresses(wcn);
>>>>>>         SET_IEEE80211_PERM_ADDR(wcn->hw, wcn->addresses[0].addr);
>>>>>>
>>>>>>         ret = ieee80211_register_hw(wcn->hw);
>>>>>>         if (ret)
>>>>>> -               goto out_free_nv;
>>>>>> +               goto out_unmap;
>>>>>>
>>>>>>         return 0;
>>>>>>
>>>>>> -out_free_nv:
>>>>>> -       release_firmware(wcn->nv);
>>>>>>  out_unmap:
>>>>>>         iounmap(wcn->mmio);
>>>>>>  out_wq:
>>>>>> @@ -999,7 +991,6 @@ static void __exit wcn36xx_exit(void)
>>>>>>         ieee80211_unregister_hw(hw);
>>>>>>         destroy_workqueue(wcn->wq);
>>>>>>         iounmap(wcn->mmio);
>>>>>> -       release_firmware(wcn->nv);
>>>>>>         ieee80211_free_hw(hw);
>>>>>>  }
>>>>>>  module_exit(wcn36xx_exit);
>>>>>> diff --git a/smd.c b/smd.c
>>>>>> index d3597ca..8ca15f1 100644
>>>>>> --- a/smd.c
>>>>>> +++ b/smd.c
>>>>>> @@ -15,6 +15,7 @@
>>>>>>   */
>>>>>>
>>>>>>  #include <linux/etherdevice.h>
>>>>>> +#include <linux/firmware.h>
>>>>>>  #include "smd.h"
>>>>>>
>>>>>>  static int wcn36xx_smd_send_and_wait(struct wcn36xx *wcn, size_t len)
>>>>>> @@ -84,22 +85,27 @@ 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 = 0, fw_size, i = 0;
>>>>>> +       int ret;
>>>>>>         u16 fm_offset = 0;
>>>>>> -       i = 0;
>>>>>>
>>>>>> -       nv_d = (struct nv_data *)wcn->nv->data;
>>>>>> -       fw_size = wcn->nv->size;
>>>>>> +       ret = request_firmware(&nv, WLAN_NV_FILE, wcn->dev);
>>>>>> +       if (ret) {
>>>>>> +               wcn36xx_error("Failed to load nv file %s: %d", WLAN_NV_FILE, ret);
>>>>>> +               goto out_free_nv;
>>>>>> +       }
>>>>>> +
>>>>>> +       nv_d = (struct nv_data *)nv->data;
>>>>>>         INIT_HAL_MSG(msg_body, WCN36XX_HAL_DOWNLOAD_NV_REQ);
>>>>>>
>>>>>>         msg_body.header.len += WCN36XX_NV_FRAGMENT_SIZE;
>>>>>>
>>>>>>         msg_body.frag_number = 0;
>>>>>>         do {
>>>>>> -               fw_bytes_left = wcn->nv->size - fm_offset - 4;
>>>>>> +               fw_bytes_left = 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;
>>>>>> @@ -132,6 +138,9 @@ int wcn36xx_smd_load_nv(struct wcn36xx *wcn)
>>>>>>
>>>>>>         } while (msg_body.last_fragment != 1);
>>>>>>
>>>>>> +out_free_nv:
>>>>>> +       release_firmware(nv);
>>>>>> +
>>>>>>         return ret;
>>>>>>  }
>>>>>>
>>>>>> diff --git a/wcn36xx.h b/wcn36xx.h
>>>>>> index 3c70e7a..743325e 100644
>>>>>> --- a/wcn36xx.h
>>>>>> +++ b/wcn36xx.h
>>>>>> @@ -19,7 +19,6 @@
>>>>>>
>>>>>>  #include <linux/completion.h>
>>>>>>  #include <linux/printk.h>
>>>>>> -#include <linux/firmware.h>
>>>>>>  #include <linux/spinlock.h>
>>>>>>  #include <linux/workqueue.h>
>>>>>>  #include <mach/msm_smd.h>
>>>>>> @@ -112,7 +111,6 @@ struct wcn36xx {
>>>>>>         struct ieee80211_hw     *hw;
>>>>>>         struct workqueue_struct *wq;
>>>>>>         struct device           *dev;
>>>>>> -       const struct firmware   *nv;
>>>>>>         struct mac_address      addresses[2];
>>>>>>         struct wcn36xx_hal_mac_ssid ssid;
>>>>>>         u16                     aid;
>>>>>> --
>>>>>> 1.8.3.1
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> wcn36xx mailing list
>>>>> wcn36xx at lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/wcn36xx
>>>>
>>>>
>>>>
>>>> --
>>>> Best regards,
>>>> Eugene
>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Eugene
>
>
>
> --
> Best regards,
> Eugene



More information about the wcn36xx mailing list