[PATCH v3 0/9] misc brcmfmac fixes (M1/T2 series spin-off)

[PATCH v3 0/9] misc brcmfmac fixes (M1/T2 series spin-off)

[Hector Martin]

Hi everyone,

This series contains just the fixes / misc improvements from the
previously submitted series:

brcmfmac: Support Apple T2 and M1 platforms

Patches 8-9 aren't strictly bugfixes but rather just general
improvements; they can be safely skipped, although patch 8 will be a
dependency of the subsequent series to avoid a compile warning.

Hector Martin (9):
  brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path
  brcmfmac: firmware: Allocate space for default boardrev in nvram
  brcmfmac: firmware: Do not crash on a NULL board_type
  brcmfmac: pcie: Declare missing firmware files in pcie.c
  brcmfmac: pcie: Replace brcmf_pcie_copy_mem_todev with memcpy_toio
  brcmfmac: pcie: Fix crashes due to early IRQs
  brcmfmac: of: Use devm_kstrdup for board_type & check for errors
  brcmfmac: fwil: Constify iovar name arguments
  brcmfmac: pcie: Read the console on init and shutdown

 .../broadcom/brcm80211/brcmfmac/firmware.c    |  5 ++
 .../broadcom/brcm80211/brcmfmac/fwil.c        | 34 ++++----
 .../broadcom/brcm80211/brcmfmac/fwil.h        | 28 +++----
 .../wireless/broadcom/brcm80211/brcmfmac/of.c |  8 +-
 .../broadcom/brcm80211/brcmfmac/pcie.c        | 77 ++++++++-----------
 .../broadcom/brcm80211/brcmfmac/sdio.c        |  1 -
 6 files changed, 72 insertions(+), 81 deletions(-)

-- 
2.33.0

[PATCH v3 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Hector Martin]

This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
the CLM blob is released in the device remove path.

Fixes: 82f93cf46d60 ("brcmfmac: get chip's default RAM info during PCIe setup")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 8b149996fc00..f876b1d8d00d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1777,6 +1777,8 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
 	ret = brcmf_chip_get_raminfo(devinfo->ci);
 	if (ret) {
 		brcmf_err(bus, "Failed to get RAM info\n");
+		release_firmware(fw);
+		brcmf_fw_nvram_free(nvram);
 		goto fail;
 	}
 
-- 
2.33.0

[PATCH v3 2/9] brcmfmac: firmware: Allocate space for default boardrev in nvram

[Hector Martin]

If boardrev is missing from the NVRAM we add a default one, but this
might need more space in the output buffer than was allocated. Ensure
we have enough padding for this in the buffer.

Fixes: 46f2b38a91b0 ("brcmfmac: insert default boardrev in nvram data if missing")
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 0eb13e5df517..1001c8888bfe 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -207,6 +207,8 @@ static int brcmf_init_nvram_parser(struct nvram_parser *nvp,
 		size = BRCMF_FW_MAX_NVRAM_SIZE;
 	else
 		size = data_len;
+	/* Add space for properties we may add */
+	size += strlen(BRCMF_FW_DEFAULT_BOARDREV) + 1;
 	/* Alloc for extra 0 byte + roundup by 4 + length field */
 	size += 1 + 3 + sizeof(u32);
 	nvp->nvram = kzalloc(size, GFP_KERNEL);
-- 
2.33.0

[PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Hector Martin]

This unbreaks support for USB devices, which do not have a board_type
to create an alt_path out of and thus were running into a NULL
dereference.

Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 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 1001c8888bfe..63821856bbe1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, const char *board_type)
 	char alt_path[BRCMF_FW_NAME_LEN];
 	char suffix[5];
 
+	if (!board_type)
+		return NULL;
+
 	strscpy(alt_path, path, BRCMF_FW_NAME_LEN);
 	/* At least one character + suffix */
 	if (strlen(alt_path) < 5)
-- 
2.33.0

[PATCH v3 4/9] brcmfmac: pcie: Declare missing firmware files in pcie.c

[Hector Martin]

Move one of the declarations from sdio.c to pcie.c, since it makes no
sense in the former (SDIO support is optional), and add missing ones.

Fixes: 75729e110e68 ("brcmfmac: expose firmware config files through modinfo")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 7 +++++++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 1 -
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index f876b1d8d00d..b1ae6c41013f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -59,6 +59,13 @@ BRCMF_FW_DEF(4366B, "brcmfmac4366b-pcie");
 BRCMF_FW_DEF(4366C, "brcmfmac4366c-pcie");
 BRCMF_FW_DEF(4371, "brcmfmac4371-pcie");
 
+/* firmware config files */
+MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.txt");
+MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.*.txt");
+
+/* per-board firmware binaries */
+MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.*.bin");
+
 static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
 	BRCMF_FW_ENTRY(BRCM_CC_43602_CHIP_ID, 0xFFFFFFFF, 43602),
 	BRCMF_FW_ENTRY(BRCM_CC_43465_CHIP_ID, 0xFFFFFFF0, 4366C),
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 8effeb7a7269..5d156e591b35 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -629,7 +629,6 @@ BRCMF_FW_CLM_DEF(43752, "brcmfmac43752-sdio");
 
 /* firmware config files */
 MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-sdio.*.txt");
-MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.*.txt");
 
 /* per-board firmware binaries */
 MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-sdio.*.bin");
-- 
2.33.0

[PATCH v3 5/9] brcmfmac: pcie: Replace brcmf_pcie_copy_mem_todev with memcpy_toio

[Hector Martin]

The alignment check was wrong (e.g. & 4 instead of & 3), and the logic
was also inefficient if the length was not a multiple of 4, since it
would needlessly fall back to copying the entire buffer bytewise.

We already have a perfectly good memcpy_toio function, so just call that
instead of rolling our own copy logic here. brcmf_pcie_init_ringbuffers
was already using it anyway.

Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../broadcom/brcm80211/brcmfmac/pcie.c        | 48 ++-----------------
 1 file changed, 4 insertions(+), 44 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index b1ae6c41013f..c25f48db1f60 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -12,6 +12,7 @@
 #include <linux/interrupt.h>
 #include <linux/bcma/bcma.h>
 #include <linux/sched.h>
+#include <linux/io.h>
 #include <asm/unaligned.h>
 
 #include <soc.h>
@@ -454,47 +455,6 @@ brcmf_pcie_write_ram32(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
 }
 
 
-static void
-brcmf_pcie_copy_mem_todev(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
-			  void *srcaddr, u32 len)
-{
-	void __iomem *address = devinfo->tcm + mem_offset;
-	__le32 *src32;
-	__le16 *src16;
-	u8 *src8;
-
-	if (((ulong)address & 4) || ((ulong)srcaddr & 4) || (len & 4)) {
-		if (((ulong)address & 2) || ((ulong)srcaddr & 2) || (len & 2)) {
-			src8 = (u8 *)srcaddr;
-			while (len) {
-				iowrite8(*src8, address);
-				address++;
-				src8++;
-				len--;
-			}
-		} else {
-			len = len / 2;
-			src16 = (__le16 *)srcaddr;
-			while (len) {
-				iowrite16(le16_to_cpu(*src16), address);
-				address += 2;
-				src16++;
-				len--;
-			}
-		}
-	} else {
-		len = len / 4;
-		src32 = (__le32 *)srcaddr;
-		while (len) {
-			iowrite32(le32_to_cpu(*src32), address);
-			address += 4;
-			src32++;
-			len--;
-		}
-	}
-}
-
-
 static void
 brcmf_pcie_copy_dev_tomem(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
 			  void *dstaddr, u32 len)
