diff mbox

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

Message ID 20180318192147.16813-1-hdegoede@redhat.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

Hans de Goede March 18, 2018, 7:21 p.m. UTC 
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 <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@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(-)

Comments

kernel test robot March 19, 2018, 2:37 p.m. UTC | #1 
Hi Hans,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180319]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/brcmfmac-Add-support-for-getting-nvram-contents-from-EFI-variables/20180319-164541
config: arm-tegra_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c: In function 'brcmf_fw_request_nvram_done':
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c:545:44: error: passing argument 1 of 'brcmf_fw_nvram_from_efi' from incompatible pointer type [-Werror=incompatible-pointer-types]
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
                                               ^~~~~
   drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c:525:12: note: expected 'size_t * {aka unsigned int *}' but argument is of type 'struct brcmf_fw *'
    static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
               ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c:545:20: error: too many arguments to function 'brcmf_fw_nvram_from_efi'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
                       ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c:525:12: note: declared here
    static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
               ^~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/brcmf_fw_nvram_from_efi +545 drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c

   527	
   528	static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
   529	{
   530		struct brcmf_fw *fwctx = ctx;
   531		bool free_bcm47xx_nvram = false;
   532		bool kfree_nvram = false;
   533		u32 nvram_length = 0;
   534		void *nvram = NULL;
   535		u8 *data = NULL;
   536		size_t data_len;
   537	
   538		brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
   539		if (fw && fw->data) {
   540			data = (u8 *)fw->data;
   541			data_len = fw->size;
   542		} else {
   543			if ((data = bcm47xx_nvram_get_contents(&data_len)))
   544				free_bcm47xx_nvram = true;
 > 545			else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
   546				kfree_nvram = true;
   547			else if (!(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
   548				goto fail;
   549		}
   550	
   551		if (data)
   552			nvram = brcmf_fw_nvram_strip(data, data_len, &nvram_length,
   553						     fwctx->domain_nr, fwctx->bus_nr);
   554	
   555		if (free_bcm47xx_nvram)
   556			bcm47xx_nvram_release_contents(data);
   557		if (kfree_nvram)
   558			kfree(data);
   559	
   560		release_firmware(fw);
   561		if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
   562			goto fail;
   563	
   564		fwctx->done(fwctx->dev, 0, fwctx->code, nvram, nvram_length);
   565		kfree(fwctx);
   566		return;
   567	
   568	fail:
   569		brcmf_dbg(TRACE, "failed: dev=%s\n", dev_name(fwctx->dev));
   570		release_firmware(fwctx->code);
   571		fwctx->done(fwctx->dev, -ENOENT, NULL, NULL, 0);
   572		kfree(fwctx);
   573	}
   574	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot March 19, 2018, 4:49 p.m. UTC | #2 
Hi Hans,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180319]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/brcmfmac-Add-support-for-getting-nvram-contents-from-EFI-variables/20180319-164541
config: x86_64-randconfig-x001-201811 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/dmi.h:5,
                    from drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:17:
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c: In function 'brcmf_fw_request_nvram_done':
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:44: error: passing argument 1 of 'brcmf_fw_nvram_from_efi' from incompatible pointer type [-Werror=incompatible-pointer-types]
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
                                               ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:8: note: in expansion of macro 'if'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
           ^~
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:525:12: note: expected 'size_t * {aka long unsigned int *}' but argument is of type 'struct brcmf_fw *'
    static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
               ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/dmi.h:5,
                    from drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:17:
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:20: error: too many arguments to function 'brcmf_fw_nvram_from_efi'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
                       ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:8: note: in expansion of macro 'if'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
           ^~
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:525:12: note: declared here
    static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
               ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/dmi.h:5,
                    from drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:17:
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:44: error: passing argument 1 of 'brcmf_fw_nvram_from_efi' from incompatible pointer type [-Werror=incompatible-pointer-types]
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
                                               ^
   include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:8: note: in expansion of macro 'if'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
           ^~
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:525:12: note: expected 'size_t * {aka long unsigned int *}' but argument is of type 'struct brcmf_fw *'
    static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
               ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/dmi.h:5,
                    from drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:17:
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:20: error: too many arguments to function 'brcmf_fw_nvram_from_efi'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
                       ^
   include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:8: note: in expansion of macro 'if'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
           ^~
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:525:12: note: declared here
    static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
               ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/dmi.h:5,
                    from drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:17:
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:44: error: passing argument 1 of 'brcmf_fw_nvram_from_efi' from incompatible pointer type [-Werror=incompatible-pointer-types]
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
                                               ^
   include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:8: note: in expansion of macro 'if'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
           ^~
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:525:12: note: expected 'size_t * {aka long unsigned int *}' but argument is of type 'struct brcmf_fw *'
    static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
               ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/dmi.h:5,
                    from drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:17:
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:20: error: too many arguments to function 'brcmf_fw_nvram_from_efi'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
                       ^
   include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:545:8: note: in expansion of macro 'if'
      else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
           ^~
   drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c:525:12: note: declared here
    static u8 *brcmf_fw_nvram_from_efi(size_t *data_len) { return NULL; }
               ^~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/if +545 drivers/net/wireless//broadcom/brcm80211/brcmfmac/firmware.c

   527	
   528	static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
   529	{
   530		struct brcmf_fw *fwctx = ctx;
   531		bool free_bcm47xx_nvram = false;
   532		bool kfree_nvram = false;
   533		u32 nvram_length = 0;
   534		void *nvram = NULL;
   535		u8 *data = NULL;
   536		size_t data_len;
   537	
   538		brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev));
   539		if (fw && fw->data) {
   540			data = (u8 *)fw->data;
   541			data_len = fw->size;
   542		} else {
   543			if ((data = bcm47xx_nvram_get_contents(&data_len)))
   544				free_bcm47xx_nvram = true;
 > 545			else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
   546				kfree_nvram = true;
   547			else if (!(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
   548				goto fail;
   549		}
   550	
   551		if (data)
   552			nvram = brcmf_fw_nvram_strip(data, data_len, &nvram_length,
   553						     fwctx->domain_nr, fwctx->bus_nr);
   554	
   555		if (free_bcm47xx_nvram)
   556			bcm47xx_nvram_release_contents(data);
   557		if (kfree_nvram)
   558			kfree(data);
   559	
   560		release_firmware(fw);
   561		if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
   562			goto fail;
   563	
   564		fwctx->done(fwctx->dev, 0, fwctx->code, nvram, nvram_length);
   565		kfree(fwctx);
   566		return;
   567	
   568	fail:
   569		brcmf_dbg(TRACE, "failed: dev=%s\n", dev_name(fwctx->dev));
   570		release_firmware(fwctx->code);
   571		fwctx->done(fwctx->dev, -ENOENT, NULL, NULL, 0);
   572		kfree(fwctx);
   573	}
   574	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

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 <linux/dmi.h>
+#include <linux/efi.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/device.h>
@@ -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(&nvram_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, &data_len);
+	if (err)
+		goto fail;
+
+	data = kmalloc(data_len, GFP_KERNEL);
+	if (!data)
+		goto fail;
+
+	err = efivar_entry_get(nvram_efivar, NULL, &data_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_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;
+	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));
 	if (fw && fw->data) {
 		data = (u8 *)fw->data;
 		data_len = fw->size;
-		raw_nvram = false;
 	} else {
-		data = bcm47xx_nvram_get_contents(&data_len);
-		if (!data && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
+		if ((data = bcm47xx_nvram_get_contents(&data_len)))
+			free_bcm47xx_nvram = true;
+		else if ((data = brcmf_fw_nvram_from_efi(fwctx, &data_len)))
+			kfree_nvram = true;
+		else if (!(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
 			goto fail;
-		raw_nvram = true;
 	}
 
 	if (data)
 		nvram = brcmf_fw_nvram_strip(data, data_len, &nvram_length,
 					     fwctx->domain_nr, fwctx->bus_nr);
 
-	if (raw_nvram)
+	if (free_bcm47xx_nvram)
 		bcm47xx_nvram_release_contents(data);
+	if (kfree_nvram)
+		kfree(data);
+
 	release_firmware(fw);
 	if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))
 		goto fail;