Re: [PATCH V2 2/8] brcmfmac: set F2 watermark to 256 for 4373
On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote: From: Wright Feng We got SDIO_CRC_ERROR with 4373 on SDR104 when doing bi-directional throughput test. Enable watermark to 256 to guarantee the operation stability. Please see my comment on patch 6/8 regarding introducing queuing latency with this change. Regards, Arend Reviewed-by: Arend van Spriel Signed-off-by: Wright Feng Signed-off-by: Chi-Hsien Lin --- .../wireless/broadcom/brcm80211/brcmfmac/sdio.c| 26 -- 1 file changed, 24 insertions(+), 2 deletions(-)
Re: [PATCH V2 1/8] brcmfmac: add 4354 raw pcie device id
On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote: From: Winnie Chang Add the raw 4354 PCIe device ID for unprogrammed Cypress boards. Already have my review tag so this is fine as is. Reviewed-by: Arend Van Spriel Signed-off-by: Winnie Chang Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 1 + drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 1 + 2 files changed, 2 insertions(+)
Re: [PATCH V2 6/8] brcmfmac: update 43012 F2 watermark setting to fix DMA Error during UDP RX Traffic
On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote: From: Naveen Gupta The number of words that the read FIFO has to contain except the end of frame before sends data back to the host. Max watermark = (512B - 2* (BurstLength))/4 = (512 - 128)/4 = 384/4 = 0x60 so if burst length (i.e. BurstLength = 64) is increased, watermark has to be reduced. This is the optimal setting for this chip. Reviewed-by: Arend van Spriel Signed-off-by: Naveen Gupta Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 7707b0169c21..e1708e297d07 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -52,6 +52,7 @@ /* watermark expressed in number of words */ #define DEFAULT_F2_WATERMARK0x8 #define CY_4373_F2_WATERMARK0x40 +#define CY_43012_F2_WATERMARK0x60 So basically you increase queuing in firmware rx path. How does this affect TCP latency. The DMA error obviously needs fixing, but why go from a watermark of 32 bytes to the maximum of 384 bytes. Same question for 4373. Regards, Arend
Re: [PATCH V2 3/8] brcmfmac: set SDIO F1 MesBusyCtrl for CYW4373
On 11/12/2018 8:29 AM, Chi-Hsien Lin wrote: From: Madhan Mohan R Along with F2 watermark (existing) configuration, F1 MesBusyCtrl should be enabled & configured to avoid overflow errors. I am a bit confused. Why is it necessary to program the watermark in both SBSDIO_WATERMARK (0x10008) and SDSDIO_FUNC1_MESBUSYCTRL (0x1001D). Looks suspicious to me. Regards, Arend
Re: [PATCH v2] brcmfmac: support STA info struct v7
On 11/9/2018 5:38 PM, Dan Haab wrote: The newest firmwares provide STA info using v7 of the struct. As v7 isn't backward compatible, a union is needed. Even though brcmfmac does not use any of the new info it's important to provide the proper struct buffer. Without this change new firmwares will fallback to the very limited v3 instead of something in between such as v4. I would expect a changelog, but looking at the patch is addresses my comments nicely so Reviewed-by: Arend van Spriel Signed-off-by: Dan Haab Reviewed-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/fwil_types.h | 40 ++ 1 file changed, 33 insertions(+), 7 deletions(-)
Re: [PATCH 3/5] brcmfmac: p2p cert 6.1.9-support GOUT handling p2p presence request
+ Jouni On 11/8/2018 4:48 AM, Chi-Hsien Lin wrote: From: Madhan Mohan R Send p2p presence response from the p2p interface address instead of the p2p device address. This is needed for p2p cert 6.1.9 to pass. I am not really into the P2P spec, but if this is indeed a requirement (@Jouni: can you confirm?) I would expect wpa_supplicant to send the action frame over the correct netdevice (although there is no netdev for P2P_DEVICE interface so expect primary interface will be used). So instead of looking at the action frame subtype it seems to be more appropriate to determine and pass the appropriate vif in brcmf_p2p_send_action_frame(). Regards, Arend Signed-off-by: Madhan Mohan R Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-)
Re: [PATCH 5/5] brcmfmac: add vendor ie for association responses
On 11/8/2018 4:48 AM, Chi-Hsien Lin wrote: From: Ryohei Kondo Miracast Certification clause 6.1.2 may fail if there is no WFD IE in p2p assoc response. This change allows WFD IE to be added to p2p assoc response. Related WFA certification: 6.1.2 P-SnUT operating as a Group Owner accepts a WFD Session with a Reference Source Reviewed-by: Arend van Spriel Signed-off-by: Ryohei Kondo Signed-off-by: Chi-Hsien Lin --- .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c| 14 ++ .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h| 4 2 files changed, 18 insertions(+)
Re: [PATCH 4/5] brcmfmac: only generate random p2p address when needed
+ Hans On 11/8/2018 4:48 AM, Chi-Hsien Lin wrote: P2p spec mentioned that the p2p device address should be the globally administered address with locally administered bit set. Therefore, follow this guideline by default. When the primary interface is set to a locally administered address, the locally administered bit cannot be set again. Generate a random locally administered address for this case. We discussed this on the linux-wireless list a while ago. I have no problem with the approach in this patch. Reviewed-by: Arend van Spriel Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-)
Re: [PATCH 1/5] brcmfmac: set apsta to 1 when AP start on primary interface.
On 11/8/2018 4:48 AM, Chi-Hsien Lin wrote: From: Wright Feng APSTA can work on two band concurrently with using VSDB(Virtual Simultaneous Dual-Band) or RSDB(Real Simultaneous Dual-Band) features. In this case, we have to keep apsta is 1 in firmware side. However, if we start wpa_supplicant on wlan0 and then start hostapd on wlan 1, the apsta will be set to 0, and we will see data stall on wlan0(station) So that, we only set apsta to 1 when AP start on primary interface. The description makes my head spin. From reading the commit message I think the code should add a !VSDB check instead of dropping the !RSDB check. Would you agree? Regards, Arend Signed-off-by: Wright Feng Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Re: [PATCH 03/11] brcmfmac: set SDIO F1 MesBusyCtrl for CYW4373
On 11/9/2018 8:34 AM, Chi-Hsien Lin wrote: On 11/08/2018 7:53, Arend van Spriel wrote: On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: From: Madhan Mohan R Along with F2 watermark (existing) configuration, F1 MesBusyCtrl should be enabled & configured to avoid overflow errors. Reviewed-by: Arend van Spriel Signed-off-by: Madhan Mohan R Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 3 +++ drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 541d54661c9e..34a838fcc319 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -51,6 +51,7 @@ #define DEFAULT_F2_WATERMARK0x8 #define CY_4373_F2_WATERMARK0x40 +#define CY_4373_F1_MESBUSYCTRL (CY_4373_F2_WATERMARK | SBSDIO_MESBUSYCTRL_ENAB) I don't see much value for this define. It is use once below so just or it there. That way you can "directly" see what is written to the register. #ifdef DEBUG @@ -4118,6 +4119,8 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err, devctl |= SBSDIO_DEVCTL_F2WM_ENAB; brcmf_sdiod_writeb(sdiod, SBSDIO_DEVICE_CTL, devctl, &err); +brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_MESBUSYCTRL, + CY_4373_F1_MESBUSYCTRL, &err); just use 'CY_4373_F2_WATERMARK | SBSDIO_MESBUSYCTRL_ENAB' here. No braces needed. Thanks for the input. The biggest difference is to prevent a 4-line function call like below so it's more readable. I'll make this change in V2. Please let me know if below looks too messy then I'll move back to V1: brcmf_sdiod_writeb(sdiod, CY_4373_F2_WATERMARK | SBSDIO_MESBUSYCTRL_ENAB, CY_4373_F2_WATERMARK | SBSDIO_MESBUSYCTRL_ENAB, &err); The second argument should just be SBSDIO_FUNC1_MESBUSYCTRL so that will make it a bit less messy. Look okay to me. Regards, Arend
Re: [PATCH 10/11] brcmfmac: disable command decode in sdio_aos for 4354
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: From: Double Lo Transaction between AOS and SDIOD is not protected, and if cmd 52 received in AOS and in the middle of response state changed from AOS to SDIOD, response is corrupted and it causes to SDIO Host controller to hang. See comment in PATCH 09/11 Signed-off-by: Double Lo Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Re: [PATCH 11/11] brcmfmac: disable command decode in sdio_aos for 4373
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: From: Madhan Mohan R By disabling command decode, sdiod_aos module supports the detection of sdio command line toggle only and generates a wakeup request to PMU and to sdiod core. It does not decode any sdio command and generates no response to any command. See comment in PATCH 09/11 Signed-off-by: Madhan Mohan R Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + 1 file changed, 1 insertion(+)
Re: [PATCH 08/11] brcmfmac: 4373 save-restore support
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote:
Use sr_eng_en bit to check 4373 sr support.
Reviewed-by: Arend van Spriel
Signed-off-by: Chi-Hsien Lin
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index a8d3b96b727f..dfc2af943bff 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -1332,7 +1332,7 @@ bool brcmf_chip_set_active(struct brcmf_chip *pub, u32
rstvec)
bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
{
- u32 base, addr, reg, pmu_cc3_mask = ~0;
+ u32 base, addr, reg, sr_eng_en, pmu_cc3_mask = ~0;
struct brcmf_chip_priv *chip;
struct brcmf_core *pmu = brcmf_chip_get_pmu(pub);
@@ -1365,6 +1365,12 @@ bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
addr = CORE_CC_REG(base, sr_control1);
reg = chip->ops->read32(chip->ctx, addr);
return reg != 0;
+ case CY_CC_4373_CHIP_ID:
+ /* explicitly check SR engine enable bit */
+ sr_eng_en = BIT(0);
+ addr = CORE_CC_REG(base, sr_control0);
+ reg = chip->ops->read32(chip->ctx, addr);
+ return (reg & sr_eng_en) != 0;
Drop the variable and just do (reg & BIT(0)).
Re: [PATCH 09/11] brcmfmac: disable command decode in sdio_aos for 43012/4339/4345
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote:
From: Wright Feng
Transaction between AOS and SDIOD is not protected, and if cmd 52
received in AOS and in the middle of response state changed from AOS to
SDIOD, response is corrupted and it causes to SDIO Host controller to
hang.
I think it would be good to elaborate on what AOS stands for. It is a
part of the SDIOD core that becomes active when the rest of SDIOD is
sleeping to keep SDIO bus alive responding to reduced set of commands.
I would actually suggest to collapse patch 10 and 11 in this one.
Reviewed-by: Arend van Spriel
Signed-off-by: Wright Feng
Signed-off-by: Chi-Hsien Lin
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index e7ee144dc5dd..d507d8f15e48 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3363,15 +3363,21 @@ static void brcmf_sdio_sr_init(struct brcmf_sdio *bus)
if (bus->ci->chip == CY_CC_43012_CHIP_ID) {
wakeupctrl = SBSDIO_FUNC1_WCTRL_ALPWAIT_SHIFT;
- cardcap = SDIO_CCCR_BRCM_CARDCAP_CMD_NODEC;
chipclkcsr = SBSDIO_HT_AVAIL_REQ;
} else {
wakeupctrl = SBSDIO_FUNC1_WCTRL_HTWAIT_SHIFT;
- cardcap = (SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT |
- SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT);
chipclkcsr = SBSDIO_FORCE_HT;
}
+ if (bus->ci->chip == CY_CC_43012_CHIP_ID ||
+ bus->ci->chip == BRCM_CC_4339_CHIP_ID ||
+ bus->ci->chip == BRCM_CC_4345_CHIP_ID) {
use brcmf_sdio_aos_no_decode(bus) helper here.
+ cardcap = SDIO_CCCR_BRCM_CARDCAP_CMD_NODEC;
+ } else {
+ cardcap = (SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT |
+ SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT);
+ }
+
val = brcmf_sdiod_readb(bus->sdiodev, SBSDIO_FUNC1_WAKEUPCTRL, &err);
if (err) {
brcmf_err("error reading SBSDIO_FUNC1_WAKEUPCTRL\n");
Re: [PATCH 07/11] brcmfmac: update 43012 F2 watermark setting to fix DMA Error during UDP RX Traffic
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: From: Naveen Gupta The number of words that the read FIFO has to contain except the end of frame before sends data back to the host. Max watermark = (512B - 2* (BurstLength))/4 = (512 - 128)/4 = 384/4 = 0x60 so if burst length (i.e. BurstLength = 64) is increased, watermark has to be reduced. This is the optimal setting for this chip. Nice formula. So could the BurstLength be retrieved from firmware so the driver can determine and update the F2 watermark in brcmf_sdio_bus_preinit(). Reviewed-by: Arend van Spriel Signed-off-by: Naveen Gupta Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 1 file changed, 12 insertions(+)
Re: [PATCH 05/11] brcmfmac: allow GCI core enumuration
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: GCI core is needed for ULP operation. Allow GCI core enumuration with below changes: - Allow GCI to be added to core list even when it doesn't have a wrapper. - Allow 8K address space size. - Don't overwrite the address value when an additional size descriptor is in place. Reviewed-by: Arend van Spriel Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-)
Re: [PATCH 06/11] brcmfmac: saverestore support changes for 43012
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote:
From: Praveen Babu C
Add saverestore register settings for 43012.
I would collapse this commit with PATCH 04/11.
Reviewed-by: Arend van Spriel
Signed-off-by: Praveen Babu C
Signed-off-by: Chi-Hsien Lin
---
.../wireless/broadcom/brcm80211/brcmfmac/chip.c| 5 +
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c| 22 +-
2 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 299f59f58d8c..a32eb5f868b5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3354,16 +3354,29 @@ static void brcmf_sdio_sr_init(struct brcmf_sdio *bus)
{
int err = 0;
u8 val;
+ u8 wakeupctrl;
+ u8 cardcap;
+ u8 chipclkcsr;
brcmf_dbg(TRACE, "Enter\n");
+ if (bus->ci->chip == CY_CC_43012_CHIP_ID) {
Use brcmf_chip_is_ulp() here as well (see PATCH 04/11).
hmmm..wait. This is actually disabling the command decode for 43012,
which is claimed to be done in PATCH 09/11. So maybe another helper
would be more appropriate here, eg. brcmf_sdio_aos_no_decode(bus).
+ wakeupctrl = SBSDIO_FUNC1_WCTRL_ALPWAIT_SHIFT;
+ cardcap = SDIO_CCCR_BRCM_CARDCAP_CMD_NODEC;
+ chipclkcsr = SBSDIO_HT_AVAIL_REQ;
+ } else {
+ wakeupctrl = SBSDIO_FUNC1_WCTRL_HTWAIT_SHIFT;
+ cardcap = (SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT |
+ SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT);
+ chipclkcsr = SBSDIO_FORCE_HT;
+ }
+
Re: [PATCH 04/11] brcmfmac: add support for CYW43012 SDIO chipset
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote:
CYW43012 is a 1x1 802.11a/b/g/n Dual-Band HT20, 256-QAM/Turbo QAM. It
is an Ultra Low Power WLAN+BT combo chip.
comments below
Reviewed-by: Arend van Spriel
Signed-off-by: Chi-Hsien Lin
---
.../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 +
.../wireless/broadcom/brcm80211/brcmfmac/chip.c| 9 -
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c| 40 ++
.../broadcom/brcm80211/include/brcm_hw_ids.h | 1 +
include/linux/mmc/sdio_ids.h | 1 +
5 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 34a838fcc319..299f59f58d8c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -677,6 +679,15 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
/* 1st KSO write goes to AOS wake up core if device is asleep */
brcmf_sdiod_writeb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, wr_val, &err);
+ /* In case of 43012 chip, the chip could go down immediately after
+* KSO bit is cleared. So the further reads of KSO register could
+* fail. Thereby just bailing out immediately after clearing KSO
+* bit, to avoid polling of KSO bit.
+*/
+ if (!on && bus->ci->chip == CY_CC_43012_CHIP_ID) {
+ return err;
+ }
kernel coding style does not require curly braces here.
if (on) {
/* device WAKEUP through KSO:
* write bit 0 & read back until
@@ -2436,9 +2447,20 @@ static void brcmf_sdio_bus_stop(struct device *dev)
/* Force backplane clocks to assure F2 interrupt propagates */
saveclk = brcmf_sdiod_readb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
&err);
- if (!err)
- brcmf_sdiod_writeb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
- (saveclk | SBSDIO_FORCE_HT), &err);
+ if (!err) {
+ if (bus->ci->chip == CY_CC_43012_CHIP_ID) {
+ brcmf_sdiod_writeb(sdiodev,
+ SBSDIO_FUNC1_CHIPCLKCSR,
+ (saveclk |
+ SBSDIO_HT_AVAIL_REQ),
+ &err);
+ } else {
+ brcmf_sdiod_writeb(sdiodev,
+ SBSDIO_FUNC1_CHIPCLKCSR,
+ (saveclk | SBSDIO_FORCE_HT),
+ &err);
+ }
+ }
I prefer we avoid checks for chip id as much as possible. Maybe have
chip module provide helper function, ie.
if (!err) {
bpreq = saveclk;
bpreq |= brcmf_chip_is_ulp(bus->ci) ?
SBSDIO_HT_AVAIL_REQ : SBSDIO_FORCE_HT;
brcmf_sdio_writeb(sdiodev,
SBSDIO_FUNC1_CHIPCLKCSR,
bpreq, &err);
}
Re: [PATCH 01/11] brcmfmac: add 4354 raw pcie device id
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: From: Winnie Chang Add the raw 4354 PCIe device ID. What is the motivation for adding this. I can do an educated guess, but I would like to see it in the commit message. Why only for 4354? Regards, Arend Signed-off-by: Winnie Chang Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 1 + drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 1 + 2 files changed, 2 insertions(+)
Re: [PATCH 03/11] brcmfmac: set SDIO F1 MesBusyCtrl for CYW4373
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: From: Madhan Mohan R Along with F2 watermark (existing) configuration, F1 MesBusyCtrl should be enabled & configured to avoid overflow errors. Reviewed-by: Arend van Spriel Signed-off-by: Madhan Mohan R Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 3 +++ drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 541d54661c9e..34a838fcc319 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -51,6 +51,7 @@ #define DEFAULT_F2_WATERMARK0x8 #define CY_4373_F2_WATERMARK0x40 +#define CY_4373_F1_MESBUSYCTRL (CY_4373_F2_WATERMARK | SBSDIO_MESBUSYCTRL_ENAB) I don't see much value for this define. It is use once below so just or it there. That way you can "directly" see what is written to the register. #ifdef DEBUG @@ -4118,6 +4119,8 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err, devctl |= SBSDIO_DEVCTL_F2WM_ENAB; brcmf_sdiod_writeb(sdiod, SBSDIO_DEVICE_CTL, devctl, &err); + brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_MESBUSYCTRL, + CY_4373_F1_MESBUSYCTRL, &err); just use 'CY_4373_F2_WATERMARK | SBSDIO_MESBUSYCTRL_ENAB' here. No braces needed.
Re: [PATCH 02/11] brcmfmac: set F2 watermark to 256 for 4373
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: From: Wright Feng We got SDIO_CRC_ERROR with 4373 on SDR104 when doing bi-directional throughput test. Enable watermark to 256 to guarantee the operation stability. Maybe it would be better to clarify the unit of the watermark. Here you use bytes, but in the register it needs number of words (word being 4 bytes). So is this really only applicable to 4373? I have had question about SDIO crc errors before for other chips. Other than that Reviewed-by: Arend van Spriel Signed-off-by: Wright Feng Signed-off-by: Chi-Hsien Lin --- .../wireless/broadcom/brcm80211/brcmfmac/sdio.c| 25 -- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index b2e1ab5adb64..541d54661c9e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -49,6 +49,9 @@ #define DCMD_RESP_TIMEOUT msecs_to_jiffies(2500) #define CTL_DONE_TIMEOUT msecs_to_jiffies(2500) Mention the unit so: +/* watermark expressed in number of words */ +#define DEFAULT_F2_WATERMARK0x8 +#define CY_4373_F2_WATERMARK0x40 or +#define DEFAULT_F2_WATERMARK (32 >> 2) +@define CY_4373_F2_WATERMARK (256 >> 2)
Re: [PATCH] brcmfmac: support STA info struct v7
On 10/11/2018 10:19 PM, Dan Haab wrote:
The newest firmwares provide STA info using v7 of the struct. As v7
isn't backward compatible, a union is needed.
Even though brcmfmac does not use any of the new info it's important to
provide the proper struct buffer. Without this change new firmwares will
fallback to the very limited v3 instead of something in between such as
v4.
Signed-off-by: Dan Haab
---
.../broadcom/brcm80211/brcmfmac/fwil_types.h | 39 ++
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index d5bb81e..189d576 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -176,6 +176,8 @@
#define BRCMF_VHT_CAP_MCS_MAP_NSS_MAX 8
+#define BRCMF_HE_CAP_MCS_MAP_NSS_MAX 8
+
/* MAX_CHUNK_LEN is the maximum length for data passing to firmware in each
* ioctl. It is relatively small because firmware has small maximum size input
* playload restriction for ioctls.
@@ -601,13 +603,36 @@ struct brcmf_sta_info_le {
__le32 rx_pkts_retried;/* # rx with retry bit set */
__le32 tx_rate_fallback; /* lowest fallback TX rate */
- /* Fields valid for ver >= 5 */
- struct {
- __le32 count; /* # rates in
this set */
- u8 rates[BRCMF_MAXRATES_IN_SET];/* rates in
500kbps units w/hi bit set if basic */
- u8 mcs[BRCMF_MCSSET_LEN]; /* supported
mcs index bit map */
- __le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX]; /* supported
mcs index bit map per nss */
- } rateset_adv;
+ union {
+ struct {
+ struct {
+ __le32 count;
/* # rates in this set */
+ u8 rates[BRCMF_MAXRATES_IN_SET];
/* rates in 500kbps units w/hi bit set if basic */
+ u8 mcs[BRCMF_MCSSET_LEN];
/* supported mcs index bit map */
+ __le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX];
/* supported mcs index bit map per nss */
+ } rateset_adv;
+ } v5;
+
+ struct {
+ __le32 rx_dur_total;/* total user RX duration
(estimated) */
+ __le16 chanspec;/** chanspec this sta is on */
+ __le16 pad;
Here we add explicity padding field.
+ struct {
+ __le16 version;
/* version */
+ __le16 len;
/* length */
+ __le32 count;
/* # rates in this set */
+ u8 rates[BRCMF_MAXRATES_IN_SET];
/* rates in 500kbps units w/hi bit set if basic */
+ u8 mcs[BRCMF_MCSSET_LEN];
/* supported mcs index bit map */
+ __le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX];
/* supported mcs index bit map per nss */
+ __le16 he_mcs[BRCMF_HE_CAP_MCS_MAP_NSS_MAX];
/* supported he mcs index bit map per nss */
+ } rateset_adv; /* rateset along with mcs index
bitmap */
+ __le16 wpauth; /* authentication type */
+ u8 algo;/* crypto algorithm */
So I prefer to have padding explicit here as well, ie. a u8 field.
+ __le32 tx_rspec;/* Rate of last successful tx
frame */
+ __le32 rx_rspec;/* Rate of last successful rx
frame */
+ __le32 wnm_cap; /* wnm capabilities */
+ } v7;
+ };
};
Regards,
Arend
Re: [PATCH] brcmfmac: support STA info struct v7
On 11/7/2018 11:02 AM, Kalle Valo wrote: Rafał Miłecki writes: On Thu, 11 Oct 2018 at 22:21, Dan Haab wrote: The newest firmwares provide STA info using v7 of the struct. As v7 isn't backward compatible, a union is needed. Even though brcmfmac does not use any of the new info it's important to provide the proper struct buffer. Without this change new firmwares will fallback to the very limited v3 instead of something in between such as v4. Signed-off-by: Dan Haab It's too bad Broadcom's existing struct has been changed instead of just being extended. The patch looks good to me though. I just wanted to share my opinion / ping due to patch being marked as "Deferred". Reviewed-by: Rafał Miłecki Good that you brought this up, I wasn't sure what to do with it so I marked as Deferred. Arend, please let me know what I should do. Currently there is no issue as there is currently no image in linux-firmware relying on v7 structure. However, it is not unlikely that people are using firmware from other sources. As said earlier I am fine with this change although v7 structure already was extended. I have a small remark on the patch, which I will send out later. Regards, Arend
Re: [PATCH v2 1/3] lib: cordic: Move cordic macros and defines to header file
On 11/6/2018 8:14 AM, Priit Laes wrote: On Mon, Nov 05, 2018 at 11:02:35PM +0100, Arend van Spriel wrote: On 11/5/2018 8:37 PM, Priit Laes wrote: Also append CORDIC_ prefix to nonprefixed macros. Bit annoying that you made me look at LKML for this patch. Can you just post the entire series to linux-wireless so Kalle can take it all through his tree. Also the commit message is a bit thin. Please explain why these are moved to the header file. Sorry, I messed up again while trying to automate the send-email with get_maintainers script (dry-run argument did not work .gitconfig :S) No worries. It was only a bit annoying ;-) Regards, Arend
Re: [PATCH v2 3/3] b43: Use cordic algorithm from kernel library
On 11/5/2018 10:39 PM, Michael Büsch wrote: On Mon, 5 Nov 2018 21:37:18 +0200 Priit Laes wrote: Kernel library has a common cordic algorithm which is identical to internally implementatd one, so use it and drop the duplicate implementation. Signed-off-by: Priit Laes This looks nice. But what is the testing status of this? Has this been tested on actual b43-LP hardware? Is the replacement algorithm exactly the same, or are there slight differences (e.g. in corner cases)? Hi Michael, I recall doing a comparison between the algorithms and thought I put that in the original commit message. However, it is not there. It is not exactly the same as in b43 so there are difference for certain angles, most results are the same however. This implementation is slightly more accurate on the full scale.
Re: [PATCH v2 1/3] lib: cordic: Move cordic macros and defines to header file
On 11/5/2018 8:37 PM, Priit Laes wrote: Also append CORDIC_ prefix to nonprefixed macros. Bit annoying that you made me look at LKML for this patch. Can you just post the entire series to linux-wireless so Kalle can take it all through his tree. Also the commit message is a bit thin. Please explain why these are moved to the header file. Reviewed-by: Arend van Spriel Signed-off-by: Priit Laes --- include/linux/cordic.h | 9 + lib/cordic.c | 23 +++ 2 files changed, 16 insertions(+), 16 deletions(-)
Re: [PATCH 6/6] brcmfmac: Cleanup brcmf_fw_request_done()
On 10/9/2018 2:47 PM, Hans de Goede wrote: The "cur" variable is now only used for a debug print and we already print the same info from brcmf_fw_complete_request(), so the debug print does not provide any extra info and we can remove it. I guess this could have been collapsed in the first patch introducing brcmf_fw_complete_request(). All-in-all a nice cleanup. Thanks. Reviewed-by: Arend van Spriel Signed-off-by: Hans de Goede --- .../net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-)
Re: [PATCH 4/6] brcmfmac: Set board_type used for nvram file selection to machine-compatible
On 10/9/2018 2:47 PM, Hans de Goede wrote: For of/devicetree using machines, set the board_type used for nvram file selection to the first string listed in the top-level's node compatible string, aka the machine-compatible as used by of_machine_is_compatible(). The board_type setting is used to load the board-specific nvram file with a board-specific name so that we can ship files for each supported board in linux-firmware. Reviewed-by: Arend van Spriel Signed-off-by: Hans de Goede --- .../net/wireless/broadcom/brcm80211/brcmfmac/common.h | 1 + drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 11 ++- .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 1 + .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 + 4 files changed, 13 insertions(+), 1 deletion(-)
Re: [PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines
On 10/9/2018 2:47 PM, Hans de Goede wrote:
For x86 based machines, set the board_type used for nvram file selection
based on the DMI sys-vendor and product-name strings.
Since on some models these strings are too generic, this commit also adds
a quirk table overriding the strings for models listed in that table.
The board_type setting is used to load the board-specific nvram file with
a board-specific name so that we can ship files for each supported board
in linux-firmware.
some comments below
Reviewed-by: Arend van Spriel
Signed-off-by: Hans de Goede
---
.../broadcom/brcm80211/brcmfmac/Makefile | 2 +
.../broadcom/brcm80211/brcmfmac/common.c | 3 +-
.../broadcom/brcm80211/brcmfmac/common.h | 7 ++
.../broadcom/brcm80211/brcmfmac/dmi.c | 104 ++
4 files changed, 115 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
index 1f5a9b948abf..22fd95a736a8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
@@ -54,3 +54,5 @@ brcmfmac-$(CONFIG_BRCM_TRACING) += \
tracepoint.o
brcmfmac-$(CONFIG_OF) += \
of.o
+brcmfmac-$(CONFIG_DMI) += \
+ dmi.o
Assuming OF and DMI are mutual exclusive, right?
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
new file mode 100644
index ..fadc0ec745b8
--- /dev/null
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
[...]
+static const struct dmi_system_id dmi_platform_data[] = {
maybe call this dmi_platform_quirk as in brcmf_dmi_probe() you call this
a "quirk table".
Re: [PATCH 3/6] brcmfmac: Add support for first trying to get a board specific nvram file
On 10/9/2018 2:47 PM, Hans de Goede wrote:
The nvram files which some brcmfmac chips need are board-specific. To be
able to distribute these as part of linux-firmware, so that devices with
such a wifi chip will work OOTB, multiple (one per board) versions must
co-exist under /lib/firmware.
This commit adds support for callers of the brcmfmac/firmware.c code to
pass in a board_type parameter through the request structure.
If that parameter is set then the code will first try to load
chipmodel.board_type.txt before falling back to the old chipmodel.txt name.
minor comment below...
Reviewed-by: Arend van Spriel
Signed-off-by: Hans de Goede
---
.../broadcom/brcm80211/brcmfmac/firmware.c| 27 ++-
.../broadcom/brcm80211/brcmfmac/firmware.h| 1 +
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 08aaf99fee34..6755b2388fbc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -532,6 +532,31 @@ static int brcmf_fw_complete_request(const struct firmware
*fw,
return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
}
+static int brcmf_fw_request_firmware(const struct firmware **fw,
+struct brcmf_fw *fwctx)
+{
+ struct brcmf_fw_item *cur = &fwctx->req->items[fwctx->curpos];
+ int ret;
+
+ /* nvram files are board-specific, first try a board-specific path */
+ if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
+ char alt_path[BRCMF_FW_NAME_LEN];
+
+ strlcpy(alt_path, cur->path, BRCMF_FW_NAME_LEN);
+ /* strip .txt at the end */
+ alt_path[strlen(alt_path) - 4] = 0;
+ strlcat(alt_path, ".", BRCMF_FW_NAME_LEN);
why not string just "txt"?
+ strlcat(alt_path, fwctx->req->board_type, BRCMF_FW_NAME_LEN);
+ strlcat(alt_path, ".txt", BRCMF_FW_NAME_LEN);
+
+ ret = request_firmware(fw, alt_path, fwctx->dev);
+ if (ret == 0)
+ return ret;
+ }
+
+ return request_firmware(fw, cur->path, fwctx->dev);
+}
+
static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
{
struct brcmf_fw *fwctx = ctx;
@@ -544,7 +569,7 @@ static void brcmf_fw_request_done(const struct firmware
*fw, void *ctx)
while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
cur = &fwctx->req->items[fwctx->curpos];
- request_firmware(&fw, cur->path, fwctx->dev);
+ brcmf_fw_request_firmware(&fw, fwctx);
ret = brcmf_fw_complete_request(fw, ctx);
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
index 2893e56910f0..a0834be8864e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
@@ -70,6 +70,7 @@ struct brcmf_fw_request {
u16 domain_nr;
u16 bus_nr;
u32 n_items;
+ const char *board_type;
struct brcmf_fw_item items[0];
};
Re: [PATCH 2/6] brcmfmac: Remove recursion from firmware load error handling
On 10/9/2018 2:47 PM, Hans de Goede wrote: Before this commit brcmf_fw_request_done would call brcmf_fw_request_next_item to load the next item, which on an error would call brcmf_fw_request_done, which if the error is recoverable (*) will then continue calling brcmf_fw_request_next_item for the next item again which on an error will call brcmf_fw_request_done again... This does not blow up because we only have a limited number of items so we never recurse too deep. But the recursion is still quite ugly and frankly is giving me a headache, so lets fix this. This commit fixes this by removing brcmf_fw_request_next_item and by making brcmf_fw_get_firmwares and brcmf_fw_request_done directly call firmware_request_nowait resp. firmware_request themselves. *) brcmf_fw_request_nvram_done fallback path succeeds or BRCMF_FW_REQF_OPTIONAL is set Reviewed-by: Arend van Spriel Signed-off-by: Hans de Goede --- .../broadcom/brcm80211/brcmfmac/firmware.c| 65 ++- 1 file changed, 19 insertions(+), 46 deletions(-)
Re: [PATCH 1/6] brcmfmac: Remove firmware-loading code duplication
On 10/9/2018 2:47 PM, Hans de Goede wrote: brcmf_fw_request_next_item and brcmf_fw_request_done both have identical code to complete the fw-request depending on the item-type. This commit adds a new brcmf_fw_complete_request helper removing this code duplication. Reviewed-by: Arend van Spriel Signed-off-by: Hans de Goede --- .../broadcom/brcm80211/brcmfmac/firmware.c| 62 +-- 1 file changed, 31 insertions(+), 31 deletions(-)
Re: [PATCH 2/5] brcmfmac: Use common CORDIC_FLOAT macro from lib
On 11/5/2018 10:05 AM, Kalle Valo wrote: Priit Laes writes: Now that cordic library has the CORDIC_FLOAT macro, use that Signed-off-by: Priit Laes --- drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c | 4 ++-- drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c | 4 ++-- The driver is "brcmsmac" (note the 's', not 'f'), you should fix the title accordingly. --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c @@ -3447,8 +3447,8 @@ wlc_lcnphy_start_tx_tone(struct brcms_phy *pi, s32 f_kHz, u16 max_val, theta += rot; - i_samp = (u16) (FLOAT(tone_samp.i * max_val) & 0x3ff); - q_samp = (u16) (FLOAT(tone_samp.q * max_val) & 0x3ff); + i_samp = (u16)(CORDIC_FLOAT(tone_samp.i * max_val) & 0x3ff); + q_samp = (u16)(CORDIC_FLOAT(tone_samp.q * max_val) & 0x3ff); I haven't seen the patch 1 yet, but just from seeing this patch I don't get what's the benefit. The FLOAT macro was defined in brcmsmac (see patch 3). It is now moved to the cordic library simply because it is more closely related to that. Regards, Arend
Re: [PATCH 5/5] b43: Drop internal cordic algorithm implementation
On 11/5/2018 10:09 AM, Kalle Valo wrote: Priit Laes writes: Signed-off-by: Priit Laes No empty commit logs, please. And IMHO you could fold patch 5 into patch 4. Similarly 2 and 3. Regards, Arend
Re: 7f9a3e150ec7d breaks brcmfmac with modparam roamoff=1
On 11/4/2018 8:04 PM, Stijn Tintel wrote: [ 259.240131] WARNING: CPU: 0 PID: 50 at /home/build/lede/build_dir/target-arm_arm1176jzf-s+vfp_musl_eabi/linux-brcm2708_bcm2708/backports-4.19-rc5-1/net/wireless/core.c:736 wiphy_register+0x280/0xa58 [cfg80211] [ 259.274067] Modules linked in: brcmfmac(+) pppoe ppp_async pppox Hi Stijn, Thanks for the report. The code fails on the check below: diff --git a/net/wireless/core.c b/net/wireless/core.c index c0fd8a85e7f7..5fe35aafdd9c 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -725,6 +725,10 @@ int wiphy_register(struct wiphy *wiphy) (!rdev->ops->set_pmk || !rdev->ops->del_pmk))) return -EINVAL; + if (WARN_ON(!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_FW_ROAM) && + rdev->ops->update_connect_params)) + return -EINVAL; + if (wiphy->addresses) memcpy(wiphy->perm_addr, wiphy->addresses[0].addr, ETH_ALEN); It seemed to makes sense to me at the time to have this check and I would say it still does. So I probably need to fix brcmfmac for this. Regards, Arend
Re: [PATCH 0/5] Use common cordic algorithm for b43
On 11/3/2018 10:59 AM, Priit Laes wrote: b43 wireless driver included internal implementation of cordic algorithm which has now been removed in favor of library implementation. During the process, brcmfmac was driver was also cleaned. You actually touched the *brcmsmac* driver, not brcmfmac. Please fix the driver prefix where appropriate in this series, ie. patches 2 and 3. Please note that this series is only compile-tested, as I do not have access to the hardware. I can/will verify brcmsmac. As Kalle mentioned it makes more sense to push the 'lib: cordic:' patch through the wireless tree as well as it only is used by wireless drivers right now. Regards, Arend
Re: [PATCH 0/5] Use common cordic algorithm for b43
On 11/5/2018 9:02 AM, Kalle Valo wrote: Also I don't see MAINTAINERS entry for cordic.[c|h], that would be good to have as well. We added the cordic library functions during brcm80211 staging cleanup. We can add it to MAINTAINERS file. Regards, Arend
Re: [PATCH v2 0/2] brcmfmac: throughput enhancement for flow control mode
On 11/2/2018 9:24 AM, Wright Feng wrote: The patches are for throughput enhancement with flow control mode enabled and introduce a new module parameter to enhance TX throughput as well. Changes since v1: Remove the patch "calling skb_orphan before sending skb to SDIO bus" Revise the patch "brcmfmac: add credit numbers updating support" I also saw a patch Chi-Hsien Lin regarding fwsignal support. It would be good if that patch was part of this patch series as they seem dependent or at least related. Regards, Arend Wright Feng (2): brcmfmac: add credit numbers updating support brcmfmac: make firmware frameburst mode a module parameter .../broadcom/brcm80211/brcmfmac/cfg80211.c| 7 ++ .../broadcom/brcm80211/brcmfmac/common.c | 5 .../broadcom/brcm80211/brcmfmac/common.h | 2 ++ .../broadcom/brcm80211/brcmfmac/fwil.h| 1 + .../broadcom/brcm80211/brcmfmac/fwsignal.c| 23 --- 5 files changed, 30 insertions(+), 8 deletions(-)
Re: [PATCH v2 2/2] brcmfmac: make firmware frameburst mode a module parameter
On 11/2/2018 9:24 AM, Wright Feng wrote:
This patch is for adding a new module parameter "frameburst".
With setting "frameburst=1" in module parameters, firmware frameburst mode
will be enabled. The feature can enable per-packet framebursting in
firmware side and get higher TX throughput in High Throughput(HT) mode.
So why not always enable it in firmware as it seems to be the default in
recent firmware anyway.
Regards,
Arend
Signed-off-by: Wright Feng
---
.../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c| 7 +++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 5 +
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 ++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h| 1 +
4 files changed, 15 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 230a378c26fc..4a05d3f50cff 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6638,6 +6638,13 @@ static s32 brcmf_config_dongle(struct
brcmf_cfg80211_info *cfg)
brcmf_configure_arp_nd_offload(ifp, true);
+ if (ifp->drvr->settings->frameburst) {
+ err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_FAKEFRAG, 1);
+ if (err)
+ brcmf_info("setting frameburst mode failed\n");
+ brcmf_dbg(INFO, "frameburst mode enabled\n");
+ }
+
cfg->dongle_up = true;
default_conf_out:
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 94044a7a6021..0ad4c3196e5d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -78,6 +78,10 @@ static int brcmf_iapp_enable;
module_param_named(iapp, brcmf_iapp_enable, int, 0);
MODULE_PARM_DESC(iapp, "Enable partial support for the obsoleted Inter-Access Point
Protocol");
+static int brcmf_frameburst;
+module_param_named(frameburst, brcmf_frameburst, int, 0);
+MODULE_PARM_DESC(frameburst, "Enable firmware frameburst feature");
+
#ifdef DEBUG
/* always succeed brcmf_bus_started() */
static int brcmf_ignore_probe_fail;
@@ -419,6 +423,7 @@ struct brcmf_mp_device *brcmf_get_module_param(struct
device *dev,
settings->fcmode = brcmf_fcmode;
settings->roamoff = !!brcmf_roamoff;
settings->iapp = !!brcmf_iapp_enable;
+ settings->frameburst = !!brcmf_frameburst;
#ifdef DEBUG
settings->ignore_probe_fail = !!brcmf_ignore_probe_fail;
#endif
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index a34642cb4d2f..b91975258a68 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -47,6 +47,7 @@ extern struct brcmf_mp_global_t brcmf_mp_global;
* @feature_disable: Feature_disable bitmask.
* @fcmode: FWS flow control.
* @roamoff: Firmware roaming off?
+ * @frameburst: Firmware frame burst mode.
* @ignore_probe_fail: Ignore probe failure.
* @country_codes: If available, pointer to struct for translating country
codes
* @bus: Bus specific platform data. Only SDIO at the mmoment.
@@ -57,6 +58,7 @@ struct brcmf_mp_device {
int fcmode;
boolroamoff;
booliapp;
+ boolframeburst;
boolignore_probe_fail;
struct brcmfmac_pd_cc *country_codes;
union {
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
index 63b1287e2e6d..b6b183b18413 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
@@ -80,6 +80,7 @@
#define BRCMF_C_SCB_DEAUTHENTICATE_FOR_REASON 201
#define BRCMF_C_SET_ASSOC_PREFER 205
#define BRCMF_C_GET_VALID_CHANNELS 217
+#define BRCMF_C_SET_FAKEFRAG 219
#define BRCMF_C_GET_KEY_PRIMARY235
#define BRCMF_C_SET_KEY_PRIMARY236
#define BRCMF_C_SET_SCAN_PASSIVE_TIME 258
Re: [PATCH 3/3] brcmfmac: make firmware frameburst mode a module parameter
On 10/29/2018 11:27 AM, Wright Feng wrote: This patch is for adding a new module parameter "frameburst". With setting "frameburst=1" in module parameters, firmware frameburst mode will be enabled. The feature can enable per-packet framebursting in firmware side and get higher TX throughput in High Throughput(HT) mode. I am not sure about every firmware image, but in recent branches here I see firmware enables frameburst by default. So not sure about the motivation to add this. You could also consider adding this to the nl80211 api. I am sure other vendors have similar features. Regards, Arend Signed-off-by: Wright Feng --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 7 +++ drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 5 + drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 ++ drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h | 1 + 4 files changed, 15 insertions(+)
Re: [PATCH 2/3] brcmfmac: add credit numbers updating support
On 10/29/2018 11:27 AM, Wright Feng wrote:
The credit numbers are static and tunable per chip in firmware side.
However the credit number may be changed that is based on packet pool
length and will send BRCMF_E_FIFO_CREDIT_MAP event to notify host driver
updates the credit numbers during interface up.
The purpose of this patch is making host driver has ability of updating
the credit numbers when receiving the BRCMF_E_FIFO_CREDIT_MAP event.
Reviewed-by: Arend van Spriel
Signed-off-by: Wright Feng
---
.../broadcom/brcm80211/brcmfmac/fwsignal.c | 23 ++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
index f3cbf78..e0910c5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
[...]
@@ -1595,19 +1599,21 @@ static int brcmf_fws_notify_credit_map(struct brcmf_if
*ifp,
brcmf_err("event payload too small (%d)\n", e->datalen);
return -EINVAL;
}
- if (fws->creditmap_received)
- return 0;
fws->creditmap_received = true;
I think the creditmap_received struct member is no longer needed.
brcmf_dbg(TRACE, "enter: credits %pM\n", credits);
brcmf_fws_lock(fws);
for (i = 0; i < ARRAY_SIZE(fws->fifo_credit); i++) {
- if (*credits)
+ fws->fifo_credit[i] += credits[i] - fws->init_fifo_credit[i];
+ fws->init_fifo_credit[i] = credits[i];
+ if (fws->fifo_credit[i] > 0)
fws->fifo_credit_map |= 1 << i;
else
fws->fifo_credit_map &= ~(1 << i);
- fws->fifo_credit[i] = *credits++;
+ if (fws->fifo_credit[i] < 0)
+ brcmf_err("fifo_credit[%d] value is negative(%d)\n",
+ i, fws->fifo_credit[i]);
This looks like it should not happen so maybe warrants a WARN or WARN_ONCE?
}
brcmf_fws_schedule_deq(fws);
brcmf_fws_unlock(fws);
Re: 4.18.16: memory leak in sta_set_sinfo
On 10/22/2018 10:37 AM, Johannes Berg wrote: On Mon, 2018-10-22 at 10:35 +0200, Johannes Stezenbach wrote: commit 848e616e66d4592fe9afc40743d3504deb7632b4 Author: Stefan Seyfried Date: Sun Sep 30 12:53:00 2018 +0200 cfg80211: fix wext-compat memory leak Hopefully that'll eventually propagate to stable. Good to know it's fixed, but "hopefully" makes me wonder, I'd love to hear the confirmation that it's been queued. Well, normally it works automatically when you have a Fixes tag, so I don't usually try to queue anything manually. Maybe Greg has had his hands full with the 4.19 release :-) Missed the memo. Does that mean Cc: to stable is no longer needed? It is still documented as such in Documentation/process/stable-kernel-rules.rst. Regards, Arend
Re: [PATCH] brcmfmac: fix spelling mistake "Retreiving" -> "Retrieving"
On 10/16/2018 7:43 PM, Colin King wrote: From: Colin Ian King Trivial fix to spelling mistake in brcmf_err error message. Acked-by: Arend van Spriel Signed-off-by: Colin Ian King --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Re: [PATCH] mac80211: allow hardware scan to fall back to software
On 10/18/2018 10:35 AM, Johannes Berg wrote:
From: Johannes Berg
In some cases, like in the rsi driver hardware scan offload, there
may be scenarios in which hardware scan might not be available or
desirable.
Allow drivers to cope with this by letting them fall back to software
scan by returning the special value 1 from the hardware scan method.
Requested-by: Sushant Kumar Mishra
Requested-by: Siva Rebbagondla
Signed-off-by: Johannes Berg
---
include/net/mac80211.h | 5 +
net/mac80211/scan.c| 22 ++
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 71985e95d2d9..4631b3b6fc37 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3239,6 +3239,11 @@ enum ieee80211_reconfig_type {
* When the scan finishes, ieee80211_scan_completed() must be called;
* note that it also must be called when the scan cannot finish due to
* any error unless this callback returned a negative error code.
+ * This callback is also allowed to return the special return value 1,
+ * this indicates that hardware scan isn't desirable right now and a
+ * software scan should be done instead. A driver wishing to use this
+ * capability must ensure its (hardware) scan capabilities aren't
+ * advertised as more capable than mac80211's software scan is.
Hi Johannes,
Not sure what is meant by the last sentence. What does "more capable"
mean? What are (or where to find) the mac80211 software scan capabilities?
Regards,
Arend
Re: brcmfmac with BCM4359 on arm64 (RK3399) and SDIO
On 10/12/2018 10:59 AM, Christoph Müllner wrote:
On 10/12/18 10:00 AM, Arend van Spriel wrote:
On 10/11/2018 6:04 PM, Christoph Müllner wrote:
Hi Franky and Arend,
today I could get a SDIO Wifi module, which includes a BCM43455.
I was able to get this up and running without any issues with the
brcmfmac
driver and a 4.19 kernel. For me that's enough evidence to say that
the SDIO
driver works.
However, the BCM4359 still does not work.
It times out in brcmf_sdio_firmware_callback(), while enabling func2.
I've inserted tons of debug log outputs in both, the DHD driver and the
brcmfmac driver, and compared them. Differences which I've found so far
are: a) brcmfmac strips out whitespaces from nvram contents and
b) DHD downloads firmware first and brcmfmac downloads nvram first.
I've adapted the DHD driver to behave like brcmfmac in both cases
and it still works.
I've increased the timeout for enabling func2 from 3 seconds to 10
seconds,
but that did not help.
Any ideas left?
When enabling func2 fails it generally means the firmware crashed. I am
not sure if the patch below works to get console information. It might
show up empty or simply fail if firmware did not fill shared memory
info, but it may be worth a try.
I added the patch and additionally added debug output for all error
cases in the two called functions. Here's the output:
[ 14.746092] brcmfmac: brcmf_sdio_firmware_callback: enable F2: err=-62
[ 14.767523] brcmfmac: brcmf_sdio_checkdied: firmware not built with
-assert
[ 14.778777] brcmfmac: brcmf_sdio_checkdied: firmware trap in dongle
[ 14.789220] brcmfmac: brcmf_sdio_readconsole: brcmf_sdio_readconsole:
bus->console_addr == 0!
Do you have an educated guess, what causes the firmware crash, when
being loaded via the brcmfmac driver?
Maybe we can find out with the patch below.
Regards,
Arend
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/ne
index b2e1ab5..b1a4631 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -2969,21 +2969,35 @@ static int brcmf_sdio_trap_info(struct seq_file
*seq, str
if (error < 0)
return error;
- seq_printf(seq,
- "dongle trap info: type 0x%x @ epc 0x%08x\n"
- " cpsr 0x%08x spsr 0x%08x sp 0x%08x\n"
- " lr 0x%08x pc 0x%08x offset 0x%x\n"
- " r0 0x%08x r1 0x%08x r2 0x%08x r3 0x%08x\n"
- " r4 0x%08x r5 0x%08x r6 0x%08x r7 0x%08x\n",
- le32_to_cpu(tr.type), le32_to_cpu(tr.epc),
- le32_to_cpu(tr.cpsr), le32_to_cpu(tr.spsr),
- le32_to_cpu(tr.r13), le32_to_cpu(tr.r14),
- le32_to_cpu(tr.pc), sh->trap_addr,
- le32_to_cpu(tr.r0), le32_to_cpu(tr.r1),
- le32_to_cpu(tr.r2), le32_to_cpu(tr.r3),
- le32_to_cpu(tr.r4), le32_to_cpu(tr.r5),
- le32_to_cpu(tr.r6), le32_to_cpu(tr.r7));
-
+ if (seq)
+ seq_printf(seq,
+ "dongle trap info: type 0x%x @ epc 0x%08x\n"
+ " cpsr 0x%08x spsr 0x%08x sp 0x%08x\n"
+ " lr 0x%08x pc 0x%08x offset 0x%x\n"
+ " r0 0x%08x r1 0x%08x r2 0x%08x r3 0x%08x\n"
+ " r4 0x%08x r5 0x%08x r6 0x%08x r7
0x%08x\n",
+ le32_to_cpu(tr.type), le32_to_cpu(tr.epc),
+ le32_to_cpu(tr.cpsr), le32_to_cpu(tr.spsr),
+ le32_to_cpu(tr.r13), le32_to_cpu(tr.r14),
+ le32_to_cpu(tr.pc), sh->trap_addr,
+ le32_to_cpu(tr.r0), le32_to_cpu(tr.r1),
+ le32_to_cpu(tr.r2), le32_to_cpu(tr.r3),
+ le32_to_cpu(tr.r4), le32_to_cpu(tr.r5),
+ le32_to_cpu(tr.r6), le32_to_cpu(tr.r7));
+ else
+ pr_debug("dongle trap info: type 0x%x @ epc 0x%08x\n"
+" cpsr 0x%08x spsr 0x%08x sp 0x%08x\n"
+" lr 0x%08x pc 0x%08x offset 0x%x\n"
+" r0 0x%08x r1 0x%08x r2 0x%08x r3 0x%08x\n"
+" r4 0x%08x r5 0x%08x r6 0x%08x r7 0x%08x\n",
+le32_to_cpu(tr.type), le32_to_cpu(tr.epc),
+le32_to_cpu(tr.cpsr), le32_to_cpu(tr.spsr),
+le32_to_cpu(tr.r13), le32_to_cpu(tr.r14),
+le32_to_cpu(tr.pc), sh->trap_addr,
+le32_to_cpu(tr.r0), le32_to_cpu(tr.r1),
+le32_to_cpu(tr.r2), le32_to_cpu(tr.r3),
+
Re: [RFC v3] cfg80211: add peer measurement with FTM API
On 10/12/2018 12:41 PM, Johannes Berg wrote: + NL80211_ATTR_TIMEOUT, + Guess you consider reuse of the TIMEOUT attribute? Yes, I was actually surprised we don't have one already :-) Dito. Arend
Re: [RFC v3] cfg80211: add peer measurement with FTM API
On 10/12/2018 12:08 PM, Johannes Berg wrote:
From: Johannes Berg
Add a new "peer measurement" API, that can be used to measure
certain things related to a peer. Right now, only implement
FTM (flight time measurement) over it, but the idea is that
it'll be extensible to also support measuring the necessary
things to calculate e.g. angle-of-arrival for WiGig.
The API is structured to have a generic list of peers and
channels to measure with/on, and then for each of those a
set of measurements (again, only FTM right now) to perform.
Results are sent to the requesting socket, including a final
complete message.
Closing the controlling netlink socket will abort a running
measurement.
Signed-off-by: Johannes Berg
---
You have no recollection what happened in the earlier versions, right? :-p
v3:
- add a bit to report "final" for partial results
- remove list keeping etc. and just unicast out the results
to the requester (big code reduction ...)
- also send complete message unicast, and as a result
remove the multicast group
- separate out struct cfg80211_pmsr_ftm_request_peer
from struct cfg80211_pmsr_request_peer
- document timeout == 0 if no timeout
- disallow setting timeout nl80211 attribute to 0,
must not include attribute for no timeout
All these negations make my head spin (a little). Let's look at the
actual documentation further down...
- make MAC address randomization optional
- change num bursts exponent default to 0 (1 burst, rather
rather than the old default of 15==don't care)
---
include/net/cfg80211.h | 255 +++
include/uapi/linux/nl80211.h | 410 ++
net/wireless/Makefile| 1 +
net/wireless/core.c | 33 +++
net/wireless/core.h | 4 +
net/wireless/nl80211.c | 200 ---
net/wireless/nl80211.h | 41 +++
net/wireless/pmsr.c | 576 +++
net/wireless/rdev-ops.h | 25 ++
net/wireless/trace.h | 68 +
10 files changed, 1579 insertions(+), 34 deletions(-)
create mode 100644 net/wireless/pmsr.c
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0e16e723dcef..2837ea5f37e1 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2841,6 +2841,191 @@ struct cfg80211_ftm_responder_stats {
u32 out_of_window_triggers_num;
};
+/**
+ * struct cfg80211_pmsr_ftm_result - FTM result
+ * @failure_reason: if this measurement failed (PMSR status is
+ * %NL80211_PMSR_STATUS_FAILURE), this gives a more precise
+ * reason than just "failure"
+ * @burst_index: if reporting partial results, this is the index
+ * in [0 .. num_bursts-1] of the burst that's being reported
+ * @num_ftmr_attempts: number of FTM request frames transmitted
+ * @num_ftmr_successes: number of FTM request frames acked
+ * @busy_retry_time: if failure_reason is %NL80211_PMSR_FTM_FAILURE_PEER_BUSY,
+ * fill this to indicate in how many seconds a retry is deemed possible
+ * by the responder
+ * @num_bursts_exp: actual number of bursts exponent negotiated
+ * @burst_duration: actual burst duration negotiated
+ * @frames_per_burst: actual frames per burst negotiated
+ * @lci_len: length of LCI information (if present)
+ * @civicloc_len: length of civic location information (if present)
+ * @lci: LCI data (may be %NULL)
+ * @civicloc: civic location data (may be %NULL)
+ * @rssi_avg: average RSSI over FTM action frames reported
+ * @rssi_spread: spread of the RSSI over FTM action frames reported
+ * @tx_rate: bitrate for transmitted FTM action frame response
+ * @rx_rate: bitrate of received FTM action frame
+ * @rtt_avg: average of RTTs measured (must have either this or @dist_avg)
+ * @rtt_variance: variance of RTTs measured (note that standard deviation is
+ * the square root of the variance)
+ * @rtt_spread: spread of the RTTs measured
+ * @dist_avg: average of distances (mm) measured
+ * (must have either this or @rtt_avg)
+ * @dist_variance: variance of distances measured (see also @rtt_variance)
+ * @dist_spread: spread of distances measured (see also @rtt_spread)
+ * @num_ftmr_attempts_valid: @num_ftmr_attempts is valid
+ * @num_ftmr_successes_valid: @num_ftmr_successes is valid
+ * @rssi_avg_valid: @rssi_avg is valid
+ * @rssi_spread_valid: @rssi_spread is valid
+ * @tx_rate_valid: @tx_rate is valid
+ * @rx_rate_valid: @rx_rate is valid
+ * @rtt_avg_valid: @rtt_avg is valid
+ * @rtt_variance_valid: @rtt_variance is valid
+ * @rtt_spread_valid: @rtt_spread is valid
+ * @dist_avg_valid: @dist_avg is valid
+ * @dist_variance_valid: @dist_variance is valid
+ * @dist_spread_valid: @dist_spread is valid
+ */
+struct cfg80211_pmsr_ftm_result {
+ const u8 *lci;
+ const u8 *civicloc;
+ unsigned int lci_len;
+ unsigned int civicloc_len;
+ enum nl80211_peer_measurement_ftm_failure_reasons failure_reason;
+ u32 num_ftmr_attempts, num_ftmr_successes;
Re: brcmfmac with BCM4359 on arm64 (RK3399) and SDIO
On 10/12/2018 10:59 AM, Christoph Müllner wrote: On 10/12/18 10:00 AM, Arend van Spriel wrote: On 10/11/2018 6:04 PM, Christoph Müllner wrote: Hi Franky and Arend, today I could get a SDIO Wifi module, which includes a BCM43455. I was able to get this up and running without any issues with the brcmfmac driver and a 4.19 kernel. For me that's enough evidence to say that the SDIO driver works. However, the BCM4359 still does not work. It times out in brcmf_sdio_firmware_callback(), while enabling func2. I've inserted tons of debug log outputs in both, the DHD driver and the brcmfmac driver, and compared them. Differences which I've found so far are: a) brcmfmac strips out whitespaces from nvram contents and b) DHD downloads firmware first and brcmfmac downloads nvram first. I've adapted the DHD driver to behave like brcmfmac in both cases and it still works. I've increased the timeout for enabling func2 from 3 seconds to 10 seconds, but that did not help. Any ideas left? When enabling func2 fails it generally means the firmware crashed. I am not sure if the patch below works to get console information. It might show up empty or simply fail if firmware did not fill shared memory info, but it may be worth a try. I added the patch and additionally added debug output for all error cases in the two called functions. Here's the output: [ 14.746092] brcmfmac: brcmf_sdio_firmware_callback: enable F2: err=-62 [ 14.767523] brcmfmac: brcmf_sdio_checkdied: firmware not built with -assert [ 14.778777] brcmfmac: brcmf_sdio_checkdied: firmware trap in dongle [ 14.789220] brcmfmac: brcmf_sdio_readconsole: brcmf_sdio_readconsole: bus->console_addr == 0! Do you have an educated guess, what causes the firmware crash, when being loaded via the brcmfmac driver? Let's look at the firmware+nvram you are using. Can you do: $ strings brcmfmac4359-sdio.bin | tail -3 and send the output and the nvram file. Regards, Arend
Re: [PATCH] brcmfmac: support STA info struct v7
On 10/11/2018 10:19 PM, Dan Haab wrote:
The newest firmwares provide STA info using v7 of the struct. As v7
isn't backward compatible, a union is needed.
Even though brcmfmac does not use any of the new info it's important to
provide the proper struct buffer. Without this change new firmwares will
fallback to the very limited v3 instead of something in between such as
v4.
Hi Dan,
I don't have a real problem with adding this v7 stuff, but it looks
different from what I have here which has additional fields. For what
new firmwares and more importantly what devices do you need this. I only
know it is used for 43684, which we do not support with brcmfmac (yet).
Regards,
Arend
Signed-off-by: Dan Haab
---
.../broadcom/brcm80211/brcmfmac/fwil_types.h | 39 ++
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index d5bb81e..189d576 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -176,6 +176,8 @@
#define BRCMF_VHT_CAP_MCS_MAP_NSS_MAX 8
+#define BRCMF_HE_CAP_MCS_MAP_NSS_MAX 8
+
/* MAX_CHUNK_LEN is the maximum length for data passing to firmware in each
* ioctl. It is relatively small because firmware has small maximum size input
* playload restriction for ioctls.
@@ -601,13 +603,36 @@ struct brcmf_sta_info_le {
__le32 rx_pkts_retried;/* # rx with retry bit set */
__le32 tx_rate_fallback; /* lowest fallback TX rate */
- /* Fields valid for ver >= 5 */
- struct {
- __le32 count; /* # rates in
this set */
- u8 rates[BRCMF_MAXRATES_IN_SET];/* rates in
500kbps units w/hi bit set if basic */
- u8 mcs[BRCMF_MCSSET_LEN]; /* supported
mcs index bit map */
- __le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX]; /* supported
mcs index bit map per nss */
- } rateset_adv;
+ union {
+ struct {
+ struct {
+ __le32 count;
/* # rates in this set */
+ u8 rates[BRCMF_MAXRATES_IN_SET];
/* rates in 500kbps units w/hi bit set if basic */
+ u8 mcs[BRCMF_MCSSET_LEN];
/* supported mcs index bit map */
+ __le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX];
/* supported mcs index bit map per nss */
+ } rateset_adv;
+ } v5;
+
+ struct {
+ __le32 rx_dur_total;/* total user RX duration
(estimated) */
+ __le16 chanspec;/** chanspec this sta is on */
+ __le16 pad;
+ struct {
+ __le16 version;
/* version */
+ __le16 len;
/* length */
+ __le32 count;
/* # rates in this set */
+ u8 rates[BRCMF_MAXRATES_IN_SET];
/* rates in 500kbps units w/hi bit set if basic */
+ u8 mcs[BRCMF_MCSSET_LEN];
/* supported mcs index bit map */
+ __le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX];
/* supported mcs index bit map per nss */
+ __le16 he_mcs[BRCMF_HE_CAP_MCS_MAP_NSS_MAX];
/* supported he mcs index bit map per nss */
+ } rateset_adv; /* rateset along with mcs index
bitmap */
+ __le16 wpauth; /* authentication type */
+ u8 algo;/* crypto algorithm */
+ __le32 tx_rspec;/* Rate of last successful tx
frame */
+ __le32 rx_rspec;/* Rate of last successful rx
frame */
+ __le32 wnm_cap; /* wnm capabilities */
+ } v7;
+ };
};
struct brcmf_chanspec_list {
Re: brcmfmac with BCM4359 on arm64 (RK3399) and SDIO
On 10/11/2018 6:04 PM, Christoph Müllner wrote:
Hi Franky and Arend,
today I could get a SDIO Wifi module, which includes a BCM43455.
I was able to get this up and running without any issues with the brcmfmac
driver and a 4.19 kernel. For me that's enough evidence to say that the SDIO
driver works.
However, the BCM4359 still does not work.
It times out in brcmf_sdio_firmware_callback(), while enabling func2.
I've inserted tons of debug log outputs in both, the DHD driver and the
brcmfmac driver, and compared them. Differences which I've found so far
are: a) brcmfmac strips out whitespaces from nvram contents and
b) DHD downloads firmware first and brcmfmac downloads nvram first.
I've adapted the DHD driver to behave like brcmfmac in both cases
and it still works.
I've increased the timeout for enabling func2 from 3 seconds to 10 seconds,
but that did not help.
Any ideas left?
When enabling func2 fails it generally means the firmware crashed. I am
not sure if the patch below works to get console information. It might
show up empty or simply fail if firmware did not fill shared memory
info, but it may be worth a try.
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/
index c99a191..739dbaa 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4109,7 +4109,7 @@ static void brcmf_sdio_firmware_callback(struct
device *d
} else {
/* Disable F2 again */
sdio_disable_func(sdiod->func2);
- goto release;
+ goto checkdied;
}
if (brcmf_chip_sr_capable(bus->ci)) {
@@ -4151,6 +4151,9 @@ static void brcmf_sdio_firmware_callback(struct
device *d
/* ready */
return;
+checkdied:
+ brcmf_sdio_checkdied(bus);
+ brcmf_sdio_readconsole(bus);
release:
sdio_release_host(sdiod->func1);
fail:
Re: [PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines
On 10/10/2018 9:59 AM, Hans de Goede wrote: Hi Arend, On 10-10-18 09:52, Arend van Spriel wrote: On 10/10/2018 9:28 AM, Hans de Goede wrote: So how do you want to proceed with this, do you want me to just put the full ISC text in the header for now as the rest of brcmfmac does? This is not entirely true as far as I know. I assume you are referring to this: /* * Copyright (c) 2010 Broadcom Corporation * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above * copyright notice and this permission notice appear in all copies. * * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ As far as I recall we opted for BSD license and ISC is equivalent. I believe it is the other way around, you opted for the ISC license which is more or less equivalent to the 2 clause BSD, see: https://spdx.org/licenses/BSD-2-Clause.html https://spdx.org/licenses/ISC The ISC text is a 1:1 match to the license used in brcmfmac, and it seems sensible to me to be consistent and use the same license for all brcmfmac files even if the 2 are more or less equivalent. Looking at the ISC text you are probably right. My recollection was from a verbal notification and the person telling probably was mistaken. So I am fine with a follow-up patch to change all files to use ISC SPDX tag once the ISC license is listed under LICENSES. Hans p.s. Any chance you could do a patch-review of this series? Yup and test for regressions with some of the chipsets I have here. Regards, Arend
Re: [PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines
On 10/10/2018 9:28 AM, Hans de Goede wrote: So how do you want to proceed with this, do you want me to just put the full ISC text in the header for now as the rest of brcmfmac does? This is not entirely true as far as I know. I assume you are referring to this: /* * Copyright (c) 2010 Broadcom Corporation * * Permission to use, copy, modify, and/or distribute this software for any * purpose with or without fee is hereby granted, provided that the above * copyright notice and this permission notice appear in all copies. * * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ As far as I recall we opted for BSD license and ISC is equivalent. However, The BSD license are already in place so why not use that. I would say BSD-2-Clause should cover it. As this is a new file I guess it is up to you although I would prefer to stick with a permissive license. Regards, Arend
Re: brcmfmac with BCM4359 on arm64 (RK3399) and SDIO
On 10/9/2018 8:10 PM, Christoph Müllner wrote: Hi Arend, recently I got an SDIO module, which includes a BCM4359. I tried to get it up and running via the SD card interface on a RK3399 SoC and succeeded in doing so with bcmdhd.1.579.77.41.x and a vendor kernel (based on Linux 4.4). All that was necessary was configure BCMDHD to run with in-band IRQs and use a GPIO as gpio_wl_reg_on. Since I can run a mainline kernel as well, I gave it a try and tried brcmfmac on Linux 4.19-rc7. As the BCM4359 is not in the list of supported SDIO devices, but is supported USB device, I've created a patch (attached), which adds the support for that device. Okay. However, I should say that there is no BCM4359 USB device. Regardless the patch looks okay. Additionally I've patched my DTS to include the WL_REG_ON pin as part of mmc-pwrseq-simple's reset-gpios and added a bcm4329-fmac node in the mmc node. During bootup I see messages from brcmfmac, which indicate that the BCM4359 has been found, but loading the nvram file continuously fails: [5.993741] brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4359-sdio for chip BCM4359/9 [...] [7.987167] brcmfmac: brcmf_sdiod_ramrw: membytes transfer failed [8.008715] brcmfmac: brcmf_sdio_verifymemory: error -84 on reading 2048 membytes at 0x0025f1f0 [8.021182] brcmfmac: brcmf_sdio_download_firmware: dongle nvram file download failed That -84 means EILSEQ, which is the error value, which represents a CRC error during SDIO data exchange (returned by the function dw_mci_data_complete() in the MMC driver). Whenever I see -84 my suspicion are about power-supply for the chip. However, it may also be the mmc host driver not adhering to so-called "card busy" indication, which is in the SDIO spec. To address this, I've reduced the clock speed (in several steps) to 400 kHz (and verified the clock signal on an oscilloscope), but the issue persists. I've also tried to use Linux 4.14.74, where I see the same issue. I can confirm that the MMC interface works in general (I can use an SD card to host my rootfs). SD and SDIO are not the same thing. More importantly you have it working with bcmdhd on a vendor kernel. So start from there. FWIW I'm using the same exact same firmware and nvram file for the DHD driver and the brcmfmac. Noted. Do you have any ideas how to debug this issue? Would focus on getting logs from bcmdhd. Regards, Arend Thanks, Christoph
Re: [PATCH] mt76x0: add quirk to disable 2.4GHz band for Archer T1U
On 9/25/2018 11:48 AM, Stanislaw Gruszka wrote:
On Tue, Sep 25, 2018 at 11:07:47AM +0200, Lorenzo Bianconi wrote:
On Sep 25, Felix Fietkau wrote:
On 2018-09-25 09:54, Lorenzo Bianconi wrote:
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
index 76d607f73758..b7a1069ecd0e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
@@ -45,7 +45,8 @@
{ USB_DEVICE(0x20f4, 0x806b) }, /* TRENDnet TEW-806UBH */
{ USB_DEVICE(0x7392, 0xc711) }, /* Devolo Wifi ac Stick */
{ USB_DEVICE(0x0df6, 0x0079) }, /* Sitecom Europe B.V. ac Stick */
- { USB_DEVICE(0x2357, 0x0105) }, /* TP-LINK Archer T1U */
+ { USB_DEVICE(0x2357, 0x0105),
+ .driver_info = 1, }, /* TP-LINK Archer T1U */
Hi Stanislaw,
what about using 'ieee80211-freq-limit' OF property to limit available wireless
band? In this way we can take into account even the opposite case (no 5GHz).
That doesn't make sense for USB devices, which can be plugged into any
machine.
Ack, right.
What about a more general solution like adding an enum { NO_5GHz = 1, NO_2GHz }?
Does it worth to implement it now?
I do not see reason for that. Basically band information should be
read from EEPROM, I do not expect need for more quirks like this.
Well. Never say never :-p And the phrase "quirks like this" does seem to
leave the door open for other quirks. Or did you mean "quirks using
driver_info"?
Regards,
Arend
Re: [PATCH] New functionality for aborting ongoing CAC.
On 9/25/2018 11:28 AM, Enrique Giraldo wrote: The main reason is to be able to stop the CAC when you want to make a channel switch and the CAC is ongoing. It's true that the radio would not pass to the next phase, the behavior is the same as when during the CAC a radar event is detected. In the case of aborting, a later action is expected, for example, switch to the desired new channel. So for mac80211 the function ieee80211_dfs_cancel_cac() indeed seems to just abort. So what is the scenario here? If user-space sends NL80211_CMD_START_AP with DFS channel, a CAC period is started. So you are saying you want to do a channel switch? Does it not make sense to issue a NL80211_CMD_STOP_AP and then NL80211_CMD_START_AP for the desired channel? Maybe the use-case you are looking at is not AP mode, but there is no context whatsoever in the commit message so I can only guess. Regards, Arend
Re: [PATCH] New functionality for aborting ongoing CAC.
On 9/25/2018 10:19 AM, Enrique Giraldo wrote: Add NL80211_CMD_ABORT_CAC to the nl80211 interface. This one really needs a good motivation. The CAC duration is a hard requirement so aborting it means that the radio can not proceed to the next phase. You really need to describe your reasoning for wanting this in the commit message. Regards, Arend
Re: [PATCH v3] brcmsmac: AP mode: update beacon when TIM changes
On 9/23/2018 11:54 AM, Ali MJ Al-Nasrawy wrote: Beacons are not updated to reflect TIM changes. This is not compliant with power-saving client stations as the beacons do not have valid TIM and can cause the network to stall at random occasions with highly variable latencies. Fix it by updating beacon templates on mac80211 set_tim callback. Addresses an issue described in: https://marc.info/?i=20180911163534.21312d08%20()%20manjaro Signed-off-by: Ali MJ Al-Nasrawy --- Missing 'v1 -> v2' in this changelog. Regards, Arend v2 -> v3: - removed tabs from blank lines - extended the commit message to answer "Why" .../broadcom/brcm80211/brcmsmac/mac80211_if.c | 21 +++ .../broadcom/brcm80211/brcmsmac/main.h| 1 + 2 files changed, 22 insertions(+)
Re: [PATCH] brcmsmac: ap mode: update beacon when TIM changes
On 9/11/2018 7:26 PM, Ali MJ Al-Nasrawy wrote:
Beacons+TIM are created/updated for fw beaconing only when BSS_CHANGED_BEACON.
This is not compliant with power-saving stations.
Fix it by updating beacon templates on mac80211 set_tim callback.
Adresses the issue in:
https://marc.info/?i=20180911163534.21312d08%20()%20manjaro
A few remarks below but here is my
Reviewed-by: Arend van Spriel
Signed-off-by: Ali MJ Al-Nasrawy
---
.../broadcom/brcm80211/brcmsmac/mac80211_if.c | 23 +++
.../broadcom/brcm80211/brcmsmac/main.h| 2 ++
2 files changed, 25 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
index ddfdfe1..ee92bb5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/mac80211_if.c
@@ -502,6 +502,7 @@ brcms_ops_add_interface(struct ieee80211_hw *hw, struct
ieee80211_vif *vif)
}
spin_lock_bh(&wl->lock);
+ wl->wlc->vif = vif;
wl->mute_tx = false;
brcms_c_mute(wl->wlc, false);
if (vif->type == NL80211_IFTYPE_STATION)
@@ -937,6 +938,27 @@ static void brcms_ops_set_tsf(struct ieee80211_hw *hw,
spin_unlock_bh(&wl->lock);
}
+static int brcms_ops_beacon_set_tim(struct ieee80211_hw *hw,
+struct ieee80211_sta *sta, bool set)
+{
+ /*FIXME: this may be more efficiently handled by delegating
+beacon upload to the beacon interrupt handler*/
No sure what you mean by this remark. The code below looks ok to me, ie.
same as with bss update. So please drop the remark.
+ struct brcms_info *wl = hw->priv;
+ struct sk_buff *beacon;
+ u16 tim_offset = 0;
+
+ beacon = ieee80211_beacon_get_tim(hw, wl->wlc->vif,
+ &tim_offset, NULL);
+ if (beacon){
+ spin_lock_bh(&wl->lock);
+ brcms_c_set_new_beacon(wl->wlc, beacon, tim_offset,
+ wl->wlc->vif->bss_conf.dtim_period);
+ spin_unlock_bh(&wl->lock);
+ }
+
+ return 0;
+}
+
static const struct ieee80211_ops brcms_ops = {
.tx = brcms_ops_tx,
.start = brcms_ops_start,
@@ -955,6 +977,7 @@ static const struct ieee80211_ops brcms_ops = {
.flush = brcms_ops_flush,
.get_tsf = brcms_ops_get_tsf,
.set_tsf = brcms_ops_set_tsf,
+ .set_tim = brcms_ops_beacon_set_tim,
};
void brcms_dpc(unsigned long data)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.h
b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.h
index c4d135c..e3939fc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/main.h
@@ -568,6 +568,8 @@ struct brcms_c_info {
u16 beacon_tim_offset;
u16 beacon_dtim_period;
struct sk_buff *probe_resp;
+
Just get rid of the empty line (+TAB) above. I am not keen on storing
the vif, but it seems there is no other way to get that.
+ struct ieee80211_vif *vif;
};
/* antsel module specific state */
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
On 8/29/2018 10:17 PM, Rafał Miłecki wrote: Hi Arend, On Mon, 25 Jun 2018 at 10:31, Arend van Spriel wrote: On 6/25/2018 6:40 AM, Rafał Miłecki wrote: On Sun, 24 Jun 2018 at 13:48, Rafał Miłecki wrote: On Fri, 22 Jun 2018 at 20:45, Arend van Spriel wrote: Here a couple of patches in preparation of monitor mode support. It is mostly about querying firmware for support. While at it I stumbled upon the fact that 160MHz was not completely implemented in the driver so there is a patch for that as well. The first two patches are actually some changes to the patches that Rafal submitted. So this series depend on: [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1] These apply to the master branch of the wireless-drivers-next repository. [1] https://patchwork.kernel.org/patch/10474799/ I find it pointless to submit fixes for patches that weren't accepted yet. Let's work on improving original patches while they are still pending. I sent V4 with changes pointed our by Arend. That obsoletes all (?) monitor patches from this patchset. I believe it was cleaner to improve original patchset (not pushed yet). My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor related) and drop the others. Well. Given that Kalle prefers applying "all-or-nothing" I will resubmit the series addressing the issues you found. Would you have a moment to resubmit these 2 patches or do you mind if I do that? Found a moment. Should be in patchwork now. Regards, Arend
Re: [PATCH] mac80211: use non-zero TID only for QoS frames
On 9/5/2018 10:00 AM, Johannes Berg wrote:
From: Johannes Berg
Some frames may have a non-zero skb->priority assigned by
mac80211 internally, e.g. TDLS setup frames, regardless of
support for QoS.
Currently, we set skb->priority to 0 for all data frames.
Note that there's a comment that this is "required for
correct WPA/11i MIC", but that doesn't seem true as we use
if (ieee80211_is_data_qos(hdr->frame_control))
qos_tid = ieee80211_get_tid(hdr);
else
qos_tid = 0;
in the code there. We could therefore reconsider this, but
it seems like unnecessary complexity for the unlikely (and
not very useful) case of not having QoS on the connection.
This situation then causes something strange - most data
frames will go on TXQ for TID 0 for non-QoS connections,
but very few exceptions that are internally generated will
go on another TXQ, possibly causing confusion.
Avoid this confusion by putting non-QoS data frames into
TID 0 regardless of the skb->priority.
Signed-off-by: Johannes Berg
---
net/mac80211/tx.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 5b6e06ad35ff..6540595addd1 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1260,7 +1260,10 @@ static struct txq_info *ieee80211_get_txq(struct
ieee80211_local *local,
txq = sta->sta.txq[IEEE80211_NUM_TIDS];
}
} else if (sta) {
- u8 tid = skb->priority & IEEE80211_QOS_CTL_TID_MASK;
+ u8 tid = 0;
+
+ if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA))
+ tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
Is the use of different mask intentional here? Just a quick glance so
did not look into it further.
Gr. AvS
[PATCH V2 1/2] brcmfmac: fix for proper support of 160MHz bandwidth
Decoding of firmware channel information was not complete for 160MHz
support. This resulted in the following warning:
WARNING: CPU: 2 PID: at .../broadcom/brcm80211/brcmutil/d11.c:196
brcmu_d11ac_decchspec+0x2e/0x100 [brcmutil]
Modules linked in: brcmfmac(O) brcmutil(O) sha256_generic cfg80211 ...
CPU: 2 PID: Comm: kworker/2:0 Tainted: G O
4.17.0-wt-testing-x64-2-gf1bed50 #1
Hardware name: Dell Inc. Latitude E6410/07XJP9, BIOS A07 02/15/2011
Workqueue: events request_firmware_work_func
RIP: 0010:brcmu_d11ac_decchspec+0x2e/0x100 [brcmutil]
RSP: 0018:c9047bd0 EFLAGS: 00010206
RAX: e832 RBX: 8801146fe910 RCX: 8801146fd3c0
RDX: 2800 RSI: 0070 RDI: c9047c30
RBP: c9047bd0 R08: R09: a0798c80
R10: 88012bca55e0 R11: 880110a4ea00 R12: 8801146f8000
R13: c9047c30 R14: 8801146fe930 R15: 8801138e02e0
FS: () GS:88012bc8() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 7f18ce8b8070 CR3: 0200a003 CR4: 000206e0
Call Trace:
brcmf_setup_wiphybands+0x212/0x780 [brcmfmac]
brcmf_cfg80211_attach+0xae2/0x11a0 [brcmfmac]
brcmf_attach+0x1fc/0x4b0 [brcmfmac]
? __kmalloc+0x13c/0x1c0
brcmf_pcie_setup+0x99b/0xe00 [brcmfmac]
brcmf_fw_request_done+0x16a/0x1f0 [brcmfmac]
request_firmware_work_func+0x36/0x60
process_one_work+0x146/0x350
worker_thread+0x4a/0x3b0
kthread+0x102/0x140
? process_one_work+0x350/0x350
? kthread_bind+0x20/0x20
ret_from_fork+0x35/0x40
Code: 66 90 0f b7 07 55 48 89 e5 89 c2 88 47 02 88 47 03 66 81 e2 00 38
66 81 fa 00 18 74 6e 66 81 fa 00 20 74 39 66 81 fa 00 10 74 14 <0f>
0b 66 25 00 c0 74 20 66 3d 00 c0 75 20 c6 47 04 01 5d c3 66
---[ end trace 550c46682415b26d ]---
brcmfmac: brcmf_construct_chaninfo: Ignoring unexpected firmware channel 50
This patch adds the missing stuff to properly handle this.
Reviewed-by: Hante Meuleman
Reviewed-by: Pieter-Paul Giesberts
Reviewed-by: Franky Lin
Signed-off-by: Arend van Spriel
---
.../net/wireless/broadcom/brcm80211/brcmutil/d11.c | 34 +-
.../broadcom/brcm80211/include/brcmu_wifi.h| 2 ++
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
index d8b79cb..e7584b8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
@@ -77,6 +77,8 @@ static u16 d11ac_bw(enum brcmu_chan_bw bw)
return BRCMU_CHSPEC_D11AC_BW_40;
case BRCMU_CHAN_BW_80:
return BRCMU_CHSPEC_D11AC_BW_80;
+ case BRCMU_CHAN_BW_160:
+ return BRCMU_CHSPEC_D11AC_BW_160;
default:
WARN_ON(1);
}
@@ -190,8 +192,38 @@ static void brcmu_d11ac_decchspec(struct brcmu_chan *ch)
break;
}
break;
- case BRCMU_CHSPEC_D11AC_BW_8080:
case BRCMU_CHSPEC_D11AC_BW_160:
+ switch (ch->sb) {
+ case BRCMU_CHAN_SB_LLL:
+ ch->control_ch_num -= CH_70MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_LLU:
+ ch->control_ch_num -= CH_50MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_LUL:
+ ch->control_ch_num -= CH_30MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_LUU:
+ ch->control_ch_num -= CH_10MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_ULL:
+ ch->control_ch_num += CH_10MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_ULU:
+ ch->control_ch_num += CH_30MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_UUL:
+ ch->control_ch_num += CH_50MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_UUU:
+ ch->control_ch_num += CH_70MHZ_APART;
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ break;
+ }
+ break;
+ case BRCMU_CHSPEC_D11AC_BW_8080:
default:
WARN_ON_ONCE(1);
break;
diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
index 91fca79..dddebaa 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
@@ -29,6 +29,8 @@
#define CH_UPPER_SB0x01
#define CH_LOWER_SB
[PATCH V2 0/2] brcmfmac: fix for 160MHz and firmware capabilities
Resending two patches which apply to the master branch of the wireless-drivers-next repository. Arend van Spriel (2): brcmfmac: fix for proper support of 160MHz bandwidth brcmfmac: increase buffer for obtaining firmware capabilities .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 2 +- .../net/wireless/broadcom/brcm80211/brcmutil/d11.c | 34 +- .../broadcom/brcm80211/include/brcmu_wifi.h| 2 ++ 3 files changed, 36 insertions(+), 2 deletions(-) -- 1.9.1
[PATCH V2 2/2] brcmfmac: increase buffer for obtaining firmware capabilities
When obtaining the firmware capability a buffer is provided of 512
bytes. However, if all features in firmware are supported the buffer
needs to be 565 bytes as otherwise truncated information is retrieved
from firmware. Increasing the buffer to 768 bytes on stack.
Reviewed-by: Hante Meuleman
Reviewed-by: Pieter-Paul Giesberts
Reviewed-by: Franky Lin
Signed-off-by: Arend van Spriel
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 8347da6..4c5a399 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -178,7 +178,7 @@ static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
ifp->fwil_fwerr = false;
}
-#define MAX_CAPS_BUFFER_SIZE 512
+#define MAX_CAPS_BUFFER_SIZE 768
static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
{
char caps[MAX_CAPS_BUFFER_SIZE];
--
1.9.1
Re: [PATCH 12/16] iwlwifi: mvm: enable sending HE_AIR_SNIFFER command via debugfs
On 8/31/2018 11:56 AM, Luca Coelho wrote:
From: Shaul Triebitz
In order to receive TB (Trigger Based) PPDU in monitor mode,
the Driver must send the HE_AIR_SNIFFER_CONFIG_CMD host command.
Enable that via debugfs.
Signed-off-by: Liad Kaufman
Signed-off-by: Ido Yariv
Signed-off-by: Shaul Triebitz
Signed-off-by: Luca Coelho
---
.../wireless/intel/iwlwifi/fw/api/datapath.h | 5 ++
.../net/wireless/intel/iwlwifi/fw/api/mac.h | 14 +
.../net/wireless/intel/iwlwifi/mvm/debugfs.c | 53 +++
3 files changed, 72 insertions(+)
[snip]
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index 202158d03d36..725b97ecb447 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -1727,6 +1727,57 @@ iwl_dbgfs_send_echo_cmd_write(struct iwl_mvm *mvm, char
*buf,
return ret ?: count;
}
+static ssize_t
+iwl_dbgfs_set_aid_read(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ return 0;
+}
+
+static ssize_t
+iwl_dbgfs_set_aid_write(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct iwl_mvm *mvm = file->private_data;
+ struct iwl_he_monitor_cmd he_mon_cmd = {};
+ u8 tmp_buf[32];
+ u32 aid;
+ int ret;
+
+ if (!iwl_mvm_firmware_running(mvm))
+ return -EIO;
+
+ if (count > sizeof(tmp_buf))
+ return -EINVAL;
+
+ if (copy_from_user(tmp_buf, user_buf, count))
+ return -EIO;
+
+ ret = sscanf(tmp_buf, "%x %2hhx:%2hhx:%2hhx:%2hhx:%2hhx:%2hhx", &aid,
+&he_mon_cmd.bssid[0], &he_mon_cmd.bssid[1],
+&he_mon_cmd.bssid[2], &he_mon_cmd.bssid[3],
+&he_mon_cmd.bssid[4], &he_mon_cmd.bssid[5]);
+ if (ret != 7)
+ return -EINVAL;
+
+ he_mon_cmd.aid = cpu_to_le16(aid);
+
+ mutex_lock(&mvm->mutex);
+ ret = iwl_mvm_send_cmd_pdu(mvm, iwl_cmd_id(HE_AIR_SNIFFER_CONFIG_CMD,
+ DATA_PATH_GROUP, 0), 0,
+ sizeof(he_mon_cmd), &he_mon_cmd);
+ mutex_unlock(&mvm->mutex);
+
+ return ret ?: count;
+}
+
+static const struct file_operations iwl_dbgfs_set_aid_ops = {
+ .read = iwl_dbgfs_set_aid_read,
+ .write = iwl_dbgfs_set_aid_write,
+ .open = simple_open,
+ .llseek = default_llseek,
+};
What's with the name 'set_aid'?
Regards,
Arend
Re: feedback on mainlining wilc1000 staging driver
On 8/17/2018 9:49 AM, Kalle Valo wrote: Ajay Singh writes: On Thu, 16 Aug 2018 13:53:50 +0300 Kalle Valo wrote: Kalle Valo writes: Ajay Singh writes: Hi Greg, We all are working on submitting and reviewing patches for wilc1000 in staging driver for quite some time. We would like to have feedback on the next steps to bring wilc1000 driver closer to move into the wireless subsystem tree. In summary, the following major things from TODO have been addressed in staging: -remove the defined feature as kernel versions -remove OS wrapper functions -remove custom debug and tracing functions -rework comments and function headers(also coding style) -remove build warnings -replace all semaphores with mutexes or completions -make spi and sdio components coexist in one build -turn compile-time platform configuration (BEAGLE_BOARD, PANDA_BOARD, PLAT_WMS8304, PLAT_RK, CUSTOMER_PLATFORM, ...) into run-time options that are read from DT -replace SIOCDEVPRIVATE commands with generic API functions -use wext-core handling instead of private SIOCSIWPRIV implementation From wireless point of view: if I see wext mentioned anywhere in the driver I stop right there. cfg80211 is a hard requirement for us nowadays. Clarification: Depending on the hardware design either cfg80211 or mac80211 is a hard requirement. I haven't checked wilc1000 at all so I don't know what design it has but if it's a "softmac" design then it has to use mac80211, we do not want to support multiple 802.11 UMAC stacks. The TODO item to make use of wext-core is obsolete. Previously wilc1000 driver also had few wext ioctl use but now it’s completely removed and cfg80211 operation callbacks are used. wilc1000 driver make use of cfg80211 and isn’t a "softmac". Good. We need help to review and identify if there are any pending items for wilc1000 driver, so we can address those issues and make it ready to move to the wireless subsystem. I think the best way to get that forward is to submit a patch (or patchset) to linux-wireless, that's the easiest for reviewers. For brcm80211 drivers we used a single patch introducing it under the wireless drivers folder. Because it was quite a sizable patch we parked it on the wireless wiki page. Had a few iterations doing it like that. Regards, Arend
Re: [DEBUG PATCH] brcmfmac: add firmware watchdog running on host machine
On 8/15/2018 8:01 PM, Rafał Miłecki wrote:
From: Rafał Miłecki
It may happen for FullMAC firmware to crash. It should be detected and
ideally somehow handled by a driver.
Since commit ff4445a8502c ("brcmfmac: expose device memory to
devcoredump subsystem") brcmfmac has BRCMF_E_PSM_WATCHDOG event handler
but it wasn't enough to detect all kind of crashes. E.g. Netgear R8000
(BCM4709A0 + 3 x BCM43602) user was exepriencing firmware crashes that
never resulted in passing BRCMF_E_PSM_WATCHDOG to the host driver.
Thanks, Rafał
The PSM watchdog is actually not a firmware crash. It means the lower
part of the stack, which runs in the d11 core aka PSM, did not complete
its work in the required timing budget.
That made me implement this trivial software watchdog that simply checks
periodically if firmware still replies to the commands.
Luckily this patch DOES NOT seem to be needed anymore since the commit
8a3ab2f38f16 ("brcmfmac: trigger memory dump upon firmware halt
signal"). That change allows brcmfmac to detect firmware crashes
successfully.
It can still miss a firmware crash if it happens really soon. Those
crashes mostly happens when trying to load firmware for chip revision A
on chip with revision B due to differences in code contained in ROM.
Regards,
Arend
This patch is being posted for research/debugging purposes only. It
SHOULD NOT be applied until someone notices a firmware crash that
doesn't trigger the BRCMF_D2H_DEV_FWHALT signal.
Signed-off-by: Rafał Miłecki
---
Re: [PATCH] Add CYW89342 mini-PCIe device
O, and add "brcmfmac:" prefix in the subject. Regards, Arend
Re: [PATCH] Add CYW89342 mini-PCIe device
On 8/15/2018 11:06 AM, Chi-Hsien Lin wrote: From: Jia-Shyr Chuang CYW89342 is a 2x2 MIMO, 802.11a/b/g/n/ac, SDIO 3.0 and PCIe 3.0 for WLAN. It is a member of 4355/4359 family. So the device support SDIO, but this patch only adds the PCIe variant. The subject mentions that already, but mentioning SDIO in the commit message may confuse people. I would just drop the host interface references, ie. SDIO and PCIe from the commit message. Regards, Arend Signed-off-by: Jia-Shyr Chuang Signed-off-by: Chi-Hsien Lin --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 1 + 1 file changed, 1 insertion(+)
Re: [PATCH 2/5] staging: wilc1000: fixes for undefined reference to `__this_module' error
On 8/14/2018 12:56 PM, Dan Carpenter wrote: On Sun, Aug 12, 2018 at 05:48:52PM +0530, Ajay Singh wrote: Yes, currently only code to read and writing for "wilc_debug_level" exists. The main purpose for this file was to configure(enable/disable) specific level debug logs to print. The kernel's dev_dbg() macros are really very clever. You can enable them for one file or for a specific line or for a function. Provided you have CONFIG_DYNAMIC_DEBUG selected. Gr. AvS
Re: [PATCH] mac80211: Run TXQ teardown code before de-registering interfaces
On 8/13/2018 2:16 PM, Toke Høiland-Jørgensen wrote: The TXQ teardown code can reference the vif data structures that are stored in the netdev private memory area if there are still packets on the queue when it is being freed. Since the TXQ teardown code is run after the netdevs are freed, this can lead to a use-after-free. Fix this by moving the TXQ teardown code to earlier in ieee80211_unregister_hw(). Just off the bat, but from reading the above I am wondering whether the use-after-free could also happen upon removing an interface? Regards, Arend Reported-by: Ben Greear Tested-by: Ben Greear Signed-off-by: Toke Høiland-Jørgensen --- net/mac80211/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: wireless dongle causing entire machine to hang
On 8/8/2018 7:58 PM, Randy Oostdyk wrote: Good day all, Hi Randy I'm writing to report an issue with the linux kernel, and I'm hoping this is the right place to report it. (short aside: I tried to ask in the #linux-wireless IRC channel, but wasn't permitted to speak!) uhm. why? I'm aware that this is a USB protocol issue, and so this may be the wrong place to report the bug, but the warning seems to be generated by the wireless driver, and that appears to be the key issue here. My USB wifi dongle is on the end of a very long USB cable, and was connected to a USB hub. On two different occasions (after hours or days of use), the machine it was attached to (Raspberry Pi 3) stopped responding. I was unable to SSH in, even over ethernet. After a hard reboot, I found that the following error was repeated **many thousands of times per second** in three different log files: I assume that USB is powered, right? Now probably asking the obvious, but you did not write it down so here it is: did you try with a very short USB cable? I guess you read this already [1] Rpi kernel: [857011.857581] ieee80211 phy1: rt2800usb_tx_sta_fifo_read_completed: Warning - TX status read failed -71 So this -EPROTO. The usb host controller of the RPi3 can be found here: https://github.com/raspberrypi/linux/tree/rpi-4.18.y/drivers/usb/host/dwc_otg Not sure it that tells us anything though. Regards, Arend [1] https://www.raspberrypi.org/documentation/hardware/raspberrypi/usb/README.md
Re: [PATCH v2] brcmfmac: fix brcmf_wiphy_wowl_params() NULL pointer dereference
On 8/7/2018 3:38 PM, Chi-Hsien Lin wrote: From: Winnie Chang The kernel BUG happens when wowl is enabled from firmware. In brcmf_wiphy_wowl_params(), cfg is a NULL pointer because it is drvr->config returned from wiphy_to_cfg(), and drvr->config is not set yet. To fix it, set drvr->config before brcmf_setup_wiphy() which calls brcmf_wiphy_wowl_params(). The kernel panic is introduced in below commit: commit id: 856d5a011c86b59f6564be4508912fb1d866adfc brcmfmac: allocate struct brcmf_pub instance using wiphy_new() The above info should be in Fixes: tag, but maybe Kalle is willing to reformat it this time. Regards, Arend Signed-off-by: Winnie Chang Signed-off-by: Chi-Hsien Lin ---
Re: iw scanning broken?
+ linux-wireless On 8/5/2018 4:59 AM, Dan Kosek wrote: Arend, Thank you for getting back to me. Adding the “flush” variable to the command fixes the issue on both systems. Wow is the single channel scan fast with the flush. Is appears the full scan (all channels) is faster with the flush option as well. Glad it worked for you. I have attached my bash logs. No I have don’t have kernel logfiles. I would be glad to collect the kernel log files for you tomorrow. There is not really an issue here so no need. Can you provide me some instructions or a reference and I will forward them. (I am a wireless engineer my trade and learning to program now.) Welcome to the dark side :-p Getting the logs depends on you linux distro, but using dmesg tends to work on most of them. Regards, Arend
Re: [PATCH 5/6] qtnfmac: add support for PTA configuration
On 8/5/2018 5:22 PM, Sergey Matyukevich wrote:
Implement support for PTA (Packet Traffic Arbitration) configuration.
The PTA mechanism is used to coordinate sharing of the medium between
WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
This patch implements core infrastructure and vendor specific commands
to control PTA functionality in firmware.
And no description of the actual interface which would have helped with
the review.
Anyway, the vendor commands are pain and they just make me grumpy. The
original idea was that upstream drivers should not support them at all,
later we flexed the rules so that low level hardware specific interfaces
might be ok, for example we added one in wil6210.
If I would even consider applying a patch which adds a vendor command it
needs a really good commit log with a proper description of the actual
interface and good justifications why a generic nl80211 command won't
work. I don't see anything even remotely close here.
Sorry for being grumpy, I just hate these vendor commands. Especially
when I see that a generic nl80211 command has not even be consired at
all.
For what it is worth, looking at part of the patch:
+/**
+ * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
+ *
+ * @QLINK_PTA_MODE_DISABLED: PTA is disabled
+ * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
+ */
+enum qlink_pta_mode {
+ QLINK_PTA_MODE_DISABLED = 0,
+ QLINK_PTA_MODE_2_WIRE = 2
+};
+
it smells very much like low-level btcoex. The question is whether this
needs to be conveyed from user-space or should these be device
configuration, eg. like DT properties.
Hello Arend,
Yes, this is some kind of low-level BT coexistence mechanism, when WiFi and
BT cards coordinate access to wireless media in 2.4G using gpio lines.
Those lines connect WiFi and BT cards directly w/o host mediation.
Right.
As for DT properties, the qustion still remains the same: how to propagate
those settings to WiFi card. AFAIK there is no 'standard' interface for
this kind of things. So using vendor commands looked like the only option.
So DT properties are available to the kernel so it is just between
device driver and wifi card. There is no involvement with user-space needed.
Ok, makes sense. But IIUC this approach with DT
does not cover PCIe/USB wireless cards.
It can cover any device in the system regardless type of host interface.
For example in the ath10k bindings document [1] you see:
pci {
pcie@0 {
reg = <0 0 0 0 0>;
#interrupt-cells = <1>;
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
ath10k@0,0 {
reg = <0 0 0 0 0>;
device_type = "pci";
qcom,ath10k-calibration-data = [ 01 02 03 ... ];
};
};
};
Regards,
Arend
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt?h=linux-4.17.y
Re: [PATCH 5/6] qtnfmac: add support for PTA configuration
On 8/1/2018 10:23 AM, Sergey Matyukevich wrote:
Implement support for PTA (Packet Traffic Arbitration) configuration.
The PTA mechanism is used to coordinate sharing of the medium between
WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
This patch implements core infrastructure and vendor specific commands
to control PTA functionality in firmware.
And no description of the actual interface which would have helped with
the review.
Anyway, the vendor commands are pain and they just make me grumpy. The
original idea was that upstream drivers should not support them at all,
later we flexed the rules so that low level hardware specific interfaces
might be ok, for example we added one in wil6210.
If I would even consider applying a patch which adds a vendor command it
needs a really good commit log with a proper description of the actual
interface and good justifications why a generic nl80211 command won't
work. I don't see anything even remotely close here.
Sorry for being grumpy, I just hate these vendor commands. Especially
when I see that a generic nl80211 command has not even be consired at
all.
For what it is worth, looking at part of the patch:
+/**
+ * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
+ *
+ * @QLINK_PTA_MODE_DISABLED: PTA is disabled
+ * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
+ */
+enum qlink_pta_mode {
+ QLINK_PTA_MODE_DISABLED = 0,
+ QLINK_PTA_MODE_2_WIRE = 2
+};
+
it smells very much like low-level btcoex. The question is whether this
needs to be conveyed from user-space or should these be device
configuration, eg. like DT properties.
Hello Arend,
Yes, this is some kind of low-level BT coexistence mechanism, when WiFi and
BT cards coordinate access to wireless media in 2.4G using gpio lines.
Those lines connect WiFi and BT cards directly w/o host mediation.
Right.
As for DT properties, the qustion still remains the same: how to propagate
those settings to WiFi card. AFAIK there is no 'standard' interface for
this kind of things. So using vendor commands looked like the only option.
So DT properties are available to the kernel so it is just between
device driver and wifi card. There is no involvement with user-space needed.
Regards,
Arend
Re: iw scanning broken?
On 8/4/2018 3:08 PM, Dan Kosek wrote: If you enter the following command, you should get a single channel scan: sudo iw wlan0 scan freq 2412 You do not. Instead you get a full (all) channels scan, which takes 3-5 seconds. Do you have any logs to support that claim. Please note that the command returns all BSS-es that are listed in wireless subsystem, ie. in cfg80211. That includes BSS-es found in previous scans unless their lifetime expired. What happens if you do... $ sudo iw wlan0 scan freq 2412 flush This has been tested on Raspberry Pi Raspian 9 and Peppermint OS 7 ( Linux 4.4.0-53-generic #74-Ubuntu) with the same results. i believe iw is broken or some command knowledge is missing. So what drivers are involved. Please include kernel logs. Regards, Arend
Re: [PATCH 5/6] qtnfmac: add support for PTA configuration
On 7/30/2018 4:06 PM, Kalle Valo wrote:
Sergey Matyukevich writes:
From: Andrey Shevchenko
Implement support for PTA (Packet Traffic Arbitration) configuration.
The PTA mechanism is used to coordinate sharing of the medium between
WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
This patch implements core infrastructure and vendor specific commands
to control PTA functionality in firmware.
And no description of the actual interface which would have helped with
the review.
Anyway, the vendor commands are pain and they just make me grumpy. The
original idea was that upstream drivers should not support them at all,
later we flexed the rules so that low level hardware specific interfaces
might be ok, for example we added one in wil6210.
If I would even consider applying a patch which adds a vendor command it
needs a really good commit log with a proper description of the actual
interface and good justifications why a generic nl80211 command won't
work. I don't see anything even remotely close here.
Sorry for being grumpy, I just hate these vendor commands. Especially
when I see that a generic nl80211 command has not even be consired at
all.
For what it is worth, looking at part of the patch:
+/**
+ * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
+ *
+ * @QLINK_PTA_MODE_DISABLED: PTA is disabled
+ * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
+ */
+enum qlink_pta_mode {
+ QLINK_PTA_MODE_DISABLED = 0,
+ QLINK_PTA_MODE_2_WIRE = 2
+};
+
it smells very much like low-level btcoex. The question is whether this
needs to be conveyed from user-space or should these be device
configuration, eg. like DT properties.
Regards,
Arend
Re: [PATCH] brcmfmac: fix regression in parsing NVRAM for multiple devices
On 7/22/2018 11:46 PM, Rafał Miłecki wrote:
From: Rafał Miłecki
NVRAM is designed to work with Broadcom's SDK Linux kernel which fakes
PCI domain 0 for all internal MMIO devices. Since official Linux kernel
uses platform devices for that purpose there is a mismatch in numbering
PCI domains.
There used to be a fix for that problem but it was accidentally dropped
during the last firmware loading rework. That resulted in brcmfmac not
being able to extract device specific NVRAM content and all kind of
calibration problems.
Reported-by: Aditya Xavier
Fixes: 2baa3aaee27f ("brcmfmac: introduce brcmf_fw_alloc_request() function")
Cc: sta...@vger.kernel.org # v4.17+
oops. my bad.
Acked-by: Arend van Spriel
Signed-off-by: Rafał Miłecki
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 45928b5b8d97..4fffa6988087 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1785,7 +1785,8 @@ brcmf_pcie_prepare_fw_request(struct brcmf_pciedev_info
*devinfo)
fwreq->items[BRCMF_PCIE_FW_CODE].type = BRCMF_FW_TYPE_BINARY;
fwreq->items[BRCMF_PCIE_FW_NVRAM].type = BRCMF_FW_TYPE_NVRAM;
fwreq->items[BRCMF_PCIE_FW_NVRAM].flags = BRCMF_FW_REQF_OPTIONAL;
- fwreq->domain_nr = pci_domain_nr(devinfo->pdev->bus);
+ /* NVRAM reserves PCI domain 0 for Broadcom's SDK faked bus */
+ fwreq->domain_nr = pci_domain_nr(devinfo->pdev->bus) + 1;
fwreq->bus_nr = devinfo->pdev->bus->number;
return fwreq;
Re: [PATCH] brcmfmac: specify some features per firmware version
resending without HTML...
On 7/4/2018 10:31 PM, Rafał Miłecki wrote:
From: Rafał Miłecki
Some features supported by firmware aren't advertised and there is no
way for a driver to query them. This includes e.g. monitor mode details.
Most firmwares support monitor interface but only the latest ones
/announce/ it with a "monitor" flag in the "cap" iovar. There isn't any
reliable detection method for older firmwares (BRCMF_C_MONITOR was tried
but "it only indicates the core part of the stack supports").
Similarly support for tagging monitor frames and building radiotap
headers can't be reliably detected for all firmwares.
This commit adds table that allows mapping features to firmware version.
It adds mappings for 43602a1 and 4366b1 firmwares from
linux-firmware.git. Both were confirmed to be passing monitor frames.
Reviewed-by: Arend van Spriel
Signed-off-by: Rafał Miłecki
---
.../wireless/broadcom/brcm80211/brcmfmac/feature.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 4db4d07a..ab1d9eb1e9dc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -93,6 +93,42 @@ static int brcmf_feat_debugfs_read(struct seq_file *seq,
void *data)
}
#endif /* DEBUG */
+struct brcmf_feat_fwfeat {
+ const char * const fwid;
+ u32 feat_flags;
+};
+
+static const struct brcmf_feat_fwfeat brcmf_feat_fwfeat_map[] = {
+ /* brcmfmac43602-pcie.ap.bin from linux-firmware.git ea1178515b88 */
+ { "01-6cb8e269", BIT(BRCMF_FEAT_MONITOR) },
+ /* brcmfmac4366b-pcie.bin from linux-firmware.git 52442afee990 */
+ { "01-c47a91a4", BIT(BRCMF_FEAT_MONITOR) },
+};
Not sure if people will realize what the sha1 implies here. Can we just
drop the comments here?
Regards,
Arend
Re: [PATCH] nl80211/mac80211: Fix cfg80211_rx_control_port
Hi Denis,
I prefer the subject summarizes the issue, eg. "allow non-linear skb
data passed to cfg80211_rx_control_port".
On 7/3/2018 8:26 PM, Denis Kenzior wrote:
The current implementation of cfg80211_rx_control_port assumed that the
caller could provide a contiguous region of memory for the control port
frame to be sent up to userspace. Unfortunately, many drivers produce
non-linear skbs, especially for data frames. This resulted in userspace
getting notified of control port frames with correct metadata (from
address, port, etc) yet garbage / nonsense contents, resulting in bad
handshakes, disconnections, etc.
[snip]
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9ba1f289c439..94842c2a2f73 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5937,10 +5937,10 @@ void cfg80211_mgmt_tx_status(struct wireless_dev *wdev,
u64 cookie,
/**
* cfg80211_rx_control_port - notification about a received control port frame
* @dev: The device the frame matched to
- * @buf: control port frame
- * @len: length of the frame data
- * @addr: The peer from which the frame was received
- * @proto: frame protocol, typically PAE or Pre-authentication
+ * @skb: The skbuf with the control port frame. It is assumed that the skbuf
+ * is 802.3 formatted (with 802.3 header). The skb can be non-linear. This
+ * function does not take ownership of the skb, so the caller is responsible
+ * for any cleanup.
* @unencrypted: Whether the frame was received unencrypted
*
* This function is used to inform userspace about a received control port
@@ -5953,8 +5953,7 @@ void cfg80211_mgmt_tx_status(struct wireless_dev *wdev,
u64 cookie,
* Return: %true if the frame was passed to userspace
*/
bool cfg80211_rx_control_port(struct net_device *dev,
- const u8 *buf, size_t len,
- const u8 *addr, u16 proto, bool unencrypted);
+ struct sk_buff *skb, bool unencrypted);
If we are changing the signature, could we change the return type to
int. I don't see much value in the int-2-bool conversion.
/**
* cfg80211_cqm_rssi_notify - connection quality monitoring rssi event
[snip]
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8db59129c095..b75a0144c0a5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -15036,20 +15036,24 @@ void cfg80211_mgmt_tx_status(struct wireless_dev
*wdev, u64 cookie,
EXPORT_SYMBOL(cfg80211_mgmt_tx_status);
static int __nl80211_rx_control_port(struct net_device *dev,
-const u8 *buf, size_t len,
-const u8 *addr, u16 proto,
+struct sk_buff *skb,
bool unencrypted, gfp_t gfp)
{
struct wireless_dev *wdev = dev->ieee80211_ptr;
struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+ struct ethhdr *ehdr = eth_hdr(skb);
+ const u8 *addr = ehdr->h_source;
+ u16 proto = be16_to_cpu(skb->protocol);
So this means skb->protocol has to be set by driver (using
eth_type_trans() helper). Guess mac80211 does it for sure, but better
make it a clear requirement for cfg80211-based drivers.
struct sk_buff *msg;
void *hdr;
+ struct nlattr *frame;
+
u32 nlportid = READ_ONCE(wdev->conn_owner_nlportid);
if (!nlportid)
return -ENOENT;
- msg = nlmsg_new(100 + len, gfp);
+ msg = nlmsg_new(100 + skb->len, gfp);
if (!msg)
return -ENOMEM;
@@ -15063,13 +15067,17 @@ static int __nl80211_rx_control_port(struct
net_device *dev,
nla_put_u32(msg, NL80211_ATTR_IFINDEX, dev->ifindex) ||
nla_put_u64_64bit(msg, NL80211_ATTR_WDEV, wdev_id(wdev),
NL80211_ATTR_PAD) ||
- nla_put(msg, NL80211_ATTR_FRAME, len, buf) ||
nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, addr) ||
nla_put_u16(msg, NL80211_ATTR_CONTROL_PORT_ETHERTYPE, proto) ||
(unencrypted && nla_put_flag(msg,
NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT)))
goto nla_put_failure;
+ frame = nla_reserve(msg, NL80211_ATTR_FRAME, skb->len);
+ if (!frame)
+ goto nla_put_failure;
+
+ skb_copy_bits(skb, 0, nla_data(frame), skb->len);
genlmsg_end(msg, hdr);
return genlmsg_unicast(wiphy_net(&rdev->wiphy), msg, nlportid);
@@ -15080,14 +15088,12 @@ static int __nl80211_rx_control_port(struct
net_device *dev,
}
bool cfg80211_rx_control_port(struct net_device *dev,
- const u8 *buf, size_t len,
- const u8 *addr, u16 proto, bool unencrypted)
+ struct sk_buff *skb, bool unencrypted)
{
int ret;
- trace_cfg80211_rx_control_port(dev, buf, len,
Re: [PATCH] mac80211_hwsim: Add support for HE
On 6/30/2018 5:42 PM, kbuild test robot wrote:
2560 .he_mcs_nss_supp = {
>2561.rx_msc_80 = cpu_to_le16(0xfffa),
>2562.tx_msc_80 = cpu_to_le16(0xfffa),
>2563.rx_msc_160 = cpu_to_le16(0x),
>2564.tx_msc_160 = cpu_to_le16(0x),
>2565.rx_msc_80p80 = cpu_to_le16(0x),
>2566.tx_msc_80p80 = cpu_to_le16(0x),
2567 },
I believe I commented in earlier version that those fields in the type
defintion should be mcs iso msc and this initializer did not get fixed?
Regards,
Arend
Re: [PATCH] brcmfmac: update STA info struct to the v5
On 6/28/2018 12:36 PM, Rafał Miłecki wrote:
From: Rafał Miłecki
That struct is used when querying firmware for the STA. It seem is has
been changing during the time. Luckily its format seems to be backward
compatible starting with v2 (the only breakage was v1 -> v2).
The version that was supported by brcmfmac so far was v4. It was what
43602a1 and 4366b1 firmwares (7.35.177.56 and 10.10.69.3309 accordingly)
were using. It also seems to be used by early 4366c0 firmwares
(10.10.69.6908 and 10.10.69.69017).
The problem appears when switching to the 10.10.122.20 firmware. It uses
v5 and instead of falling back to v4 when submitted buffer isn't big
enough it fallbacks to the v3.
To receive all v4 specific info with the newest firmware we have to
submit a struct (buffer) that matches v5.
So do you have firmware that actually return v5 in the 'ver' field. I am
asking because I see a comment that version 5 is obsoleted and in recent
branch we are at version 7. Just curious what the actual version is. If
you do not explicitly need version 5 I would prefer to skip it and move
to version 6 struct. Otherwise it is...
Acked-by: Arend van Spriel
Signed-off-by: Rafał Miłecki
---
.../net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index 9b3a58e89dd1..d5bb81e88762 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -174,6 +174,8 @@
#define BRCMF_MFP_CAPABLE 1
#define BRCMF_MFP_REQUIRED2
+#define BRCMF_VHT_CAP_MCS_MAP_NSS_MAX 8
+
/* MAX_CHUNK_LEN is the maximum length for data passing to firmware in each
* ioctl. It is relatively small because firmware has small maximum size input
* playload restriction for ioctls.
@@ -550,6 +552,8 @@ struct brcmf_sta_info_le {
/* w/hi bit set if basic */
__le32 in; /* seconds elapsed since associated */
__le32 listen_interval_inms; /* Min Listen interval in ms for STA */
+
+ /* Fields valid for ver >= 3 */
__le32 tx_pkts; /* # of packets transmitted */
__le32 tx_failures; /* # of packets failed */
__le32 rx_ucast_pkts; /* # of unicast packets received */
@@ -558,6 +562,8 @@ struct brcmf_sta_info_le {
__le32 rx_rate; /* Rate of last successful rx frame */
__le32 rx_decrypt_succeeds; /* # of packet decrypted successfully */
__le32 rx_decrypt_failures; /* # of packet decrypted failed */
+
+ /* Fields valid for ver >= 4 */
__le32 tx_tot_pkts;/* # of tx pkts (ucast + mcast) */
__le32 rx_tot_pkts;/* # of data packets recvd (uni + mcast) */
__le32 tx_mcast_pkts; /* # of mcast pkts txed */
@@ -594,6 +600,14 @@ struct brcmf_sta_info_le {
*/
__le32 rx_pkts_retried;/* # rx with retry bit set */
__le32 tx_rate_fallback; /* lowest fallback TX rate */
+
+ /* Fields valid for ver >= 5 */
+ struct {
+ __le32 count; /* # rates in
this set */
+ u8 rates[BRCMF_MAXRATES_IN_SET];/* rates in
500kbps units w/hi bit set if basic */
+ u8 mcs[BRCMF_MCSSET_LEN]; /* supported
mcs index bit map */
+ __le16 vht_mcs[BRCMF_VHT_CAP_MCS_MAP_NSS_MAX]; /* supported
mcs index bit map per nss */
+ } rateset_adv;
};
struct brcmf_chanspec_list {
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
On 6/25/2018 6:40 AM, Rafał Miłecki wrote: On Sun, 24 Jun 2018 at 13:48, Rafał Miłecki wrote: On Fri, 22 Jun 2018 at 20:45, Arend van Spriel wrote: Here a couple of patches in preparation of monitor mode support. It is mostly about querying firmware for support. While at it I stumbled upon the fact that 160MHz was not completely implemented in the driver so there is a patch for that as well. The first two patches are actually some changes to the patches that Rafal submitted. So this series depend on: [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1] These apply to the master branch of the wireless-drivers-next repository. [1] https://patchwork.kernel.org/patch/10474799/ I find it pointless to submit fixes for patches that weren't accepted yet. Let's work on improving original patches while they are still pending. I sent V4 with changes pointed our by Arend. That obsoletes all (?) monitor patches from this patchset. I believe it was cleaner to improve original patchset (not pushed yet). My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor related) and drop the others. Well. Given that Kalle prefers applying "all-or-nothing" I will resubmit the series addressing the issues you found. Regards, Arend
Re: [PATCH V3 0/2] brcmfmac: initial work for adding monitor mode
On 6/24/2018 4:20 PM, Rafał Miłecki wrote: On Fri, 22 Jun 2018 at 20:59, Arend van Spriel wrote: On 6/19/2018 10:25 PM, Rafał Miłecki wrote: On 2018-06-19 22:01, Arend van Spriel wrote: On 6/19/2018 5:48 PM, Rafał Miłecki wrote: From: Rafał Miłecki After a bit long discussions in various e-mail threads I'm coming with this simple & small patchset. It isn't complete support for monitor mode but just a pair of preparing patches that should be clear & well discussed by now to make them acceptable. The main missing bit is code setting MONITOR_FMT_RADIOTAP which I expect Arend to handle soon, as he already has a patch using "sta_monitor" iovar for that. Then we have to discuss a flag for marking firmwares which are capable for tagging monitor frames. While still incomplete, I believe that with my previous patches, we can agree this is a good direction. Arend: if you find these 2 patches OK, could you ack them, to make it clear for Kalle if they look OK now (or not yet)? I'd be great if you could sent your "sta_monitor" work on top of this. I acked them and I will submit my changes later. Either after these are applied or simply indicate the dependency. Now as for where we are with this. With what we have here we know firmware can monitor packets with and without radiotap. However, we do not have an indication whether firmware can transport these monitor packets to the host. What I need to look into next is whether the 802.11 flag in msgbuf is linked to a particular version of the protocol, but we may need to resort to the fwid table. Being able to detect if firmware tags monitor packets would be great. The 802.11 tag as opposed the 802.3 tag is specified in the PCIe host interface spec. The brcmfmac driver support rev 5 and 6 of that spec and both specify the tags. I did some digging and I believe it is safe to say that firmware that includes the "monitor" tag in the "cap" iovar does support these packets tags as well. OK, this is nice. The problem is we still need some fallback for the old firmwares. Only the newest ones specify "monitor" tag in the "cap" iovar. Older firmwares don't specify it but they still may include support for monitor mode and radiotap header. So in your list of firmwares, do you know (or can you find out :-p ) which ones return "monitor" and which do not? My educated guess was that only firmwares returning "monitor" tag have host-interface code in place to send monitor packets up to the host. You may prove me wrong. Regards, Arend
Re: [PATCH 6/6] brcmfmac: fallback mechanism to determine monitor mode features
On 6/24/2018 4:08 PM, Rafał Miłecki wrote: On Fri, 22 Jun 2018 at 20:45, Arend van Spriel wrote: Firmwares may not provide all monitor mode features in the "cap" iovar. For those this fallback mechanism uses "sta_monitor" iovar. If firmware is compiled with stamon, this iovar will fail with BCME_NOTUP; Otherwise it fails with BCME_UNSUPPORTED. It's probably not the first time ever, but it appears your research (theory) doesn't match my experience (practice) ;) I'm afraid you missed some important check when analyzing firmware code. It was not all theory ;-) but apparently I did not cover all bases. I only checked with 4366c0 (actually with 43664 aka 4366E) on the release branch I am working on. I've just tested all firmwares I got (for 43602a1, 4366b1 and 4366c0) and all of them return -4 (BCME_NOTUP) for "sta_monitor" when firmware/interface is down. It appears this test requires bringing firmware/interface up to make it reliable. Apparently even firmwares *without* sta_monitor return -4 (BCME_NOTUP) when firmware/interface is down. That is crap. So back to the drawing board. Thanks for keeping the taps on this. Regards, Arend
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
On 6/24/2018 1:48 PM, Rafał Miłecki wrote: On Fri, 22 Jun 2018 at 20:45, Arend van Spriel wrote: Here a couple of patches in preparation of monitor mode support. It is mostly about querying firmware for support. While at it I stumbled upon the fact that 160MHz was not completely implemented in the driver so there is a patch for that as well. The first two patches are actually some changes to the patches that Rafal submitted. So this series depend on: [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1] These apply to the master branch of the wireless-drivers-next repository. [1] https://patchwork.kernel.org/patch/10474799/ I find it pointless to submit fixes for patches that weren't accepted yet. Let's work on improving original patches while they are still pending. It is not unprecedented. There are enough patch series sent in the past that relied on other still pending series. I was unsure how to deal with it as I acked your changes already and did not exactly know when Kalle would start applying the patches. I am fine either way. Regards, Arend
Re: [PATCH V3 0/2] brcmfmac: initial work for adding monitor mode
On 6/19/2018 10:25 PM, Rafał Miłecki wrote: On 2018-06-19 22:01, Arend van Spriel wrote: On 6/19/2018 5:48 PM, Rafał Miłecki wrote: From: Rafał Miłecki After a bit long discussions in various e-mail threads I'm coming with this simple & small patchset. It isn't complete support for monitor mode but just a pair of preparing patches that should be clear & well discussed by now to make them acceptable. The main missing bit is code setting MONITOR_FMT_RADIOTAP which I expect Arend to handle soon, as he already has a patch using "sta_monitor" iovar for that. Then we have to discuss a flag for marking firmwares which are capable for tagging monitor frames. While still incomplete, I believe that with my previous patches, we can agree this is a good direction. Arend: if you find these 2 patches OK, could you ack them, to make it clear for Kalle if they look OK now (or not yet)? I'd be great if you could sent your "sta_monitor" work on top of this. I acked them and I will submit my changes later. Either after these are applied or simply indicate the dependency. Now as for where we are with this. With what we have here we know firmware can monitor packets with and without radiotap. However, we do not have an indication whether firmware can transport these monitor packets to the host. What I need to look into next is whether the 802.11 flag in msgbuf is linked to a particular version of the protocol, but we may need to resort to the fwid table. Being able to detect if firmware tags monitor packets would be great. The 802.11 tag as opposed the 802.3 tag is specified in the PCIe host interface spec. The brcmfmac driver support rev 5 and 6 of that spec and both specify the tags. I did some digging and I believe it is safe to say that firmware that includes the "monitor" tag in the "cap" iovar does support these packets tags as well. Unfortunately, the BRCMF_C_MONITOR command support does not guarantee. So I just sent a patch to remove the fall-back mechanism for detecting monitor mode support. Regards, Arend
[PATCH 3/6] brcmfmac: fix for proper support of 160MHz bandwidth
Decoding of firmware channel information was not complete for 160MHz
support. This resulted in the following warning:
WARNING: CPU: 2 PID: at .../broadcom/brcm80211/brcmutil/d11.c:196
brcmu_d11ac_decchspec+0x2e/0x100 [brcmutil]
Modules linked in: brcmfmac(O) brcmutil(O) sha256_generic cfg80211 ...
CPU: 2 PID: Comm: kworker/2:0 Tainted: G O
4.17.0-wt-testing-x64-2-gf1bed50 #1
Hardware name: Dell Inc. Latitude E6410/07XJP9, BIOS A07 02/15/2011
Workqueue: events request_firmware_work_func
RIP: 0010:brcmu_d11ac_decchspec+0x2e/0x100 [brcmutil]
RSP: 0018:c9047bd0 EFLAGS: 00010206
RAX: e832 RBX: 8801146fe910 RCX: 8801146fd3c0
RDX: 2800 RSI: 0070 RDI: c9047c30
RBP: c9047bd0 R08: R09: a0798c80
R10: 88012bca55e0 R11: 880110a4ea00 R12: 8801146f8000
R13: c9047c30 R14: 8801146fe930 R15: 8801138e02e0
FS: () GS:88012bc8() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 7f18ce8b8070 CR3: 0200a003 CR4: 000206e0
Call Trace:
brcmf_setup_wiphybands+0x212/0x780 [brcmfmac]
brcmf_cfg80211_attach+0xae2/0x11a0 [brcmfmac]
brcmf_attach+0x1fc/0x4b0 [brcmfmac]
? __kmalloc+0x13c/0x1c0
brcmf_pcie_setup+0x99b/0xe00 [brcmfmac]
brcmf_fw_request_done+0x16a/0x1f0 [brcmfmac]
request_firmware_work_func+0x36/0x60
process_one_work+0x146/0x350
worker_thread+0x4a/0x3b0
kthread+0x102/0x140
? process_one_work+0x350/0x350
? kthread_bind+0x20/0x20
ret_from_fork+0x35/0x40
Code: 66 90 0f b7 07 55 48 89 e5 89 c2 88 47 02 88 47 03 66 81 e2 00 38
66 81 fa 00 18 74 6e 66 81 fa 00 20 74 39 66 81 fa 00 10 74 14 <0f>
0b 66 25 00 c0 74 20 66 3d 00 c0 75 20 c6 47 04 01 5d c3 66
---[ end trace 550c46682415b26d ]---
brcmfmac: brcmf_construct_chaninfo: Ignoring unexpected firmware channel 50
This patch adds the missing stuff to properly handle this.
Reviewed-by: Hante Meuleman
Reviewed-by: Pieter-Paul Giesberts
Reviewed-by: Franky Lin
Signed-off-by: Arend van Spriel
---
.../net/wireless/broadcom/brcm80211/brcmutil/d11.c | 34 +-
.../broadcom/brcm80211/include/brcmu_wifi.h| 2 ++
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
index d8b79cb..e7584b8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
@@ -77,6 +77,8 @@ static u16 d11ac_bw(enum brcmu_chan_bw bw)
return BRCMU_CHSPEC_D11AC_BW_40;
case BRCMU_CHAN_BW_80:
return BRCMU_CHSPEC_D11AC_BW_80;
+ case BRCMU_CHAN_BW_160:
+ return BRCMU_CHSPEC_D11AC_BW_160;
default:
WARN_ON(1);
}
@@ -190,8 +192,38 @@ static void brcmu_d11ac_decchspec(struct brcmu_chan *ch)
break;
}
break;
- case BRCMU_CHSPEC_D11AC_BW_8080:
case BRCMU_CHSPEC_D11AC_BW_160:
+ switch (ch->sb) {
+ case BRCMU_CHAN_SB_LLL:
+ ch->control_ch_num -= CH_70MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_LLU:
+ ch->control_ch_num -= CH_50MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_LUL:
+ ch->control_ch_num -= CH_30MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_LUU:
+ ch->control_ch_num -= CH_10MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_ULL:
+ ch->control_ch_num += CH_10MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_ULU:
+ ch->control_ch_num += CH_30MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_UUL:
+ ch->control_ch_num += CH_50MHZ_APART;
+ break;
+ case BRCMU_CHAN_SB_UUU:
+ ch->control_ch_num += CH_70MHZ_APART;
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ break;
+ }
+ break;
+ case BRCMU_CHSPEC_D11AC_BW_8080:
default:
WARN_ON_ONCE(1);
break;
diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
index 7b9a779..75b2a04 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
@@ -29,6 +29,8 @@
#define CH_UPPER_SB0x01
#define CH_LOWER_SB
[PATCH 4/6] brcmfmac: increase buffer for obtaining firmware capabilities
When obtaining the firmware capability a buffer is provided of 512
bytes. However, if all features in firmware are supported the buffer
needs to be 565 bytes as otherwise truncated information is retrieved
from firmware. Increasing the buffer to 768 bytes on stack.
Reviewed-by: Hante Meuleman
Reviewed-by: Pieter-Paul Giesberts
Reviewed-by: Franky Lin
Signed-off-by: Arend van Spriel
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index a78b9ba..0ee3a25 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -141,7 +141,7 @@ static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
ifp->fwil_fwerr = false;
}
-#define MAX_CAPS_BUFFER_SIZE 512
+#define MAX_CAPS_BUFFER_SIZE 768
static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
{
char caps[MAX_CAPS_BUFFER_SIZE];
--
1.9.1
[PATCH 2/6] brcmfmac: rename BRCMF_FEAT_MONITOR_FMT_RADIOTAP to BRCMF_FEAT_RADIOTAP
Just a bit of bike-shedding so nothing functionally changed.
Signed-off-by: Arend van Spriel
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c| 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index b1f702f..d3fdbb3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -407,7 +407,7 @@ void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff
*skb)
void brcmf_netif_mon_rx(struct brcmf_if *ifp, struct sk_buff *skb)
{
- if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR_FMT_RADIOTAP)) {
+ if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_RADIOTAP)) {
/* Do nothing */
} else {
struct ieee80211_radiotap_header *radiotap;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
index 0782f2a..31a746b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
@@ -34,7 +34,7 @@
* GSCAN: enhanced scan offload feature.
* FWSUP: Firmware supplicant.
* MONITOR: monitor interface
- * MONITOR_FMT_RADIOTAP: monitor packets include radiotap header
+ * RADIOTAP: monitor packets include radiotap header
*/
#define BRCMF_FEAT_LIST \
BRCMF_FEAT_DEF(MBSS) \
@@ -52,7 +52,7 @@
BRCMF_FEAT_DEF(GSCAN) \
BRCMF_FEAT_DEF(FWSUP) \
BRCMF_FEAT_DEF(MONITOR) \
- BRCMF_FEAT_DEF(MONITOR_FMT_RADIOTAP)
+ BRCMF_FEAT_DEF(RADIOTAP)
/*
* Quirks:
--
1.9.1
[PATCH 6/6] brcmfmac: fallback mechanism to determine monitor mode features
Firmwares may not provide all monitor mode features in the "cap" iovar.
For those this fallback mechanism uses "sta_monitor" iovar. If firmware
is compiled with stamon, this iovar will fail with BCME_NOTUP; Otherwise
it fails with BCME_UNSUPPORTED.
Reviewed-by: Hante Meuleman
Reviewed-by: Pieter-Paul Giesberts
Reviewed-by: Franky Lin
Signed-off-by: Arend van Spriel
---
.../wireless/broadcom/brcm80211/brcmfmac/feature.c | 15 +++
.../broadcom/brcm80211/brcmfmac/fwil_types.h| 21 +
2 files changed, 36 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index f70fec6..cb57a4a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -207,6 +207,7 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
struct brcmf_pno_macaddr_le pfn_mac;
struct brcmf_gscan_config gscan_cfg;
+ struct brcmf_stamon_sta_config stamon_cfg;
u32 wowl_cap;
s32 err;
@@ -217,6 +218,20 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
brcmf_feat_iovar_data_set(ifp, BRCMF_FEAT_GSCAN,
"pfn_gscan_cfg",
&gscan_cfg, sizeof(gscan_cfg));
+
+ if (!brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR) ||
+ !brcmf_feat_is_enabled(ifp, BRCMF_FEAT_RADIOTAP)) {
+ ifp->fwil_fwerr = true;
+ memset(&stamon_cfg, 0, sizeof(stamon_cfg));
+ /* fails either way as firmware stack is not up yet */
+ err = brcmf_fil_iovar_data_set(ifp, "sta_monitor", &stamon_cfg,
+ sizeof(stamon_cfg));
+ if (err != BRCMF_FW_UNSUPPORTED) {
+ ifp->drvr->feat_flags |= BIT(BRCMF_FEAT_MONITOR);
+ ifp->drvr->feat_flags |= BIT(BRCMF_FEAT_RADIOTAP);
+ }
+ ifp->fwil_fwerr = false;
+ }
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_PNO, "pfn");
if (drvr->bus_if->wowl_supported)
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_WOWL, "wowl");
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index 4b29070..db56c81 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -963,4 +963,25 @@ struct brcmf_gscan_config {
struct brcmf_gscan_bucket_config bucket[1];
};
+/**
+ * struct brcmf_stamon_sta_config - configuration data for sta monitor.
+ *
+ * @cmd: subcommand for this configuration.
+ * @mac: mac address of STA for which @cmd is intended.
+ * @version: version of this configuration structure.
+ * @length: number of bytes following this field.
+ * @chanspec: channel of the STA.
+ * @mon_time: time for which STA's are monitored (ms).
+ * @offchan_time: timer for which off-channel STA's are monitored.
+ */
+struct brcmf_stamon_sta_config {
+ __le32 cmd;
+ u8 mac[ETH_ALEN];
+ __le16 version;
+ __le16 length;
+ __le16 chanspec;
+ __le32 monitor_time;
+ __le32 offchan_time;
+};
+
#endif /* FWIL_TYPES_H_ */
--
1.9.1
[PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
Here a couple of patches in preparation of monitor mode support. It is mostly about querying firmware for support. While at it I stumbled upon the fact that 160MHz was not completely implemented in the driver so there is a patch for that as well. The first two patches are actually some changes to the patches that Rafal submitted. So this series depend on: [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1] These apply to the master branch of the wireless-drivers-next repository. [1] https://patchwork.kernel.org/patch/10474799/ Arend van Spriel (6): brcmfmac: remove fallback mechanism for BRCMF_FEAT_MONITOR brcmfmac: rename BRCMF_FEAT_MONITOR_FMT_RADIOTAP to BRCMF_FEAT_RADIOTAP brcmfmac: fix for proper support of 160MHz bandwidth brcmfmac: increase buffer for obtaining firmware capabilities brcmfmac: add new feature flags for monitor mode operation brcmfmac: fallback mechanism to determine monitor mode features .../wireless/broadcom/brcm80211/brcmfmac/core.c| 2 +- .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 43 +- .../wireless/broadcom/brcm80211/brcmfmac/feature.h | 6 +-- .../broadcom/brcm80211/brcmfmac/fwil_types.h | 21 +++ .../net/wireless/broadcom/brcm80211/brcmutil/d11.c | 34 - .../broadcom/brcm80211/include/brcmu_wifi.h| 2 + 6 files changed, 77 insertions(+), 31 deletions(-) -- 1.9.1
[PATCH 5/6] brcmfmac: add new feature flags for monitor mode operation
Add two new feature flags to be used for monitor mode. One indicating
support to enable packet capture and one indicating captured packets
from firmware already include radiotap header.
Signed-off-by: Rafal Milecki
[arend: change flags and description]
Reviewed-by: Hante Meuleman
Reviewed-by: Pieter-Paul Giesberts
Reviewed-by: Franky Lin
Signed-off-by: Arend van Spriel
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 1 +
drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 0ee3a25..f70fec6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -49,6 +49,7 @@ struct brcmf_feat_fwcap {
{ BRCMF_FEAT_MCHAN, "mchan" },
{ BRCMF_FEAT_P2P, "p2p" },
{ BRCMF_FEAT_MONITOR, "monitor" },
+ { BRCMF_FEAT_RADIOTAP, "rtap" },
};
#ifdef DEBUG
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
index 31a746b..31198d0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
@@ -33,8 +33,8 @@
* MFP: 802.11w Management Frame Protection.
* GSCAN: enhanced scan offload feature.
* FWSUP: Firmware supplicant.
- * MONITOR: monitor interface
- * RADIOTAP: monitor packets include radiotap header
+ * MONITOR: firmware can pass monitor packets to host.
+ * RADIOTAP: firmware provides monitor packets including radiotap header
*/
#define BRCMF_FEAT_LIST \
BRCMF_FEAT_DEF(MBSS) \
--
1.9.1
[PATCH 1/6] brcmfmac: remove fallback mechanism for BRCMF_FEAT_MONITOR
Commit 19f505aa3ae0 ("brcmfmac: detect firmware support for monitor
interface") introduced a fallback mechanism attempting BRCMF_C_MONITOR
command. However, that does not indicate firmware support. Unfortunately,
it only indicates the core part of the stack supports it, but not whether
it is supported in full-dongle mode, ie. firmware has means to transfer
the monitor packets to the host. Firmwares that return the "monitor"
flag in the "cap" iovar are able to send the packets to the host.
Signed-off-by: Arend van Spriel
---
.../wireless/broadcom/brcm80211/brcmfmac/feature.c | 25 --
1 file changed, 25 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 48d7978..a78b9ba 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -92,26 +92,6 @@ static int brcmf_feat_debugfs_read(struct seq_file *seq,
void *data)
}
#endif /* DEBUG */
-static void brcmf_feat_cmd_int_get(struct brcmf_if *ifp, enum brcmf_feat_id id,
- u32 cmd)
-{
- u32 data;
- int err;
-
- ifp->fwil_fwerr = true;
-
- err = brcmf_fil_cmd_int_get(ifp, cmd, &data);
- if (err == 0) {
- brcmf_dbg(INFO, "enabling feature: %s\n", brcmf_feat_names[id]);
- ifp->drvr->feat_flags |= BIT(id);
- } else {
- brcmf_dbg(TRACE, "%s feature check failed: %d\n",
- brcmf_feat_names[id], err);
- }
-
- ifp->fwil_fwerr = false;
-}
-
/**
* brcmf_feat_iovar_int_get() - determine feature through iovar query.
*
@@ -272,11 +252,6 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
}
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_FWSUP, "sup_wpa");
- /* Fallback detection for older firmwares */
- if (!brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR))
- brcmf_feat_cmd_int_get(ifp, BRCMF_FEAT_MONITOR,
- BRCMF_C_GET_MONITOR);
-
/* set chip related quirks */
switch (drvr->bus_if->chip) {
case BRCM_CC_43236_CHIP_ID:
--
1.9.1
Re: [PATCH] mac80211: Fix oops in ieee80211_tx_control_port
On 6/19/2018 5:39 PM, Denis Kenzior wrote: On pre-emption enabled kernels the following oops was being seen due to missing local_bh_disable/local_bh_enable calls. mac80211 assumes that pre-emption is disabled in the data path. No sure if "assumes" is the right term here. It seems like it is required and there is probably a good reason for that. Would be nice to know what that reason is. Regards, Arend
Re: [PATCH] mac80211: Fix oops in ieee80211_tx_control_port
On 6/20/2018 9:53 AM, Johannes Berg wrote: On Wed, 2018-06-20 at 09:51 +0200, Arend van Spriel wrote: On 6/19/2018 5:39 PM, Denis Kenzior wrote: On pre-emption enabled kernels the following oops was being seen due to missing local_bh_disable/local_bh_enable calls. mac80211 assumes that pre-emption is disabled in the data path. No sure if "assumes" is the right term here. It seems like it is required and there is probably a good reason for that. Would be nice to know what that reason is. It's using per-CPU data. Hence the smp_processor_id() call which Denis mentioned in the bug report? Regards, Arend
Re: [PATCH V3 0/2] brcmfmac: initial work for adding monitor mode
On 6/19/2018 5:48 PM, Rafał Miłecki wrote: From: Rafał Miłecki After a bit long discussions in various e-mail threads I'm coming with this simple & small patchset. It isn't complete support for monitor mode but just a pair of preparing patches that should be clear & well discussed by now to make them acceptable. The main missing bit is code setting MONITOR_FMT_RADIOTAP which I expect Arend to handle soon, as he already has a patch using "sta_monitor" iovar for that. Then we have to discuss a flag for marking firmwares which are capable for tagging monitor frames. While still incomplete, I believe that with my previous patches, we can agree this is a good direction. Arend: if you find these 2 patches OK, could you ack them, to make it clear for Kalle if they look OK now (or not yet)? I'd be great if you could sent your "sta_monitor" work on top of this. I acked them and I will submit my changes later. Either after these are applied or simply indicate the dependency. Now as for where we are with this. With what we have here we know firmware can monitor packets with and without radiotap. However, we do not have an indication whether firmware can transport these monitor packets to the host. What I need to look into next is whether the 802.11 flag in msgbuf is linked to a particular version of the protocol, but we may need to resort to the fwid table. Regards, Arend
Re: [PATCH V3 2/2] brcmfmac: handle monitor mode marked msgbuf packets
On 6/19/2018 5:48 PM, Rafał Miłecki wrote: From: Rafał Miłecki New Broadcom firmwares mark monitor mode packets using a newly defined bit in the flags field. Use it to filter them out and pass to the monitor interface. These defines were found in bcmmsgbuf.h from SDK. As not every firmware generates radiotap header this commit introduces BRCMF_FEAT_MONITOR_FMT_RADIOTAP flag. It has to be has based on firmware capabilities. If not present brcmf_netif_mon_rx() will assume packet is a raw 802.11 frame and will prepend it with an empty radiotap header. This new code is limited to the msgbuf protocol at this point. Adding support for SDIO/USB devices will require some extra work (possibly a new firmware release). Acked-by: Arend van Spriel Signed-off-by: Rafał Miłecki --- V2: Use cpu_to_le16 when setting it_len V3: Update TODO comments Rename flag (after adding MONITOR) Update commit message --- .../wireless/broadcom/brcm80211/brcmfmac/core.c| 25 ++ .../wireless/broadcom/brcm80211/brcmfmac/core.h| 2 ++ .../wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 +++- .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 17 +++ 4 files changed, 47 insertions(+), 1 deletion(-)
Re: [PATCH V3 1/2] brcmfmac: detect firmware support for monitor interface
On 6/19/2018 5:48 PM, Rafał Miłecki wrote: From: Rafał Miłecki Most of firmwares support creating monitor interface. The newest ones explicitly /announce/ it using a "monitor" entry in the list of capabilities. Check (and store) internally info about monitor mode support using a new feature flag. Once we sort out all details of handling monitor interface it will be used when reporting available interfaces to the cfg80211. Acked-by: Arend van Spriel Signed-off-by: Rafał Miłecki --- V3: Patch added to the series --- .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 26 ++ .../wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 +++- .../wireless/broadcom/brcm80211/brcmfmac/fwil.h| 2 ++ 3 files changed, 31 insertions(+), 1 deletion(-)
Re: Research + questions on brcmfmac and support for monitor mode
On 6/19/2018 10:32 AM, Rafał Miłecki wrote:
On 19.06.2018 09:53, Arend van Spriel wrote:
On 6/19/2018 9:27 AM, Rafał Miłecki wrote:
On Mon, 11 Jun 2018 at 12:48, Arend van Spriel
wrote:
On 5/30/2018 1:52 PM, Rafał Miłecki wrote:
I'm providing extra version info of tested firmware images as
requested
by Arend in another e-mail thread.
Looking into our firmware repo it there are two flags, ie. WL_MONITOR
and WL_RADIOTAP. It seems both are set for firmware containing -stamon-
feature. Your list below confirms that. I still plan to add indication
for WL_RADIOTAP in the "cap" iovar, but a stamon feature check could be
used for older firmwares.
I just checked wl.mk (it's an open source file) and found following
line:
WLFILES_SRC_HI += src/wl/sys/wlc_stamon.c
not guarded by any ifeq.
wl.mk is used for NIC driver (softmac) so it is not used for fullmac
firmware.
Weird. I see a few rte references in wl.mk which suggests it's used for
both (softmac & fullmac firmware).
yeah. my mistake.
It appears wlc_stamon.c is always being compiled in. Are you 100% sure
that wlc_stamon.c depends & uses radiotap? Are you sure it's
impossible to include stamon support without radiotap support?
I'm asking because we're going to check "sta_monitor" iovar to find
out if radiotap support is included. I'd like to be sure it's 100%
reliable.
I have already created a patch for this (see below). I will submit it
this week.
Just to be clear could you also answer my above question, please?
Did you check if it's impossible to build firmware *with* stamon.c (and
sta_monitor iovar) and *without WL_RADIOTAP?
Yes. The functions in stamon.c, most importantly wlc_stamon_attach, are
only invoked in firmware when WL_STA_MONITOR is defined.
diff --git
a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index f70fec6..67d7fc7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -207,6 +207,7 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
struct brcmf_pno_macaddr_le pfn_mac;
struct brcmf_gscan_config gscan_cfg;
+struct brcmf_stamon_sta_config stamon_cfg;
u32 wowl_cap;
s32 err;
@@ -217,6 +218,20 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
brcmf_feat_iovar_data_set(ifp, BRCMF_FEAT_GSCAN,
"pfn_gscan_cfg",
&gscan_cfg, sizeof(gscan_cfg));
+
+if (!brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR) ||
+!brcmf_feat_is_enabled(ifp, BRCMF_FEAT_RADIOTAP)) {
+ifp->fwil_fwerr = true;
+memset(&stamon_cfg, 0, sizeof(stamon_cfg));
+/* iovar requires IOVF_SET_UP so this fails either way */
+err = brcmf_fil_iovar_data_set(ifp, "sta_monitor", &stamon_cfg,
+ sizeof(stamon_cfg));
I think it may be simpler (and maybe less invasive) to use
brcmf_fil_iovar_data_get.
Thanks for the comment, but looking at the firmware I can not concur. In
the get-path there is yet another compile flag that requires different
query for different firmwares. For the set there is a prerequisite that
the firmware stack is up and we know it is not upon executing
brcmf_feat_attach() so it fails before entering the specific iovar
handler in firmware.
Regards,
Arend