@@ -1570,8 +1530,8 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
 		return err;
 
 	brcmf_dbg(PCIE, "Download FW %s\n", devinfo->fw_name);
-	brcmf_pcie_copy_mem_todev(devinfo, devinfo->ci->rambase,
-				  (void *)fw->data, fw->size);
+	memcpy_toio(devinfo->tcm + devinfo->ci->rambase,
+		    (void *)fw->data, fw->size);
 
 	resetintr = get_unaligned_le32(fw->data);
 	release_firmware(fw);
@@ -1585,7 +1545,7 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
 		brcmf_dbg(PCIE, "Download NVRAM %s\n", devinfo->nvram_name);
 		address = devinfo->ci->rambase + devinfo->ci->ramsize -
 			  nvram_len;
-		brcmf_pcie_copy_mem_todev(devinfo, address, nvram, nvram_len);
+		memcpy_toio(devinfo->tcm + address, nvram, nvram_len);
 		brcmf_fw_nvram_free(nvram);
 	} else {
 		brcmf_dbg(PCIE, "No matching NVRAM file found %s\n",
-- 
2.33.0

[PATCH v3 6/9] brcmfmac: pcie: Fix crashes due to early IRQs

[Hector Martin]

The driver was enabling IRQs before the message processing was
initialized. This could cause IRQs to come in too early and crash the
driver. Instead, move the IRQ enable and hostready to a bus preinit
function, at which point everything is properly initialized.

Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../wireless/broadcom/brcm80211/brcmfmac/pcie.c  | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index c25f48db1f60..3ff4997e1c97 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1315,6 +1315,18 @@ static void brcmf_pcie_down(struct device *dev)
 {
 }
 
+static int brcmf_pcie_preinit(struct device *dev)
+{
+	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+	struct brcmf_pciedev *buspub = bus_if->bus_priv.pcie;
+
+	brcmf_dbg(PCIE, "Enter\n");
+
+	brcmf_pcie_intr_enable(buspub->devinfo);
+	brcmf_pcie_hostready(buspub->devinfo);
+
+	return 0;
+}
 
 static int brcmf_pcie_tx(struct device *dev, struct sk_buff *skb)
 {
@@ -1423,6 +1435,7 @@ static int brcmf_pcie_reset(struct device *dev)
 }
 
 static const struct brcmf_bus_ops brcmf_pcie_bus_ops = {
+	.preinit = brcmf_pcie_preinit,
 	.txdata = brcmf_pcie_tx,
 	.stop = brcmf_pcie_down,
 	.txctl = brcmf_pcie_tx_ctlpkt,
@@ -1795,9 +1808,6 @@ static void brcmf_pcie_setup(struct device *dev, int ret,
 
 	init_waitqueue_head(&devinfo->mbdata_resp_wait);
 
-	brcmf_pcie_intr_enable(devinfo);
-	brcmf_pcie_hostready(devinfo);
-
 	ret = brcmf_attach(&devinfo->pdev->dev);
 	if (ret)
 		goto fail;
-- 
2.33.0

[PATCH v3 7/9] brcmfmac: of: Use devm_kstrdup for board_type & check for errors

[Hector Martin]

This was missing a NULL check, and we can collapse the strlen/alloc/copy
into a devm_kstrdup().

Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index 513c7e6421b2..5dc1e942e9e7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
@@ -79,8 +79,12 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
 
 		/* get rid of '/' in the compatible string to be able to find the FW */
 		len = strlen(tmp) + 1;
-		board_type = devm_kzalloc(dev, len, GFP_KERNEL);
-		strscpy(board_type, tmp, len);
+		board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);
+		if (!board_type) {
+			brcmf_err("out of memory allocating board_type\n");
+			of_node_put(root);
+			return;
+		}
 		for (i = 0; i < board_type[i]; i++) {
 			if (board_type[i] == '/')
 				board_type[i] = '-';
-- 
2.33.0

[PATCH v3 8/9] brcmfmac: fwil: Constify iovar name arguments

[Hector Martin]

Make all the iovar name arguments const char * instead of just char *.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../broadcom/brcm80211/brcmfmac/fwil.c        | 34 +++++++++----------
 .../broadcom/brcm80211/brcmfmac/fwil.h        | 28 +++++++--------
 2 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
index d5578ca681bb..72fe8bce6eaf 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
@@ -192,7 +192,7 @@ brcmf_fil_cmd_int_get(struct brcmf_if *ifp, u32 cmd, u32 *data)
 }
 
 static u32
-brcmf_create_iovar(char *name, const char *data, u32 datalen,
+brcmf_create_iovar(const char *name, const char *data, u32 datalen,
 		   char *buf, u32 buflen)
 {
 	u32 len;
@@ -213,7 +213,7 @@ brcmf_create_iovar(char *name, const char *data, u32 datalen,
 
 
 s32
-brcmf_fil_iovar_data_set(struct brcmf_if *ifp, char *name, const void *data,
+brcmf_fil_iovar_data_set(struct brcmf_if *ifp, const char *name, const void *data,
 			 u32 len)
 {
 	struct brcmf_pub *drvr = ifp->drvr;
@@ -241,7 +241,7 @@ brcmf_fil_iovar_data_set(struct brcmf_if *ifp, char *name, const void *data,
 }
 
 s32
-brcmf_fil_iovar_data_get(struct brcmf_if *ifp, char *name, void *data,
+brcmf_fil_iovar_data_get(struct brcmf_if *ifp, const char *name, void *data,
 			 u32 len)
 {
 	struct brcmf_pub *drvr = ifp->drvr;
@@ -272,7 +272,7 @@ brcmf_fil_iovar_data_get(struct brcmf_if *ifp, char *name, void *data,
 }
 
 s32
-brcmf_fil_iovar_int_set(struct brcmf_if *ifp, char *name, u32 data)
+brcmf_fil_iovar_int_set(struct brcmf_if *ifp, const char *name, u32 data)
 {
 	__le32 data_le = cpu_to_le32(data);
 
@@ -280,7 +280,7 @@ brcmf_fil_iovar_int_set(struct brcmf_if *ifp, char *name, u32 data)
 }
 
 s32
-brcmf_fil_iovar_int_get(struct brcmf_if *ifp, char *name, u32 *data)
+brcmf_fil_iovar_int_get(struct brcmf_if *ifp, const char *name, u32 *data)
 {
 	__le32 data_le = cpu_to_le32(*data);
 	s32 err;
@@ -292,7 +292,7 @@ brcmf_fil_iovar_int_get(struct brcmf_if *ifp, char *name, u32 *data)
 }
 
 static u32
-brcmf_create_bsscfg(s32 bsscfgidx, char *name, char *data, u32 datalen,
+brcmf_create_bsscfg(s32 bsscfgidx, const char *name, char *data, u32 datalen,
 		    char *buf, u32 buflen)
 {
 	const s8 *prefix = "bsscfg:";
@@ -337,7 +337,7 @@ brcmf_create_bsscfg(s32 bsscfgidx, char *name, char *data, u32 datalen,
 }
 
 s32
-brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, char *name,
+brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, const char *name,
 			  void *data, u32 len)
 {
 	struct brcmf_pub *drvr = ifp->drvr;
@@ -366,7 +366,7 @@ brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, char *name,
 }
 
 s32
-brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, char *name,
+brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, const char *name,
 			  void *data, u32 len)
 {
 	struct brcmf_pub *drvr = ifp->drvr;
@@ -396,7 +396,7 @@ brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, char *name,
 }
 
 s32
-brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, char *name, u32 data)
+brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, const char *name, u32 data)
 {
 	__le32 data_le = cpu_to_le32(data);
 
@@ -405,7 +405,7 @@ brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, char *name, u32 data)
 }
 
 s32
-brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, char *name, u32 *data)
+brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, const char *name, u32 *data)
 {
 	__le32 data_le = cpu_to_le32(*data);
 	s32 err;
@@ -417,7 +417,7 @@ brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, char *name, u32 *data)
 	return err;
 }
 
-static u32 brcmf_create_xtlv(char *name, u16 id, char *data, u32 len,
+static u32 brcmf_create_xtlv(const char *name, u16 id, char *data, u32 len,
 			     char *buf, u32 buflen)
 {
 	u32 iolen;
@@ -438,7 +438,7 @@ static u32 brcmf_create_xtlv(char *name, u16 id, char *data, u32 len,
 	return iolen;
 }
 
-s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, char *name, u16 id,
+s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, const char *name, u16 id,
 			    void *data, u32 len)
 {
 	struct brcmf_pub *drvr = ifp->drvr;
@@ -466,7 +466,7 @@ s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, char *name, u16 id,
 	return err;
 }
 
-s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, char *name, u16 id,
+s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, const char *name, u16 id,
 			    void *data, u32 len)
 {
 	struct brcmf_pub *drvr = ifp->drvr;
@@ -495,7 +495,7 @@ s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, char *name, u16 id,
 	return err;
 }
 
-s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, char *name, u16 id, u32 data)
+s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, const char *name, u16 id, u32 data)
 {
 	__le32 data_le = cpu_to_le32(data);
 
@@ -503,7 +503,7 @@ s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, char *name, u16 id, u32 data)
 					 sizeof(data_le));
 }
 
