All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafał Miłecki" <zajec5@gmail.com>
To: "Luis R . Rodriguez" <mcgrof@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Bjorn Andersson" <bjorn.andersson@linaro.org>,
	"Daniel Wagner" <wagi@monom.org>,
	"David Woodhouse" <dwmw2@infradead.org>,
	"Arend van Spriel" <arend.vanspriel@broadcom.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	yi1.li@linux.intel.com, atull@kernel.org,
	"Moritz Fischer" <moritz.fischer@ettus.com>,
	pmladek@suse.com, "Johannes Berg" <johannes.berg@intel.com>,
	emmanuel.grumbach@intel.com, luciano.coelho@intel.com,
	"Kalle Valo" <kvalo@codeaurora.org>,
	luto@kernel.org, "Linus Torvalds" <torvalds@linux-foundation.org>,
	"Kees Cook" <keescook@chromium.org>,
	"AKASHI Takahiro" <takahiro.akashi@linaro.org>,
	"David Howells" <dhowells@redhat.com>,
	pjones@redhat.com, "Hans de Goede" <hdegoede@redhat.com>,
	alan@linux.intel.com, tytso@mit.edu,
	lkml <linux-kernel@vger.kernel.org>,
	"Franky Lin" <franky.lin@broadcom.com>,
	"Hante Meuleman" <hante.meuleman@broadcom.com>,
	"Chi-Hsien Lin" <chi-hsien.lin@cypress.com>,
	"Wright Feng" <wright.feng@cypress.com>,
	"Pieter-Paul Giesberts" <pieter-paul.giesberts@broadcom.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	"Rafał Miłecki" <rafal@milecki.pl>
Subject: [PATCH V5 2/2] brcmfmac: don't warn user about NVRAM if fallback to the platform one succeeds
Date: Mon, 31 Jul 2017 17:09:45 +0200	[thread overview]
Message-ID: <20170731150945.8925-2-zajec5@gmail.com> (raw)
In-Reply-To: <20170731150945.8925-1-zajec5@gmail.com>

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.

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");
+			if (!(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) {
+				brcmf_err("Loading NVRAM from %s and using platform one both failed\n",
+					  fwctx->nvram_name);
+				goto fail;
+			}
+		}
 		raw_nvram = true;
 	}
 
@@ -491,6 +497,9 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
 static void brcmf_fw_request_code_done(const struct firmware *fw, void *ctx)
 {
 	struct brcmf_fw *fwctx = ctx;
+	struct firmware_opts fw_opts = {
+		.optional = true,
+	};
 	int ret = 0;
 
 	brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
@@ -503,9 +512,8 @@ static void brcmf_fw_request_code_done(const struct firmware *fw, void *ctx)
 		goto done;
 
 	fwctx->code = fw;
-	ret = request_firmware_nowait(THIS_MODULE, true, fwctx->nvram_name,
-				      fwctx->dev, GFP_KERNEL, fwctx,
-				      brcmf_fw_request_nvram_done);
+	ret = request_firmware_async(fwctx->nvram_name, &fw_opts, fwctx->dev,
+				     fwctx, brcmf_fw_request_nvram_done);
 
 	/* pass NULL to nvram callback for bcm47xx fallback */
 	if (ret)
-- 
2.11.0

WARNING: multiple messages have this Message-ID (diff)
From: "Rafał Miłecki" <zajec5@gmail.com>
To: "Luis R . Rodriguez" <mcgrof@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Daniel Wagner <wagi@monom.org>,
	David Woodhouse <dwmw2@infradead.org>,
	Arend van Spriel <arend.vanspriel@broadcom.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	yi1.li@linux.intel.com, atull@kernel.org,
	Moritz Fischer <moritz.fischer@ettus.com>,
	pmladek@suse.com, Johannes Berg <johannes.berg@intel.com>,
	emmanuel.grumbach@intel.com, luciano.coelho@intel.com,
	Kalle Valo <kvalo@codeaurora.org>,
	luto@kernel.org, Linus Torvalds <torvalds@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	AKASHI Takahiro <takahiro.akashi@linaro.org>,
	David Howells <dhowells@redhat.com>,
	pjones@redhat.com, Hans de Goede <hdegoede@redhat.com>,
	alan@linux.intel.com, tytso@mit.edu,
	lkml <linux-kernel@vger.kernel.org>,
	Franky Lin <franky.li
Subject: [PATCH V5 2/2] brcmfmac: don't warn user about NVRAM if fallback to the platform one succeeds
Date: Mon, 31 Jul 2017 17:09:45 +0200	[thread overview]
Message-ID: <20170731150945.8925-2-zajec5@gmail.com> (raw)
In-Reply-To: <20170731150945.8925-1-zajec5@gmail.com>

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.

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");
+			if (!(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) {
+				brcmf_err("Loading NVRAM from %s and using platform one both failed\n",
+					  fwctx->nvram_name);
+				goto fail;
+			}
+		}
 		raw_nvram = true;
 	}
 
