[PATCH] brcmfmac: Call brcmf_dmi_probe before brcmf_of_probe

[Hans de Goede]

ARM systems with UEFI may have both devicetree (of) and DMI data in this
case we end up setting brcmf_mp_device.board_type twice.

In this case we should prefer the devicetree data, because:
1) The devicerree data is more reliable
2) Some ARM systems (e.g. the Raspberry Pi 3 models) support both UEFI and
   classic uboot booting, the devicetree data is always there, so using it
   makes sure we ask for the same nvram file independent of how we booted.

This commit moves the brcmf_dmi_probe call to before the brcmf_of_probe
call, so that the latter can override the value of the first if both are
set.

Fixes: bd1e82bb420a ("brcmfmac: Set board_type from DMI on x86 based ...")
Cc: Peter Robinson 
Tested-and-reported-by: Peter Robinson 
Signed-off-by: Hans de Goede 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index d52774c6489b..0bb16bf574e3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -449,8 +449,8 @@ struct brcmf_mp_device *brcmf_get_module_param(struct 
device *dev,
}
if (!found) {
/* No platform data for this device, try OF and DMI data */
-   brcmf_of_probe(dev, bus_type, settings);
brcmf_dmi_probe(settings, chip, chiprev);
+   brcmf_of_probe(dev, bus_type, settings);
}
return settings;
 }
-- 
2.19.1

Re: [PATCH 1/6] brcmfmac: Remove firmware-loading code duplication

[Hans de Goede]

Hi,

On 05-11-18 16:05, Kalle Valo wrote:

Arend van Spriel  writes:


On 10/9/2018 2:47 PM, Hans de Goede wrote:

brcmf_fw_request_next_item and brcmf_fw_request_done both have identical
code to complete the fw-request depending on the item-type.

This commit adds a new brcmf_fw_complete_request helper removing this code
duplication.


Reviewed-by: Arend van Spriel 


For some reason you commented on v1 but there was v2 posted already:

https://patchwork.kernel.org/patch/10634355/

I guess I can take v2 still?


Yes the only thing different in v2 was dropping the SPDX license header
for the new drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
file and replacing it with the full ISC license text as seen in other
brcmfmac files. Nothing else was changed, so the review of v1 is
valid for v2 too.

Arend had one very minor comment on the name of a variable in the fifth
patch, but that is not important so if you want to pick up v2 as is
go for it.

Note the ISC license text is now in Torvald's tree as:
LICENSES/other/ISC

So could even go with v1, but I guess you prefer to move all files of
a driver over to the SPDX headers in one go ...

Regards,

Hans

Re: [PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines

[Hans de Goede]

Hi,

On 05-11-18 12:41, Arend van Spriel wrote:

On 10/9/2018 2:47 PM, Hans de Goede wrote:

For x86 based machines, set the board_type used for nvram file selection
based on the DMI sys-vendor and product-name strings.

Since on some models these strings are too generic, this commit also adds
a quirk table overriding the strings for models listed in that table.

The board_type setting is used to load the board-specific nvram file with
a board-specific name so that we can ship files for each supported board
in linux-firmware.


some comments below

Reviewed-by: Arend van Spriel 

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/Makefile  |   2 +
 .../broadcom/brcm80211/brcmfmac/common.c  |   3 +-
 .../broadcom/brcm80211/brcmfmac/common.h  |   7 ++
 .../broadcom/brcm80211/brcmfmac/dmi.c | 104 ++
 4 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
index 1f5a9b948abf..22fd95a736a8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
@@ -54,3 +54,5 @@ brcmfmac-$(CONFIG_BRCM_TRACING) += \
 tracepoint.o
 brcmfmac-$(CONFIG_OF) += \
 of.o
+brcmfmac-$(CONFIG_DMI) += \
+    dmi.o


Assuming OF and DMI are mutual exclusive, right?


Not necessarily ACPI tables can embed of/devicetree data now, so an x86
machine could have of-data, but having both compiled in is not an issue
I would expect only one of them to actually be present (so either of-data
or an entry in the DMI platform-data table).



diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
new file mode 100644
index ..fadc0ec745b8
--- /dev/null
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c


[...]


+static const struct dmi_system_id dmi_platform_data[] = {


maybe call this dmi_platform_quirk as in brcmf_dmi_probe() you call this a "quirk 
table".


OK, will fix for v3. I will also switch back to the SPDX license header
for v3, since the ISC license text is now in Linus' tree.

Regards,

Hans

Re: [PATCH 6/6] brcmfmac: Cleanup brcmf_fw_request_done()

[Hans de Goede]

Hi,

On 05-11-18 12:42, Arend van Spriel wrote:

On 10/9/2018 2:47 PM, Hans de Goede wrote:

The "cur" variable is now only used for a debug print and we already
print the same info from brcmf_fw_complete_request(), so the debug print
does not provide any extra info and we can remove it.


I guess this could have been collapsed in the first patch introducing 
brcmf_fw_complete_request(). All-in-all a nice cleanup. Thanks.


The idea of doing this in 2 separate patches is to make the first
patch (which is non trivial) easier to review.

Regards,

Hans




Reviewed-by: Arend van Spriel 

Signed-off-by: Hans de Goede 
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/firmware.c   | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

Re: [PATCH 3/6] brcmfmac: Add support for first trying to get a board specific nvram file

[Hans de Goede]

Hi,

Thank you for the reviews.

On 05-11-18 12:41, Arend van Spriel wrote:

On 10/9/2018 2:47 PM, Hans de Goede wrote:

The nvram files which some brcmfmac chips need are board-specific. To be
able to distribute these as part of linux-firmware, so that devices with
such a wifi chip will work OOTB, multiple (one per board) versions must
co-exist under /lib/firmware.

This commit adds support for callers of the brcmfmac/firmware.c code to
pass in a board_type parameter through the request structure.

If that parameter is set then the code will first try to load
chipmodel.board_type.txt before falling back to the old chipmodel.txt name.


minor comment below...

Reviewed-by: Arend van Spriel 

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/firmware.c    | 27 ++-
 .../broadcom/brcm80211/brcmfmac/firmware.h    |  1 +
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 08aaf99fee34..6755b2388fbc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -532,6 +532,31 @@ static int brcmf_fw_complete_request(const struct firmware 
*fw,
 return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
 }

+static int brcmf_fw_request_firmware(const struct firmware **fw,
+ struct brcmf_fw *fwctx)
+{
+    struct brcmf_fw_item *cur = >req->items[fwctx->curpos];
+    int ret;
+
+    /* nvram files are board-specific, first try a board-specific path */
+    if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
+    char alt_path[BRCMF_FW_NAME_LEN];
+
+    strlcpy(alt_path, cur->path, BRCMF_FW_NAME_LEN);
+    /* strip .txt at the end */
+    alt_path[strlen(alt_path) - 4] = 0;
+    strlcat(alt_path, ".", BRCMF_FW_NAME_LEN);


why not string just "txt"?


I'm not entirely sure what your question exactly is here?

Regards,

Hans





+    strlcat(alt_path, fwctx->req->board_type, BRCMF_FW_NAME_LEN);
+    strlcat(alt_path, ".txt", BRCMF_FW_NAME_LEN);
+
+    ret = request_firmware(fw, alt_path, fwctx->dev);
+    if (ret == 0)
+    return ret;
+    }
+
+    return request_firmware(fw, cur->path, fwctx->dev);
+}
+
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
 {
 struct brcmf_fw *fwctx = ctx;
@@ -544,7 +569,7 @@ static void brcmf_fw_request_done(const struct firmware 
*fw, void *ctx)

 while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
 cur = >req->items[fwctx->curpos];
-    request_firmware(, cur->path, fwctx->dev);
+    brcmf_fw_request_firmware(, fwctx);
 ret = brcmf_fw_complete_request(fw, ctx);
 }

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
index 2893e56910f0..a0834be8864e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
@@ -70,6 +70,7 @@ struct brcmf_fw_request {
 u16 domain_nr;
 u16 bus_nr;
 u32 n_items;
+    const char *board_type;
 struct brcmf_fw_item items[0];
 };

Re: [PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines

[Hans de Goede]

Hi,

On 10-10-18 10:15, Arend van Spriel wrote:

On 10/10/2018 9:59 AM, Hans de Goede wrote:



Any chance you could do a patch-review of this series?


Yup and test for regressions with some of the chipsets I have here.


Have you gotten around to review and testing this series yet?
it would be nice to get this upstream.

Also a review of the 2 patch series starting with:
"brcmfmac: Add support for getting nvram contents from EFI variables"
would be appreciated.

Regards,

Hans

[PATCH v4 2/2] brcmfmac: Fix ccode from EFI nvram when necessary

[Hans de Goede]

In some cases the EFI-var stored nvram contains "ccode=ALL" or "ccode=XV"
to specify "worldwide" compatible settings, but these 2 ccode-s do not work
properly.

I've tested the different known "worldwide" ccode-s used in various nvram
sources with the latest firmwares from linux-firmware for various brcmfmac
models, here is a simplified (*) table with what each setting results in:

ALL: 12-14 disab, U-NII-1, U-NII-2 no-IR/radar, U-NII-3
XV:  12-14 no-IR, disables all 5G channels
XY:  12-13 enab, 14 disab, U-NII-1 enab, U-NII-2 no-IR/radar, U-NII-3 disab
X2:  12-13 no-IR, 14 dis, U-NII-1 no-IR, U-NII-2 no-IR/radar, U-NII-3 no-IR

Where 12,13,14 are 2.4G channels 12-14 and U-NII-1/2/3 are the 3 different
5G channel groups. no-IR is no-Initiate-Radiation, we will never send on
these channels without first having received valid wifi traffic there.

This immediately shows that both ALL and XV are not as worldwide as we want
them to be. ALL causes channels 12 and 13 to not be available and XV causes
all 5GHz channels to not be available. Also ALL unconditionally enables the
U-NII-1 and U-NII-3 5G groups, while we really should be using no-IR for
these.

This commit replace XV and ALL with X2, which allows usage of chan 12-13
and 5G channels, but only after receiving valid wifi traffic there first.

Note that this configure the firmware's channel limits, the kernels own
regulatory restrictions based on e.g. regulatory info received from the
access-point, will be applied on top of this.

This fixes channels 12+13 not working on the Asus T200TA and the Lenovo
Mixx 2 8 and 5G channels not working on the Asus T100HA.

This has been tested on the following models: Acer Iconia Tab8 w1-810,
Acer One 10, Asus T100CHI, Asus T100HA, Asus T100TA, Asus T200TA and a
Lenovo Mixx 2 8.

*) There are some exceptions to this table:
1) On really old firmware e.g. linux-firmware's 2011 brcmfmac4330-sdio.bin
   ALL really means all, unconditionally enabling everything
2) The exact meaning might be influenced by setting the regrev nvram var.
   Specifically using ccode=XV + regrev=1 on brcmfmac43241b4 leads to:
   12-14 no-ir, U-NII-1 no-ir, U-NII-2 no-ir/radar, U-NII-3 no-ir
   But only on the brcmfmac43241b4 and not on e.g. the brcmfmac43340

Tested-by: Hans de Goede 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Rebase for changes to "brcmfmac: Add support for getting nvram contents
 from EFI variables" patch

Changes in v3:
-New patch in v3 of this patch-set
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 24 +++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 965ae5c24c2d..72d8c0c3c3a1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -447,6 +447,29 @@ struct brcmf_fw {
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx);
 
 #ifdef CONFIG_EFI
+/* In some cases the EFI-var stored nvram contains "ccode=ALL" or "ccode=XV"
+ * to specify "worldwide" compatible settings, but these 2 ccode-s do not work
+ * properly. "ccode=ALL" causes channels 12 and 13 to not be available,
+ * "ccode=XV" causes all 5GHz channels to not be available. So we replace both
+ * with "ccode=X2" which allows channels 12+13 and 5Ghz channels in
+ * no-Initiate-Radiation mode. This means that we will never send on these
+ * channels without first having received valid wifi traffic on the channel.
+ */
+static void brcmf_fw_fix_efi_nvram_ccode(char *data, unsigned long data_len)
+{
+   char *ccode;
+
+   ccode = strnstr((char *)data, "ccode=ALL", data_len);
+   if (!ccode)
+   ccode = strnstr((char *)data, "ccode=XV\r", data_len);
+   if (!ccode)
+   return;
+
+   ccode[6] = 'X';
+   ccode[7] = '2';
+   ccode[8] = '\r';
+}
+
 static u8 *brcmf_fw_nvram_from_efi(size_t *data_len_ret)
 {
const u16 name[] = { 'n', 'v', 'r', 'a', 'm', 0 };
@@ -476,6 +499,7 @@ static u8 *brcmf_fw_nvram_from_efi(size_t *data_len_ret)
if (err)
goto fail;
 
+   brcmf_fw_fix_efi_nvram_ccode(data, data_len);
brcmf_info("Using nvram EFI variable\n");
 
kfree(nvram_efivar);
-- 
2.19.0

[PATCH v4 1/2] brcmfmac: Add support for getting nvram contents from EFI variables

[Hans de Goede]

Various X86 laptops with a SDIO attached brcmfmac wifi chip, store the
nvram contents in a special EFI variable. This commit adds support for
getting nvram directly from this EFI variable, without the user needing
to manually copy it.

This makes Wifi / Bluetooth work out of the box on these devices instead of
requiring manual setup.

This has been tested on the following models: Acer Iconia Tab8 w1-810,
Acer One 10, Asus T100CHI, Asus T100HA, Asus T100TA, Asus T200TA and a
Lenovo Mixx 2 8.

Tested-by: Hans de Goede 
Signed-off-by: Hans de Goede 
---
Changes in v4:
-Drop unused path argument from brcmf_fw_nvram_from_efi(), this makes its
 prototype the same again as the empty implementation which gets used when
 CONFIG_EFI is not defined, fixing compilation without CONFIG_EFI

Changes in v3:
-Drop ccode fixup code (to be re-added in a separate commit), so that we
 can get the main EFI nvram support merged while we figure out the ccode
 handling

Changes in v2:
-Stop testing for asus in the dmi sys_vendor string at least the Toshiba
 Encore uses the nvram efivar variable too
-Use a table mapping the firmare name to the correct ccode value (XV / X2) to
 use for the worldwide regulatory domain for that firmware, assuming the
 firmware from linux-firmware is used. This fixes the T200TA and T100HA not
 seeing 5GHz networks
-Not only fixup ccode=ALL, but also ccode=XV, this is necessary since the
 T100HA nvram efivar containts ccode=XV, but the firmware from Linux firmware
 needs ccode=X2 for proper operation
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 63 +--
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index b38c4b40b235..965ae5c24c2d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -14,6 +14,7 @@
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -445,6 +446,51 @@ struct brcmf_fw {
 
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx);
 
+#ifdef CONFIG_EFI
+static u8 *brcmf_fw_nvram_from_efi(size_t *data_len_ret)
+{
+   const u16 name[] = { 'n', 'v', 'r', 'a', 'm', 0 };
+   struct efivar_entry *nvram_efivar;
+   unsigned long data_len = 0;
+   u8 *data = NULL;
+   int err;
+
+   nvram_efivar = kzalloc(sizeof(*nvram_efivar), GFP_KERNEL);
+   if (!nvram_efivar)
+   return NULL;
+
+   memcpy(_efivar->var.VariableName, name, sizeof(name));
+   nvram_efivar->var.VendorGuid = EFI_GUID(0x74b00bd9, 0x805a, 0x4d61,
+   0xb5, 0x1f, 0x43, 0x26,
+   0x81, 0x23, 0xd1, 0x13);
+
+   err = efivar_entry_size(nvram_efivar, _len);
+   if (err)
+   goto fail;
+
+   data = kmalloc(data_len, GFP_KERNEL);
+   if (!data)
+   goto fail;
+
+   err = efivar_entry_get(nvram_efivar, NULL, _len, data);
+   if (err)
+   goto fail;
+
+   brcmf_info("Using nvram EFI variable\n");
+
+   kfree(nvram_efivar);
+   *data_len_ret = data_len;
+   return data;
+
+fail:
+   kfree(data);
+   kfree(nvram_efivar);
+   return NULL;
+}
+#else
+static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
+#endif
+
 static void brcmf_fw_free_request(struct brcmf_fw_request *req)
 {
struct brcmf_fw_item *item;
@@ -463,11 +509,12 @@ static int brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
struct brcmf_fw_item *cur;
+   bool free_bcm47xx_nvram = false;
+   bool kfree_nvram = false;
u32 nvram_length = 0;
void *nvram = NULL;
u8 *data = NULL;
size_t data_len;
-   bool raw_nvram;
 
brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
 
@@ -476,12 +523,13 @@ static int brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
if (fw && fw->data) {
data = (u8 *)fw->data;
data_len = fw->size;
-   raw_nvram = false;
} else {
-   data = bcm47xx_nvram_get_contents(_len);
-   if (!data && !(cur->flags & BRCMF_FW_REQF_OPTIONAL))
+   if ((data = bcm47xx_nvram_get_contents(_len)))
+   free_bcm47xx_nvram = true;
+   else if ((data = brcmf_fw_nvram_from_efi(_len)))
+   kfree_nvram = true;
+   else if (!(cur->flags & BRCMF_FW_REQF_OPTIONAL))
goto fail;
-   raw_nvram = true;
}
 
if (data)
@@ -489,8 +537,11 @@ static int brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx

[PATCH v2 4/6] brcmfmac: Set board_type used for nvram file selection to machine-compatible

[Hans de Goede]

For of/devicetree using machines, set the board_type used for nvram file
selection to the first string listed in the top-level's node compatible
string, aka the machine-compatible as used by of_machine_is_compatible().

The board_type setting is used to load the board-specific nvram file with
a board-specific name so that we can ship files for each supported board
in linux-firmware.

Signed-off-by: Hans de Goede 
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/common.h |  1 +
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 11 ++-
 .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c   |  1 +
 .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c   |  1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index a34642cb4d2f..e63a273642e9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -59,6 +59,7 @@ struct brcmf_mp_device {
booliapp;
boolignore_probe_fail;
struct brcmfmac_pd_cc *country_codes;
+   const char  *board_type;
union {
struct brcmfmac_sdio_pd sdio;
} bus;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index aee6e5937c41..84e3373289eb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -27,11 +27,20 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type 
bus_type,
struct brcmf_mp_device *settings)
 {
struct brcmfmac_sdio_pd *sdio = >bus.sdio;
-   struct device_node *np = dev->of_node;
+   struct device_node *root, *np = dev->of_node;
+   struct property *prop;
int irq;
u32 irqf;
u32 val;
 
+   /* Set board-type to the first string of the machine compatible prop */
+   root = of_find_node_by_path("/");
+   if (root) {
+   prop = of_find_property(root, "compatible", NULL);
+   settings->board_type = of_prop_next_string(prop, NULL);
+   of_node_put(root);
+   }
+
if (!np || bus_type != BRCMF_BUSTYPE_SDIO ||
!of_device_is_compatible(np, "brcm,bcm4329-fmac"))
return;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 4fffa6988087..b12f3e0ee69c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1785,6 +1785,7 @@ 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->board_type = devinfo->settings->board_type;
/* 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;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index a907d7b065fa..3d117563 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4177,6 +4177,7 @@ brcmf_sdio_prepare_fw_request(struct brcmf_sdio *bus)
 
fwreq->items[BRCMF_SDIO_FW_CODE].type = BRCMF_FW_TYPE_BINARY;
fwreq->items[BRCMF_SDIO_FW_NVRAM].type = BRCMF_FW_TYPE_NVRAM;
+   fwreq->board_type = bus->sdiodev->settings->board_type;
 
return fwreq;
 }
-- 
2.19.0

[PATCH v2 1/6] brcmfmac: Remove firmware-loading code duplication

[Hans de Goede]

brcmf_fw_request_next_item and brcmf_fw_request_done both have identical
code to complete the fw-request depending on the item-type.

This commit adds a new brcmf_fw_complete_request helper removing this code
duplication.

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 62 +--
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 9095b830ae4d..784c84f0e9e7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -504,6 +504,34 @@ static int brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
return -ENOENT;
 }
 
+static int brcmf_fw_complete_request(const struct firmware *fw,
+struct brcmf_fw *fwctx)
+{
+   struct brcmf_fw_item *cur = >req->items[fwctx->curpos];
+   int ret = 0;
+
+   brcmf_dbg(TRACE, "firmware %s %sfound\n", cur->path, fw ? "" : "not ");
+
+   switch (cur->type) {
+   case BRCMF_FW_TYPE_NVRAM:
+   ret = brcmf_fw_request_nvram_done(fw, fwctx);
+   break;
+   case BRCMF_FW_TYPE_BINARY:
+   if (fw)
+   cur->binary = fw;
+   else
+   ret = -ENOENT;
+   break;
+   default:
+   /* something fishy here so bail out early */
+   brcmf_err("unknown fw type: %d\n", cur->type);
+   release_firmware(fw);
+   ret = -EINVAL;
+   }
+
+   return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
+}
+
 static int brcmf_fw_request_next_item(struct brcmf_fw *fwctx, bool async)
 {
struct brcmf_fw_item *cur;
@@ -525,15 +553,7 @@ static int brcmf_fw_request_next_item(struct brcmf_fw 
*fwctx, bool async)
if (ret < 0) {
brcmf_fw_request_done(NULL, fwctx);
} else if (!async && fw) {
-   brcmf_dbg(TRACE, "firmware %s %sfound\n", cur->path,
- fw ? "" : "not ");
-   if (cur->type == BRCMF_FW_TYPE_BINARY)
-   cur->binary = fw;
-   else if (cur->type == BRCMF_FW_TYPE_NVRAM)
-   brcmf_fw_request_nvram_done(fw, fwctx);
-   else
-   release_firmware(fw);
-
+   brcmf_fw_complete_request(fw, fwctx);
return -EAGAIN;
}
return 0;
@@ -547,28 +567,8 @@ static void brcmf_fw_request_done(const struct firmware 
*fw, void *ctx)
 
cur = >req->items[fwctx->curpos];
 
-   brcmf_dbg(TRACE, "enter: firmware %s %sfound\n", cur->path,
- fw ? "" : "not ");
-
-   if (!fw)
-   ret = -ENOENT;
-
-   switch (cur->type) {
-   case BRCMF_FW_TYPE_NVRAM:
-   ret = brcmf_fw_request_nvram_done(fw, fwctx);
-   break;
-   case BRCMF_FW_TYPE_BINARY:
-   cur->binary = fw;
-   break;
-   default:
-   /* something fishy here so bail out early */
-   brcmf_err("unknown fw type: %d\n", cur->type);
-   release_firmware(fw);
-   ret = -EINVAL;
-   goto fail;
-   }
-
-   if (ret < 0 && !(cur->flags & BRCMF_FW_REQF_OPTIONAL))
+   ret = brcmf_fw_complete_request(fw, fwctx);
+   if (ret < 0)
goto fail;
 
do {
-- 
2.19.0

[PATCH v2 3/6] brcmfmac: Add support for first trying to get a board specific nvram file

[Hans de Goede]

The nvram files which some brcmfmac chips need are board-specific. To be
able to distribute these as part of linux-firmware, so that devices with
such a wifi chip will work OOTB, multiple (one per board) versions must
co-exist under /lib/firmware.

This commit adds support for callers of the brcmfmac/firmware.c code to
pass in a board_type parameter through the request structure.

If that parameter is set then the code will first try to load
chipmodel.board_type.txt before falling back to the old chipmodel.txt name.

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 27 ++-
 .../broadcom/brcm80211/brcmfmac/firmware.h|  1 +
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 08aaf99fee34..6755b2388fbc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -532,6 +532,31 @@ static int brcmf_fw_complete_request(const struct firmware 
*fw,
return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
 }
 
+static int brcmf_fw_request_firmware(const struct firmware **fw,
+struct brcmf_fw *fwctx)
+{
+   struct brcmf_fw_item *cur = >req->items[fwctx->curpos];
+   int ret;
+
+   /* nvram files are board-specific, first try a board-specific path */
+   if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
+   char alt_path[BRCMF_FW_NAME_LEN];
+
+   strlcpy(alt_path, cur->path, BRCMF_FW_NAME_LEN);
+   /* strip .txt at the end */
+   alt_path[strlen(alt_path) - 4] = 0;
+   strlcat(alt_path, ".", BRCMF_FW_NAME_LEN);
+   strlcat(alt_path, fwctx->req->board_type, BRCMF_FW_NAME_LEN);
+   strlcat(alt_path, ".txt", BRCMF_FW_NAME_LEN);
+
+   ret = request_firmware(fw, alt_path, fwctx->dev);
+   if (ret == 0)
+   return ret;
+   }
+
+   return request_firmware(fw, cur->path, fwctx->dev);
+}
+
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
@@ -544,7 +569,7 @@ static void brcmf_fw_request_done(const struct firmware 
*fw, void *ctx)
 
while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
cur = >req->items[fwctx->curpos];
-   request_firmware(, cur->path, fwctx->dev);
+   brcmf_fw_request_firmware(, fwctx);
ret = brcmf_fw_complete_request(fw, ctx);
}
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
index 2893e56910f0..a0834be8864e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
@@ -70,6 +70,7 @@ struct brcmf_fw_request {
u16 domain_nr;
u16 bus_nr;
u32 n_items;
+   const char *board_type;
struct brcmf_fw_item items[0];
 };
 
-- 
2.19.0

[PATCH v2 2/6] brcmfmac: Remove recursion from firmware load error handling

[Hans de Goede]

Before this commit brcmf_fw_request_done would call
brcmf_fw_request_next_item to load the next item, which on an error would
call brcmf_fw_request_done, which if the error is recoverable (*) will
then continue calling brcmf_fw_request_next_item for the next item again
which on an error will call brcmf_fw_request_done again...

This does not blow up because we only have a limited number of items so
we never recurse too deep. But the recursion is still quite ugly and
frankly is giving me a headache, so lets fix this.

This commit fixes this by removing brcmf_fw_request_next_item and by
making brcmf_fw_get_firmwares and brcmf_fw_request_done directly call
firmware_request_nowait resp. firmware_request themselves.

*) brcmf_fw_request_nvram_done fallback path succeeds or
   BRCMF_FW_REQF_OPTIONAL is set

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 65 ++-
 1 file changed, 19 insertions(+), 46 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 784c84f0e9e7..08aaf99fee34 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -532,33 +532,6 @@ static int brcmf_fw_complete_request(const struct firmware 
*fw,
return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
 }
 
-static int brcmf_fw_request_next_item(struct brcmf_fw *fwctx, bool async)
-{
-   struct brcmf_fw_item *cur;
-   const struct firmware *fw = NULL;
-   int ret;
-
-   cur = >req->items[fwctx->curpos];
-
-   brcmf_dbg(TRACE, "%srequest for %s\n", async ? "async " : "",
- cur->path);
-
-   if (async)
-   ret = request_firmware_nowait(THIS_MODULE, true, cur->path,
- fwctx->dev, GFP_KERNEL, fwctx,
- brcmf_fw_request_done);
-   else
-   ret = request_firmware(, cur->path, fwctx->dev);
-
-   if (ret < 0) {
-   brcmf_fw_request_done(NULL, fwctx);
-   } else if (!async && fw) {
-   brcmf_fw_complete_request(fw, fwctx);
-   return -EAGAIN;
-   }
-   return 0;
-}
-
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
@@ -568,26 +541,19 @@ static void brcmf_fw_request_done(const struct firmware 
*fw, void *ctx)
cur = >req->items[fwctx->curpos];
 
ret = brcmf_fw_complete_request(fw, fwctx);
-   if (ret < 0)
-   goto fail;
-
-   do {
-   if (++fwctx->curpos == fwctx->req->n_items) {
-   ret = 0;
-   goto done;
-   }
 
-   ret = brcmf_fw_request_next_item(fwctx, false);
-   } while (ret == -EAGAIN);
-
-   return;
+   while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
+   cur = >req->items[fwctx->curpos];
+   request_firmware(, cur->path, fwctx->dev);
+   ret = brcmf_fw_complete_request(fw, ctx);
+   }
 