-s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, char *name, u16 id, u32 *data)
+s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, const char *name, u16 id, u32 *data)
 {
 	__le32 data_le = cpu_to_le32(*data);
 	s32 err;
@@ -514,12 +514,12 @@ s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, char *name, u16 id, u32 *data)
 	return err;
 }
 
-s32 brcmf_fil_xtlv_int8_get(struct brcmf_if *ifp, char *name, u16 id, u8 *data)
+s32 brcmf_fil_xtlv_int8_get(struct brcmf_if *ifp, const char *name, u16 id, u8 *data)
 {
 	return brcmf_fil_xtlv_data_get(ifp, name, id, data, sizeof(*data));
 }
 
-s32 brcmf_fil_xtlv_int16_get(struct brcmf_if *ifp, char *name, u16 id, u16 *data)
+s32 brcmf_fil_xtlv_int16_get(struct brcmf_if *ifp, const char *name, u16 id, u16 *data)
 {
 	__le16 data_le = cpu_to_le16(*data);
 	s32 err;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
index cb26f8c59c21..bc693157c4b1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
@@ -84,26 +84,26 @@ s32 brcmf_fil_cmd_data_get(struct brcmf_if *ifp, u32 cmd, void *data, u32 len);
 s32 brcmf_fil_cmd_int_set(struct brcmf_if *ifp, u32 cmd, u32 data);
 s32 brcmf_fil_cmd_int_get(struct brcmf_if *ifp, u32 cmd, u32 *data);
 
-s32 brcmf_fil_iovar_data_set(struct brcmf_if *ifp, char *name, const void *data,
+s32 brcmf_fil_iovar_data_set(struct brcmf_if *ifp, const char *name, const void *data,
 			     u32 len);
-s32 brcmf_fil_iovar_data_get(struct brcmf_if *ifp, char *name, void *data,
+s32 brcmf_fil_iovar_data_get(struct brcmf_if *ifp, const char *name, void *data,
 			     u32 len);
-s32 brcmf_fil_iovar_int_set(struct brcmf_if *ifp, char *name, u32 data);
-s32 brcmf_fil_iovar_int_get(struct brcmf_if *ifp, char *name, u32 *data);
+s32 brcmf_fil_iovar_int_set(struct brcmf_if *ifp, const char *name, u32 data);
+s32 brcmf_fil_iovar_int_get(struct brcmf_if *ifp, const char *name, u32 *data);
 
-s32 brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, char *name, void *data,
+s32 brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, const char *name, void *data,
 			      u32 len);
-s32 brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, char *name, void *data,
+s32 brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, const char *name, void *data,
 			      u32 len);
-s32 brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, char *name, u32 data);
-s32 brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, char *name, u32 *data);
-s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, char *name, u16 id,
+s32 brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, const char *name, u32 data);
+s32 brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, const char *name, u32 *data);
+s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, const char *name, u16 id,
 			    void *data, u32 len);
-s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, char *name, u16 id,
+s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, const char *name, u16 id,
 			    void *data, u32 len);
-s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, char *name, u16 id, u32 data);
-s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, char *name, u16 id, u32 *data);
-s32 brcmf_fil_xtlv_int8_get(struct brcmf_if *ifp, char *name, u16 id, u8 *data);
-s32 brcmf_fil_xtlv_int16_get(struct brcmf_if *ifp, char *name, u16 id, u16 *data);
+s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, const char *name, u16 id, u32 data);
+s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, const char *name, u16 id, u32 *data);
+s32 brcmf_fil_xtlv_int8_get(struct brcmf_if *ifp, const char *name, u16 id, u8 *data);
+s32 brcmf_fil_xtlv_int16_get(struct brcmf_if *ifp, const char *name, u16 id, u16 *data);
 
 #endif /* _fwil_h_ */
-- 
2.33.0

[PATCH v3 9/9] brcmfmac: pcie: Read the console on init and shutdown

[Hector Martin]

This allows us to get console messages if the firmware crashed during
early init, or if an operation failed and we're about to shut down.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 3ff4997e1c97..4fe341376a16 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -744,6 +744,8 @@ static void brcmf_pcie_bus_console_read(struct brcmf_pciedev_info *devinfo,
 		return;
 
 	console = &devinfo->shared.console;
+	if (!console->base_addr)
+		return;
 	addr = console->base_addr + BRCMF_CONSOLE_WRITEIDX_OFFSET;
 	newidx = brcmf_pcie_read_tcm32(devinfo, addr);
 	while (newidx != console->read_idx) {
@@ -1520,6 +1522,7 @@ brcmf_pcie_init_share_ram_info(struct brcmf_pciedev_info *devinfo,
 		  shared->max_rxbufpost, shared->rx_dataoffset);
 
 	brcmf_pcie_bus_console_init(devinfo);
+	brcmf_pcie_bus_console_read(devinfo, false);
 
 	return 0;
 }
