A recent LKML thread (http://lkml.indiana.edu/hypermail/linux/kernel/1112.3/00965.html)
discusses warnings that occur during a suspend/resume cycle. The driver
attempts to read the firmware file before userspace is ready, leading to the
following warning:
WARNING: at drivers/base/firmware_class.c:537 _request_firmware+0x3f6/0x420()
For rtl8192cu, the problem is fixed by storing the firmware in a global buffer
rather than one allocated per device. The usage count is increased when
suspending and decreased when resuming. This way, the firmware is retained
through a suspend/resume cycle, and does not have to be reread.
This patch should fix the bug reported in
https://bugzilla.redhat.com/show_bug.cgi?id=771002.
Note: This patch also touches rtl8192ce as the "firmware" loaded message
is now printed in the wrong place.
Reported-by: Mohammed Arafa <[email protected]>
Reported-by: Dave Jones <[email protected]>
Signed-off-by: Larry Finger <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Stable <[email protected]>
---
John/Linus,
This patch should be pushed in V3.2 (if possible).
There will be similar patches for rtl8192{ce,se,de}, but as they do not
trigger a warning, I'll save them for later.
Larry
---
Index: wireless-testing-new/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
+++ wireless-testing-new/drivers/net/wireless/rtlwifi/rtl8192c/fw_common.c
@@ -265,7 +265,6 @@ int rtl92c_download_fw(struct ieee80211_
if (!rtlhal->pfirmware)
return 1;
- pr_info("Loading firmware file %s\n", rtlpriv->cfg->fw_name);
pfwheader = (struct rtl92c_firmware_header *)rtlhal->pfirmware;
pfwdata = (u8 *) rtlhal->pfirmware;
fwsize = rtlhal->fwsize;
Index: wireless-testing-new/drivers/net/wireless/rtlwifi/rtl8192ce/sw.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/rtlwifi/rtl8192ce/sw.c
+++ wireless-testing-new/drivers/net/wireless/rtlwifi/rtl8192ce/sw.c
@@ -186,6 +186,7 @@ int rtl92c_init_sw_vars(struct ieee80211
memcpy(rtlpriv->rtlhal.pfirmware, firmware->data, firmware->size);
rtlpriv->rtlhal.fwsize = firmware->size;
release_firmware(firmware);
+ pr_info("rtl8192ce: Loaded firmware file %s\n", rtlpriv->cfg->fw_name);
return 0;
}
Index: wireless-testing-new/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
+++ wireless-testing-new/drivers/net/wireless/rtlwifi/rtl8192cu/sw.c
@@ -43,6 +43,8 @@
#include "hw.h"
#include <linux/vmalloc.h>
#include <linux/module.h>
+#include <linux/atomic.h>
+#include <linux/types.h>
MODULE_AUTHOR("Georgia <[email protected]>");
MODULE_AUTHOR("Ziv Huang <[email protected]>");
@@ -51,6 +53,10 @@ MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Realtek 8192C/8188C 802.11n USB wireless");
MODULE_FIRMWARE("rtlwifi/rtl8192cufw.bin");
+static char *rtl8192cu_firmware; /* pointer to firmware */
+static int firmware_size;
+static atomic_t usage_count;
+
static int rtl92cu_init_sw_vars(struct ieee80211_hw *hw)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
@@ -62,12 +68,21 @@ static int rtl92cu_init_sw_vars(struct i
rtlpriv->dm.disable_framebursting = 0;
rtlpriv->dm.thermalvalue = 0;
rtlpriv->dbg.global_debuglevel = rtlpriv->cfg->mod_params->debug;
- rtlpriv->rtlhal.pfirmware = vmalloc(0x4000);
- if (!rtlpriv->rtlhal.pfirmware) {
+
+ if (rtl8192cu_firmware) {
+ /* firmware already loaded - true for suspend/resume
+ * and multiple instances of the device */
+ rtlpriv->rtlhal.pfirmware = rtl8192cu_firmware;
+ rtlpriv->rtlhal.fwsize = firmware_size;
+ return 0;
+ }
+ rtl8192cu_firmware = vzalloc(0x4000);
+ if (!rtl8192cu_firmware) {
RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
("Can't alloc buffer for fw.\n"));
return 1;
}
+
/* request fw */
err = request_firmware(&firmware, rtlpriv->cfg->fw_name,
rtlpriv->io.dev);
@@ -82,9 +97,14 @@ static int rtl92cu_init_sw_vars(struct i
release_firmware(firmware);
return 1;
}
- memcpy(rtlpriv->rtlhal.pfirmware, firmware->data, firmware->size);
+ pr_info("rtl8192cu: Loaded firmware from file %s\n",
+ rtlpriv->cfg->fw_name);
+ memcpy(rtl8192cu_firmware, firmware->data, firmware->size);
+ firmware_size = firmware->size;
rtlpriv->rtlhal.fwsize = firmware->size;
+ rtlpriv->rtlhal.pfirmware = rtl8192cu_firmware;
release_firmware(firmware);
+ atomic_inc(&usage_count);
return 0;
}
@@ -93,12 +113,30 @@ static void rtl92cu_deinit_sw_vars(struc
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
- if (rtlpriv->rtlhal.pfirmware) {
- vfree(rtlpriv->rtlhal.pfirmware);
+ atomic_dec(&usage_count);
+ if (!atomic_read(&usage_count) && rtlpriv->rtlhal.pfirmware) {
+ vfree(rtl8192cu_firmware);
+ rtl8192cu_firmware = NULL;
rtlpriv->rtlhal.pfirmware = NULL;
}
}
+#ifdef CONFIG_PM
+static int rtl8192cu_usb_suspend(struct usb_interface *pusb_intf,
+ pm_message_t message)
+{
+ /* Increase usage_count to Save loaded fw across suspend/resume */
+ atomic_inc(&usage_count);
+ return 0;
+}
+
+static int rtl8192cu_usb_resume(struct usb_interface *pusb_intf)
+{
+ atomic_dec(&usage_count); /* after resume, decrease usage count */
+ return 0;
+}
+#endif
+
static struct rtl_hal_ops rtl8192cu_hal_ops = {
.init_sw_vars = rtl92cu_init_sw_vars,
.deinit_sw_vars = rtl92cu_deinit_sw_vars,
@@ -375,9 +413,9 @@ static struct usb_driver rtl8192cu_drive
.id_table = rtl8192c_usb_ids,
#ifdef CONFIG_PM
- /* .suspend = rtl_usb_suspend, */
- /* .resume = rtl_usb_resume, */
- /* .reset_resume = rtl8192c_resume, */
+ .suspend = rtl8192cu_usb_suspend,
+ .resume = rtl8192cu_usb_resume,
+ .reset_resume = rtl8192cu_usb_resume,
#endif /* CONFIG_PM */
#ifdef CONFIG_AUTOSUSPEND
.supports_autosuspend = 1,
Sorry, Larry, I didn't see this til it was included in John's mail this morning.
On Tue, Jan 3, 2012 at 10:39 AM, Larry Finger <[email protected]> wrote:
>
> +#ifdef CONFIG_PM
> +static int rtl8192cu_usb_suspend(struct usb_interface *pusb_intf,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pm_message_t message)
> +{
> + ? ? ? /* Increase usage_count to Save loaded fw across suspend/resume */
> + ? ? ? atomic_inc(&usage_count);
> + ? ? ? return 0;
> +}
> +
> +static int rtl8192cu_usb_resume(struct usb_interface *pusb_intf)
> +{
> + ? ? ? atomic_dec(&usage_count); ? ? ? /* after resume, decrease usage count */
> + ? ? ? return 0;
> +}
> +#endif
Based on the warning-fix I submitted yesterday, this probably produces
a warning if CONFIG_PM is set but not CONFIG_PM_SLEEP. My patch was
for an ethernet driver, but iwlwifi and ath5k appear to follow the
same pattern using CONFIG_PM_SLEEP instead of the above.
See: http://www.spinics.net/lists/netdev/msg184717.html for the patch
and http://www.spinics.net/lists/netdev/msg184715.html for the
discussion.
Dan
On Wed, Jan 4, 2012 at 9:14 AM, Larry Finger <[email protected]> wrote:
> On 01/04/2012 10:16 AM, Daniel Halperin wrote:
>>
>> Sorry, Larry, I didn't see this til it was included in John's mail this
>> morning.
>>
>> On Tue, Jan 3, 2012 at 10:39 AM, Larry Finger<[email protected]>
>> ?wrote:
>>>
>>>
>>> +#ifdef CONFIG_PM
>>> +static int rtl8192cu_usb_suspend(struct usb_interface *pusb_intf,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pm_message_t message)
>>> +{
>>> + ? ? ? /* Increase usage_count to Save loaded fw across suspend/resume
>>> */
>>> + ? ? ? atomic_inc(&usage_count);
>>> + ? ? ? return 0;
>>> +}
>>> +
>>> +static int rtl8192cu_usb_resume(struct usb_interface *pusb_intf)
>>> +{
>>> + ? ? ? atomic_dec(&usage_count); ? ? ? /* after resume, decrease usage
>>> count */
>>> + ? ? ? return 0;
>>> +}
>>> +#endif
>>
>>
>> Based on the warning-fix I submitted yesterday, this probably produces
>> a warning if CONFIG_PM is set but not CONFIG_PM_SLEEP. My patch was
>> for an ethernet driver, but iwlwifi and ath5k appear to follow the
>> same pattern using CONFIG_PM_SLEEP instead of the above.
>>
>> See: http://www.spinics.net/lists/netdev/msg184717.html for the patch
>> and http://www.spinics.net/lists/netdev/msg184715.html for the
>> discussion.
>
>
> Thanks for the note. I will prepare a revised patch.
>
> What architecture gave the warning? Of x86_64, I was unable to get
> CONFIG_PM_SLEEP without CONFIG_PM.
>
x86-64. I realized that I actually explained this problem rather
clearly when I fixed it for iwlwifi 5 months ago. :)
https://github.com/mirrors/linux/commit/f090fba305658fe6e464e2fbd25fad81957ece26
"""
CC [M] drivers/net/wireless/iwlwifi/iwl-pci.o
drivers/net/wireless/iwlwifi/iwl-pci.c:506: warning:
?iwl_pci_suspend? defined but not used
drivers/net/wireless/iwlwifi/iwl-pci.c:519: warning:
?iwl_pci_resume? defined but not used
These are only used if CONFIG_PM_SLEEP is enabled. CONFIG_PM depends
(CONFIG_PM_SLEEP || CONFIG_PM_RUNTIME), so it can be set without
CONFIG_PM_SLEEP selected.
"""
Dan
On 01/04/2012 10:16 AM, Daniel Halperin wrote:
> Sorry, Larry, I didn't see this til it was included in John's mail this morning.
>
> On Tue, Jan 3, 2012 at 10:39 AM, Larry Finger<[email protected]> wrote:
>>
>> +#ifdef CONFIG_PM
>> +static int rtl8192cu_usb_suspend(struct usb_interface *pusb_intf,
>> + pm_message_t message)
>> +{
>> + /* Increase usage_count to Save loaded fw across suspend/resume */
>> + atomic_inc(&usage_count);
>> + return 0;
>> +}
>> +
>> +static int rtl8192cu_usb_resume(struct usb_interface *pusb_intf)
>> +{
>> + atomic_dec(&usage_count); /* after resume, decrease usage count */
>> + return 0;
>> +}
>> +#endif
>
> Based on the warning-fix I submitted yesterday, this probably produces
> a warning if CONFIG_PM is set but not CONFIG_PM_SLEEP. My patch was
> for an ethernet driver, but iwlwifi and ath5k appear to follow the
> same pattern using CONFIG_PM_SLEEP instead of the above.
>
> See: http://www.spinics.net/lists/netdev/msg184717.html for the patch
> and http://www.spinics.net/lists/netdev/msg184715.html for the
> discussion.
Thanks for the note. I will prepare a revised patch.
What architecture gave the warning? Of x86_64, I was unable to get
CONFIG_PM_SLEEP without CONFIG_PM.
Larry