Re: [PATCH V5 2/2] brcmfmac: don't warn user about NVRAM if fallback to the platform one succeeds
From: Arend van Spriel <arend.vanspriel@broadcom.com>
Date: 2017-08-01 21:20:39
Also in:
lkml, netdev
On 31-07-17 17:09, Rafał Miłecki wrote:
From: Rafał Miłecki <rafal@milecki.pl> Failing to load NVRAM *file* isn't critical if we manage to get platform NVRAM in the fallback path. It means warnings like: [ 10.801506] brcmfmac 0000:01:00.0: Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2 are unnecessary & disturbing for people with *platform* NVRAM as they are not expected to have NVRAM file. This is a very common case for Broadcom home routers. Instead of printing warning immediately within the firmware subsystem let's try our fallback code first. If that fails as well, then it's a right moment to print an error. This should reduce amount of false reports from users seeing this warning while having wireless working perfectly fine with the platform NVRAM.
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
quoted hunk ↗ jump to hunk
Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- V2: Update commit message as it wasn't clear enough (thanks Andy) & add extra messages to the firmware.c. V3: Set FW_OPT_UEVENT to don't change behavior V4: Switch to the new request_firmware_async syntax V5: Rebase, update commit message, resend after drvdata discussion --- .../wireless/broadcom/brcm80211/brcmfmac/firmware.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c index d231042f19d6..524442b3870f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c@@ -462,8 +462,14 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx) raw_nvram = false; } else { data = bcm47xx_nvram_get_contents(&data_len); - if (!data && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) - goto fail; + if (!data) { + brcmf_dbg(TRACE, "Failed to get platform NVRAM\n");
Better make this INFO level instead of TRACE. The intent of TRACE level is for entry/exit points in functions. Regards, Arend