-fail:
-   brcmf_dbg(TRACE, "failed err=%d: dev=%s, fw=%s\n", ret,
- dev_name(fwctx->dev), cur->path);
-   brcmf_fw_free_request(fwctx->req);
-   fwctx->req = NULL;
-done:
+   if (ret) {
+   brcmf_dbg(TRACE, "failed err=%d: dev=%s, fw=%s\n", ret,
+ dev_name(fwctx->dev), cur->path);
+   brcmf_fw_free_request(fwctx->req);
+   fwctx->req = NULL;
+   }
fwctx->done(fwctx->dev, ret, fwctx->req);
kfree(fwctx);
 }
@@ -611,7 +577,9 @@ int brcmf_fw_get_firmwares(struct device *dev, struct 
brcmf_fw_request *req,
   void (*fw_cb)(struct device *dev, int err,
 struct brcmf_fw_request *req))
 {
+   struct brcmf_fw_item *first = >items[0];
struct brcmf_fw *fwctx;
+   int ret;
 
brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(dev));
if (!fw_cb)
@@ -628,7 +596,12 @@ int brcmf_fw_get_firmwares(struct device *dev, struct 
brcmf_fw_request *req,
fwctx->req = req;
fwctx->done = fw_cb;
 
-   brcmf_fw_request_next_item(fwctx, true);
+   ret = request_firmware_nowait(THIS_MODULE, true, first->path,
+ fwctx->dev, GFP_KERNEL, fwctx,
+ brcmf_fw_request_done);
+   if (ret < 0)
+   brcmf_fw_request_done(NULL, fwctx);
+
return 0;
 }
 
-- 
2.19.0

[PATCH v2 6/6] brcmfmac: Cleanup brcmf_fw_request_done()

[Hans de Goede]

The "cur" variable is now only used for a debug print and we already
print the same info from brcmf_fw_complete_request(), so the debug print
does not provide any extra info and we can remove it.

Signed-off-by: Hans de Goede 
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/firmware.c   | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 6755b2388fbc..b38c4b40b235 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -560,22 +560,16 @@ static int brcmf_fw_request_firmware(const struct 
firmware **fw,
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
-   struct brcmf_fw_item *cur;
-   int ret = 0;
-
-   cur = >req->items[fwctx->curpos];
+   int ret;
 
ret = brcmf_fw_complete_request(fw, fwctx);
 
while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
-   cur = >req->items[fwctx->curpos];
brcmf_fw_request_firmware(, fwctx);
ret = brcmf_fw_complete_request(fw, ctx);
}
 
if (ret) {
-   brcmf_dbg(TRACE, "failed err=%d: dev=%s, fw=%s\n", ret,
- dev_name(fwctx->dev), cur->path);
brcmf_fw_free_request(fwctx->req);
fwctx->req = NULL;
}
-- 
2.19.0

[PATCH v2 5/6] brcmfmac: Set board_type from DMI on x86 based machines

[Hans de Goede]

For x86 based machines, set the board_type used for nvram file selection
based on the DMI sys-vendor and product-name strings.

Since on some models these strings are too generic, this commit also adds
a quirk table overriding the strings for models listed in that table.

The board_type setting is used to load the board-specific nvram file with
a board-specific name so that we can ship files for each supported board
in linux-firmware.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Use full ISC text for now instead of SPDX tag, because the ISC is not
 yet listed under LICENSES
---
 .../broadcom/brcm80211/brcmfmac/Makefile  |   2 +
 .../broadcom/brcm80211/brcmfmac/common.c  |   3 +-
 .../broadcom/brcm80211/brcmfmac/common.h  |   7 ++
 .../broadcom/brcm80211/brcmfmac/dmi.c | 116 ++
 4 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
index 1f5a9b948abf..22fd95a736a8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
@@ -54,3 +54,5 @@ brcmfmac-$(CONFIG_BRCM_TRACING) += \
tracepoint.o
 brcmfmac-$(CONFIG_OF) += \
of.o
+brcmfmac-$(CONFIG_DMI) += \
+   dmi.o
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index cd3651069d0c..a4bcbd1a57ac 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -450,8 +450,9 @@ struct brcmf_mp_device *brcmf_get_module_param(struct 
device *dev,
}
}
if (!found) {
-   /* No platform data for this device, try OF (Open Firwmare) */
+   /* No platform data for this device, try OF and DMI data */
brcmf_of_probe(dev, bus_type, settings);
+   brcmf_dmi_probe(settings, chip, chiprev);
}
return settings;
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index e63a273642e9..4ce56be90b74 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -75,4 +75,11 @@ void brcmf_release_module_param(struct brcmf_mp_device 
*module_param);
 /* Sets dongle media info (drv_version, mac address). */
 int brcmf_c_preinit_dcmds(struct brcmf_if *ifp);
 
+#ifdef CONFIG_DMI
+void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev);
+#else
+static inline void
+brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev) {}
+#endif
+
 #endif /* BRCMFMAC_COMMON_H */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