@@ -491,6 +497,9 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
 static void brcmf_fw_request_code_done(const struct firmware *fw, void *ctx)
 {
 	struct brcmf_fw *fwctx = ctx;
+	struct firmware_opts fw_opts = {
+		.optional = true,
+	};
 	int ret = 0;
 
 	brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
@@ -503,9 +512,8 @@ static void brcmf_fw_request_code_done(const struct firmware *fw, void *ctx)
 		goto done;
 
 	fwctx->code = fw;
-	ret = request_firmware_nowait(THIS_MODULE, true, fwctx->nvram_name,
-				      fwctx->dev, GFP_KERNEL, fwctx,
-				      brcmf_fw_request_nvram_done);
+	ret = request_firmware_async(fwctx->nvram_name, &fw_opts, fwctx->dev,
+				     fwctx, brcmf_fw_request_nvram_done);
 
 	/* pass NULL to nvram callback for bcm47xx fallback */
 	if (ret)
-- 
2.11.0

  reply	other threads:[~2017-07-31 15:11 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-31 15:09 [PATCH V5 1/2] firmware: add more flexible request_firmware_async function Rafał Miłecki
2017-07-31 15:09 ` Rafał Miłecki
2017-07-31 15:09 ` Rafał Miłecki [this message]
2017-07-31 15:09   ` [PATCH V5 2/2] brcmfmac: don't warn user about NVRAM if fallback to the platform one succeeds Rafał Miłecki
2017-08-01 21:20   ` Arend van Spriel
2017-08-01 21:20     ` Arend van Spriel
2017-07-31 23:01 ` [PATCH V5 1/2] firmware: add more flexible request_firmware_async function kbuild test robot
2017-07-31 23:01   ` kbuild test robot
2017-08-02 21:30 ` Luis R. Rodriguez
2017-08-02 21:30   ` Luis R. Rodriguez
2017-08-03  5:23   ` Kalle Valo
2017-08-03  5:23     ` Kalle Valo
2017-08-03  5:55     ` Coelho, Luciano
2017-08-03  5:55       ` Coelho, Luciano
2017-08-03  5:55       ` Coelho, Luciano
2017-08-03 16:31       ` Luis R. Rodriguez
2017-08-03 16:31         ` Luis R. Rodriguez
2017-08-10 17:07       ` Luis R. Rodriguez
2017-08-10 17:07         ` Luis R. Rodriguez
2017-08-10 17:05     ` Luis R. Rodriguez
2017-08-10 17:05       ` Luis R. Rodriguez

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=20170731150945.8925-2-zajec5@gmail.com \
    --to=zajec5@gmail.com \
    --cc=alan@linux.intel.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=atull@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=chi-hsien.lin@cypress.com \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=emmanuel.grumbach@intel.com \
    --cc=franky.lin@broadcom.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hante.meuleman@broadcom.com \
    --cc=hdegoede@redhat.com \
    --cc=johannes.berg@intel.com \
    --cc=keescook@chromium.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=luciano.coelho@intel.com \
    --cc=luto@kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=moritz.fischer@ettus.com \
    --cc=netdev@vger.kernel.org \
    --cc=pieter-paul.giesberts@broadcom.com \
    --cc=pjones@redhat.com \
    --cc=pmladek@suse.com \
    --cc=rafal@milecki.pl \
    --cc=rjw@rjwysocki.net \
    --cc=takahiro.akashi@linaro.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=wagi@monom.org \
    --cc=wright.feng@cypress.com \
    --cc=yi1.li@linux.intel.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.