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
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 3e15d64..6d565f1 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 6687812..605f260 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -59,11 +59,14 @@
pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
} while (0)
#endif
+#define brcmf_info(fmt, ...) pr_info("%s: " fmt, __func__, ##__VA_ARGS__)
#else
__printf(2, 3)
void __brcmf_err(const char *func, const char *fmt, ...);
#define brcmf_err(fmt, ...) \
__brcmf_err(__func__, fmt, ##__VA_ARGS__)
+/* For tracing purposes treat info messages as errors */
+#define brcmf_info brcmf_err
#endif
#if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
--
2.9.3
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]>
---
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 c0b7f69..4c740b74 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3105,6 +3105,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
The firmware responding with -EBUSY when trying to add an extra virtual-if
is a normal thing, do not print an error for this.
Signed-off-by: Hans de Goede <[email protected]>
---
.../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 14 ++++++++++----
drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 5 ++++-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 7ffc4ab..c54e8b4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -688,11 +688,17 @@ static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy,
return ERR_PTR(-EINVAL);
}
- if (IS_ERR(wdev))
- brcmf_err("add iface %s type %d failed: err=%d\n",
- name, type, (int)PTR_ERR(wdev));
- else
+ if (IS_ERR(wdev)) {
+ err = PTR_ERR(wdev);
+ if (err != -EBUSY)
+ brcmf_err("add iface %s type %d failed: err=%d\n",
+ name, type, err);
+ else
+ brcmf_dbg(INFO, "add iface %s type %d failed: err=%d\n",
+ name, type, err);
+ } else {
brcmf_cfg80211_update_proto_addr_mode(wdev);
+ }
return wdev;
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index de19c7c..b5df0a0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2090,7 +2090,10 @@ static struct wireless_dev *brcmf_p2p_create_p2pdev(struct brcmf_p2p_info *p2p,
/* Initialize P2P Discovery in the firmware */
err = brcmf_fil_iovar_int_set(pri_ifp, "p2p_disc", 1);
if (err < 0) {
- brcmf_err("set p2p_disc error\n");
+ if (err != -EBUSY)
+ brcmf_err("set p2p_disc error\n");
+ else
+ brcmf_dbg(INFO, "set p2p_disc error\n");
brcmf_fweh_p2pdev_setup(pri_ifp, false);
brcmf_cfg80211_arm_vif_event(p2p->cfg, NULL);
goto fail;
--
2.9.3
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]>
---
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 c54e8b4..c0b7f69 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6705,6 +6705,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
Hi,
On 07-03-17 11:03, Arend Van Spriel wrote:
>
>
> On 27-2-2017 22:45, Hans de Goede wrote:
>> The firmware responding with -EBUSY when trying to add an extra virtual-if
>> is a normal thing, do not print an error for this.
>
> This may be something we need to look into. It seems to me the interface
> combinations needs to be fixed so we do not try to provision firmware.
> Can you explain the scenario here?
I'm not doing anything special just connecting to my isp provided accesspoint
using NetworkManager.
Regards,
Hans
>
> Regards,
> Arend
>
>> Signed-off-by: Hans de Goede <[email protected]>
>> ---
>> .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 14 ++++++++++----
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 5 ++++-
>> 2 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index 7ffc4ab..c54e8b4 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -688,11 +688,17 @@ static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy,
>> return ERR_PTR(-EINVAL);
>> }
>>
>> - if (IS_ERR(wdev))
>> - brcmf_err("add iface %s type %d failed: err=%d\n",
>> - name, type, (int)PTR_ERR(wdev));
>> - else
>> + if (IS_ERR(wdev)) {
>> + err = PTR_ERR(wdev);
>> + if (err != -EBUSY)
>> + brcmf_err("add iface %s type %d failed: err=%d\n",
>> + name, type, err);
>> + else
>> + brcmf_dbg(INFO, "add iface %s type %d failed: err=%d\n",
>> + name, type, err);
>> + } else {
>> brcmf_cfg80211_update_proto_addr_mode(wdev);
>> + }
>>
>> return wdev;
>> }
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>> index de19c7c..b5df0a0 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>> @@ -2090,7 +2090,10 @@ static struct wireless_dev *brcmf_p2p_create_p2pdev(struct brcmf_p2p_info *p2p,
>> /* Initialize P2P Discovery in the firmware */
>> err = brcmf_fil_iovar_int_set(pri_ifp, "p2p_disc", 1);
>> if (err < 0) {
>> - brcmf_err("set p2p_disc error\n");
>> + if (err != -EBUSY)
>> + brcmf_err("set p2p_disc error\n");
>> + else
>> + brcmf_dbg(INFO, "set p2p_disc error\n");
>> brcmf_fweh_p2pdev_setup(pri_ifp, false);
>> brcmf_cfg80211_arm_vif_event(p2p->cfg, NULL);
>> goto fail;
>>
On 27-2-2017 22:45, Hans de Goede wrote:
> 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.
Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 +++
> 1 file changed, 3 insertions(+)
Hi,
On 08-03-17 11:15, Arend Van Spriel wrote:
> On 8-3-2017 10:26, Arend Van Spriel wrote:
>> On 8-3-2017 9:24, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 07-03-17 11:03, Arend Van Spriel wrote:
>>>>
>>>>
>>>> On 27-2-2017 22:45, Hans de Goede wrote:
>>>>> The firmware responding with -EBUSY when trying to add an extra
>>>>> virtual-if
>>>>> is a normal thing, do not print an error for this.
>>>>
>>>> This may be something we need to look into. It seems to me the interface
>>>> combinations needs to be fixed so we do not try to provision firmware.
>>>> Can you explain the scenario here?
>>>
>>> I'm not doing anything special just connecting to my isp provided
>>> accesspoint
>>> using NetworkManager.
>>
>> Ok. So it is probably the P2P_DEVICE interface as that is the only
>> interface being created in that scenario. Can you provide a log with
>> brcmfmac driver debug=0x1416.
>
> I guess you are seeing this with 43362. Is that correct?
Maybe, it is with the same card as this bug:
https://bugzilla.kernel.org/show_bug.cgi?id=185661
Regards,
Hans
On 8-3-2017 11:08, Hans de Goede wrote:
> HI,
>
> On 08-03-17 10:57, Kalle Valo wrote:
>> Arend Van Spriel <[email protected]> writes:
>>
>>> On 8-3-2017 9:23, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 07-03-17 10:59, Arend Van Spriel wrote:
>>>>> On 27-2-2017 22:45, 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.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>> -Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
>>>>>> ---
>>>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
>>>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 3 +++
>>>>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>>>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>>>>> index 3e15d64..6d565f1 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 6687812..605f260 100644
>>>>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>>>>> @@ -59,11 +59,14 @@
>>>>>> pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
>>>>>> } while (0)
>>>>>> #endif
>>>>>> +#define brcmf_info(fmt, ...) pr_info("%s: " fmt, __func__,
>>>>>> ##__VA_ARGS__)
>>>>>
>>>>> Prefer using the same style as for brcmf_err, ie. using do {} while
>>>>> (0)
>>>>
>>>> OK, v3 with this fixed coming up.
>>>
>>> I think Kalle prefers the whole series to be resubmitted.
>>>
>>> Kalle,
>>>
>>> Can you confirm (or deny)?
>>
>> Exactly, I want to apply the full series (with the highest version
>> number). Not waste time guessing what patches I should take and what to
>> drop, with the increased risk of guessing wrong.
>
> Ok, so patch 2/4 is still under discussion (and may be so for a while)
> shall I send a new version with that patch dropped ? And then send
> that patch (or a modified version) separately later ?
Yeah. Let's drop that patch for now.
Regards,
Arend
On 10-3-2017 9:08, Hans de Goede wrote:
> Hi,
>
> On 08-03-17 12:16, Hans de Goede wrote:
>> Hi,
>>
>> On 08-03-17 11:15, Arend Van Spriel wrote:
>>> On 8-3-2017 10:26, Arend Van Spriel wrote:
>>>> On 8-3-2017 9:24, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 07-03-17 11:03, Arend Van Spriel wrote:
>>>>>>
>>>>>>
>>>>>> On 27-2-2017 22:45, Hans de Goede wrote:
>>>>>>> The firmware responding with -EBUSY when trying to add an extra
>>>>>>> virtual-if
>>>>>>> is a normal thing, do not print an error for this.
>>>>>>
>>>>>> This may be something we need to look into. It seems to me the
>>>>>> interface
>>>>>> combinations needs to be fixed so we do not try to provision
>>>>>> firmware.
>>>>>> Can you explain the scenario here?
>>>>>
>>>>> I'm not doing anything special just connecting to my isp provided
>>>>> accesspoint
>>>>> using NetworkManager.
>>>>
>>>> Ok. So it is probably the P2P_DEVICE interface as that is the only
>>>> interface being created in that scenario. Can you provide a log with
>>>> brcmfmac driver debug=0x1416.
>>>
>>> I guess you are seeing this with 43362. Is that correct?
>
> Attached is a dmesg log from the brcmfmac driver build with debugging
> enabled and brcmfmac.debug set to 0x1416.
I suspect the P2P_DEVICE interface is assigned the same address as
wlp1s0. Can you apply the change below and create the log again.
Regards,
Arend
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
b/drivers/ne
index de19c7c..43fa9f45 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2158,7 +2158,8 @@ struct wireless_dev *brcmf_p2p_add_vif(struct
wiphy *wiphy
if (brcmf_cfg80211_vif_event_armed(cfg))
return ERR_PTR(-EBUSY);
- brcmf_dbg(INFO, "adding vif \"%s\" (type=%d)\n", name, type);
+ brcmf_dbg(INFO, "adding vif \"%s\" (type=%d, addr=%pM)\n", name,
type,
+ params->macaddr);
switch (type) {
case NL80211_IFTYPE_P2P_CLIENT:
On 27-2-2017 22:45, Hans de Goede wrote:
> The firmware responding with -EBUSY when trying to add an extra virtual-if
> is a normal thing, do not print an error for this.
This may be something we need to look into. It seems to me the interface
combinations needs to be fixed so we do not try to provision firmware.
Can you explain the scenario here?
Regards,
Arend
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 14 ++++++++++----
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 5 ++++-
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 7ffc4ab..c54e8b4 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -688,11 +688,17 @@ static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy,
> return ERR_PTR(-EINVAL);
> }
>
> - if (IS_ERR(wdev))
> - brcmf_err("add iface %s type %d failed: err=%d\n",
> - name, type, (int)PTR_ERR(wdev));
> - else
> + if (IS_ERR(wdev)) {
> + err = PTR_ERR(wdev);
> + if (err != -EBUSY)
> + brcmf_err("add iface %s type %d failed: err=%d\n",
> + name, type, err);
> + else
> + brcmf_dbg(INFO, "add iface %s type %d failed: err=%d\n",
> + name, type, err);
> + } else {
> brcmf_cfg80211_update_proto_addr_mode(wdev);
> + }
>
> return wdev;
> }
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> index de19c7c..b5df0a0 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> @@ -2090,7 +2090,10 @@ static struct wireless_dev *brcmf_p2p_create_p2pdev(struct brcmf_p2p_info *p2p,
> /* Initialize P2P Discovery in the firmware */
> err = brcmf_fil_iovar_int_set(pri_ifp, "p2p_disc", 1);
> if (err < 0) {
> - brcmf_err("set p2p_disc error\n");
> + if (err != -EBUSY)
> + brcmf_err("set p2p_disc error\n");
> + else
> + brcmf_dbg(INFO, "set p2p_disc error\n");
> brcmf_fweh_p2pdev_setup(pri_ifp, false);
> brcmf_cfg80211_arm_vif_event(p2p->cfg, NULL);
> goto fail;
>
Hi,
On 07-03-17 10:59, Arend Van Spriel wrote:
> On 27-2-2017 22:45, 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.
>>
>> Signed-off-by: Hans de Goede <[email protected]>
>> ---
>> Changes in v2:
>> -Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
>> ---
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 3 +++
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>> index 3e15d64..6d565f1 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 6687812..605f260 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>> @@ -59,11 +59,14 @@
>> pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
>> } while (0)
>> #endif
>> +#define brcmf_info(fmt, ...) pr_info("%s: " fmt, __func__, ##__VA_ARGS__)
>
> Prefer using the same style as for brcmf_err, ie. using do {} while (0)
OK, v3 with this fixed coming up.
Regards,
Hans
>
> Regards,
> Arend
>
>> #else
>> __printf(2, 3)
>> void __brcmf_err(const char *func, const char *fmt, ...);
>> #define brcmf_err(fmt, ...) \
>> __brcmf_err(__func__, fmt, ##__VA_ARGS__)
>> +/* For tracing purposes treat info messages as errors */
>> +#define brcmf_info brcmf_err
>> #endif
>>
>> #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
>>
HI,
On 08-03-17 10:57, Kalle Valo wrote:
> Arend Van Spriel <[email protected]> writes:
>
>> On 8-3-2017 9:23, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 07-03-17 10:59, Arend Van Spriel wrote:
>>>> On 27-2-2017 22:45, 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.
>>>>>
>>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>>> ---
>>>>> Changes in v2:
>>>>> -Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
>>>>> ---
>>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
>>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 3 +++
>>>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git
>>>>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>>>> index 3e15d64..6d565f1 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 6687812..605f260 100644
>>>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>>>> @@ -59,11 +59,14 @@
>>>>> pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
>>>>> } while (0)
>>>>> #endif
>>>>> +#define brcmf_info(fmt, ...) pr_info("%s: " fmt, __func__,
>>>>> ##__VA_ARGS__)
>>>>
>>>> Prefer using the same style as for brcmf_err, ie. using do {} while (0)
>>>
>>> OK, v3 with this fixed coming up.
>>
>> I think Kalle prefers the whole series to be resubmitted.
>>
>> Kalle,
>>
>> Can you confirm (or deny)?
>
> Exactly, I want to apply the full series (with the highest version
> number). Not waste time guessing what patches I should take and what to
> drop, with the increased risk of guessing wrong.
Ok, so patch 2/4 is still under discussion (and may be so for a while)
shall I send a new version with that patch dropped ? And then send
that patch (or a modified version) separately later ?
Regards,
Hans
>
Arend Van Spriel <[email protected]> writes:
> On 8-3-2017 9:23, Hans de Goede wrote:
>> Hi,
>>
>> On 07-03-17 10:59, Arend Van Spriel wrote:
>>> On 27-2-2017 22:45, 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.
>>>>
>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>> ---
>>>> Changes in v2:
>>>> -Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
>>>> ---
>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
>>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 3 +++
>>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git
>>>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>>> index 3e15d64..6d565f1 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 6687812..605f260 100644
>>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>>> @@ -59,11 +59,14 @@
>>>> pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
>>>> } while (0)
>>>> #endif
>>>> +#define brcmf_info(fmt, ...) pr_info("%s: " fmt, __func__,
>>>> ##__VA_ARGS__)
>>>
>>> Prefer using the same style as for brcmf_err, ie. using do {} while (0)
>>
>> OK, v3 with this fixed coming up.
>
> I think Kalle prefers the whole series to be resubmitted.
>
> Kalle,
>
> Can you confirm (or deny)?
Exactly, I want to apply the full series (with the highest version
number). Not waste time guessing what patches I should take and what to
drop, with the increased risk of guessing wrong.
--
Kalle Valo
Hi,
On 10-03-17 10:35, Arend Van Spriel wrote:
> On 10-3-2017 9:08, Hans de Goede wrote:
>> Hi,
>>
>> On 08-03-17 12:16, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 08-03-17 11:15, Arend Van Spriel wrote:
>>>> On 8-3-2017 10:26, Arend Van Spriel wrote:
>>>>> On 8-3-2017 9:24, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 07-03-17 11:03, Arend Van Spriel wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 27-2-2017 22:45, Hans de Goede wrote:
>>>>>>>> The firmware responding with -EBUSY when trying to add an extra
>>>>>>>> virtual-if
>>>>>>>> is a normal thing, do not print an error for this.
>>>>>>>
>>>>>>> This may be something we need to look into. It seems to me the
>>>>>>> interface
>>>>>>> combinations needs to be fixed so we do not try to provision
>>>>>>> firmware.
>>>>>>> Can you explain the scenario here?
>>>>>>
>>>>>> I'm not doing anything special just connecting to my isp provided
>>>>>> accesspoint
>>>>>> using NetworkManager.
>>>>>
>>>>> Ok. So it is probably the P2P_DEVICE interface as that is the only
>>>>> interface being created in that scenario. Can you provide a log with
>>>>> brcmfmac driver debug=0x1416.
>>>>
>>>> I guess you are seeing this with 43362. Is that correct?
>>
>> Attached is a dmesg log from the brcmfmac driver build with debugging
>> enabled and brcmfmac.debug set to 0x1416.
>
> I suspect the P2P_DEVICE interface is assigned the same address as
> wlp1s0. Can you apply the change below and create the log again.
>
> Regards,
> Arend
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> b/drivers/ne
> index de19c7c..43fa9f45 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> @@ -2158,7 +2158,8 @@ struct wireless_dev *brcmf_p2p_add_vif(struct
> wiphy *wiphy
> if (brcmf_cfg80211_vif_event_armed(cfg))
> return ERR_PTR(-EBUSY);
>
> - brcmf_dbg(INFO, "adding vif \"%s\" (type=%d)\n", name, type);
> + brcmf_dbg(INFO, "adding vif \"%s\" (type=%d, addr=%pM)\n", name,
> type,
> + params->macaddr);
>
> switch (type) {
> case NL80211_IFTYPE_P2P_CLIENT:
>
Ok, new log attached.
Regards,
Hans
On 27-2-2017 22:45, Hans de Goede wrote:
> The country code gets set to "00" by default at boot, ignore this
> rather then logging an error about it.
Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++++
> 1 file changed, 4 insertions(+)
On 8-3-2017 10:26, Arend Van Spriel wrote:
> On 8-3-2017 9:24, Hans de Goede wrote:
>> Hi,
>>
>> On 07-03-17 11:03, Arend Van Spriel wrote:
>>>
>>>
>>> On 27-2-2017 22:45, Hans de Goede wrote:
>>>> The firmware responding with -EBUSY when trying to add an extra
>>>> virtual-if
>>>> is a normal thing, do not print an error for this.
>>>
>>> This may be something we need to look into. It seems to me the interface
>>> combinations needs to be fixed so we do not try to provision firmware.
>>> Can you explain the scenario here?
>>
>> I'm not doing anything special just connecting to my isp provided
>> accesspoint
>> using NetworkManager.
>
> Ok. So it is probably the P2P_DEVICE interface as that is the only
> interface being created in that scenario. Can you provide a log with
> brcmfmac driver debug=0x1416.
I guess you are seeing this with 43362. Is that correct?
Regards,
Arend
Hans de Goede <[email protected]> writes:
> 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
Oh, missed that there was v2.
--
Kalle Valo
On 27-2-2017 22:45, 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.
>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> Changes in v2:
> -Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> index 3e15d64..6d565f1 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 6687812..605f260 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
> @@ -59,11 +59,14 @@
> pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
> } while (0)
> #endif
> +#define brcmf_info(fmt, ...) pr_info("%s: " fmt, __func__, ##__VA_ARGS__)
Prefer using the same style as for brcmf_err, ie. using do {} while (0)
Regards,
Arend
> #else
> __printf(2, 3)
> void __brcmf_err(const char *func, const char *fmt, ...);
> #define brcmf_err(fmt, ...) \
> __brcmf_err(__func__, fmt, ##__VA_ARGS__)
> +/* For tracing purposes treat info messages as errors */
> +#define brcmf_info brcmf_err
> #endif
>
> #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
>
On 8-3-2017 9:23, Hans de Goede wrote:
> Hi,
>
> On 07-03-17 10:59, Arend Van Spriel wrote:
>> On 27-2-2017 22:45, 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.
>>>
>>> Signed-off-by: Hans de Goede <[email protected]>
>>> ---
>>> Changes in v2:
>>> -Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
>>> ---
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 3 +++
>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
>>> index 3e15d64..6d565f1 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 6687812..605f260 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
>>> @@ -59,11 +59,14 @@
>>> pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
>>> } while (0)
>>> #endif
>>> +#define brcmf_info(fmt, ...) pr_info("%s: " fmt, __func__,
>>> ##__VA_ARGS__)
>>
>> Prefer using the same style as for brcmf_err, ie. using do {} while (0)
>
> OK, v3 with this fixed coming up.
I think Kalle prefers the whole series to be resubmitted.
Kalle,
Can you confirm (or deny)?
Regards,
Arend
> Regards,
>
> Hans
>
>
>>
>> Regards,
>> Arend
>>
>>> #else
>>> __printf(2, 3)
>>> void __brcmf_err(const char *func, const char *fmt, ...);
>>> #define brcmf_err(fmt, ...) \
>>> __brcmf_err(__func__, fmt, ##__VA_ARGS__)
>>> +/* For tracing purposes treat info messages as errors */
>>> +#define brcmf_info brcmf_err
>>> #endif
>>>
>>> #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
>>>
Hans de Goede <[email protected]> writes:
>>> I think Kalle prefers the whole series to be resubmitted.
>>>
>>> Kalle,
>>>
>>> Can you confirm (or deny)?
>>
>> Exactly, I want to apply the full series (with the highest version
>> number). Not waste time guessing what patches I should take and what to
>> drop, with the increased risk of guessing wrong.
>
> Ok, so patch 2/4 is still under discussion (and may be so for a while)
> shall I send a new version with that patch dropped ? And then send
> that patch (or a modified version) separately later ?
Yeah, for me that would be the easiest. Thanks.
--
Kalle Valo
Hi,
On 08-03-17 12:16, Hans de Goede wrote:
> Hi,
>
> On 08-03-17 11:15, Arend Van Spriel wrote:
>> On 8-3-2017 10:26, Arend Van Spriel wrote:
>>> On 8-3-2017 9:24, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 07-03-17 11:03, Arend Van Spriel wrote:
>>>>>
>>>>>
>>>>> On 27-2-2017 22:45, Hans de Goede wrote:
>>>>>> The firmware responding with -EBUSY when trying to add an extra
>>>>>> virtual-if
>>>>>> is a normal thing, do not print an error for this.
>>>>>
>>>>> This may be something we need to look into. It seems to me the interface
>>>>> combinations needs to be fixed so we do not try to provision firmware.
>>>>> Can you explain the scenario here?
>>>>
>>>> I'm not doing anything special just connecting to my isp provided
>>>> accesspoint
>>>> using NetworkManager.
>>>
>>> Ok. So it is probably the P2P_DEVICE interface as that is the only
>>> interface being created in that scenario. Can you provide a log with
>>> brcmfmac driver debug=0x1416.
>>
>> I guess you are seeing this with 43362. Is that correct?
Attached is a dmesg log from the brcmfmac driver build with debugging
enabled and brcmfmac.debug set to 0x1416.
Regards,
Hans
On 8-3-2017 9:24, Hans de Goede wrote:
> Hi,
>
> On 07-03-17 11:03, Arend Van Spriel wrote:
>>
>>
>> On 27-2-2017 22:45, Hans de Goede wrote:
>>> The firmware responding with -EBUSY when trying to add an extra
>>> virtual-if
>>> is a normal thing, do not print an error for this.
>>
>> This may be something we need to look into. It seems to me the interface
>> combinations needs to be fixed so we do not try to provision firmware.
>> Can you explain the scenario here?
>
> I'm not doing anything special just connecting to my isp provided
> accesspoint
> using NetworkManager.
Ok. So it is probably the P2P_DEVICE interface as that is the only
interface being created in that scenario. Can you provide a log with
brcmfmac driver debug=0x1416.
Regards,
Arend
> Regards,
>
> Hans
>
>
>>
>> Regards,
>> Arend
>>
>>> Signed-off-by: Hans de Goede <[email protected]>
>>> ---
>>> .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 14
>>> ++++++++++----
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 5 ++++-
>>> 2 files changed, 14 insertions(+), 5 deletions(-)
>>>
>>> diff --git
>>> a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> index 7ffc4ab..c54e8b4 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> @@ -688,11 +688,17 @@ static struct wireless_dev
>>> *brcmf_cfg80211_add_iface(struct wiphy *wiphy,
>>> return ERR_PTR(-EINVAL);
>>> }
>>>
>>> - if (IS_ERR(wdev))
>>> - brcmf_err("add iface %s type %d failed: err=%d\n",
>>> - name, type, (int)PTR_ERR(wdev));
>>> - else
>>> + if (IS_ERR(wdev)) {
>>> + err = PTR_ERR(wdev);
>>> + if (err != -EBUSY)
>>> + brcmf_err("add iface %s type %d failed: err=%d\n",
>>> + name, type, err);
>>> + else
>>> + brcmf_dbg(INFO, "add iface %s type %d failed: err=%d\n",
>>> + name, type, err);
>>> + } else {
>>> brcmf_cfg80211_update_proto_addr_mode(wdev);
>>> + }
>>>
>>> return wdev;
>>> }
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>> index de19c7c..b5df0a0 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>> @@ -2090,7 +2090,10 @@ static struct wireless_dev
>>> *brcmf_p2p_create_p2pdev(struct brcmf_p2p_info *p2p,
>>> /* Initialize P2P Discovery in the firmware */
>>> err = brcmf_fil_iovar_int_set(pri_ifp, "p2p_disc", 1);
>>> if (err < 0) {
>>> - brcmf_err("set p2p_disc error\n");
>>> + if (err != -EBUSY)
>>> + brcmf_err("set p2p_disc error\n");
>>> + else
>>> + brcmf_dbg(INFO, "set p2p_disc error\n");
>>> brcmf_fweh_p2pdev_setup(pri_ifp, false);
>>> brcmf_cfg80211_arm_vif_event(p2p->cfg, NULL);
>>> goto fail;
>>>