[PATCH] Remove firmware allocation from wcn struct
Eugene Krasnikov
k.eugene.e at gmail.com
Fri Jul 12 09:44:53 EDT 2013
True, thanx for explanation. Merged;)
2013/7/12 Olof Johansson <dev at skyshaper.net>:
> 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
--
Best regards,
Eugene
More information about the wcn36xx
mailing list