@@ -1959,6 +1962,7 @@ brcmf_pcie_remove(struct pci_dev *pdev)
 		return;
 
 	devinfo = bus->bus_priv.pcie->devinfo;
+	brcmf_pcie_bus_console_read(devinfo, false);
 
 	devinfo->state = BRCMFMAC_PCIE_STATE_DOWN;
 	if (devinfo->ci)
-- 
2.33.0

Re: [PATCH v3 0/9] misc brcmfmac fixes (M1/T2 series spin-off)

[Andy Shevchenko]

On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>
> Hi everyone,
>
> This series contains just the fixes / misc improvements from the
> previously submitted series:
>
> brcmfmac: Support Apple T2 and M1 platforms
>
> Patches 8-9 aren't strictly bugfixes but rather just general
> improvements; they can be safely skipped, although patch 8 will be a
> dependency of the subsequent series to avoid a compile warning.

Have I given you a tag? If so, I do not see it applied in the patches...

> Hector Martin (9):
>   brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path
>   brcmfmac: firmware: Allocate space for default boardrev in nvram
>   brcmfmac: firmware: Do not crash on a NULL board_type
>   brcmfmac: pcie: Declare missing firmware files in pcie.c
>   brcmfmac: pcie: Replace brcmf_pcie_copy_mem_todev with memcpy_toio
>   brcmfmac: pcie: Fix crashes due to early IRQs
>   brcmfmac: of: Use devm_kstrdup for board_type & check for errors
>   brcmfmac: fwil: Constify iovar name arguments
>   brcmfmac: pcie: Read the console on init and shutdown
>
>  .../broadcom/brcm80211/brcmfmac/firmware.c    |  5 ++
>  .../broadcom/brcm80211/brcmfmac/fwil.c        | 34 ++++----
>  .../broadcom/brcm80211/brcmfmac/fwil.h        | 28 +++----
>  .../wireless/broadcom/brcm80211/brcmfmac/of.c |  8 +-
>  .../broadcom/brcm80211/brcmfmac/pcie.c        | 77 ++++++++-----------
>  .../broadcom/brcm80211/brcmfmac/sdio.c        |  1 -
>  6 files changed, 72 insertions(+), 81 deletions(-)
>
> --
> 2.33.0
>


--
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/9] misc brcmfmac fixes (M1/T2 series spin-off)

[Hector Martin]

On 18/01/2022 19.43, Andy Shevchenko wrote:
> On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>>
>> Hi everyone,
>>
>> This series contains just the fixes / misc improvements from the
>> previously submitted series:
>>
>> brcmfmac: Support Apple T2 and M1 platforms
>>
>> Patches 8-9 aren't strictly bugfixes but rather just general
>> improvements; they can be safely skipped, although patch 8 will be a
>> dependency of the subsequent series to avoid a compile warning.
> 
> Have I given you a tag? If so, I do not see it applied in the patches...

I didn't see any review tags from you in the previous thread. Did I miss
any?

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

Re: [PATCH v3 0/9] misc brcmfmac fixes (M1/T2 series spin-off)

[Andy Shevchenko]

On Tue, Jan 18, 2022 at 5:32 PM Hector Martin <marcan@marcan.st> wrote:
> On 18/01/2022 19.43, Andy Shevchenko wrote:
> > On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
> >>
> >> Hi everyone,
> >>
> >> This series contains just the fixes / misc improvements from the
> >> previously submitted series:
> >>
> >> brcmfmac: Support Apple T2 and M1 platforms
> >>
> >> Patches 8-9 aren't strictly bugfixes but rather just general
> >> improvements; they can be safely skipped, although patch 8 will be a
> >> dependency of the subsequent series to avoid a compile warning.
> >
> > Have I given you a tag? If so, I do not see it applied in the patches...
>
> I didn't see any review tags from you in the previous thread. Did I miss
> any?

I checked myself and indeed it seems my memory is about something
else, I'll check v3, last time I remember I found no (big) issues with
the fixes patches, I believe they are in shape.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

On 1/17/2022 3:29 PM, Hector Martin wrote:
> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
> the CLM blob is released in the device remove path.
> 
> Fixes: 82f93cf46d60 ("brcmfmac: get chip's default RAM info during PCIe setup")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 ++
>   1 file changed, 2 insertions(+)

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 2/9] brcmfmac: firmware: Allocate space for default boardrev in nvram

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On 1/17/2022 3:29 PM, Hector Martin wrote:
> If boardrev is missing from the NVRAM we add a default one, but this
> might need more space in the output buffer than was allocated. Ensure
> we have enough padding for this in the buffer.
> 
> Fixes: 46f2b38a91b0 ("brcmfmac: insert default boardrev in nvram data if missing")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 2 ++
>   1 file changed, 2 insertions(+)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

On 1/17/2022 3:29 PM, Hector Martin wrote:
> This unbreaks support for USB devices, which do not have a board_type
> to create an alt_path out of and thus were running into a NULL
> dereference.
> 
> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c | 3 +++
>   1 file changed, 3 insertions(+)

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 5/9] brcmfmac: pcie: Replace brcmf_pcie_copy_mem_todev with memcpy_toio

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 1443 bytes --]

On 1/17/2022 3:29 PM, Hector Martin wrote:
> The alignment check was wrong (e.g. & 4 instead of & 3), and the logic
> was also inefficient if the length was not a multiple of 4, since it
> would needlessly fall back to copying the entire buffer bytewise.
> 
> We already have a perfectly good memcpy_toio function, so just call that
> instead of rolling our own copy logic here. brcmf_pcie_init_ringbuffers
> was already using it anyway.
> 
> Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   .../broadcom/brcm80211/brcmfmac/pcie.c        | 48 ++-----------------
>   1 file changed, 4 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index b1ae6c41013f..c25f48db1f60 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -12,6 +12,7 @@
>   #include <linux/interrupt.h>
>   #include <linux/bcma/bcma.h>
>   #include <linux/sched.h>
> +#include <linux/io.h>

As brcmf_pcie_init_ringbuffers was already using it I suppose this was 
already implicitly included. Still good to make it explicit.

>   #include <asm/unaligned.h>
>   
>   #include <soc.h>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 7/9] brcmfmac: of: Use devm_kstrdup for board_type & check for errors

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On 1/17/2022 3:29 PM, Hector Martin wrote:
> This was missing a NULL check, and we can collapse the strlen/alloc/copy
> into a devm_kstrdup().

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> index 513c7e6421b2..5dc1e942e9e7 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
> @@ -79,8 +79,12 @@ void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
>   
>   		/* get rid of '/' in the compatible string to be able to find the FW */
>   		len = strlen(tmp) + 1;
> -		board_type = devm_kzalloc(dev, len, GFP_KERNEL);
> -		strscpy(board_type, tmp, len);
> +		board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);
> +		if (!board_type) {
> +			brcmf_err("out of memory allocating board_type\n");

Drop the message. mm will blurb out with a stack trace providing all the 
info you need.

> +			of_node_put(root);
> +			return;
> +		}
>   		for (i = 0; i < board_type[i]; i++) {
>   			if (board_type[i] == '/')
>   				board_type[i] = '-';

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 8/9] brcmfmac: fwil: Constify iovar name arguments

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]



