From: "Arend van Spriel" <arend@broadcom.com>
To: "John W. Linville" <linville@tuxdriver.com>
Cc: linux-wireless <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH for-3.9] brcmsmac: request firmware in .start() callback
Date: Thu, 4 Apr 2013 11:36:53 +0200 [thread overview]
Message-ID: <515D49B5.6070803@broadcom.com> (raw)
In-Reply-To: <20130403184936.GA320@tuxdriver.com>
On 04/03/2013 08:49 PM, John W. Linville wrote:
> CC drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.o
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c: In function ‘brcms_remove’:
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c:337:3: error: implicit declaration of function ‘brcms_led_unregister’ [-Werror=implicit-function-declaration]
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c: In function ‘brcms_request_fw’:
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c:385:2: error: implicit declaration of function ‘brcms_release_fw’ [-Werror=implicit-function-declaration]
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c: At top level:
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c:393:13: warning: conflicting types for ‘brcms_release_fw’ [enabled by default]
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c:393:13: error: static declaration of ‘brcms_release_fw’ follows non-static declaration
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c:385:2: note: previous implicit declaration of ‘brcms_release_fw’ was here
> drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c:393:13: warning: ‘brcms_release_fw’ defined but not used [-Wunused-function]
> cc1: some warnings being treated as errors
Yikes. That is not properly rebased on the wireless tree. Sorry about
that. I will correct that and send a working patch.
Gr. AvS
> On Wed, Apr 03, 2013 at 10:41:28AM +0200, Arend van Spriel wrote:
>> The firmware is requested from user-space. To assure the request
>> can be handled it is recommended to do the request upon IFF_UP.
>> For a mac80211 driver the .start() callback can be considered
>> the equivalent.
>>
>> Reported-by: John Talbut <jt@dpets.co.uk>
>> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
>> Reviewed-by: Piotr Haber <phaber@broadcom.com>
>> Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
>> Signed-off-by: Arend van Spriel <arend@broadcom.com>
>> ---
>> Hi John,
>>
>> Forgot this one for the 3.9 kernel, which was reported recently
>> using a system with an encrypted root filesystem although the issue
>> is more generic.
>>
>> It applies to the master branch in the wireless repository.
>>
>> Gr. AvS
>> ---
>> .../net/wireless/brcm80211/brcmsmac/mac80211_if.c | 267 ++++++++++----------
>> 1 file changed, 134 insertions(+), 133 deletions(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
>> index c6451c6..2f063a2 100644
>> --- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
>> +++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
>> @@ -274,6 +274,131 @@ static void brcms_set_basic_rate(struct brcm_rateset *rs, u16 rate, bool is_br)
>> }
>> }
>>
>> +/**
>> + * This function frees the WL per-device resources.
>> + *
>> + * This function frees resources owned by the WL device pointed to
>> + * by the wl parameter.
>> + *
>> + * precondition: can both be called locked and unlocked
>> + *
>> + */
>> +static void brcms_free(struct brcms_info *wl)
>> +{
>> + struct brcms_timer *t, *next;
>> +
>> + /* free ucode data */
>> + if (wl->fw.fw_cnt)
>> + brcms_ucode_data_free(&wl->ucode);
>> + if (wl->irq)
>> + free_irq(wl->irq, wl);
>> +
>> + /* kill dpc */
>> + tasklet_kill(&wl->tasklet);
>> +
>> + if (wl->pub) {
>> + brcms_debugfs_detach(wl->pub);
>> + brcms_c_module_unregister(wl->pub, "linux", wl);
>> + }
>> +
>> + /* free common resources */
>> + if (wl->wlc) {
>> + brcms_c_detach(wl->wlc);
>> + wl->wlc = NULL;
>> + wl->pub = NULL;
>> + }
>> +
>> + /* virtual interface deletion is deferred so we cannot spinwait */
>> +
>> + /* wait for all pending callbacks to complete */
>> + while (atomic_read(&wl->callbacks) > 0)
>> + schedule();
>> +
>> + /* free timers */
>> + for (t = wl->timers; t; t = next) {
>> + next = t->next;
>> +#ifdef DEBUG
>> + kfree(t->name);
>> +#endif
>> + kfree(t);
>> + }
>> +}
>> +
>> +/*
>> +* called from both kernel as from this kernel module (error flow on attach)
>> +* precondition: perimeter lock is not acquired.
>> +*/
>> +static void brcms_remove(struct bcma_device *pdev)
>> +{
>> + struct ieee80211_hw *hw = bcma_get_drvdata(pdev);
>> + struct brcms_info *wl = hw->priv;
>> +
>> + if (wl->wlc) {
>> + brcms_led_unregister(wl);
>> + wiphy_rfkill_set_hw_state(wl->pub->ieee_hw->wiphy, false);
>> + wiphy_rfkill_stop_polling(wl->pub->ieee_hw->wiphy);
>> + ieee80211_unregister_hw(hw);
>> + }
>> +
>> + brcms_free(wl);
>> +
>> + bcma_set_drvdata(pdev, NULL);
>> + ieee80211_free_hw(hw);
>> +}
>> +
>> +/*
>> + * Precondition: Since this function is called in brcms_pci_probe() context,
>> + * no locking is required.
>> + */
>> +static int brcms_request_fw(struct brcms_info *wl, struct bcma_device *pdev)
>> +{
>> + int status;
>> + struct device *device = &pdev->dev;
>> + char fw_name[100];
>> + int i;
>> +
>> + memset(&wl->fw, 0, sizeof(struct brcms_firmware));
>> + for (i = 0; i < MAX_FW_IMAGES; i++) {
>> + if (brcms_firmwares[i] == NULL)
>> + break;
>> + sprintf(fw_name, "%s-%d.fw", brcms_firmwares[i],
>> + UCODE_LOADER_API_VER);
>> + status = request_firmware(&wl->fw.fw_bin[i], fw_name, device);
>> + if (status) {
>> + wiphy_err(wl->wiphy, "%s: fail to load firmware %s\n",
>> + KBUILD_MODNAME, fw_name);
>> + return status;
>> + }
>> + sprintf(fw_name, "%s_hdr-%d.fw", brcms_firmwares[i],
>> + UCODE_LOADER_API_VER);
>> + status = request_firmware(&wl->fw.fw_hdr[i], fw_name, device);
>> + if (status) {
>> + wiphy_err(wl->wiphy, "%s: fail to load firmware %s\n",
>> + KBUILD_MODNAME, fw_name);
>> + return status;
>> + }
>> + wl->fw.hdr_num_entries[i] =
>> + wl->fw.fw_hdr[i]->size / (sizeof(struct firmware_hdr));
>> + }
>> + wl->fw.fw_cnt = i;
>> + status = brcms_ucode_data_init(wl, &wl->ucode);
>> + brcms_release_fw(wl);
>> + return status;
>> +}
>> +
>> +/*
>> + * Precondition: Since this function is called in brcms_pci_probe() context,
>> + * no locking is required.
>> + */
>> +static void brcms_release_fw(struct brcms_info *wl)
>> +{
>> + int i;
>> + for (i = 0; i < MAX_FW_IMAGES; i++) {
>> + release_firmware(wl->fw.fw_bin[i]);
>> + release_firmware(wl->fw.fw_hdr[i]);
>> + }
>> +}
>> +
>> static void brcms_ops_tx(struct ieee80211_hw *hw,
>> struct ieee80211_tx_control *control,
>> struct sk_buff *skb)
>> @@ -297,7 +422,7 @@ static int brcms_ops_start(struct ieee80211_hw *hw)
>> {
>> struct brcms_info *wl = hw->priv;
>> bool blocked;
>> - int err;
>> + int err = 0;
>>
>> ieee80211_wake_queues(hw);
>> spin_lock_bh(&wl->lock);
>> @@ -306,6 +431,14 @@ static int brcms_ops_start(struct ieee80211_hw *hw)
>> if (!blocked)
>> wiphy_rfkill_stop_polling(wl->pub->ieee_hw->wiphy);
>>
>> + if (!wl->ucode.bcm43xx_bomminor) {
>> + err = brcms_request_fw(wl, wl->wlc->hw->d11core);
>> + if (err) {
>> + brcms_remove(wl->wlc->hw->d11core);
>> + return -ENOENT;
>> + }
>> + }
>> +
>> spin_lock_bh(&wl->lock);
>> /* avoid acknowledging frames before a non-monitor device is added */
>> wl->mute_tx = true;
>> @@ -793,128 +926,6 @@ void brcms_dpc(unsigned long data)
>> wake_up(&wl->tx_flush_wq);
>> }
>>
>> -/*
>> - * Precondition: Since this function is called in brcms_pci_probe() context,
>> - * no locking is required.
>> - */
>> -static int brcms_request_fw(struct brcms_info *wl, struct bcma_device *pdev)
>> -{
>> - int status;
>> - struct device *device = &pdev->dev;
>> - char fw_name[100];
>> - int i;
>> -
>> - memset(&wl->fw, 0, sizeof(struct brcms_firmware));
>> - for (i = 0; i < MAX_FW_IMAGES; i++) {
>> - if (brcms_firmwares[i] == NULL)
>> - break;
>> - sprintf(fw_name, "%s-%d.fw", brcms_firmwares[i],
>> - UCODE_LOADER_API_VER);
>> - status = request_firmware(&wl->fw.fw_bin[i], fw_name, device);
>> - if (status) {
>> - wiphy_err(wl->wiphy, "%s: fail to load firmware %s\n",
>> - KBUILD_MODNAME, fw_name);
>> - return status;
>> - }
>> - sprintf(fw_name, "%s_hdr-%d.fw", brcms_firmwares[i],
>> - UCODE_LOADER_API_VER);
>> - status = request_firmware(&wl->fw.fw_hdr[i], fw_name, device);
>> - if (status) {
>> - wiphy_err(wl->wiphy, "%s: fail to load firmware %s\n",
>> - KBUILD_MODNAME, fw_name);
>> - return status;
>> - }
>> - wl->fw.hdr_num_entries[i] =
>> - wl->fw.fw_hdr[i]->size / (sizeof(struct firmware_hdr));
>> - }
>> - wl->fw.fw_cnt = i;
>> - return brcms_ucode_data_init(wl, &wl->ucode);
>> -}
>> -
>> -/*
>> - * Precondition: Since this function is called in brcms_pci_probe() context,
>> - * no locking is required.
>> - */
>> -static void brcms_release_fw(struct brcms_info *wl)
>> -{
>> - int i;
>> - for (i = 0; i < MAX_FW_IMAGES; i++) {
>> - release_firmware(wl->fw.fw_bin[i]);
>> - release_firmware(wl->fw.fw_hdr[i]);
>> - }
>> -}
>> -
>> -/**
>> - * This function frees the WL per-device resources.
>> - *
>> - * This function frees resources owned by the WL device pointed to
>> - * by the wl parameter.
>> - *
>> - * precondition: can both be called locked and unlocked
>> - *
>> - */
>> -static void brcms_free(struct brcms_info *wl)
>> -{
>> - struct brcms_timer *t, *next;
>> -
>> - /* free ucode data */
>> - if (wl->fw.fw_cnt)
>> - brcms_ucode_data_free(&wl->ucode);
>> - if (wl->irq)
>> - free_irq(wl->irq, wl);
>> -
>> - /* kill dpc */
>> - tasklet_kill(&wl->tasklet);
>> -
>> - if (wl->pub) {
>> - brcms_debugfs_detach(wl->pub);
>> - brcms_c_module_unregister(wl->pub, "linux", wl);
>> - }
>> -
>> - /* free common resources */
>> - if (wl->wlc) {
>> - brcms_c_detach(wl->wlc);
>> - wl->wlc = NULL;
>> - wl->pub = NULL;
>> - }
>> -
>> - /* virtual interface deletion is deferred so we cannot spinwait */
>> -
>> - /* wait for all pending callbacks to complete */
>> - while (atomic_read(&wl->callbacks) > 0)
>> - schedule();
>> -
>> - /* free timers */
>> - for (t = wl->timers; t; t = next) {
>> - next = t->next;
>> -#ifdef DEBUG
>> - kfree(t->name);
>> -#endif
>> - kfree(t);
>> - }
>> -}
>> -
>> -/*
>> -* called from both kernel as from this kernel module (error flow on attach)
>> -* precondition: perimeter lock is not acquired.
>> -*/
>> -static void brcms_remove(struct bcma_device *pdev)
>> -{
>> - struct ieee80211_hw *hw = bcma_get_drvdata(pdev);
>> - struct brcms_info *wl = hw->priv;
>> -
>> - if (wl->wlc) {
>> - wiphy_rfkill_set_hw_state(wl->pub->ieee_hw->wiphy, false);
>> - wiphy_rfkill_stop_polling(wl->pub->ieee_hw->wiphy);
>> - ieee80211_unregister_hw(hw);
>> - }
>> -
>> - brcms_free(wl);
>> -
>> - bcma_set_drvdata(pdev, NULL);
>> - ieee80211_free_hw(hw);
>> -}
>> -
>> static irqreturn_t brcms_isr(int irq, void *dev_id)
>> {
>> struct brcms_info *wl;
>> @@ -1047,18 +1058,8 @@ static struct brcms_info *brcms_attach(struct bcma_device *pdev)
>> spin_lock_init(&wl->lock);
>> spin_lock_init(&wl->isr_lock);
>>
>> - /* prepare ucode */
>> - if (brcms_request_fw(wl, pdev) < 0) {
>> - wiphy_err(wl->wiphy, "%s: Failed to find firmware usually in "
>> - "%s\n", KBUILD_MODNAME, "/lib/firmware/brcm");
>> - brcms_release_fw(wl);
>> - brcms_remove(pdev);
>> - return NULL;
>> - }
>> -
>> /* common load-time initialization */
>> wl->wlc = brcms_c_attach((void *)wl, pdev, unit, false, &err);
>> - brcms_release_fw(wl);
>> if (!wl->wlc) {
>> wiphy_err(wl->wiphy, "%s: attach() failed with code %d\n",
>> KBUILD_MODNAME, err);
>> --
>> 1.7.10.4
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
prev parent reply other threads:[~2013-04-04 9:37 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-03 8:41 [PATCH for-3.9] brcmsmac: request firmware in .start() callback Arend van Spriel
2013-04-03 8:55 ` Johannes Berg
2013-04-03 9:27 ` Arend van Spriel
2013-04-03 18:49 ` John W. Linville
2013-04-04 9:36 ` Arend van Spriel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=515D49B5.6070803@broadcom.com \
--to=arend@broadcom.com \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.