new file mode 100644
index ..51d76ac45075
--- /dev/null
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
@@ -0,0 +1,116 @@
+/*
+ * Copyright 2018 Hans de Goede 
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+#include "core.h"
+#include "common.h"
+#include "brcm_hw_ids.h"
+
+/* The DMI data never changes so we can use a static buf for this */
+static char dmi_board_type[128];
+
+struct brcmf_dmi_data {
+   u32 chip;
+   u32 chiprev;
+   const char *board_type;
+};
+
+/* NOTE: Please keep all entries sorted alphabetically */
+
+static const struct brcmf_dmi_data gpd_win_pocket_data = {
+   BRCM_CC_4356_CHIP_ID, 2, "gpd-win-pocket"
+};
+
+static const struct brcmf_dmi_data jumper_ezpad_mini3_data = {
+   BRCM_CC_43430_CHIP_ID, 0, "jumper-ezpad-mini3"
+};
+
+static const struct brcmf_dmi_data meegopad_t08_data = {
+   BRCM_CC_43340_CHIP_ID, 2, "meegopad-t08"
+};
+
+static const struct dmi_system_id dmi_platform_data[] = {
+   {
+   /* Match for the GPDwin which unfortunately uses somewhat
+* generic dmi strings, which is why we test for 4

[PATCH] brcmfmac: Fix ccode from EFI nvram when necessary

[Hans de Goede]

In some cases the EFI-var stored nvram contains "ccode=ALL" or "ccode=XV"
to specify "worldwide" compatible settings, but these 2 ccode-s do not work
properly.

I've tested the different known "worldwide" ccode-s used in various nvram
sources with the latest firmwares from linux-firmware for various brcmfmac
models, here is a simplified (*) table with what each setting results in:

ALL: 12-14 disab, U-NII-1, U-NII-2 no-IR/radar, U-NII-3
XV:  12-14 no-IR, disables all 5G channels
XY:  12-13 enab, 14 disab, U-NII-1 enab, U-NII-2 no-IR/radar, U-NII-3 disab
X2:  12-13 no-IR, 14 dis, U-NII-1 no-IR, U-NII-2 no-IR/radar, U-NII-3 no-IR

Where 12,13,14 are 2.4G channels 12-14 and U-NII-1/2/3 are the 3 different
5G channel groups. no-IR is no-Initiate-Radiation, we will never send on
these channels without first having received valid wifi traffic there.

This immediately shows that both ALL and XV are not as worldwide as we want
them to be. ALL causes channels 12 and 13 to not be available and XV causes
all 5GHz channels to not be available. Also ALL unconditionally enables the
U-NII-1 and U-NII-3 5G groups, while we really should be using no-IR for
these.

This commit replace XV and ALL with X2, which allows usage of chan 12-13
and 5G channels, but only after receiving valid wifi traffic there first.

Note that this configure the firmware's channel limits, the kernels own
regulatory restrictions based on e.g. regulatory info received from the
access-point, will be applied on top of this.

This fixes channels 12+13 not working on the Asus T200TA and the Lenovo
Mixx 2 8 and 5G channels not working on the Asus T100HA.

This has been tested on the following models: Acer Iconia Tab8 w1-810,
Acer One 10, Asus T100CHI, Asus T100HA, Asus T100TA, Asus T200TA and a
Lenovo Mixx 2 8.

*) There are some exceptions to this table:
1) On really old firmware e.g. linux-firmware's 2011 brcmfmac4330-sdio.bin
   ALL really means all, unconditionally enabling everything
2) The exact meaning might be influenced by setting the regrev nvram var.
   Specifically using ccode=XV + regrev=1 on brcmfmac43241b4 leads to:
   12-14 no-ir, U-NII-1 no-ir, U-NII-2 no-ir/radar, U-NII-3 no-ir
   But only on the brcmfmac43241b4 and not on e.g. the brcmfmac43340

Tested-by: Hans de Goede 
Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 24 +++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index a1dfe75c835f..d1db8874aaa9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -447,6 +447,29 @@ struct brcmf_fw {
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx);
 
 #ifdef CONFIG_EFI
+/* In some cases the EFI-var stored nvram contains "ccode=ALL" or "ccode=XV"
+ * to specify "worldwide" compatible settings, but these 2 ccode-s do not work
+ * properly. "ccode=ALL" causes channels 12 and 13 to not be available,
+ * "ccode=XV" causes all 5GHz channels to not be available. So we replace both
+ * with "ccode=X2" which allows channels 12+13 and 5Ghz channels in
+ * no-Initiate-Radiation mode. This means that we will never send on these
+ * channels without first having received valid wifi traffic on the channel.
+ */
+static void brcmf_fw_fix_efi_nvram_ccode(char *data, unsigned long data_len)
+{
+   char *ccode;
+
+   ccode = strnstr((char *)data, "ccode=ALL", data_len);
+   if (!ccode)
+   ccode = strnstr((char *)data, "ccode=XV\r", data_len);
+   if (!ccode)
+   return;
+
+   ccode[6] = 'X';
+   ccode[7] = '2';
+   ccode[8] = '\r';
+}
+
 static u8 *brcmf_fw_nvram_from_efi(const char *path, size_t *data_len_ret)
 {
const u16 name[] = { 'n', 'v', 'r', 'a', 'm', 0 };
@@ -476,6 +499,7 @@ static u8 *brcmf_fw_nvram_from_efi(const char *path, size_t 
*data_len_ret)
if (err)
goto fail;
 
+   brcmf_fw_fix_efi_nvram_ccode(data, data_len);
brcmf_info("Using nvram EFI variable\n");
 
kfree(nvram_efivar);
-- 
2.19.0

Re: [PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines

[Hans de Goede]

Hi,

On 10-10-18 09:57, Kalle Valo wrote:

Arend van Spriel  writes:


On 10/10/2018 9:28 AM, Hans de Goede wrote:

So how do you want to proceed with this, do you want me to just
put the full ISC text in the header for now as the rest of brcmfmac
does?


This is not entirely true as far as I know. I assume you are referring
to this:

/*
  * Copyright (c) 2010 Broadcom Corporation
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
  * copyright notice and this permission notice appear in all copies.
  *
  * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
  * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
  * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE
FOR ANY
  * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
  * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
AN ACTION
  * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */

As far as I recall we opted for BSD license and ISC is equivalent.


Oh, sorry for missing that.


However, The BSD license are already in place so why not use that. I
would say BSD-2-Clause should cover it. As this is a new file I guess
it is up to you although I would prefer to stick with a permissive
license.


 From maintainer's point of view I very much prefer that a driver uses
the same license in all files. It's very confusing if the license
changes within different files.


Ack, I will just copy over the header from other brcmfmac files for v2
of the series. Which FWIW is a 1:1 copy of:

https://spdx.org/licenses/ISC

(I checked before setting the SPDX tag)

Regards,

Hans

Re: [PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines

[Hans de Goede]

Hi Arend,

On 10-10-18 09:52, Arend van Spriel wrote:

On 10/10/2018 9:28 AM, Hans de Goede wrote:

So how do you want to proceed with this, do you want me to just
put the full ISC text in the header for now as the rest of brcmfmac
does?


This is not entirely true as far as I know. I assume you are referring to this:

/*
  * Copyright (c) 2010 Broadcom Corporation
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
  * copyright notice and this permission notice appear in all copies.
  *
  * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
  * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
  * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
  * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
  * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
  * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */

As far as I recall we opted for BSD license and ISC is equivalent.


I believe it is the other way around, you opted for the ISC license
which is more or less equivalent to the 2 clause BSD, see:

https://spdx.org/licenses/BSD-2-Clause.html
https://spdx.org/licenses/ISC

The ISC text is a 1:1 match to the license used in brcmfmac, and it seems
sensible to me to be consistent and use the same license for all
brcmfmac files even if the 2 are more or less equivalent.


However, The BSD license are already in place so why not use that. I would say 
BSD-2-Clause should cover it. As this is a new file I guess it is up to you 
although I would prefer to stick with a permissive license.


I've no problem with a permissive license, I will just stick with
the ISC / same header as the rest of brcmfmac for consistency.

Regards,

Hans

p.s.

Any chance you could do a patch-review of this series?

Re: [PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines

[Hans de Goede]

Hi,

On 10-10-18 09:09, Kalle Valo wrote:

Hans de Goede  writes:


For x86 based machines, set the board_type used for nvram file selection
based on the DMI sys-vendor and product-name strings.

Since on some models these strings are too generic, this commit also adds
a quirk table overriding the strings for models listed in that table.

The board_type setting is used to load the board-specific nvram file with
a board-specific name so that we can ship files for each supported board
in linux-firmware.

Signed-off-by: Hans de Goede 


[...]


+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: ISC


I don't see the ISC file in LICENSES directory[1] and I don't feel
comfortable taking SPDX tags which which don't have a license file.


Ok. I need to do a patch for the LICENSES directory anyways, so I will
also submit the ISC license upstream while at it, I will hopefully
get around to doing that today.

So how do you want to proceed with this, do you want me to just
put the full ISC text in the header for now as the rest of brcmfmac
does?

Then later someone (me if I get around to it) can replace all of
the headers with // SPDX-License-Identifier: ISC once it has been
added under LICENSES.

Regards,

Hans

[PATCH v3] brcmfmac: Add support for getting nvram contents from EFI variables

[Hans de Goede]

Various X86 laptops with a SDIO attached brcmfmac wifi chip, store the
nvram contents in a special EFI variable. This commit adds support for
getting nvram directly from this EFI variable, without the user needing
to manually copy it.

This makes Wifi / Bluetooth work out of the box on these devices instead of
requiring manual setup.

This has been tested on the following models: Acer Iconia Tab8 w1-810,
Acer One 10, Asus T100CHI, Asus T100HA, Asus T100TA, Asus T200TA and a
Lenovo Mixx 2 8.

Tested-by: Hans de Goede 
Signed-off-by: Hans de Goede 
---
Changes in v3:
-Drop ccode fixup code (to be re-added in a separate commit), so that we
 can get the main EFI nvram support merged while we figure out the ccode
 handling

Changes in v2:
-Stop testing for asus in the dmi sys_vendor string at least the Toshiba
 Encore uses the nvram efivar variable too
-Use a table mapping the firmare name to the correct ccode value (XV / X2) to
 use for the worldwide regulatory domain for that firmware, assuming the
 firmware from linux-firmware is used. This fixes the T200TA and T100HA not
 seeing 5GHz networks
-Not only fixup ccode=ALL, but also ccode=XV, this is necessary since the
 T100HA nvram efivar containts ccode=XV, but the firmware from Linux firmware
 needs ccode=X2 for proper operation
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 63 +--
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index b38c4b40b235..a1dfe75c835f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -14,6 +14,7 @@
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -445,6 +446,51 @@ struct brcmf_fw {
 
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx);
 
+#ifdef CONFIG_EFI
+static u8 *brcmf_fw_nvram_from_efi(const char *path, size_t *data_len_ret)
+{
+   const u16 name[] = { 'n', 'v', 'r', 'a', 'm', 0 };
+   struct efivar_entry *nvram_efivar;
+   unsigned long data_len = 0;
+   u8 *data = NULL;
+   int err;
+
+   nvram_efivar = kzalloc(sizeof(*nvram_efivar), GFP_KERNEL);
+   if (!nvram_efivar)
+   return NULL;
+
+   memcpy(_efivar->var.VariableName, name, sizeof(name));
+   nvram_efivar->var.VendorGuid = EFI_GUID(0x74b00bd9, 0x805a, 0x4d61,
+   0xb5, 0x1f, 0x43, 0x26,
+   0x81, 0x23, 0xd1, 0x13);
+
+   err = efivar_entry_size(nvram_efivar, _len);
+   if (err)
+   goto fail;
+
+   data = kmalloc(data_len, GFP_KERNEL);
+   if (!data)
+   goto fail;
+
+   err = efivar_entry_get(nvram_efivar, NULL, _len, data);
+   if (err)
+   goto fail;
+
+   brcmf_info("Using nvram EFI variable\n");
+
+   kfree(nvram_efivar);
+   *data_len_ret = data_len;
+   return data;
+
+fail:
+   kfree(data);
+   kfree(nvram_efivar);
+   return NULL;
+}
+#else
+static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
+#endif
+
 static void brcmf_fw_free_request(struct brcmf_fw_request *req)
 {
struct brcmf_fw_item *item;
@@ -463,11 +509,12 @@ static int brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
struct brcmf_fw_item *cur;
+   bool free_bcm47xx_nvram = false;
+   bool kfree_nvram = false;
u32 nvram_length = 0;
void *nvram = NULL;
u8 *data = NULL;
size_t data_len;
-   bool raw_nvram;
 
brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
 
@@ -476,12 +523,13 @@ static int brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
if (fw && fw->data) {
data = (u8 *)fw->data;
data_len = fw->size;
-   raw_nvram = false;
} else {
-   data = bcm47xx_nvram_get_contents(_len);
-   if (!data && !(cur->flags & BRCMF_FW_REQF_OPTIONAL))
+   if ((data = bcm47xx_nvram_get_contents(_len)))
+   free_bcm47xx_nvram = true;
+   else if ((data = brcmf_fw_nvram_from_efi(cur->path, _len)))
+   kfree_nvram = true;
+   else if (!(cur->flags & BRCMF_FW_REQF_OPTIONAL))
goto fail;
-   raw_nvram = true;
}
 
if (data)
@@ -489,8 +537,11 @@ static int brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
 fwctx->req->domain_nr,
 fwctx->req->bus_nr);
 
-   if (raw_nvram)
+   if (free_bcm47xx_n

[PATCH 1/6] brcmfmac: Remove firmware-loading code duplication

[Hans de Goede]

brcmf_fw_request_next_item and brcmf_fw_request_done both have identical
code to complete the fw-request depending on the item-type.

This commit adds a new brcmf_fw_complete_request helper removing this code
duplication.

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 62 +--
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 9095b830ae4d..784c84f0e9e7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -504,6 +504,34 @@ static int brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
return -ENOENT;
 }
 
+static int brcmf_fw_complete_request(const struct firmware *fw,
+struct brcmf_fw *fwctx)
+{
+   struct brcmf_fw_item *cur = >req->items[fwctx->curpos];
+   int ret = 0;
+
+   brcmf_dbg(TRACE, "firmware %s %sfound\n", cur->path, fw ? "" : "not ");
+
+   switch (cur->type) {
+   case BRCMF_FW_TYPE_NVRAM:
+   ret = brcmf_fw_request_nvram_done(fw, fwctx);
+   break;
+   case BRCMF_FW_TYPE_BINARY:
+   if (fw)
+   cur->binary = fw;
+   else
+   ret = -ENOENT;
+   break;
+   default:
+   /* something fishy here so bail out early */
+   brcmf_err("unknown fw type: %d\n", cur->type);
+   release_firmware(fw);
+   ret = -EINVAL;
+   }
+
+   return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
+}
+
 static int brcmf_fw_request_next_item(struct brcmf_fw *fwctx, bool async)
 {
struct brcmf_fw_item *cur;
@@ -525,15 +553,7 @@ static int brcmf_fw_request_next_item(struct brcmf_fw 
*fwctx, bool async)
if (ret < 0) {
brcmf_fw_request_done(NULL, fwctx);
} else if (!async && fw) {
-   brcmf_dbg(TRACE, "firmware %s %sfound\n", cur->path,
- fw ? "" : "not ");
-   if (cur->type == BRCMF_FW_TYPE_BINARY)
-   cur->binary = fw;
-   else if (cur->type == BRCMF_FW_TYPE_NVRAM)
-   brcmf_fw_request_nvram_done(fw, fwctx);
-   else
-   release_firmware(fw);
-
+   brcmf_fw_complete_request(fw, fwctx);
return -EAGAIN;
}
return 0;
@@ -547,28 +567,8 @@ static void brcmf_fw_request_done(const struct firmware 
*fw, void *ctx)
 
cur = >req->items[fwctx->curpos];
 
-   brcmf_dbg(TRACE, "enter: firmware %s %sfound\n", cur->path,
- fw ? "" : "not ");
-
-   if (!fw)
-   ret = -ENOENT;
-
-   switch (cur->type) {
-   case BRCMF_FW_TYPE_NVRAM:
-   ret = brcmf_fw_request_nvram_done(fw, fwctx);
-   break;
-   case BRCMF_FW_TYPE_BINARY:
-   cur->binary = fw;
-   break;
-   default:
-   /* something fishy here so bail out early */
-   brcmf_err("unknown fw type: %d\n", cur->type);
-   release_firmware(fw);
-   ret = -EINVAL;
-   goto fail;
-   }
-
-   if (ret < 0 && !(cur->flags & BRCMF_FW_REQF_OPTIONAL))
+   ret = brcmf_fw_complete_request(fw, fwctx);
+   if (ret < 0)
goto fail;
 
do {
-- 
2.19.0

[PATCH 2/6] brcmfmac: Remove recursion from firmware load error handling

[Hans de Goede]

Before this commit brcmf_fw_request_done would call
brcmf_fw_request_next_item to load the next item, which on an error would
call brcmf_fw_request_done, which if the error is recoverable (*) will
then continue calling brcmf_fw_request_next_item for the next item again
which on an error will call brcmf_fw_request_done again...

This does not blow up because we only have a limited number of items so
we never recurse too deep. But the recursion is still quite ugly and
frankly is giving me a headache, so lets fix this.

This commit fixes this by removing brcmf_fw_request_next_item and by
making brcmf_fw_get_firmwares and brcmf_fw_request_done directly call
firmware_request_nowait resp. firmware_request themselves.

*) brcmf_fw_request_nvram_done fallback path succeeds or
   BRCMF_FW_REQF_OPTIONAL is set

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 65 ++-
 1 file changed, 19 insertions(+), 46 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 784c84f0e9e7..08aaf99fee34 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -532,33 +532,6 @@ static int brcmf_fw_complete_request(const struct firmware 
*fw,
return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
 }
 
-static int brcmf_fw_request_next_item(struct brcmf_fw *fwctx, bool async)
-{
-   struct brcmf_fw_item *cur;
-   const struct firmware *fw = NULL;
-   int ret;
-
-   cur = >req->items[fwctx->curpos];
-
-   brcmf_dbg(TRACE, "%srequest for %s\n", async ? "async " : "",
- cur->path);
-
-   if (async)
-   ret = request_firmware_nowait(THIS_MODULE, true, cur->path,
- fwctx->dev, GFP_KERNEL, fwctx,
- brcmf_fw_request_done);
-   else
-   ret = request_firmware(, cur->path, fwctx->dev);
-
-   if (ret < 0) {
-   brcmf_fw_request_done(NULL, fwctx);
-   } else if (!async && fw) {
-   brcmf_fw_complete_request(fw, fwctx);
-   return -EAGAIN;
-   }
-   return 0;
-}
-
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
@@ -568,26 +541,19 @@ static void brcmf_fw_request_done(const struct firmware 
*fw, void *ctx)
cur = >req->items[fwctx->curpos];
 
ret = brcmf_fw_complete_request(fw, fwctx);
-   if (ret < 0)
-   goto fail;
-
-   do {
-   if (++fwctx->curpos == fwctx->req->n_items) {
-   ret = 0;
-   goto done;
-   }
 
-   ret = brcmf_fw_request_next_item(fwctx, false);
-   } while (ret == -EAGAIN);
-
-   return;
+   while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
+   cur = >req->items[fwctx->curpos];
+   request_firmware(, cur->path, fwctx->dev);
+   ret = brcmf_fw_complete_request(fw, ctx);
+   }
 
-fail:
-   brcmf_dbg(TRACE, "failed err=%d: dev=%s, fw=%s\n", ret,
- dev_name(fwctx->dev), cur->path);
-   brcmf_fw_free_request(fwctx->req);
-   fwctx->req = NULL;
-done:
+   if (ret) {
+   brcmf_dbg(TRACE, "failed err=%d: dev=%s, fw=%s\n", ret,
+ dev_name(fwctx->dev), cur->path);
+   brcmf_fw_free_request(fwctx->req);
+   fwctx->req = NULL;
+   }
fwctx->done(fwctx->dev, ret, fwctx->req);
kfree(fwctx);
 }
@@ -611,7 +577,9 @@ int brcmf_fw_get_firmwares(struct device *dev, struct 
brcmf_fw_request *req,
   void (*fw_cb)(struct device *dev, int err,
 struct brcmf_fw_request *req))
 {
+   struct brcmf_fw_item *first = >items[0];
struct brcmf_fw *fwctx;
+   int ret;
 
brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(dev));
if (!fw_cb)
@@ -628,7 +596,12 @@ int brcmf_fw_get_firmwares(struct device *dev, struct 
brcmf_fw_request *req,
fwctx->req = req;
fwctx->done = fw_cb;
 
-   brcmf_fw_request_next_item(fwctx, true);
+   ret = request_firmware_nowait(THIS_MODULE, true, first->path,
+ fwctx->dev, GFP_KERNEL, fwctx,
+ brcmf_fw_request_done);
+   if (ret < 0)
+   brcmf_fw_request_done(NULL, fwctx);
+
return 0;
 }
 
-- 
2.19.0

[PATCH 3/6] brcmfmac: Add support for first trying to get a board specific nvram file

[Hans de Goede]

The nvram files which some brcmfmac chips need are board-specific. To be
able to distribute these as part of linux-firmware, so that devices with
such a wifi chip will work OOTB, multiple (one per board) versions must
co-exist under /lib/firmware.

This commit adds support for callers of the brcmfmac/firmware.c code to
pass in a board_type parameter through the request structure.

If that parameter is set then the code will first try to load
chipmodel.board_type.txt before falling back to the old chipmodel.txt name.

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/firmware.c| 27 ++-
 .../broadcom/brcm80211/brcmfmac/firmware.h|  1 +
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 08aaf99fee34..6755b2388fbc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -532,6 +532,31 @@ static int brcmf_fw_complete_request(const struct firmware 
*fw,
return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret;
 }
 
+static int brcmf_fw_request_firmware(const struct firmware **fw,
+struct brcmf_fw *fwctx)
+{
+   struct brcmf_fw_item *cur = >req->items[fwctx->curpos];
+   int ret;
+
+   /* nvram files are board-specific, first try a board-specific path */
+   if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
+   char alt_path[BRCMF_FW_NAME_LEN];
+
+   strlcpy(alt_path, cur->path, BRCMF_FW_NAME_LEN);
+   /* strip .txt at the end */
+   alt_path[strlen(alt_path) - 4] = 0;
+   strlcat(alt_path, ".", BRCMF_FW_NAME_LEN);
+   strlcat(alt_path, fwctx->req->board_type, BRCMF_FW_NAME_LEN);
+   strlcat(alt_path, ".txt", BRCMF_FW_NAME_LEN);
+
+   ret = request_firmware(fw, alt_path, fwctx->dev);
+   if (ret == 0)
+   return ret;
+   }
+
+   return request_firmware(fw, cur->path, fwctx->dev);
+}
+
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
@@ -544,7 +569,7 @@ static void brcmf_fw_request_done(const struct firmware 
*fw, void *ctx)
 
while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
cur = >req->items[fwctx->curpos];
-   request_firmware(, cur->path, fwctx->dev);
+   brcmf_fw_request_firmware(, fwctx);
ret = brcmf_fw_complete_request(fw, ctx);
}
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
index 2893e56910f0..a0834be8864e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.h
@@ -70,6 +70,7 @@ struct brcmf_fw_request {
u16 domain_nr;
u16 bus_nr;
u32 n_items;
+   const char *board_type;
struct brcmf_fw_item items[0];
 };
 
-- 
2.19.0

[PATCH 6/6] brcmfmac: Cleanup brcmf_fw_request_done()

[Hans de Goede]

The "cur" variable is now only used for a debug print and we already
print the same info from brcmf_fw_complete_request(), so the debug print
does not provide any extra info and we can remove it.

Signed-off-by: Hans de Goede 
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/firmware.c   | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 6755b2388fbc..b38c4b40b235 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -560,22 +560,16 @@ static int brcmf_fw_request_firmware(const struct 
firmware **fw,
 static void brcmf_fw_request_done(const struct firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
-   struct brcmf_fw_item *cur;
-   int ret = 0;
-
-   cur = >req->items[fwctx->curpos];
+   int ret;
 
ret = brcmf_fw_complete_request(fw, fwctx);
 
while (ret == 0 && ++fwctx->curpos < fwctx->req->n_items) {
-   cur = >req->items[fwctx->curpos];
brcmf_fw_request_firmware(, fwctx);
ret = brcmf_fw_complete_request(fw, ctx);
}
 
if (ret) {
-   brcmf_dbg(TRACE, "failed err=%d: dev=%s, fw=%s\n", ret,
- dev_name(fwctx->dev), cur->path);
brcmf_fw_free_request(fwctx->req);
fwctx->req = NULL;
}
-- 
2.19.0

[PATCH 4/6] brcmfmac: Set board_type used for nvram file selection to machine-compatible

[Hans de Goede]

For of/devicetree using machines, set the board_type used for nvram file
selection to the first string listed in the top-level's node compatible
string, aka the machine-compatible as used by of_machine_is_compatible().

The board_type setting is used to load the board-specific nvram file with
a board-specific name so that we can ship files for each supported board
in linux-firmware.

Signed-off-by: Hans de Goede 
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/common.h |  1 +
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 11 ++-
 .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c   |  1 +
 .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c   |  1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index a34642cb4d2f..e63a273642e9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -59,6 +59,7 @@ struct brcmf_mp_device {
booliapp;
boolignore_probe_fail;
struct brcmfmac_pd_cc *country_codes;
+   const char  *board_type;
union {
struct brcmfmac_sdio_pd sdio;
} bus;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index aee6e5937c41..84e3373289eb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -27,11 +27,20 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type 
bus_type,
struct brcmf_mp_device *settings)
 {
struct brcmfmac_sdio_pd *sdio = >bus.sdio;
-   struct device_node *np = dev->of_node;
+   struct device_node *root, *np = dev->of_node;
+   struct property *prop;
int irq;
u32 irqf;
u32 val;
 
+   /* Set board-type to the first string of the machine compatible prop */
+   root = of_find_node_by_path("/");
+   if (root) {
+   prop = of_find_property(root, "compatible", NULL);
+   settings->board_type = of_prop_next_string(prop, NULL);
+   of_node_put(root);
+   }
+
if (!np || bus_type != BRCMF_BUSTYPE_SDIO ||
!of_device_is_compatible(np, "brcm,bcm4329-fmac"))
return;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 4fffa6988087..b12f3e0ee69c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1785,6 +1785,7 @@ 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->board_type = devinfo->settings->board_type;
/* 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;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index a907d7b065fa..3d117563 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4177,6 +4177,7 @@ brcmf_sdio_prepare_fw_request(struct brcmf_sdio *bus)
 
fwreq->items[BRCMF_SDIO_FW_CODE].type = BRCMF_FW_TYPE_BINARY;
fwreq->items[BRCMF_SDIO_FW_NVRAM].type = BRCMF_FW_TYPE_NVRAM;
+   fwreq->board_type = bus->sdiodev->settings->board_type;
 
return fwreq;
 }
-- 
2.19.0

[PATCH 5/6] brcmfmac: Set board_type from DMI on x86 based machines

[Hans de Goede]

For x86 based machines, set the board_type used for nvram file selection
based on the DMI sys-vendor and product-name strings.

Since on some models these strings are too generic, this commit also adds
a quirk table overriding the strings for models listed in that table.

The board_type setting is used to load the board-specific nvram file with
a board-specific name so that we can ship files for each supported board
in linux-firmware.

Signed-off-by: Hans de Goede 
---
 .../broadcom/brcm80211/brcmfmac/Makefile  |   2 +
 .../broadcom/brcm80211/brcmfmac/common.c  |   3 +-
 .../broadcom/brcm80211/brcmfmac/common.h  |   7 ++
 .../broadcom/brcm80211/brcmfmac/dmi.c | 104 ++
 4 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
index 1f5a9b948abf..22fd95a736a8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
@@ -54,3 +54,5 @@ brcmfmac-$(CONFIG_BRCM_TRACING) += \
tracepoint.o
 brcmfmac-$(CONFIG_OF) += \
of.o
+brcmfmac-$(CONFIG_DMI) += \
+   dmi.o
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index cd3651069d0c..a4bcbd1a57ac 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -450,8 +450,9 @@ struct brcmf_mp_device *brcmf_get_module_param(struct 
device *dev,
}
}
if (!found) {
-   /* No platform data for this device, try OF (Open Firwmare) */
+   /* No platform data for this device, try OF and DMI data */
brcmf_of_probe(dev, bus_type, settings);
+   brcmf_dmi_probe(settings, chip, chiprev);
}
return settings;
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index e63a273642e9..4ce56be90b74 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -75,4 +75,11 @@ void brcmf_release_module_param(struct brcmf_mp_device 
*module_param);
 /* Sets dongle media info (drv_version, mac address). */
 int brcmf_c_preinit_dcmds(struct brcmf_if *ifp);
 
+#ifdef CONFIG_DMI
+void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev);
+#else
+static inline void
+brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev) {}
+#endif
+
 #endif /* BRCMFMAC_COMMON_H */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
new file mode 100644
index ..fadc0ec745b8
--- /dev/null
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: ISC
+/*
+ * Copyright 2018 Hans de Goede 
+ */
+#include 
+#include 
+#include "core.h"
+#include "common.h"
+#include "brcm_hw_ids.h"
+
+/* The DMI data never changes so we can use a static buf for this */
+static char dmi_board_type[128];
+
+struct brcmf_dmi_data {
+   u32 chip;
+   u32 chiprev;
+   const char *board_type;
+};
+
+/* NOTE: Please keep all entries sorted alphabetically */
+
+static const struct brcmf_dmi_data gpd_win_pocket_data = {
+   BRCM_CC_4356_CHIP_ID, 2, "gpd-win-pocket"
+};
+
+static const struct brcmf_dmi_data jumper_ezpad_mini3_data = {
+   BRCM_CC_43430_CHIP_ID, 0, "jumper-ezpad-mini3"
+};
+
+static const struct brcmf_dmi_data meegopad_t08_data = {
+   BRCM_CC_43340_CHIP_ID, 2, "meegopad-t08"
+};
+
+static const struct dmi_system_id dmi_platform_data[] = {
+   {
+   /* Match for the GPDwin which unfortunately uses somewhat
+* generic dmi strings, which is why we test for 4 strings.
+* Comparing against 23 other byt/cht boards, board_vendor
+* and board_name are unique to the GPDwin, where as only one
+* other board has the same board_serial and 3 others have
+* the same default product_name. Also the GPDwin is the
+* only device to have both board_ and product_name not set.
+*/
+   .matches = {
+   DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
+   DMI_MATCH(DMI_BOARD_NAME, "Default string"),
+   DMI_MATCH(DMI_BOARD_SERIAL, "Default string"),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
+   },
+   .driver_data = (void *)_win

[PATCH 4.17 REGRESSION fix 0/1] Revert "staging:r8188eu: Use lib80211 to support TKIP"

[Hans de Goede]

Hi Greg,

While running some ASOC tests on a VIOS LTH17 laptop I noticed that
wifi was broken and often the machine would hard freeze shortly after
bringing up the wifi connection.

One of the oopses pointed to the commit which this patch reverts and
that fixes both the wifi being broken and the hard freezes when using
the wifi.

The troublesome patch was introduced in 4.17, so this is not a 4.18
regression. Still it would be nice if we can get this fix into 4.18
before it is released.

Regards,

Hans

[PATCH 4.17 REGRESSION fix] Revert "staging:r8188eu: Use lib80211 to support TKIP"

[Hans de Goede]

Commit b83b8b1881c4 ("staging:r8188eu: Use lib80211 to support TKIP")
is causing 2 problems for me:

1) One boot the wifi on a laptop with a r8188eu wifi device would not
   connect and dmesg contained an oops about scheduling while atomic
   pointing to the tkip code. This went away after reverting the commit.

2) I reverted the revert to try and get the oops from 1. again to be able
   to add it to this commit message. But now the system did connect to the
   wifi only to print a whole bunch of oopses, followed by a hardfreeze a
   few seconds later. Subsequent reboots also all lead to scenario 2. Until
   I reverted the commit again.

Revert the commit fixes both issues making the laptop usable again.

Cc: sta...@vger.kernel.org
Cc: Ivan Safonov 
Signed-off-by: Hans de Goede 
---
 drivers/staging/rtl8188eu/Kconfig |   1 -
 drivers/staging/rtl8188eu/core/rtw_recv.c | 161 +-
 drivers/staging/rtl8188eu/core/rtw_security.c |  92 +-
 3 files changed, 160 insertions(+), 94 deletions(-)

diff --git a/drivers/staging/rtl8188eu/Kconfig 
b/drivers/staging/rtl8188eu/Kconfig
index 673fdce25530..ff7832798a77 100644
--- a/drivers/staging/rtl8188eu/Kconfig
+++ b/drivers/staging/rtl8188eu/Kconfig
@@ -7,7 +7,6 @@ config R8188EU
select LIB80211
select LIB80211_CRYPT_WEP
select LIB80211_CRYPT_CCMP
-   select LIB80211_CRYPT_TKIP
---help---
This option adds the Realtek RTL8188EU USB device such as TP-Link 
TL-WN725N.
If built as a module, it will be called r8188eu.
diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c 
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 05936a45eb93..c6857a5be12a 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #define ETHERNET_HEADER_SIZE   14  /*  Ethernet Header Length */
 #define LLC_HEADER_SIZE6   /*  LLC Header Length */
@@ -221,20 +220,31 @@ u32 rtw_free_uc_swdec_pending_queue(struct adapter 
*adapter)
 static int recvframe_chkmic(struct adapter *adapter,
struct recv_frame *precvframe)
 {
-   int res = _SUCCESS;
-   struct rx_pkt_attrib *prxattrib = >attrib;
-   struct sta_info *stainfo = rtw_get_stainfo(>stapriv, 
prxattrib->ta);
+   int i, res = _SUCCESS;
+   u32 datalen;
+   u8  miccode[8];
+   u8  bmic_err = false, brpt_micerror = true;
+   u8  *pframe, *payload, *pframemic;
+   u8  *mickey;
+   struct  sta_info*stainfo;
+   struct  rx_pkt_attrib   *prxattrib = >attrib;
+   struct  security_priv   *psecuritypriv = >securitypriv;
+
+   struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
+   struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+
+   stainfo = rtw_get_stainfo(>stapriv, >ta[0]);
 
if (prxattrib->encrypt == _TKIP_) {
+   RT_TRACE(_module_rtl871x_recv_c_, _drv_info_,
+("\n %s: prxattrib->encrypt==_TKIP_\n", __func__));
+   RT_TRACE(_module_rtl871x_recv_c_, _drv_info_,
+("\n %s: 
da=0x%02x:0x%02x:0x%02x:0x%02x:0x%02x:0x%02x\n",
+ __func__, prxattrib->ra[0], prxattrib->ra[1], 
prxattrib->ra[2],
+ prxattrib->ra[3], prxattrib->ra[4], 
prxattrib->ra[5]));
+
+   /* calculate mic code */
if (stainfo) {
-   int key_idx;
-   const int iv_len = 8, icv_len = 4, key_length = 32;
-   struct sk_buff *skb = precvframe->pkt;
-   u8 key[32], iv[8], icv[4], *pframe = skb->data;
-   void *crypto_private = NULL;
-   struct lib80211_crypto_ops *crypto_ops = 
try_then_request_module(lib80211_get_crypto_ops("TKIP"), "lib80211_crypt_tkip");
-   struct security_priv *psecuritypriv = 
>securitypriv;
-
if (IS_MCAST(prxattrib->ra)) {
if (!psecuritypriv) {
res = _FAIL;
@@ -243,58 +253,115 @@ static int recvframe_chkmic(struct adapter *adapter,
DBG_88E("\n %s: didn't install group 
key!!\n", __func__);
goto exit;
}
-   key_idx = prxattrib->key_index;
-   memcpy(key, 
psecuritypriv->dot118021XGrpKey[key_idx].skey, 16);
-   memcpy(key + 16, 
psecuritypriv->dot118021XGrprxmickey[key_idx].skey, 16);
+   mickey = 
>dot118021XGrprxmickey[prxattrib->key_index].skey[0];
+
+  

Re: [PATCH V3 0/5] Update brcm firmware files

[Hans de Goede]

Hi,

On 30-05-18 09:28, Chi-Hsien Lin wrote:



On 05/27/2018 10:37, Hans de Goede wrote:

Hi,

On 18-05-18 09:04, Hans de Goede wrote:

Hi,

On 15-05-18 10:43, Arend van Spriel wrote:

On 5/14/2018 2:05 PM, Josh Boyer wrote:

n Mon, May 14, 2018 at 6:11 AM Arend van Spriel <
arend.vanspr...@broadcom.com> wrote:


On 3/16/2018 3:08 PM, Chi-Hsien Lin wrote:

Update brcm firmware files and WHENCE accordingly.



Hi firmware-maintainers,



It seems this series somehow got lost. Can these still be applied. They
can be found in the linux-wireless patchwork database. I provided links
below.


All 5 of these move the respective firmware files under the Cypress
license.  It has been pointed out that the Cypress license has some
questionable language in it and that people have been in touch to try and
get this resolved.  I'm personally waiting on applying them until the
licence issue is sorted out.


Thanks, Josh

I could not find any such response. Has it been taken off-list? Seems like 2 
months is quite some time, but maybe there are lawyers involved ;-)


Yes the discussion about this has been happening off-list. IANAL but
the gist of it is (AFAIK) that:

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.cypress

has a clause which allows Cypress to retro-actively revoke the LICENSE:

"either party may terminate this Agreement at any time with or without cause."
...
"Upon termination, you must destroy all copies of Software in your possession or
control."

So upon revokation we would have to remove the files from linux-firmware
(rewrite git history?) and since distros get their redistribution rights
from this license too they would also need to remove it from their packages
including all mirrors and archives of older versions. Which simply is not
feasible and no other license in linux-firmware has such a clause.


Chi-Hsien or other people from Cypress, any progress on getting the license
updated to remove the troublesome termination without cause clause?

Hi all,

I have received the updated license from the Legal department today. Have just 
submitted a V4 with the updated license.


Great, thank you (note I've not checked the new license yet).

Regards,

Hans

Re: [PATCH V3 0/5] Update brcm firmware files

[Hans de Goede]

Hi,

On 18-05-18 09:04, Hans de Goede wrote:

Hi,

On 15-05-18 10:43, Arend van Spriel wrote:

On 5/14/2018 2:05 PM, Josh Boyer wrote:

n Mon, May 14, 2018 at 6:11 AM Arend van Spriel <
arend.vanspr...@broadcom.com> wrote:


On 3/16/2018 3:08 PM, Chi-Hsien Lin wrote:

Update brcm firmware files and WHENCE accordingly.



Hi firmware-maintainers,



It seems this series somehow got lost. Can these still be applied. They
can be found in the linux-wireless patchwork database. I provided links
below.


All 5 of these move the respective firmware files under the Cypress
license.  It has been pointed out that the Cypress license has some
questionable language in it and that people have been in touch to try and
get this resolved.  I'm personally waiting on applying them until the
licence issue is sorted out.


Thanks, Josh

I could not find any such response. Has it been taken off-list? Seems like 2 
months is quite some time, but maybe there are lawyers involved ;-)


Yes the discussion about this has been happening off-list. IANAL but
the gist of it is (AFAIK) that:

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.cypress

has a clause which allows Cypress to retro-actively revoke the LICENSE:

"either party may terminate this Agreement at any time with or without cause."
...
"Upon termination, you must destroy all copies of Software in your possession or
control."

So upon revokation we would have to remove the files from linux-firmware
(rewrite git history?) and since distros get their redistribution rights
from this license too they would also need to remove it from their packages
including all mirrors and archives of older versions. Which simply is not
feasible and no other license in linux-firmware has such a clause.


Chi-Hsien or other people from Cypress, any progress on getting the license
updated to remove the troublesome termination without cause clause?

Non of the other vendors with firmware in Linux firmware deem it necessary
to have such a patch.

Given that the proposed firmware updates which this is blocking are
*SECURITY* fixes it would be good to get this resolved ASAP.

Regards,

Hans

Re: [PATCH V3 0/5] Update brcm firmware files

[Hans de Goede]

Hi,

On 18-05-18 11:00, Arend van Spriel wrote:

On 5/18/2018 9:04 AM, Hans de Goede wrote:

Hi,

On 15-05-18 10:43, Arend van Spriel wrote:

On 5/14/2018 2:05 PM, Josh Boyer wrote:

n Mon, May 14, 2018 at 6:11 AM Arend van Spriel <
arend.vanspr...@broadcom.com> wrote:


On 3/16/2018 3:08 PM, Chi-Hsien Lin wrote:

Update brcm firmware files and WHENCE accordingly.



Hi firmware-maintainers,



It seems this series somehow got lost. Can these still be applied. They
can be found in the linux-wireless patchwork database. I provided links
below.


All 5 of these move the respective firmware files under the Cypress
license.  It has been pointed out that the Cypress license has some
questionable language in it and that people have been in touch to try
and
get this resolved.  I'm personally waiting on applying them until the
licence issue is sorted out.


Thanks, Josh

I could not find any such response. Has it been taken off-list? Seems
like 2 months is quite some time, but maybe there are lawyers involved
;-)


Yes the discussion about this has been happening off-list. IANAL but
the gist of it is (AFAIK) that:

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.cypress


has a clause which allows Cypress to retro-actively revoke the LICENSE:

"either party may terminate this Agreement at any time with or without
cause."
...
"Upon termination, you must destroy all copies of Software in your
possession or
control."

So upon revokation we would have to remove the files from linux-firmware
(rewrite git history?) and since distros get their redistribution rights
from this license too they would also need to remove it from their packages
including all mirrors and archives of older versions. Which simply is not
feasible and no other license in linux-firmware has such a clause.


Check again. From LICENCE.broadcom_bcm43xx:

"""
2.  Restrictions. Licensee shall distribute Software with a copy of this
Agreement. Licensee shall not remove, efface or obscure any copyright or
trademark notices from the Software. Reproductions of the Broadcom copyright
notice shall be included with each copy of the Software, except where such
Software is embedded in a manner not readily accessible to the end user.
Licensee shall not: (i) use, license, sell or otherwise distribute the Software
except as provided in this Agreement; (ii) attempt to modify in any way,
reverse engineer, decompile or disassemble any portion of the Software; or
(iii) use the Software or other material in violation of any applicable law or
regulation, including but not limited to any regulatory agency. This Agreement
shall automatically terminate upon Licensee\u2019s failure to comply with any 
of the
terms of this Agreement. In such event, Licensee will destroy all copies of the
Software and its component parts.
"""


That has the "destroy all copies of (the) Software" language, but the entire
file does not have anything equivalent to: "either party may terminate this 
Agreement
at any time with or without cause.". So as long as we abide by the license 
terms,
the LICENCE.broadcom_bcm43xx license will never terminate and the
"destroy all copies of (the) Software" language is not a problem.

The main problem is the "either party may terminate this Agreement at any time
with or without cause." language in the Cypress license. Note IANAL and TINLA.

Regards,

Hans

Re: [PATCH V3 0/5] Update brcm firmware files

[Hans de Goede]

Hi,

On 15-05-18 10:43, Arend van Spriel wrote:

On 5/14/2018 2:05 PM, Josh Boyer wrote:

n Mon, May 14, 2018 at 6:11 AM Arend van Spriel <
arend.vanspr...@broadcom.com> wrote:


On 3/16/2018 3:08 PM, Chi-Hsien Lin wrote:

Update brcm firmware files and WHENCE accordingly.



Hi firmware-maintainers,



It seems this series somehow got lost. Can these still be applied. They
can be found in the linux-wireless patchwork database. I provided links
below.


All 5 of these move the respective firmware files under the Cypress
license.  It has been pointed out that the Cypress license has some
questionable language in it and that people have been in touch to try and
get this resolved.  I'm personally waiting on applying them until the
licence issue is sorted out.


Thanks, Josh

I could not find any such response. Has it been taken off-list? Seems like 2 
months is quite some time, but maybe there are lawyers involved ;-)


Yes the discussion about this has been happening off-list. IANAL but
the gist of it is (AFAIK) that:

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.cypress

has a clause which allows Cypress to retro-actively revoke the LICENSE:

"either party may terminate this Agreement at any time with or without cause."
...
"Upon termination, you must destroy all copies of Software in your possession or
control."

So upon revokation we would have to remove the files from linux-firmware
(rewrite git history?) and since distros get their redistribution rights
from this license too they would also need to remove it from their packages
including all mirrors and archives of older versions. Which simply is not
feasible and no other license in linux-firmware has such a clause.

Regards,

Hans

Re: [PATCH 1/2] NFC: pn533: Use kmalloc-ed memory for USB transfer buffers

[Hans de Goede]

Samuel,

Can I get an ack for these please?

Anything I need to do to get these picked up / merged?

Regards,

Hans


On 19-03-18 17:40, Hans de Goede wrote:

Commit 8b55d7581fc5 ("NFC: pn533: use constant off-stack buffer for sending
acks"), fixed the ack case of using on stack mem for the transfer_buffer,
by making the ack buffer "static const", which is an unusual solution for
this and I wonder if this is not a problem wrt buffer alignment. It also
misses fixing the same problem for the cmd buffer in the
pn533_acr122_poweron_rdr() function.

This commit introduces an out_buf which gets kmalloc-ed on probe and
then memcpy-s the ack / cmd buffer into that buffer before submitting
the out urb. Fixing the use of on stack memory for the cmd buffer and
moving the ack code-path over to more conventional ways.

While at it this commit also changes the kmalloc of the in_buf to
devm_kmalloc, to avoid the need to introduce a new goto in the error-
handling of the kmalloc of the introduced out_buf.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1514134
Fixes: 8b55d7581fc5 ("NFC: pn533: use constant off-stack buffer ...")
Cc: Michał Mirosław <mirq-li...@rere.qmqm.pl>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  drivers/nfc/pn533/usb.c | 22 +++---
  1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index e153e8b64bb8..c9398712ba31 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -42,6 +42,9 @@
  #define ACS_VENDOR_ID 0x072f
  #define ACR122U_PRODUCT_ID 0x2200
  
+/* Large enough to hold an ack or the power-on CCID command */

+#define OUT_BUF_LEN 16
+
  static const struct usb_device_id pn533_usb_table[] = {
{ USB_DEVICE(PN533_VENDOR_ID, PN533_PRODUCT_ID),
  .driver_info = PN533_DEVICE_STD },
@@ -61,6 +64,7 @@ struct pn533_usb_phy {
  
  	struct urb *out_urb;

struct urb *in_urb;
+   u8 *out_buf;
  
  	struct pn533 *priv;

  };
@@ -152,7 +156,8 @@ static int pn533_usb_send_ack(struct pn533 *dev, gfp_t 
flags)
/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
int rc;
  
-	phy->out_urb->transfer_buffer = (u8 *)ack;

+   memcpy(phy->out_buf, ack, sizeof(ack));
+   phy->out_urb->transfer_buffer = phy->out_buf;
phy->out_urb->transfer_buffer_length = sizeof(ack);
rc = usb_submit_urb(phy->out_urb, flags);
  
@@ -373,8 +378,8 @@ static void pn533_acr122_poweron_rdr_resp(struct urb *urb)

  static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
  {
/* Power on th reader (CCID cmd) */
-   u8 cmd[10] = {PN533_ACR122_PC_TO_RDR_ICCPOWERON,
- 0, 0, 0, 0, 0, 0, 3, 0, 0};
+   static const u8 cmd[10] = { PN533_ACR122_PC_TO_RDR_ICCPOWERON,
+   0, 0, 0, 0, 0, 0, 3, 0, 0 };
int rc;
void *cntx;
struct pn533_acr122_poweron_rdr_arg arg;
@@ -387,7 +392,8 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy 
*phy)
phy->in_urb->complete = pn533_acr122_poweron_rdr_resp;
phy->in_urb->context = 
  
-	phy->out_urb->transfer_buffer = cmd;

+   memcpy(phy->out_buf, cmd, sizeof(cmd));
+   phy->out_urb->transfer_buffer = phy->out_buf;
phy->out_urb->transfer_buffer_length = sizeof(cmd);
  
  	print_hex_dump_debug("ACR122 TX: ", DUMP_PREFIX_NONE, 16, 1,

@@ -463,10 +469,14 @@ static int pn533_usb_probe(struct usb_interface 
*interface,
if (!phy)
return -ENOMEM;
  
-	in_buf = kzalloc(in_buf_len, GFP_KERNEL);

+   in_buf = devm_kzalloc(>dev, in_buf_len, GFP_KERNEL);
if (!in_buf)
return -ENOMEM;
  
+	phy->out_buf = devm_kzalloc(>dev, OUT_BUF_LEN, GFP_KERNEL);

+   if (!phy->out_buf)
+   return -ENOMEM;
+
phy->udev = usb_get_dev(interface_to_usbdev(interface));
phy->interface = interface;
  
@@ -555,7 +565,6 @@ static int pn533_usb_probe(struct usb_interface *interface,

usb_free_urb(phy->in_urb);
usb_free_urb(phy->out_urb);
usb_put_dev(phy->udev);
-   kfree(in_buf);
  
  	return rc;

  }
@@ -574,7 +583,6 @@ static void pn533_usb_disconnect(struct usb_interface 
*interface)
usb_kill_urb(phy->in_urb);
usb_kill_urb(phy->out_urb);
  
-	kfree(phy->in_urb->transfer_buffer);

usb_free_urb(phy->in_urb);
usb_free_urb(phy->out_urb);
  

[PATCH 1/2] NFC: pn533: Use kmalloc-ed memory for USB transfer buffers

[Hans de Goede]

Commit 8b55d7581fc5 ("NFC: pn533: use constant off-stack buffer for sending
acks"), fixed the ack case of using on stack mem for the transfer_buffer,
by making the ack buffer "static const", which is an unusual solution for
this and I wonder if this is not a problem wrt buffer alignment. It also
misses fixing the same problem for the cmd buffer in the
pn533_acr122_poweron_rdr() function.

This commit introduces an out_buf which gets kmalloc-ed on probe and
then memcpy-s the ack / cmd buffer into that buffer before submitting
the out urb. Fixing the use of on stack memory for the cmd buffer and
moving the ack code-path over to more conventional ways.

While at it this commit also changes the kmalloc of the in_buf to
devm_kmalloc, to avoid the need to introduce a new goto in the error-
handling of the kmalloc of the introduced out_buf.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1514134
Fixes: 8b55d7581fc5 ("NFC: pn533: use constant off-stack buffer ...")
Cc: Michał Mirosław <mirq-li...@rere.qmqm.pl>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/nfc/pn533/usb.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index e153e8b64bb8..c9398712ba31 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -42,6 +42,9 @@
 #define ACS_VENDOR_ID 0x072f
 #define ACR122U_PRODUCT_ID 0x2200
 
+/* Large enough to hold an ack or the power-on CCID command */
+#define OUT_BUF_LEN 16
+
 static const struct usb_device_id pn533_usb_table[] = {
{ USB_DEVICE(PN533_VENDOR_ID, PN533_PRODUCT_ID),
  .driver_info = PN533_DEVICE_STD },
@@ -61,6 +64,7 @@ struct pn533_usb_phy {
 
struct urb *out_urb;
struct urb *in_urb;
+   u8 *out_buf;
 
struct pn533 *priv;
 };
@@ -152,7 +156,8 @@ static int pn533_usb_send_ack(struct pn533 *dev, gfp_t 
flags)
/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
int rc;
 
-   phy->out_urb->transfer_buffer = (u8 *)ack;
+   memcpy(phy->out_buf, ack, sizeof(ack));
+   phy->out_urb->transfer_buffer = phy->out_buf;
phy->out_urb->transfer_buffer_length = sizeof(ack);
rc = usb_submit_urb(phy->out_urb, flags);
 
@@ -373,8 +378,8 @@ static void pn533_acr122_poweron_rdr_resp(struct urb *urb)
 static int pn533_acr122_poweron_rdr(struct pn533_usb_phy *phy)
 {
/* Power on th reader (CCID cmd) */
-   u8 cmd[10] = {PN533_ACR122_PC_TO_RDR_ICCPOWERON,
- 0, 0, 0, 0, 0, 0, 3, 0, 0};
+   static const u8 cmd[10] = { PN533_ACR122_PC_TO_RDR_ICCPOWERON,
+   0, 0, 0, 0, 0, 0, 3, 0, 0 };
int rc;
void *cntx;
struct pn533_acr122_poweron_rdr_arg arg;
@@ -387,7 +392,8 @@ static int pn533_acr122_poweron_rdr(struct pn533_usb_phy 
*phy)
phy->in_urb->complete = pn533_acr122_poweron_rdr_resp;
phy->in_urb->context = 
 
-   phy->out_urb->transfer_buffer = cmd;
+   memcpy(phy->out_buf, cmd, sizeof(cmd));
+   phy->out_urb->transfer_buffer = phy->out_buf;
phy->out_urb->transfer_buffer_length = sizeof(cmd);
 
print_hex_dump_debug("ACR122 TX: ", DUMP_PREFIX_NONE, 16, 1,
@@ -463,10 +469,14 @@ static int pn533_usb_probe(struct usb_interface 
*interface,
if (!phy)
return -ENOMEM;
 
-   in_buf = kzalloc(in_buf_len, GFP_KERNEL);
+   in_buf = devm_kzalloc(>dev, in_buf_len, GFP_KERNEL);
if (!in_buf)
return -ENOMEM;
 
+   phy->out_buf = devm_kzalloc(>dev, OUT_BUF_LEN, GFP_KERNEL);
+   if (!phy->out_buf)
+   return -ENOMEM;
+
phy->udev = usb_get_dev(interface_to_usbdev(interface));
phy->interface = interface;
 
@@ -555,7 +565,6 @@ static int pn533_usb_probe(struct usb_interface *interface,
usb_free_urb(phy->in_urb);
usb_free_urb(phy->out_urb);
usb_put_dev(phy->udev);
-   kfree(in_buf);
 
return rc;
 }
@@ -574,7 +583,6 @@ static void pn533_usb_disconnect(struct usb_interface 
*interface)
usb_kill_urb(phy->in_urb);
usb_kill_urb(phy->out_urb);
 
-   kfree(phy->in_urb->transfer_buffer);
usb_free_urb(phy->in_urb);
usb_free_urb(phy->out_urb);
 
-- 
2.14.3

[PATCH 2/2] NFC: pn533: Fix wrong GFP flag usage

[Hans de Goede]

pn533_recv_response() is an urb completion handler, so it must use
GFP_ATOMIC. pn533_usb_send_frame() OTOH runs from a regular sleeping
context, so the pn533_submit_urb_for_response() there (and only there)
can use the regular GFP_KERNEL flags.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1514134
Fixes: 9815c7cf22da ("NFC: pn533: Separate physical layer from ...")
Cc: Michael Thalmeier <michael.thalme...@hale.at>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/nfc/pn533/usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index c9398712ba31..2aff1ddadc1d 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -75,7 +75,7 @@ static void pn533_recv_response(struct urb *urb)
struct sk_buff *skb = NULL;
 
if (!urb->status) {
-   skb = alloc_skb(urb->actual_length, GFP_KERNEL);
+   skb = alloc_skb(urb->actual_length, GFP_ATOMIC);
if (!skb) {
nfc_err(>udev->dev, "failed to alloc memory\n");
} else {
@@ -185,7 +185,7 @@ static int pn533_usb_send_frame(struct pn533 *dev,
 
if (dev->protocol_type == PN533_PROTO_REQ_RESP) {
/* request for response for sent packet directly */
-   rc = pn533_submit_urb_for_response(phy, GFP_ATOMIC);
+   rc = pn533_submit_urb_for_response(phy, GFP_KERNEL);
if (rc)
goto error;
} else if (dev->protocol_type == PN533_PROTO_REQ_ACK_RESP) {
-- 
2.14.3

[PATCH v2] brcmfmac: Add support for getting nvram contents from EFI variables

[Hans de Goede]

Various models Asus laptops with a SDIO attached brcmfmac wifi chip, store
the nvram contents in a special EFI variable. This commit adds support for
getting nvram directly from this EFI variable, without the user needing to
manually copy it.

This makes Wifi / Bluetooth work out of the box on these devices instead of
requiring manual setup.

Note this commit also adds fixup code to make sure the right ccode is used
when the nvram efivar wants to set the worldwide regulatory domain, the
correct ccode for that depends on the firmware version. So if the nvram
efivar contains ccode=ALL (which is never correct) or ccode=XV we fix it
up to the right value (XV or X2) for the firmware, assuming the firmware
from linux-firmware is used.

This has been tested on the following models: Asus T100CHI, Asus T100HA,
Asus T100TA and Asus T200TA

Tested-by: Hans de Goede <hdego...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Stop testing for asus in the dmi sys_vendor string at least the Toshiba
 Encore uses the nvram efivar variable too
-Use a table mapping the firmare name to the correct ccode value (XV / X2) to
 use for the worldwide regulatory domain for that firmware, assuming the
 firmware from linux-firmware is used. This fixes the T200TA and T100HA not
 seeing 5GHz networks
-Not only fixup ccode=ALL, but also ccode=XV, this is necessary since the
 T100HA nvram efivar containts ccode=XV, but the firmware from Linux firmware
 needs ccode=X2 for proper operation
---
 .../broadcom/brcm80211/brcmfmac/firmware.c | 96 --
 1 file changed, 90 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 091b52979e03..ac515d7d29cf 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -14,6 +14,8 @@
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -446,33 +448,115 @@ struct brcmf_fw {
 void *nvram_image, u32 nvram_len);
 };
 
+#ifdef CONFIG_EFI
+static const char * const ccode_fixup_map[][2] = {
+   { "brcm/brcmfmac43241b4-sdio.txt", "XV\n" },
+   { "brcm/brcmfmac43340-sdio.txt", "X2\n" },
+};
+
+static u8 *brcmf_fw_nvram_from_efi(struct brcmf_fw *fwctx, size_t 
*data_len_ret)
+{
+   const u16 name[] = { 'n', 'v', 'r', 'a', 'm', 0 };
+   struct efivar_entry *nvram_efivar;
+   unsigned long data_len = 0;
+   const char *val = NULL;
+   u8 *data = NULL;
+   char *ccode;
+   int i, err;
+
+   nvram_efivar = kzalloc(sizeof(*nvram_efivar), GFP_KERNEL);
+   if (!nvram_efivar)
+   return NULL;
+
+   memcpy(_efivar->var.VariableName, name, sizeof(name));
+   nvram_efivar->var.VendorGuid = EFI_GUID(0x74b00bd9, 0x805a, 0x4d61,
+   0xb5, 0x1f, 0x43, 0x26,
+   0x81, 0x23, 0xd1, 0x13);
+
+   err = efivar_entry_size(nvram_efivar, _len);
+   if (err)
+   goto fail;
+
+   data = kmalloc(data_len, GFP_KERNEL);
+   if (!data)
+   goto fail;
+
+   err = efivar_entry_get(nvram_efivar, NULL, _len, data);
+   if (err)
+   goto fail;
+
+   /* In some cases the EFI-var stored nvram contains "ccode=ALL" or
+* "ccode=XV" to specify "worldwide" compatible settings. ccode=ALL is
+* not understood by the firmware and some of the firmware files in
+* linux-firmware support only 2.4 GHz and not 5 GHz when ccode=XV.
+*/
+   ccode = strnstr((char *)data, "ccode=ALL", data_len);
+   if (!ccode)
+   ccode = strnstr((char *)data, "ccode=XV\r", data_len);
+   if (ccode) {
+   for (i = 0; i < ARRAY_SIZE(ccode_fixup_map); i++) {
+   if (!strcmp(ccode_fixup_map[i][0], fwctx->nvram_name)) {
+   val = ccode_fixup_map[i][1];
+   break;
+   }
+   }
+   if (val) {
+   ccode[6] = val[0];
+   ccode[7] = val[1];
+   ccode[8] = val[2];
+   } else {
+   brcmf_info("Using EFI nvram specifies worldwide ccode, 
but %s not found in ccode fixup map, please report this\n",
+  fwctx->nvram_name);
+   }
+   }
+
+   brcmf_info("Using nvram EFI variable\n");
+
+   kfree(nvram_efivar);
+   *data_len_ret = data_len;
+   return data;
+
+fail:
+   kfree(data);
+   kfree(nvram_efivar);
+   return NULL;
+}
+#else
+static u8 *brcmf_fw_

Re: [PATCH] brcmfmac: Add support for getting nvram contents from EFI variables

[Hans de Goede]

Hi,

On 16-03-18 21:41, Arend van Spriel wrote:

On 3/14/2018 9:43 AM, Hans de Goede wrote:

Hi,

On 13-03-18 21:19, Arend van Spriel wrote:

On 3/12/2018 10:45 AM, Hans de Goede wrote:





Actually had a Sony device with nvram in EFI. Why not just drop this
optimization.


Ok, do you know if that variable had the same name and guid though ?
Because
if it doesn't then this code is not going to work for the Sony case.


If I am not mistaken the name and guid are defined by Broadcom/Microsoft.


Anyways the overhead is small and this only runs once, so I will drop
the
check for v2.


Due to XV issue we may want to keep the check for now.


If we're going to do ccode=ALL replacement based on a dmi-model
table then there is no need to keep the check just for the XV stuff.




Also simply selecting XV instead is not correct. There is not just one
worldwide domain in the regulatory database of the firmware image.


Right, I've read elsewhere that "X2" is the right magic value to use and
I've tested that on some other devices and that does seem to work.

I've also seen "XY" but the other Asus devices all use "XV" and that
works (makes channel 13 work) so it seemed like a good value.

Can you help me understand this problem a bit better? Is the problem
with
"XV" that it may not work with all firmware versions, so on some
firmware
versions it will be as bad as using ALL which the firmware also does not
understand? Or do all firmwares understand XV / XY / X2 but are there
subtle differences?


The firmware has a per-device recipe of what should be in the
regulatory database and per release branch it can differ. So for the
same device customer A could get XV and XY in the firmware regdb and
customer B could get XY and X2.


Hmm, so whether XV, XY and/or X2 works depends on the firmware used,
not on the model of the laptop? That means that:


So how do you suggest we deal with this?

One solution I see is:

1) check for ccode=ALL
2) if found use DMI strings to match the specific model and set a
different
ccode based on the model (so for now use XV for the T200TA only)
3) if found and the model is not known, warn about this and do nothing

Would that work for you ?


I think so.


This is no good, because the model of the laptop and which firmware build
gets used are not really coupled. I think instead it would make more sense
to assume the firmware builds from linux-firmware and have a table of
which ccode=ALL override to use based on wifi-chip model, so in the
case of the T200TA, map brcmfmac43340-sdio to a ccode=XV override
(if ccode=ALL is present).


For our customers, ie. OEM/ODM, we provide a particular device and with it 
comes a firmware build with a regulatory database aka CLM. So in that project 
flow the laptop/phone/whatever model and firmware are really coupled. However, 
for linux-firmware the story is more as you depict it, because we simply do not 
know in what kind of device the chip will be used.


So basically what I'm suggesting is:

static const char * const ccode_all_map[][2] = {
    { "brcm/brcmfmac43241b4-sdio.txt", "XV\n" }, /* Tested on Asus
T100TA, T100CHI */
    { "brcm/brcmfmac43340-sdio.txt", "XV\n" },   /* Tested on Asus
T200TA */
};

...

 ccode = strnstr((char *)data, "ccode=ALL", data_len);
 if (ccode) {
 /* lookup fwctx->nvram_name in ccode_all_map */
 /* if found patch in override string */
 /* else brcmf_info("EFI nvram contains ccode=ALL and %s is
missing from ccode-map, please report\n", fwctx->nvram_name) */
 }

So we actually decide what to replace all with based on the
firmware name, rather then on the laptop model, does that make
sense?


Somewhat. However, I am leaning to a different approach. The ALL country code 
should not be supported in the firmware so it would fallback to something else 
and I would like to know what. The country code can be retrieved and set using 
firmware command. So I would like to retrieve it in brcmf_cfg80211_attach() 
just before doing the brcmf_setup_wiphy_bands() call. I want to know if it 
returns ALL or some other fallback country code. At this stage I am not sure 
what the criteria has to be to set the country code to XV.


Ok.


I hope to get some more clarification from our regulatory team about
the use of ALL and XV. Could you tell me what happens with T200TA when
you leave ccode=ALL in place. What output do you get from "iw list"?
Only channels 1 to 11 and no 5G? Or does it only have 2G.


With ccode=ALL in place, I do see channel 13, but not 14 and channel 13
does
not work (the machine does not associate with my AP which is configured
at chan 13)
if I change it to "XV" then channel 13 does work, and shortly after
associating
channel 14 also shows up in the "iwlist wlan0 freq" output.


So XV seems to be the worldwide country code used for PC-OEMs. So that

Re: [PATCH] brcmfmac: Add support for getting nvram contents from EFI variables

[Hans de Goede]

Hi,

On 13-03-18 21:19, Arend van Spriel wrote:

On 3/12/2018 10:45 AM, Hans de Goede wrote:





Actually had a Sony device with nvram in EFI. Why not just drop this
optimization.


Ok, do you know if that variable had the same name and guid though ?
Because
if it doesn't then this code is not going to work for the Sony case.


If I am not mistaken the name and guid are defined by Broadcom/Microsoft.


Anyways the overhead is small and this only runs once, so I will drop the
check for v2.


Due to XV issue we may want to keep the check for now.


If we're going to do ccode=ALL replacement based on a dmi-model
table then there is no need to keep the check just for the XV stuff.




Also simply selecting XV instead is not correct. There is not just one
worldwide domain in the regulatory database of the firmware image.


Right, I've read elsewhere that "X2" is the right magic value to use and
I've tested that on some other devices and that does seem to work.

I've also seen "XY" but the other Asus devices all use "XV" and that
works (makes channel 13 work) so it seemed like a good value.

Can you help me understand this problem a bit better? Is the problem with
"XV" that it may not work with all firmware versions, so on some firmware
versions it will be as bad as using ALL which the firmware also does not
understand? Or do all firmwares understand XV / XY / X2 but are there
subtle differences?


The firmware has a per-device recipe of what should be in the regulatory 
database and per release branch it can differ. So for the same device customer 
A could get XV and XY in the firmware regdb and customer B could get XY and X2.


Hmm, so whether XV, XY and/or X2 works depends on the firmware used,
not on the model of the laptop? That means that:


So how do you suggest we deal with this?

One solution I see is:

1) check for ccode=ALL
2) if found use DMI strings to match the specific model and set a different
ccode based on the model (so for now use XV for the T200TA only)
3) if found and the model is not known, warn about this and do nothing

Would that work for you ?


I think so.


This is no good, because the model of the laptop and which firmware build
gets used are not really coupled. I think instead it would make more sense
to assume the firmware builds from linux-firmware and have a table of
which ccode=ALL override to use based on wifi-chip model, so in the
case of the T200TA, map brcmfmac43340-sdio to a ccode=XV override
(if ccode=ALL is present).

So basically what I'm suggesting is:

static const char * const ccode_all_map[][2] = {
   { "brcm/brcmfmac43241b4-sdio.txt", "XV\n" }, /* Tested on Asus T100TA, 
T100CHI */
   { "brcm/brcmfmac43340-sdio.txt", "XV\n" },   /* Tested on Asus T200TA */
};

...

ccode = strnstr((char *)data, "ccode=ALL", data_len);
if (ccode) {
/* lookup fwctx->nvram_name in ccode_all_map */
/* if found patch in override string */
/* else brcmf_info("EFI nvram contains ccode=ALL and %s is missing from 
ccode-map, please report\n", fwctx->nvram_name) */
}

So we actually decide what to replace all with based on the
firmware name, rather then on the laptop model, does that make
sense?


I hope to get some more clarification from our regulatory team about the use of ALL and 
XV. Could you tell me what happens with T200TA when you leave ccode=ALL in place. What 
output do you get from "iw list"? Only channels 1 to 11 and no 5G? Or does it 
only have 2G.


With ccode=ALL in place, I do see channel 13, but not 14 and channel 13 does
not work (the machine does not associate with my AP which is configured at chan 
13)
if I change it to "XV" then channel 13 does work, and shortly after associating
channel 14 also shows up in the "iwlist wlan0 freq" output.

As for 5GHz on the T200TA that is really a different topic, I can access 5GHz 
wifi
under Windows but not under Linux, the channels are there in "iwlist wlan0 freq"
but "wlist wlan0 scan" only shows 2.4 GHz APs. I've tried replacing the nvram
with the file from the Windows partition referenced by the .inf file there,
but that does not help. I'm not sure yet if this is a firmware / nvram / driver
problem, so as said this really is a different topic.


  if (raw_nvram)
-    bcm47xx_nvram_release_contents(data);
+    kvfree(data); /* vfree for bcm47xx case / kfree for efi case */


While this clearly works I can not say I like this. If you want to do
it this way, please submit a patch to remove
bcm47xx_nvram_release_contents() as it is no longer needed since we
now have kvfree(). From maintainance perspective also drop the postfix
comment. We just might end up doing vmalloc in efi case at some point
and this would likely be forgotten.


It might be better if I replace the raw_nvram varia

Re: [PATCH] brcmfmac: Add support for getting nvram contents from EFI variables

[Hans de Goede]

Hi,

On 12-03-18 09:55, Arend van Spriel wrote:

On 3/11/2018 10:47 PM, Hans de Goede wrote:

Various models Asus laptops with a SDIO attached brcmfmac wifi chip, store
the nvram contents in a special EFI variable. This commit adds support for
getting nvram directly from this EFI variable, without the user needing to
manually copy it.

This makes Wifi / Bluetooth work out of the box on these devices instead of
requiring manual setup.

Note that at least on the Asus T200TA the nvram from the EFI variable
wrongly contains "ccode=ALL" which the firmware does not understand, the
code to fetch the nvram from the EFI variable will fix this up to:
"ccode=XV" which is the correct way to specify the worldwide broadcast
regime.

This has been tested on the following models: Asus T100CHI, Asus T100HA,
Asus T100TA and Asus T200TA


Hi Hans,

I like to have this as well, but  (see below)


Tested-by: Hans de Goede <hdego...@redhat.com>


Duh. I would expect anyone to have tested their patches although you can not 
cover every system out there obviously :-p


Heh, I added it in this case as I went to the trouble of finding 4 devices
which use this and test it on all 4 devices.


Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  .../broadcom/brcm80211/brcmfmac/firmware.c | 68 +-
  1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 091b52979e03..cbac407ae132 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -14,6 +14,8 @@
   * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
   */

+#include 
+#include 
  #include 
  #include 
  #include 
@@ -446,6 +448,67 @@ struct brcmf_fw {
   void *nvram_image, u32 nvram_len);
  };

+#ifdef CONFIG_EFI
+static u8 *brcmf_fw_nvram_from_efi(size_t *data_len_ret)
+{
+    const u16 name[] = { 'n', 'v', 'r', 'a', 'm', 0 };


Isn't there some helper function to make this conversion to u16 array? Hmm, 
maybe it is my itch to scratch later :-)


Nope, I copied this style from existing code under drivers/firmware/efi


+    struct efivar_entry *nvram_efivar;
+    unsigned long data_len = 0;
+    u8 *data = NULL;
+    char *ccode;
+    int err;
+
+    /* So far only Asus devices store the nvram in an EFI var */
+    if (!dmi_name_in_vendors("ASUSTeK COMPUTER INC."))
+    return NULL;


Actually had a Sony device with nvram in EFI. Why not just drop this 
optimization.


Ok, do you know if that variable had the same name and guid though ? Because
if it doesn't then this code is not going to work for the Sony case.

Anyways the overhead is small and this only runs once, so I will drop the
check for v2.


+    nvram_efivar = kzalloc(sizeof(*nvram_efivar), GFP_KERNEL);
+    if (!nvram_efivar)
+    return NULL;
+
+    memcpy(_efivar->var.VariableName, name, sizeof(name));
+    nvram_efivar->var.VendorGuid = EFI_GUID(0x74b00bd9, 0x805a, 0x4d61,
+    0xb5, 0x1f, 0x43, 0x26,
+    0x81, 0x23, 0xd1, 0x13);
+
+    err = efivar_entry_size(nvram_efivar, _len);
+    if (err)
+    goto fail;
+
+    data = kmalloc(data_len, GFP_KERNEL);
+    if (!data)
+    goto fail;
+
+    err = efivar_entry_get(nvram_efivar, NULL, _len, data);
+    if (err)
+    goto fail;
+
+    /* In some cases the EFI-var stored nvram contains "ccode=ALL" but
+ * the firmware does not understand "ALL" change this to "XV" which
+ * is the correct way to specify the "worldwide" compatible settings.
+ */


This is actually quite bad. The ALL ccode should never end up on the market as 
it is intended for internal use only during bringup project phase so these 
seems to be programmed wrong.


I see lots of cheap Windows 10 and ARM devices with brcmfmac sdio wifi
which ship with on disk nvram files using the "ALL" ccode. I actually have
pinned my home wifi at channel 13 to catch this, because channel 13 does
not work with the ALL ccode. If I understand correctly the worldwide
domain starts with channel 13/14 in passive/listen only mode and allows
using them once it has seen valid wifi traffic on them, so they do allow
communicating with an AP on channel 13 here in the Netherlands where that
is allowed?


Also simply selecting XV instead is not correct. There is not just one 
worldwide domain in the regulatory database of the firmware image.


Right, I've read elsewhere that "X2" is the right magic value to use and
I've tested that on some other devices and that does seem to work.

I've also seen "XY" but the other Asus devices all use "XV" and that
works (makes channel 13 work) so it seemed like a good value.

Can you help me understand this problem a bit 

[PATCH] brcmfmac: Add support for getting nvram contents from EFI variables

[Hans de Goede]

Various models Asus laptops with a SDIO attached brcmfmac wifi chip, store
the nvram contents in a special EFI variable. This commit adds support for
getting nvram directly from this EFI variable, without the user needing to
manually copy it.

This makes Wifi / Bluetooth work out of the box on these devices instead of
requiring manual setup.

Note that at least on the Asus T200TA the nvram from the EFI variable
wrongly contains "ccode=ALL" which the firmware does not understand, the
code to fetch the nvram from the EFI variable will fix this up to:
"ccode=XV" which is the correct way to specify the worldwide broadcast
regime.

This has been tested on the following models: Asus T100CHI, Asus T100HA,
Asus T100TA and Asus T200TA

Tested-by: Hans de Goede <hdego...@redhat.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 .../broadcom/brcm80211/brcmfmac/firmware.c | 68 +-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 091b52979e03..cbac407ae132 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -14,6 +14,8 @@
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -446,6 +448,67 @@ struct brcmf_fw {
 void *nvram_image, u32 nvram_len);
 };
 
+#ifdef CONFIG_EFI
+static u8 *brcmf_fw_nvram_from_efi(size_t *data_len_ret)
+{
+   const u16 name[] = { 'n', 'v', 'r', 'a', 'm', 0 };
+   struct efivar_entry *nvram_efivar;
+   unsigned long data_len = 0;
+   u8 *data = NULL;
+   char *ccode;
+   int err;
+
+   /* So far only Asus devices store the nvram in an EFI var */
+   if (!dmi_name_in_vendors("ASUSTeK COMPUTER INC."))
+   return NULL;
+
+   nvram_efivar = kzalloc(sizeof(*nvram_efivar), GFP_KERNEL);
+   if (!nvram_efivar)
+   return NULL;
+
+   memcpy(_efivar->var.VariableName, name, sizeof(name));
+   nvram_efivar->var.VendorGuid = EFI_GUID(0x74b00bd9, 0x805a, 0x4d61,
+   0xb5, 0x1f, 0x43, 0x26,
+   0x81, 0x23, 0xd1, 0x13);
+
+   err = efivar_entry_size(nvram_efivar, _len);
+   if (err)
+   goto fail;
+
+   data = kmalloc(data_len, GFP_KERNEL);
+   if (!data)
+   goto fail;
+
+   err = efivar_entry_get(nvram_efivar, NULL, _len, data);
+   if (err)
+   goto fail;
+
+   /* In some cases the EFI-var stored nvram contains "ccode=ALL" but
+* the firmware does not understand "ALL" change this to "XV" which
+* is the correct way to specify the "worldwide" compatible settings.
+*/
+   ccode = strnstr((char *)data, "ccode=ALL", data_len);
+   if (ccode) {
+   ccode[6] = 'X';
+   ccode[7] = 'V';
+   ccode[8] = '\n';
+   }
+
+   brcmf_info("Using nvram EFI variable\n");
+
+   kfree(nvram_efivar);
+   *data_len_ret = data_len;
+   return data;
+
+fail:
+   kfree(data);
+   kfree(nvram_efivar);
+   return NULL;
+}
+#else
+static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
+#endif
+
 static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
 {
struct brcmf_fw *fwctx = ctx;
@@ -462,6 +525,8 @@ static void brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
raw_nvram = false;
} else {
data = bcm47xx_nvram_get_contents(_len);
+   if (!data)
+   data = brcmf_fw_nvram_from_efi(_len);
if (!data && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
goto fail;
raw_nvram = true;
@@ -472,7 +537,8 @@ static void brcmf_fw_request_nvram_done(const struct 
firmware *fw, void *ctx)
 fwctx->domain_nr, fwctx->bus_nr);
 
if (raw_nvram)
-   bcm47xx_nvram_release_contents(data);
+   kvfree(data); /* vfree for bcm47xx case / kfree for efi case */
+
release_firmware(fw);
if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
goto fail;
-- 
2.14.3

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 01-03-18 20:54, Arend van Spriel wrote:

On 3/1/2018 9:35 AM, Hans de Goede wrote:

HI,

On 28-02-18 20:43, Arend van Spriel wrote:

On 2/28/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 27-02-18 00:04, Arend van Spriel wrote:

On 2/26/2018 12:22 PM, Hans de Goede wrote:

Hi,

On 26-02-18 12:01, Arend van Spriel wrote:

On 2/26/2018 11:29 AM, Hans de Goede wrote:

Hi,

On 26-02-18 11:22, Arend van Spriel wrote:

On 2/25/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 26-05-17 12:57, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra
virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>


I'm now seeing this on another device too, but this time the error
thrown is -EBADE, this seems to be new with recent kernels:


Yup. Before we were passing firmware errors up to user-space, which
was confusing and potentially be misinterpreted. However,
looking at
the output below it would have been good to log the firmware
error as
well. And staring at it some more I suddenly realize I broke the
feature detection module with this change. Actually only the GSCAN
feature detection.


[root@localhost ~]# dmesg | grep brcmfmac
[   34.265950] usbcore: registered new interface driver brcmfmac
[   34.266059] brcmfmac :01:00.0: enabling device ( ->
0002)
[   34.376468] brcmfmac: brcmf_fw_map_chip_to_name: using
brcm/brcmfmac4356-pcie.bin for chip 0x004356(17238) rev 0x02
[   34.855143] brcmfmac :01:00.0: Direct firmware load for
brcm/brcmfmac4356-pcie.clm_blob failed with error -2
[   34.855147] brcmfmac: brcmf_c_process_clm_blob: no clm_blob
available(err=-2), device may have limited channels available
[   34.857029] brcmfmac: brcmf_c_preinit_dcmds: Firmware version =
wl0:
Jun  4 2017 16:50:07 version 7.35.101.6 (r702795) FWID 01-5e8eb735
[   34.938854] brcmfmac :01:00.0 wlp1s0: renamed from wlan0
[   37.086420] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc
error
[   37.086431] brcmfmac: brcmf_cfg80211_add_iface: add iface
p2p-dev-wlp1s0 type 10 failed: err=-52

[root@localhost ~]# strings
/lib/firmware/brcm/brcmfmac4356-pcie.bin |
tail -n 1
4356a2-roml/pcie-ag-msgbuf-splitrx-p2p-pno-aoe-pktfilter-keepalive-sr-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wl11u-mfp-tdls-amsdutx-sarctrl-proxd-hs20sta-rcc-wepso-ndoe-linkstat-gscan-hchk-logtrace-roamexp-rmon




Version: 7.35.101.6 (r702795) CRC: 4f3f65c5 Date: Sun 2017-06-04
16:51:38 PDT Ucode Ver: 963.316 FWID: 01-5e8eb735

It would be good if we can silence these errors, or maybe at a
minimum lower their log-level from error to warning?


I had a look at it and it seems to be a difference in firmware api
that we need to support in the driver. Need to do a bit more
digging,
but it seems an actual issue. You could silence it for now, but I
prefer to wait for the fix.


Ok, what is the ETA of a fix for this?


Actually went back to an old log you sent and noticed:

[   15.714569] brcmfmac: brcmf_attach Enter
[   15.714756] brcmfmac: brcmf_fweh_register event handler registered
for PSM_WATCHDOG
[   15.714757] brcmfmac: brcmf_proto_attach Enter
[   15.716598] brcmfmac: brcmf_bus_started
[   15.716603] brcmfmac: brcmf_add_if Enter, bsscfgidx=0, ifidx=0
[   15.716604] brcmfmac: brcmf_add_if allocate netdev interface
[   15.716622] brcmfmac: brcmf_add_if   pid:2a, if:wlan%d
(00:00:00:00:00:00) created ===
[   15.716624] brcmfmac: brcmf_bus_change_state 0 -> 1
[   15.717841] brcmfmac: brcmf_fil_iovar_data_get ifidx=0,
name=cur_etheraddr, len=6
[   15.717843] brcmutil: data
[   15.717847] : 44 2c 05 9e c9 02   D,

So mac address of the device is 44:2c:05:9e:c9. However, further down
I see:

[   17.819113] brcmfmac: brcmf_netdev_set_mac_address Enter,
bsscfgidx=0
[   17.819122] brcmfmac: brcmf_fil_iovar_data_set ifidx=0,
name=cur_etheraddr, len=6
[   17.819127] brcmutil: data
[   17.819135] : aa 3e 81 77 bc 40   .>.w.@
[   17.819864] brcmfmac: brcmf_netdev_set_mac_address updated to
aa:3e:81:77:bc:40

So the mac address in a local admin variant.


Right, this is likely NetworkManager randomizing the mac for privacy
reasons.


Now our firmware has a requirement for the p2p-dev interface that it
should be different from the mac address of the primary interface,
ie.
wlp1s0 in this log. In brcmfmac we try to do that by setting the
local
admin bit, but... as it is already set we end up using the same mac
address hence the -EBUSY.


Ah, that is good to know, so how can we fix this? Can userspace
specify a
different mac-address when it asks for the p2p-dev intf to be
created? Or
should we do something about this in the kernel?


So this is the patch I tested. Maybe you can verify it works for you
as well.


I can confirm that this fixes the errors, but I do not think that this
is a good
solution, using the permanent mac address for the p2p interface has the
same
privacy problem as using it regularly. IMHO it would be best t

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

HI,

On 28-02-18 20:43, Arend van Spriel wrote:

On 2/28/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 27-02-18 00:04, Arend van Spriel wrote:

On 2/26/2018 12:22 PM, Hans de Goede wrote:

Hi,

On 26-02-18 12:01, Arend van Spriel wrote:

On 2/26/2018 11:29 AM, Hans de Goede wrote:

Hi,

On 26-02-18 11:22, Arend van Spriel wrote:

On 2/25/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 26-05-17 12:57, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra
virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>


I'm now seeing this on another device too, but this time the error
thrown is -EBADE, this seems to be new with recent kernels:


Yup. Before we were passing firmware errors up to user-space, which
was confusing and potentially be misinterpreted. However, looking at
the output below it would have been good to log the firmware error as
well. And staring at it some more I suddenly realize I broke the
feature detection module with this change. Actually only the GSCAN
feature detection.


[root@localhost ~]# dmesg | grep brcmfmac
[   34.265950] usbcore: registered new interface driver brcmfmac
[   34.266059] brcmfmac :01:00.0: enabling device ( -> 0002)
[   34.376468] brcmfmac: brcmf_fw_map_chip_to_name: using
brcm/brcmfmac4356-pcie.bin for chip 0x004356(17238) rev 0x02
[   34.855143] brcmfmac :01:00.0: Direct firmware load for
brcm/brcmfmac4356-pcie.clm_blob failed with error -2
[   34.855147] brcmfmac: brcmf_c_process_clm_blob: no clm_blob
available(err=-2), device may have limited channels available
[   34.857029] brcmfmac: brcmf_c_preinit_dcmds: Firmware version =
wl0:
Jun  4 2017 16:50:07 version 7.35.101.6 (r702795) FWID 01-5e8eb735
[   34.938854] brcmfmac :01:00.0 wlp1s0: renamed from wlan0
[   37.086420] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   37.086431] brcmfmac: brcmf_cfg80211_add_iface: add iface
p2p-dev-wlp1s0 type 10 failed: err=-52

[root@localhost ~]# strings
/lib/firmware/brcm/brcmfmac4356-pcie.bin |
tail -n 1
4356a2-roml/pcie-ag-msgbuf-splitrx-p2p-pno-aoe-pktfilter-keepalive-sr-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wl11u-mfp-tdls-amsdutx-sarctrl-proxd-hs20sta-rcc-wepso-ndoe-linkstat-gscan-hchk-logtrace-roamexp-rmon



Version: 7.35.101.6 (r702795) CRC: 4f3f65c5 Date: Sun 2017-06-04
16:51:38 PDT Ucode Ver: 963.316 FWID: 01-5e8eb735

It would be good if we can silence these errors, or maybe at a
minimum lower their log-level from error to warning?


I had a look at it and it seems to be a difference in firmware api
that we need to support in the driver. Need to do a bit more digging,
but it seems an actual issue. You could silence it for now, but I
prefer to wait for the fix.


Ok, what is the ETA of a fix for this?


Actually went back to an old log you sent and noticed:

[   15.714569] brcmfmac: brcmf_attach Enter
[   15.714756] brcmfmac: brcmf_fweh_register event handler registered
for PSM_WATCHDOG
[   15.714757] brcmfmac: brcmf_proto_attach Enter
[   15.716598] brcmfmac: brcmf_bus_started
[   15.716603] brcmfmac: brcmf_add_if Enter, bsscfgidx=0, ifidx=0
[   15.716604] brcmfmac: brcmf_add_if allocate netdev interface
[   15.716622] brcmfmac: brcmf_add_if   pid:2a, if:wlan%d
(00:00:00:00:00:00) created ===
[   15.716624] brcmfmac: brcmf_bus_change_state 0 -> 1
[   15.717841] brcmfmac: brcmf_fil_iovar_data_get ifidx=0,
name=cur_etheraddr, len=6
[   15.717843] brcmutil: data
[   15.717847] : 44 2c 05 9e c9 02   D,

So mac address of the device is 44:2c:05:9e:c9. However, further down
I see:

[   17.819113] brcmfmac: brcmf_netdev_set_mac_address Enter,
bsscfgidx=0
[   17.819122] brcmfmac: brcmf_fil_iovar_data_set ifidx=0,
name=cur_etheraddr, len=6
[   17.819127] brcmutil: data
[   17.819135] : aa 3e 81 77 bc 40   .>.w.@
[   17.819864] brcmfmac: brcmf_netdev_set_mac_address updated to
aa:3e:81:77:bc:40

So the mac address in a local admin variant.


Right, this is likely NetworkManager randomizing the mac for privacy
reasons.


Now our firmware has a requirement for the p2p-dev interface that it
should be different from the mac address of the primary interface, ie.
wlp1s0 in this log. In brcmfmac we try to do that by setting the local
admin bit, but... as it is already set we end up using the same mac
address hence the -EBUSY.


Ah, that is good to know, so how can we fix this? Can userspace
specify a
different mac-address when it asks for the p2p-dev intf to be
created? Or
should we do something about this in the kernel?


So this is the patch I tested. Maybe you can verify it works for you
as well.


I can confirm that this fixes the errors, but I do not think that this
is a good
solution, using the permanent mac address for the p2p interface has the
same
privacy problem as using it regularly. IMHO it would be best to just
generate
a random mac address if none is specified. This is what the kernel seems
to do

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 27-02-18 00:04, Arend van Spriel wrote:

On 2/26/2018 12:22 PM, Hans de Goede wrote:

Hi,

On 26-02-18 12:01, Arend van Spriel wrote:

On 2/26/2018 11:29 AM, Hans de Goede wrote:

Hi,

On 26-02-18 11:22, Arend van Spriel wrote:

On 2/25/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 26-05-17 12:57, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra
virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>


I'm now seeing this on another device too, but this time the error
thrown is -EBADE, this seems to be new with recent kernels:


Yup. Before we were passing firmware errors up to user-space, which
was confusing and potentially be misinterpreted. However, looking at
the output below it would have been good to log the firmware error as
well. And staring at it some more I suddenly realize I broke the
feature detection module with this change. Actually only the GSCAN
feature detection.


[root@localhost ~]# dmesg | grep brcmfmac
[   34.265950] usbcore: registered new interface driver brcmfmac
[   34.266059] brcmfmac :01:00.0: enabling device ( -> 0002)
[   34.376468] brcmfmac: brcmf_fw_map_chip_to_name: using
brcm/brcmfmac4356-pcie.bin for chip 0x004356(17238) rev 0x02
[   34.855143] brcmfmac :01:00.0: Direct firmware load for
brcm/brcmfmac4356-pcie.clm_blob failed with error -2
[   34.855147] brcmfmac: brcmf_c_process_clm_blob: no clm_blob
available(err=-2), device may have limited channels available
[   34.857029] brcmfmac: brcmf_c_preinit_dcmds: Firmware version =
wl0:
Jun  4 2017 16:50:07 version 7.35.101.6 (r702795) FWID 01-5e8eb735
[   34.938854] brcmfmac :01:00.0 wlp1s0: renamed from wlan0
[   37.086420] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   37.086431] brcmfmac: brcmf_cfg80211_add_iface: add iface
p2p-dev-wlp1s0 type 10 failed: err=-52

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac4356-pcie.bin |
tail -n 1
4356a2-roml/pcie-ag-msgbuf-splitrx-p2p-pno-aoe-pktfilter-keepalive-sr-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wl11u-mfp-tdls-amsdutx-sarctrl-proxd-hs20sta-rcc-wepso-ndoe-linkstat-gscan-hchk-logtrace-roamexp-rmon


Version: 7.35.101.6 (r702795) CRC: 4f3f65c5 Date: Sun 2017-06-04
16:51:38 PDT Ucode Ver: 963.316 FWID: 01-5e8eb735

It would be good if we can silence these errors, or maybe at a
minimum lower their log-level from error to warning?


I had a look at it and it seems to be a difference in firmware api
that we need to support in the driver. Need to do a bit more digging,
but it seems an actual issue. You could silence it for now, but I
prefer to wait for the fix.


Ok, what is the ETA of a fix for this?


Actually went back to an old log you sent and noticed:

[   15.714569] brcmfmac: brcmf_attach Enter
[   15.714756] brcmfmac: brcmf_fweh_register event handler registered
for PSM_WATCHDOG
[   15.714757] brcmfmac: brcmf_proto_attach Enter
[   15.716598] brcmfmac: brcmf_bus_started
[   15.716603] brcmfmac: brcmf_add_if Enter, bsscfgidx=0, ifidx=0
[   15.716604] brcmfmac: brcmf_add_if allocate netdev interface
[   15.716622] brcmfmac: brcmf_add_if   pid:2a, if:wlan%d
(00:00:00:00:00:00) created ===
[   15.716624] brcmfmac: brcmf_bus_change_state 0 -> 1
[   15.717841] brcmfmac: brcmf_fil_iovar_data_get ifidx=0,
name=cur_etheraddr, len=6
[   15.717843] brcmutil: data
[   15.717847] : 44 2c 05 9e c9 02   D,

So mac address of the device is 44:2c:05:9e:c9. However, further down
I see:

[   17.819113] brcmfmac: brcmf_netdev_set_mac_address Enter, bsscfgidx=0
[   17.819122] brcmfmac: brcmf_fil_iovar_data_set ifidx=0,
name=cur_etheraddr, len=6
[   17.819127] brcmutil: data
[   17.819135] : aa 3e 81 77 bc 40   .>.w.@
[   17.819864] brcmfmac: brcmf_netdev_set_mac_address updated to
aa:3e:81:77:bc:40

So the mac address in a local admin variant.


Right, this is likely NetworkManager randomizing the mac for privacy
reasons.


Now our firmware has a requirement for the p2p-dev interface that it
should be different from the mac address of the primary interface, ie.
wlp1s0 in this log. In brcmfmac we try to do that by setting the local
admin bit, but... as it is already set we end up using the same mac
address hence the -EBUSY.


Ah, that is good to know, so how can we fix this? Can userspace specify a
different mac-address when it asks for the p2p-dev intf to be created? Or
should we do something about this in the kernel?


So this is the patch I tested. Maybe you can verify it works for you as well.


I can confirm that this fixes the errors, but I do not think that this is a good
solution, using the permanent mac address for the p2p interface has the same
privacy problem as using it regularly. IMHO it would be best to just generate
a random mac address if none is specified. This is what the kernel seems to do
in general when it needs a mac address and none is specified. You can use the
eth_rando

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 26-02-18 12:01, Arend van Spriel wrote:

On 2/26/2018 11:29 AM, Hans de Goede wrote:

Hi,

On 26-02-18 11:22, Arend van Spriel wrote:

On 2/25/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 26-05-17 12:57, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra
virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>


I'm now seeing this on another device too, but this time the error
thrown is -EBADE, this seems to be new with recent kernels:


Yup. Before we were passing firmware errors up to user-space, which
was confusing and potentially be misinterpreted. However, looking at
the output below it would have been good to log the firmware error as
well. And staring at it some more I suddenly realize I broke the
feature detection module with this change. Actually only the GSCAN
feature detection.


[root@localhost ~]# dmesg | grep brcmfmac
[   34.265950] usbcore: registered new interface driver brcmfmac
[   34.266059] brcmfmac :01:00.0: enabling device ( -> 0002)
[   34.376468] brcmfmac: brcmf_fw_map_chip_to_name: using
brcm/brcmfmac4356-pcie.bin for chip 0x004356(17238) rev 0x02
[   34.855143] brcmfmac :01:00.0: Direct firmware load for
brcm/brcmfmac4356-pcie.clm_blob failed with error -2
[   34.855147] brcmfmac: brcmf_c_process_clm_blob: no clm_blob
available(err=-2), device may have limited channels available
[   34.857029] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0:
Jun  4 2017 16:50:07 version 7.35.101.6 (r702795) FWID 01-5e8eb735
[   34.938854] brcmfmac :01:00.0 wlp1s0: renamed from wlan0
[   37.086420] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   37.086431] brcmfmac: brcmf_cfg80211_add_iface: add iface
p2p-dev-wlp1s0 type 10 failed: err=-52

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac4356-pcie.bin |
tail -n 1
4356a2-roml/pcie-ag-msgbuf-splitrx-p2p-pno-aoe-pktfilter-keepalive-sr-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wl11u-mfp-tdls-amsdutx-sarctrl-proxd-hs20sta-rcc-wepso-ndoe-linkstat-gscan-hchk-logtrace-roamexp-rmon

Version: 7.35.101.6 (r702795) CRC: 4f3f65c5 Date: Sun 2017-06-04
16:51:38 PDT Ucode Ver: 963.316 FWID: 01-5e8eb735

It would be good if we can silence these errors, or maybe at a
minimum lower their log-level from error to warning?


I had a look at it and it seems to be a difference in firmware api
that we need to support in the driver. Need to do a bit more digging,
but it seems an actual issue. You could silence it for now, but I
prefer to wait for the fix.


Ok, what is the ETA of a fix for this?


Actually went back to an old log you sent and noticed:

[   15.714569] brcmfmac: brcmf_attach Enter
[   15.714756] brcmfmac: brcmf_fweh_register event handler registered for 
PSM_WATCHDOG
[   15.714757] brcmfmac: brcmf_proto_attach Enter
[   15.716598] brcmfmac: brcmf_bus_started
[   15.716603] brcmfmac: brcmf_add_if Enter, bsscfgidx=0, ifidx=0
[   15.716604] brcmfmac: brcmf_add_if allocate netdev interface
[   15.716622] brcmfmac: brcmf_add_if   pid:2a, if:wlan%d 
(00:00:00:00:00:00) created ===
[   15.716624] brcmfmac: brcmf_bus_change_state 0 -> 1
[   15.717841] brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=cur_etheraddr, 
len=6
[   15.717843] brcmutil: data
[   15.717847] : 44 2c 05 9e c9 02   D,

So mac address of the device is 44:2c:05:9e:c9. However, further down I see:

[   17.819113] brcmfmac: brcmf_netdev_set_mac_address Enter, bsscfgidx=0
[   17.819122] brcmfmac: brcmf_fil_iovar_data_set ifidx=0, name=cur_etheraddr, 
len=6
[   17.819127] brcmutil: data
[   17.819135] : aa 3e 81 77 bc 40   .>.w.@
[   17.819864] brcmfmac: brcmf_netdev_set_mac_address updated to 
aa:3e:81:77:bc:40

So the mac address in a local admin variant.


Right, this is likely NetworkManager randomizing the mac for privacy reasons.


Now our firmware has a requirement for the p2p-dev interface that it should be 
different from the mac address of the primary interface, ie. wlp1s0 in this 
log. In brcmfmac we try to do that by setting the local admin bit, but... as it 
is already set we end up using the same mac address hence the -EBUSY.


Ah, that is good to know, so how can we fix this? Can userspace specify a
different mac-address when it asks for the p2p-dev intf to be created? Or
should we do something about this in the kernel?

Regards,

Hans



[   17.947704] brcmfmac: brcmf_cfg80211_add_iface enter: p2p-dev-wlp1s0 type 10
[   17.947714] brcmfmac: brcmf_p2p_add_vif adding vif "p2p-dev-wlp1s0" 
(type=10, addr=00:00:00:00:00:00)
[   17.947716] brcmfmac: brcmf_alloc_vif allocating virtual interface 
(size=3920)
[   17.947720] brcmfmac: brcmf_fil_cmd_int_set ifidx=0, cmd=3, value=1
[   17.948749] brcmfmac: brcmf_fil_iovar_data_set ifidx=0, name=apsta, len=4
[   17.948752] brcmutil: data
[   17.948756] : 01 00 00 00   
[   17.949620] brcmfmac: brcmf_fil_cmd_int_se

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 26-02-18 11:22, Arend van Spriel wrote:

On 2/25/2018 3:52 PM, Hans de Goede wrote:

Hi,

On 26-05-17 12:57, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra
virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>


I'm now seeing this on another device too, but this time the error
thrown is -EBADE, this seems to be new with recent kernels:


Yup. Before we were passing firmware errors up to user-space, which was 
confusing and potentially be misinterpreted. However, looking at the output 
below it would have been good to log the firmware error as well. And staring at 
it some more I suddenly realize I broke the feature detection module with this 
change. Actually only the GSCAN feature detection.


[root@localhost ~]# dmesg | grep brcmfmac
[   34.265950] usbcore: registered new interface driver brcmfmac
[   34.266059] brcmfmac :01:00.0: enabling device ( -> 0002)
[   34.376468] brcmfmac: brcmf_fw_map_chip_to_name: using
brcm/brcmfmac4356-pcie.bin for chip 0x004356(17238) rev 0x02
[   34.855143] brcmfmac :01:00.0: Direct firmware load for
brcm/brcmfmac4356-pcie.clm_blob failed with error -2
[   34.855147] brcmfmac: brcmf_c_process_clm_blob: no clm_blob
available(err=-2), device may have limited channels available
[   34.857029] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0:
Jun  4 2017 16:50:07 version 7.35.101.6 (r702795) FWID 01-5e8eb735
[   34.938854] brcmfmac :01:00.0 wlp1s0: renamed from wlan0
[   37.086420] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   37.086431] brcmfmac: brcmf_cfg80211_add_iface: add iface
p2p-dev-wlp1s0 type 10 failed: err=-52

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac4356-pcie.bin |
tail -n 1
4356a2-roml/pcie-ag-msgbuf-splitrx-p2p-pno-aoe-pktfilter-keepalive-sr-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wl11u-mfp-tdls-amsdutx-sarctrl-proxd-hs20sta-rcc-wepso-ndoe-linkstat-gscan-hchk-logtrace-roamexp-rmon
Version: 7.35.101.6 (r702795) CRC: 4f3f65c5 Date: Sun 2017-06-04
16:51:38 PDT Ucode Ver: 963.316 FWID: 01-5e8eb735

It would be good if we can silence these errors, or maybe at a
minimum lower their log-level from error to warning?


I had a look at it and it seems to be a difference in firmware api that we need 
to support in the driver. Need to do a bit more digging, but it seems an actual 
issue. You could silence it for now, but I prefer to wait for the fix.


Ok, what is the ETA of a fix for this?

Regards,

Hans





---
  .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c    | 14
++
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c |  5 -
  2 files changed, 14 insertions(+), 5 deletions(-)

diff --git
a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index cd1d6730eab7..dae88f3d041d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -684,11 +684,17 @@ static struct wireless_dev
*brcmf_cfg80211_add_iface(struct wiphy *wiphy,
  return ERR_PTR(-EINVAL);
  }
-    if (IS_ERR(wdev))
-    brcmf_err("add iface %s type %d failed: err=%d\n",
-  name, type, (int)PTR_ERR(wdev));
-    else
+    if (IS_ERR(wdev)) {
+    err = PTR_ERR(wdev);
+    if (err != -EBUSY)
+    brcmf_err("add iface %s type %d failed: err=%d\n",
+  name, type, err);
+    else
+    brcmf_dbg(INFO, "add iface %s type %d failed: err=%d\n",
+  name, type, err);
+    } else {
  brcmf_cfg80211_update_proto_addr_mode(wdev);
+    }
  return wdev;
  }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index aa299c47bfa2..1bb296ffb46f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2090,7 +2090,10 @@ static struct wireless_dev
*brcmf_p2p_create_p2pdev(struct brcmf_p2p_info *p2p,
  /* Initialize P2P Discovery in the firmware */
  err = brcmf_fil_iovar_int_set(pri_ifp, "p2p_disc", 1);
  if (err < 0) {
-    brcmf_err("set p2p_disc error\n");
+    if (err != -EBUSY)
+    brcmf_err("set p2p_disc error\n");
+    else
+    brcmf_dbg(INFO, "set p2p_disc error\n");
  brcmf_fweh_p2pdev_setup(pri_ifp, false);
  brcmf_cfg80211_arm_vif_event(p2p->cfg, NULL);
  goto fail;

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 26-05-17 12:57, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>


I'm now seeing this on another device too, but this time the error
thrown is -EBADE, this seems to be new with recent kernels:

[root@localhost ~]# dmesg | grep brcmfmac
[   34.265950] usbcore: registered new interface driver brcmfmac
[   34.266059] brcmfmac :01:00.0: enabling device ( -> 0002)
[   34.376468] brcmfmac: brcmf_fw_map_chip_to_name: using 
brcm/brcmfmac4356-pcie.bin for chip 0x004356(17238) rev 0x02
[   34.855143] brcmfmac :01:00.0: Direct firmware load for 
brcm/brcmfmac4356-pcie.clm_blob failed with error -2
[   34.855147] brcmfmac: brcmf_c_process_clm_blob: no clm_blob 
available(err=-2), device may have limited channels available
[   34.857029] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jun  4 
2017 16:50:07 version 7.35.101.6 (r702795) FWID 01-5e8eb735
[   34.938854] brcmfmac :01:00.0 wlp1s0: renamed from wlan0
[   37.086420] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   37.086431] brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlp1s0 
type 10 failed: err=-52

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac4356-pcie.bin | tail -n 1
4356a2-roml/pcie-ag-msgbuf-splitrx-p2p-pno-aoe-pktfilter-keepalive-sr-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wl11u-mfp-tdls-amsdutx-sarctrl-proxd-hs20sta-rcc-wepso-ndoe-linkstat-gscan-hchk-logtrace-roamexp-rmon
 Version: 7.35.101.6 (r702795) CRC: 4f3f65c5 Date: Sun 2017-06-04 16:51:38 PDT 
Ucode Ver: 963.316 FWID: 01-5e8eb735

It would be good if we can silence these errors, or maybe at a
minimum lower their log-level from error to warning?

Regards,

Hans



---
  .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c| 14 ++
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c |  5 -
  2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index cd1d6730eab7..dae88f3d041d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -684,11 +684,17 @@ static struct wireless_dev 
*brcmf_cfg80211_add_iface(struct wiphy *wiphy,
return ERR_PTR(-EINVAL);
}
  
-	if (IS_ERR(wdev))

-   brcmf_err("add iface %s type %d failed: err=%d\n",
- name, type, (int)PTR_ERR(wdev));
-   else
+   if (IS_ERR(wdev)) {
+   err = PTR_ERR(wdev);
+   if (err != -EBUSY)
+   brcmf_err("add iface %s type %d failed: err=%d\n",
+ name, type, err);
+   else
+   brcmf_dbg(INFO, "add iface %s type %d failed: err=%d\n",
+ name, type, err);
+   } else {
brcmf_cfg80211_update_proto_addr_mode(wdev);
+   }
  
  	return wdev;

  }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index aa299c47bfa2..1bb296ffb46f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2090,7 +2090,10 @@ static struct wireless_dev 
*brcmf_p2p_create_p2pdev(struct brcmf_p2p_info *p2p,
/* Initialize P2P Discovery in the firmware */
err = brcmf_fil_iovar_int_set(pri_ifp, "p2p_disc", 1);
if (err < 0) {
-   brcmf_err("set p2p_disc error\n");
+   if (err != -EBUSY)
+   brcmf_err("set p2p_disc error\n");
+   else
+   brcmf_dbg(INFO, "set p2p_disc error\n");
brcmf_fweh_p2pdev_setup(pri_ifp, false);
brcmf_cfg80211_arm_vif_event(p2p->cfg, NULL);
goto fail;

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Hans de Goede]

Hi,

On 02/20/2018 03:36 PM, Carlo Caione wrote:

On Tue, Feb 20, 2018 at 2:01 PM, Hans de Goede <hdego...@redhat.com> wrote:

Hi Carlo,


Hi Hans,


Is this for devices with a RTL8723BS chip? If so then they
still will not work after this since there also no longer is
a /dev/ttyS4 created for the UART for the bluetooth, instead
you probably want:

https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41

Which also puts the /dev/ttyS4 back in place.


Yeah, this problem came up while working on the RTL8723BS chip but the
driver is also broken for the other two GPS devices supported by this
driver.
Thank you for the patch BTW.


Regards,

Hans

p.s.

My college Jeremy Cline in the Cc is looking into getting proper
bluetooth support in place for the rtl8723bs using serdev binding
and having everything in the kernel, as we now already do for bcm
uart bluetooth modules.


Wasn't also Martin (+CC) working on this? See
https://www.spinics.net/lists/linux-bluetooth/msg73594.html


Ah I did not know that, cool.  Jeremy, this is probably a good starting
point :)   And you should probably coordinate with Martin on this.

Regards,

Hans

Re: [RFC PATCH 0/2] net: rfkill: gpio: Fix and support SerDev

[Hans de Goede]

Hi Carlo,

Is this for devices with a RTL8723BS chip? If so then they
still will not work after this since there also no longer is
a /dev/ttyS4 created for the UART for the bluetooth, instead
you probably want:

https://github.com/jwrdegoede/linux-sunxi/commit/c383dac5ea00738ac01d704d820aa548494fcd41

Which also puts the /dev/ttyS4 back in place.

Regards,

Hans

p.s.

My college Jeremy Cline in the Cc is looking into getting proper
bluetooth support in place for the rtl8723bs using serdev binding
and having everything in the kernel, as we now already do for bcm
uart bluetooth modules.

On 20-02-18 14:46, Carlo Caione wrote:

From: Carlo Caione 

Hi,
this patch came after investigating why the rfkill-gpio driver was failing on
the latest kernel on a machine I have. Sending this out as RFC because I'm
still not sure if this is the right way to approach this problem. I was
honestly expecting not to see the platform devices failing as consequences of
the latest work on SerDev.

Carlo Caione (2):
   net: rfkill: gpio: Fix NULL pointer deference
   net: rfkill: gpio: Convert driver to SerDev

  net/rfkill/Kconfig   |  1 +
  net/rfkill/rfkill-gpio.c | 48 +---
  2 files changed, 26 insertions(+), 23 deletions(-)

[PATCH 3/3] staging: r8188eu: Revert 4 commits breaking ARP

[Hans de Goede]

Commit 2ba8444c97b1 ("staging:r8188eu: move IV/ICV trimming into
decrypt() and also place it after rtl88eu_mon_recv_hook()") breaks ARP.

After this commit ssh-ing to a laptop with r8188eu wifi no longer works
if the machine connecting has never communicated with the laptop before.
This is 100% reproducable using "arp -d  && ssh " to ssh to
a laptop with r8188eu wifi.

This commit reverts 4 commits in total:

1. Commit 79650ffde38e ("staging:r8188eu: trim IV/ICV fields in
   validate_recv_data_frame()")
This commit depends on 2 of the other commits being reverted.

2. Commit 02b19b4c4920 ("staging:r8188eu: inline unprotect_frame() in
   mon_recv_decrypted_recv()")
The inline code is wrong the un-inlined version contains:
if (skb->len < hdr_len + iv_len + icv_len)
return;
...
Where as the inline-ed code introduced by this commit does:
if (skb->len < hdr_len + iv_len + icv_len) {
...
Note the same check, but now to actually continue doing ... instead
of to not do it, so this commit is no good.

3. Commit d86e16da6a5d ("staging:r8188eu: use different mon_recv_decrypted()
   inside rtl88eu_mon_recv_hook() and rtl88eu_mon_xmit_hook().")
This commit introduced a 1:1 copy of a function so that one of the
2 copies can be modified in the 2 commits we're already reverting.

4. Commit 2ba8444c97b1 ("staging:r8188eu: move IV/ICV trimming into
   decrypt() and also place it after rtl88eu_mon_recv_hook()")
This is the commit actually breaking ARP.

Note this commit is a straight-forward squash of the revert of these
4 commits, without any changes.

Cc: sta...@vger.kernel.org
Cc: Ivan Safonov <insafo...@gmail.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/staging/rtl8188eu/core/rtw_recv.c | 83 +++
 drivers/staging/rtl8188eu/os_dep/mon.c| 34 ++---
 2 files changed, 55 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_recv.c 
b/drivers/staging/rtl8188eu/core/rtw_recv.c
index 2f0341689e2f..6506a1587df0 100644
--- a/drivers/staging/rtl8188eu/core/rtw_recv.c
+++ b/drivers/staging/rtl8188eu/core/rtw_recv.c
@@ -264,10 +264,12 @@ static int recvframe_chkmic(struct adapter *adapter,
}
 
/* icv_len included the mic code */
-   datalen = precvframe->pkt->len-prxattrib->hdrlen - 8;
+   datalen = precvframe->pkt->len-prxattrib->hdrlen -
+ prxattrib->iv_len-prxattrib->icv_len-8;
pframe = precvframe->pkt->data;
-   payload = pframe+prxattrib->hdrlen;
+   payload = pframe+prxattrib->hdrlen+prxattrib->iv_len;
 
+   RT_TRACE(_module_rtl871x_recv_c_, _drv_info_, ("\n 
prxattrib->iv_len=%d prxattrib->icv_len=%d\n", prxattrib->iv_len, 
prxattrib->icv_len));
rtw_seccalctkipmic(mickey, pframe, payload, datalen, 
[0],
   (unsigned char)prxattrib->priority); 
/* care the length of the data */
 
@@ -413,15 +415,9 @@ static struct recv_frame *decryptor(struct adapter 
*padapter,
default:
break;
}
-   if (res != _FAIL) {
-   memmove(precv_frame->pkt->data + 
precv_frame->attrib.iv_len, precv_frame->pkt->data, precv_frame->attrib.hdrlen);
-   skb_pull(precv_frame->pkt, precv_frame->attrib.iv_len);
-   skb_trim(precv_frame->pkt, precv_frame->pkt->len - 
precv_frame->attrib.icv_len);
-   }
} else if (prxattrib->bdecrypted == 1 && prxattrib->encrypt > 0 &&
-  (psecuritypriv->busetkipkey == 1 || prxattrib->encrypt != 
_TKIP_)) {
-   psecuritypriv->hw_decrypted = true;
-   }
+  (psecuritypriv->busetkipkey == 1 || prxattrib->encrypt != 
_TKIP_))
+   psecuritypriv->hw_decrypted = true;
 
if (res == _FAIL) {
rtw_free_recvframe(return_packet, 
>recvpriv.free_recv_queue);
@@ -462,7 +458,7 @@ static struct recv_frame *portctrl(struct adapter *adapter,
 
if (auth_alg == 2) {
/* get ether_type */
-   ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE;
+   ptr = ptr + pfhdr->attrib.hdrlen + LLC_HEADER_SIZE + 
pfhdr->attrib.iv_len;
memcpy(_tmp, ptr, 2);
ether_type = ntohs(be_tmp);
 
@@ -1145,8 +1141,6 @@ static int validate_recv_data_frame(struct adapter 
*adapter,
}
 
if (pattrib->privacy) {
-   struct sk_buff *skb = precv_frame->pkt;
-
RT_TRACE(_

[PATCH 1/3] staging: rtl8188eu: Revert part of "staging: rtl8188eu: fix comments with lines over 80 characters"

[Hans de Goede]

Commit 74e1e498e84e ("staging: rtl8188eu: fix comments with lines over 80
characters") not only changed comments but also changed an if check:

-if (pmlmepriv->cur_network.join_res != true) {
+if (!(pmlmepriv->cur_network.join_res)) {

This is not equivalent as join_res is an int and can have values such
as -2 and -3.

Note for the next time, please only make one type of changes in a single
clean-up commit.

Fixes: 74e1e498e84e ("staging: rtl8188eu: fix comments with lines over 80 ...")
Cc: Juliana Rodrigues <juliana.o...@gmail.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/staging/rtl8188eu/core/rtw_ap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c 
b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 32a483769975..fa611455109a 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -754,7 +754,7 @@ static void start_bss_network(struct adapter *padapter, u8 
*pbuf)
}
 
/* setting only at  first time */
-   if (!(pmlmepriv->cur_network.join_res)) {
+   if (pmlmepriv->cur_network.join_res != true) {
/* WEP Key will be set before this function, do not
 * clear CAM.
 */
-- 
2.14.2

[PATCH 2/3] staging: rtl8188eu: Fix bug introduced by convert timers to use timer_setup()

[Hans de Goede]

Commit b7749656e946 ("staging: rtl8188eu: Convert timers to use
timer_setup()") introduces a copy and paste error which causes the
rtl8188eu driver to no longer function. This commit fixes this.

Fixes: b7749656e946 ("staging: rtl8188eu: Convert timers to use timer_setup()")
Cc: Kees Cook <keesc...@chromium.org>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c 
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index d717046a6c2f..d73e9bdc80cc 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -4806,7 +4806,7 @@ void linked_status_chk(struct adapter *padapter)
 void survey_timer_hdl(struct timer_list *t)
 {
struct adapter *padapter = from_timer(padapter, t,
-   mlmeextpriv.link_timer);
+ mlmeextpriv.survey_timer);
struct cmd_obj  *ph2c;
struct sitesurvey_parm  *psurveyPara;
struct cmd_priv *pcmdpriv = 
>cmdpriv;
-- 
2.14.2

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 30-08-17 12:30, Hans de Goede wrote:

Hi,

On 30-08-17 12:24, Hans de Goede wrote:

Hi Arend,

Sorry I was a bit slow to respond to this.

On 04-08-17 14:35, Arend van Spriel wrote:

On 5/26/2017 12:57 PM, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra virtual-if
is a normal thing, do not print an error for this.


Hi Hans,

First of all, sorry! This one is long overdue (thanks for the reminder, Kalle). 
Not sure what the claim is here. I have to check the firmware to see what could 
make this fail. Thing is that wpa_supplicant will try to create the interface 
upon startup and it is really required for P2P operations. Now people not using 
that will probably not care about the failure, but I would like to find out why 
it is failing. wpa_supplicant will not do a retry upon -EBUSY.

With which firmware (target string and version) are you seeing this so I know 
where to dive in.


[root@localhost ~]# dmesg | grep brcm
[   11.252078] brcmutil: loading out-of-tree module taints kernel.
[   11.252159] brcmutil: module verification failed: signature and/or required 
key missing - tainting kernel
[   11.484195] brcmfmac: brcmf_sdio_probe: Loading firmware 
brcm/brcmfmac43430a0-sdio.bin for chip a9a6 rev 
[   11.484290] usbcore: registered new interface driver brcmfmac
[   11.616053] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jun  6 
2014 14:50:39 version 7.10.226.49 (r) FWID 01-8962686a
[   14.782464] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   14.782488] brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlan0 type 
10 failed: err=-16
[   34.300531] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   34.300549] brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlan0 type 
10 failed: err=-16

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac43430a0-sdio.bin | tail 
-n 1
43430a0-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-lpc-wl11u-rcc-fmc-wepso-ccx-okc-fbt-noccxaka-txpwr-ampduhostreorder-clm_43xx_lg
 Version: 7.10.226.49 CRC: bf92cb0b Date: Fri 2014-06-06 14:55:15 KST FWID 
01-8962686a


Since that firmware is quite old, I've tried again with a newer version:

[   11.219863] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jul  1 
2016 18:02:40 version 7.10.1 (A0 Station/P2P feature) FWID 01-bae8afee

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac43430a0-sdio.bin | tail 
-n 1
43430a0-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-lpc-wl11u-rcc-fmc-wepso-ccx-okc-fbt-noccxaka-txpwr-ampduhostreorder-clm_43xx_lg-ndoe
 Version: 7.10.1.244 CRC: 73c82137 Date: Fri 2016-07-01 18:03:15 KST Ucode Ver: 
940.1020 FWID: 01-bae8afee

Which still results in the same errors.

Note this is with the a0 revision of the 43430 for which a firmware file is
still missing from linux-firmware. If you happen to be able to add a file
to linux-firmware while looking into this, that would be great.


Any update on this ?  Since this does not seem to be going anywhere,
can we at least merge the harmless patch silencing the errors ?

Regards,

Hans

[PATCH v2] brcmfmac: Log chip id and revision

[Hans de Goede]

For debugging some problems, it is useful to know the chip revision
add a brcmf_info message logging this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Put the brcmf_info in brcmf_fw_map_chip_to_name() so that it works for
 e.g. pcie devices too
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index d231042f19d6..091b52979e03 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -601,6 +601,9 @@ int brcmf_fw_map_chip_to_name(u32 chip, u32 chiprev,
if ((nvram_name) && (mapping_table[i].nvram))
strlcat(nvram_name, mapping_table[i].nvram, BRCMF_FW_NAME_LEN);
 
+   brcmf_info("using %s for chip %#08x(%d) rev %#08x\n",
+  fw_name, chip, chip, chiprev);
+
return 0;
 }
 
-- 
2.13.4

Re: brcmfmac4356-pci device not seeing 2.4Ghz channel 12 and 13

[Hans de Goede]

Hi,

On 06-07-17 21:43, Arend van Spriel wrote:

On 06-07-17 21:23, Hans de Goede wrote:

Hi,

On 28-06-17 14:35, Arend Van Spriel wrote:

Op 28 jun. 2017 12:07 schreef "Hans de Goede" <hdego...@redhat.com
<mailto:hdego...@redhat.com>>:
  >
  > Hi,
  >
  > I noticed today that my GPD Win (x86 clamshell mini laptop)
  > which uses a brcmfmac4356-pci wifi does not see an APs which
  > is using channel 13.
  >
  > I've tried updating brcmfmac4356-pci.txt changing "ccode=US"
  > to "ccode=EU" and then rebooted, but that does not help.

Some variables may be stored on the device. However, EU may not be
valid. Could you try NL instead?


Yes changing it to NL fixes this. This is still a bit problematic
though. Because what are we going to put in the nvram file we want
to put in linux-firmware ?


Agree. I am surprised (not pleasantly) that these devices are not
properly programmed and thus need nvram for the country code.


  > I believe that the Linux wifi stack is supposed to automatically
  > figure out the country settings based on AP provided info ?

That depends. The device runs its own wifi stack which should have
802.11d support for what you describe.

  > But that does not seem to be working here ? Any hints on howto
  > debug this further would be appreciated.

You could try doing "country" iovar get somewhere in
brcmf_bus_started() to check country setting in firmware.


Let me know if you still want me to do this given that ccode=NL fixes this.


Well. Could you try it with ccode commented out in nvram file.


That does not work, so then I looked at some of my other brcm
nvram files which use:

ccode=XY
ccode=0
ccode=ALL

I tried both 0 and ALL, which both don't work with the brcmfmac4356-pci
controller / firmware.

Then I decided to do a duckduckgo search for this problem and found:

https://wireless.wiki.kernel.org/en/users/Drivers/brcm80211

Which says:

"This generation of chips contain additional regulatory support independent of the 
driver. The devices use a single worldwide regulatory domain, with channels 12-14 (2.4 
GHz band) and channels 52-64 and 100-140 (5 GHz band) restricted to passive operation. 
Transmission on those channels is suppressed until appropriate other traffic is observed 
on those channels. Within the driver, we use the ficticious country code “X2” to 
represent this worldwide regulatory domain."

So I tried:

ccode=X2

And that works :)  So case closed.

I also tried another device which has a brcmfmac43430a0 and the stock
nvram from the OS image from the manufacturer uses:

ccode=ALL

And again no joy for accessing an AP on channel 13, switching to:

ccode=X2

Fixed things there too.

Regards,

Hans

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 30-08-17 12:24, Hans de Goede wrote:

Hi Arend,

Sorry I was a bit slow to respond to this.

On 04-08-17 14:35, Arend van Spriel wrote:

On 5/26/2017 12:57 PM, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra virtual-if
is a normal thing, do not print an error for this.


Hi Hans,

First of all, sorry! This one is long overdue (thanks for the reminder, Kalle). 
Not sure what the claim is here. I have to check the firmware to see what could 
make this fail. Thing is that wpa_supplicant will try to create the interface 
upon startup and it is really required for P2P operations. Now people not using 
that will probably not care about the failure, but I would like to find out why 
it is failing. wpa_supplicant will not do a retry upon -EBUSY.

With which firmware (target string and version) are you seeing this so I know 
where to dive in.


[root@localhost ~]# dmesg | grep brcm
[   11.252078] brcmutil: loading out-of-tree module taints kernel.
[   11.252159] brcmutil: module verification failed: signature and/or required 
key missing - tainting kernel
[   11.484195] brcmfmac: brcmf_sdio_probe: Loading firmware 
brcm/brcmfmac43430a0-sdio.bin for chip a9a6 rev 
[   11.484290] usbcore: registered new interface driver brcmfmac
[   11.616053] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jun  6 
2014 14:50:39 version 7.10.226.49 (r) FWID 01-8962686a
[   14.782464] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   14.782488] brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlan0 type 
10 failed: err=-16
[   34.300531] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   34.300549] brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlan0 type 
10 failed: err=-16

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac43430a0-sdio.bin | tail 
-n 1
43430a0-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-lpc-wl11u-rcc-fmc-wepso-ccx-okc-fbt-noccxaka-txpwr-ampduhostreorder-clm_43xx_lg
 Version: 7.10.226.49 CRC: bf92cb0b Date: Fri 2014-06-06 14:55:15 KST FWID 
01-8962686a


Since that firmware is quite old, I've tried again with a newer version:

[   11.219863] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jul  1 
2016 18:02:40 version 7.10.1 (A0 Station/P2P feature) FWID 01-bae8afee

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac43430a0-sdio.bin | tail 
-n 1
43430a0-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-lpc-wl11u-rcc-fmc-wepso-ccx-okc-fbt-noccxaka-txpwr-ampduhostreorder-clm_43xx_lg-ndoe
 Version: 7.10.1.244 CRC: 73c82137 Date: Fri 2016-07-01 18:03:15 KST Ucode Ver: 
940.1020 FWID: 01-bae8afee

Which still results in the same errors.

Note this is with the a0 revision of the 43430 for which a firmware file is
still missing from linux-firmware. If you happen to be able to add a file
to linux-firmware while looking into this, that would be great.

Regards,

Hans

Re: [PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi Arend,

Sorry I was a bit slow to respond to this.

On 04-08-17 14:35, Arend van Spriel wrote:

On 5/26/2017 12:57 PM, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra virtual-if
is a normal thing, do not print an error for this.


Hi Hans,

First of all, sorry! This one is long overdue (thanks for the reminder, Kalle). 
Not sure what the claim is here. I have to check the firmware to see what could 
make this fail. Thing is that wpa_supplicant will try to create the interface 
upon startup and it is really required for P2P operations. Now people not using 
that will probably not care about the failure, but I would like to find out why 
it is failing. wpa_supplicant will not do a retry upon -EBUSY.

With which firmware (target string and version) are you seeing this so I know 
where to dive in.


[root@localhost ~]# dmesg | grep brcm
[   11.252078] brcmutil: loading out-of-tree module taints kernel.
[   11.252159] brcmutil: module verification failed: signature and/or required 
key missing - tainting kernel
[   11.484195] brcmfmac: brcmf_sdio_probe: Loading firmware 
brcm/brcmfmac43430a0-sdio.bin for chip a9a6 rev 
[   11.484290] usbcore: registered new interface driver brcmfmac
[   11.616053] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jun  6 
2014 14:50:39 version 7.10.226.49 (r) FWID 01-8962686a
[   14.782464] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   14.782488] brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlan0 type 
10 failed: err=-16
[   34.300531] brcmfmac: brcmf_p2p_create_p2pdev: set p2p_disc error
[   34.300549] brcmfmac: brcmf_cfg80211_add_iface: add iface p2p-dev-wlan0 type 
10 failed: err=-16

[root@localhost ~]# strings /lib/firmware/brcm/brcmfmac43430a0-sdio.bin | tail 
-n 1
43430a0-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-lpc-wl11u-rcc-fmc-wepso-ccx-okc-fbt-noccxaka-txpwr-ampduhostreorder-clm_43xx_lg
 Version: 7.10.226.49 CRC: bf92cb0b Date: Fri 2014-06-06 14:55:15 KST FWID 
01-8962686a

Regards,

Hans

Re: [PATCH 2/2] brcmfmac: Use separate firmware for revision 0 of the brcm43430 chip

[Hans de Goede]

Hi,

On 25-08-17 16:49, Jörg Krause wrote:




Thanks for the files!

I've tested with brcmfmac43430a0-sdio.bin.7.10.1.244.ucode940.1020 and
brcmfmac is happy:

brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jul  1 2016
18:02:40 version 7.10.1 (A0 Station/P2P feature) FWID 01-bae8afee

Where did you got the firmware files from?


From various Android x86 images and from:
https://android.googlesource.com/platform/hardware/broadcom/wlan/+/master/bcmdhd/firmware/

Regards,

Hans

Re: [PATCH 2/2] brcmfmac: Use separate firmware for revision 0 of the brcm43430 chip

[Hans de Goede]

Hi,

On 25-08-17 15:55, Jörg Krause wrote:

Hi,

On Fri, 2017-06-16 at 15:14 +0200, Hans de Goede wrote:

The brcm43430 chip needs different firmware files for chip revision 0
and 1. The file currently in linux-firmware is for revision 1 only.

This commit makes brcmfmac request brcmfmac43430a0-sdio.bin instead
of brcmfmac43430-sdio.bin for revision 0 chips.

Note that the behavior for revision 1 chips is not changed, ideally those
would load brcmfmac43430a1-sdio.bin, but that will break existing setups.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 4ed40c94bda9..5125aeeb5310 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -612,7 +612,9 @@ BRCMF_FW_NVRAM_DEF(43340, "brcmfmac43340-sdio.bin", 
"brcmfmac43340-sdio.txt");
  BRCMF_FW_NVRAM_DEF(4335, "brcmfmac4335-sdio.bin", "brcmfmac4335-sdio.txt");
  BRCMF_FW_NVRAM_DEF(43362, "brcmfmac43362-sdio.bin", "brcmfmac43362-sdio.txt");
  BRCMF_FW_NVRAM_DEF(4339, "brcmfmac4339-sdio.bin", "brcmfmac4339-sdio.txt");
-BRCMF_FW_NVRAM_DEF(43430, "brcmfmac43430-sdio.bin", "brcmfmac43430-sdio.txt");
+BRCMF_FW_NVRAM_DEF(43430A0, "brcmfmac43430a0-sdio.bin", 
"brcmfmac43430a0-sdio.txt");
+/* Note the names are not postfixed with a1 for backward compatibility */
+BRCMF_FW_NVRAM_DEF(43430A1, "brcmfmac43430-sdio.bin", 
"brcmfmac43430-sdio.txt");
  BRCMF_FW_NVRAM_DEF(43455, "brcmfmac43455-sdio.bin", "brcmfmac43455-sdio.txt");
  BRCMF_FW_NVRAM_DEF(4354, "brcmfmac4354-sdio.bin", "brcmfmac4354-sdio.txt");
  BRCMF_FW_NVRAM_DEF(4356, "brcmfmac4356-sdio.bin", "brcmfmac4356-sdio.txt");
@@ -630,7 +632,8 @@ static struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = 
{
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4335_CHIP_ID, 0x, 4335),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43362_CHIP_ID, 0xFFFE, 43362),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4339_CHIP_ID, 0x, 4339),
-   BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0x, 43430),
+   BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0x0001, 43430A0),
+   BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0xFFFE, 43430A1),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4345_CHIP_ID, 0xFFC0, 43455),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4354_CHIP_ID, 0x, 4354),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4356_CHIP_ID, 0x, 4356)


Unfortunately, there is no "brcmfmac43430a0-sdio.bin" in linux-
firmware. Therefore, my custom board fails:

# [   24.894241] brcmfmac mmc0:0001:1: Direct firmware load for
brcm/brcmfmac43430a0-sdio.bin failed with error -2
[   25.914322] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (100):
clkctl 0x50
[   26.934186] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (100):
clkctl 0x50

Can someone provide the firmware file, please?


I've put all the versions I've been able to find here:

http://jwrdegoede.danny.cz/brcm-firmware/

Note that the 7.x.y.z version does not seem to have any useful meaning,
higher is not necessarily newer :|

[hans@shalem brcm-firmware]$ for i in brcmfmac43430a0-sdio.bin*; do echo $i && 
strings $i | tail -n 1; done
brcmfmac43430a0-sdio.bin.7.10.1.244.ucode940.1020
43430a0-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-lpc-wl11u-rcc-fmc-wepso-ccx-okc-fbt-noccxaka-txpwr-ampduhostreorder-clm_43xx_lg-ndoe
 Version: 7.10.1.244 CRC: 73c82137 Date: Fri 2016-07-01 18:03:15 KST Ucode Ver: 
940.1020 FWID: 01-bae8afee
brcmfmac43430a0-sdio.bin.7.10.226.49.no_ucode_version
43430a0-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-lpc-wl11u-rcc-fmc-wepso-ccx-okc-fbt-noccxaka-txpwr-ampduhostreorder-clm_43xx_lg
 Version: 7.10.226.49 CRC: bf92cb0b Date: Fri 2014-06-06 14:55:15 KST FWID 
01-8962686a
brcmfmac43430a0-sdio.bin.7.10.68.5.ucode997.0
43430a0-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-ampduhostreorder-lpc-wl11u-rcc-fmc-wepso-ccx-okc-anqpo-ltecx-sr-ndoe
 Version: 7.10.68.5 CRC: 5168922 Date: Thu 2015-04-30 04:20:43 PDT Ucode Ver: 
997.0 FWID: 01-2bde7d77

So date wise brcmfmac43430a0-sdio.bin.7.10.1.244.ucode940.1020 is the newest
and IIRC that one works well for me.

ucode version wise, brcmfmac43430a0-sdio.bin.7.10.68.5.ucode997.0 is newer,
but the .0 in 997.0 makes it feel like an unproven release to me.

Regards,

Hans

Re: [4.13 REGRESSION] Re: brcm43430 sdio wifi regression with 4.13-rc1

[Hans de Goede]

Hi,

On 14-08-17 17:46, Kalle Valo wrote:

Hans de Goede <hdego...@redhat.com> writes:


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index d21258d..def120c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -159,8 +159,9 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)

  brcmf_feat_firmware_capabilities(ifp);
  memset(_cfg, 0, sizeof(gscan_cfg));
-   brcmf_feat_iovar_data_set(ifp, BRCMF_FEAT_GSCAN, "pfn_gscan_cfg",
- _cfg, sizeof(gscan_cfg));
+   if (drvr->bus_if->chip != BRCM_CC_43430_CHIP_ID)
+   brcmf_feat_iovar_data_set(ifp, BRCMF_FEAT_GSCAN,
"pfn_gscan_cfg",
+ _cfg, sizeof(gscan_cfg));
  brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_PNO, "pfn");
  if (drvr->bus_if->wowl_supported)
  brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_WOWL, "wowl");



AFAICT this is still a problem with 4.13-rc5, can we at least get
the above workaround merged for 4.13 ?


Just applied it few hours ago:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=e9bf53ab1ee34bb05c104bbfd2b77c844773f8e6

I'll try to get it to -rc6 but of course it depends on pull schedules if
it makes it or not.


Ok, thank you.

Regards,

Hans

[4.13 REGRESSION] Re: brcm43430 sdio wifi regression with 4.13-rc1

[Hans de Goede]

Hi,

On 23-07-17 09:08, Hans de Goede wrote:

Hi,

On 22-07-17 21:53, Arend van Spriel wrote:

On 22-07-17 21:19, Ian Molton wrote:

On 22/07/17 20:18, Ian Molton wrote:

On 22/07/17 19:43, Hans de Goede wrote:

Hi,

When upgrading my devel environment to 4.13-rc1+ I noticed that
the brcm43430 sdio wifi on a Chuwi Hi8 plus stopped working:


There is a fix for this:

https://patchwork.kernel.org/patch/9836383/


Sorry, ignore me - this was a fix for the other 4.13-rc1 regression.
Arend is looking into he other one. It affects me too.

It appears to be the firmware going astray.


It is still an enigma although admittedly I did not put much time in it
this week. The change below fixes it as the device goes haywire from
this command. At least this was reported by Stefan Wahren ("brcmfmac:
BCM43431 won't get probed on Raspberry Pi Zero W") on linux-wireless
mailing list. Still I can not explain it. Could be that there is not
enough free memory on the device.


As mentioned in my original mail, switching firmware version seems to
fix this. linux-firmware has:

[hans@shalem ~]$ strings 
brcm-firmware/brcmfmac43430-sdio.bin.7.45.41.26.ucode1043.2060 | tail -n1
43430a1-roml/sdio-g-p2p-pool-pno-pktfilter-keepalive-aoe-mchan-tdls-proptxstatus-ampduhostreorder-lpc-sr-bcmcps
 Version: 7.45.41.26 CRC: a75d4f1b Date: Mon 2016-08-29 20:53:22 CEST Ucode 
Ver: 1043.2060 FWID: 01-4527cfab

Where as this one (from the android image on the tablet) does work:

[hans@shalem ~]$ strings 
brcm-firmware/brcmfmac43430-sdio.bin.7.45.77.0.ucode1043.2054 | tail -n1
43430a1-roml/sdio-g-pool-p2p-pno-pktfilter-keepalive-aoe-mchan-proptxstatus-ampduhostreorder-lpc-wl11u-rcc-fmc-wepso-okc-anqpo-11nprop-ndoe-tdls-hs20sta-clm_4335_ss-hwapwar-ivwar-srfast
 Version: 7.45.77.0 CRC: c1a399d4 Date: Wed 2016-03-30 11:31:45 CST Ucode Ver: 
1043.2054 FWID: 01-ee8a6268

Here: https://www.spinics.net/lists/linux-wireless/msg164304.html
you write that the firmware in linux-firmware does not have the
gscan feature, the check for which is causing the issue, enabled,
could it be the other firmware build does have it enabled? It does seem
to have a bunch of extra things enabled. Maybe there simply is an error
in the error-handling in the firmware when it is disabled ?

I've put all firmware versions I have here:

http://jwrdegoede.danny.cz/brcm-firmware/

Regards,

Hans





Regards,
Arend
---
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index d21258d..def120c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -159,8 +159,9 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)

 brcmf_feat_firmware_capabilities(ifp);
 memset(_cfg, 0, sizeof(gscan_cfg));
-   brcmf_feat_iovar_data_set(ifp, BRCMF_FEAT_GSCAN, "pfn_gscan_cfg",
- _cfg, sizeof(gscan_cfg));
+   if (drvr->bus_if->chip != BRCM_CC_43430_CHIP_ID)
+   brcmf_feat_iovar_data_set(ifp, BRCMF_FEAT_GSCAN,
"pfn_gscan_cfg",
+ _cfg, sizeof(gscan_cfg));
 brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_PNO, "pfn");
 if (drvr->bus_if->wowl_supported)
 brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_WOWL, "wowl");



AFAICT this is still a problem with 4.13-rc5, can we at least get
the above workaround merged for 4.13 ?

Regards,

Hans

brcmfmac4356-pcie 4.13 regression (frequent kernel panics) not fixed by recent 4.13 regression fix

[Hans de Goede]

Hi,

I've been seeing frequent kernel panics on wifi activity
(scp-ing a lot of files) with 4.13 on 2 different systems
which both use a brcmfmac4356-pcie wifi chip.

This is with this fix:
https://www.spinics.net/lists/linux-wireless/msg164178.html
already applied.

Here is a picture of the panic:

https://fedorapeople.org/~jwrdegoede/brcmfmac4356-pcie-4.13-panic.jpg

Reverting commit 270a6c1f65fe ("brcmfmac: rework headroom check in 
.start_xmit()"):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=270a6c1f65fe68a28a5d39cd405592c550b496c7

seems to fix this (I can no longer quickly reproduce the panic
by scp-ing a large amount of files).

Note I've first reverted the:
https://www.spinics.net/lists/linux-wireless/msg164178.html
fix so that I could cleanly revert commit 270a6c1f65fe.

Given that no code seems to use the statics commit 270a6c1f65fe
offers and it has been the cause of 2 different regressions now,
it might be best to just revert 270a6c1f65fe for 4.13.

Regards,

Hans

brcm43430 sdio wifi regression with 4.13-rc1

[Hans de Goede]

Hi,

When upgrading my devel environment to 4.13-rc1+ I noticed that
the brcm43430 sdio wifi on a Chuwi Hi8 plus stopped working:

jul 22 14:13:23 localhost.localdomain kernel: brcmfmac: brcmf_sdio_probe: 
Loading firmware brcm/brcmfmac43430-sdio.bin for chip a9a6 rev 0001
jul 22 14:13:23 localhost.localdomain kernel: usbcore: registered new interface 
driver brcmfmac
jul 22 14:13:23 localhost.localdomain kernel: brcmfmac: brcmf_c_preinit_dcmds: 
Firmware version = wl0: Aug 29 2016 20:48:16 version 7.45.41.26 (r640327) FWID 
01-4527cfab
jul 22 14:13:23 localhost.localdomain kernel: brcmfmac: brcmf_sdio_hostmail: 
Unknown mailbox data content: 0x40012
jul 22 14:13:26 localhost.localdomain kernel: brcmfmac: brcmf_sdio_bus_rxctl: 
resumed on timeout
jul 22 14:13:28 localhost.localdomain kernel: brcmfmac: brcmf_sdio_bus_rxctl: 
resumed on timeout
jul 22 14:13:31 localhost.localdomain kernel: brcmfmac: brcmf_sdio_bus_rxctl: 
resumed on timeout


The real culprit here seems to be:

jul 22 14:13:23 localhost.localdomain kernel: brcmfmac: brcmf_sdio_hostmail: 
Unknown mailbox data content: 0x40012


I've tried reverting allmost all changes under 
drivers/net/wireless/broadcom/brcm80211
as well as those under drivers/mmc, but that did not help.

Googling for the "Unknown mailbox data content" error found some Raspberry Pi 3
related comments where users report success with a different firmware version
from: 
https://android.googlesource.com/platform/hardware/broadcom/wlan/+/master/bcmdhd/firmware/bcm4343/

So I tried that as well as a version extracted from the Android install
on the tablet and both work fine, only the "version 7.45.41.26" which
happens to be the one in linux-firmware causes this problem, I've put
all (Linux build, not NDIS) versions I could find here:

http://jwrdegoede.danny.cz/brcm-firmware/

I've the feeling that the 7.45.41.26 version is a bit buggy / suspect to
certain timing problems and 4.13 just happens to trigger those timing
conditions...

russianneuromancer has reported seeing a similar problem with 4.13.

russianneuromancer, can you check if the device you are seeing this with
also has a brcm43430 sdio wifi and if so if switching to:

http://jwrdegoede.danny.cz/brcm-firmware/brcmfmac43430-sdio.bin.7.45.77.0.ucode1043.2054

Fixes this ?

Regards,

Hans

Re: brcmfmac4356-pci device not seeing 2.4Ghz channel 12 and 13

[Hans de Goede]

Hi,

On 28-06-17 14:35, Arend Van Spriel wrote:

Op 28 jun. 2017 12:07 schreef "Hans de Goede" <hdego...@redhat.com 
<mailto:hdego...@redhat.com>>:
 >
 > Hi,
 >
 > I noticed today that my GPD Win (x86 clamshell mini laptop)
 > which uses a brcmfmac4356-pci wifi does not see an APs which
 > is using channel 13.
 >
 > I've tried updating brcmfmac4356-pci.txt changing "ccode=US"
 > to "ccode=EU" and then rebooted, but that does not help.

Some variables may be stored on the device. However, EU may not be valid. Could 
you try NL instead?


Yes changing it to NL fixes this. This is still a bit problematic
though. Because what are we going to put in the nvram file we want
to put in linux-firmware ?


 > I believe that the Linux wifi stack is supposed to automatically
 > figure out the country settings based on AP provided info ?

That depends. The device runs its own wifi stack which should have 802.11d 
support for what you describe.

 > But that does not seem to be working here ? Any hints on howto
 > debug this further would be appreciated.

You could try doing "country" iovar get somewhere in brcmf_bus_started() to 
check country setting in firmware.


Let me know if you still want me to do this given that ccode=NL fixes this.

Regards,

Hans

brcmfmac4356-pci device not seeing 2.4Ghz channel 12 and 13

[Hans de Goede]

Hi,

I noticed today that my GPD Win (x86 clamshell mini laptop)
which uses a brcmfmac4356-pci wifi does not see an APs which
is using channel 13.

I've tried updating brcmfmac4356-pci.txt changing "ccode=US"
to "ccode=EU" and then rebooted, but that does not help.

I believe that the Linux wifi stack is supposed to automatically
figure out the country settings based on AP provided info ?

But that does not seem to be working here ? Any hints on howto
debug this further would be appreciated.

Regards,

Hans

[PATCH 1/2] brcmfmac: Log chip id and revision

[Hans de Goede]

For debugging some problems, it is useful to know the chip revision
add a brcmf_info message logging this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index e03450059b06..4ed40c94bda9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4214,6 +4214,8 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev 
*sdiodev)
if (ret)
goto fail;
 
+   brcmf_info("Loading firmware %s for chip %08x rev %08x\n",
+  sdiodev->fw_name, bus->ci->chip, bus->ci->chiprev);
ret = brcmf_fw_get_firmwares(sdiodev->dev, BRCMF_FW_REQUEST_NVRAM,
 sdiodev->fw_name, sdiodev->nvram_name,
 brcmf_sdio_firmware_callback);
-- 
2.13.0

[PATCH 2/2] brcmfmac: Use separate firmware for revision 0 of the brcm43430 chip

[Hans de Goede]

The brcm43430 chip needs different firmware files for chip revision 0
and 1. The file currently in linux-firmware is for revision 1 only.

This commit makes brcmfmac request brcmfmac43430a0-sdio.bin instead
of brcmfmac43430-sdio.bin for revision 0 chips.

Note that the behavior for revision 1 chips is not changed, ideally those
would load brcmfmac43430a1-sdio.bin, but that will break existing setups.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 4ed40c94bda9..5125aeeb5310 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -612,7 +612,9 @@ BRCMF_FW_NVRAM_DEF(43340, "brcmfmac43340-sdio.bin", 
"brcmfmac43340-sdio.txt");
 BRCMF_FW_NVRAM_DEF(4335, "brcmfmac4335-sdio.bin", "brcmfmac4335-sdio.txt");
 BRCMF_FW_NVRAM_DEF(43362, "brcmfmac43362-sdio.bin", "brcmfmac43362-sdio.txt");
 BRCMF_FW_NVRAM_DEF(4339, "brcmfmac4339-sdio.bin", "brcmfmac4339-sdio.txt");
-BRCMF_FW_NVRAM_DEF(43430, "brcmfmac43430-sdio.bin", "brcmfmac43430-sdio.txt");
+BRCMF_FW_NVRAM_DEF(43430A0, "brcmfmac43430a0-sdio.bin", 
"brcmfmac43430a0-sdio.txt");
+/* Note the names are not postfixed with a1 for backward compatibility */
+BRCMF_FW_NVRAM_DEF(43430A1, "brcmfmac43430-sdio.bin", 
"brcmfmac43430-sdio.txt");
 BRCMF_FW_NVRAM_DEF(43455, "brcmfmac43455-sdio.bin", "brcmfmac43455-sdio.txt");
 BRCMF_FW_NVRAM_DEF(4354, "brcmfmac4354-sdio.bin", "brcmfmac4354-sdio.txt");
 BRCMF_FW_NVRAM_DEF(4356, "brcmfmac4356-sdio.bin", "brcmfmac4356-sdio.txt");
@@ -630,7 +632,8 @@ static struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = 
{
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4335_CHIP_ID, 0x, 4335),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43362_CHIP_ID, 0xFFFE, 43362),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4339_CHIP_ID, 0x, 4339),
-   BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0x, 43430),
+   BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0x0001, 43430A0),
+   BRCMF_FW_NVRAM_ENTRY(BRCM_CC_43430_CHIP_ID, 0xFFFE, 43430A1),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4345_CHIP_ID, 0xFFC0, 43455),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4354_CHIP_ID, 0x, 4354),
BRCMF_FW_NVRAM_ENTRY(BRCM_CC_4356_CHIP_ID, 0x, 4356)
-- 
2.13.0

