2017-03-08 14:01:03

by Hans de Goede

[permalink] [raw]
Subject: [PATCH v3 1/3] brcmfmac: Do not print the firmware version as an error

Using pr_err for things which are not errors is a bad idea. E.g. it
will cause the plymouth bootsplash screen to drop back to the text
console so that the user can see the error, which is not what we
normally want to happen.

Instead add a new brcmf_info macro and use that.

Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v2:
-Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
Changes in v3:
-Use do { } while (0) around macro
-Rebase on top of v4.11-rc1
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 33b133f..7a2b495 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -161,7 +161,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
strsep(&ptr, "\n");

/* Print fw version info */
- brcmf_err("Firmware version = %s\n", buf);
+ brcmf_info("Firmware version = %s\n", buf);

/* locate firmware version number for ethtool */
ptr = strrchr(buf, ' ') + 1;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
index 0661261..afb2436 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -59,6 +59,10 @@ void __brcmf_err(const char *func, const char *fmt, ...);
} while (0)

#if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
+
+/* For debug/tracing purposes treat info messages as errors */
+#define brcmf_info brcmf_err
+
__printf(3, 4)
void __brcmf_dbg(u32 level, const char *func, const char *fmt, ...);
#define brcmf_dbg(level, fmt, ...) \
@@ -77,6 +81,11 @@ do { \

#else /* defined(DEBUG) || defined(CONFIG_BRCM_TRACING) */

+#define brcmf_info(fmt, ...) \
+ do { \
+ pr_info("%s: " fmt, __func__, ##__VA_ARGS__); \
+ } while (0)
+
#define brcmf_dbg(level, fmt, ...) no_printk(fmt, ##__VA_ARGS__)

#define BRCMF_DATA_ON() 0
--
2.9.3


2017-03-20 17:14:38

by Kalle Valo

[permalink] [raw]
Subject: Re: [v3,1/3] brcmfmac: Do not print the firmware version as an error

Hans de Goede <[email protected]> wrote:
> Using pr_err for things which are not errors is a bad idea. E.g. it
> will cause the plymouth bootsplash screen to drop back to the text
> console so that the user can see the error, which is not what we
> normally want to happen.
>
> Instead add a new brcmf_info macro and use that.
>
> Signed-off-by: Hans de Goede <[email protected]>
> Acked-by: Arend van Spriel <[email protected]>

3 patches applied to wireless-drivers-next.git, thanks.

d79fe4cb70d8 brcmfmac: Do not print the firmware version as an error
26e537884a8e brcmfmac: Do not complain about country code "00"
b9472a2e3e45 brcmfmac: Handle status == BRCMF_E_STATUS_ABORT in cfg80211_escan_handler

--
https://patchwork.kernel.org/patch/9611177/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2017-03-08 14:01:03

by Hans de Goede

[permalink] [raw]
Subject: [PATCH v3 2/3] brcmfmac: Do not complain about country code "00"

The country code gets set to "00" by default at boot, ignore this
rather then logging an error about it.

Signed-off-by: Hans de Goede <[email protected]>
Acked-by: Arend van Spriel <[email protected]>
---
Changes in v3:
-Add Arend's Acked-by
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 944b83c..7765ad0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6736,6 +6736,10 @@ static void brcmf_cfg80211_reg_notifier(struct wiphy *wiphy,
s32 err;
int i;

+ /* The country code gets set to "00" by default at boot, ignore */
+ if (req->alpha2[0] == '0' && req->alpha2[1] == '0')
+ return;
+
/* ignore non-ISO3166 country codes */
for (i = 0; i < sizeof(req->alpha2); i++)
if (req->alpha2[i] < 'A' || req->alpha2[i] > 'Z') {
--
2.9.3

2017-03-09 08:19:34

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] brcmfmac: Do not print the firmware version as an error

On 8-3-2017 14:50, Hans de Goede wrote:
> Using pr_err for things which are not errors is a bad idea. E.g. it
> will cause the plymouth bootsplash screen to drop back to the text
> console so that the user can see the error, which is not what we
> normally want to happen.
>
> Instead add a new brcmf_info macro and use that.

Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> Changes in v2:
> -Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
> Changes in v3:
> -Use do { } while (0) around macro
> -Rebase on top of v4.11-rc1
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 9 +++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> index 33b133f..7a2b495 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> @@ -161,7 +161,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
> strsep(&ptr, "\n");
>
> /* Print fw version info */
> - brcmf_err("Firmware version = %s\n", buf);
> + brcmf_info("Firmware version = %s\n", buf);
>
> /* locate firmware version number for ethtool */
> ptr = strrchr(buf, ' ') + 1;
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> index 0661261..afb2436 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> @@ -59,6 +59,10 @@ void __brcmf_err(const char *func, const char *fmt, ...);
> } while (0)
>
> #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
> +
> +/* For debug/tracing purposes treat info messages as errors */
> +#define brcmf_info brcmf_err
> +
> __printf(3, 4)
> void __brcmf_dbg(u32 level, const char *func, const char *fmt, ...);
> #define brcmf_dbg(level, fmt, ...) \
> @@ -77,6 +81,11 @@ do { \
>
> #else /* defined(DEBUG) || defined(CONFIG_BRCM_TRACING) */
>
> +#define brcmf_info(fmt, ...) \
> + do { \
> + pr_info("%s: " fmt, __func__, ##__VA_ARGS__); \
> + } while (0)
> +
> #define brcmf_dbg(level, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
>
> #define BRCMF_DATA_ON() 0
>

2017-03-08 14:01:03

by Hans de Goede

[permalink] [raw]
Subject: [PATCH v3 3/3] brcmfmac: Handle status == BRCMF_E_STATUS_ABORT in cfg80211_escan_handler

If a scan gets aborted BRCMF_SCAN_STATUS_BUSY gets cleared in
cfg->scan_status and when we receive an abort event from the firmware
the BRCMF_SCAN_STATUS_BUSY check in the cfg80211_escan_handler will
trigger resulting in multiple errors getting logged.

Check for a status of BRCMF_E_STATUS_ABORT and in this case simply
cleanly exit the cfg80211_escan_handler. This also avoids a
BRCMF_E_STATUS_ABORT event arriving after a new scan has been started
causing the new scan to complete prematurely without any data.

Signed-off-by: Hans de Goede <[email protected]>
Acked-by: Arend van Spriel <[email protected]>
---
Changes in v3:
-Add Arend's Acked-by
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 7765ad0..4e1ca69 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3097,6 +3097,9 @@ brcmf_cfg80211_escan_handler(struct brcmf_if *ifp,

status = e->status;

+ if (status == BRCMF_E_STATUS_ABORT)
+ goto exit;
+
if (!test_bit(BRCMF_SCAN_STATUS_BUSY, &cfg->scan_status)) {
brcmf_err("scan not ready, bsscfgidx=%d\n", ifp->bsscfgidx);
return -EPERM;
--
2.9.3