On 1/17/2022 3:29 PM, Hector Martin wrote:
> Make all the iovar name arguments const char * instead of just char *.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   .../broadcom/brcm80211/brcmfmac/fwil.c        | 34 +++++++++----------
>   .../broadcom/brcm80211/brcmfmac/fwil.h        | 28 +++++++--------
>   2 files changed, 31 insertions(+), 31 deletions(-)

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 9/9] brcmfmac: pcie: Read the console on init and shutdown

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

On 1/17/2022 3:29 PM, Hector Martin wrote:
> This allows us to get console messages if the firmware crashed during
> early init, or if an operation failed and we're about to shut down.

We do have a module parameter that forces probing to succeed regardless 
any failure so we can create memory dump of the wifi device, read the 
console, etc. Still it can be useful to add these console read calls. 
Thanks.

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 4 ++++
>   1 file changed, 4 insertions(+)

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Andy Shevchenko]

On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>
> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
> the CLM blob is released in the device remove path.

...

>         if (ret) {

>                 brcmf_err(bus, "Failed to get RAM info\n");
> +               release_firmware(fw);
> +               brcmf_fw_nvram_free(nvram);

Can we first undo the things and only after print a message?

>                 goto fail;
>         }


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Dmitry Osipenko]

19.01.2022 20:49, Andy Shevchenko пишет:
> On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>>
>> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
>> the CLM blob is released in the device remove path.
> 
> ...
> 
>>         if (ret) {
> 
>>                 brcmf_err(bus, "Failed to get RAM info\n");
>> +               release_firmware(fw);
>> +               brcmf_fw_nvram_free(nvram);
> 
> Can we first undo the things and only after print a message?

Having message first usually is more preferred because at minimum you'll
get the message if "undoing the things" crashes, i.e. will be more
obvious what happened.

Re: [PATCH v3 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Andy Shevchenko]

On Wed, Jan 19, 2022 at 11:22 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 19.01.2022 20:49, Andy Shevchenko пишет:
> > On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
> >>
> >> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
> >> the CLM blob is released in the device remove path.
> >
> > ...
> >
> >>         if (ret) {
> >
> >>                 brcmf_err(bus, "Failed to get RAM info\n");
> >> +               release_firmware(fw);
> >> +               brcmf_fw_nvram_free(nvram);
> >
> > Can we first undo the things and only after print a message?
>
> Having message first usually is more preferred because at minimum you'll
> get the message if "undoing the things" crashes, i.e. will be more
> obvious what happened.

If "undo the things" crashes, I would rather like to see that crash
report, while serial UART at 9600 will continue flushing the message
and then hang without any pointers to what the heck happened. Not
here, but in general, messages are also good to be out of the locks.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/9] brcmfmac: firmware: Allocate space for default boardrev in nvram

[Andy Shevchenko]

On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>
> If boardrev is missing from the NVRAM we add a default one, but this
> might need more space in the output buffer than was allocated. Ensure
> we have enough padding for this in the buffer.

Do you know this ahead (before allocation happens)?
If yes, the size change should happen conditionally.

Alternatively, krealloc() may be used.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Andy Shevchenko]

On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>
> This unbreaks support for USB devices, which do not have a board_type
> to create an alt_path out of and thus were running into a NULL
> dereference.

In v5.16 we have two call sites:

1.
  if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
    ...
    alt_path = brcm_alt_fw_path(cur->path, fwctx->req->board_type);

2.
  alt_path = brcm_alt_fw_path(first->path, fwctx->req->board_type);
  if (alt_path) {
    ...

Looking at them I would rather expect to see (as a quick fix, the
better solution is to unify those call sites by splitting out a common
helper):

  if (fwctx->req->board_type) {
    alt_path = brcm_alt_fw_path(first->path, fwctx->req->board_type);
  else
    alt_path = NULL;
   ...


> @@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, const char *board_type)
>         char alt_path[BRCMF_FW_NAME_LEN];
>         char suffix[5];
>
> +       if (!board_type)
> +               return NULL;


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Dmitry Osipenko]

17.01.2022 17:29, Hector Martin пишет:
> This unbreaks support for USB devices, which do not have a board_type
> to create an alt_path out of and thus were running into a NULL
> dereference.
> 
> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
> Signed-off-by: Hector Martin <marcan@marcan.st>

Technically, all patches that are intended to be included into next
stable kernel update require the "Cc: stable@vger.kernel.org" tag.

In practice such patches usually auto-picked by the patch bot, so no
need to resend.

> ---
>  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 1001c8888bfe..63821856bbe1 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> @@ -599,6 +599,9 @@ static char *brcm_alt_fw_path(const char *path, const char *board_type)
>  	char alt_path[BRCMF_FW_NAME_LEN];
>  	char suffix[5];
>  
> +	if (!board_type)
> +		return NULL;
> +
>  	strscpy(alt_path, path, BRCMF_FW_NAME_LEN);
>  	/* At least one character + suffix */
>  	if (strlen(alt_path) < 5)

Good catch!

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>

Re: [PATCH v3 2/9] brcmfmac: firmware: Allocate space for default boardrev in nvram

[Hector Martin]

On 20/01/2022 06.35, Andy Shevchenko wrote:
> On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>>
>> If boardrev is missing from the NVRAM we add a default one, but this
>> might need more space in the output buffer than was allocated. Ensure
>> we have enough padding for this in the buffer.
> 
> Do you know this ahead (before allocation happens)?
> If yes, the size change should happen conditionally.
> 
> Alternatively, krealloc() may be used.

We don't know if we will have to add a boardrev until we find it's
missing. The buffer is already oversized anyway, as its size is computed
based on the input NVRAM size, while comments and such will be removed.
This is just increasing the existing worst-case allocation to properly
account for the true worst-case case.

I don't think trying to keep the buffer perfectly sized buys us anything
here, since it will be freed after download anyway. There's no point in
saving a few bytes of memory in the interim.

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Hector Martin]