Re: [PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Hans de Goede]

Hi,

On 26-05-17 13:15, Kalle Valo wrote:

Hans de Goede <hdego...@redhat.com> writes:


From: Arend Van Spriel <arend.vanspr...@broadcom.com>


Ah I see I set the Author to Arend when I added this to my
tree a while back, that is fine as he did all the work
for this one. I was under the impression Arend would submit
this himself, but since I did not see a submission yet
I decided to go ahead and submit this.


This fixes the following errors showing up in dmesg:

[   32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8
[   32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8
[   33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple of 8

Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA")
Suggested-by: Arend van Spriel <arend.vanspr...@broadcom.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>


I'll queue this to 4.12.


Thank you, given that Arend is set as the Author you can add my:

Tested-by: Hans de Goede <hdego...@redhat.com>

And maybe drop the Suggested-by: Arend van Spriel ?

Regards,

Hans

[PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Hans de Goede]

From: Arend Van Spriel <arend.vanspr...@broadcom.com>

This fixes the following errors showing up in dmesg:

[   32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8
[   32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8
[   33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple of 8

Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA")
Suggested-by: Arend van Spriel <arend.vanspr...@broadcom.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index fc64b8913aa6..e03450059b06 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3422,7 +3422,7 @@ static int brcmf_sdio_bus_preinit(struct device *dev)
/* otherwise, set txglomalign */
value = sdiodev->settings->bus.sdio.sd_sgentry_align;
/* SDIO ADMA requires at least 32 bit alignment */
-   value = max_t(u32, value, 4);
+   value = max_t(u32, value, ALIGNMENT);
err = brcmf_iovar_data_set(dev, "bus:txglomalign", ,
   sizeof(u32));
}
-- 
2.13.0

[PATCH resend] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

The firmware responding with -EBUSY when trying to add an extra virtual-if
is a normal thing, do not print an error for this.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c| 14 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c |  5 -
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index cd1d6730eab7..dae88f3d041d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -684,11 +684,17 @@ static struct wireless_dev 
*brcmf_cfg80211_add_iface(struct wiphy *wiphy,
return ERR_PTR(-EINVAL);
}
 
-   if (IS_ERR(wdev))
-   brcmf_err("add iface %s type %d failed: err=%d\n",
- name, type, (int)PTR_ERR(wdev));
-   else
+   if (IS_ERR(wdev)) {
+   err = PTR_ERR(wdev);
+   if (err != -EBUSY)
+   brcmf_err("add iface %s type %d failed: err=%d\n",
+ name, type, err);
+   else
+   brcmf_dbg(INFO, "add iface %s type %d failed: err=%d\n",
+ name, type, err);
+   } else {
brcmf_cfg80211_update_proto_addr_mode(wdev);
+   }
 
return wdev;
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index aa299c47bfa2..1bb296ffb46f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2090,7 +2090,10 @@ static struct wireless_dev 
*brcmf_p2p_create_p2pdev(struct brcmf_p2p_info *p2p,
/* Initialize P2P Discovery in the firmware */
err = brcmf_fil_iovar_int_set(pri_ifp, "p2p_disc", 1);
if (err < 0) {
-   brcmf_err("set p2p_disc error\n");
+   if (err != -EBUSY)
+   brcmf_err("set p2p_disc error\n");
+   else
+   brcmf_dbg(INFO, "set p2p_disc error\n");
brcmf_fweh_p2pdev_setup(pri_ifp, false);
brcmf_cfg80211_arm_vif_event(p2p->cfg, NULL);
goto fail;
-- 
2.13.0

[PATCH resend 0/1] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

This is a resend of a patch which I send a while ago, back then there
was some discussion to come up with a better fix, but that has not lead
to anything and the driver is still periodically spamming the logs.

As such I would like to move forward with this patch for now, which
makes no functional changes other then changing the messages in question
into debug messages when the error is -EBUSY.

Regards,

Hans

Re: New brcmfmac errors in 4.12: brcmf_sdio_rxglom: sublen ... not multiple of 8

[Hans de Goede]

Hi,

On 14-05-17 10:21, Arend Van Spriel wrote:

On 13-5-2017 16:55, Hans de Goede wrote:

Hi,

On 13-05-17 15:39, Heiner Kallweit wrote:

Am 13.05.2017 um 14:35 schrieb Hans de Goede:



Hi,

On 13-05-17 14:19, Hans de Goede wrote:

Hi,

I've just rebased my personal kernel tree to what will soon be 4.12-rc1
and I'm getting my dmesg log filled with the following errors:

[   32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple
of 8
[   32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8
[   33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple
of 8


After a brief look at the code I'm not sure that the check actually
checks
for an error condition. Apart from the error messages:
Do you face issues with the functionality of the driver?


Yes after a while I get -ETIMEOUT errors for any sdio transfers
to the device. But I'm not sure if this is caused by this commit,
I think I've seen this once with 4.11 too.

I've reverted the commit for now, but I'm fine with instead of
doing the revert dropping the error check if the brcmfmac developers
think that is ok. Currently the ETIMEOUT seems to be gone, so
if dropping the revert causes it to re-appear then we know more.


Instead of reverting please give this patch a try and let me know if it
works for you.

Regards,
Arend
---
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
b/drivers/n
index fc64b89..e034500 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3422,7 +3422,7 @@ static int brcmf_sdio_bus_preinit(struct device *dev)
 /* otherwise, set txglomalign */
 value = sdiodev->settings->bus.sdio.sd_sgentry_align;
 /* SDIO ADMA requires at least 32 bit alignment */
-   value = max_t(u32, value, 4);
+   value = max_t(u32, value, ALIGNMENT);
 err = brcmf_iovar_data_set(dev, "bus:txglomalign", ,
sizeof(u32));
 }



I can confirm that the above fix fixes the messages.

Regards,

Hans

Re: New brcmfmac errors in 4.12: brcmf_sdio_rxglom: sublen ... not multiple of 8

[Hans de Goede]

Hi,

On 13-05-17 15:39, Heiner Kallweit wrote:

Am 13.05.2017 um 14:35 schrieb Hans de Goede:



Hi,

On 13-05-17 14:19, Hans de Goede wrote:

Hi,

I've just rebased my personal kernel tree to what will soon be 4.12-rc1
and I'm getting my dmesg log filled with the following errors:

[   32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8
[   32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8
[   33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple of 8


After a brief look at the code I'm not sure that the check actually checks
for an error condition. Apart from the error messages:
Do you face issues with the functionality of the driver?


Yes after a while I get -ETIMEOUT errors for any sdio transfers
to the device. But I'm not sure if this is caused by this commit,
I think I've seen this once with 4.11 too.

I've reverted the commit for now, but I'm fine with instead of
doing the revert dropping the error check if the brcmfmac developers
think that is ok. Currently the ETIMEOUT seems to be gone, so
if dropping the revert causes it to re-appear then we know more.

Regards,

Hans

Re: New brcmfmac errors in 4.12: brcmf_sdio_rxglom: sublen ... not multiple of 8

[Hans de Goede]

Hi,

On 13-05-17 14:19, Hans de Goede wrote:

Hi,

I've just rebased my personal kernel tree to what will soon be 4.12-rc1
and I'm getting my dmesg log filled with the following errors:

[   32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8
[   32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8
[   33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple of 8

Other brcmfmac dmesg messages:

[   16.020013] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jul 17 
2013 07:36:07 version 6.10.197.71 (r412987) FWID 01-882d2634

This is on a Asus T100TA which uses a 43241b1


Ok, this seems to be a regression caused by:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6e84ab604bded
"brcmfmac: properly align buffers on certain platforms with 64 bit DMA"

I believe this may need a different fix then that.

Regards,

Hans

Re: New brcmfmac errors in 4.12: brcmf_sdio_rxglom: sublen ... not multiple of 8

[Hans de Goede]

Hi,

On 13-05-17 14:19, Hans de Goede wrote:

Hi,

I've just rebased my personal kernel tree to what will soon be 4.12-rc1
and I'm getting my dmesg log filled with the following errors:

[   32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8
[   32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8
[   33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple of 8

Other brcmfmac dmesg messages:

[   16.020013] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jul 17 
2013 07:36:07 version 6.10.197.71 (r412987) FWID 01-882d2634

This is on a Asus T100TA which uses a 43241b1


Ok, this seems to be a regression caused by:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6e84ab604bded
"brcmfmac: properly align buffers on certain platforms with 64 bit DMA"

I believe this may need a different fix then that.

Regards,

Hans

New brcmfmac errors in 4.12: brcmf_sdio_rxglom: sublen ... not multiple of 8

[Hans de Goede]

Hi,

I've just rebased my personal kernel tree to what will soon be 4.12-rc1
and I'm getting my dmesg log filled with the following errors:

[   32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8
[   32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8
[   33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple of 8

Other brcmfmac dmesg messages:

[   16.020013] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Jul 17 
2013 07:36:07 version 6.10.197.71 (r412987) FWID 01-882d2634

This is on a Asus T100TA which uses a 43241b1

Regards,

Hans

Re: [PATCH v2] staging: rtl8723bs: remove re-positioned call to kfree in os_dep/ioctl_cfg80211.c

[Hans de Goede]

Hi,

On 28-04-17 00:50, Ian W MORRISON wrote:

This patch is to remove the re-positioned call to kfree() in
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c which otherwise
results in segmentation fault.

Signed-off-by: Ian W Morrison <linux...@linuxium.com.au>


Patch looks good to me:

Reviewed-by: Hans de Goede <hdego...@redhat.com>

Regards,

Hans



---
  drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index f17f4fb..2ee9df5 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -3527,7 +3527,6 @@ int rtw_wdev_alloc(struct adapter *padapter,
struct device *dev)
 pwdev_priv->power_mgmt = true;
 else
 pwdev_priv->power_mgmt = false;
-   kfree((u8 *)wdev);

 return ret;

Re: brcmfmac: don't warn user if requested nvram fails

[Hans de Goede]

Hi,

On 27-04-17 02:36, Luis R. Rodriguez wrote:

On Tue, Apr 11, 2017 at 10:53:57AM +0200, Hans de Goede wrote:





Right, sorry. For the pcie device I'm looking at the
name is brcmfmac4356-pcie.txt and I would like to propose
to first check for "brcmfmac4356--.txt"


So who is going to provide these nvram files. We can not
maintain that as there are too many variants and they are under control
of the OEM/ODM.


Users / people like me who are interested in using certain
devices with Linux. The idea is to at least make it possible to
have these devices just work. E.g. I would like a user to be
able to insert a USB-stick with a live Fedora 27 and then
have everything just work on the GPD win.

To make this happen I will submit the nvram file from the
Windows install on the GPDwin to linux-firmware as
"brcmfmac4356--.txt"
and yes I've checked that there are sensible values in
the subsys ids.


I suppose the "nvram file from the Windows install" than has a
redistributable license?


IANAL but I fail to see how the contents of this file is
anything but functional and as such not copyright-able.


We take licensing serious on linux-firmware, a IANAL is no excuse for
being sloppy.


I don't mean to be sloppy. As I already stated I plan to make it
clear in the commit msg that there is no license info for the nvram
file and that in my non expert opinion that is not a problem.

If people disagree then we will likely need to ask an actual lawyer
for advice and see from there,

Regards,

Hans

[PATCH] staging: rtl8188eu: force driver to be built as a module

[Hans de Goede]

The rtl8188eu driver defines a ton of global symbols which tend to
conflict with other realtek wifi drivers, force it to be built as
a module.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/staging/rtl8188eu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/rtl8188eu/Kconfig 
b/drivers/staging/rtl8188eu/Kconfig
index 94f3879..cb836c5 100644
--- a/drivers/staging/rtl8188eu/Kconfig
+++ b/drivers/staging/rtl8188eu/Kconfig
@@ -1,6 +1,7 @@
 config R8188EU
tristate "Realtek RTL8188EU Wireless LAN NIC driver"
depends on WLAN && USB && CFG80211
+   depends on m
select WIRELESS_EXT
select WEXT_PRIV
---help---
-- 
2.9.3

[PATCH] staging: rtl8723bs: Add missing include to fix compile error

[Hans de Goede]

As reported by Stephen Rothwell when merging staging-next,
drivers/staging/rtl8723bs/core/rtw_ieee80211.c does not compile due
to of_get_property not being declared.

Explicitly include  to fix this compile error.

Cc: Stephen Rothwell <s...@canb.auug.org.au>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c 
b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
index af44a8c..74f5ee0 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
@@ -16,7 +16,7 @@
 
 #include 
 #include 
-
+#include 
 
 u8 RTW_WPA_OUI_TYPE[] = { 0x00, 0x50, 0xf2, 1 };
 u16 RTW_WPA_VERSION = 1;
-- 
2.9.3

Re: [PATCH] net: rfkill: gpio: Add OBDA8723 ACPI HID

[Hans de Goede]

Hi,

On 09-04-17 10:01, Marcel Holtmann wrote:

Hi Hans,


The OBDA8723 ACPI HID is used on quite a few Bay Trail based tablets
for bluetooth rfkill functionality.

Tested-by: russianneuroman...@ya.ru <russianneuroman...@ya.ru>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
net/rfkill/rfkill-gpio.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 76c01cb..50ca65e 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -163,6 +163,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM4752", RFKILL_TYPE_GPS },
{ "LNV4752", RFKILL_TYPE_GPS },
+   { "OBDA8723", RFKILL_TYPE_BLUETOOTH },
{ },
};


NAK. We are integrating these with hci_bcm.c or hci_intel.c drivers.


This is for the bluetooth side of the rtl8723bs driver which recently
(yesterday) got merged in into drivers/staging. Which still needs
hciattach from userspace. I completely agree that eventually we should
fix that. In the mean time it would be nice if we could carry this
one line patch to give people using the staging driver working bluetooth.

Regards,

Hans

[PATCH] net: rfkill: gpio: Add OBDA8723 ACPI HID

[Hans de Goede]

The OBDA8723 ACPI HID is used on quite a few Bay Trail based tablets
for bluetooth rfkill functionality.

Tested-by: russianneuroman...@ya.ru <russianneuroman...@ya.ru>
Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 net/rfkill/rfkill-gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 76c01cb..50ca65e 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -163,6 +163,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
 static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM4752", RFKILL_TYPE_GPS },
{ "LNV4752", RFKILL_TYPE_GPS },
+   { "OBDA8723", RFKILL_TYPE_BLUETOOTH },
{ },
 };
 MODULE_DEVICE_TABLE(acpi, rfkill_acpi_match);
-- 
2.9.3

Re: brcmfmac: don't warn user if requested nvram fails

[Hans de Goede]

Hi,

On 07-04-17 23:43, Arend Van Spriel wrote:



On 6-4-2017 14:14, Hans de Goede wrote:

Hi,

I noticed your patch-series on the lwn.net kernel page,
and I took a peek :)

I don't think that this patch:

https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/commit/?h=20170329-driver-data-v2-try3=3968dd3031d1ff7e7be4acfb810948c70c2d4490


Is a good idea, specifically the "do not warn" part,
although the brcmfmac driver will indeed try to continue
without a nvram file, in my experience it almost all
the time will not work properly without the nvram file.


Actually, brcmfmac will only continue without nvram for PCIe devices.
For SDIO it is a different story, which may be the kind of devices you
have experienced.


Ah, no I've experience with both now, and the device I've
with a PCIE which needs nvram:

http://www.gpd.hk/gpdwin.asp

Will not work without the nvram file, so I really think
we should at least keep a warning msg here.


Arend, should we maybe just make the missing nvram file
a fatal error ?

###

While on the subject of nvram file loading, I want to make
some changes to how brcmfmac does this, for pcie
devices I want it to first try:

brcmfmac-4xxx-sdio--.txt
and only if that is not present fallback to
brcmfmac-4xxx-sdio.txt, so that we can include the
brcmfmac-4xxx-sdio--.txt
in the linux-firmware repo and have things just work.


You got me confused. I suppose the -sdio- should not be in the filename
examples above.


Right, sorry. For the pcie device I'm looking at the
name is brcmfmac4356-pcie.txt and I would like to propose
to first check for "brcmfmac4356--.txt"


So who is going to provide these nvram files. We can not
maintain that as there are too many variants and they are under control
of the OEM/ODM.


Users / people like me who are interested in using certain
devices with Linux. The idea is to at least make it possible to
have these devices just work. E.g. I would like a user to be
able to insert a USB-stick with a live Fedora 27 and then
have everything just work on the GPD win.

To make this happen I will submit the nvram file from the
Windows install on the GPDwin to linux-firmware as
"brcmfmac4356--.txt"
and yes I've checked that there are sensible values in
the subsys ids.

So would you be willing to accept a brcmfmac patch
trying such a post-fixed nvram filename first ?


Likewise for sdio devices on devicetree platforms
first try brcmfmac-4xxx-sdio-.txt.

And for sdio devices on ACPI/x86 systems I want to
do something similar too.

Luis, do you think it would be a good idea to add
.optional_postfix member to driver_data_req_params
for this? I believe other sdio based wifi devices
may want to do the same, or should this be handled
in the driver by doing 2 driver_data_request_async
calls (the 2nd when the 1st fails) ?


I think we briefly touched this subject a while ago.


Correct back then I wanted to achieve the same goals
(and I still do) for some ARM boards. What I'm striving
for here is "it just works" user experience when
people use a new enough distro which has all the
bits in place.

> It would be great

if the driver_data api could be given a list/array of files which can be
flagged as required or optional. Just not sure how to deal with drivers
that request a firmware file and fallback to requesting older ones
repeatedly thus stop processing upon first successful load. Both
use-cases seem present in drivers.


Regards,

Hans

Re: brcmfmac: don't warn user if requested nvram fails

[Hans de Goede]

Hi,

I noticed your patch-series on the lwn.net kernel page,
and I took a peek :)

I don't think that this patch:

https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/commit/?h=20170329-driver-data-v2-try3=3968dd3031d1ff7e7be4acfb810948c70c2d4490

Is a good idea, specifically the "do not warn" part,
although the brcmfmac driver will indeed try to continue
without a nvram file, in my experience it almost all
the time will not work properly without the nvram file.

Arend, should we maybe just make the missing nvram file
a fatal error ?

###

While on the subject of nvram file loading, I want to make
some changes to how brcmfmac does this, for pcie
devices I want it to first try:

brcmfmac-4xxx-sdio--.txt
and only if that is not present fallback to
brcmfmac-4xxx-sdio.txt, so that we can include the
brcmfmac-4xxx-sdio--.txt
in the linux-firmware repo and have things just work.

Likewise for sdio devices on devicetree platforms
first try brcmfmac-4xxx-sdio-.txt.

And for sdio devices on ACPI/x86 systems I want to
do something similar too.

Luis, do you think it would be a good idea to add
.optional_postfix member to driver_data_req_params
for this? I believe other sdio based wifi devices
may want to do the same, or should this be handled
in the driver by doing 2 driver_data_request_async
calls (the 2nd when the 1st fails) ?

Regards,

Hans

Re: [PATCH] staging: Add rtl8723bs sdio wifi driver

[Hans de Goede]

Hi,

On 06-04-17 11:04, Bastien Nocera wrote:

On Thu, 2017-04-06 at 08:55 +0200, Hans de Goede wrote:





Good thank you. So what is the plan with the github version ?

Note that my submission contains a few small fixes on top of
the github version, for which I intended to submit a pull-req
but I've not gotten around to that yet, I've done so now:

https://github.com/hadess/rtl8723bs/pull/125

But do we want to keep maintaining the github version (for a while
at least) I wonder, as that does mean double work?


My plan is to remove everything and point to the upstream tree as soon
as the support has landed in Linus' tree.

While there is some use in keeping it around for older kernels, we keep
getting asked to support even older kernels than reasonable, or have
users complaining about non-working machines once they use the driver,
which simply uncovers kernel bugs that were fixed upstream.

I don't think it's a good use of our time to carry on supporting this
out-of-tree. I'll however make sure to try and document the migration
in a way that's helpful to users.


Ok, that works for me.

Regards,

Hans

Re: [PATCH] staging: Add rtl8723bs sdio wifi driver

[Hans de Goede]

Hi,

On 05-04-17 18:32, Larry Finger wrote:

On 04/05/2017 04:36 AM, Hans de Goede wrote:

Hi,

On 05-04-17 01:41, Larry Finger wrote:

On 04/04/2017 04:53 PM, Hans de Goede wrote:

Hi,

On 04/04/2017 11:38 PM, Arend Van Spriel wrote:



On 4-4-2017 20:53, Hans de Goede wrote:

Hi,

On 04/04/2017 08:31 PM, Larry Finger wrote:

On 03/29/2017 12:47 PM, Hans de Goede wrote:

The rtl8723bs is found on quite a few systems used by Linux users,
such as on Atom systems (Intel Computestick and various other
Atom based devices) and on many (budget) ARM boards such as
the CHIP.

The plan moving forward with this is for the new clean,
written from scratch, rtl8xxxu driver to eventually gain
support for sdio devices. But there is no clear timeline
for that, so lets add this driver included in staging for now.


Hans,

I started looking at the Smatch errors. This one may be the result of
a serious problem:

  CHECK   drivers/staging/rtl8723bs/core/rtw_debug.c
drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info()
error: we previously assumed 'phead' could be null (see line 453)
drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info()
warn: variable dereferenced before check 'phead' (see line 454)

A snippet of the code in question is as follows:

spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
phead = get_list_head(queue);
plist = phead ? get_next(phead) : NULL;
plist = get_next(phead);
if ((!phead) || (!plist)) {
spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
return 0;
}

This code comes directly from the hadess repo, but I am suspicious of
the double call to get_next(phead). I cannot imagine any valid reason
to skip every other entry on that list.


If you look closer and simplify the first getnext line what is written is:

 plist = get_next(phead);
 plist = get_next(phead);

Which indeed looks like it could use improvement, but I don't
think it is seriously broken.


So get_list_head(queue) can never return NULL?


I don't know.


Otherwise I don't know
how close I need to get for a simplified look ;-)


I simplified things so as to make clear that Larry's worry was
not really a problem, I do agree the smatch complaint is valid.


I think the following fixes the problem:

diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c
b/drivers/staging/rtl8723bs/core/rtw_debug.c
index d34747b29309..51cef55d3f76 100644
--- a/drivers/staging/rtl8723bs/core/rtw_debug.c
+++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
@@ -451,7 +451,6 @@ int proc_get_survey_info(struct seq_file *m, void *v)
spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
phead = get_list_head(queue);
plist = phead ? get_next(phead) : NULL;
-   plist = get_next(phead);
if ((!phead) || (!plist)) {
spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
return 0;

Pointer plist is set in the 3rd line, thus the second set of it can be removed.


Agreed, when I've some time I plan to do a follow-up patch
to my initial submission fixing all the Smatch errors. But feel
free to beat me to it.

Greg, if I understood you correctly you plan to merge my initial
submission as is and then we can fix this (and other things) with
follow up patches, right ?


Hans,

I took GregKH out of the Cc list when I started the discussion of the Smatch 
errors/warnings, and he probably has not seen the recent E-mails in this thread.


Ah I missed that (clearly I did).


It it was my understanding that he planned to apply your submission in the form 
you sent.


That is my understanding too.


I have several patches nearly ready to fix most of the Smatch warnings and 
errors. I will send them as soon as the original submission is applied.


Good thank you. So what is the plan with the github version ?

Note that my submission contains a few small fixes on top of
the github version, for which I intended to submit a pull-req
but I've not gotten around to that yet, I've done so now:

https://github.com/hadess/rtl8723bs/pull/125

But do we want to keep maintaining the github version (for a while
at least) I wonder, as that does mean double work?

Regards,

Hans

Re: [PATCH] staging: Add rtl8723bs sdio wifi driver

[Hans de Goede]

Hi,

On 05-04-17 01:41, Larry Finger wrote:

On 04/04/2017 04:53 PM, Hans de Goede wrote:

Hi,

On 04/04/2017 11:38 PM, Arend Van Spriel wrote:



On 4-4-2017 20:53, Hans de Goede wrote:

Hi,

On 04/04/2017 08:31 PM, Larry Finger wrote:

On 03/29/2017 12:47 PM, Hans de Goede wrote:

The rtl8723bs is found on quite a few systems used by Linux users,
such as on Atom systems (Intel Computestick and various other
Atom based devices) and on many (budget) ARM boards such as
the CHIP.

The plan moving forward with this is for the new clean,
written from scratch, rtl8xxxu driver to eventually gain
support for sdio devices. But there is no clear timeline
for that, so lets add this driver included in staging for now.


Hans,

I started looking at the Smatch errors. This one may be the result of
a serious problem:

  CHECK   drivers/staging/rtl8723bs/core/rtw_debug.c
drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info()
error: we previously assumed 'phead' could be null (see line 453)
drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info()
warn: variable dereferenced before check 'phead' (see line 454)

A snippet of the code in question is as follows:

spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
phead = get_list_head(queue);
plist = phead ? get_next(phead) : NULL;
plist = get_next(phead);
if ((!phead) || (!plist)) {
spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
return 0;
}

This code comes directly from the hadess repo, but I am suspicious of
the double call to get_next(phead). I cannot imagine any valid reason
to skip every other entry on that list.


If you look closer and simplify the first getnext line what is written is:

 plist = get_next(phead);
 plist = get_next(phead);

Which indeed looks like it could use improvement, but I don't
think it is seriously broken.


So get_list_head(queue) can never return NULL?


I don't know.


Otherwise I don't know
how close I need to get for a simplified look ;-)


I simplified things so as to make clear that Larry's worry was
not really a problem, I do agree the smatch complaint is valid.


I think the following fixes the problem:

diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c 
b/drivers/staging/rtl8723bs/core/rtw_debug.c
index d34747b29309..51cef55d3f76 100644
--- a/drivers/staging/rtl8723bs/core/rtw_debug.c
+++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
@@ -451,7 +451,6 @@ int proc_get_survey_info(struct seq_file *m, void *v)
spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
phead = get_list_head(queue);
plist = phead ? get_next(phead) : NULL;
-   plist = get_next(phead);
if ((!phead) || (!plist)) {
spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
return 0;

Pointer plist is set in the 3rd line, thus the second set of it can be removed.


Agreed, when I've some time I plan to do a follow-up patch
to my initial submission fixing all the Smatch errors. But feel
free to beat me to it.

Greg, if I understood you correctly you plan to merge my initial
submission as is and then we can fix this (and other things) with
follow up patches, right ?

Regards,

Hans

Re: [PATCH] staging: Add rtl8723bs sdio wifi driver

[Hans de Goede]

Hi,

On 04/04/2017 11:38 PM, Arend Van Spriel wrote:



On 4-4-2017 20:53, Hans de Goede wrote:

Hi,

On 04/04/2017 08:31 PM, Larry Finger wrote:

On 03/29/2017 12:47 PM, Hans de Goede wrote:

The rtl8723bs is found on quite a few systems used by Linux users,
such as on Atom systems (Intel Computestick and various other
Atom based devices) and on many (budget) ARM boards such as
the CHIP.

The plan moving forward with this is for the new clean,
written from scratch, rtl8xxxu driver to eventually gain
support for sdio devices. But there is no clear timeline
for that, so lets add this driver included in staging for now.


Hans,

I started looking at the Smatch errors. This one may be the result of
a serious problem:

  CHECK   drivers/staging/rtl8723bs/core/rtw_debug.c
drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info()
error: we previously assumed 'phead' could be null (see line 453)
drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info()
warn: variable dereferenced before check 'phead' (see line 454)

A snippet of the code in question is as follows:

spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
phead = get_list_head(queue);
plist = phead ? get_next(phead) : NULL;
plist = get_next(phead);
if ((!phead) || (!plist)) {
spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
return 0;
}

This code comes directly from the hadess repo, but I am suspicious of
the double call to get_next(phead). I cannot imagine any valid reason
to skip every other entry on that list.


If you look closer and simplify the first getnext line what is written is:

 plist = get_next(phead);
 plist = get_next(phead);

Which indeed looks like it could use improvement, but I don't
think it is seriously broken.


So get_list_head(queue) can never return NULL?


I don't know.


Otherwise I don't know
how close I need to get for a simplified look ;-)


I simplified things so as to make clear that Larry's worry was
not really a problem, I do agree the smatch complaint is valid.

Regards,

Hans

Re: [PATCH] staging: Add rtl8723bs sdio wifi driver

[Hans de Goede]

Hi,

On 04/04/2017 08:31 PM, Larry Finger wrote:

On 03/29/2017 12:47 PM, Hans de Goede wrote:

The rtl8723bs is found on quite a few systems used by Linux users,
such as on Atom systems (Intel Computestick and various other
Atom based devices) and on many (budget) ARM boards such as
the CHIP.

The plan moving forward with this is for the new clean,
written from scratch, rtl8xxxu driver to eventually gain
support for sdio devices. But there is no clear timeline
for that, so lets add this driver included in staging for now.


Hans,

I started looking at the Smatch errors. This one may be the result of a serious 
problem:

  CHECK   drivers/staging/rtl8723bs/core/rtw_debug.c
drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() error: we 
previously assumed 'phead' could be null (see line 453)
drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() warn: 
variable dereferenced before check 'phead' (see line 454)

A snippet of the code in question is as follows:

spin_lock_bh(&(pmlmepriv->scanned_queue.lock));
phead = get_list_head(queue);
plist = phead ? get_next(phead) : NULL;
plist = get_next(phead);
if ((!phead) || (!plist)) {
spin_unlock_bh(&(pmlmepriv->scanned_queue.lock));
return 0;
}

This code comes directly from the hadess repo, but I am suspicious of the 
double call to get_next(phead). I cannot imagine any valid reason to skip every 
other entry on that list.


If you look closer and simplify the first getnext line what is written is:

 plist = get_next(phead);
 plist = get_next(phead);

Which indeed looks like it could use improvement, but I don't
think it is seriously broken.

Regards,

Hans

Re: [PATCH] staging: Add rtl8723bs sdio wifi driver

[Hans de Goede]

Hi All,

I forgot to add --compose so I could add the cover letter I had
already written, so here it goes:

Hi Greg, all,

I'm hereby submitting the rtl8723bs driver which
Bastien Nocera and Larry Finger have been maintaining at:

https://github.com/hadess/rtl8723bs

For inclusion into staging, like most of the other rtl*
drivers in staging this is a cleaned up version of Realtek's
own driver code for Linux.

I would like to see this added to staging because the
rtl8723bs is found on quite a few systems used by Linux users,
such as on Atom systems (Intel Computestick and various other
Atom based devices) and on many (budget) ARM boards such as
the CHIP.

The plan moving forward with this is for the new clean,
written from scratch, rtl8xxxu driver to eventually gain
support for sdio devices. But there is no clear timeline
for that as Jes is doing that in his spare time, so
I would like to see this driver included in staging for now.

Regards,

Hans

Re: [PATCH v3] brcmfmac: Do not print the firmware version as an error

[Hans de Goede]

Hi,

On 09-03-17 09:17, Arend Van Spriel wrote:

On 9-3-2017 9:09, Hans de Goede wrote:

Hi all,

On 08-03-17 21:44, Arend Van Spriel wrote:



On 8-3-2017 18:53, Joe Perches wrote:

On Wed, 2017-03-08 at 17:57 +0100, Hans de Goede wrote:

Hi,


And hello back to you.


Also hello.


On 08-03-17 17:34, Joe Perches wrote:

On Wed, 2017-03-08 at 09:23 +0100, Hans de Goede wrote:

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

Instead add a new brcmf_info macro and use that.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
Changes in v3:
-Use do { } while (0) around macro


why?  Single statement macros do not need a do/while


Because Arend ask me to during review of v2.


Well, maybe Arend should learn that single statement macros
don't need do/while guards and that do/while guards are
generally not used in the kernel for single statements.


Always good to learn from an expert. The intent behind my remark was to
follow the same pattern as brcmf_err for the sake of consistency. I was
not clear.


Ok, so what is it going to be, are we going to keep this as is
with the do .. while added or shall I do a v4 dropping it again?


Sorry, Hans

My bad. Let's stick with v3 and I will do a follow-up patch.


Ok, that works for me :)

Regards,

Hans

Re: [PATCH v3] brcmfmac: Do not print the firmware version as an error

[Hans de Goede]

Hi all,

On 08-03-17 21:44, Arend Van Spriel wrote:



On 8-3-2017 18:53, Joe Perches wrote:

On Wed, 2017-03-08 at 17:57 +0100, Hans de Goede wrote:

Hi,


And hello back to you.


Also hello.


On 08-03-17 17:34, Joe Perches wrote:

On Wed, 2017-03-08 at 09:23 +0100, Hans de Goede wrote:

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

Instead add a new brcmf_info macro and use that.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
Changes in v3:
-Use do { } while (0) around macro


why?  Single statement macros do not need a do/while


Because Arend ask me to during review of v2.


Well, maybe Arend should learn that single statement macros
don't need do/while guards and that do/while guards are
generally not used in the kernel for single statements.


Always good to learn from an expert. The intent behind my remark was to
follow the same pattern as brcmf_err for the sake of consistency. I was
not clear.


Ok, so what is it going to be, are we going to keep this as is
with the do .. while added or shall I do a v4 dropping it again?

Regards,

Hans

Re: [PATCH v3] brcmfmac: Do not print the firmware version as an error

[Hans de Goede]

Hi,

On 08-03-17 17:34, Joe Perches wrote:

On Wed, 2017-03-08 at 09:23 +0100, Hans de Goede wrote:

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

Instead add a new brcmf_info macro and use that.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
Changes in v3:
-Use do { } while (0) around macro


why?  Single statement macros do not need a do/while


Because Arend ask me to during review of v2.




diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h

[]

@@ -59,6 +59,10 @@ void __brcmf_err(const char *func, const char *fmt, ...);
} while (0)

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

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

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


#define brcmf_info(fmt, ...)
pr_info("%s: " fmt, __func__, ##__VA_ARGS__)


+
 #define brcmf_dbg(level, fmt, ...) no_printk(fmt, ##__VA_ARGS__)


I think the separate defines for DEBUG/CONFIG_BRCM_TRACING
are not necessary.


When tracing we want the message logging the firmware version
to show up in the trace, which requires calling __brcmf_err()

Regards,

Hans

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

[Hans de Goede]

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

Instead add a new brcmf_info macro and use that.

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

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 33b133f..7a2b495 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -161,7 +161,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
strsep(, "\n");
 
/* Print fw version info */
-   brcmf_err("Firmware version = %s\n", buf);
+   brcmf_info("Firmware version = %s\n", buf);
 
/* locate firmware version number for ethtool */
ptr = strrchr(buf, ' ') + 1;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
index 0661261..afb2436 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -59,6 +59,10 @@ void __brcmf_err(const char *func, const char *fmt, ...);
} while (0)
 
 #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
+
+/* For debug/tracing purposes treat info messages as errors */
+#define brcmf_info brcmf_err
+
 __printf(3, 4)
 void __brcmf_dbg(u32 level, const char *func, const char *fmt, ...);
 #define brcmf_dbg(level, fmt, ...) \
@@ -77,6 +81,11 @@ do { 
\
 
 #else /* defined(DEBUG) || defined(CONFIG_BRCM_TRACING) */
 
+#define brcmf_info(fmt, ...)   \
+   do {\
+   pr_info("%s: " fmt, __func__, ##__VA_ARGS__);   \
+   } while (0)
+
 #define brcmf_dbg(level, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
 
 #define BRCMF_DATA_ON()0
-- 
2.9.3

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

[Hans de Goede]

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

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

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 944b83c..7765ad0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6736,6 +6736,10 @@ static void brcmf_cfg80211_reg_notifier(struct wiphy 
*wiphy,
s32 err;
int i;
 
+   /* The country code gets set to "00" by default at boot, ignore */
+   if (req->alpha2[0] == '0' && req->alpha2[1] == '0')
+   return;
+
/* ignore non-ISO3166 country codes */
for (i = 0; i < sizeof(req->alpha2); i++)
if (req->alpha2[i] < 'A' || req->alpha2[i] > 'Z') {
-- 
2.9.3

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

[Hans de Goede]

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

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

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

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 7765ad0..4e1ca69 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3097,6 +3097,9 @@ brcmf_cfg80211_escan_handler(struct brcmf_if *ifp,
 
status = e->status;
 
+   if (status == BRCMF_E_STATUS_ABORT)
+   goto exit;
+
if (!test_bit(BRCMF_SCAN_STATUS_BUSY, >scan_status)) {
brcmf_err("scan not ready, bsscfgidx=%d\n", ifp->bsscfgidx);
return -EPERM;
-- 
2.9.3

Re: [PATCH v2 2/4] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 08-03-17 11:15, Arend Van Spriel wrote:

On 8-3-2017 10:26, Arend Van Spriel wrote:

On 8-3-2017 9:24, Hans de Goede wrote:

Hi,

On 07-03-17 11:03, Arend Van Spriel wrote:



On 27-2-2017 22:45, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra
virtual-if
is a normal thing, do not print an error for this.


This may be something we need to look into. It seems to me the interface
combinations needs to be fixed so we do not try to provision firmware.
Can you explain the scenario here?


I'm not doing anything special just connecting to my isp provided
accesspoint
using NetworkManager.


Ok. So it is probably the P2P_DEVICE interface as that is the only
interface being created in that scenario. Can you provide a log with
brcmfmac driver debug=0x1416.


I guess you are seeing this with 43362. Is that correct?


Maybe, it is with the same card as this bug:

https://bugzilla.kernel.org/show_bug.cgi?id=185661

Regards,

Hans

Re: [PATCH v2 1/4] brcmfmac: Do not print the firmware version as an error

[Hans de Goede]

HI,

On 08-03-17 10:57, Kalle Valo wrote:

Arend Van Spriel <arend.vanspr...@broadcom.com> writes:


On 8-3-2017 9:23, Hans de Goede wrote:

Hi,

On 07-03-17 10:59, Arend Van Spriel wrote:

On 27-2-2017 22:45, Hans de Goede wrote:

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

Instead add a new brcmf_info macro and use that.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h  | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git
a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 3e15d64..6d565f1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -161,7 +161,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
 strsep(, "\n");

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

 /* locate firmware version number for ethtool */
 ptr = strrchr(buf, ' ') + 1;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
index 6687812..605f260 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -59,11 +59,14 @@
 pr_err("%s: " fmt, __func__, ##__VA_ARGS__);\
 } while (0)
 #endif
+#define brcmf_info(fmt, ...)pr_info("%s: " fmt, __func__,
##__VA_ARGS__)


Prefer using the same style as for brcmf_err, ie. using do {} while (0)


OK, v3 with this fixed coming up.


I think Kalle prefers the whole series to be resubmitted.

Kalle,

Can you confirm (or deny)?


Exactly, I want to apply the full series (with the highest version
number). Not waste time guessing what patches I should take and what to
drop, with the increased risk of guessing wrong.


Ok, so patch 2/4 is still under discussion (and may be so for a while)
shall I send a new version with that patch dropped ?  And then send
that patch (or a modified version) separately later ?

Regards,

Hans

Re: [PATCH v2 2/4] brcmfmac: p2p and normal ap access are not always possible at the same time

[Hans de Goede]

Hi,

On 07-03-17 11:03, Arend Van Spriel wrote:



On 27-2-2017 22:45, Hans de Goede wrote:

The firmware responding with -EBUSY when trying to add an extra virtual-if
is a normal thing, do not print an error for this.


This may be something we need to look into. It seems to me the interface
combinations needs to be fixed so we do not try to provision firmware.
Can you explain the scenario here?


I'm not doing anything special just connecting to my isp provided accesspoint
using NetworkManager.

Regards,

Hans




Regards,
Arend


Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c| 14 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c |  5 -
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 7ffc4ab..c54e8b4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -688,11 +688,17 @@ static struct wireless_dev 
*brcmf_cfg80211_add_iface(struct wiphy *wiphy,
return ERR_PTR(-EINVAL);
}

-   if (IS_ERR(wdev))
-   brcmf_err("add iface %s type %d failed: err=%d\n",
- name, type, (int)PTR_ERR(wdev));
-   else
+   if (IS_ERR(wdev)) {
+   err = PTR_ERR(wdev);
+   if (err != -EBUSY)
+   brcmf_err("add iface %s type %d failed: err=%d\n",
+ name, type, err);
+   else
+   brcmf_dbg(INFO, "add iface %s type %d failed: err=%d\n",
+ name, type, err);
+   } else {
brcmf_cfg80211_update_proto_addr_mode(wdev);
+   }

return wdev;
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index de19c7c..b5df0a0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2090,7 +2090,10 @@ static struct wireless_dev 
*brcmf_p2p_create_p2pdev(struct brcmf_p2p_info *p2p,
/* Initialize P2P Discovery in the firmware */
err = brcmf_fil_iovar_int_set(pri_ifp, "p2p_disc", 1);
if (err < 0) {
-   brcmf_err("set p2p_disc error\n");
+   if (err != -EBUSY)
+   brcmf_err("set p2p_disc error\n");
+   else
+   brcmf_dbg(INFO, "set p2p_disc error\n");
brcmf_fweh_p2pdev_setup(pri_ifp, false);
brcmf_cfg80211_arm_vif_event(p2p->cfg, NULL);
goto fail;

[PATCH v3] brcmfmac: Do not print the firmware version as an error

[Hans de Goede]

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

Instead add a new brcmf_info macro and use that.

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

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 33b133f..7a2b495 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -161,7 +161,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
strsep(, "\n");
 
/* Print fw version info */
-   brcmf_err("Firmware version = %s\n", buf);
+   brcmf_info("Firmware version = %s\n", buf);
 
/* locate firmware version number for ethtool */
ptr = strrchr(buf, ' ') + 1;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
index 0661261..afb2436 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -59,6 +59,10 @@ void __brcmf_err(const char *func, const char *fmt, ...);
} while (0)
 
 #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
+
+/* For debug/tracing purposes treat info messages as errors */
+#define brcmf_info brcmf_err
+
 __printf(3, 4)
 void __brcmf_dbg(u32 level, const char *func, const char *fmt, ...);
 #define brcmf_dbg(level, fmt, ...) \
@@ -77,6 +81,11 @@ do { 
\
 
 #else /* defined(DEBUG) || defined(CONFIG_BRCM_TRACING) */
 
+#define brcmf_info(fmt, ...)   \
+   do {\
+   pr_info("%s: " fmt, __func__, ##__VA_ARGS__);   \
+   } while (0)
+
 #define brcmf_dbg(level, fmt, ...) no_printk(fmt, ##__VA_ARGS__)
 
 #define BRCMF_DATA_ON()0
-- 
2.9.3

Re: [PATCH v2 1/4] brcmfmac: Do not print the firmware version as an error

[Hans de Goede]

Hi,

On 07-03-17 10:59, Arend Van Spriel wrote:

On 27-2-2017 22:45, Hans de Goede wrote:

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

Instead add a new brcmf_info macro and use that.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
Changes in v2:
-Fix brcm_err typo (should be brcmf_err) in CONFIG_BRCM_TRACING case
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h  | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 3e15d64..6d565f1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -161,7 +161,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
strsep(, "\n");

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

/* locate firmware version number for ethtool */
ptr = strrchr(buf, ' ') + 1;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
index 6687812..605f260 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -59,11 +59,14 @@
pr_err("%s: " fmt, __func__, ##__VA_ARGS__);  \
} while (0)
 #endif
+#define brcmf_info(fmt, ...)   pr_info("%s: " fmt, __func__, ##__VA_ARGS__)


Prefer using the same style as for brcmf_err, ie. using do {} while (0)


OK, v3 with this fixed coming up.

Regards,

Hans




Regards,
Arend


 #else
 __printf(2, 3)
 void __brcmf_err(const char *func, const char *fmt, ...);
 #define brcmf_err(fmt, ...) \
__brcmf_err(__func__, fmt, ##__VA_ARGS__)
+/* For tracing purposes treat info messages as errors */
+#define brcmf_info brcmf_err
 #endif

 #if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)

[PATCH v2 4/4] brcmfmac: Handle status == BRCMF_E_STATUS_ABORT in cfg80211_escan_handler

[Hans de Goede]

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

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

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index c0b7f69..4c740b74 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3105,6 +3105,9 @@ brcmf_cfg80211_escan_handler(struct brcmf_if *ifp,
 
status = e->status;
 
+   if (status == BRCMF_E_STATUS_ABORT)
+   goto exit;
+
if (!test_bit(BRCMF_SCAN_STATUS_BUSY, >scan_status)) {
brcmf_err("scan not ready, bsscfgidx=%d\n", ifp->bsscfgidx);
return -EPERM;
-- 
2.9.3