Re: [PATCH 1/3] brcmfmac: move "cfg80211_ops" pointer to another struct
On Tue, 3 Sep 2019 at 20:59, Arend Van Spriel wrote: > On 9/3/2019 6:29 AM, Rafał Miłecki wrote: > > From: Rafał Miłecki > > > > This moves "ops" pointer from "struct brcmf_cfg80211_info" to the > > "struct brcmf_pub". This movement makes it possible to allocate wiphy > > without attaching cfg80211 (brcmf_cfg80211_attach()). It's required for > > later separation of wiphy allocation and driver initialization. > > > > While at it fix also an unlikely memory leak in the brcmf_attach(). > > Always good ;-) > > I recall there is some fiddling with the callback ops in cfg80211.c. Is > that broken by this reorg. Need to look into that. I don't see how this patch could break that. It still calls brcmf_cfg80211_get_ops() and passes settings as an argument. -- Rafał
Re: [PATCH 0/3] brcmfmac: keep wiphy during PCIe driver lifetime
On Tue, 3 Sep 2019 at 20:51, Arend Van Spriel wrote: > On 9/3/2019 6:29 AM, Rafał Miłecki wrote: > > From: Rafał Miłecki > > > > Driver's main init/attach function brcmf_attach() was handling both: > > wiphy allocation and driver initialization. It meant getting a new wiphy > > on every init (initial, resume, error recovery). > > > > For supplicants/authenticators and Linux users it's move convenient to > > have the same wiphy over driver's lifetime. It allows e.g. automatic > > recovery after a firmware crash. > > Typo: 'move' should be 'more'. Well, it's just a cover letter (0/3), that message won't go anywhere to the git repository. -- Rafał
[PATCH 3/3] brcmfmac: don't realloc wiphy during PCIe reset
From: Rafał Miłecki Providing a new wiphy on every PCIe reset was confusing and was causing configuration problems for some users (supplicant and authenticators). Sticking to the existing wiphy should make error recovery much simpler and more reliable. Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index b01b33e99c14..6c463475e90b 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -1430,7 +1430,6 @@ static int brcmf_pcie_reset(struct device *dev) brcmf_pcie_bus_console_read(devinfo, true); brcmf_detach(dev); - brcmf_free(dev); brcmf_pcie_release_irq(devinfo); brcmf_pcie_release_scratchbuffers(devinfo); @@ -1826,9 +1825,6 @@ static void brcmf_pcie_setup(struct device *dev, int ret, brcmf_pcie_intr_enable(devinfo); brcmf_pcie_hostready(devinfo); - ret = brcmf_alloc(&devinfo->pdev->dev, devinfo->settings); - if (ret) - goto fail; ret = brcmf_attach(&devinfo->pdev->dev); if (ret) goto fail; @@ -1931,6 +1927,10 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) bus->wowl_supported = pci_pme_capable(pdev, PCI_D3hot); dev_set_drvdata(&pdev->dev, bus); + ret = brcmf_alloc(&devinfo->pdev->dev, devinfo->settings); + if (ret) + goto fail_bus; + fwreq = brcmf_pcie_prepare_fw_request(devinfo); if (!fwreq) { ret = -ENOMEM; -- 2.21.0
[PATCH 1/3] brcmfmac: move "cfg80211_ops" pointer to another struct
From: Rafał Miłecki This moves "ops" pointer from "struct brcmf_cfg80211_info" to the "struct brcmf_pub". This movement makes it possible to allocate wiphy without attaching cfg80211 (brcmf_cfg80211_attach()). It's required for later separation of wiphy allocation and driver initialization. While at it fix also an unlikely memory leak in the brcmf_attach(). Signed-off-by: Rafał Miłecki --- .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 1 - .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h | 1 - drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 9 ++--- drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h | 1 + 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 581d0013f33e..c476f854f3ae 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -7210,7 +7210,6 @@ void brcmf_cfg80211_detach(struct brcmf_cfg80211_info *cfg) brcmf_pno_detach(cfg); brcmf_btcoex_detach(cfg); wiphy_unregister(cfg->wiphy); - kfree(cfg->ops); wl_deinit_priv(cfg); brcmf_free_wiphy(cfg->wiphy); kfree(cfg); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h index b7b50b07f776..14d5bbad1db1 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h @@ -292,7 +292,6 @@ struct brcmf_cfg80211_wowl { */ struct brcmf_cfg80211_info { struct wiphy *wiphy; - struct cfg80211_ops *ops; struct brcmf_cfg80211_conf *conf; struct brcmf_p2p_info p2p; struct brcmf_btcoex_info *btcoex; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 21e07d1ceeae..e8c488376ff9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -1224,12 +1224,15 @@ int brcmf_attach(struct device *dev, struct brcmf_mp_device *settings) return -ENOMEM; wiphy = wiphy_new(ops, sizeof(*drvr)); - if (!wiphy) + if (!wiphy) { + kfree(ops); return -ENOMEM; + } set_wiphy_dev(wiphy, dev); drvr = wiphy_priv(wiphy); drvr->wiphy = wiphy; + drvr->ops = ops; for (i = 0; i < ARRAY_SIZE(drvr->if2bss); i++) drvr->if2bss[i] = BRCMF_BSSIDX_INVALID; @@ -1262,12 +1265,10 @@ int brcmf_attach(struct device *dev, struct brcmf_mp_device *settings) goto fail; } - drvr->config->ops = ops; return 0; fail: brcmf_detach(dev); - kfree(ops); return ret; } @@ -1353,6 +1354,8 @@ void brcmf_detach(struct device *dev) bus_if->drvr = NULL; + kfree(drvr->ops); + wiphy_free(drvr->wiphy); } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h index 86517a3d74b1..6699637d3bf8 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h @@ -97,6 +97,7 @@ struct brcmf_pub { struct brcmf_bus *bus_if; struct brcmf_proto *proto; struct wiphy *wiphy; + struct cfg80211_ops *ops; struct brcmf_cfg80211_info *config; /* Internal brcmf items */ -- 2.21.0
[PATCH 2/3] brcmfmac: split brcmf_attach() and brcmf_detach() functions
From: Rafał Miłecki Move code allocating/freeing wiphy out of above functions. This will allow reinitializing the driver (e.g. on some error) without allocating a new wiphy. Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/bus.h | 4 ++- .../broadcom/brcm80211/brcmfmac/core.c| 33 ++ .../broadcom/brcm80211/brcmfmac/pcie.c| 13 +-- .../broadcom/brcm80211/brcmfmac/sdio.c| 15 ++-- .../broadcom/brcm80211/brcmfmac/usb.c | 34 +++ 5 files changed, 80 insertions(+), 19 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h index 0988a166a785..623c0168da79 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h @@ -253,10 +253,12 @@ void brcmf_rx_frame(struct device *dev, struct sk_buff *rxp, bool handle_event); /* Receive async event packet from firmware. Callee disposes of rxp. */ void brcmf_rx_event(struct device *dev, struct sk_buff *rxp); +int brcmf_alloc(struct device *dev, struct brcmf_mp_device *settings); /* Indication from bus module regarding presence/insertion of dongle. */ -int brcmf_attach(struct device *dev, struct brcmf_mp_device *settings); +int brcmf_attach(struct device *dev); /* Indication from bus module regarding removal/absence of dongle */ void brcmf_detach(struct device *dev); +void brcmf_free(struct device *dev); /* Indication from bus module that dongle should be reset */ void brcmf_dev_reset(struct device *dev); /* Request from bus module to initiate a coredump */ diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index e8c488376ff9..406b367c284c 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -1209,13 +1209,11 @@ static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) return ret; } -int brcmf_attach(struct device *dev, struct brcmf_mp_device *settings) +int brcmf_alloc(struct device *dev, struct brcmf_mp_device *settings) { struct wiphy *wiphy; struct cfg80211_ops *ops; struct brcmf_pub *drvr = NULL; - int ret = 0; - int i; brcmf_dbg(TRACE, "Enter\n"); @@ -1233,6 +1231,21 @@ int brcmf_attach(struct device *dev, struct brcmf_mp_device *settings) drvr = wiphy_priv(wiphy); drvr->wiphy = wiphy; drvr->ops = ops; + drvr->bus_if = dev_get_drvdata(dev); + drvr->bus_if->drvr = drvr; + drvr->settings = settings; + + return 0; +} + +int brcmf_attach(struct device *dev) +{ + struct brcmf_bus *bus_if = dev_get_drvdata(dev); + struct brcmf_pub *drvr = bus_if->drvr; + int ret = 0; + int i; + + brcmf_dbg(TRACE, "Enter\n"); for (i = 0; i < ARRAY_SIZE(drvr->if2bss); i++) drvr->if2bss[i] = BRCMF_BSSIDX_INVALID; @@ -1241,9 +1254,6 @@ int brcmf_attach(struct device *dev, struct brcmf_mp_device *settings) /* Link to bus module */ drvr->hdrlen = 0; - drvr->bus_if = dev_get_drvdata(dev); - drvr->bus_if->drvr = drvr; - drvr->settings = settings; /* Attach and link in the protocol */ ret = brcmf_proto_attach(drvr); @@ -1259,7 +1269,7 @@ int brcmf_attach(struct device *dev, struct brcmf_mp_device *settings) /* attach firmware event handler */ brcmf_fweh_attach(drvr); - ret = brcmf_bus_started(drvr, ops); + ret = brcmf_bus_started(drvr, drvr->ops); if (ret != 0) { bphy_err(drvr, "dongle is not responding: err=%d\n", ret); goto fail; @@ -1351,6 +1361,15 @@ void brcmf_detach(struct device *dev) brcmf_cfg80211_detach(drvr->config); drvr->config = NULL; } +} + +void brcmf_free(struct device *dev) +{ + struct brcmf_bus *bus_if = dev_get_drvdata(dev); + struct brcmf_pub *drvr = bus_if->drvr; + + if (!drvr) + return; bus_if->drvr = NULL; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 7ac945369762..b01b33e99c14 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -1430,6 +1430,7 @@ static int brcmf_pcie_reset(struct device *dev) brcmf_pcie_bus_console_read(devinfo, true); brcmf_detach(dev); + brcmf_free(dev); brcmf_pcie_release_irq(devinfo); brcmf_pcie_release_scratchbuffers(devinfo); @@ -1824,11 +1825,18 @@ static void brcmf_pcie_setup(struct device *dev, int ret, brcmf_pcie_intr_enable(dev
[PATCH 0/3] brcmfmac: keep wiphy during PCIe driver lifetime
From: Rafał Miłecki Driver's main init/attach function brcmf_attach() was handling both: wiphy allocation and driver initialization. It meant getting a new wiphy on every init (initial, resume, error recovery). For supplicants/authenticators and Linux users it's move convenient to have the same wiphy over driver's lifetime. It allows e.g. automatic recovery after a firmware crash. This patchset was tested on BCM4366 (PCIe) and BCM43430 (SDIO). Right now only PCIe makes use of keeping the same wiphy. I got RPi Zero W so I'm planning to add firmware crash recovery & keep wiphy for SDIO in the future. Rafał Miłecki (3): brcmfmac: move "cfg80211_ops" pointer to another struct brcmfmac: split brcmf_attach() and brcmf_detach() functions brcmfmac: don't realloc wiphy during PCIe reset .../broadcom/brcm80211/brcmfmac/bus.h | 4 +- .../broadcom/brcm80211/brcmfmac/cfg80211.c| 1 - .../broadcom/brcm80211/brcmfmac/cfg80211.h| 1 - .../broadcom/brcm80211/brcmfmac/core.c| 42 ++- .../broadcom/brcm80211/brcmfmac/core.h| 1 + .../broadcom/brcm80211/brcmfmac/pcie.c| 13 +- .../broadcom/brcm80211/brcmfmac/sdio.c| 15 +-- .../broadcom/brcm80211/brcmfmac/usb.c | 34 --- 8 files changed, 87 insertions(+), 24 deletions(-) -- 2.21.0
[PATCH V2 1/2] brcmfmac: add stub version of brcmf_debugfs_get_devdir()
From: Rafał Miłecki In case of compiling driver without DEBUG expose a stub function to make writing debug code much simpler (no extra conditions). This will allow e.g. using debugfs_create_file() without any magic if or #ifdef. Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h index ea6e8e839cae..9b221b509ade 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h @@ -121,6 +121,10 @@ int brcmf_debugfs_add_entry(struct brcmf_pub *drvr, const char *fn, int brcmf_debug_create_memdump(struct brcmf_bus *bus, const void *data, size_t len); #else +static inline struct dentry *brcmf_debugfs_get_devdir(struct brcmf_pub *drvr) +{ + return ERR_PTR(-ENOENT); +} static inline int brcmf_debugfs_add_entry(struct brcmf_pub *drvr, const char *fn, int (*read_fn)(struct seq_file *seq, void *data)) -- 2.21.0
[PATCH V2 2/2] brcmfmac: add "reset" debugfs entry for testing reset
From: Rafał Miłecki This is a trivial debugfs entry for triggering reset just like in case of firmware crash. It works by writing 1 to it: echo 1 > reset Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/core.c| 25 +++ 1 file changed, 25 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 705b8cc53c3e..21e07d1ceeae 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -1086,6 +1086,29 @@ static void brcmf_core_bus_reset(struct work_struct *work) brcmf_bus_reset(drvr->bus_if); } +static ssize_t bus_reset_write(struct file *file, const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct brcmf_pub *drvr = file->private_data; + u8 value; + + if (kstrtou8_from_user(user_buf, count, 0, &value)) + return -EINVAL; + + if (value != 1) + return -EINVAL; + + schedule_work(&drvr->bus_reset); + + return count; +} + +static const struct file_operations bus_reset_fops = { + .open = simple_open, + .llseek = no_llseek, + .write = bus_reset_write, +}; + static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) { int ret = -1; @@ -1161,6 +1184,8 @@ static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) /* populate debugfs */ brcmf_debugfs_add_entry(drvr, "revinfo", brcmf_revinfo_read); + debugfs_create_file("reset", 0600, brcmf_debugfs_get_devdir(drvr), drvr, + &bus_reset_fops); brcmf_feat_debugfs_create(drvr); brcmf_proto_debugfs_create(drvr); brcmf_bus_debugfs_create(bus_if); -- 2.21.0
Re: [PATCH] brcmfmac: add "reset" debugfs entry for testing reset
On 28.08.2019 17:47, Rafał Miłecki wrote: From: Rafał Miłecki This is a trivial debugfs entry for triggering reset just like in case of firmware crash. It works by writing 1 to it: echo 1 > reset Signed-off-by: Rafał Miłecki Please drop this patch drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c: In function 'brcmf_bus_started': drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c:1187:37: error: implicit declaration of function 'brcmf_debugfs_get_devdir'; did you mean 'brcmf_debugfs_add_entry'? [-Werror=implicit-function-declaration] debugfs_create_file("reset", 0600, brcmf_debugfs_get_devdir(drvr), drvr, ^~~~ brcmf_debugfs_add_entry
[PATCH -next] brcmfmac: get chip's default RAM info during PCIe setup
From: Rafał Miłecki Getting RAM info just once per driver's lifetime (during chip recognition) is not enough as it may get adjusted later (depending on the used firmware). Subsequent inits may load different firmwares so a full RAM recognition is required on every PCIe setup. This is especially important since implementing hardware reset on a firmware crash. Moreover calling brcmf_chip_get_raminfo() makes sure that RAM core is up. It's important as having BCMA_CORE_SYS_MEM down on BCM4366 was resulting in firmware failing to initialize and following error: [ 65.657546] brcmfmac :01:00.0: brcmf_pcie_download_fw_nvram: Invalid shared RAM address 0x0401 This change makes brcmf_chip_get_raminfo() call during chip recognition redundant for PCIe devices but SDIO and USB still need it and it's a very small overhead anyway. Fixes: 4684997d9eea ("brcmfmac: reset PCIe bus on a firmware crash") Signed-off-by: Rafał Miłecki --- This is a refreshed version of previous: [PATCH 2/4] brcmfmac: get RAM info right before downloading PCIe firmware https://patchwork.kernel.org/patch/10830171/ After Arend's comment I did some extra research on driver's RAM info code and updated commit properly. --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 6 -- drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h | 1 + drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 6 ++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c index 1ec48c4f4d4a..dd586a96b57a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c @@ -696,8 +696,10 @@ static u32 brcmf_chip_tcm_rambase(struct brcmf_chip_priv *ci) return 0; } -static int brcmf_chip_get_raminfo(struct brcmf_chip_priv *ci) +int brcmf_chip_get_raminfo(struct brcmf_chip *pub) { + struct brcmf_chip_priv *ci = container_of(pub, struct brcmf_chip_priv, + pub); struct brcmf_core_priv *mem_core; struct brcmf_core *mem; @@ -979,7 +981,7 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci) brcmf_chip_set_passive(&ci->pub); } - return brcmf_chip_get_raminfo(ci); + return brcmf_chip_get_raminfo(&ci->pub); } static void brcmf_chip_disable_arm(struct brcmf_chip_priv *chip, u16 id) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h index 206d7695d57a..7b00f6a59e89 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h @@ -69,6 +69,7 @@ struct brcmf_buscore_ops { void (*activate)(void *ctx, struct brcmf_chip *chip, u32 rstvec); }; +int brcmf_chip_get_raminfo(struct brcmf_chip *pub); struct brcmf_chip *brcmf_chip_attach(void *ctx, const struct brcmf_buscore_ops *ops); void brcmf_chip_detach(struct brcmf_chip *chip); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 8d0e74416643..7ac945369762 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -1770,6 +1770,12 @@ static void brcmf_pcie_setup(struct device *dev, int ret, nvram_len = fwreq->items[BRCMF_PCIE_FW_NVRAM].nv_data.len; kfree(fwreq); + ret = brcmf_chip_get_raminfo(devinfo->ci); + if (ret) { + brcmf_err(bus, "Failed to get RAM info\n"); + goto fail; + } + /* Some of the firmwares have the size of the memory of the device * defined inside the firmware. This is because part of the memory in * the device is shared and the devision is determined by FW. Parse -- 2.21.0
[PATCH] brcmfmac: add "reset" debugfs entry for testing reset
From: Rafał Miłecki This is a trivial debugfs entry for triggering reset just like in case of firmware crash. It works by writing 1 to it: echo 1 > reset Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/core.c| 25 +++ 1 file changed, 25 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 705b8cc53c3e..21e07d1ceeae 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -1086,6 +1086,29 @@ static void brcmf_core_bus_reset(struct work_struct *work) brcmf_bus_reset(drvr->bus_if); } +static ssize_t bus_reset_write(struct file *file, const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct brcmf_pub *drvr = file->private_data; + u8 value; + + if (kstrtou8_from_user(user_buf, count, 0, &value)) + return -EINVAL; + + if (value != 1) + return -EINVAL; + + schedule_work(&drvr->bus_reset); + + return count; +} + +static const struct file_operations bus_reset_fops = { + .open = simple_open, + .llseek = no_llseek, + .write = bus_reset_write, +}; + static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) { int ret = -1; @@ -1161,6 +1184,8 @@ static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) /* populate debugfs */ brcmf_debugfs_add_entry(drvr, "revinfo", brcmf_revinfo_read); + debugfs_create_file("reset", 0600, brcmf_debugfs_get_devdir(drvr), drvr, + &bus_reset_fops); brcmf_feat_debugfs_create(drvr); brcmf_proto_debugfs_create(drvr); brcmf_bus_debugfs_create(bus_if); -- 2.21.0
[PATCH] brcmfmac: don't net_ratelimit() CONSOLE messages on firmware crash
From: Rafał Miłecki Firmware crash is a pretty rare event and can't happen too frequently as it has to be followed by a hardware reinitialization and config reload. It should be safe to don't use net_ratelimit() when it happens. For reporting & debugging purposes it's important to provide a complete log as the last lines are actually the most important. This change modifies brcmfmac to print all messages in an unlimited way in that specific case. With this change there should be finally a backtrace of firmware finally visible after a crash. 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 e488b1aaaee2..7ac945369762 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -794,7 +794,8 @@ static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo, if (ch == '\n') { console->log_str[console->log_idx] = 0; if (error) - brcmf_err(bus, "CONSOLE: %s", console->log_str); + __brcmf_err(bus, __func__, "CONSOLE: %s", + console->log_str); else pr_debug("CONSOLE: %s", console->log_str); console->log_idx = 0; -- 2.21.0
Re: [PATCH 2/7] brcmfmac: change the order of things in brcmf_detach()
On 11.07.2019 11:05, Arend van Spriel wrote: When brcmf_detach() from the bus layer upon rmmod we can no longer communicate. Hence we will set the bus state to DOWN and cleanup the event and protocol layer. The network interfaces need to be deleted before brcmf_cfg80211_detach() because the latter does the wiphy_unregister() which issues a warning if there are still network devices linked to the wiphy instance. Reviewed-by: Hante Meuleman Reviewed-by: Pieter-Paul Giesberts Reviewed-by: Franky Lin Signed-off-by: Arend van Spriel This fixes a rmmod crash in brcmf_txfinalize() that I reported in the: brcmfmac: NULL pointer dereference during brcmf_detach() after firmware crash https://www.spinics.net/lists/linux-wireless/msg182913.html Tested-by: Rafał Miłecki
Re: [PATCH] brcmfmac: change the order of things in brcmf_detach()
On 30.04.2019 12:10, Arend Van Spriel wrote: On 4/30/2019 10:11 AM, Piotr Figiel wrote: Hi Arend, On Mon, Apr 29, 2019 at 12:09:21PM +0200, Arend van Spriel wrote: When brcmf_detach() from the bus layer upon rmmod we can no longer communicate. Hence we will set the bus state to DOWN and cleanup the event and protocol layer. The network interfaces need to be deleted before brcmf_cfg80211_detach() because the latter does the wiphy_unregister() which issues a warning if there are still network devices linked to the wiphy instance. This seems to already happen - brcmf_cfg80211_detach() is called after the interfaces are removed. Right. This was just to remind me why brcmf_cfg80211_detach() must be called after removing the interfaces. This change solves a null pointer dereference issue which happened upon issueing rmmod while there are packets queued in bus protocol layer. Reported-by: Rafał Miłecki Reviewed-by: Hante Meuleman Reviewed-by: Pieter-Paul Giesberts Reviewed-by: Franky Lin Signed-off-by: Arend van Spriel --- Hi Piotr, While working on an issue with msgbuf protocol (used for PCIe devices) your change 5cdb0ef6144f ("brcmfmac: fix NULL pointer derefence during USB disconnect") conflicted. I suspect my reordering stuff in brcmf_detach() also fixes your issue so could you retest this patch, which basically reverts your change and applies my reordering, and see whether my suspicion can be confirmed. Does the issue reported by Rafał you are trying to solve with this patch occur on current linux-next? Looking at you patch I suspect it does, because brcmf_proto_msgbuf_detach() is invoked in brcmf_proto_detach_post_delif(). However, I could not reproduce it with or without the patch. Rafał, Do you know whether your reported issue, ie. calling brcmf_tx_finalize() after interfaces were removed, still exists in wireless-testing (or linux-next). Sorry for a terribly late reply. It took me many attempts to crash a firmware in a fully reproducible way. I can say for sure this patch fixes crashes in brcmf_txfinalize() I saw when unloading brcmfmac after a crash.
Re: [PATCH 3/7] brcmsmac: switch source files to using SPDX license identifier
On Fri, 17 May 2019 at 01:25, Arend Van Spriel wrote: > On 5/16/2019 10:01 PM, Greg Kroah-Hartman wrote: > > On Thu, May 16, 2019 at 09:45:19PM +0200, Arend Van Spriel wrote: > >> On 5/16/2019 7:31 PM, Greg Kroah-Hartman wrote: > >>> On Thu, May 16, 2019 at 02:04:07PM +0200, Arend van Spriel wrote: > With ISC license text in place under the LICENSES folder switch > to using the SPDX license identifier to refer to the ISC license. > > Cc: Thomas Gleixner > Cc: Greg Kroah-Hartman > Reviewed-by: Hante Meuleman > Reviewed-by: Pieter-Paul Giesberts > Reviewed-by: Franky Lin > Signed-off-by: Arend van Spriel > --- > Hi Thomas, Greg, > > The file drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c > did not have license information nor copyright notice and as such > it got included in commit b24413180f56 ("License cleanup: add SPDX > GPL-2.0 license identifier to files with no license"). I added you > guys as I propose to align this source file with the rest of > the driver sources and change it to ISC license and add the missing > copyright notice while at it (not sure if that warrants a separate > patch). > >>> > >>> A separate patch would be good, to make it explicit that you are > >>> changing the license of the file. > >> > >> Ok. > >> > >>> And ISC, ick, why... :) > >> > >> Because the license text in the other driver source files is a 1:1 match > >> with the ISC license. > > > > Oh, I am not disagreeing with that, yes, that is obviously the license > > of the files. Just complaining about that choice for Linux kernel code :) > > I see. > > >> Another option could be MIT license which is in the preferred folder. > >> Will have to consult our legal department about it though. > > > > Hey, if your legal department is going to get asked this, why not just > > switch it to GPLv2? That would make everything much simpler. > > Hah. Because I already know the answer to that. ;-) It's not that obvious to me, sorry. Does your legal department require something more permissive than GPLv2? Is that worth asking them about dual-licensing? Something like GPL-2.0 OR MIT ? That assures driver is compatible with Linux, no matter what's the current lawyers interpretation of MIT vs. GPL 2.0. I believe Alan Cox once told/suggested that dual-licensing is safer for legal reasons. -- Rafał
Re: [PATCH] brcmfmac: print firmware messages after a firmware crash
On 2019-04-28 23:06, Arend Van Spriel wrote: On 4/27/2019 8:30 PM, Rafał Miłecki wrote: From: Rafał Miłecki Normally firmware messages are printed with debugging enabled only. It's a good idea as firmware may print a lot of messages that normal users don't need to care about. However, on firmware crash, it may be very helpful to log all recent messages. There is almost always a backtrace available as well as rought info on the latest actions/state. nice... there is one minor nit below, but other than that... Reviewed-by: Arend van Spriel Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/pcie.c| 24 ++- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 637973fe8928..f519b050aff3 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -764,15 +764,22 @@ static void brcmf_pcie_bus_console_init(struct brcmf_pciedev_info *devinfo) console->base_addr, console->buf_addr, console->bufsize); } - -static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo) +/** + * brcmf_pcie_bus_console_read - reads firmware messages + * + * @error: specifies if error has occurred (prints messages unconditionally) + */ +static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo, + int error) Given how it is called I would say 'bool error' makes a bit more sense. I even call that function passing "true" or "false" as argument. Nice catch!
[PATCH V2] brcmfmac: print firmware messages after a firmware crash
From: Rafał Miłecki Normally firmware messages are printed with debugging enabled only. It's a good idea as firmware may print a lot of messages that normal users don't need to care about. However, on firmware crash, it may be very helpful to log all recent messages. There is almost always a backtrace available as well as rought info on the latest actions/state. Signed-off-by: Rafał Miłecki Reviewed-by: Arend van Spriel --- V2: Use "bool" for brcmf_pcie_bus_console_read() error argument --- .../broadcom/brcm80211/brcmfmac/pcie.c| 24 ++- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 637973fe8928..594c14be5df0 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -764,15 +764,22 @@ static void brcmf_pcie_bus_console_init(struct brcmf_pciedev_info *devinfo) console->base_addr, console->buf_addr, console->bufsize); } - -static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo) +/** + * brcmf_pcie_bus_console_read - reads firmware messages + * + * @error: specifies if error has occurred (prints messages unconditionally) + */ +static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo, + bool error) { + struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); struct brcmf_pcie_console *console; u32 addr; u8 ch; u32 newidx; - if (!BRCMF_FWCON_ON()) + if (!error && !BRCMF_FWCON_ON()) return; console = &devinfo->shared.console; @@ -796,7 +803,10 @@ static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo) } if (ch == '\n') { console->log_str[console->log_idx] = 0; - pr_debug("CONSOLE: %s", console->log_str); + if (error) + brcmf_err(bus, "CONSOLE: %s", console->log_str); + else + pr_debug("CONSOLE: %s", console->log_str); console->log_idx = 0; } } @@ -857,7 +867,7 @@ static irqreturn_t brcmf_pcie_isr_thread(int irq, void *arg) &devinfo->pdev->dev); } } - brcmf_pcie_bus_console_read(devinfo); + brcmf_pcie_bus_console_read(devinfo, false); if (devinfo->state == BRCMFMAC_PCIE_STATE_UP) brcmf_pcie_intr_enable(devinfo); devinfo->in_irq = false; @@ -1426,6 +1436,8 @@ static int brcmf_pcie_reset(struct device *dev) struct brcmf_fw_request *fwreq; int err; + brcmf_pcie_bus_console_read(devinfo, true); + brcmf_detach(dev); brcmf_pcie_release_irq(devinfo); @@ -1824,7 +1836,7 @@ static void brcmf_pcie_setup(struct device *dev, int ret, if (brcmf_attach(&devinfo->pdev->dev, devinfo->settings) == 0) return; - brcmf_pcie_bus_console_read(devinfo); + brcmf_pcie_bus_console_read(devinfo, false); fail: device_release_driver(dev); -- 2.21.0
[PATCH] brcmfmac: print firmware messages after a firmware crash
From: Rafał Miłecki Normally firmware messages are printed with debugging enabled only. It's a good idea as firmware may print a lot of messages that normal users don't need to care about. However, on firmware crash, it may be very helpful to log all recent messages. There is almost always a backtrace available as well as rought info on the latest actions/state. Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/pcie.c| 24 ++- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 637973fe8928..f519b050aff3 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -764,15 +764,22 @@ static void brcmf_pcie_bus_console_init(struct brcmf_pciedev_info *devinfo) console->base_addr, console->buf_addr, console->bufsize); } - -static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo) +/** + * brcmf_pcie_bus_console_read - reads firmware messages + * + * @error: specifies if error has occurred (prints messages unconditionally) + */ +static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo, + int error) { + struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); struct brcmf_pcie_console *console; u32 addr; u8 ch; u32 newidx; - if (!BRCMF_FWCON_ON()) + if (!error && !BRCMF_FWCON_ON()) return; console = &devinfo->shared.console; @@ -796,7 +803,10 @@ static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo) } if (ch == '\n') { console->log_str[console->log_idx] = 0; - pr_debug("CONSOLE: %s", console->log_str); + if (error) + brcmf_err(bus, "CONSOLE: %s", console->log_str); + else + pr_debug("CONSOLE: %s", console->log_str); console->log_idx = 0; } } @@ -857,7 +867,7 @@ static irqreturn_t brcmf_pcie_isr_thread(int irq, void *arg) &devinfo->pdev->dev); } } - brcmf_pcie_bus_console_read(devinfo); + brcmf_pcie_bus_console_read(devinfo, false); if (devinfo->state == BRCMFMAC_PCIE_STATE_UP) brcmf_pcie_intr_enable(devinfo); devinfo->in_irq = false; @@ -1426,6 +1436,8 @@ static int brcmf_pcie_reset(struct device *dev) struct brcmf_fw_request *fwreq; int err; + brcmf_pcie_bus_console_read(devinfo, true); + brcmf_detach(dev); brcmf_pcie_release_irq(devinfo); @@ -1824,7 +1836,7 @@ static void brcmf_pcie_setup(struct device *dev, int ret, if (brcmf_attach(&devinfo->pdev->dev, devinfo->settings) == 0) return; - brcmf_pcie_bus_console_read(devinfo); + brcmf_pcie_bus_console_read(devinfo, false); fail: device_release_driver(dev); -- 2.21.0
Re: brcmfmac: NULL pointer dereference during brcmf_detach() after firmware crash
On Fri, 15 Feb 2019 at 07:15, Rafał Miłecki wrote: > On Thu, 14 Feb 2019 at 23:37, Arend Van Spriel > wrote: > > On 2/14/2019 11:30 PM, Rafał Miłecki wrote: > > > I've just found a well reproducible brcmfmac crash (NULL pointer > > > dereference). > > > > > > Steps: > > > 1. Wait for or trigger a FullMAC firmware crash > > > 2. Wait for some skb to get queued on a flowring > > > 3. Call rmmod brcmfmac > > > > > > Problem: > > > There is a NULL pointer dereference in one of the brcmf_detach() calls. > > > > > > Explanation: > > > brcmf_detach() first frees all "ifp"s and then deletes flowrings. If any > > > flowring has a skb it results in calling brcmf_txfinalize() which tries > > > to access "ifp" (struct brcmf_if) which is a NULL. > > > > Hi Rafał, > > > > Thanks for diving in. That was my suspicion. Does it mean you are > > working on a patch or shall I take care of it. > > It would be nice to have someone more experienced with detaching & > rings look at it. Is adding a simple > if (ifp) > enough? Or should that code get redesigned? Should we e.g. reorder detach > order? Hi Arend, would you find a moment to look at that crash, please? -- Rafał
Re: brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000): clkctl 0x50
On Sun, 3 Mar 2019 at 20:06, Krzysiek Rosinski wrote: > On Sun, Mar 3, 2019 at 10:22 AM Krzysiek Rosinski > wrote: > > I'm working on a custom board with cyw4343 connected to osd3358-sm (TI > > am3358), when I load the brcmfmac driver I get 'HT Avail timeout' > > error. The same combination of driver+firmware works fine on Raspberry > > Pi Zero W. > > > > I've tried to gather as much information I can from both; the custom > > board and pi. This is available here: > > > > https://www.dropbox.com/s/vqejyrixbxx1z8d/logs.tar.gz?dl=0 > > > > I have a custom board with cyw4343 chip fitted and also flyleded > > Murata 1DX to the PocketBeagle, both give the same error code. > > > > My setup is as follows: > > > > osd335x-sm (TI am3348), cyw4343, 37.4 MHz xtal. > > Linux beaglebone 4.14.93-bone17.1 #1 PREEMPT Mon Feb 25 16:27:58 UTC > > 2019 armv7l GNU/Linux > > brcmfmac: cypress v4.14.52-backports > > brcm43430-sdio.bin: Version: 7.45.98.38 > > > > Is there something obvious which I'm doing wrong? Can you give me a > > hint on how to move with this further, please? You have to find new enough bcmdevs.h. Try Googling BFL3_FEMCTRL_SUB and you should find many BFL3_* defines easily.
[PATCH 1/4] brcmfmac: support repeated brcmf_fw_alloc_request() calls
From: Rafał Miłecki During a normal brcmfmac lifetime brcmf_fw_alloc_request() is called once only during the probe. It's safe to assume provided array is clear. Further brcmfmac improvements may require calling it multiple times though. This patch allows it by fixing invalid firmware paths like: brcm/brcmfmac4366c-pcie.binbrcm/brcmfmac4366c-pcie.bin Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c index 8209a42dea72..65098a02e1ad 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c @@ -743,6 +743,7 @@ brcmf_fw_alloc_request(u32 chip, u32 chiprev, for (j = 0; j < n_fwnames; j++) { fwreq->items[j].path = fwnames[j].path; + fwnames[j].path[0] = '\0'; /* check if firmware path is provided by module parameter */ if (brcmf_mp_global.firmware_path[0] != '\0') { strlcpy(fwnames[j].path, mp_path, -- 2.20.1
[PATCH 2/4] brcmfmac: get RAM info right before downloading PCIe firmware
From: Rafał Miłecki It's important as brcmf_chip_get_raminfo() also makes sure that memory is properly setup. Without it the firmware could report invalid RAM address like 0x0401. During a normal brcmfmac lifetime brcmf_chip_get_raminfo() is called on probe by the brcmf_chip_recognition(). This change allows implementing further improvements like handling errors by resetting a device with the brcmf_pcie_reset_device() and redownloading a firmware afterwards. Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 6 -- drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h | 1 + drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 6 ++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c index 22534bf2a90c..fcaf19165891 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c @@ -707,8 +707,10 @@ static u32 brcmf_chip_tcm_rambase(struct brcmf_chip_priv *ci) return 0; } -static int brcmf_chip_get_raminfo(struct brcmf_chip_priv *ci) +int brcmf_chip_get_raminfo(struct brcmf_chip *pub) { + struct brcmf_chip_priv *ci = container_of(pub, struct brcmf_chip_priv, + pub); struct brcmf_core_priv *mem_core; struct brcmf_core *mem; @@ -990,7 +992,7 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci) brcmf_chip_set_passive(&ci->pub); } - return brcmf_chip_get_raminfo(ci); + return brcmf_chip_get_raminfo(&ci->pub); } static void brcmf_chip_disable_arm(struct brcmf_chip_priv *chip, u16 id) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h index 0ae3b33bab62..4794cf38b4d3 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h @@ -80,6 +80,7 @@ struct brcmf_buscore_ops { void (*activate)(void *ctx, struct brcmf_chip *chip, u32 rstvec); }; +int brcmf_chip_get_raminfo(struct brcmf_chip *pub); struct brcmf_chip *brcmf_chip_attach(void *ctx, const struct brcmf_buscore_ops *ops); void brcmf_chip_detach(struct brcmf_chip *chip); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 58a6bc379358..39f6421885f6 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -1727,6 +1727,12 @@ static void brcmf_pcie_setup(struct device *dev, int ret, nvram_len = fwreq->items[BRCMF_PCIE_FW_NVRAM].nv_data.len; kfree(fwreq); + ret = brcmf_chip_get_raminfo(devinfo->ci); + if (ret) { + brcmf_err(bus, "Failed to get RAM info\n"); + goto fail; + } + /* Some of the firmwares have the size of the memory of the device * defined inside the firmware. This is because part of the memory in * the device is shared and the devision is determined by FW. Parse -- 2.20.1
[PATCH 0/4] brcmfmac: recover PCIe devices from firmware crashes
From: Rafał Miłecki So far PCIe firmware halts / crashes were resulting in massive timeouts and running out of resources (e.g. slots). There wasn't even a clear message indicating the problem source. This patches improves it by: 1) Printing an error 2) Reloading a firmware After that user can setup interface(s) & use a wireless card again. It should be much more convenient than reloading a module manually thanks to: 1) Automation 2) Not affecting other (working) wireless cards Rafał Miłecki (4): brcmfmac: support multiple brcmf_fw_alloc_request() calls brcmfmac: get RAM info right before downloading PCIe firmware brcmfmac: add function designated for handling firmware failures brcmfmac: reset PCIe bus on firmware crash .../broadcom/brcm80211/brcmfmac/bus.h | 12 ++ .../broadcom/brcm80211/brcmfmac/chip.c| 6 ++- .../broadcom/brcm80211/brcmfmac/chip.h| 1 + .../broadcom/brcm80211/brcmfmac/core.c| 22 ++ .../broadcom/brcm80211/brcmfmac/core.h| 2 + .../broadcom/brcm80211/brcmfmac/firmware.c| 1 + .../broadcom/brcm80211/brcmfmac/pcie.c| 43 ++- .../broadcom/brcm80211/brcmfmac/sdio.c| 4 +- 8 files changed, 86 insertions(+), 5 deletions(-) -- 2.20.1
[PATCH 3/4] brcmfmac: add a function designated for handling firmware fails
From: Rafał Miłecki This improves handling PCIe firmware halts by printing a clear error message and replaces a similar code in the SDIO bus support. It will also allow further improvements like trying to recover from a firmware crash. Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 2 ++ .../net/wireless/broadcom/brcm80211/brcmfmac/core.c| 10 ++ .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c| 2 +- .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c| 4 ++-- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h index 3d441c5c745c..801106583ae7 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h @@ -262,6 +262,8 @@ void brcmf_detach(struct device *dev); void brcmf_dev_reset(struct device *dev); /* Request from bus module to initiate a coredump */ void brcmf_dev_coredump(struct device *dev); +/* Indication that firmware has halted or crashed */ +void brcmf_fw_crashed(struct device *dev); /* Configure the "global" bus state used by upper layers */ void brcmf_bus_change_state(struct brcmf_bus *bus, enum brcmf_bus_state state); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 4fbe8791f674..7f4d9356b79e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -1273,6 +1273,16 @@ void brcmf_dev_coredump(struct device *dev) brcmf_dbg(TRACE, "failed to create coredump\n"); } +void brcmf_fw_crashed(struct device *dev) +{ + struct brcmf_bus *bus_if = dev_get_drvdata(dev); + struct brcmf_pub *drvr = bus_if->drvr; + + bphy_err(drvr, "Firmware has halted or crashed\n"); + + brcmf_dev_coredump(dev); +} + void brcmf_detach(struct device *dev) { s32 i; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 39f6421885f6..cfa34672315f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -730,7 +730,7 @@ static void brcmf_pcie_handle_mb_data(struct brcmf_pciedev_info *devinfo) } if (dtoh_mb_data & BRCMF_D2H_DEV_FWHALT) { brcmf_dbg(PCIE, "D2H_MB_DATA: FW HALT\n"); - brcmf_dev_coredump(&devinfo->pdev->dev); + brcmf_fw_crashed(&devinfo->pdev->dev); } } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 4d104ab80fd8..a06af0cd4a7f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -1090,8 +1090,8 @@ static u32 brcmf_sdio_hostmail(struct brcmf_sdio *bus) /* dongle indicates the firmware has halted/crashed */ if (hmb_data & HMB_DATA_FWHALT) { - brcmf_err("mailbox indicates firmware halted\n"); - brcmf_dev_coredump(&sdiod->func1->dev); + brcmf_dbg(SDIO, "mailbox indicates firmware halted\n"); + brcmf_fw_crashed(&sdiod->func1->dev); } /* Dongle recomposed rx frames, accept them again */ -- 2.20.1
[PATCH 4/4] brcmfmac: reset PCIe bus on a firmware crash
From: Rafał Miłecki This includes bus reset & reloading a firmware. It should be sufficient for a user space to (setup and) use a wireless device again. Support for reset on USB & SDIO can be added later. Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/bus.h | 10 ++ .../broadcom/brcm80211/brcmfmac/core.c| 12 +++ .../broadcom/brcm80211/brcmfmac/core.h| 2 ++ .../broadcom/brcm80211/brcmfmac/pcie.c| 35 +++ 4 files changed, 59 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h index 801106583ae7..2fe167eae22c 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h @@ -91,6 +91,7 @@ struct brcmf_bus_ops { int (*get_fwname)(struct device *dev, const char *ext, unsigned char *fw_name); void (*debugfs_create)(struct device *dev); + int (*reset)(struct device *dev); }; @@ -245,6 +246,15 @@ void brcmf_bus_debugfs_create(struct brcmf_bus *bus) return bus->ops->debugfs_create(bus->dev); } +static inline +int brcmf_bus_reset(struct brcmf_bus *bus) +{ + if (!bus->ops->reset) + return -EOPNOTSUPP; + + return bus->ops->reset(bus->dev); +} + /* * interface functions from common layer */ diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 7f4d9356b79e..5f3548b13639 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -1084,6 +1084,14 @@ static int brcmf_revinfo_read(struct seq_file *s, void *data) return 0; } +static void brcmf_core_bus_reset(struct work_struct *work) +{ + struct brcmf_pub *drvr = container_of(work, struct brcmf_pub, + bus_reset); + + brcmf_bus_reset(drvr->bus_if); +} + static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) { int ret = -1; @@ -1155,6 +1163,8 @@ static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) #endif #endif /* CONFIG_INET */ + INIT_WORK(&drvr->bus_reset, brcmf_core_bus_reset); + /* populate debugfs */ brcmf_debugfs_add_entry(drvr, "revinfo", brcmf_revinfo_read); brcmf_feat_debugfs_create(drvr); @@ -1281,6 +1291,8 @@ void brcmf_fw_crashed(struct device *dev) bphy_err(drvr, "Firmware has halted or crashed\n"); brcmf_dev_coredump(dev); + + schedule_work(&drvr->bus_reset); } void brcmf_detach(struct device *dev) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h index d8085ce579f4..9f09aa31eeda 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h @@ -143,6 +143,8 @@ struct brcmf_pub { struct notifier_block inet6addr_notifier; struct brcmf_mp_device *settings; + struct work_struct bus_reset; + u8 clmver[BRCMF_DCMD_SMLEN]; }; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index cfa34672315f..e941039ee1c3 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -345,6 +345,10 @@ static const u32 brcmf_ring_itemsize[BRCMF_NROF_COMMON_MSGRINGS] = { BRCMF_D2H_MSGRING_RX_COMPLETE_ITEMSIZE }; +static void brcmf_pcie_setup(struct device *dev, int ret, +struct brcmf_fw_request *fwreq); +static struct brcmf_fw_request * +brcmf_pcie_prepare_fw_request(struct brcmf_pciedev_info *devinfo); static u32 brcmf_pcie_read_reg32(struct brcmf_pciedev_info *devinfo, u32 reg_offset) @@ -1409,6 +1413,36 @@ int brcmf_pcie_get_fwname(struct device *dev, const char *ext, u8 *fw_name) return 0; } +static int brcmf_pcie_reset(struct device *dev) +{ + struct brcmf_bus *bus_if = dev_get_drvdata(dev); + struct brcmf_pciedev *buspub = bus_if->bus_priv.pcie; + struct brcmf_pciedev_info *devinfo = buspub->devinfo; + struct brcmf_fw_request *fwreq; + int err; + + brcmf_detach(dev); + + brcmf_pcie_release_irq(devinfo); + brcmf_pcie_release_scratchbuffers(devinfo); + brcmf_pcie_release_ringbuffers(devinfo); + brcmf_pcie_reset_device(devinfo); + + fwreq = brcmf_pcie_prepare_fw_request(devinfo); + if (!fwreq) { + dev_err(dev, "Failed to prepare FW request\n"); + return -ENOMEM; + } + + err = brcmf_fw_get_firmwares(dev, fwreq, brcmf_pcie_se
[PATCH] brcmfmac: run firmware state watchdog on the host machine
From: Rafał Miłecki FullMAC firmware may happen to crash due to some potential bugs exposed by e.g. a specific traffic or host-requested setup. It usually results in various timeouts & running our of resources (e.g. ring slots). Monitoring firmware state allows handling such a situation more gracefully. At this point the watchdog: 1) Prints a clear error message about a firmware crash 2) Tries to dump a crash info data That should be helpful for users & should allow providing a valuable reports for the Broadcom developers. It obviously doesn't really fix anything. This watchdog is important for USB & SDIO devices which don't have anything alike implemented yet. It's also there to complement PCIe with its FWHALT signal which does not work in all situations. It has been successfully tested with the recently released brcmfmac4366c-pcie.bin. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201853 Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/core.c| 28 +++ .../broadcom/brcm80211/brcmfmac/core.h| 2 ++ 2 files changed, 30 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 4fbe8791f674..ab7a54b2d831 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -40,6 +40,7 @@ #include "common.h" #define MAX_WAIT_FOR_8021X_TX msecs_to_jiffies(950) +#define BRCMF_FW_WATCHDOG_POLL msecs_to_jiffies(1000) #define BRCMF_BSSIDX_INVALID -1 @@ -1084,6 +1085,28 @@ static int brcmf_revinfo_read(struct seq_file *s, void *data) return 0; } +static void brcmf_fw_watchdog(struct work_struct *work) +{ + struct brcmf_pub *drvr = container_of(work, struct brcmf_pub, + fw_watchdog.work); + + if (drvr->bus_if->state == BRCMF_BUS_UP) { + struct brcmf_if *ifp = drvr->iflist[0]; + s32 ver; + int err; + + err = brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_VERSION, &ver); + if (err) { + bphy_err(drvr, "Firmware has most likely crashed and didn't respond: %d\n", +err); + brcmf_dev_coredump(drvr->bus_if->dev); + return; + } + } + + schedule_delayed_work(&drvr->fw_watchdog, BRCMF_FW_WATCHDOG_POLL); +} + static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) { int ret = -1; @@ -1155,6 +1178,9 @@ static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) #endif #endif /* CONFIG_INET */ + INIT_DELAYED_WORK(&drvr->fw_watchdog, brcmf_fw_watchdog); + schedule_delayed_work(&drvr->fw_watchdog, BRCMF_FW_WATCHDOG_POLL); + /* populate debugfs */ brcmf_debugfs_add_entry(drvr, "revinfo", brcmf_revinfo_read); brcmf_feat_debugfs_create(drvr); @@ -1284,6 +1310,8 @@ void brcmf_detach(struct device *dev) if (drvr == NULL) return; + cancel_delayed_work_sync(&drvr->fw_watchdog); + #ifdef CONFIG_INET unregister_inetaddr_notifier(&drvr->inetaddr_notifier); #endif diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h index d8085ce579f4..1b44d243d05f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h @@ -143,6 +143,8 @@ struct brcmf_pub { struct notifier_block inet6addr_notifier; struct brcmf_mp_device *settings; + struct delayed_work fw_watchdog; + u8 clmver[BRCMF_DCMD_SMLEN]; }; -- 2.20.1
Re: [PATCH wireless-drivers-next 2/2] brcmfmac: print firmware reported general status errors
On 2019-02-21 11:45, Arend Van Spriel wrote: On 2/21/2019 11:33 AM, Rafał Miłecki wrote: From: Rafał Miłecki Firmware may report general errors using a special message type. Add basic support for it by simply decoding & printing an error number. A sample situation in which firmware reports a buf error: CONSOLE: 027084.733 no host response IOCTL buffer available..so fail the request will now produce a "Firmware reported general error: 9" on the host. Could have meaningful message instead of a number. I can do that in follow up patch if you do not have the information. That would be nice, thanks!
[PATCH wireless-drivers-next 1/2] brcmfmac: fix size of the struct msgbuf_ring_status
From: Rafał Miłecki This updates host struct to match the in-firmawre definition. It's a cosmetic change as it only applies to the reserved struct space. Fixes: c988b78244df ("brcmfmac: print firmware reported ring status errors") Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c index aef2d4092872..d711dc8ed606 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c @@ -139,7 +139,7 @@ struct msgbuf_ring_status { struct msgbuf_common_hdrmsg; struct msgbuf_completion_hdrcompl_hdr; __le16 write_idx; - __le32 rsvd0[5]; + __le16 rsvd0[5]; }; struct msgbuf_rx_event { -- 2.20.1
[PATCH wireless-drivers-next 2/2] brcmfmac: print firmware reported general status errors
From: Rafał Miłecki Firmware may report general errors using a special message type. Add basic support for it by simply decoding & printing an error number. A sample situation in which firmware reports a buf error: CONSOLE: 027084.733 no host response IOCTL buffer available..so fail the request will now produce a "Firmware reported general error: 9" on the host. Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/msgbuf.c | 24 +++ 1 file changed, 24 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c index d711dc8ed606..d3780eae7f19 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c @@ -134,6 +134,14 @@ struct msgbuf_completion_hdr { __le16 flow_ring_id; }; +/* Data struct for the MSGBUF_TYPE_GEN_STATUS */ +struct msgbuf_gen_status { + struct msgbuf_common_hdrmsg; + struct msgbuf_completion_hdrcompl_hdr; + __le16 write_idx; + __le32 rsvd0[3]; +}; + /* Data struct for the MSGBUF_TYPE_RING_STATUS */ struct msgbuf_ring_status { struct msgbuf_common_hdrmsg; @@ -1194,6 +1202,18 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) brcmf_netif_rx(ifp, skb); } +static void brcmf_msgbuf_process_gen_status(struct brcmf_msgbuf *msgbuf, + void *buf) +{ + struct msgbuf_gen_status *gen_status = buf; + struct brcmf_pub *drvr = msgbuf->drvr; + int err; + + err = le16_to_cpu(gen_status->compl_hdr.status); + if (err) + bphy_err(drvr, "Firmware reported general error: %d\n", err); +} + static void brcmf_msgbuf_process_ring_status(struct brcmf_msgbuf *msgbuf, void *buf) { @@ -1273,6 +1293,10 @@ static void brcmf_msgbuf_process_msgtype(struct brcmf_msgbuf *msgbuf, void *buf) msg = (struct msgbuf_common_hdr *)buf; switch (msg->msgtype) { + case MSGBUF_TYPE_GEN_STATUS: + brcmf_dbg(MSGBUF, "MSGBUF_TYPE_GEN_STATUS\n"); + brcmf_msgbuf_process_gen_status(msgbuf, buf); + break; case MSGBUF_TYPE_RING_STATUS: brcmf_dbg(MSGBUF, "MSGBUF_TYPE_RING_STATUS\n"); brcmf_msgbuf_process_ring_status(msgbuf, buf); -- 2.20.1
Re: [PATCH wireless-drivers-next] brcmfmac: add basic validation of shared RAM address
On Thu, 21 Feb 2019 at 09:59, Arend Van Spriel wrote: > On 2/21/2019 9:01 AM, Rafał Miłecki wrote: > > On Wed, 20 Feb 2019 at 11:31, Rafał Miłecki wrote: > >> From: Rafał Miłecki > >> > >> While experimenting with firmware loading I ended up in a state of > >> firmware reporting shared RAM address 0x0401. It was causing: > >> [ 94.448015] Unable to handle kernel paging request at virtual address > >> cd680001 > >> due to reading out of the mapped memory. > >> > >> This patch adds some basic validation to avoid kernel crashes due to the > >> unexpected firmware behavior. > > > > For a reference for the further hackers. That has been caused by a > > BCMA_CORE_SYS_MEM core on my BCM4366/4 not being up. > > Thanks, Rafał > > Does that happen all the time or in some specific scenario. Anyway, it > seems like we need to add a check in brcmf_chip_sysmem_ramsize() and > bringup the core if needed. Although, I am curious in what the state the > other cores are at that time. Might need a chip-wide reset. It happens after brcmf_pcie_reset_device() which is probably a 100% expected case ;) Normally brcmfmac does not call brcmf_pcie_reset_device() between brcmf_chip_sysmem_ramsize() and the brcmf_pcie_download_fw_nvram() so it was only my hacking that caused that issue for me. -- Rafał
Re: [PATCH wireless-drivers-next] brcmfmac: add basic validation of shared RAM address
On Wed, 20 Feb 2019 at 11:31, Rafał Miłecki wrote: > From: Rafał Miłecki > > While experimenting with firmware loading I ended up in a state of > firmware reporting shared RAM address 0x0401. It was causing: > [ 94.448015] Unable to handle kernel paging request at virtual address > cd680001 > due to reading out of the mapped memory. > > This patch adds some basic validation to avoid kernel crashes due to the > unexpected firmware behavior. For a reference for the further hackers. That has been caused by a BCMA_CORE_SYS_MEM core on my BCM4366/4 not being up.
[PATCH wireless-drivers-next] brcmfmac: add basic validation of shared RAM address
From: Rafał Miłecki While experimenting with firmware loading I ended up in a state of firmware reporting shared RAM address 0x0401. It was causing: [ 94.448015] Unable to handle kernel paging request at virtual address cd680001 due to reading out of the mapped memory. This patch adds some basic validation to avoid kernel crashes due to the unexpected firmware behavior. Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 257f919c52cc..58a6bc379358 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -1560,6 +1560,12 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo, brcmf_err(bus, "FW failed to initialize\n"); return -ENODEV; } + if (sharedram_addr < devinfo->ci->rambase || + sharedram_addr >= devinfo->ci->rambase + devinfo->ci->ramsize) { + brcmf_err(bus, "Invalid shared RAM address 0x%08x\n", + sharedram_addr); + return -ENODEV; + } brcmf_dbg(PCIE, "Shared RAM addr: 0x%08x\n", sharedram_addr); return (brcmf_pcie_init_share_ram_info(devinfo, sharedram_addr)); -- 2.20.1
[PATCH V4] brcmfmac: use bphy_err() in all wiphy-related code
From: Rafał Miłecki This recently added macro provides more meaningful error messages thanks to identifying a specific wiphy. It's especially important on systems with few cards supported by the same (brcmfmac) driver. Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel --- V2: Fix one line over 80 chars V3: Pass drvr to the brcmf_fweh_call_event_handler() to make getting wiphy 100% reliable V4: Rebase on top of the commit 16e646768396 ("brcmfmac: rework bphy_err() to take struct brcmf_pub argument") --- .../broadcom/brcm80211/brcmfmac/bcdc.c| 22 ++--- .../broadcom/brcm80211/brcmfmac/common.c | 39 .../broadcom/brcm80211/brcmfmac/core.c| 74 .../broadcom/brcm80211/brcmfmac/feature.c | 8 +- .../broadcom/brcm80211/brcmfmac/fweh.c| 25 +++--- .../broadcom/brcm80211/brcmfmac/fwil.c| 10 +-- .../broadcom/brcm80211/brcmfmac/fwsignal.c| 38 .../broadcom/brcm80211/brcmfmac/msgbuf.c | 65 -- .../broadcom/brcm80211/brcmfmac/p2p.c | 88 +++ .../broadcom/brcm80211/brcmfmac/pno.c | 22 +++-- .../broadcom/brcm80211/brcmfmac/proto.c | 6 +- 11 files changed, 224 insertions(+), 173 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c index 1068a2a4494c..73d3c1a0a7c9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c @@ -178,8 +178,8 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, *fwerr = 0; ret = brcmf_proto_bcdc_msg(drvr, ifidx, cmd, buf, len, false); if (ret < 0) { - brcmf_err("brcmf_proto_bcdc_msg failed w/status %d\n", - ret); + bphy_err(drvr, "brcmf_proto_bcdc_msg failed w/status %d\n", +ret); goto done; } @@ -195,9 +195,9 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, if ((id < bcdc->reqid) && (++retries < RETRIES)) goto retry; if (id != bcdc->reqid) { - brcmf_err("%s: unexpected request id %d (expected %d)\n", - brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, - bcdc->reqid); + bphy_err(drvr, "%s: unexpected request id %d (expected %d)\n", +brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, +bcdc->reqid); ret = -EINVAL; goto done; } @@ -245,9 +245,9 @@ brcmf_proto_bcdc_set_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, id = (flags & BCDC_DCMD_ID_MASK) >> BCDC_DCMD_ID_SHIFT; if (id != bcdc->reqid) { - brcmf_err("%s: unexpected request id %d (expected %d)\n", - brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, - bcdc->reqid); + bphy_err(drvr, "%s: unexpected request id %d (expected %d)\n", +brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, +bcdc->reqid); ret = -EINVAL; goto done; } @@ -312,8 +312,8 @@ brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, } if (((h->flags & BCDC_FLAG_VER_MASK) >> BCDC_FLAG_VER_SHIFT) != BCDC_PROTO_VER) { - brcmf_err("%s: non-BCDC packet received, flags 0x%x\n", - brcmf_ifname(tmp_if), h->flags); + bphy_err(drvr, "%s: non-BCDC packet received, flags 0x%x\n", +brcmf_ifname(tmp_if), h->flags); return -EBADE; } @@ -460,7 +460,7 @@ int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr) /* ensure that the msg buf directly follows the cdc msg struct */ if ((unsigned long)(&bcdc->msg + 1) != (unsigned long)bcdc->buf) { - brcmf_err("struct brcmf_proto_bcdc is not correctly defined\n"); + bphy_err(drvr, "struct brcmf_proto_bcdc is not correctly defined\n"); goto fail; } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c index c62009a06617..96b8d5b3aeed 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c @@ -90,6 +90,7 @@ struct brcmf_mp_global_t brcmf_mp_global; void brcmf_c_set_joinpref_default(struct brcmf_if *ifp) { + struct brcmf_pub *drvr = ifp->drvr; struct brcmf_join_pref_params join_pref_params[2]; int err; @@ -106,7 +107,7 @@ void brcmf_c
[PATCH] brcmfmac: rework bphy_err() to take struct brcmf_pub argument
From: Rafał Miłecki This macro will be used in more places not just the cfg80211.c. It makes sense to pass some common struct to it as "struct wiphy" is mostly referenced in cfg80211 code only. A very common one (used above the bus abstraction layer) is struct brcmf_pub. Many functions already keep reference to it which will make using bphy_err() simpler. It should also allow extending that macro's logic if it's ever needed. This improves code recently added in the commit 3ef005b82e2a ("brcmfmac: add bphy_err() and use it in the cfg80211.c"). Signed-off-by: Rafał Miłecki --- While working on the: [PATCH] brcmfmac: use bphy_err() in all wiphy-related code [PATCH V2] brcmfmac: use bphy_err() in all wiphy-related code [PATCH V3] brcmfmac: use bphy_err() in all wiphy-related code I realized it would be: 1) Simpler for the rest of the code 2) More consistent with driver's design to make bphy_err() take brcmfmac's internal & generic struct as argument. So I put above work on hold & I decided to modify bphy_err() to sth more generic before we start using it all around. --- .../broadcom/brcm80211/brcmfmac/cfg80211.c| 510 ++ .../broadcom/brcm80211/brcmfmac/debug.h | 4 +- 2 files changed, 281 insertions(+), 233 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index fa752ec04f22..60b5fb29e5f3 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -457,7 +457,7 @@ static void convert_key_from_CPU(struct brcmf_wsec_key *key, static int send_key_to_dongle(struct brcmf_if *ifp, struct brcmf_wsec_key *key) { - struct wiphy *wiphy = ifp->drvr->wiphy; + struct brcmf_pub *drvr = ifp->drvr; int err; struct brcmf_wsec_key_le key_le; @@ -469,7 +469,7 @@ send_key_to_dongle(struct brcmf_if *ifp, struct brcmf_wsec_key *key) sizeof(key_le)); if (err) - bphy_err(wiphy, "wsec_key error (%d)\n", err); + bphy_err(drvr, "wsec_key error (%d)\n", err); return err; } @@ -509,7 +509,7 @@ static int brcmf_get_first_free_bsscfgidx(struct brcmf_pub *drvr) static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp) { - struct wiphy *wiphy = ifp->drvr->wiphy; + struct brcmf_pub *drvr = ifp->drvr; struct brcmf_mbss_ssid_le mbss_ssid_le; int bsscfgidx; int err; @@ -526,7 +526,7 @@ static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp) err = brcmf_fil_bsscfg_data_set(ifp, "bsscfg:ssid", &mbss_ssid_le, sizeof(mbss_ssid_le)); if (err < 0) - bphy_err(wiphy, "setting ssid failed %d\n", err); + bphy_err(drvr, "setting ssid failed %d\n", err); return err; } @@ -544,6 +544,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, { struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy); struct brcmf_if *ifp = netdev_priv(cfg_to_ndev(cfg)); + struct brcmf_pub *drvr = cfg->pub; struct brcmf_cfg80211_vif *vif; int err; @@ -569,7 +570,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, BRCMF_VIF_EVENT_TIMEOUT); brcmf_cfg80211_arm_vif_event(cfg, NULL); if (!err) { - bphy_err(wiphy, "timeout occurred\n"); + bphy_err(drvr, "timeout occurred\n"); err = -EIO; goto fail; } @@ -577,7 +578,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, /* interface created in firmware */ ifp = vif->ifp; if (!ifp) { - bphy_err(wiphy, "no if pointer provided\n"); + bphy_err(drvr, "no if pointer provided\n"); err = -ENOENT; goto fail; } @@ -585,7 +586,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, strncpy(ifp->ndev->name, name, sizeof(ifp->ndev->name) - 1); err = brcmf_net_attach(ifp, true); if (err) { - bphy_err(wiphy, "Registering netdevice failed\n"); + bphy_err(drvr, "Registering netdevice failed\n"); free_netdev(ifp->ndev); goto fail; } @@ -616,13 +617,15 @@ static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy, enum nl80211_iftype type, struct vif_params *params) { + struct brcmf_cfg8021
Re: [PATCH V3] brcmfmac: use bphy_err() in all wiphy-related code
On Fri, 15 Feb 2019 at 07:43, Rafał Miłecki wrote: > From: Rafał Miłecki > > This recently added macro provides more meaningful error messages thanks > to identifying a specific wiphy. It's especially important on systems > with few cards supported by the same (brcmfmac) driver. > > Signed-off-by: Rafał Miłecki > Acked-by: Arend van Spriel > --- > V2: Fix one line over 80 chars > V3: Pass drvr to the brcmf_fweh_call_event_handler() to make getting > wiphy 100% reliable Kalle: please kindly hold on with this patch until we're done discussing bphy_err(). Mark it as deferred maybe?
Re: [PATCH] brcmfmac: use bphy_err() in all wiphy-related code
On Fri, 15 Feb 2019 at 11:04, Kalle Valo wrote: > Dan Carpenter writes: > > > Hi Rafał, > > > > url: > > https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/brcmfmac-use-bphy_err-in-all-wiphy-related-code/20190214-140004 > > base: > > https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git > > master > > > > smatch warnings: > > drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:114 > > brcmf_fweh_call_event_handler() warn: variable dereferenced before > > check 'ifp' (see line 110) > > drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:187 > > brcmf_fweh_handle_if_event() error: we previously assumed 'ifp' could > > be null (see line 184) > > drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:189 > > brcmf_fweh_handle_if_event() warn: variable dereferenced before check > > 'ifp' (see line 187) > > > > # > > https://github.com/0day-ci/linux/commit/e12dba9f5ed77216c5984a4b57ddc31ba23376c9 > > git remote add linux-review https://github.com/0day-ci/linux > > git remote update linux-review > > git checkout e12dba9f5ed77216c5984a4b57ddc31ba23376c9 > > vim +/ifp +114 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c > > > > 5c36b99a drivers/net/wireless/brcm80211/brcmfmac/fweh.c Arend van > > Spriel 2012-11-14 104 > > 3e0a97e1 drivers/net/wireless/brcm80211/brcmfmac/fweh.c Arend van > > Spriel 2012-11-14 105 static int brcmf_fweh_call_event_handler(struct > > brcmf_if *ifp, > > 3e0a97e1 drivers/net/wireless/brcm80211/brcmfmac/fweh.c Arend van > > Spriel 2012-11-14 106 enum brcmf_fweh_event_code code, > > 3e0a97e1 drivers/net/wireless/brcm80211/brcmfmac/fweh.c Arend van > > Spriel 2012-11-14 107 struct brcmf_event_msg *emsg, > > 3e0a97e1 drivers/net/wireless/brcm80211/brcmfmac/fweh.c Arend van > > Spriel 2012-11-14 108 void *data) > > 3e0a97e1 drivers/net/wireless/brcm80211/brcmfmac/fweh.c Arend van > > This report is very hard to read as the lines seem to be wrapped. I think it's your client playing tricks on you. I got the report correctly formatted and so did patchwork: https://patchwork.kernel.org/comment/22483031/ -- Rafał
Re: [PATCH V3] brcmfmac: use bphy_err() in all wiphy-related code
On Fri, 15 Feb 2019 at 10:40, Arend Van Spriel wrote: > On 2/15/2019 9:37 AM, Rafał Miłecki wrote: > > On Fri, 15 Feb 2019 at 07:43, Rafał Miłecki wrote: > >> From: Rafał Miłecki > >> > >> This recently added macro provides more meaningful error messages thanks > >> to identifying a specific wiphy. It's especially important on systems > >> with few cards supported by the same (brcmfmac) driver. > >> > >> Signed-off-by: Rafał Miłecki > >> Acked-by: Arend van Spriel > > > > Arend, let me ask one more question before getting this patch pushed. > > It's quite late (I spent quite some time on this), but maybe still > > better than fixing it later. > > > > It seems the most common struct that is: > > 1) Often passed as argument > > 2) Often having it's own variable in functions > > 3) Used when calling functions from different file > > is struct brcmf_pub. > > That is true for the function above the bus layer. In sdio/usb/pcie we > can probably get it, but it is less straight forward. Maybe there we > just want to use dev_err() as the struct device is short at hand there. Correct. For the bus level I mean to continue using dev_err() just like started with the commits: 5cc898fbcb35 ("brcmfmac: modify __brcmf_err() to take bus as a parameter") 8602e62441ab ("brcmfmac: pass bus to the __brcmf_err() in pcie.c") > > Now I started wondering if we really want to have bphy_err() accept > > wiphy. Maybe it would match brcmfmac's design better to have > > bphy_err(struct brcmf_pub, fmt, ...)? > > Just might, but you still want to expand that to wiphy_err() right? Correct! -- Rafał
Re: [PATCH V3] brcmfmac: use bphy_err() in all wiphy-related code
On Fri, 15 Feb 2019 at 07:43, Rafał Miłecki wrote: > From: Rafał Miłecki > > This recently added macro provides more meaningful error messages thanks > to identifying a specific wiphy. It's especially important on systems > with few cards supported by the same (brcmfmac) driver. > > Signed-off-by: Rafał Miłecki > Acked-by: Arend van Spriel Arend, let me ask one more question before getting this patch pushed. It's quite late (I spent quite some time on this), but maybe still better than fixing it later. It seems the most common struct that is: 1) Often passed as argument 2) Often having it's own variable in functions 3) Used when calling functions from different file is struct brcmf_pub. Now I started wondering if we really want to have bphy_err() accept wiphy. Maybe it would match brcmfmac's design better to have bphy_err(struct brcmf_pub, fmt, ...)?
[PATCH V3] brcmfmac: use bphy_err() in all wiphy-related code
From: Rafał Miłecki This recently added macro provides more meaningful error messages thanks to identifying a specific wiphy. It's especially important on systems with few cards supported by the same (brcmfmac) driver. Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel --- V2: Fix one line over 80 chars V3: Pass drvr to the brcmf_fweh_call_event_handler() to make getting wiphy 100% reliable --- .../broadcom/brcm80211/brcmfmac/bcdc.c| 26 +++--- .../broadcom/brcm80211/brcmfmac/common.c | 38 + .../broadcom/brcm80211/brcmfmac/core.c| 79 ++--- .../broadcom/brcm80211/brcmfmac/feature.c | 6 +- .../broadcom/brcm80211/brcmfmac/fweh.c| 32 --- .../broadcom/brcm80211/brcmfmac/fwil.c| 15 ++-- .../broadcom/brcm80211/brcmfmac/fwsignal.c| 36 +--- .../broadcom/brcm80211/brcmfmac/msgbuf.c | 65 -- .../broadcom/brcm80211/brcmfmac/p2p.c | 85 +++ .../broadcom/brcm80211/brcmfmac/pno.c | 22 +++-- .../broadcom/brcm80211/brcmfmac/proto.c | 7 +- 11 files changed, 247 insertions(+), 164 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c index 1068a2a4494c..9291dd7f6c56 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c @@ -169,6 +169,7 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, { struct brcmf_bcdc *bcdc = (struct brcmf_bcdc *)drvr->proto->pd; struct brcmf_proto_bcdc_dcmd *msg = &bcdc->msg; + struct wiphy *wiphy = drvr->wiphy; void *info; int ret = 0, retries = 0; u32 id, flags; @@ -178,8 +179,8 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, *fwerr = 0; ret = brcmf_proto_bcdc_msg(drvr, ifidx, cmd, buf, len, false); if (ret < 0) { - brcmf_err("brcmf_proto_bcdc_msg failed w/status %d\n", - ret); + bphy_err(wiphy, "brcmf_proto_bcdc_msg failed w/status %d\n", +ret); goto done; } @@ -195,9 +196,9 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, if ((id < bcdc->reqid) && (++retries < RETRIES)) goto retry; if (id != bcdc->reqid) { - brcmf_err("%s: unexpected request id %d (expected %d)\n", - brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, - bcdc->reqid); + bphy_err(wiphy, "%s: unexpected request id %d (expected %d)\n", +brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, +bcdc->reqid); ret = -EINVAL; goto done; } @@ -227,6 +228,7 @@ brcmf_proto_bcdc_set_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, { struct brcmf_bcdc *bcdc = (struct brcmf_bcdc *)drvr->proto->pd; struct brcmf_proto_bcdc_dcmd *msg = &bcdc->msg; + struct wiphy *wiphy = drvr->wiphy; int ret; u32 flags, id; @@ -245,9 +247,9 @@ brcmf_proto_bcdc_set_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, id = (flags & BCDC_DCMD_ID_MASK) >> BCDC_DCMD_ID_SHIFT; if (id != bcdc->reqid) { - brcmf_err("%s: unexpected request id %d (expected %d)\n", - brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, - bcdc->reqid); + bphy_err(wiphy, "%s: unexpected request id %d (expected %d)\n", +brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, +bcdc->reqid); ret = -EINVAL; goto done; } @@ -290,6 +292,7 @@ static int brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, struct sk_buff *pktbuf, struct brcmf_if **ifp) { + struct wiphy *wiphy = drvr->wiphy; struct brcmf_proto_bcdc_header *h; struct brcmf_if *tmp_if; @@ -312,8 +315,8 @@ brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, } if (((h->flags & BCDC_FLAG_VER_MASK) >> BCDC_FLAG_VER_SHIFT) != BCDC_PROTO_VER) { - brcmf_err("%s: non-BCDC packet received, flags 0x%x\n", - brcmf_ifname(tmp_if), h->flags); + bphy_err(wiphy, "%s: non-BCDC packet received, flags 0x%x\n", +brcmf_ifname(tmp_if), h->flags); return -EBADE; } @@ -452,6 +455,7 @@ static void brcmf_proto_bcdc_debugfs_create(struct brcmf_pub *drvr) int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr) { + stru
Re: brcmfmac: NULL pointer dereference during brcmf_detach() after firmware crash
On Thu, 14 Feb 2019 at 23:37, Arend Van Spriel wrote: > On 2/14/2019 11:30 PM, Rafał Miłecki wrote: > > I've just found a well reproducible brcmfmac crash (NULL pointer > > dereference). > > > > Steps: > > 1. Wait for or trigger a FullMAC firmware crash > > 2. Wait for some skb to get queued on a flowring > > 3. Call rmmod brcmfmac > > > > Problem: > > There is a NULL pointer dereference in one of the brcmf_detach() calls. > > > > Explanation: > > brcmf_detach() first frees all "ifp"s and then deletes flowrings. If any > > flowring has a skb it results in calling brcmf_txfinalize() which tries > > to access "ifp" (struct brcmf_if) which is a NULL. > > Hi Rafał, > > Thanks for diving in. That was my suspicion. Does it mean you are > working on a patch or shall I take care of it. It would be nice to have someone more experienced with detaching & rings look at it. Is adding a simple if (ifp) enough? Or should that code get redesigned? Should we e.g. reorder detach order? -- Rafał
brcmfmac: NULL pointer dereference during brcmf_detach() after firmware crash
Hi, I've just found a well reproducible brcmfmac crash (NULL pointer dereference). Steps: 1. Wait for or trigger a FullMAC firmware crash 2. Wait for some skb to get queued on a flowring 3. Call rmmod brcmfmac Problem: There is a NULL pointer dereference in one of the brcmf_detach() calls. Explanation: brcmf_detach() first frees all "ifp"s and then deletes flowrings. If any flowring has a skb it results in calling brcmf_txfinalize() which tries to access "ifp" (struct brcmf_if) which is a NULL. Pseudo-code forward-trace: brcmf_detach foreach brcmf_remove_interface brcmf_del_if brcmf_net_detach unregister_netdev(ice) brcmf_proto_detach brcmf_proto_msgbuf_detach brcmf_flowring_detach foreach brcmf_msgbuf_delete_flowring brcmf_msgbuf_remove_flowring brcmf_flowring_delete brcmf_txfinalize [ 2608.673022] brcmfmac: brcmf_msgbuf_delete_flowring: Failed to submit RING_DELETE, flowring will be removed [ 2608.682938] Unable to handle kernel NULL pointer dereference at virtual address 0020 [ 2608.691066] pgd = c4d2c000 [ 2608.693807] [0020] *pgd=05337831, *pte=, *ppte= [ 2608.700115] Internal error: Oops: 17 [#1] SMP ARM [ 2608.704828] Modules linked in: pppoe ppp_async l2tp_ppp iptable_nat brcmfmac(-) pptp pppox ppp_mppe ppp_generic nf_nat_ipv4 nf_conntrack_ipv6 nf_conntrack_ipv4 ipt_REJECT ipt_MASQUERADE cfg80211 xt_time xt_tcpudp xt_tcpmss xt_statistic xt_state xt_recent xt_policy xtc [ 2608.889523] CPU: 1 PID: 4798 Comm: rmmod Not tainted 4.4.167 #0 [ 2608.895453] Hardware name: BCM5301X [ 2608.898946] task: c4d44000 ti: c4d06000 task.ti: c4d06000 [ 2608.904380] PC is at brcmf_txfinalize+0x7c/0x90 [brcmfmac] [ 2608.909886] LR is at brcmf_flowring_delete+0xa4/0xb4 [brcmfmac] [ 2608.915814] pc : []lr : []psr: 6013 [ 2608.915814] sp : c4d07d28 ip : c4d07d48 fp : c4d07d44 [ 2608.927319] r10: r9 : c58ec080 r8 : [ 2608.932551] r7 : c58ed080 r6 : r5 : c5386b00 r4 : [ 2608.939088] r3 : 0806 r2 : 888e r1 : c5386b00 r0 : c5386b00 [ 2608.945626] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 2608.952770] Control: 10c5387d Table: 04d2c04a DAC: 0051 [ 2608.958523] Process rmmod (pid: 4798, stack limit = 0xc4d06190) [ 2608.964451] Stack: (0xc4d07d28 to 0xc4d08000) [ 2608.968814] 7d20: c4c91708 c4c91700 c58ed080 c4d07d6c c4d07d48 [ 2608.977014] 7d40: bf548600 bf54409c c66e4e00 c7af2ae8 c0019b8c cc486000 05348000 [ 2608.985213] 7d60: c4d07d9c c4d07d70 bf548dcc bf548568 c4d07d98 c642 [ 2608.993412] 7d80: fffb c66e4e00 c5954000 c4d06000 c4d07dc4 c4d07da0 bf54a48c bf548d14 [ 2609.001611] 7da0: c0032794 c58ec080 c58ed080 c58ed4cc c4d07dec c4d07dc8 [ 2609.009810] 7dc0: bf548718 bf54a3cc c66e4e00 c66e4ea4 c5954000 0100 c00098c4 c4d06000 [ 2609.018010] 7de0: c4d07e1c c4d07df0 bf54a9d0 bf5486d0 c4d07e1c c51b c5954000 c6708300 [ 2609.026209] 7e00: c5954098 c5954098 c00098c4 c4d06000 c4d07e34 c4d07e20 bf5425cc bf54a944 [ 2609.034409] 7e20: c5954000 c6708300 c4d07e54 c4d07e38 bf544fcc bf542580 c5917380 c6708300 [ 2609.042608] 7e40: c7af2a80 0081 c4d07e74 c4d07e58 bf54d318 bf544f04 c7af2ae8 c7af2a80 [ 2609.050807] 7e60: c7af2b1c 0081 c4d07e8c c4d07e78 c01d71cc bf54d2cc c7af2ae8 bf5599c8 [ 2609.059006] 7e80: c4d07ea4 c4d07e90 c01ff0b4 c01d71a4 c7af2ae8 bf5599c8 c4d07ec4 c4d07ea8 [ 2609.067205] 7ea0: c01ff20c c01ff03c bf5599c8 c04e03c8 b6f25c00 0081 c4d07edc c4d07ec8 [ 2609.075404] 7ec0: c01fe694 c01ff16c bf5599c8 c04e03c8 c4d07ef4 c4d07ee0 c01ff65c c01fe634 [ 2609.083603] 7ee0: bf559994 c04e03c8 c4d07f14 c4d07ef8 c01d6b38 c01ff624 bf559c80 c04e03c8 [ 2609.091802] 7f00: b6f25c00 0081 c4d07f24 c4d07f18 bf54ea34 c01d6b2c c4d07f34 c4d07f28 [ 2609.11] 7f20: bf54edfc bf54ea1c c4d07f44 c4d07f38 bf54edb4 bf54edec c4d07fa4 c4d07f48 [ 2609.108200] 7f40: c0078234 bf54edb0 6d637262 63616d66 c4d07f60 c00c63cc c00c61c4 [ 2609.116400] 7f60: c4d07f8c c4d07f70 c003805c c000f0c8 c00098c4 c4d06010 c4d07fb0 c4d06000 [ 2609.124599] 7f80: 00d07fac dc8ba6a7 c0015ba0 00010034 1fbe3c7e 0002302c c4d07fa8 [ 2609.132797] 7fa0: c0009700 c0078134 00010034 1fbe3c7e b6f25c00 [ 2609.140988] 7fc0: 00010034 1fbe3c7e 0002302c 0081 bed23e74 0001 [ 2609.149187] 7fe0: bed23d94 bed23d78 00010e98 b6f5e6e4 a010 b6f25c00 [ 2609.157374] Backtrace: [ 2609.159854] [] (brcmf_txfinalize [brcmfmac]) from [] (brcmf_flowring_delete+0xa4/0xb4 [brcmfmac]) [ 2609.170482] r7:c58ed080 r6: r5:c4c91700 r4:c4c91708 [ 2609.176202] [] (brcmf_flowring_delete [brcmfmac]) from [] (brcmf_msgbuf_rxreorder+0x3d4/0x84c [brcmfmac]) [ 2609.187525] r9:0
[PATCH V2] brcmfmac: use bphy_err() in all wiphy-related code
From: Rafał Miłecki This recently added macro provides more meaningful error messages thanks to identifying a specific wiphy. It's especially important on systems with few cards supported by the same (brcmfmac) driver. Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel --- V2: Fix one line over 80 chars --- .../broadcom/brcm80211/brcmfmac/bcdc.c| 26 +++--- .../broadcom/brcm80211/brcmfmac/common.c | 38 + .../broadcom/brcm80211/brcmfmac/core.c| 79 ++--- .../broadcom/brcm80211/brcmfmac/feature.c | 6 +- .../broadcom/brcm80211/brcmfmac/fweh.c| 22 +++-- .../broadcom/brcm80211/brcmfmac/fwil.c| 15 ++-- .../broadcom/brcm80211/brcmfmac/fwsignal.c| 36 +--- .../broadcom/brcm80211/brcmfmac/msgbuf.c | 65 -- .../broadcom/brcm80211/brcmfmac/p2p.c | 85 +++ .../broadcom/brcm80211/brcmfmac/pno.c | 22 +++-- .../broadcom/brcm80211/brcmfmac/proto.c | 7 +- 11 files changed, 241 insertions(+), 160 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c index 1068a2a4494c..9291dd7f6c56 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c @@ -169,6 +169,7 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, { struct brcmf_bcdc *bcdc = (struct brcmf_bcdc *)drvr->proto->pd; struct brcmf_proto_bcdc_dcmd *msg = &bcdc->msg; + struct wiphy *wiphy = drvr->wiphy; void *info; int ret = 0, retries = 0; u32 id, flags; @@ -178,8 +179,8 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, *fwerr = 0; ret = brcmf_proto_bcdc_msg(drvr, ifidx, cmd, buf, len, false); if (ret < 0) { - brcmf_err("brcmf_proto_bcdc_msg failed w/status %d\n", - ret); + bphy_err(wiphy, "brcmf_proto_bcdc_msg failed w/status %d\n", +ret); goto done; } @@ -195,9 +196,9 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, if ((id < bcdc->reqid) && (++retries < RETRIES)) goto retry; if (id != bcdc->reqid) { - brcmf_err("%s: unexpected request id %d (expected %d)\n", - brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, - bcdc->reqid); + bphy_err(wiphy, "%s: unexpected request id %d (expected %d)\n", +brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, +bcdc->reqid); ret = -EINVAL; goto done; } @@ -227,6 +228,7 @@ brcmf_proto_bcdc_set_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, { struct brcmf_bcdc *bcdc = (struct brcmf_bcdc *)drvr->proto->pd; struct brcmf_proto_bcdc_dcmd *msg = &bcdc->msg; + struct wiphy *wiphy = drvr->wiphy; int ret; u32 flags, id; @@ -245,9 +247,9 @@ brcmf_proto_bcdc_set_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, id = (flags & BCDC_DCMD_ID_MASK) >> BCDC_DCMD_ID_SHIFT; if (id != bcdc->reqid) { - brcmf_err("%s: unexpected request id %d (expected %d)\n", - brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, - bcdc->reqid); + bphy_err(wiphy, "%s: unexpected request id %d (expected %d)\n", +brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, +bcdc->reqid); ret = -EINVAL; goto done; } @@ -290,6 +292,7 @@ static int brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, struct sk_buff *pktbuf, struct brcmf_if **ifp) { + struct wiphy *wiphy = drvr->wiphy; struct brcmf_proto_bcdc_header *h; struct brcmf_if *tmp_if; @@ -312,8 +315,8 @@ brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, } if (((h->flags & BCDC_FLAG_VER_MASK) >> BCDC_FLAG_VER_SHIFT) != BCDC_PROTO_VER) { - brcmf_err("%s: non-BCDC packet received, flags 0x%x\n", - brcmf_ifname(tmp_if), h->flags); + bphy_err(wiphy, "%s: non-BCDC packet received, flags 0x%x\n", +brcmf_ifname(tmp_if), h->flags); return -EBADE; } @@ -452,6 +455,7 @@ static void brcmf_proto_bcdc_debugfs_create(struct brcmf_pub *drvr) int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr) { + struct wiphy *wiphy = drvr->wiphy; struct brcmf_bcdc *bcdc; bcdc = kzalloc(sizeof(*bcdc), GFP_
Re: [PATCH] brcmfmac: use bphy_err() in all wiphy-related code
On Thu, 14 Feb 2019 at 13:38, Arend Van Spriel wrote: > On 2/13/2019 12:26 PM, Rafał Miłecki wrote: > > From: Rafał Miłecki > > > > This recently added macro provides more meaningful error messages thanks > > to identifying a specific wiphy. It's especially important on systems > > with few cards supported by the same (brcmfmac) driver. > > Acked-by: Arend van Spriel > > Signed-off-by: Rafał Miłecki > > --- > > I obviously couldn't hit & test all error cases. To perform some basic > > testing I added: > > bphy_err(wiphy, "test\n"); > > for every wiphy variable added by this patch & tested in on BCM4366. > > > > I got a lot of messages like: > > [ 2842.424524] ieee80211 phy1: brcmf_netdev_start_xmit: test > > [ 2842.434921] ieee80211 phy1: brcmf_msgbuf_txflow: test > > [ 2842.440275] ieee80211 phy1: brcmf_msgbuf_process_msgtype: test > > [ 2842.446141] ieee80211 phy1: brcmf_get_ifp: test > > so it seems to work correctly. > > I was thinking about fact that module name is no longer prefixed, but I > am not sure that is a bad thing here. I think including function name should allow identifying error message in 99,9% cases. If for some reason we want the "brcmfmac: " prefix, it should only take a simple change to the bphy_err() macro. > > .../broadcom/brcm80211/brcmfmac/bcdc.c| 26 +++--- > > .../broadcom/brcm80211/brcmfmac/common.c | 38 + > > .../broadcom/brcm80211/brcmfmac/core.c| 78 ++--- > > .../broadcom/brcm80211/brcmfmac/feature.c | 6 +- > > .../broadcom/brcm80211/brcmfmac/fweh.c| 22 +++-- > > .../broadcom/brcm80211/brcmfmac/fwil.c| 15 ++-- > > .../broadcom/brcm80211/brcmfmac/fwsignal.c| 36 +--- > > .../broadcom/brcm80211/brcmfmac/msgbuf.c | 65 -- > > .../broadcom/brcm80211/brcmfmac/p2p.c | 85 +++ > > .../broadcom/brcm80211/brcmfmac/pno.c | 22 +++-- > > .../broadcom/brcm80211/brcmfmac/proto.c | 7 +- > > 11 files changed, 240 insertions(+), 160 deletions(-) > > [...] > > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c > > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c > > index e772c0845638..2e0e2badfb80 100644 > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c > > [...] > > > @@ -141,7 +142,8 @@ void brcmf_configure_arp_nd_offload(struct brcmf_if > > *ifp, bool enable) > > > > static void _brcmf_set_multicast_list(struct work_struct *work) > > { > > - struct brcmf_if *ifp; > > + struct brcmf_if *ifp = container_of(work, struct brcmf_if, > > multicast_work); > > checkpatch complains about the length here. One little warning. I thought it may pass unnoticed ;) Let me fix that.
Re: PROBLEM: Oops - Null pointer dereference in brcm80211/brcmfmac [BCM4350]
On 12.02.2019 06:57, Aaron Blair wrote: [18990.872335] brcmfmac: brcmf_cfg80211_get_station: GET STA INFO failed, -12 [18990.872433] brcmfmac: brcmf_msgbuf_tx_ioctl: Failed to reserve space in commonring [18990.872439] brcmfmac: brcmf_cfg80211_get_tx_power: error (-12) FullMAC firmware (the one running on card's ARM CPU) has crashed. Upon trying to rmmod && modload brcmfmac, I receive the oops dump included below, and am unable to continue using the wireless NIC until I reboot. Interestingly I've experienced the same problem (for the first time) last week. I started working on it but didn't achieve anything yet.
[PATCH] brcmfmac: use bphy_err() in all wiphy-related code
From: Rafał Miłecki This recently added macro provides more meaningful error messages thanks to identifying a specific wiphy. It's especially important on systems with few cards supported by the same (brcmfmac) driver. Signed-off-by: Rafał Miłecki --- I obviously couldn't hit & test all error cases. To perform some basic testing I added: bphy_err(wiphy, "test\n"); for every wiphy variable added by this patch & tested in on BCM4366. I got a lot of messages like: [ 2842.424524] ieee80211 phy1: brcmf_netdev_start_xmit: test [ 2842.434921] ieee80211 phy1: brcmf_msgbuf_txflow: test [ 2842.440275] ieee80211 phy1: brcmf_msgbuf_process_msgtype: test [ 2842.446141] ieee80211 phy1: brcmf_get_ifp: test so it seems to work correctly. --- .../broadcom/brcm80211/brcmfmac/bcdc.c| 26 +++--- .../broadcom/brcm80211/brcmfmac/common.c | 38 + .../broadcom/brcm80211/brcmfmac/core.c| 78 ++--- .../broadcom/brcm80211/brcmfmac/feature.c | 6 +- .../broadcom/brcm80211/brcmfmac/fweh.c| 22 +++-- .../broadcom/brcm80211/brcmfmac/fwil.c| 15 ++-- .../broadcom/brcm80211/brcmfmac/fwsignal.c| 36 +--- .../broadcom/brcm80211/brcmfmac/msgbuf.c | 65 -- .../broadcom/brcm80211/brcmfmac/p2p.c | 85 +++ .../broadcom/brcm80211/brcmfmac/pno.c | 22 +++-- .../broadcom/brcm80211/brcmfmac/proto.c | 7 +- 11 files changed, 240 insertions(+), 160 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c index 1068a2a4494c..9291dd7f6c56 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c @@ -169,6 +169,7 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, { struct brcmf_bcdc *bcdc = (struct brcmf_bcdc *)drvr->proto->pd; struct brcmf_proto_bcdc_dcmd *msg = &bcdc->msg; + struct wiphy *wiphy = drvr->wiphy; void *info; int ret = 0, retries = 0; u32 id, flags; @@ -178,8 +179,8 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, *fwerr = 0; ret = brcmf_proto_bcdc_msg(drvr, ifidx, cmd, buf, len, false); if (ret < 0) { - brcmf_err("brcmf_proto_bcdc_msg failed w/status %d\n", - ret); + bphy_err(wiphy, "brcmf_proto_bcdc_msg failed w/status %d\n", +ret); goto done; } @@ -195,9 +196,9 @@ brcmf_proto_bcdc_query_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, if ((id < bcdc->reqid) && (++retries < RETRIES)) goto retry; if (id != bcdc->reqid) { - brcmf_err("%s: unexpected request id %d (expected %d)\n", - brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, - bcdc->reqid); + bphy_err(wiphy, "%s: unexpected request id %d (expected %d)\n", +brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, +bcdc->reqid); ret = -EINVAL; goto done; } @@ -227,6 +228,7 @@ brcmf_proto_bcdc_set_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, { struct brcmf_bcdc *bcdc = (struct brcmf_bcdc *)drvr->proto->pd; struct brcmf_proto_bcdc_dcmd *msg = &bcdc->msg; + struct wiphy *wiphy = drvr->wiphy; int ret; u32 flags, id; @@ -245,9 +247,9 @@ brcmf_proto_bcdc_set_dcmd(struct brcmf_pub *drvr, int ifidx, uint cmd, id = (flags & BCDC_DCMD_ID_MASK) >> BCDC_DCMD_ID_SHIFT; if (id != bcdc->reqid) { - brcmf_err("%s: unexpected request id %d (expected %d)\n", - brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, - bcdc->reqid); + bphy_err(wiphy, "%s: unexpected request id %d (expected %d)\n", +brcmf_ifname(brcmf_get_ifp(drvr, ifidx)), id, +bcdc->reqid); ret = -EINVAL; goto done; } @@ -290,6 +292,7 @@ static int brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, struct sk_buff *pktbuf, struct brcmf_if **ifp) { + struct wiphy *wiphy = drvr->wiphy; struct brcmf_proto_bcdc_header *h; struct brcmf_if *tmp_if; @@ -312,8 +315,8 @@ brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, } if (((h->flags & BCDC_FLAG_VER_MASK) >> BCDC_FLAG_VER_SHIFT) != BCDC_PROTO_VER) { - brcmf_err("%s: non-BCDC packet received, flags 0x%x\n", - brcmf_ifname(tmp_if), h->flags); +
[PATCH 1/2] brcmfmac: improve code handling bandwidth of firmware reported channels
From: Rafał Miłecki 1) Use switch to simplify/improve the code & avoid some duplication 2) Add warning for unsupported values Signed-off-by: Rafał Miłecki --- .../wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index b5e291ed9496..fa752ec04f22 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -6049,11 +6049,18 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg, /* assuming the chanspecs order is HT20, * HT40 upper, HT40 lower, and VHT80. */ - if (ch.bw == BRCMU_CHAN_BW_80) { + switch (ch.bw) { + case BRCMU_CHAN_BW_80: channel->flags &= ~IEEE80211_CHAN_NO_80MHZ; - } else if (ch.bw == BRCMU_CHAN_BW_40) { + break; + case BRCMU_CHAN_BW_40: brcmf_update_bw40_channel_flag(channel, &ch); - } else { + break; + default: + wiphy_warn(wiphy, "Firmware reported unsupported bandwidth %d\n", + ch.bw); + /* fall through */ + case BRCMU_CHAN_BW_20: /* enable the channel and disable other bandwidths * for now as mentioned order assure they are enabled * for subsequent chanspecs. -- 2.20.1
[PATCH 2/2] brcmfmac: support firmware reporting 160 MHz channels
From: Rafał Miłecki So far 160 MHz channels were treated as 20 MHz ones which was breaking support for 40/80 MHz due to the brcmf_construct_chaninfo() logic and its assumptions. Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index fa752ec04f22..b3611f0f68bf 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -6050,6 +6050,9 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg, * HT40 upper, HT40 lower, and VHT80. */ switch (ch.bw) { + case BRCMU_CHAN_BW_160: + channel->flags &= ~IEEE80211_CHAN_NO_160MHZ; + break; case BRCMU_CHAN_BW_80: channel->flags &= ~IEEE80211_CHAN_NO_80MHZ; break; -- 2.20.1
brcmfmac: race in init path related to the wiphy_register() call
Hi, During debugging some potential problems I hit a race bug in the brcmfmac. It calls wiphy_register() before it's ready to handle all the cfg80211 callbacks correctly. In brcmf_cfg80211_attach() there is a wiphy_register() call performed before setting "config" in the struct brcmf_pub. So if: 1) User tries to create interface *right* after wiphy appears 2) brcmf_cfg80211_add_iface() gets executed quickly enough 3) Firmware sends BRCMF_E_IF event Then brcmf_notify_vif_event() will crash the kernel: [ 32.071904] [0300] *pgd= [ 32.075562] Internal error: Oops: 17 [#1] SMP ARM [ 32.080277] Modules linked in: pppoe ppp_async l2tp_ppp iptable_nat brcmfmac pptp pppox ppp_mppe ppp_generic nf_nat_ipv4 nf_conntrack_ipv6 nf_conntrack_ipv4 ipt_REJECT ipt_MASQUERADE cfg80211 xt_time xt_tcpudp xt_tcpmss xt_statistic xt_state xt_recent xt_policy xt_na0 [ 32.178487] ip_set_hash_ipportip ip_set_hash_ipport ip_set_hash_ipmark ip_set_hash_ip ip_set_bitmap_port ip_set_bitmap_ipmac ip_set_bitmap_ip ip_set nfnetlink ip6t_REJECT nf_reject_ipv6 nf_log_ipv6 nf_log_common ip6table_mangle ip6table_filter ip6_tables x_tables msc [ 32.274776] CPU: 1 PID: 12 Comm: kworker/1:0 Not tainted 4.4.167 #0 [ 32.281054] Hardware name: BCM5301X [ 32.284580] Workqueue: events brcmf_fweh_event_worker [brcmfmac] [ 32.290604] task: c7845980 ti: c7872000 task.ti: c7872000 [ 32.296017] PC is at _raw_spin_lock+0x10/0x4c [ 32.300388] LR is at brcmf_notify_vif_event+0x6c/0x1a4 [brcmfmac] [ 32.306489] pc : []lr : []psr: 6013 [ 32.306489] sp : c7873e18 ip : c7873e28 fp : c7873e24 [ 32.317995] r10: c7873eaa r9 : c7873ea4 r8 : c59c8000 [ 32.323227] r7 : c535e948 r6 : c4d50480 r5 : r4 : 0300 [ 32.329763] r3 : r2 : c7873e24 r1 : bf54f7b3 r0 : 0300 [ 32.336301] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 32.343445] Control: 10c5387d Table: 04cf004a DAC: 0051 [ 32.349199] Process kworker/1:0 (pid: 12, stack limit = 0xc7872190) [ 32.355474] Stack: (0xc7873e18 to 0xc7874000) [ 32.359836] 3e00: c7873e54 c7873e28 [ 32.368029] 3e20: bf5387dc c0012094 0001 0002 c7873e2c bf538770 c59ca138 [ 32.376228] 3e40: c4d50480 c7873e6c c7873e58 bf53c1e8 bf53877c c535e900 c59ca138 [ 32.384426] 3e60: c7873eec c7873e70 bf53c578 bf53c1b8 c7873eaa c7873ea4 0001 [ 32.392617] 3e80: 72ed795e c04e03c8 0089 0002 0036 [ 32.400816] 3ea0: 0005 6a4e13a4 6c77b80b 00322e30 0201 dc8ba6a7 [ 32.409007] 3ec0: c7a2eec0 c78333c0 c6bda300 c59ca138 c78333d8 c6bdd700 0008 [ 32.417206] 3ee0: c7873f2c c7873ef0 c0034458 bf53c27c c6bda300 c6bda300 c78333d8 [ 32.425405] 3f00: c78333c0 c6bda300 c6bda300 c78333d8 c6bda314 0008 [ 32.433604] 3f20: c7873f5c c7873f30 c0035234 c0034284 c7845980 c7832700 c78333c0 [ 32.441803] 3f40: c0034f40 c7873fac c7873f60 c003984c c0034f4c [ 32.449994] 3f60: c78333c0 c7873f78 c7873f78 [ 32.458184] 3f80: c7873f88 c7873f88 c7832700 c003974c [ 32.466383] 3fa0: c7873fb0 c00097d0 c0039758 [ 32.474573] 3fc0: [ 32.482764] 3fe0: 0013 [ 32.490950] Backtrace: [ 32.493420] [] (_raw_spin_lock) from [] (brcmf_notify_vif_event+0x6c/0x1a4 [brcmfmac]) [ 32.503112] [] (brcmf_notify_vif_event [brcmfmac]) from [] (brcmf_fil_bsscfg_int_get+0x78/0xb0 [brcmfmac]) [ 32.514521] r7: r6:c4d50480 r5:c59ca138 r4:bf538770 [ 32.520240] [] (brcmf_fil_bsscfg_int_get [brcmfmac]) from [] (brcmf_fweh_event_worker+0x308/0x3d4 [brcmfmac]) [ 32.531912] r5:c59ca138 r4:c535e900 [ 32.535524] [] (brcmf_fweh_event_worker [brcmfmac]) from [] (process_one_work+0x1e0/0x318) [ 32.545543] r10:0008 r9:c6bdd700 r8: r7:c78333d8 r6:c59ca138 r5:c6bda300 [ 32.553428] r4:c78333c0 [ 32.555975] [] (process_one_work) from [] (worker_thread+0x2f4/0x448) [ 32.564169] r10:0008 r9: r8:c6bda314 r7:c78333d8 r6:c6bda300 r5:c6bda300 [ 32.572055] r4:c78333c0 [ 32.574607] [] (worker_thread) from [] (kthread+0x100/0x114) [ 32.582012] r10: r9: r8: r7:c0034f40 r6:c78333c0 r5: [ 32.589898] r4:c7832700 r3:c7845980 [ 32.593499] [] (kthread) from [] (ret_from_fork+0x14/0x24) [ 32.600735] r7: r6: r5:c003974c r4:c7832700 [ 32.606440] Code: e1a0c00d e92dd800 e24cb004 f590f000 (e1902f9f) [ 32.612609] ---[ end trace 68af7a0bd4231f2f ]--- I think it was meant to be avoided by call brcmf_fweh_activate_events() being executed late enough but it do
[PATCH] brcmfmac: print firmware reported ring status errors
From: Rafał Miłecki Firmware is capable of reporting ring status. It's used e.g. to signal some problem with a specific ring setup. This patch adds support for printing ring & error number which may be useful for debugging setup issues. Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/msgbuf.c | 25 +++ 1 file changed, 25 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c index 4e8397a0cbc8..bbef2d41cb3b 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c @@ -134,6 +134,14 @@ struct msgbuf_completion_hdr { __le16 flow_ring_id; }; +/* Data struct for the MSGBUF_TYPE_RING_STATUS */ +struct msgbuf_ring_status { + struct msgbuf_common_hdrmsg; + struct msgbuf_completion_hdrcompl_hdr; + __le16 write_idx; + __le32 rsvd0[5]; +}; + struct msgbuf_rx_event { struct msgbuf_common_hdrmsg; struct msgbuf_completion_hdrcompl_hdr; @@ -1180,6 +1188,19 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) brcmf_netif_rx(ifp, skb); } +static void brcmf_msgbuf_process_ring_status(struct brcmf_msgbuf *msgbuf, +void *buf) +{ + struct msgbuf_ring_status *ring_status = buf; + int err; + + err = le16_to_cpu(ring_status->compl_hdr.status); + if (err) { + int ring = le16_to_cpu(ring_status->compl_hdr.flow_ring_id); + + brcmf_err("Firmware reported ring %d error: %d\n", ring, err); + } +} static void brcmf_msgbuf_process_flow_ring_create_response(struct brcmf_msgbuf *msgbuf, @@ -1241,6 +1262,10 @@ static void brcmf_msgbuf_process_msgtype(struct brcmf_msgbuf *msgbuf, void *buf) msg = (struct msgbuf_common_hdr *)buf; switch (msg->msgtype) { + case MSGBUF_TYPE_RING_STATUS: + brcmf_dbg(MSGBUF, "MSGBUF_TYPE_RING_STATUS\n"); + brcmf_msgbuf_process_ring_status(msgbuf, buf); + break; case MSGBUF_TYPE_FLOW_RING_CREATE_CMPLT: brcmf_dbg(MSGBUF, "MSGBUF_TYPE_FLOW_RING_CREATE_CMPLT\n"); brcmf_msgbuf_process_flow_ring_create_response(msgbuf, buf); -- 2.20.1
[PATCH V2] brcmfmac: support monitor frames with the hardware/ucode header
From: Rafał Miłecki So far there were two monitor frame formats: 1) 802.11 frames (with frame (sub)type & all addresses) 2) 802.11 frames with the radiotap header Testing the latest FullMAC firmwares for 4366b1/4366c0 resulted in discovering a new format being used. It seems (almost?) identical to the one known from ucode used in SoftMAC devices which is most likely the same codebase anyway. While new firmwares will /announce/ radiotap header support using the "rtap" fw capability string it seems no string was added for the new ucode header format. All above means that: 1) We need new format support when dealing with a received frame 2) A new feature bit & mapping quirks have to be added manually As for now only an empty radiotap is being created. Adding support for extracting some info (band, channel, signal, etc.) is planned for the future. Signed-off-by: Rafał Miłecki --- V2: Update commit message (only): 1) Don't say the new firmwares were expected to provide radiotap frames. That was my misinterpretation of adding "rtap" string. 2) List all monitor frame formats as it apparently got a bit confusing at this point (there are 3 different ones!). --- .../broadcom/brcm80211/brcmfmac/core.c| 55 +++ .../broadcom/brcm80211/brcmfmac/feature.c | 4 ++ .../broadcom/brcm80211/brcmfmac/feature.h | 4 +- 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 860a4372cb56..e772c0845638 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -43,6 +43,36 @@ #define BRCMF_BSSIDX_INVALID -1 +#defineRXS_PBPRES BIT(2) + +#defineD11_PHY_HDR_LEN 6 + +struct d11rxhdr_le { + __le16 RxFrameSize; + u16 PAD; + __le16 PhyRxStatus_0; + __le16 PhyRxStatus_1; + __le16 PhyRxStatus_2; + __le16 PhyRxStatus_3; + __le16 PhyRxStatus_4; + __le16 PhyRxStatus_5; + __le16 RxStatus1; + __le16 RxStatus2; + __le16 RxTSFTime; + __le16 RxChan; + u8 unknown[12]; +} __packed; + +struct wlc_d11rxhdr { + struct d11rxhdr_le rxhdr; + __le32 tsf_l; + s8 rssi; + s8 rxpwr0; + s8 rxpwr1; + s8 do_rssi_ma; + s8 rxpwr[4]; +} __packed; + char *brcmf_ifname(struct brcmf_if *ifp) { if (!ifp) @@ -409,6 +439,31 @@ void brcmf_netif_mon_rx(struct brcmf_if *ifp, struct sk_buff *skb) { if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR_FMT_RADIOTAP)) { /* Do nothing */ + } else if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR_FMT_HW_RX_HDR)) { + struct wlc_d11rxhdr *wlc_rxhdr = (struct wlc_d11rxhdr *)skb->data; + struct ieee80211_radiotap_header *radiotap; + unsigned int offset; + u16 RxStatus1; + + RxStatus1 = le16_to_cpu(wlc_rxhdr->rxhdr.RxStatus1); + + offset = sizeof(struct wlc_d11rxhdr); + /* MAC inserts 2 pad bytes for a4 headers or QoS or A-MSDU +* subframes +*/ + if (RxStatus1 & RXS_PBPRES) + offset += 2; + offset += D11_PHY_HDR_LEN; + + skb_pull(skb, offset); + + /* TODO: use RX header to fill some radiotap data */ + radiotap = skb_push(skb, sizeof(*radiotap)); + memset(radiotap, 0, sizeof(*radiotap)); + radiotap->it_len = cpu_to_le16(sizeof(*radiotap)); + + /* TODO: 4 bytes with receive status? */ + skb->len -= 4; } else { struct ieee80211_radiotap_header *radiotap; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c index 4c5a3995dc35..b91b7ecbfedf 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c @@ -103,6 +103,10 @@ static const struct brcmf_feat_fwfeat brcmf_feat_fwfeat_map[] = { { "01-6cb8e269", BIT(BRCMF_FEAT_MONITOR) }, /* brcmfmac4366b-pcie.bin from linux-firmware.git commit 52442afee990 */ { "01-c47a91a4", BIT(BRCMF_FEAT_MONITOR) }, + /* brcmfmac4366b-pcie.bin from linux-firmware.git commit 211de1679a68 */ + { "01-801fb449", BIT(BRCMF_FEAT_MONITOR_FMT_HW_RX_HDR) }, + /* brcmfmac4366c-pcie.bin from linux-firmware.git commit 211de1679a68 */ + { "01-d2cbb8fd", BIT(BRCMF_FEAT_MONITOR_FMT_HW_RX_HDR) }, }; static void brcmf_feat_firmware_overrides(struct brcmf_pub *drv) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/f
Re: [PATCH] brcmfmac: support monitor frames with hardware/ucode header
On Thu, 7 Feb 2019 at 17:36, Kalle Valo wrote: > Rafał Miłecki wrote: > > > From: Rafał Miłecki > > > > The new FullMAC firmwares for 4366b1/4366c0 were supposed to provide > > monitor frames with radiotap header but it doesn't seem to be the case. > > Testing the latest release resulted in discovering a new format being > > used. It seems (almost?) identical to the one known from ucode used in > > SoftMAC devices which is most likely the same codebase anyway. > > > > While radiotap support was meant to be announced with the "rtap" fw > > capability string it seems no alternative was added for the hw/ucode > > format. It means each firmware requires a feature mapping quirk. > > > > As for now only an empty radiotap is being created. Adding support for > > extracting some info (band, channel, signal, etc.) is planned for the > > future. > > > > Signed-off-by: Rafał Miłecki > > It's unclear what I should do with this patch, take it? It seems to me there are no objections regarding the code. It was only my commit message that was a bit confusing. I'll send V2 with updated commit message. -- Rafał
[PATCH V3 2/2] brcmfmac: pass bus to the __brcmf_err() in pcie.c
From: Rafał Miłecki This enables dev_err() usage (instead of pr_err()) in the __brcmf_err(). It makes error messages more meaningful and is important for debugging errors/bugs on systems with multiple brcmfmac supported devices. All bus files should follow & get updated similarly (soon). Signed-off-by: Rafał Miłecki --- V2: Modify pcie.c & not cfg80211.c. The later should use wiphy_err() V3: Fix CONFIG_PM=y compliation errors: error: redeclaration of 'bus' with no linkage --- .../broadcom/brcm80211/brcmfmac/debug.h | 2 + .../broadcom/brcm80211/brcmfmac/pcie.c| 59 +++ 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h index b499f90d94f6..c1f260718c8e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h @@ -52,6 +52,7 @@ void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...); /* Macro for error messages. When debugging / tracing the driver all error * messages are important to us. */ +#ifndef brcmf_err #define brcmf_err(fmt, ...)\ do {\ if (IS_ENABLED(CONFIG_BRCMDBG) || \ @@ -59,6 +60,7 @@ void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...); net_ratelimit())\ __brcmf_err(NULL, __func__, fmt, ##__VA_ARGS__);\ } while (0) +#endif #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 16d7dda965d8..8f68e30fa5fa 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -30,6 +30,15 @@ #include #include +/* Custom brcmf_err() that takes bus arg and passes it further */ +#define brcmf_err(bus, fmt, ...) \ + do {\ + if (IS_ENABLED(CONFIG_BRCMDBG) || \ + IS_ENABLED(CONFIG_BRCM_TRACING) || \ + net_ratelimit())\ + __brcmf_err(bus, __func__, fmt, ##__VA_ARGS__); \ + } while (0) + #include "debug.h" #include "bus.h" #include "commonring.h" @@ -531,6 +540,7 @@ static void brcmf_pcie_select_core(struct brcmf_pciedev_info *devinfo, u16 coreid) { const struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); struct brcmf_core *core; u32 bar0_win; @@ -548,7 +558,7 @@ brcmf_pcie_select_core(struct brcmf_pciedev_info *devinfo, u16 coreid) } } } else { - brcmf_err("Unsupported core selected %x\n", coreid); + brcmf_err(bus, "Unsupported core selected %x\n", coreid); } } @@ -848,9 +858,8 @@ static irqreturn_t brcmf_pcie_isr_thread(int irq, void *arg) static int brcmf_pcie_request_irq(struct brcmf_pciedev_info *devinfo) { - struct pci_dev *pdev; - - pdev = devinfo->pdev; + struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); brcmf_pcie_intr_disable(devinfo); @@ -861,7 +870,7 @@ static int brcmf_pcie_request_irq(struct brcmf_pciedev_info *devinfo) brcmf_pcie_isr_thread, IRQF_SHARED, "brcmf_pcie_intr", devinfo)) { pci_disable_msi(pdev); - brcmf_err("Failed to request IRQ %d\n", pdev->irq); + brcmf_err(bus, "Failed to request IRQ %d\n", pdev->irq); return -EIO; } devinfo->irq_allocated = true; @@ -871,15 +880,14 @@ static int brcmf_pcie_request_irq(struct brcmf_pciedev_info *devinfo) static void brcmf_pcie_release_irq(struct brcmf_pciedev_info *devinfo) { - struct pci_dev *pdev; + struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); u32 status; u32 count; if (!devinfo->irq_allocated) return; - pdev = devinfo->pdev; - brcmf_pcie_intr_disable(devinfo); free_irq(pdev->irq, devinfo); pci_disable_msi(pdev); @@ -891,7 +899,7 @@ static void brcmf_pcie_release_irq(struct brcmf_pciedev_info *devinfo) count++; } if (devinfo->i
[PATCH V3 1/2] brcmfmac: modify __brcmf_err() to take bus as a parameter
From: Rafał Miłecki So far __brcmf_err() was using pr_err() which didn't allow identifying device that was affected by an error. It's crucial for systems with more than 1 device supported by brcmfmac (a common case for home routers). This change allows passing struct brcmf_bus to the __brcmf_err(). That struct has been agreed to be the most common one. It allows accessing struct device easily & using dev_err() printing helper. Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel --- V2: Add missing #include V3: Add #include "bus.h" to fix CONFIG_BRCM_TRACING=y compilation error: error: dereferencing pointer to incomplete type 'struct brcmf_bus' --- .../net/wireless/broadcom/brcm80211/brcmfmac/common.c| 7 +-- drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 8 +--- .../wireless/broadcom/brcm80211/brcmfmac/tracepoint.c| 9 +++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c index 0ce1d8174e6d..c62009a06617 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c @@ -350,7 +350,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp) } #ifndef CONFIG_BRCM_TRACING -void __brcmf_err(const char *func, const char *fmt, ...) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...) { struct va_format vaf; va_list args; @@ -359,7 +359,10 @@ void __brcmf_err(const char *func, const char *fmt, ...) vaf.fmt = fmt; vaf.va = &args; - pr_err("%s: %pV", func, &vaf); + if (bus) + dev_err(bus->dev, "%s: %pV", func, &vaf); + else + pr_err("%s: %pV", func, &vaf); va_end(args); } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h index cfed0626bf5a..b499f90d94f6 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h @@ -45,8 +45,10 @@ #undef pr_fmt #define pr_fmt(fmt)KBUILD_MODNAME ": " fmt -__printf(2, 3) -void __brcmf_err(const char *func, const char *fmt, ...); +struct brcmf_bus; + +__printf(3, 4) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...); /* Macro for error messages. When debugging / tracing the driver all error * messages are important to us. */ @@ -55,7 +57,7 @@ void __brcmf_err(const char *func, const char *fmt, ...); if (IS_ENABLED(CONFIG_BRCMDBG) || \ IS_ENABLED(CONFIG_BRCM_TRACING) || \ net_ratelimit())\ - __brcmf_err(__func__, fmt, ##__VA_ARGS__); \ + __brcmf_err(NULL, __func__, fmt, ##__VA_ARGS__);\ } while (0) #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c index fe6755944b7b..a5c271bff446 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c @@ -14,14 +14,16 @@ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include #include /* bug in tracepoint.h, it should include this */ #ifndef __CHECKER__ #define CREATE_TRACE_POINTS +#include "bus.h" #include "tracepoint.h" #include "debug.h" -void __brcmf_err(const char *func, const char *fmt, ...) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...) { struct va_format vaf = { .fmt = fmt, @@ -30,7 +32,10 @@ void __brcmf_err(const char *func, const char *fmt, ...) va_start(args, fmt); vaf.va = &args; - pr_err("%s: %pV", func, &vaf); + if (bus) + dev_err(bus->dev, "%s: %pV", func, &vaf); + else + pr_err("%s: %pV", func, &vaf); trace_brcmf_err(func, &vaf); va_end(args); } -- 2.20.1
Re: [PATCH V2 1/2] brcmfmac: modify __brcmf_err() to take bus as a parameter
On 01.02.2019 13:14, Kalle Valo wrote: Rafał Miłecki wrote: From: Rafał Miłecki So far __brcmf_err() was using pr_err() which didn't allow identifying device that was affected by an error. It's crucial for systems with more than 1 device supported by brcmfmac (a common case for home routers). This change allows passing struct brcmf_bus to the __brcmf_err(). That struct has been agreed to be the most common one. It allows accessing struct device easily & using dev_err() printing helper. Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel Fails to build for me: drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c: In function 'brcmf_pcie_pm_enter_D3': drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c:1948:20: error: redeclaration of 'bus' with no linkage struct brcmf_bus *bus; ^~~ drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c:1946:20: note: previous definition of 'bus' was here struct brcmf_bus *bus = dev_get_drvdata(dev); ^~~ drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c: In function 'brcmf_pcie_pm_leave_D3': drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c:1978:20: error: redeclaration of 'bus' with no linkage struct brcmf_bus *bus; ^~~ drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c:1976:20: note: previous definition of 'bus' was here struct brcmf_bus *bus = dev_get_drvdata(dev); ^~~ make[6]: *** [drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.o] Error 1 make[6]: *** Waiting for unfinished jobs drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c: In function '__brcmf_err': drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c:35:15: error: dereferencing pointer to incomplete type 'struct brcmf_bus' dev_err(bus->dev, "%s: %pV", func, &vaf); ^~ I have no idea why my gcc didn't complain. Sorry. I'll send V3. $ mips-suse-linux-gcc -v Using built-in specs. COLLECT_GCC=mips-suse-linux-gcc COLLECT_LTO_WRAPPER=/usr/lib64/gcc/mips-suse-linux/8/lto-wrapper Target: mips-suse-linux Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++ --enable-checking=release --disable-werror --with-gxx-include-dir=/usr/include/c++/8 --enable-ssp --disable-libssp --disable-libvtv --disable-libmpx --disable-cet --disable-libcc1 --disable-plugin --with-bugurl=http://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --with-slibdir=/lib64 --with-system-zlib --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex --enable-gnu-indirect-function --program-suffix=-8 --program-prefix=mips-suse-linux- --target=mips-suse-linux --disable-nls --with-sysroot=/usr/mips-suse-linux --with-build-sysroot=/usr/mips-suse-linux --with-build-time-tools=/usr/mips-suse-linux/bin --build=x86_64-suse-linux --host=x86_64-suse-linux Thread model: posix gcc version 8.2.1 20181108 [gcc-8-branch revision 265914] (SUSE Linux)
Re: [PATCH] brcmfmac: support monitor frames with hardware/ucode header
On 2019-01-25 09:51, Arend Van Spriel wrote: On 1/22/2019 12:08 PM, Rafał Miłecki wrote: From: Rafał Miłecki The new FullMAC firmwares for 4366b1/4366c0 were supposed to provide monitor frames with radiotap header but it doesn't seem to be the case. I was not aware that this was supposed to. I did not build a radiotap variant to keep it feature-wise similar to 4366b0 firmware. Well, then apparently I got confused :( When you wrote: On Mon, 11 Jun 2018 at 12:48, Arend van Spriel wrote: 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 assumed you'll add "rtap" cap value (to the internal wl code repository?) because you mean to release a firmware using it. Maybe you just meant it to be for your customers compiling firmwares on their own? Testing the latest release resulted in discovering a new format being used. It seems (almost?) identical to the one known from ucode used in SoftMAC devices which is most likely the same codebase anyway. I am a bit confused. How many formats are there? It is either ucode format or radiotap, right? So far we got two formats: 1) 802.11 frames (with frame (sub)type & all addresses) 2) 802.11 frames with the radiotap header and now we also have: 3) 802.11 frames with the ucode header For more info please check my original post: Research + questions on brcmfmac and support for monitor mode https://www.spinics.net/lists/linux-wireless/msg173573.html While radiotap support was meant to be announced with the "rtap" fw capability string it seems no alternative was added for the hw/ucode format. It means each firmware requires a feature mapping quirk. I thought we only needed a quirk for the firmware that provide radiotap but do not announce it. For the others we can assume ucode format if monitor mode is supported. Probably missing something. 802.11 frames with ucode header is something entirely new, I didn't see any Asus/Linksys/Netgear/TP-LINK firmware using it. As the old firmwares were providing frames without any header (also making it impossible to e.g. read signal strength) we need this new flag to distinguish firmwares with ucode header from them.
Re: [PATCH wireless-drivers-next] bcma: get SoC device struct & copy its DMA params to the subdevices
On Mon, 21 Jan 2019 at 15:46, Christoph Hellwig wrote: > On Mon, Jan 21, 2019 at 11:11:21AM +0100, Rafał Miłecki wrote: > > From: Rafał Miłecki > > > > For bus devices to be fully usable it's required to set their DMA > > parameters. > > > > For years it has been missing and remained unnoticed because of > > mips_dma_alloc_coherent() silently handling the empty coherent_dma_mask. > > Kernel 4.19 came with a lot of DMA changes and caused a regression on > > the bcm47xx. Starting with the commit f8c55dc6e828 ("MIPS: use generic > > dma noncoherent ops for simple noncoherent platforms") DMA coherent > > allocations just fail. Example: > > [1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed > > [1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA > > [1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 > > [1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loaded > > > > This change fixes above regression in addition to the MIPS bcm47xx > > commit 321c46b91550 ("MIPS: BCM47XX: Setup struct device for the SoC"). > > > > It also fixes another *old* GPIO regression caused by a parent pointing > > to the NULL: > > [0.157054] missing gpiochip .dev parent pointer > > [0.157287] bcma: bus0: Error registering GPIO driver: -22 > > introduced by the commit 74f4e0cc6108 ("bcma: switch GPIO portions to > > use GPIOLIB_IRQCHIP"). > > > > Fixes: f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple > > noncoherent platforms") > > Fixes: 74f4e0cc6108 ("bcma: switch GPIO portions to use GPIOLIB_IRQCHIP") > > Cc: linux-m...@linux-mips.org > > Cc: Christoph Hellwig > > Cc: Linus Walleij > > Signed-off-by: Rafał Miłecki > > --- > > While this patch is a regression fix, it depends on a change present in > > the wireless-drivers-next.git: > > bcma: keep a direct pointer to the struct device > > > > That's why I suggest pushing it into the wireless-drivers-next.git and I > > can take care of picking it for the sta...@kernel.org later. > > > > Another option would be cherry-picking commit 5a1c18b761dd ("bcma: keep > > a direct pointer to the struct device") to the wireless-drivers.git but > > I don't think it's a common practice. > > --- > > drivers/bcma/host_soc.c | 2 ++ > > drivers/bcma/main.c | 6 +- > > include/linux/bcma/bcma_soc.h | 1 + > > 3 files changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/bcma/host_soc.c b/drivers/bcma/host_soc.c > > index c8073b509a2b..1fdfb704f22d 100644 > > --- a/drivers/bcma/host_soc.c > > +++ b/drivers/bcma/host_soc.c > > @@ -191,6 +191,8 @@ int __init bcma_host_soc_init(struct bcma_soc *soc) > > struct bcma_bus *bus = &soc->bus; > > int err; > > > > + bus->dev = soc->dev; > > + > > /* Scan bus and initialize it */ > > err = bcma_bus_early_register(bus); > > if (err) > > diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c > > index 6535614a7dc1..433ca5e2ed2c 100644 > > --- a/drivers/bcma/main.c > > +++ b/drivers/bcma/main.c > > @@ -236,12 +236,16 @@ EXPORT_SYMBOL(bcma_core_irq); > > > > void bcma_prepare_core(struct bcma_bus *bus, struct bcma_device *core) > > { > > + struct device *dev = &core->dev; > > + > > core->dev.release = bcma_release_core_dev; > > core->dev.bus = &bcma_bus_type; > > dev_set_name(&core->dev, "bcma%d:%d", bus->num, core->core_index); > > core->dev.parent = bus->dev; > > - if (bus->dev) > > + if (bus->dev) { > > bcma_of_fill_device(bus->dev, core); > > + dma_coerce_mask_and_coherent(dev, > > bus->dev->coherent_dma_mask); > > + } > > I don't think this does the right thing for bcma devices behind > PCI/PCIe, as those might need a IOMMU which relies on having a device > it has properly enumerated to be passed into the DMA API. In other > words: I think you need to change the layering so that the DMA API > is always called on the underlying PCI/PCIe/platform device, not of > the child bus. This is already implemented (we have dma_dev pointer), but it may be indeed more correct to apply this change for the SoC code only. I'll send V2. -- Rafał
[PATCH] brcmfmac: support monitor frames with hardware/ucode header
From: Rafał Miłecki The new FullMAC firmwares for 4366b1/4366c0 were supposed to provide monitor frames with radiotap header but it doesn't seem to be the case. Testing the latest release resulted in discovering a new format being used. It seems (almost?) identical to the one known from ucode used in SoftMAC devices which is most likely the same codebase anyway. While radiotap support was meant to be announced with the "rtap" fw capability string it seems no alternative was added for the hw/ucode format. It means each firmware requires a feature mapping quirk. As for now only an empty radiotap is being created. Adding support for extracting some info (band, channel, signal, etc.) is planned for the future. Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/core.c| 55 +++ .../broadcom/brcm80211/brcmfmac/feature.c | 4 ++ .../broadcom/brcm80211/brcmfmac/feature.h | 4 +- 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 860a4372cb56..e772c0845638 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -43,6 +43,36 @@ #define BRCMF_BSSIDX_INVALID -1 +#defineRXS_PBPRES BIT(2) + +#defineD11_PHY_HDR_LEN 6 + +struct d11rxhdr_le { + __le16 RxFrameSize; + u16 PAD; + __le16 PhyRxStatus_0; + __le16 PhyRxStatus_1; + __le16 PhyRxStatus_2; + __le16 PhyRxStatus_3; + __le16 PhyRxStatus_4; + __le16 PhyRxStatus_5; + __le16 RxStatus1; + __le16 RxStatus2; + __le16 RxTSFTime; + __le16 RxChan; + u8 unknown[12]; +} __packed; + +struct wlc_d11rxhdr { + struct d11rxhdr_le rxhdr; + __le32 tsf_l; + s8 rssi; + s8 rxpwr0; + s8 rxpwr1; + s8 do_rssi_ma; + s8 rxpwr[4]; +} __packed; + char *brcmf_ifname(struct brcmf_if *ifp) { if (!ifp) @@ -409,6 +439,31 @@ void brcmf_netif_mon_rx(struct brcmf_if *ifp, struct sk_buff *skb) { if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR_FMT_RADIOTAP)) { /* Do nothing */ + } else if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR_FMT_HW_RX_HDR)) { + struct wlc_d11rxhdr *wlc_rxhdr = (struct wlc_d11rxhdr *)skb->data; + struct ieee80211_radiotap_header *radiotap; + unsigned int offset; + u16 RxStatus1; + + RxStatus1 = le16_to_cpu(wlc_rxhdr->rxhdr.RxStatus1); + + offset = sizeof(struct wlc_d11rxhdr); + /* MAC inserts 2 pad bytes for a4 headers or QoS or A-MSDU +* subframes +*/ + if (RxStatus1 & RXS_PBPRES) + offset += 2; + offset += D11_PHY_HDR_LEN; + + skb_pull(skb, offset); + + /* TODO: use RX header to fill some radiotap data */ + radiotap = skb_push(skb, sizeof(*radiotap)); + memset(radiotap, 0, sizeof(*radiotap)); + radiotap->it_len = cpu_to_le16(sizeof(*radiotap)); + + /* TODO: 4 bytes with receive status? */ + skb->len -= 4; } else { struct ieee80211_radiotap_header *radiotap; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c index 4c5a3995dc35..b91b7ecbfedf 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c @@ -103,6 +103,10 @@ static const struct brcmf_feat_fwfeat brcmf_feat_fwfeat_map[] = { { "01-6cb8e269", BIT(BRCMF_FEAT_MONITOR) }, /* brcmfmac4366b-pcie.bin from linux-firmware.git commit 52442afee990 */ { "01-c47a91a4", BIT(BRCMF_FEAT_MONITOR) }, + /* brcmfmac4366b-pcie.bin from linux-firmware.git commit 211de1679a68 */ + { "01-801fb449", BIT(BRCMF_FEAT_MONITOR_FMT_HW_RX_HDR) }, + /* brcmfmac4366c-pcie.bin from linux-firmware.git commit 211de1679a68 */ + { "01-d2cbb8fd", BIT(BRCMF_FEAT_MONITOR_FMT_HW_RX_HDR) }, }; static void brcmf_feat_firmware_overrides(struct brcmf_pub *drv) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h index 0b4974df353a..5e88a7f16ad2 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h @@ -35,6 +35,7 @@ * FWSUP: Firmware supplicant. * MONITOR: firmware can pass monitor packets to host. * MONITOR_FMT_RADIOTAP: firmware provides monitor packets with radiotap header + * MONITOR_FMT_HW_RX_HDR
[PATCH wireless-drivers-next] bcma: get SoC device struct & copy its DMA params to the subdevices
From: Rafał Miłecki For bus devices to be fully usable it's required to set their DMA parameters. For years it has been missing and remained unnoticed because of mips_dma_alloc_coherent() silently handling the empty coherent_dma_mask. Kernel 4.19 came with a lot of DMA changes and caused a regression on the bcm47xx. Starting with the commit f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") DMA coherent allocations just fail. Example: [1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed [1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA [1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 [1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loaded This change fixes above regression in addition to the MIPS bcm47xx commit 321c46b91550 ("MIPS: BCM47XX: Setup struct device for the SoC"). It also fixes another *old* GPIO regression caused by a parent pointing to the NULL: [0.157054] missing gpiochip .dev parent pointer [0.157287] bcma: bus0: Error registering GPIO driver: -22 introduced by the commit 74f4e0cc6108 ("bcma: switch GPIO portions to use GPIOLIB_IRQCHIP"). Fixes: f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") Fixes: 74f4e0cc6108 ("bcma: switch GPIO portions to use GPIOLIB_IRQCHIP") Cc: linux-m...@linux-mips.org Cc: Christoph Hellwig Cc: Linus Walleij Signed-off-by: Rafał Miłecki --- While this patch is a regression fix, it depends on a change present in the wireless-drivers-next.git: bcma: keep a direct pointer to the struct device That's why I suggest pushing it into the wireless-drivers-next.git and I can take care of picking it for the sta...@kernel.org later. Another option would be cherry-picking commit 5a1c18b761dd ("bcma: keep a direct pointer to the struct device") to the wireless-drivers.git but I don't think it's a common practice. --- drivers/bcma/host_soc.c | 2 ++ drivers/bcma/main.c | 6 +- include/linux/bcma/bcma_soc.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/bcma/host_soc.c b/drivers/bcma/host_soc.c index c8073b509a2b..1fdfb704f22d 100644 --- a/drivers/bcma/host_soc.c +++ b/drivers/bcma/host_soc.c @@ -191,6 +191,8 @@ int __init bcma_host_soc_init(struct bcma_soc *soc) struct bcma_bus *bus = &soc->bus; int err; + bus->dev = soc->dev; + /* Scan bus and initialize it */ err = bcma_bus_early_register(bus); if (err) diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c index 6535614a7dc1..433ca5e2ed2c 100644 --- a/drivers/bcma/main.c +++ b/drivers/bcma/main.c @@ -236,12 +236,16 @@ EXPORT_SYMBOL(bcma_core_irq); void bcma_prepare_core(struct bcma_bus *bus, struct bcma_device *core) { + struct device *dev = &core->dev; + core->dev.release = bcma_release_core_dev; core->dev.bus = &bcma_bus_type; dev_set_name(&core->dev, "bcma%d:%d", bus->num, core->core_index); core->dev.parent = bus->dev; - if (bus->dev) + if (bus->dev) { bcma_of_fill_device(bus->dev, core); + dma_coerce_mask_and_coherent(dev, bus->dev->coherent_dma_mask); + } switch (bus->hosttype) { case BCMA_HOSTTYPE_PCI: diff --git a/include/linux/bcma/bcma_soc.h b/include/linux/bcma/bcma_soc.h index 7cca5f859a90..f3c43519baa7 100644 --- a/include/linux/bcma/bcma_soc.h +++ b/include/linux/bcma/bcma_soc.h @@ -6,6 +6,7 @@ struct bcma_soc { struct bcma_bus bus; + struct device *dev; }; int __init bcma_host_soc_register(struct bcma_soc *soc); -- 2.20.1
[PATCH] brcmfmac: add bphy_err() and use it in the cfg80211.c
From: Rafał Miłecki This new macro uses wiphy_err() which: 1) Should be the best choice with wiphy already created 2) Uses dev_err() which allows identifying error-affected device Signed-off-by: Rafał Miłecki --- This is based on Arend's feedback from [PATCH 2/2] brcmfmac: pass bus to the __brcmf_err() in cfg80211.c who wrote: > I was thinking about using wiphy_err() for this file. I'm not sure how much do you guys like the bphy_err() name. I was considering brcmf_phy_err() but that seemed too long. Kalle: this applies cleanly on top of the [PATCH V2 2/2] brcmfmac: pass bus to the __brcmf_err() in pcie.c otherwise debug.h change will require a trivial rebase. --- .../broadcom/brcm80211/brcmfmac/cfg80211.c| 497 ++ .../broadcom/brcm80211/brcmfmac/debug.h | 9 + 2 files changed, 282 insertions(+), 224 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 35301237d435..b5e291ed9496 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -457,6 +457,7 @@ static void convert_key_from_CPU(struct brcmf_wsec_key *key, static int send_key_to_dongle(struct brcmf_if *ifp, struct brcmf_wsec_key *key) { + struct wiphy *wiphy = ifp->drvr->wiphy; int err; struct brcmf_wsec_key_le key_le; @@ -468,7 +469,7 @@ send_key_to_dongle(struct brcmf_if *ifp, struct brcmf_wsec_key *key) sizeof(key_le)); if (err) - brcmf_err("wsec_key error (%d)\n", err); + bphy_err(wiphy, "wsec_key error (%d)\n", err); return err; } @@ -508,6 +509,7 @@ static int brcmf_get_first_free_bsscfgidx(struct brcmf_pub *drvr) static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp) { + struct wiphy *wiphy = ifp->drvr->wiphy; struct brcmf_mbss_ssid_le mbss_ssid_le; int bsscfgidx; int err; @@ -524,7 +526,7 @@ static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp) err = brcmf_fil_bsscfg_data_set(ifp, "bsscfg:ssid", &mbss_ssid_le, sizeof(mbss_ssid_le)); if (err < 0) - brcmf_err("setting ssid failed %d\n", err); + bphy_err(wiphy, "setting ssid failed %d\n", err); return err; } @@ -567,7 +569,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, BRCMF_VIF_EVENT_TIMEOUT); brcmf_cfg80211_arm_vif_event(cfg, NULL); if (!err) { - brcmf_err("timeout occurred\n"); + bphy_err(wiphy, "timeout occurred\n"); err = -EIO; goto fail; } @@ -575,7 +577,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, /* interface created in firmware */ ifp = vif->ifp; if (!ifp) { - brcmf_err("no if pointer provided\n"); + bphy_err(wiphy, "no if pointer provided\n"); err = -ENOENT; goto fail; } @@ -583,7 +585,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, strncpy(ifp->ndev->name, name, sizeof(ifp->ndev->name) - 1); err = brcmf_net_attach(ifp, true); if (err) { - brcmf_err("Registering netdevice failed\n"); + bphy_err(wiphy, "Registering netdevice failed\n"); free_netdev(ifp->ndev); goto fail; } @@ -620,7 +622,7 @@ static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy, brcmf_dbg(TRACE, "enter: %s type %d\n", name, type); err = brcmf_vif_add_validate(wiphy_to_cfg(wiphy), type); if (err) { - brcmf_err("iface validation failed: err=%d\n", err); + bphy_err(wiphy, "iface validation failed: err=%d\n", err); return ERR_PTR(err); } switch (type) { @@ -645,8 +647,8 @@ static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy, } if (IS_ERR(wdev)) - brcmf_err("add iface %s type %d failed: err=%d\n", - name, type, (int)PTR_ERR(wdev)); + bphy_err(wiphy, "add iface %s type %d failed: err=%d\n", name, +type, (int)PTR_ERR(wdev)); else brcmf_cfg80211_update_proto_addr_mode(wdev); @@ -661,12 +663,13 @@ static void brcmf_scan_config_mpc(struct brcmf_if *ifp, int mpc) void brcmf_set_mpc(struct brcmf_if *ifp, int mpc) { + struct wiphy *wiphy = ifp->drvr
[PATCH V2 1/2] brcmfmac: modify __brcmf_err() to take bus as a parameter
From: Rafał Miłecki So far __brcmf_err() was using pr_err() which didn't allow identifying device that was affected by an error. It's crucial for systems with more than 1 device supported by brcmfmac (a common case for home routers). This change allows passing struct brcmf_bus to the __brcmf_err(). That struct has been agreed to be the most common one. It allows accessing struct device easily & using dev_err() printing helper. Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel --- V2: Add missing #include --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 7 +-- drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 8 +--- .../net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c | 8 ++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c index 0ce1d8174e6d..c62009a06617 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c @@ -350,7 +350,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp) } #ifndef CONFIG_BRCM_TRACING -void __brcmf_err(const char *func, const char *fmt, ...) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...) { struct va_format vaf; va_list args; @@ -359,7 +359,10 @@ void __brcmf_err(const char *func, const char *fmt, ...) vaf.fmt = fmt; vaf.va = &args; - pr_err("%s: %pV", func, &vaf); + if (bus) + dev_err(bus->dev, "%s: %pV", func, &vaf); + else + pr_err("%s: %pV", func, &vaf); va_end(args); } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h index cfed0626bf5a..b499f90d94f6 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h @@ -45,8 +45,10 @@ #undef pr_fmt #define pr_fmt(fmt)KBUILD_MODNAME ": " fmt -__printf(2, 3) -void __brcmf_err(const char *func, const char *fmt, ...); +struct brcmf_bus; + +__printf(3, 4) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...); /* Macro for error messages. When debugging / tracing the driver all error * messages are important to us. */ @@ -55,7 +57,7 @@ void __brcmf_err(const char *func, const char *fmt, ...); if (IS_ENABLED(CONFIG_BRCMDBG) || \ IS_ENABLED(CONFIG_BRCM_TRACING) || \ net_ratelimit())\ - __brcmf_err(__func__, fmt, ##__VA_ARGS__); \ + __brcmf_err(NULL, __func__, fmt, ##__VA_ARGS__);\ } while (0) #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c index fe6755944b7b..9c440a5e1c5f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c @@ -14,6 +14,7 @@ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#include #include /* bug in tracepoint.h, it should include this */ #ifndef __CHECKER__ @@ -21,7 +22,7 @@ #include "tracepoint.h" #include "debug.h" -void __brcmf_err(const char *func, const char *fmt, ...) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...) { struct va_format vaf = { .fmt = fmt, @@ -30,7 +31,10 @@ void __brcmf_err(const char *func, const char *fmt, ...) va_start(args, fmt); vaf.va = &args; - pr_err("%s: %pV", func, &vaf); + if (bus) + dev_err(bus->dev, "%s: %pV", func, &vaf); + else + pr_err("%s: %pV", func, &vaf); trace_brcmf_err(func, &vaf); va_end(args); } -- 2.20.1
[PATCH V2 2/2] brcmfmac: pass bus to the __brcmf_err() in pcie.c
From: Rafał Miłecki This enables dev_err() usage (instead of pr_err()) in the __brcmf_err(). It makes error messages more meaningful and is important for debugging errors/bugs on systems with multiple brcmfmac supported devices. All bus files should follow & get updated similarly (soon). Signed-off-by: Rafał Miłecki --- V2: Modify pcie.c & not cfg80211.c. The later should use wiphy_err() --- .../broadcom/brcm80211/brcmfmac/debug.h | 2 + .../broadcom/brcm80211/brcmfmac/pcie.c| 61 --- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h index b499f90d94f6..c1f260718c8e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h @@ -52,6 +52,7 @@ void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...); /* Macro for error messages. When debugging / tracing the driver all error * messages are important to us. */ +#ifndef brcmf_err #define brcmf_err(fmt, ...)\ do {\ if (IS_ENABLED(CONFIG_BRCMDBG) || \ @@ -59,6 +60,7 @@ void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...); net_ratelimit())\ __brcmf_err(NULL, __func__, fmt, ##__VA_ARGS__);\ } while (0) +#endif #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index 16d7dda965d8..f24731cdd6f5 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -30,6 +30,15 @@ #include #include +/* Custom brcmf_err() that takes bus arg and passes it further */ +#define brcmf_err(bus, fmt, ...) \ + do {\ + if (IS_ENABLED(CONFIG_BRCMDBG) || \ + IS_ENABLED(CONFIG_BRCM_TRACING) || \ + net_ratelimit())\ + __brcmf_err(bus, __func__, fmt, ##__VA_ARGS__); \ + } while (0) + #include "debug.h" #include "bus.h" #include "commonring.h" @@ -531,6 +540,7 @@ static void brcmf_pcie_select_core(struct brcmf_pciedev_info *devinfo, u16 coreid) { const struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); struct brcmf_core *core; u32 bar0_win; @@ -548,7 +558,7 @@ brcmf_pcie_select_core(struct brcmf_pciedev_info *devinfo, u16 coreid) } } } else { - brcmf_err("Unsupported core selected %x\n", coreid); + brcmf_err(bus, "Unsupported core selected %x\n", coreid); } } @@ -848,9 +858,8 @@ static irqreturn_t brcmf_pcie_isr_thread(int irq, void *arg) static int brcmf_pcie_request_irq(struct brcmf_pciedev_info *devinfo) { - struct pci_dev *pdev; - - pdev = devinfo->pdev; + struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); brcmf_pcie_intr_disable(devinfo); @@ -861,7 +870,7 @@ static int brcmf_pcie_request_irq(struct brcmf_pciedev_info *devinfo) brcmf_pcie_isr_thread, IRQF_SHARED, "brcmf_pcie_intr", devinfo)) { pci_disable_msi(pdev); - brcmf_err("Failed to request IRQ %d\n", pdev->irq); + brcmf_err(bus, "Failed to request IRQ %d\n", pdev->irq); return -EIO; } devinfo->irq_allocated = true; @@ -871,15 +880,14 @@ static int brcmf_pcie_request_irq(struct brcmf_pciedev_info *devinfo) static void brcmf_pcie_release_irq(struct brcmf_pciedev_info *devinfo) { - struct pci_dev *pdev; + struct pci_dev *pdev = devinfo->pdev; + struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev); u32 status; u32 count; if (!devinfo->irq_allocated) return; - pdev = devinfo->pdev; - brcmf_pcie_intr_disable(devinfo); free_irq(pdev->irq, devinfo); pci_disable_msi(pdev); @@ -891,7 +899,7 @@ static void brcmf_pcie_release_irq(struct brcmf_pciedev_info *devinfo) count++; } if (devinfo->in_irq) - brcmf_err("Still in IRQ (processing) !!!\n"); + brcmf_err(bus, "
Re: [PATCH 1/2] brcmfmac: modify __brcmf_err() to take bus as a parameter
On Tue, 15 Jan 2019 at 09:48, Arend Van Spriel wrote: > On 1/15/2019 7:19 AM, Rafał Miłecki wrote: > > From: Rafał Miłecki > > > > So far __brcmf_err() was using pr_err() which didn't allow identifying > > device that was affected by an error. It's crucial for systems with more > > than 1 device supported by brcmfmac (a common case for home routers). > > > > This change allows passing struct brcmf_bus to the __brcmf_err(). That > > struct has been agreed to be the most common one. It allows accessing > > struct device easily & using dev_err() printing helper. > > Acked-by: Arend van Spriel > > Signed-off-by: Rafał Miłecki > > --- > > This is my another try on improving brcmf_err after the failure from 2 > > years ago: > > [PATCH V3 4/9] brcmfmac: add struct brcmf_pub parameter to the __brcmf_err > > https://patchwork.kernel.org/patch/9553255/ > > > > Back then my change has been rejected due to miscommunication and late > > realisation that struct brcmf_pub (a previous choice instead of struct > > brcmf_bus) was a bad idea. Back then Arend wrote: > >> So I would think using struct brcmf_bus in brcmf_err() would be best > >> fit. > > > > So this patch follows that suggestion & updates __brcmf_err() > > accordingly. > > Thanks, Rafał > > Little less than two years ago I played with your idea and using GCC > builtin __builtin_types_compatible_p(t1,t2). Anyway, it looks good. So > you want to limit it to brcmf_err() or brcmf_dbg() as well? I believe all messages printed by brcmfmac should specify a device. brcmf_err, brcmf_info & brcmf_dbg. I can work on brcmf_info & brcmf_dbg once I get done with brcmf_err :) -- Rafał
[PATCH 2/2] brcmfmac: pass bus to the __brcmf_err() in cfg80211.c
From: Rafał Miłecki This enables dev_err() usage (instead of pr_err()) in the __brcmf_err(). It makes error messages more meaningful and is important for debugging errors/bugs on systems with multiple brcmfmac supported devices. All other files should follow & get updated similarly (soon). Signed-off-by: Rafał Miłecki --- Ideally all files should get updated at the same time but I'm reluctant to work on that until I see my changes accepted. There are over 500 occurrences of brcmf_err() and I'll loose hours again if someone nacks this. Thus the safe way - updating a single file only as for now. I promise to work on the rest of the code as soon as it hits the Kalle's tree. Sounds acceptable? --- .../broadcom/brcm80211/brcmfmac/cfg80211.c| 502 +++--- .../broadcom/brcm80211/brcmfmac/debug.h | 2 + 2 files changed, 305 insertions(+), 199 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 35301237d435..da04c2a2cc59 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -23,6 +23,15 @@ #include #include +/* Custom brcmf_err() that takes bus arg and passes it further */ +#define brcmf_err(bus, fmt, ...) \ + do {\ + if (IS_ENABLED(CONFIG_BRCMDBG) || \ + IS_ENABLED(CONFIG_BRCM_TRACING) || \ + net_ratelimit())\ + __brcmf_err(bus, __func__, fmt, ##__VA_ARGS__); \ + } while (0) + #include #include #include @@ -457,6 +466,7 @@ static void convert_key_from_CPU(struct brcmf_wsec_key *key, static int send_key_to_dongle(struct brcmf_if *ifp, struct brcmf_wsec_key *key) { + struct brcmf_bus *bus = ifp->drvr->bus_if; int err; struct brcmf_wsec_key_le key_le; @@ -468,7 +478,7 @@ send_key_to_dongle(struct brcmf_if *ifp, struct brcmf_wsec_key *key) sizeof(key_le)); if (err) - brcmf_err("wsec_key error (%d)\n", err); + brcmf_err(bus, "wsec_key error (%d)\n", err); return err; } @@ -508,6 +518,7 @@ static int brcmf_get_first_free_bsscfgidx(struct brcmf_pub *drvr) static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp) { + struct brcmf_bus *bus = ifp->drvr->bus_if; struct brcmf_mbss_ssid_le mbss_ssid_le; int bsscfgidx; int err; @@ -524,7 +535,7 @@ static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp) err = brcmf_fil_bsscfg_data_set(ifp, "bsscfg:ssid", &mbss_ssid_le, sizeof(mbss_ssid_le)); if (err < 0) - brcmf_err("setting ssid failed %d\n", err); + brcmf_err(bus, "setting ssid failed %d\n", err); return err; } @@ -541,6 +552,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, struct vif_params *params) { struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy); + struct brcmf_bus *bus = cfg->pub->bus_if; struct brcmf_if *ifp = netdev_priv(cfg_to_ndev(cfg)); struct brcmf_cfg80211_vif *vif; int err; @@ -567,7 +579,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, BRCMF_VIF_EVENT_TIMEOUT); brcmf_cfg80211_arm_vif_event(cfg, NULL); if (!err) { - brcmf_err("timeout occurred\n"); + brcmf_err(bus, "timeout occurred\n"); err = -EIO; goto fail; } @@ -575,7 +587,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, /* interface created in firmware */ ifp = vif->ifp; if (!ifp) { - brcmf_err("no if pointer provided\n"); + brcmf_err(bus, "no if pointer provided\n"); err = -ENOENT; goto fail; } @@ -583,7 +595,7 @@ struct wireless_dev *brcmf_ap_add_vif(struct wiphy *wiphy, const char *name, strncpy(ifp->ndev->name, name, sizeof(ifp->ndev->name) - 1); err = brcmf_net_attach(ifp, true); if (err) { - brcmf_err("Registering netdevice failed\n"); + brcmf_err(bus, "Registering netdevice failed\n"); free_netdev(ifp->ndev); goto fail; } @@ -614,13 +626,15 @@ static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy,
[PATCH 1/2] brcmfmac: modify __brcmf_err() to take bus as a parameter
From: Rafał Miłecki So far __brcmf_err() was using pr_err() which didn't allow identifying device that was affected by an error. It's crucial for systems with more than 1 device supported by brcmfmac (a common case for home routers). This change allows passing struct brcmf_bus to the __brcmf_err(). That struct has been agreed to be the most common one. It allows accessing struct device easily & using dev_err() printing helper. Signed-off-by: Rafał Miłecki --- This is my another try on improving brcmf_err after the failure from 2 years ago: [PATCH V3 4/9] brcmfmac: add struct brcmf_pub parameter to the __brcmf_err https://patchwork.kernel.org/patch/9553255/ Back then my change has been rejected due to miscommunication and late realisation that struct brcmf_pub (a previous choice instead of struct brcmf_bus) was a bad idea. Back then Arend wrote: > So I would think using struct brcmf_bus in brcmf_err() would be best > fit. So this patch follows that suggestion & updates __brcmf_err() accordingly. --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 7 +-- drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 8 +--- .../net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c | 7 +-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c index 0ce1d8174e6d..c62009a06617 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c @@ -350,7 +350,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp) } #ifndef CONFIG_BRCM_TRACING -void __brcmf_err(const char *func, const char *fmt, ...) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...) { struct va_format vaf; va_list args; @@ -359,7 +359,10 @@ void __brcmf_err(const char *func, const char *fmt, ...) vaf.fmt = fmt; vaf.va = &args; - pr_err("%s: %pV", func, &vaf); + if (bus) + dev_err(bus->dev, "%s: %pV", func, &vaf); + else + pr_err("%s: %pV", func, &vaf); va_end(args); } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h index cfed0626bf5a..b499f90d94f6 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h @@ -45,8 +45,10 @@ #undef pr_fmt #define pr_fmt(fmt)KBUILD_MODNAME ": " fmt -__printf(2, 3) -void __brcmf_err(const char *func, const char *fmt, ...); +struct brcmf_bus; + +__printf(3, 4) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...); /* Macro for error messages. When debugging / tracing the driver all error * messages are important to us. */ @@ -55,7 +57,7 @@ void __brcmf_err(const char *func, const char *fmt, ...); if (IS_ENABLED(CONFIG_BRCMDBG) || \ IS_ENABLED(CONFIG_BRCM_TRACING) || \ net_ratelimit())\ - __brcmf_err(__func__, fmt, ##__VA_ARGS__); \ + __brcmf_err(NULL, __func__, fmt, ##__VA_ARGS__);\ } while (0) #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c index fe6755944b7b..f9359ea9cb13 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c @@ -21,7 +21,7 @@ #include "tracepoint.h" #include "debug.h" -void __brcmf_err(const char *func, const char *fmt, ...) +void __brcmf_err(struct brcmf_bus *bus, const char *func, const char *fmt, ...) { struct va_format vaf = { .fmt = fmt, @@ -30,7 +30,10 @@ void __brcmf_err(const char *func, const char *fmt, ...) va_start(args, fmt); vaf.va = &args; - pr_err("%s: %pV", func, &vaf); + if (bus) + dev_err(bus->dev, "%s: %pV", func, &vaf); + else + pr_err("%s: %pV", func, &vaf); trace_brcmf_err(func, &vaf); va_end(args); } -- 2.20.1
Re: [PATCH mips-fixes] MIPS: BCM47XX: Setup struct device for the SoC
On 2019-01-02 16:50, Hauke Mehrtens wrote: On 1/2/19 1:51 PM, Rafał Miłecki wrote: From: Rafał Miłecki So far we never had any device registered for the SoC. This resulted in some small issues that we kept ignoring like: 1) Not working GPIOLIB_IRQCHIP (gpiochip_irqchip_add_key() failing) 2) Lack of proper tree in the /sys/devices/ 3) mips_dma_alloc_coherent() silently handling empty coherent_dma_mask Kernel 4.19 came with a lot of DMA changes and caused a regression on bcm47xx. Starting with the commit f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") DMA coherent allocations just fail. Example: [1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed [1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA [1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 [1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loaded The bgmac driver also triggers a WARNING: [0.959486] [ cut here ] [0.964387] WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516 bgmac_enet_probe+0x1b4/0x5c4 [0.973751] Modules linked in: [0.976913] CPU: 0 PID: 1 Comm: swapper Not tainted 4.19.9 #0 [0.982750] Stack : 804a 804597c4 80458fd8 8381bc2c 838282d4 80481a47 [0.991367] 8042e3ec 0001 804d38f0 0204 8398 0065 8381bbe0 6f55b24f [0.75] 8052 2018 0075 0007 [1.008583] 8048 000ee811 80432c00 80248db8 [1.017196] 0009 0204 8398 803ad7b0 801feeec 804d [1.025804] ... [1.028325] Call Trace: [1.030875] [<8000aef8>] show_stack+0x58/0x100 [1.035513] [<8001f8b4>] __warn+0xe4/0x118 [1.039708] [<8001f9a4>] warn_slowpath_null+0x48/0x64 [1.044935] [<80248db8>] bgmac_enet_probe+0x1b4/0x5c4 [1.050101] [<802498e0>] bgmac_probe+0x558/0x590 [1.054906] [<80252fd0>] bcma_device_probe+0x38/0x70 [1.060017] [<8020e1e8>] really_probe+0x170/0x2e8 [1.064891] [<8020e714>] __driver_attach+0xa4/0xec [1.069784] [<8020c1e0>] bus_for_each_dev+0x58/0xb0 [1.074833] [<8020d590>] bus_add_driver+0xf8/0x218 [1.079731] [<8020ef24>] driver_register+0xcc/0x11c [1.084804] [<804b54cc>] bgmac_init+0x1c/0x44 [1.089258] [<8000121c>] do_one_initcall+0x7c/0x1a0 [1.094343] [<804a1d34>] kernel_init_freeable+0x150/0x218 [1.099886] [<803a082c>] kernel_init+0x10/0x104 [1.104583] [<80005878>] ret_from_kernel_thread+0x14/0x1c [1.110107] ---[ end trace f441c0d873d1fb5b ]--- This patch setups a "struct device" (and passes it to the bcma) which allows fixing all the mentioned problems. It'll also require a tiny bcma patch which will follow through the wireless tree & its maintainer. Fixes: f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") Cc: Christoph Hellwig Cc: sta...@vger.kernel.org # v4.19+ Signed-off-by: Rafał Miłecki Acked-by: Hauke Mehrtens I assume that the old ssb based devices also have such problems did you had a look into those? I've ordered some device with ssb based SoC & I'm planning to test it next week hopefully.
[PATCH V2 mips-fixes] MIPS: BCM47XX: Setup struct device for the SoC
From: Rafał Miłecki So far we never had any device registered for the SoC. This resulted in some small issues that we kept ignoring like: 1) Not working GPIOLIB_IRQCHIP (gpiochip_irqchip_add_key() failing) 2) Lack of proper tree in the /sys/devices/ 3) mips_dma_alloc_coherent() silently handling empty coherent_dma_mask Kernel 4.19 came with a lot of DMA changes and caused a regression on bcm47xx. Starting with the commit f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") DMA coherent allocations just fail. Example: [1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed [1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA [1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 [1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loaded The bgmac driver also triggers a WARNING: [0.959486] [ cut here ] [0.964387] WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516 bgmac_enet_probe+0x1b4/0x5c4 [0.973751] Modules linked in: [0.976913] CPU: 0 PID: 1 Comm: swapper Not tainted 4.19.9 #0 [0.982750] Stack : 804a 804597c4 80458fd8 8381bc2c 838282d4 80481a47 [0.991367] 8042e3ec 0001 804d38f0 0204 8398 0065 8381bbe0 6f55b24f [0.75] 8052 2018 0075 0007 [1.008583] 8048 000ee811 80432c00 80248db8 [1.017196] 0009 0204 8398 803ad7b0 801feeec 804d [1.025804] ... [1.028325] Call Trace: [1.030875] [<8000aef8>] show_stack+0x58/0x100 [1.035513] [<8001f8b4>] __warn+0xe4/0x118 [1.039708] [<8001f9a4>] warn_slowpath_null+0x48/0x64 [1.044935] [<80248db8>] bgmac_enet_probe+0x1b4/0x5c4 [1.050101] [<802498e0>] bgmac_probe+0x558/0x590 [1.054906] [<80252fd0>] bcma_device_probe+0x38/0x70 [1.060017] [<8020e1e8>] really_probe+0x170/0x2e8 [1.064891] [<8020e714>] __driver_attach+0xa4/0xec [1.069784] [<8020c1e0>] bus_for_each_dev+0x58/0xb0 [1.074833] [<8020d590>] bus_add_driver+0xf8/0x218 [1.079731] [<8020ef24>] driver_register+0xcc/0x11c [1.084804] [<804b54cc>] bgmac_init+0x1c/0x44 [1.089258] [<8000121c>] do_one_initcall+0x7c/0x1a0 [1.094343] [<804a1d34>] kernel_init_freeable+0x150/0x218 [1.099886] [<803a082c>] kernel_init+0x10/0x104 [1.104583] [<80005878>] ret_from_kernel_thread+0x14/0x1c [1.110107] ---[ end trace f441c0d873d1fb5b ]--- This patch setups a "struct device" (and passes it to the bcma) which allows fixing all the mentioned problems. It'll also require a tiny bcma patch which will follow through the wireless tree & its maintainer. Fixes: f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") Cc: Christoph Hellwig Cc: Linus Walleij Cc: linux-wireless@vger.kernel.org Cc: sta...@vger.kernel.org # v4.19+ Signed-off-by: Rafał Miłecki Acked-by: Hauke Mehrtens --- V2: Reorder struct bcma_soc fields (trivial optimization - thanks Hauke) Add pr_err on error --- arch/mips/bcm47xx/setup.c | 31 +++ include/linux/bcma/bcma_soc.h | 1 + 2 files changed, 32 insertions(+) diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c index 6054d49e608e..fe3773539eff 100644 --- a/arch/mips/bcm47xx/setup.c +++ b/arch/mips/bcm47xx/setup.c @@ -173,6 +173,31 @@ void __init plat_mem_setup(void) pm_power_off = bcm47xx_machine_halt; } +#ifdef CONFIG_BCM47XX_BCMA +static struct device * __init bcm47xx_setup_device(void) +{ + struct device *dev; + int err; + + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) + return NULL; + + err = dev_set_name(dev, "bcm47xx_soc"); + if (err) { + pr_err("Failed to set SoC device name: %d\n", err); + kfree(dev); + return NULL; + } + + err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); + if (err) + pr_err("Failed to set SoC DMA mask: %d\n", err); + + return dev; +} +#endif + /* * This finishes bus initialization doing things that were not possible without * kmalloc. Make sure to call it late enough (after mm_init). @@ -183,6 +208,10 @@ void __init bcm47xx_bus_setup(void) if (bcm47xx_bus_type == BCM47XX_BUS_TYPE_BCMA) { int err; + bcm47xx_bus.bcma.dev = bcm47xx_setup_device(); + if (!bcm47xx_bus.bcma.dev) + panic("Failed to setup SoC device\n"); + err = bcma_host_soc_init(&bcm47xx_bus.bcma); if (err) panic("Failed to initialize BCMA b
[PATCH mips-fixes] MIPS: BCM47XX: Setup struct device for the SoC
From: Rafał Miłecki So far we never had any device registered for the SoC. This resulted in some small issues that we kept ignoring like: 1) Not working GPIOLIB_IRQCHIP (gpiochip_irqchip_add_key() failing) 2) Lack of proper tree in the /sys/devices/ 3) mips_dma_alloc_coherent() silently handling empty coherent_dma_mask Kernel 4.19 came with a lot of DMA changes and caused a regression on bcm47xx. Starting with the commit f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") DMA coherent allocations just fail. Example: [1.114914] bgmac_bcma bcma0:2: Allocation of TX ring 0x200 failed [1.121215] bgmac_bcma bcma0:2: Unable to alloc memory for DMA [1.127626] bgmac_bcma: probe of bcma0:2 failed with error -12 [1.133838] bgmac_bcma: Broadcom 47xx GBit MAC driver loaded The bgmac driver also triggers a WARNING: [0.959486] [ cut here ] [0.964387] WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516 bgmac_enet_probe+0x1b4/0x5c4 [0.973751] Modules linked in: [0.976913] CPU: 0 PID: 1 Comm: swapper Not tainted 4.19.9 #0 [0.982750] Stack : 804a 804597c4 80458fd8 8381bc2c 838282d4 80481a47 [0.991367] 8042e3ec 0001 804d38f0 0204 8398 0065 8381bbe0 6f55b24f [0.75] 8052 2018 0075 0007 [1.008583] 8048 000ee811 80432c00 80248db8 [1.017196] 0009 0204 8398 803ad7b0 801feeec 804d [1.025804] ... [1.028325] Call Trace: [1.030875] [<8000aef8>] show_stack+0x58/0x100 [1.035513] [<8001f8b4>] __warn+0xe4/0x118 [1.039708] [<8001f9a4>] warn_slowpath_null+0x48/0x64 [1.044935] [<80248db8>] bgmac_enet_probe+0x1b4/0x5c4 [1.050101] [<802498e0>] bgmac_probe+0x558/0x590 [1.054906] [<80252fd0>] bcma_device_probe+0x38/0x70 [1.060017] [<8020e1e8>] really_probe+0x170/0x2e8 [1.064891] [<8020e714>] __driver_attach+0xa4/0xec [1.069784] [<8020c1e0>] bus_for_each_dev+0x58/0xb0 [1.074833] [<8020d590>] bus_add_driver+0xf8/0x218 [1.079731] [<8020ef24>] driver_register+0xcc/0x11c [1.084804] [<804b54cc>] bgmac_init+0x1c/0x44 [1.089258] [<8000121c>] do_one_initcall+0x7c/0x1a0 [1.094343] [<804a1d34>] kernel_init_freeable+0x150/0x218 [1.099886] [<803a082c>] kernel_init+0x10/0x104 [1.104583] [<80005878>] ret_from_kernel_thread+0x14/0x1c [1.110107] ---[ end trace f441c0d873d1fb5b ]--- This patch setups a "struct device" (and passes it to the bcma) which allows fixing all the mentioned problems. It'll also require a tiny bcma patch which will follow through the wireless tree & its maintainer. Fixes: f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for simple noncoherent platforms") Cc: Christoph Hellwig Cc: sta...@vger.kernel.org # v4.19+ Signed-off-by: Rafał Miłecki --- arch/mips/bcm47xx/setup.c | 30 ++ include/linux/bcma/bcma_soc.h | 1 + 2 files changed, 31 insertions(+) diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c index 6054d49e608e..9339a31a0a87 100644 --- a/arch/mips/bcm47xx/setup.c +++ b/arch/mips/bcm47xx/setup.c @@ -173,6 +173,31 @@ void __init plat_mem_setup(void) pm_power_off = bcm47xx_machine_halt; } +#ifdef CONFIG_BCM47XX_BCMA +static struct device * __init bcm47xx_setup_device(void) +{ + struct device *dev; + int err; + + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) + return NULL; + + err = dev_set_name(dev, "bcm47xx_soc"); + if (err) { + pr_err("Failed to set SoC device name: %d\n", err); + kfree(dev); + return NULL; + } + + err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); + if (err) + pr_err("Failed to set SoC DMA mask: %d\n", err); + + return dev; +} +#endif + /* * This finishes bus initialization doing things that were not possible without * kmalloc. Make sure to call it late enough (after mm_init). @@ -183,6 +208,10 @@ void __init bcm47xx_bus_setup(void) if (bcm47xx_bus_type == BCM47XX_BUS_TYPE_BCMA) { int err; + bcm47xx_bus.bcma.dev = bcm47xx_setup_device(); + if (!bcm47xx_bus.bcma.dev) + panic("Failed to setup SoC device\n"); + err = bcma_host_soc_init(&bcm47xx_bus.bcma); if (err) panic("Failed to initialize BCMA bus (err %d)", err); @@ -235,6 +264,7 @@ static int __init bcm47xx_register_bus_complete(void) #endif #ifdef CONFIG_BCM47XX_BCMA case BCM47XX_BUS_TYPE_BCMA: + de
[PATCH 1/2] bcma: keep a direct pointer to the struct device
From: Rafał Miłecki Accessing struct device is pretty useful/common so having a direct pointer: 1) Simplifies some code 2) Makes bcma_bus_get_host_dev() unneeded 3) Allows further improvements like using dev_* printing helpers Signed-off-by: Rafał Miłecki --- drivers/bcma/bcma_private.h | 1 - drivers/bcma/driver_gpio.c | 2 +- drivers/bcma/host_pci.c | 2 ++ drivers/bcma/host_soc.c | 4 ++-- drivers/bcma/main.c | 45 + include/linux/bcma/bcma.h | 11 +++-- 6 files changed, 18 insertions(+), 47 deletions(-) diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h index a4aac370f21f..1f0e66310b23 100644 --- a/drivers/bcma/bcma_private.h +++ b/drivers/bcma/bcma_private.h @@ -33,7 +33,6 @@ int __init bcma_bus_early_register(struct bcma_bus *bus); int bcma_bus_suspend(struct bcma_bus *bus); int bcma_bus_resume(struct bcma_bus *bus); #endif -struct device *bcma_bus_get_host_dev(struct bcma_bus *bus); /* scan.c */ void bcma_detect_chip(struct bcma_bus *bus); diff --git a/drivers/bcma/driver_gpio.c b/drivers/bcma/driver_gpio.c index 2c0ffb77d738..a5df3d111334 100644 --- a/drivers/bcma/driver_gpio.c +++ b/drivers/bcma/driver_gpio.c @@ -183,7 +183,7 @@ int bcma_gpio_init(struct bcma_drv_cc *cc) chip->direction_input = bcma_gpio_direction_input; chip->direction_output = bcma_gpio_direction_output; chip->owner = THIS_MODULE; - chip->parent= bcma_bus_get_host_dev(bus); + chip->parent= bus->dev; #if IS_BUILTIN(CONFIG_OF) chip->of_node = cc->core->dev.of_node; #endif diff --git a/drivers/bcma/host_pci.c b/drivers/bcma/host_pci.c index 63410ecfe640..f52239feb4cb 100644 --- a/drivers/bcma/host_pci.c +++ b/drivers/bcma/host_pci.c @@ -196,6 +196,8 @@ static int bcma_host_pci_probe(struct pci_dev *dev, goto err_pci_release_regions; } + bus->dev = &dev->dev; + /* Map MMIO */ err = -ENOMEM; bus->mmio = pci_iomap(dev, 0, ~0UL); diff --git a/drivers/bcma/host_soc.c b/drivers/bcma/host_soc.c index 2dce34789329..c8073b509a2b 100644 --- a/drivers/bcma/host_soc.c +++ b/drivers/bcma/host_soc.c @@ -179,7 +179,6 @@ int __init bcma_host_soc_register(struct bcma_soc *soc) /* Host specific */ bus->hosttype = BCMA_HOSTTYPE_SOC; bus->ops = &bcma_host_soc_ops; - bus->host_pdev = NULL; /* Initialize struct, detect chip */ bcma_init_bus(bus); @@ -213,6 +212,8 @@ static int bcma_host_soc_probe(struct platform_device *pdev) if (!bus) return -ENOMEM; + bus->dev = dev; + /* Map MMIO */ bus->mmio = of_iomap(np, 0); if (!bus->mmio) @@ -221,7 +222,6 @@ static int bcma_host_soc_probe(struct platform_device *pdev) /* Host specific */ bus->hosttype = BCMA_HOSTTYPE_SOC; bus->ops = &bcma_host_soc_ops; - bus->host_pdev = pdev; /* Initialize struct, detect chip */ bcma_init_bus(bus); diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c index fc1f4acdd189..6535614a7dc1 100644 --- a/drivers/bcma/main.c +++ b/drivers/bcma/main.c @@ -223,8 +223,8 @@ unsigned int bcma_core_irq(struct bcma_device *core, int num) mips_irq = bcma_core_mips_irq(core); return mips_irq <= 4 ? mips_irq + 2 : 0; } - if (bus->host_pdev) - return bcma_of_get_irq(&bus->host_pdev->dev, core, num); + if (bus->dev) + return bcma_of_get_irq(bus->dev, core, num); return 0; case BCMA_HOSTTYPE_SDIO: return 0; @@ -239,18 +239,18 @@ void bcma_prepare_core(struct bcma_bus *bus, struct bcma_device *core) core->dev.release = bcma_release_core_dev; core->dev.bus = &bcma_bus_type; dev_set_name(&core->dev, "bcma%d:%d", bus->num, core->core_index); - core->dev.parent = bcma_bus_get_host_dev(bus); - if (core->dev.parent) - bcma_of_fill_device(core->dev.parent, core); + core->dev.parent = bus->dev; + if (bus->dev) + bcma_of_fill_device(bus->dev, core); switch (bus->hosttype) { case BCMA_HOSTTYPE_PCI: - core->dma_dev = &bus->host_pci->dev; + core->dma_dev = bus->dev; core->irq = bus->host_pci->irq; break; case BCMA_HOSTTYPE_SOC: - if (IS_ENABLED(CONFIG_OF) && bus->host_pdev) { - core->dma_dev = &bus->host_pdev->dev; + if (IS_ENABLED(CONFIG_OF) && bus->dev) { + core->dma_dev = bus->dev;
[PATCH 2/2] bcma: use dev_* printing functions
From: Rafał Miłecki It provides more meaningful messages. Signed-off-by: Rafał Miłecki --- drivers/bcma/bcma_private.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h index 1f0e66310b23..6eded32d1aac 100644 --- a/drivers/bcma/bcma_private.h +++ b/drivers/bcma/bcma_private.h @@ -10,13 +10,13 @@ #include #define bcma_err(bus, fmt, ...) \ - pr_err("bus%d: " fmt, (bus)->num, ##__VA_ARGS__) + dev_err((bus)->dev, "bus%d: " fmt, (bus)->num, ##__VA_ARGS__) #define bcma_warn(bus, fmt, ...) \ - pr_warn("bus%d: " fmt, (bus)->num, ##__VA_ARGS__) + dev_warn((bus)->dev, "bus%d: " fmt, (bus)->num, ##__VA_ARGS__) #define bcma_info(bus, fmt, ...) \ - pr_info("bus%d: " fmt, (bus)->num, ##__VA_ARGS__) + dev_info((bus)->dev, "bus%d: " fmt, (bus)->num, ##__VA_ARGS__) #define bcma_debug(bus, fmt, ...) \ - pr_debug("bus%d: " fmt, (bus)->num, ##__VA_ARGS__) + dev_dbg((bus)->dev, "bus%d: " fmt, (bus)->num, ##__VA_ARGS__) struct bcma_bus; -- 2.20.1
Re: [PATCH] brcm: provide new firmwares for BCM4366 chipset
On Sat, 15 Dec 2018 at 12:03, Arend Van Spriel wrote: > On 12/14/2018 2:02 PM, Josh Boyer wrote: > > On Wed, Nov 7, 2018 at 3:45 PM Arend van Spriel > > wrote: > >> > >> These firmwares are for the BCM4366 3x3 802.11 ac chipsets, which also > >> comprise of BCM4366E or BCM43664 devices. Signed-off-by: Arend van Spriel > > > > This didn't add brcm/brcmfmac4366c-pcie.bin to WHENCE. I'm assuming > > it's under the same license as the rest of the brcmfmac files? If so, > > I can just amend the patch. > > Hi Josh, > > I was not sure this patch ever made it through. It did not appear on > linux-wireless list probably due to exceeding a size limit. So I resend > it as two patches on December 10th. However, I indeed forgot to update > the WHENCE file in that series as well. Indeed there is no license change. Unfortunately I have only 1 device with 4366B1 (D-Link DIR-885L) and it seems to have dummy NVRAM restored from the bootloader's copy: 1:ccode=ALL 0:regrev=0 0:ccode=ALL 1:regrev=0 1) Old fw 10.10.69.3309 (r610991) FWID 01-c47a91a4 2 GHz: very weak signal (about -79 dBm) 5 GHz: good signal & works stable (this also matches vendor firmware behavior) 2) New fw 10.28.2 (r769115) FWID 01-801fb449 2 GHz: very weak signal (about -79 dBm) 5 GHz: good signal but huge packet loss So the new firmware introduces 5 GHz regression for me, but I guess I should not complain since my device seems to come with a missing NVRAM regulatory info. Is there any chance you know a correct ccode + regrev for D-Link DIR-885L for any region? -- Rafał
Re: [PATCH 0/2] brcm: new firmware for BCM4366 chip family
On Mon, 10 Dec 2018 at 10:32, Arend van Spriel wrote: > This series includes the firmwares for the BCM4366 3x3 802.11ac chipset. > The firmware for the bcm4366c0 has been long overdue. > > I did submit the firmware files early November in a single patch, but > probably hit the majordomo limit of 100,000 character with it. So here > is the 2nd attempt. > > These patches apply to the master branch of the linux-firmware repository. Wow, this is a great news! Thank you! I didn't receive patch 1/2 and I also don't see it in the archive: https://marc.info/?l=linux-wireless&r=1&b=201812&w=2 Not sure if it has reached linux-firmware ML (I can't find its archives anywhere).
Re: brcmfmac: regression using AP mode
On 2018-11-24 18:58, Stefan Wahren wrote: today i wanted to setup an access point on my Raspberry Pi 3 A+ (BCM43455). Unfortunately the hostapd 2.4 shipped with Raspbian failed with recent Linux kernel: (...) I was able to bisect this issue down to this commit: 1204aa17f3b4 ("brcmfmac: set WIPHY_FLAG_HAVE_AP_SME flag") After reverting this commit hostapd works as expected. It ringed a bell, I did a quick research and found it. It's because of that ancient hostapd you're using. 2,5 years ago hostapd received a fix for its discovery of driver capabilities: commit f4830bed661f4adff51f50a0d37c64ceb748e780 Author: Rafał Miłecki Date: Mon Apr 25 17:10:47 2016 +0200 nl80211: Try running without mgmt frame subscription (driver AP SME) So your problem is the ancient hostapd that can't run with drivers that: 1) Report NL80211_ATTR_DEVICE_AP_SME 2) Don't support subscribing for PROBE_REQ and/or ACTION frames Technically there is nothing wrong with such drivers and it's just a hostapd bug (that's why it was fixed long time ago). Now, you could try *not* reporting NL80211_ATTR_DEVICE_AP_SME but then hostapd for drivers that: 1) Support monitor mode 2) Don't support subscribing for PROBE_REQ and/or ACTION frames will hit yet another mode discovery path in and break as well. That said there is no perfect solution. brcmfmac *should* set WIPHY_FLAG_HAVE_AP_SME. It's required for a sane capabilities discovery in hostapd. It's required to make sure other supplicants can work with brcmfmac as well. Possibly you can just update hostapd to anything more recent? I'm afraid the version you're using may suffer from a lot of security issues anyway
Re: [PATCH v9] Add new mac80211 driver mwlwifi.
On Wed, 8 Feb 2017 at 08:55, David Lin wrote: > > From: Rafał Miłecki [mailto:zaj...@gmail.com] worte: > > On 8 February 2017 at 08:28, David Lin wrote: > > >> From: Rafał Miłecki [mailto:zaj...@gmail.com] worte: > > >> Sent: Wednesday, February 08, 2017 3:24 PM On 8 February 2017 at > > >> 07:30, David Lin wrote: > > >> > steve.deros...@gmail.com [mailto:steve.deros...@gmail.com] wrote: > > >> >> On Tue, Feb 7, 2017 at 10:15 PM, David Lin wrote: > > >> >> >> Rafał Miłecki [mailto:zaj...@gmail.com] wrote: > > >> >> >> Please use ieee80211-freq-limit: > > >> >> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git > > >> >> >> /co > > >> >> >> mmi > > >> >> >> t/?id=b3 > > >> >> >> 30b25eaabda00d74e47566d9200907da381896 > > >> >> >> > > >> >> >> Most likely with wiphy_read_of_freq_limits helper: > > >> >> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git > > >> >> >> /co > > >> >> >> mmi > > >> >> >> t/?id=e6 > > >> >> >> 91ac2f75b69bee743f0370d79454ba4429b175 > > >> >> > > > >> >> > I already replied meaning of these parameters: > > >> >> > is used to disable 2g band. > > >> >> > is used to disable 5g band. > > >> >> > is used to specify antenna number (if > > >> >> > default number > > >> >> is suitable for you, there is no need to use this parameter). > > >> >> > should not be used for chip with device > > >> >> > power > > >> table. > > >> >> In fact, this parameter should not be used any more. > > >> >> > > > >> >> > > >> >> David, I think you're not understanding the comment, or at least > > >> >> that's what it looks like to me. Yes, you did reply as to the meaning. > > >> >> And, your reply, while informative, didn't tell us you understood > > >> >> and were willing to fix the problem. I doubt you meant it this > > >> >> way, but it feels defensive and like a brush-off. > > >> >> > > >> >> First off, you will still have to document any DT bindings you're > > >> >> adding. Just because you answer the question in the review doesn't > > >> >> mean you're not responsible for doing so. > > >> >> > > >> >> And second off, I think that Rafal (and sorry about my spelling, > > >> >> looks like there's some sort of accent on the l that I don't know > > >> >> how to make my keyboard do) is saying: there's already some > > >> >> generic bindings that can be used to disable the 2g or 5g bands. > > >> >> Granted they're even newer than your patch, but I do think if said > > >> >> bindings exist and > > >> are appropriate, you should use them. > > >> >> > > >> >> - Steve > > >> > > > >> > These parameters are marvell proprietary parameters, I don't think > > >> > it should > > >> be added to DT bindings. > > >> > > >> Steve is correct. > > >> > > >> You have to document new properties, just because they are Marvell > > >> specific doesn't change anything. You just document them in a proper > > place. > > >> > > > > > > All right. I will do that. > > > > > >> > > >> > BTW, and are only used for mwlwifi to > > >> > report > > >> supported bands, it is not related to limitation of frequency. > > >> > > >> How reporting a single band doesn't limit reported frequencies? You > > >> can achieve exactly the same using generic property, so there is no > > >> place for Marvell specific ones. > > >> > > >> In fact there were drivers of 3 vendors requiring band/freq-related > > >> description in DT: Broadcom, Marvell & Mediatek. This property was > > >> discussed & designed to support all limitation cases we found > > >> possible to make it usable with > > >> (hopefully) all drivers. > > >> > > > > > > I only need simple way to disable 2g or 5g band. I will follow your > > > suggestion > > to document these marvell proprietary parameters. > > > > Seriously? Refusing to use generic binding because you think marvell,5ghz; > > is > > simpler than ieee80211-freq-limit = <2402000 2482000>; (not to mention your > > property seems reversed!)? > > > > I don't know how else to explain this to you. We don't want duplicated > > properties where one can work. Just use existing one. Don't add new one even > > if documented. > > > > All right. I will check and let patch v10 to use it. For previous parameters, > they will only be used by previous OpenWrt projects. Hi David, how are things going? Could we get v10? -- Rafał
Re: wiki: tree labels in patches
On 2018-11-16 09:46, Kalle Valo wrote: (changing subject for better visibility and trimming Cc) Rafał Miłecki writes: On 2018-11-09 15:05, Kalle Valo wrote: Rafał Miłecki writes: From: Rafał Miłecki Driver can report IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ so it's important to provide valid & complete info about supported bands for each channel. By default no support for 160 MHz should be assumed unless firmware reports it for a given channel later. This fixes info passed to the userspace. Without that change userspace could try to use invalid channel and fail to start an interface. Signed-off-by: Rafał Miłecki Cc: sta...@vger.kernel.org Should this be queued to 4.20? That's my suggestion. I try to mark fixes (patches for currently developed release) with an extra FIX tag in a subject. Do you have any other method in mind that would be preferred by you? Yes, I do see your FIX tag in patchwork: [ 31] [FIX] brcmfmac: fix reporting support for 160 MHz channels 2018-11-08 But "FIX" is a bit ambigous as not all fixes not go to wireless-drivers, they can also go to wireless-drivers-next. So I prefer using the release number (or name of the tree) like this: [PATCH 4.20] brcmfmac: fix reporting support for 160 MHz channels After seeing your question I added something about this to the wiki which hopefully helps others: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#tree_labels Got it, thanks!
Re: [PATCH FIX] brcmfmac: fix reporting support for 160 MHz channels
On 2018-11-09 15:05, Kalle Valo wrote: Rafał Miłecki writes: From: Rafał Miłecki Driver can report IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ so it's important to provide valid & complete info about supported bands for each channel. By default no support for 160 MHz should be assumed unless firmware reports it for a given channel later. This fixes info passed to the userspace. Without that change userspace could try to use invalid channel and fail to start an interface. Signed-off-by: Rafał Miłecki Cc: sta...@vger.kernel.org Should this be queued to 4.20? That's my suggestion. I try to mark fixes (patches for currently developed release) with an extra FIX tag in a subject. Do you see those when using your patchwork tool? Do you have any other method in mind that would be preferred by you?
[PATCH FIX] brcmfmac: fix reporting support for 160 MHz channels
From: Rafał Miłecki Driver can report IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ so it's important to provide valid & complete info about supported bands for each channel. By default no support for 160 MHz should be assumed unless firmware reports it for a given channel later. This fixes info passed to the userspace. Without that change userspace could try to use invalid channel and fail to start an interface. Signed-off-by: Rafał Miłecki Cc: sta...@vger.kernel.org --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 230a378c26fc..7f0a5bade70a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -6005,7 +6005,8 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg, * for subsequent chanspecs. */ channel->flags = IEEE80211_CHAN_NO_HT40 | -IEEE80211_CHAN_NO_80MHZ; +IEEE80211_CHAN_NO_80MHZ | +IEEE80211_CHAN_NO_160MHZ; ch.bw = BRCMU_CHAN_BW_20; cfg->d11inf.encchspec(&ch); chaninfo = ch.chspec; -- 2.13.7
Re: [PATCH] brcmfmac: support STA info struct v7
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
[PATCH] brcmutil: print invalid chanspec when WARN-ing
From: Rafał Miłecki On one of my devices I got WARNINGs when brcmfmac tried to decode chanspec. I couldn't tell if it was some unsupported format or just a malformed value passed by a firmware. Print chanspec value so it's possible to debug a possible problem. Signed-off-by: Rafał Miłecki --- There is no rush with this one, it can wait for post 4.20 merge window. --- drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c index eb5db94f5745..8ac34821f1c1 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c @@ -128,7 +128,7 @@ static void brcmu_d11n_decchspec(struct brcmu_chan *ch) } break; default: - WARN_ON_ONCE(1); + WARN_ONCE(1, "Invalid chanspec 0x%04x\n", ch->chspec); break; } @@ -140,7 +140,7 @@ static void brcmu_d11n_decchspec(struct brcmu_chan *ch) ch->band = BRCMU_CHAN_BAND_2G; break; default: - WARN_ON_ONCE(1); + WARN_ONCE(1, "Invalid chanspec 0x%04x\n", ch->chspec); break; } } @@ -167,7 +167,7 @@ static void brcmu_d11ac_decchspec(struct brcmu_chan *ch) ch->sb = BRCMU_CHAN_SB_U; ch->control_ch_num += CH_10MHZ_APART; } else { - WARN_ON_ONCE(1); + WARN_ONCE(1, "Invalid chanspec 0x%04x\n", ch->chspec); } break; case BRCMU_CHSPEC_D11AC_BW_80: @@ -188,7 +188,7 @@ static void brcmu_d11ac_decchspec(struct brcmu_chan *ch) ch->control_ch_num += CH_30MHZ_APART; break; default: - WARN_ON_ONCE(1); + WARN_ONCE(1, "Invalid chanspec 0x%04x\n", ch->chspec); break; } break; @@ -222,13 +222,13 @@ static void brcmu_d11ac_decchspec(struct brcmu_chan *ch) ch->control_ch_num += CH_70MHZ_APART; break; default: - WARN_ON_ONCE(1); + WARN_ONCE(1, "Invalid chanspec 0x%04x\n", ch->chspec); break; } break; case BRCMU_CHSPEC_D11AC_BW_8080: default: - WARN_ON_ONCE(1); + WARN_ONCE(1, "Invalid chanspec 0x%04x\n", ch->chspec); break; } @@ -240,7 +240,7 @@ static void brcmu_d11ac_decchspec(struct brcmu_chan *ch) ch->band = BRCMU_CHAN_BAND_2G; break; default: - WARN_ON_ONCE(1); + WARN_ONCE(1, "Invalid chanspec 0x%04x\n", ch->chspec); break; } } -- 2.13.7
[PATCH 4.20 FIX] brcmutil: really fix decoding channel info for 160 MHz bandwidth
From: Rafał Miłecki Previous commit /adding/ support for 160 MHz chanspecs was incomplete. It didn't set bandwidth info and didn't extract control channel info. As the result it was also using uninitialized "sb" var. This change has been tested for two chanspecs found to be reported by some devices/firmwares: 1) 60/160 (0xee32) Before: chnum:50 control_ch_num:36 After: chnum:50 control_ch_num:60 2) 120/160 (0xed72) Before: chnum:114 control_ch_num:100 After: chnum:114 control_ch_num:120 Fixes: 330994e8e8ec ("brcmfmac: fix for proper support of 160MHz bandwidth") Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c index e7584b842dce..eb5db94f5745 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c @@ -193,6 +193,9 @@ static void brcmu_d11ac_decchspec(struct brcmu_chan *ch) } break; case BRCMU_CHSPEC_D11AC_BW_160: + ch->bw = BRCMU_CHAN_BW_160; + ch->sb = brcmu_maskget16(ch->chspec, BRCMU_CHSPEC_D11AC_SB_MASK, +BRCMU_CHSPEC_D11AC_SB_SHIFT); switch (ch->sb) { case BRCMU_CHAN_SB_LLL: ch->control_ch_num -= CH_70MHZ_APART; -- 2.13.7
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
On Wed, 5 Sep 2018 at 13:06, Arend van Spriel wrote: > 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. Thank you! :) -- Rafał
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
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? -- Rafał
[DEBUG PATCH] brcmfmac: add firmware watchdog running on host machine
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. 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. 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 --- .../net/wireless/broadcom/brcm80211/brcmfmac/core.c | 21 + .../net/wireless/broadcom/brcm80211/brcmfmac/core.h | 2 ++ 2 files changed, 23 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 03bfbcc3ca6e..4d928ca795b1 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -40,6 +40,7 @@ #include "common.h" #define MAX_WAIT_FOR_8021X_TX msecs_to_jiffies(950) +#define BRCMF_FW_WATCHDOG_POLL msecs_to_jiffies(3000) #define BRCMF_BSSIDX_INVALID -1 @@ -1088,6 +1089,20 @@ static int brcmf_revinfo_read(struct seq_file *s, void *data) return 0; } +static void brcmf_fw_watchdog(struct work_struct *work) +{ + struct brcmf_pub *pub = container_of(work, struct brcmf_pub, fw_watchdog.work); + struct brcmf_if *ifp = pub->iflist[0]; + s32 ver; + int err; + + err = brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_VERSION, &ver); + if (err) + brcmf_err("Firmware didn't respond: %d (firmware crash?)\n", err); + + schedule_delayed_work(&pub->fw_watchdog, BRCMF_FW_WATCHDOG_POLL); +} + static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) { int ret = -1; @@ -1159,6 +1174,9 @@ static int brcmf_bus_started(struct brcmf_pub *drvr, struct cfg80211_ops *ops) #endif #endif /* CONFIG_INET */ + INIT_DELAYED_WORK(&drvr->fw_watchdog, brcmf_fw_watchdog); + schedule_delayed_work(&drvr->fw_watchdog, BRCMF_FW_WATCHDOG_POLL); + /* populate debugfs */ brcmf_debugfs_add_entry(drvr, "revinfo", brcmf_revinfo_read); brcmf_feat_debugfs_create(drvr); @@ -1287,6 +1305,9 @@ void brcmf_detach(struct device *dev) if (drvr == NULL) return; + cancel_delayed_work(&drvr->fw_watchdog); + flush_scheduled_work(); + #ifdef CONFIG_INET unregister_inetaddr_notifier(&drvr->inetaddr_notifier); #endif diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h index 2d37a2fc6a6f..1afb3f0ca585 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h @@ -143,6 +143,8 @@ struct brcmf_pub { struct notifier_block inet6addr_notifier; struct brcmf_mp_device *settings; + struct delayed_work fw_watchdog; + u8 clmver[BRCMF_DCMD_SMLEN]; }; -- 2.13.7
[PATCH] brcmfmac: fix regression in parsing NVRAM for multiple devices
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+ 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; -- 2.13.7
[PATCH V2] brcmfmac: specify some features per firmware version
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. Signed-off-by: Rafał Miłecki Reviewed-by: Arend van Spriel --- V2: Added "commit" word when specifying SHAs. Thanks Arend! --- .../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..8347da632a5b 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 commit ea1178515b88 */ + { "01-6cb8e269", BIT(BRCMF_FEAT_MONITOR) }, + /* brcmfmac4366b-pcie.bin from linux-firmware.git commit 52442afee990 */ + { "01-c47a91a4", BIT(BRCMF_FEAT_MONITOR) }, +}; + +static void brcmf_feat_firmware_overrides(struct brcmf_pub *drv) +{ + const struct brcmf_feat_fwfeat *e; + u32 feat_flags = 0; + int i; + + for (i = 0; i < ARRAY_SIZE(brcmf_feat_fwfeat_map); i++) { + e = &brcmf_feat_fwfeat_map[i]; + if (!strcmp(e->fwid, drv->fwver)) { + feat_flags = e->feat_flags; + break; + } + } + + if (!feat_flags) + return; + + for (i = 0; i < BRCMF_FEAT_LAST; i++) + if (feat_flags & BIT(i)) + brcmf_dbg(INFO, "enabling firmware feature: %s\n", + brcmf_feat_names[i]); + drv->feat_flags |= feat_flags; +} + /** * brcmf_feat_iovar_int_get() - determine feature through iovar query. * @@ -253,6 +289,8 @@ void brcmf_feat_attach(struct brcmf_pub *drvr) } brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_FWSUP, "sup_wpa"); + brcmf_feat_firmware_overrides(drvr); + /* set chip related quirks */ switch (drvr->bus_if->chip) { case BRCM_CC_43236_CHIP_ID: -- 2.13.7
[PATCH] brcmfmac: specify some features per firmware version
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. 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) }, +}; + +static void brcmf_feat_firmware_overrides(struct brcmf_pub *drv) +{ + const struct brcmf_feat_fwfeat *e; + u32 feat_flags = 0; + int i; + + for (i = 0; i < ARRAY_SIZE(brcmf_feat_fwfeat_map); i++) { + e = &brcmf_feat_fwfeat_map[i]; + if (!strcmp(e->fwid, drv->fwver)) { + feat_flags = e->feat_flags; + break; + } + } + + if (!feat_flags) + return; + + for (i = 0; i < BRCMF_FEAT_LAST; i++) + if (feat_flags & BIT(i)) + brcmf_dbg(INFO, "enabling firmware feature: %s\n", + brcmf_feat_names[i]); + drv->feat_flags |= feat_flags; +} + /** * brcmf_feat_iovar_int_get() - determine feature through iovar query. * @@ -253,6 +289,8 @@ void brcmf_feat_attach(struct brcmf_pub *drvr) } brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_FWSUP, "sup_wpa"); + brcmf_feat_firmware_overrides(drvr); + /* set chip related quirks */ switch (drvr->bus_if->chip) { case BRCM_CC_43236_CHIP_ID: -- 2.13.7
Re: [PATCH] brcmfmac: update STA info struct to the v5
On 2018-06-30 20:48, Arend van Spriel wrote: 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... Yes. As stated in the commit message, firmware 10.10.122.20 uses v5. I didn't list vendors using that firmware, but if you take a look at the side thread "Research + questions on brcmfmac and support for monitor mode", you'll see that 10.10.122.20 is used in: 1) FW_EA9500v2_EA9500S_2.1.1.183171_prod.img 2) GT-AC5300_3.0.0.4_382_15984-gf481f58_cferom_ubi_0824.w 3) ArcherC5400X(US)_171023.bin That means I explicitly need v5 to make brcmfmac work with any 10.10.122.20 firmware from above images. Unfortunately I didn't find any vendor release their GPL package with Broadcom's SDK using v6 or v7. I hope it's backward compatible and maybe you'll be able to provide patch on top of mine? Acked-by: Arend van Spriel Thanks!
Re: [PATCH V4 2/3] brcmfmac: detect firmware support for radiotap monitor frames
On 2018-06-29 09:03, Kalle Valo wrote: Rafał Miłecki writes: From: Rafał Miłecki Depending on used build-time options some firmwares may already include radiotap header in passed monitor frames. Add a new feature flag to store info about it. It's needed for proper handling of received frames before passing them up. Signed-off-by: Rafał Miłecki Signed-off-by: Arend van Spriel BTW, we now have Co-Developed-by: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by But no need to change this patch, just a tip for the future. I assume this patchset is good to go and will apply it soon. Nice to know, thanks!
[PATCH] brcmfmac: update STA info struct to the v5
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. 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_REQUIRED 2 +#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 { -- 2.13.7
[PATCH] brcmfmac: define more bits for the flags of struct brcmf_sta_info_le
From: Rafał Miłecki That struct is passed by a firmware when querying for STA info. Flags are used to indicate what info could be obtained. These new defines may allow passing more info to the cfg80211 in the future. They had been obtained from Broadcom's SDK file wlioctl_defs.h used by DD-WRT. Signed-off-by: Rafał Miłecki --- .../broadcom/brcm80211/brcmfmac/fwil_types.h | 29 ++ 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h index 4b290705e3e6..9b3a58e89dd1 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h @@ -32,11 +32,30 @@ #defineBRCMF_BSS_INFO_VERSION 109 /* curr ver of brcmf_bss_info_le struct */ #define BRCMF_BSS_RSSI_ON_CHANNEL 0x0002 -#define BRCMF_STA_WME 0x0002 /* WMM association */ -#define BRCMF_STA_AUTHE0x0008 /* Authenticated */ -#define BRCMF_STA_ASSOC0x0010 /* Associated */ -#define BRCMF_STA_AUTHO0x0020 /* Authorized */ -#define BRCMF_STA_SCBSTATS 0x4000 /* Per STA debug stats */ +#define BRCMF_STA_BRCM 0x0001 /* Running a Broadcom driver */ +#define BRCMF_STA_WME 0x0002 /* WMM association */ +#define BRCMF_STA_NONERP 0x0004 /* No ERP */ +#define BRCMF_STA_AUTHE0x0008 /* Authenticated */ +#define BRCMF_STA_ASSOC0x0010 /* Associated */ +#define BRCMF_STA_AUTHO0x0020 /* Authorized */ +#define BRCMF_STA_WDS 0x0040 /* Wireless Distribution System */ +#define BRCMF_STA_WDS_LINKUP 0x0080 /* WDS traffic/probes flowing properly */ +#define BRCMF_STA_PS 0x0100 /* STA is in power save mode from AP's viewpoint */ +#define BRCMF_STA_APSD_BE 0x0200 /* APSD delv/trigger for AC_BE is default enabled */ +#define BRCMF_STA_APSD_BK 0x0400 /* APSD delv/trigger for AC_BK is default enabled */ +#define BRCMF_STA_APSD_VI 0x0800 /* APSD delv/trigger for AC_VI is default enabled */ +#define BRCMF_STA_APSD_VO 0x1000 /* APSD delv/trigger for AC_VO is default enabled */ +#define BRCMF_STA_N_CAP0x2000 /* STA 802.11n capable */ +#define BRCMF_STA_SCBSTATS 0x4000 /* Per STA debug stats */ +#define BRCMF_STA_AMPDU_CAP0x8000 /* STA AMPDU capable */ +#define BRCMF_STA_AMSDU_CAP0x0001 /* STA AMSDU capable */ +#define BRCMF_STA_MIMO_PS 0x0002 /* mimo ps mode is enabled */ +#define BRCMF_STA_MIMO_RTS 0x0004 /* send rts in mimo ps mode */ +#define BRCMF_STA_RIFS_CAP 0x0008 /* rifs enabled */ +#define BRCMF_STA_VHT_CAP 0x0010 /* STA VHT(11ac) capable */ +#define BRCMF_STA_WPS 0x0020 /* WPS state */ +#define BRCMF_STA_DWDS_CAP 0x0100 /* DWDS CAP */ +#define BRCMF_STA_DWDS 0x0200 /* DWDS active */ /* size of brcmf_scan_params not including variable length array */ #define BRCMF_SCAN_PARAMS_FIXED_SIZE 64 -- 2.13.7
Re: [PATCH V3 0/2] brcmfmac: initial work for adding monitor mode
On 2018-06-25 10:28, Arend van Spriel wrote: 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. None of my firmwares have "monitor" flag in the "cap" iovar. This seems to be something really new. I'm still looking for some firmware new enough to report "monitor" flag. Unfortunately most vendors seem to be using 10.10.69.* or 10.10.122.20 which aren't new enough for that.
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
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. I wasn't aware of this, good to know, thanks! -- Rafał
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
On Mon, 25 Jun 2018 at 10:18, Arend van Spriel wrote: > 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. Well, things happen, I don't mind if you un-ack my patch, after discovering some problem with it :) -- Rafał
Re: Research + questions on brcmfmac and support for monitor mode
On 15.05.2018 14:29, Rafał Miłecki wrote: I was hoping that maybe looking at fw-reported capabilities will give me any hint regarding that but I'm afraid I'm out of luck. Below is a list of firmwares I tested and summary of each of them. Note: as every firmware reports following capabilities: 802.11d 802.11h ampdu ampdu_rx ampdu_tx amsdurx amsdutx anqpo ap bcm_dcs bsstrans cac cqa dfrts dwds led mfp p2po probresp_mac_filter pspretend psr psta radio_pwrsave rm rxchain_pwrsave sta stbc-rx-1ss stbc-tx traffic-mgmt traffic-mgmt-dwm vht-prop-rates wds wet wet_tunnel wme wnm I omitted them below. I'm sending an updated (with "cap" and "sta_monitor" info) summary of firmwares I tested. * 1) brcmfmac43602-pcie.ap.bin from linux-firmware.git 43602a1-roml/pcie-ag-splitrx-fdap-mbss-mfp-wl11k-wl11u-txbf-pktctx-amsdutx-ampduretry-chkd2hdma-proptxstatus-11nprop Version: 7.35.177.56 CRC: 548eccd4 Date: Fri 2015-09-18 03:31:06 PDT Ucode Ver: 986.122 FWID: 01-6cb8e269 Firmware version = wl0: Sep 18 2015 03:30:01 version 7.35.177.56 (r587209) FWID 01-6cb8e269 Monitor frames: yes Monitor frame flags: 0x0001 Radiotap: no Flag "monitor" in the "cap" iovar: no FW "sta_monitor" error (DOWN): -4 (BCME_NOTUP) FW "sta_monitor" error (UP): -23 (BCME_UNSUPPORTED) * 2) brcmfmac4366b-pcie.bin from linux-firmware.git 4366b1-roml/pcie-ag-splitrx-fdap-mbss-mfp-wnm-osen-wl11k-wl11u-txbf-pktctx-amsdutx-ampduretry-chkd2hdma-proptxstatus-11nprop-obss-dbwsw-ringer-dmaindex16-bgdfs Version: 10.10.69.237 CRC: 4bc48c7b Date: Fri 2016-01-08 12:55:25 PST Ucode Ver: 1073.531 FWID: 01-c47a91a4 Firmware version = wl0: Jan 8 2016 12:54:07 version 10.10.69.3309 (r610991) FWID 01-c47a91a4 Monitor frames: yes Monitor frame flags: 0x0001 Radiotap: no Flag "monitor" in the "cap" iovar: no FW "sta_monitor" error (DOWN): -4 (BCME_NOTUP) FW "sta_monitor" error (UP): -23 (BCME_UNSUPPORTED) * 3) 4366b1 development branch (from Arend) 4366b1-roml/pcie-ag-splitrx-fdap-mbss-mfp-wnm-osen-wl11k-wl11u-txbf-pktctx-amsdutx-ampduretry-chkd2hdma-proptxstatus-11nprop-obss-dbwsw-ringer-dmaindex16-bgdfs Version: 10.10 (TOB) (r663589) CRC: 50d4be62 Date: Thu 2016-10-06 10:46:42 BST Ucode Ver: 1128.155 FWID: 01-6c5a1687 Firmware version = wl0: Oct 6 2016 10:17:32 version 10.10 (TOB) (r663589) FWID 01-6c5a1687 Monitor frames: yes Monitor frame flags: 0x0001 Radiotap: no Flag "monitor" in the "cap" iovar: no FW "sta_monitor" error (DOWN): -4 (BCME_NOTUP) FW "sta_monitor" error (UP): -23 (BCME_UNSUPPORTED) * 4) brcmfmac4366c-pcie.bin.k3 4366c0-roml/pcie-ag-splitrx-fdap-mbss-mfgtest-seqcmds-phydbg-phydump-txbf-pktctx-amsdutx-ampduretry-chkd2hdma-11nprop-dbgam-dbgams-ringer-dmaindex16-bgdfs-hostpmac Version: 10.10.69.74 CRC: a6268b76 Date: Mon 2016-09-12 16:39:23 CST FWID 01-5c0166fa Firmware version = wl0: Aug 19 2016 15:22:35 version 10.10.69.74 (r629731 WLTEST) FWID 01-5c0166fa Monitor frames: yes Monitor frame flags: 0x0001 Radiotap: no Flag "monitor" in the "cap" iovar: no FW "sta_monitor" error (DOWN): -4 (BCME_NOTUP) FW "sta_monitor" error (UP): -23 (BCME_UNSUPPORTED) * 5) brcmfmac4366c-pcie.bin.ea9500 4366c0-roml/pcie-ag-splitrx-fdap-mbss-mfp-wnm-osen-wl11k-wl11u-txbf-pktctx-amsdutx-ampduretry-chkd2hdma-proptxstatus-11nprop-obss-dbwsw-ringer-dmaindex16-bgdfs-hostpmac Version: 10.10.69.69 CRC: 34d30c8c Date: Tue 2016-08-23 17:31:24 PDT FWID 01-8438621f Firmware version = wl0: Aug 23 2016 17:19:51 version 10.10.69.69 (r625687) FWID 01-8438621f Monitor frames: yes Monitor frame flags: 0x0001 Radiotap: no Flag "monitor" in the "cap" iovar: no FW "sta_monitor" error (DOWN): -4 (BCME_NOTUP) FW "sta_monitor" error (UP): -23 (BCME_UNSUPPORTED) * 6) brcmfmac4366c-pcie.bin.ac88u 4366c0-roml/pcie-ag-splitrx-fdap-mbss-mfp-wnm-osen-wl11k-wl11u-txbf-pktctx-amsdutx-ampduretry-chkd2hdma-proptxstatus-11nprop-obss-dbwsw-ringer-dmaindex16-bgdfs-hostpmac-txpwr Version: 10.10.69.252 CRC: 9fa88ab1 Date: Mon 2016-09-12 13:28:49 CST Ucode Ver: 1073.579 FWID: 01-fed440e1 Firmware version = wl0: Sep 12 2016 13:26:44 version 10.10.69.6908 (r658761) FWID 01-fed440e1 Monitor frames: yes Monitor frame flags: 0x0001 Radiotap: no Flag "monitor" in the "cap" iovar: no FW "sta_monitor" error (DOWN): -4 (BCME_NOTUP) FW "sta_monitor" error (UP): -23 (BCME_UNSUPPORTED) * 7) brcmfmac4366c-pcie.bin.asus-dhd24 4366c0-roml/pcie-ag-splitrx-fdap-mbss-mfp-wnm-osen-wl11k-wl11u-txbf-pktctx-amsdutx-ampduretry-chkd2hdma-proptxstatus-11nprop-obss-dbwsw-ringer-dmaindex16-bgdfs-hostpmac-txpwr-stamon Version: 10.10.69.69017 (r730013) CRC: cf0b5621 Date: Tue 2017-11-07 12:34:00 CST Ucode Ver: 1073.579 FWID: 01-e258597c Firmware version = wl0: Nov 7 2017 12:2
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
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. -- Rafał
[PATCH V4 1/3] brcmfmac: detect firmware support for monitor interface
From: Rafał Miłecki Many/most of firmwares support creating monitor interface but only the most recent ones explicitly /announce/ it using a "monitor" entry in the list of capabilities. Check for that entry 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. Later some fallback detecion method may be added for older firmwares. For now just stick to the "monitor" capability which should be 100% reliable. Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel --- V3: Patch added to the series V4: Dropped fallback code as it was reported to be incorrect Updated new feature flag description as preferred by Arend --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 1 + drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c index 800a423c7bc2..a78b9bae44e0 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c @@ -48,6 +48,7 @@ static const struct brcmf_feat_fwcap brcmf_fwcap_map[] = { { BRCMF_FEAT_MBSS, "mbss" }, { BRCMF_FEAT_MCHAN, "mchan" }, { BRCMF_FEAT_P2P, "p2p" }, + { BRCMF_FEAT_MONITOR, "monitor" }, }; #ifdef DEBUG diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h index d1193825e559..3415d5d4d6b5 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h @@ -33,6 +33,7 @@ * MFP: 802.11w Management Frame Protection. * GSCAN: enhanced scan offload feature. * FWSUP: Firmware supplicant. + * MONITOR: firmware can pass monitor packets to host. */ #define BRCMF_FEAT_LIST \ BRCMF_FEAT_DEF(MBSS) \ @@ -48,7 +49,8 @@ BRCMF_FEAT_DEF(WOWL_ARP_ND) \ BRCMF_FEAT_DEF(MFP) \ BRCMF_FEAT_DEF(GSCAN) \ - BRCMF_FEAT_DEF(FWSUP) + BRCMF_FEAT_DEF(FWSUP) \ + BRCMF_FEAT_DEF(MONITOR) /* * Quirks: -- 2.13.7
[PATCH V4 3/3] brcmfmac: handle msgbuf packets marked with monitor mode flag
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). Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel --- V2: Use cpu_to_le16 when setting it_len V3: Update TODO comments Rename flag (after adding MONITOR) Update commit message V4: Added missing return; after brcmu_pkt_buf_free_skb() call --- .../wireless/broadcom/brcm80211/brcmfmac/core.c| 25 ++ .../wireless/broadcom/brcm80211/brcmfmac/core.h| 2 ++ .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 18 3 files changed, 45 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 72954fd6df3b..b1f702faff4f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -404,6 +405,30 @@ void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb) netif_rx_ni(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)) { + /* Do nothing */ + } else { + struct ieee80211_radiotap_header *radiotap; + + /* TODO: use RX status to fill some radiotap data */ + radiotap = skb_push(skb, sizeof(*radiotap)); + memset(radiotap, 0, sizeof(*radiotap)); + radiotap->it_len = cpu_to_le16(sizeof(*radiotap)); + + /* TODO: 4 bytes with receive status? */ + skb->len -= 4; + } + + skb->dev = ifp->ndev; + skb_reset_mac_header(skb); + skb->pkt_type = PACKET_OTHERHOST; + skb->protocol = htons(ETH_P_802_2); + + brcmf_netif_rx(ifp, skb); +} + static int brcmf_rx_hdrpull(struct brcmf_pub *drvr, struct sk_buff *skb, struct brcmf_if **ifp) { diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h index 401f50458686..dcf6e27cc16f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h @@ -121,6 +121,7 @@ struct brcmf_pub { struct brcmf_if *iflist[BRCMF_MAX_IFS]; s32 if2bss[BRCMF_MAX_IFS]; + struct brcmf_if *mon_if; struct mutex proto_block; unsigned char proto_buf[BRCMF_DCMD_MAXLEN]; @@ -216,6 +217,7 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp, enum brcmf_netif_stop_reason reason, bool state); void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success); 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); void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on); int __init brcmf_core_init(void); void __exit brcmf_core_exit(void); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c index c40ba8855cd5..4e8397a0cbc8 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c @@ -69,6 +69,8 @@ #define BRCMF_MSGBUF_MAX_EVENTBUF_POST 8 #define BRCMF_MSGBUF_PKT_FLAGS_FRAME_802_3 0x01 +#define BRCMF_MSGBUF_PKT_FLAGS_FRAME_802_110x02 +#define BRCMF_MSGBUF_PKT_FLAGS_FRAME_MASK 0x07 #define BRCMF_MSGBUF_PKT_FLAGS_PRIO_SHIFT 5 #define BRCMF_MSGBUF_TX_FLUSH_CNT1 32 @@ -1128,6 +1130,7 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) struct sk_buff *skb; u16 data_offset; u16 buflen; + u16 flags; u32 idx; struct brcmf_if *ifp; @@ -1137,6 +1140,7 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) data_offset = le16_to_cpu(rx_complete->data_offset); buflen = le16_to_cpu(rx_complete->data_len); idx = le32_to_cpu(rx_complete->msg.request_id); + flags = le16_to_cpu(rx_complete->flags); skb = brcmf_msgbuf_get_pktid(msgbuf->drvr->bus_if->dev, msgbu
[PATCH V4 2/3] brcmfmac: detect firmware support for radiotap monitor frames
From: Rafał Miłecki Depending on used build-time options some firmwares may already include radiotap header in passed monitor frames. Add a new feature flag to store info about it. It's needed for proper handling of received frames before passing them up. Signed-off-by: Rafał Miłecki Signed-off-by: Arend van Spriel --- V4: Patch extracted out of: [PATCH V3 2/2] brcmfmac: handle monitor mode marked msgbuf packets Added mapping for the "rtap" string as reported by Arend Updated new feature flag description as preferred by Arend --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 1 + drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c index a78b9bae44e0..4db4d07a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c @@ -49,6 +49,7 @@ static const struct brcmf_feat_fwcap brcmf_fwcap_map[] = { { BRCMF_FEAT_MCHAN, "mchan" }, { BRCMF_FEAT_P2P, "p2p" }, { BRCMF_FEAT_MONITOR, "monitor" }, + { BRCMF_FEAT_MONITOR_FMT_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 3415d5d4d6b5..0b4974df353a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h @@ -34,6 +34,7 @@ * GSCAN: enhanced scan offload feature. * FWSUP: Firmware supplicant. * MONITOR: firmware can pass monitor packets to host. + * MONITOR_FMT_RADIOTAP: firmware provides monitor packets with radiotap header */ #define BRCMF_FEAT_LIST \ BRCMF_FEAT_DEF(MBSS) \ @@ -50,7 +51,8 @@ BRCMF_FEAT_DEF(MFP) \ BRCMF_FEAT_DEF(GSCAN) \ BRCMF_FEAT_DEF(FWSUP) \ - BRCMF_FEAT_DEF(MONITOR) + BRCMF_FEAT_DEF(MONITOR) \ + BRCMF_FEAT_DEF(MONITOR_FMT_RADIOTAP) /* * Quirks: -- 2.13.7
Re: [PATCH V3 0/2] brcmfmac: initial work for adding monitor mode
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. > 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. -- Rafał
Re: [PATCH 6/6] brcmfmac: fallback mechanism to determine monitor mode features
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. 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. -- Rafał
Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode
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.