On 20/01/2022 06.45, Andy Shevchenko wrote:
> On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>>
>> This unbreaks support for USB devices, which do not have a board_type
>> to create an alt_path out of and thus were running into a NULL
>> dereference.
> 
> In v5.16 we have two call sites:
> 
> 1.
>   if (cur->type == BRCMF_FW_TYPE_NVRAM && fwctx->req->board_type) {
>     ...
>     alt_path = brcm_alt_fw_path(cur->path, fwctx->req->board_type);
> 
> 2.
>   alt_path = brcm_alt_fw_path(first->path, fwctx->req->board_type);
>   if (alt_path) {
>     ...
> 
> Looking at them I would rather expect to see (as a quick fix, the
> better solution is to unify those call sites by splitting out a common
> helper):
> 
>   if (fwctx->req->board_type) {
>     alt_path = brcm_alt_fw_path(first->path, fwctx->req->board_type);
>   else
>     alt_path = NULL;
>    ...
> 

Since brcm_alt_fw_path can fail anyway, and its return value is already
NULL-checked, it makes sense to propagate the NULL board_path there
rather than doing it at all the callsites. That's a common pattern, e.g.
the entire DT API is designed to accept NULL nodes. That does mean that
the first callsite has a redundant NULL check, yes, but that doesn't hurt.

This is all going to change with subsequent patches anyway; the point of
this patch is just to fix the regression.

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

Re: [PATCH v3 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

On 1/19/2022 6:49 PM, Andy Shevchenko wrote:
> On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>>
>> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
>> the CLM blob is released in the device remove path.
> 
> ...
> 
>>          if (ret) {
> 
>>                  brcmf_err(bus, "Failed to get RAM info\n");
>> +               release_firmware(fw);
>> +               brcmf_fw_nvram_free(nvram);
> 
> Can we first undo the things and only after print a message?

What would be your motivation? When reading logs I am used to seeing an 
error message followed by cleanup related messages. Following your 
suggestion you could see cleanup related messages, the error print as 
above, followed by more cleanup related messages. The cleanup routine 
would preferably be silent, but I tend to flip on extra debug message 
levels.

Regards,
Arend

>>                  goto fail;
>>          }
> 
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

On 1/19/2022 11:02 PM, Dmitry Osipenko wrote:
> 17.01.2022 17:29, Hector Martin пишет:
>> This unbreaks support for USB devices, which do not have a board_type
>> to create an alt_path out of and thus were running into a NULL
>> dereference.
>>
>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware binaries")
>> Signed-off-by: Hector Martin <marcan@marcan.st>
> 
> Technically, all patches that are intended to be included into next
> stable kernel update require the "Cc: stable@vger.kernel.org" tag.

Being the nit picker that I am I would say it is recommended to safe 
yourself extra work, not required, for the reason you give below.

> In practice such patches usually auto-picked by the patch bot, so no
> need to resend.

Regards,
Arend

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v3 5/9] brcmfmac: pcie: Replace brcmf_pcie_copy_mem_todev with memcpy_toio

[Andy Shevchenko]

On Mon, Jan 17, 2022 at 4:31 PM Hector Martin <marcan@marcan.st> wrote:
>
> The alignment check was wrong (e.g. & 4 instead of & 3), and the logic
> was also inefficient if the length was not a multiple of 4, since it
> would needlessly fall back to copying the entire buffer bytewise.
>
> We already have a perfectly good memcpy_toio function, so just call that
> instead of rolling our own copy logic here. brcmf_pcie_init_ringbuffers
> was already using it anyway.

My gosh, what a nice fix!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../broadcom/brcm80211/brcmfmac/pcie.c        | 48 ++-----------------
>  1 file changed, 4 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index b1ae6c41013f..c25f48db1f60 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -12,6 +12,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/bcma/bcma.h>
>  #include <linux/sched.h>
> +#include <linux/io.h>
>  #include <asm/unaligned.h>
>
>  #include <soc.h>
> @@ -454,47 +455,6 @@ brcmf_pcie_write_ram32(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
>  }
>
>
> -static void
> -brcmf_pcie_copy_mem_todev(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
> -                         void *srcaddr, u32 len)
> -{
> -       void __iomem *address = devinfo->tcm + mem_offset;
> -       __le32 *src32;
> -       __le16 *src16;
> -       u8 *src8;
> -
> -       if (((ulong)address & 4) || ((ulong)srcaddr & 4) || (len & 4)) {
> -               if (((ulong)address & 2) || ((ulong)srcaddr & 2) || (len & 2)) {
> -                       src8 = (u8 *)srcaddr;
> -                       while (len) {
> -                               iowrite8(*src8, address);
> -                               address++;
> -                               src8++;
> -                               len--;
> -                       }
> -               } else {
> -                       len = len / 2;
> -                       src16 = (__le16 *)srcaddr;
> -                       while (len) {
> -                               iowrite16(le16_to_cpu(*src16), address);
> -                               address += 2;
> -                               src16++;
> -                               len--;
> -                       }
> -               }
> -       } else {
> -               len = len / 4;
> -               src32 = (__le32 *)srcaddr;
> -               while (len) {
> -                       iowrite32(le32_to_cpu(*src32), address);
> -                       address += 4;
> -                       src32++;
> -                       len--;
> -               }
> -       }
> -}
> -
> -
>  static void
>  brcmf_pcie_copy_dev_tomem(struct brcmf_pciedev_info *devinfo, u32 mem_offset,
>                           void *dstaddr, u32 len)
> @@ -1570,8 +1530,8 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
>                 return err;
>
>         brcmf_dbg(PCIE, "Download FW %s\n", devinfo->fw_name);
> -       brcmf_pcie_copy_mem_todev(devinfo, devinfo->ci->rambase,
> -                                 (void *)fw->data, fw->size);
> +       memcpy_toio(devinfo->tcm + devinfo->ci->rambase,
> +                   (void *)fw->data, fw->size);
>
>         resetintr = get_unaligned_le32(fw->data);
>         release_firmware(fw);
> @@ -1585,7 +1545,7 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
>                 brcmf_dbg(PCIE, "Download NVRAM %s\n", devinfo->nvram_name);
>                 address = devinfo->ci->rambase + devinfo->ci->ramsize -
>                           nvram_len;
> -               brcmf_pcie_copy_mem_todev(devinfo, address, nvram, nvram_len);
> +               memcpy_toio(devinfo->tcm + address, nvram, nvram_len);
>                 brcmf_fw_nvram_free(nvram);
>         } else {
>                 brcmf_dbg(PCIE, "No matching NVRAM file found %s\n",
> --
> 2.33.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 7/9] brcmfmac: of: Use devm_kstrdup for board_type & check for errors

[Andy Shevchenko]

On Mon, Jan 17, 2022 at 4:31 PM Hector Martin <marcan@marcan.st> wrote:
>
> This was missing a NULL check, and we can collapse the strlen/alloc/copy
> into a devm_kstrdup().

Nice patch. After dropping the message,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>                 /* get rid of '/' in the compatible string to be able to find the FW */
>                 len = strlen(tmp) + 1;
> -               board_type = devm_kzalloc(dev, len, GFP_KERNEL);
> -               strscpy(board_type, tmp, len);
> +               board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);
> +               if (!board_type) {
> +                       brcmf_err("out of memory allocating board_type\n");
> +                       of_node_put(root);
> +                       return;
> +               }

>                 for (i = 0; i < board_type[i]; i++) {
>                         if (board_type[i] == '/')
>                                 board_type[i] = '-';

Next step is to replace this with NIH strreplace()

And
  of_property_read_string_index(root, "compatible", 0, &tmp);
with
  of_property_read_string(root, "compatible", &tmp);

And might add an error check, but I believe if there is no compatible
property present, this can't be called.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 8/9] brcmfmac: fwil: Constify iovar name arguments

[Andy Shevchenko]

On Mon, Jan 17, 2022 at 4:31 PM Hector Martin <marcan@marcan.st> wrote:
>
> Make all the iovar name arguments const char * instead of just char *.

Makes sense.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../broadcom/brcm80211/brcmfmac/fwil.c        | 34 +++++++++----------
>  .../broadcom/brcm80211/brcmfmac/fwil.h        | 28 +++++++--------
>  2 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
> index d5578ca681bb..72fe8bce6eaf 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
> @@ -192,7 +192,7 @@ brcmf_fil_cmd_int_get(struct brcmf_if *ifp, u32 cmd, u32 *data)
>  }
>
>  static u32
> -brcmf_create_iovar(char *name, const char *data, u32 datalen,
> +brcmf_create_iovar(const char *name, const char *data, u32 datalen,
>                    char *buf, u32 buflen)
>  {
>         u32 len;
> @@ -213,7 +213,7 @@ brcmf_create_iovar(char *name, const char *data, u32 datalen,
>
>
>  s32
> -brcmf_fil_iovar_data_set(struct brcmf_if *ifp, char *name, const void *data,
> +brcmf_fil_iovar_data_set(struct brcmf_if *ifp, const char *name, const void *data,
>                          u32 len)
>  {
>         struct brcmf_pub *drvr = ifp->drvr;
> @@ -241,7 +241,7 @@ brcmf_fil_iovar_data_set(struct brcmf_if *ifp, char *name, const void *data,
>  }
>
>  s32
> -brcmf_fil_iovar_data_get(struct brcmf_if *ifp, char *name, void *data,
> +brcmf_fil_iovar_data_get(struct brcmf_if *ifp, const char *name, void *data,
>                          u32 len)
>  {
>         struct brcmf_pub *drvr = ifp->drvr;
> @@ -272,7 +272,7 @@ brcmf_fil_iovar_data_get(struct brcmf_if *ifp, char *name, void *data,
>  }
>
>  s32
> -brcmf_fil_iovar_int_set(struct brcmf_if *ifp, char *name, u32 data)
> +brcmf_fil_iovar_int_set(struct brcmf_if *ifp, const char *name, u32 data)
>  {
>         __le32 data_le = cpu_to_le32(data);
>
> @@ -280,7 +280,7 @@ brcmf_fil_iovar_int_set(struct brcmf_if *ifp, char *name, u32 data)
>  }
>
>  s32
> -brcmf_fil_iovar_int_get(struct brcmf_if *ifp, char *name, u32 *data)
> +brcmf_fil_iovar_int_get(struct brcmf_if *ifp, const char *name, u32 *data)
>  {
>         __le32 data_le = cpu_to_le32(*data);
>         s32 err;
> @@ -292,7 +292,7 @@ brcmf_fil_iovar_int_get(struct brcmf_if *ifp, char *name, u32 *data)
>  }
>
>  static u32
> -brcmf_create_bsscfg(s32 bsscfgidx, char *name, char *data, u32 datalen,
> +brcmf_create_bsscfg(s32 bsscfgidx, const char *name, char *data, u32 datalen,
>                     char *buf, u32 buflen)
>  {
>         const s8 *prefix = "bsscfg:";
> @@ -337,7 +337,7 @@ brcmf_create_bsscfg(s32 bsscfgidx, char *name, char *data, u32 datalen,
>  }
>
>  s32
> -brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, char *name,
> +brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, const char *name,
>                           void *data, u32 len)
>  {
>         struct brcmf_pub *drvr = ifp->drvr;
> @@ -366,7 +366,7 @@ brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, char *name,
>  }
>
>  s32
> -brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, char *name,
> +brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, const char *name,
>                           void *data, u32 len)
>  {
>         struct brcmf_pub *drvr = ifp->drvr;
> @@ -396,7 +396,7 @@ brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, char *name,
>  }
>
>  s32
> -brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, char *name, u32 data)
> +brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, const char *name, u32 data)
>  {
>         __le32 data_le = cpu_to_le32(data);
>
> @@ -405,7 +405,7 @@ brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, char *name, u32 data)
>  }
>
>  s32
> -brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, char *name, u32 *data)
> +brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, const char *name, u32 *data)
>  {
>         __le32 data_le = cpu_to_le32(*data);
>         s32 err;
> @@ -417,7 +417,7 @@ brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, char *name, u32 *data)
>         return err;
>  }
>
> -static u32 brcmf_create_xtlv(char *name, u16 id, char *data, u32 len,
> +static u32 brcmf_create_xtlv(const char *name, u16 id, char *data, u32 len,
>                              char *buf, u32 buflen)
>  {
>         u32 iolen;
> @@ -438,7 +438,7 @@ static u32 brcmf_create_xtlv(char *name, u16 id, char *data, u32 len,
>         return iolen;
>  }
>
> -s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, char *name, u16 id,
> +s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, const char *name, u16 id,
>                             void *data, u32 len)
>  {
>         struct brcmf_pub *drvr = ifp->drvr;
> @@ -466,7 +466,7 @@ s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, char *name, u16 id,
>         return err;
>  }
>
> -s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, char *name, u16 id,
> +s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, const char *name, u16 id,
>                             void *data, u32 len)
>  {
>         struct brcmf_pub *drvr = ifp->drvr;
> @@ -495,7 +495,7 @@ s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, char *name, u16 id,
>         return err;
>  }
>
> -s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, char *name, u16 id, u32 data)
> +s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, const char *name, u16 id, u32 data)
>  {
>         __le32 data_le = cpu_to_le32(data);
>
> @@ -503,7 +503,7 @@ s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, char *name, u16 id, u32 data)
>                                          sizeof(data_le));
>  }
>
> -s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, char *name, u16 id, u32 *data)
> +s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, const char *name, u16 id, u32 *data)
>  {
>         __le32 data_le = cpu_to_le32(*data);
>         s32 err;
> @@ -514,12 +514,12 @@ s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, char *name, u16 id, u32 *data)
>         return err;
>  }
>
> -s32 brcmf_fil_xtlv_int8_get(struct brcmf_if *ifp, char *name, u16 id, u8 *data)
> +s32 brcmf_fil_xtlv_int8_get(struct brcmf_if *ifp, const char *name, u16 id, u8 *data)
>  {
>         return brcmf_fil_xtlv_data_get(ifp, name, id, data, sizeof(*data));
>  }
>
> -s32 brcmf_fil_xtlv_int16_get(struct brcmf_if *ifp, char *name, u16 id, u16 *data)
> +s32 brcmf_fil_xtlv_int16_get(struct brcmf_if *ifp, const char *name, u16 id, u16 *data)
>  {
>         __le16 data_le = cpu_to_le16(*data);
>         s32 err;
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
> index cb26f8c59c21..bc693157c4b1 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.h
> @@ -84,26 +84,26 @@ s32 brcmf_fil_cmd_data_get(struct brcmf_if *ifp, u32 cmd, void *data, u32 len);
>  s32 brcmf_fil_cmd_int_set(struct brcmf_if *ifp, u32 cmd, u32 data);
>  s32 brcmf_fil_cmd_int_get(struct brcmf_if *ifp, u32 cmd, u32 *data);
>
> -s32 brcmf_fil_iovar_data_set(struct brcmf_if *ifp, char *name, const void *data,
> +s32 brcmf_fil_iovar_data_set(struct brcmf_if *ifp, const char *name, const void *data,
>                              u32 len);
> -s32 brcmf_fil_iovar_data_get(struct brcmf_if *ifp, char *name, void *data,
> +s32 brcmf_fil_iovar_data_get(struct brcmf_if *ifp, const char *name, void *data,
>                              u32 len);
> -s32 brcmf_fil_iovar_int_set(struct brcmf_if *ifp, char *name, u32 data);
> -s32 brcmf_fil_iovar_int_get(struct brcmf_if *ifp, char *name, u32 *data);
> +s32 brcmf_fil_iovar_int_set(struct brcmf_if *ifp, const char *name, u32 data);
> +s32 brcmf_fil_iovar_int_get(struct brcmf_if *ifp, const char *name, u32 *data);
>
> -s32 brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, char *name, void *data,
> +s32 brcmf_fil_bsscfg_data_set(struct brcmf_if *ifp, const char *name, void *data,
>                               u32 len);
> -s32 brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, char *name, void *data,
> +s32 brcmf_fil_bsscfg_data_get(struct brcmf_if *ifp, const char *name, void *data,
>                               u32 len);
> -s32 brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, char *name, u32 data);
> -s32 brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, char *name, u32 *data);
> -s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, char *name, u16 id,
> +s32 brcmf_fil_bsscfg_int_set(struct brcmf_if *ifp, const char *name, u32 data);
> +s32 brcmf_fil_bsscfg_int_get(struct brcmf_if *ifp, const char *name, u32 *data);
> +s32 brcmf_fil_xtlv_data_set(struct brcmf_if *ifp, const char *name, u16 id,
>                             void *data, u32 len);
> -s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, char *name, u16 id,
> +s32 brcmf_fil_xtlv_data_get(struct brcmf_if *ifp, const char *name, u16 id,
>                             void *data, u32 len);
> -s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, char *name, u16 id, u32 data);
> -s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, char *name, u16 id, u32 *data);
> -s32 brcmf_fil_xtlv_int8_get(struct brcmf_if *ifp, char *name, u16 id, u8 *data);
> -s32 brcmf_fil_xtlv_int16_get(struct brcmf_if *ifp, char *name, u16 id, u16 *data);
> +s32 brcmf_fil_xtlv_int_set(struct brcmf_if *ifp, const char *name, u16 id, u32 data);
> +s32 brcmf_fil_xtlv_int_get(struct brcmf_if *ifp, const char *name, u16 id, u32 *data);
> +s32 brcmf_fil_xtlv_int8_get(struct brcmf_if *ifp, const char *name, u16 id, u8 *data);
> +s32 brcmf_fil_xtlv_int16_get(struct brcmf_if *ifp, const char *name, u16 id, u16 *data);
>
>  #endif /* _fwil_h_ */
> --
> 2.33.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/9] brcmfmac: pcie: Release firmwares in the brcmf_pcie_setup error path

[Dmitry Osipenko]

20.01.2022 00:31, Andy Shevchenko пишет:
> On Wed, Jan 19, 2022 at 11:22 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 19.01.2022 20:49, Andy Shevchenko пишет:
>>> On Mon, Jan 17, 2022 at 4:30 PM Hector Martin <marcan@marcan.st> wrote:
>>>>
>>>> This avoids leaking memory if brcmf_chip_get_raminfo fails. Note that
>>>> the CLM blob is released in the device remove path.
>>>
>>> ...
>>>
>>>>         if (ret) {
>>>
>>>>                 brcmf_err(bus, "Failed to get RAM info\n");
>>>> +               release_firmware(fw);
>>>> +               brcmf_fw_nvram_free(nvram);
>>>
>>> Can we first undo the things and only after print a message?
>>
>> Having message first usually is more preferred because at minimum you'll
>> get the message if "undoing the things" crashes, i.e. will be more
>> obvious what happened.
> 
> If "undo the things" crashes, I would rather like to see that crash
> report, while serial UART at 9600 will continue flushing the message
> and then hang without any pointers to what the heck happened. Not
> here, but in general, messages are also good to be out of the locks.

The hang is actually a better example. It's the most annoying when there
is a silent hang and no error messages.

Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Dmitry Osipenko]

20.01.2022 11:29, Arend van Spriel пишет:
> On 1/19/2022 11:02 PM, Dmitry Osipenko wrote:
>> 17.01.2022 17:29, Hector Martin пишет:
>>> This unbreaks support for USB devices, which do not have a board_type
>>> to create an alt_path out of and thus were running into a NULL
>>> dereference.
>>>
>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>> binaries")
>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>
>> Technically, all patches that are intended to be included into next
>> stable kernel update require the "Cc: stable@vger.kernel.org" tag.
> 
> Being the nit picker that I am I would say it is recommended to safe
> yourself extra work, not required, for the reason you give below.

Will be nice if stable tag could officially become a recommendation,
implying the stable tag. It's a requirement today, at least Greg KH
always demands to add it :)

Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Dmitry Osipenko]

20.01.2022 16:23, Dmitry Osipenko пишет:
> 20.01.2022 11:29, Arend van Spriel пишет:
>> On 1/19/2022 11:02 PM, Dmitry Osipenko wrote:
>>> 17.01.2022 17:29, Hector Martin пишет:
>>>> This unbreaks support for USB devices, which do not have a board_type
>>>> to create an alt_path out of and thus were running into a NULL
>>>> dereference.
>>>>
>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>> binaries")
>>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>>
>>> Technically, all patches that are intended to be included into next
>>> stable kernel update require the "Cc: stable@vger.kernel.org" tag.
>>
>> Being the nit picker that I am I would say it is recommended to safe
>> yourself extra work, not required, for the reason you give below.
> 
> Will be nice if stable tag could officially become a recommendation,
> implying the stable tag. It's a requirement today, at least Greg KH
> always demands to add it :)

*implying the stable tag if "fixes" tag presents.

Re: [PATCH v3 3/9] brcmfmac: firmware: Do not crash on a NULL board_type

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

On 1/20/2022 2:24 PM, Dmitry Osipenko wrote:
> 20.01.2022 16:23, Dmitry Osipenko пишет:
>> 20.01.2022 11:29, Arend van Spriel пишет:
>>> On 1/19/2022 11:02 PM, Dmitry Osipenko wrote:
>>>> 17.01.2022 17:29, Hector Martin пишет:
>>>>> This unbreaks support for USB devices, which do not have a board_type
>>>>> to create an alt_path out of and thus were running into a NULL
>>>>> dereference.
>>>>>
>>>>> Fixes: 5ff013914c62 ("brcmfmac: firmware: Allow per-board firmware
>>>>> binaries")
>>>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>>>
>>>> Technically, all patches that are intended to be included into next
>>>> stable kernel update require the "Cc: stable@vger.kernel.org" tag.
>>>
>>> Being the nit picker that I am I would say it is recommended to safe
>>> yourself extra work, not required, for the reason you give below.
>>
>> Will be nice if stable tag could officially become a recommendation,
>> implying the stable tag. It's a requirement today, at least Greg KH
>> always demands to add it :)
> 
> *implying the stable tag if "fixes" tag presents.

I was a little confused reading your previous email in this thread. This 
makes a lot more sense :-p

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]