2017-10-05 07:26:37

by Wright Feng

[permalink] [raw]
Subject: [PATCH v6] brcmfmac: add CLM download support

From: Chung-Hsien Hsu <[email protected]>

The firmware for brcmfmac devices includes information regarding
regulatory constraints. For certain devices this information is kept
separately in a binary form that needs to be downloaded to the device.
This patch adds support to download this so-called CLM blob file. It
uses the same naming scheme as the other firmware files with extension
of .clm_blob.

The CLM blob file is optional. If the file does not exist, the download
process will be bypassed. It will not affect the driver loading.

Signed-off-by: Chung-Hsien Hsu <[email protected]>
---
v2: Revise commit message to describe in more detail
v3: Add error handling in brcmf_c_get_clm_name function
v4: Correct the length of dload_buf in brcmf_c_download function
v5: Remove unnecessary cast and alignment
v6: Add debug log for the case of no CLM file present
---
.../net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 10 ++
.../wireless/broadcom/brcm80211/brcmfmac/common.c | 162 +++++++++++++++++++++
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +
.../wireless/broadcom/brcm80211/brcmfmac/core.h | 2 +
.../broadcom/brcm80211/brcmfmac/fwil_types.h | 31 ++++
.../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 19 +++
.../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 19 +++
.../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 18 +++
8 files changed, 263 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
index b55c329..df42e09 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
@@ -71,6 +71,7 @@ struct brcmf_bus_dcmd {
* @wowl_config: specify if dongle is configured for wowl when going to suspend
* @get_ramsize: obtain size of device memory.
* @get_memdump: obtain device memory dump in provided buffer.
+ * @get_fwname: obtain firmware name.
*
* This structure provides an abstract interface towards the
* bus specific driver. For control messages to common driver
@@ -87,6 +88,8 @@ struct brcmf_bus_ops {
void (*wowl_config)(struct device *dev, bool enabled);
size_t (*get_ramsize)(struct device *dev);
int (*get_memdump)(struct device *dev, void *data, size_t len);
+ int (*get_fwname)(struct device *dev, uint chip, uint chiprev,
+ unsigned char *fw_name);
};


@@ -214,6 +217,13 @@ int brcmf_bus_get_memdump(struct brcmf_bus *bus, void *data, size_t len)
return bus->ops->get_memdump(bus->dev, data, len);
}

+static inline
+int brcmf_bus_get_fwname(struct brcmf_bus *bus, uint chip, uint chiprev,
+ unsigned char *fw_name)
+{
+ return bus->ops->get_fwname(bus->dev, chip, chiprev, fw_name);
+}
+
/*
* interface functions from common layer
*/
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 7a2b495..5397727 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -18,6 +18,7 @@
#include <linux/string.h>
#include <linux/netdevice.h>
#include <linux/module.h>
+#include <linux/firmware.h>
#include <brcmu_wifi.h>
#include <brcmu_utils.h>
#include "core.h"
@@ -28,6 +29,7 @@
#include "tracepoint.h"
#include "common.h"
#include "of.h"
+#include "firmware.h"

MODULE_AUTHOR("Broadcom Corporation");
MODULE_DESCRIPTION("Broadcom 802.11 wireless LAN fullmac driver.");
@@ -104,12 +106,140 @@ void brcmf_c_set_joinpref_default(struct brcmf_if *ifp)
brcmf_err("Set join_pref error (%d)\n", err);
}

+static int brcmf_c_download(struct brcmf_if *ifp, u16 flag,
+ struct brcmf_dload_data_le *dload_buf,
+ u32 len)
+{
+ s32 err;
+
+ flag |= (DLOAD_HANDLER_VER << DLOAD_FLAG_VER_SHIFT);
+ dload_buf->flag = cpu_to_le16(flag);
+ dload_buf->dload_type = cpu_to_le16(DL_TYPE_CLM);
+ dload_buf->len = cpu_to_le32(len);
+ dload_buf->crc = cpu_to_le32(0);
+ len = sizeof(*dload_buf) + len - 1;
+
+ err = brcmf_fil_iovar_data_set(ifp, "clmload", dload_buf, len);
+
+ return err;
+}
+
+static int brcmf_c_get_clm_name(struct brcmf_if *ifp, u8 *clm_name)
+{
+ struct brcmf_bus *bus = ifp->drvr->bus_if;
+ struct brcmf_rev_info *ri = &ifp->drvr->revinfo;
+ u8 fw_name[BRCMF_FW_NAME_LEN];
+ u8 *ptr;
+ size_t len;
+ s32 err;
+
+ memset(fw_name, 0, BRCMF_FW_NAME_LEN);
+ err = brcmf_bus_get_fwname(bus, ri->chipnum, ri->chiprev, fw_name);
+ if (err) {
+ brcmf_err("get firmware name failed (%d)\n", err);
+ goto done;
+ }
+
+ /* generate CLM blob file name */
+ ptr = strrchr(fw_name, '.');
+ if (!ptr) {
+ err = -ENOENT;
+ goto done;
+ }
+
+ len = ptr - fw_name + 1;
+ if (len + strlen(".clm_blob") > BRCMF_FW_NAME_LEN) {
+ err = -E2BIG;
+ } else {
+ strlcpy(clm_name, fw_name, len);
+ strlcat(clm_name, ".clm_blob", BRCMF_FW_NAME_LEN);
+ }
+done:
+ return err;
+}
+
+static int brcmf_c_process_clm_blob(struct brcmf_if *ifp)
+{
+ struct device *dev = ifp->drvr->bus_if->dev;
+ struct brcmf_dload_data_le *chunk_buf;
+ const struct firmware *clm = NULL;
+ u8 clm_name[BRCMF_FW_NAME_LEN];
+ u32 chunk_len;
+ u32 datalen;
+ u32 cumulative_len;
+ u16 dl_flag = DL_BEGIN;
+ u32 status;
+ s32 err;
+
+ brcmf_dbg(TRACE, "Enter\n");
+
+ memset(clm_name, 0, BRCMF_FW_NAME_LEN);
+ err = brcmf_c_get_clm_name(ifp, clm_name);
+ if (err) {
+ brcmf_err("get CLM blob file name failed (%d)\n", err);
+ return err;
+ }
+
+ err = request_firmware(&clm, clm_name, dev);
+ if (err) {
+ if (err == -ENOENT) {
+ brcmf_dbg(INFO, "continue with CLM data currently present in firmware\n");
+ return 0;
+ }
+ brcmf_err("request CLM blob file failed (%d)\n", err);
+ return err;
+ }
+
+ chunk_buf = kzalloc(sizeof(*chunk_buf) + MAX_CHUNK_LEN - 1, GFP_KERNEL);
+ if (!chunk_buf) {
+ err = -ENOMEM;
+ goto done;
+ }
+
+ datalen = clm->size;
+ cumulative_len = 0;
+ do {
+ if (datalen > MAX_CHUNK_LEN) {
+ chunk_len = MAX_CHUNK_LEN;
+ } else {
+ chunk_len = datalen;
+ dl_flag |= DL_END;
+ }
+ memcpy(chunk_buf->data, clm->data + cumulative_len, chunk_len);
+
+ err = brcmf_c_download(ifp, dl_flag, chunk_buf, chunk_len);
+
+ dl_flag &= ~DL_BEGIN;
+
+ cumulative_len += chunk_len;
+ datalen -= chunk_len;
+ } while ((datalen > 0) && (err == 0));
+
+ if (err) {
+ brcmf_err("clmload (%zu byte file) failed (%d); ",
+ clm->size, err);
+ /* Retrieve clmload_status and print */
+ err = brcmf_fil_iovar_int_get(ifp, "clmload_status", &status);
+ if (err)
+ brcmf_err("get clmload_status failed (%d)\n", err);
+ else
+ brcmf_dbg(INFO, "clmload_status=%d\n", status);
+ err = -EIO;
+ }
+
+ kfree(chunk_buf);
+done:
+ release_firmware(clm);
+ return err;
+}
+
int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
{
s8 eventmask[BRCMF_EVENTING_MASK_LEN];
u8 buf[BRCMF_DCMD_SMLEN];
struct brcmf_rev_info_le revinfo;
struct brcmf_rev_info *ri;
+ char *clmver;
char *ptr;
s32 err;

@@ -148,6 +278,13 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
}
ri->result = err;

+ /* Do any CLM downloading */
+ err = brcmf_c_process_clm_blob(ifp);
+ if (err < 0) {
+ brcmf_err("download CLM blob file failed, %d\n", err);
+ goto done;
+ }
+
/* query for 'ver' to get version info from firmware */
memset(buf, 0, sizeof(buf));
strcpy(buf, "ver");
@@ -167,6 +304,31 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
ptr = strrchr(buf, ' ') + 1;
strlcpy(ifp->drvr->fwver, ptr, sizeof(ifp->drvr->fwver));

+ /* Query for 'clmver' to get CLM version info from firmware */
+ memset(buf, 0, sizeof(buf));
+ err = brcmf_fil_iovar_data_get(ifp, "clmver", buf, sizeof(buf));
+ if (err) {
+ if (err == -23) {
+ brcmf_dbg(INFO, "clmver iovar unsupported, %d\n", err);
+ } else {
+ brcmf_err("retrieving clmver failed, %d\n", err);
+ goto done;
+ }
+ } else {
+ clmver = (char *)buf;
+ /* store CLM version for adding it to revinfo debugfs file */
+ memcpy(ifp->drvr->clmver, clmver, sizeof(ifp->drvr->clmver));
+
+ /* Replace all newline/linefeed characters with space
+ * character
+ */
+ ptr = clmver;
+ while ((ptr = strnchr(ptr, '\n', sizeof(buf))) != NULL)
+ *ptr = ' ';
+
+ brcmf_dbg(INFO, "CLM version = %s\n", clmver);
+ }
+
/* set mpc */
err = brcmf_fil_iovar_int_set(ifp, "mpc", 1);
if (err) {
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 511d190..7414dff 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -941,6 +941,8 @@ static int brcmf_revinfo_read(struct seq_file *s, void *data)
seq_printf(s, "anarev: %u\n", ri->anarev);
seq_printf(s, "nvramrev: %08x\n", ri->nvramrev);

+ seq_printf(s, "clmrev: %s\n", bus_if->drvr->clmver);
+
return 0;
}

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index a4dd313..ae39128 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -141,6 +141,8 @@ struct brcmf_pub {
struct notifier_block inetaddr_notifier;
struct notifier_block inet6addr_notifier;
struct brcmf_mp_device *settings;
+
+ u8 clmver[BRCMF_DCMD_SMLEN];
};

/* forward declarations */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index 9a1eb5a..fffe347 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -147,6 +147,21 @@
#define BRCMF_MFP_CAPABLE 1
#define BRCMF_MFP_REQUIRED 2

+/* MAX_CHUNK_LEN is the maximum length for data passing to firmware in each
+ * ioctl. It is relatively small because firmware has small maximum size input
+ * playload restriction for ioctls.
+ */
+#define MAX_CHUNK_LEN 1400
+
+#define DLOAD_HANDLER_VER 1 /* Downloader version */
+#define DLOAD_FLAG_VER_MASK 0xf000 /* Downloader version mask */
+#define DLOAD_FLAG_VER_SHIFT 12 /* Downloader version shift */
+
+#define DL_BEGIN 0x0002
+#define DL_END 0x0004
+
+#define DL_TYPE_CLM 2
+
/* join preference types for join_pref iovar */
enum brcmf_join_pref_types {
BRCMF_JOIN_PREF_RSSI = 1,
@@ -835,4 +850,20 @@ struct brcmf_gtk_keyinfo_le {
u8 replay_counter[BRCMF_RSN_REPLAY_LEN];
};

+/**
+ * struct brcmf_dload_data_le - data passing to firmware for downloading
+ * @flag: flags related to download data.
+ * @dload_type: type of download data.
+ * @len: length in bytes of download data.
+ * @crc: crc of download data.
+ * @data: download data.
+ */
+struct brcmf_dload_data_le {
+ __le16 flag;
+ __le16 dload_type;
+ __le32 len;
+ __le32 crc;
+ u8 data[1];
+};
+
#endif /* FWIL_TYPES_H_ */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index f878706..b370ee7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1350,6 +1350,24 @@ static int brcmf_pcie_get_memdump(struct device *dev, void *data, size_t len)
return 0;
}

+static int brcmf_pcie_get_fwname(struct device *dev, u32 chip, u32 chiprev,
+ u8 *fw_name)
+{
+ struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+ struct brcmf_pciedev *buspub = bus_if->bus_priv.pcie;
+ struct brcmf_pciedev_info *devinfo = buspub->devinfo;
+ int ret = 0;
+
+ if (devinfo->fw_name[0] != '\0')
+ strlcpy(fw_name, devinfo->fw_name, BRCMF_FW_NAME_LEN);
+ else
+ ret = brcmf_fw_map_chip_to_name(chip, chiprev,
+ brcmf_pcie_fwnames,
+ ARRAY_SIZE(brcmf_pcie_fwnames),
+ fw_name, NULL);
+
+ return ret;
+}

static const struct brcmf_bus_ops brcmf_pcie_bus_ops = {
.txdata = brcmf_pcie_tx,
@@ -1359,6 +1377,7 @@ static int brcmf_pcie_get_memdump(struct device *dev, void *data, size_t len)
.wowl_config = brcmf_pcie_wowl_config,
.get_ramsize = brcmf_pcie_get_ramsize,
.get_memdump = brcmf_pcie_get_memdump,
+ .get_fwname = brcmf_pcie_get_fwname,
};


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index b1789b1..3a71ca0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3972,6 +3972,24 @@ static void brcmf_sdio_buscore_write32(void *ctx, u32 addr, u32 val)
}
}

+static int brcmf_sdio_get_fwname(struct device *dev, u32 chip, u32 chiprev,
+ u8 *fw_name)
+{
+ struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+ struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+ int ret = 0;
+
+ if (sdiodev->fw_name[0] != '\0')
+ strlcpy(fw_name, sdiodev->fw_name, BRCMF_FW_NAME_LEN);
+ else
+ ret = brcmf_fw_map_chip_to_name(chip, chiprev,
+ brcmf_sdio_fwnames,
+ ARRAY_SIZE(brcmf_sdio_fwnames),
+ fw_name, NULL);
+
+ return ret;
+}
+
static const struct brcmf_bus_ops brcmf_sdio_bus_ops = {
.stop = brcmf_sdio_bus_stop,
.preinit = brcmf_sdio_bus_preinit,
@@ -3982,6 +4000,7 @@ static void brcmf_sdio_buscore_write32(void *ctx, u32 addr, u32 val)
.wowl_config = brcmf_sdio_wowl_config,
.get_ramsize = brcmf_sdio_bus_get_ramsize,
.get_memdump = brcmf_sdio_bus_get_memdump,
+ .get_fwname = brcmf_sdio_get_fwname,
};

static void brcmf_sdio_firmware_callback(struct device *dev, int err,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 8f20a4b..9622dd9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -1128,12 +1128,30 @@ static void brcmf_usb_wowl_config(struct device *dev, bool enabled)
device_set_wakeup_enable(devinfo->dev, false);
}

+static int brcmf_usb_get_fwname(struct device *dev, u32 chip, u32 chiprev,
+ u8 *fw_name)
+{
+ struct brcmf_usbdev_info *devinfo = brcmf_usb_get_businfo(dev);
+ int ret = 0;
+
+ if (devinfo->fw_name[0] != '\0')
+ strlcpy(fw_name, devinfo->fw_name, BRCMF_FW_NAME_LEN);
+ else
+ ret = brcmf_fw_map_chip_to_name(chip, chiprev,
+ brcmf_usb_fwnames,
+ ARRAY_SIZE(brcmf_usb_fwnames),
+ fw_name, NULL);
+
+ return ret;
+}
+
static const struct brcmf_bus_ops brcmf_usb_bus_ops = {
.txdata = brcmf_usb_tx,
.stop = brcmf_usb_down,
.txctl = brcmf_usb_tx_ctlpkt,
.rxctl = brcmf_usb_rx_ctlpkt,
.wowl_config = brcmf_usb_wowl_config,
+ .get_fwname = brcmf_usb_get_fwname,
};

static int brcmf_usb_bus_setup(struct brcmf_usbdev_info *devinfo)
--
1.9.1


2017-11-03 08:27:31

by Chung-Hsien Hsu

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

On Thu, Oct 05, 2017 at 03:31:18PM +0800, Wright Feng wrote:
> From: Chung-Hsien Hsu <[email protected]>
>
> The firmware for brcmfmac devices includes information regarding
> regulatory constraints. For certain devices this information is kept
> separately in a binary form that needs to be downloaded to the device.
> This patch adds support to download this so-called CLM blob file. It
> uses the same naming scheme as the other firmware files with extension
> of .clm_blob.
>
> The CLM blob file is optional. If the file does not exist, the download
> process will be bypassed. It will not affect the driver loading.
>
> Signed-off-by: Chung-Hsien Hsu <[email protected]>
> ---
> v2: Revise commit message to describe in more detail
> v3: Add error handling in brcmf_c_get_clm_name function
> v4: Correct the length of dload_buf in brcmf_c_download function
> v5: Remove unnecessary cast and alignment
> v6: Add debug log for the case of no CLM file present
> ---
> .../net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 10 ++
> .../wireless/broadcom/brcm80211/brcmfmac/common.c | 162 +++++++++++++++++++++
> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +
> .../wireless/broadcom/brcm80211/brcmfmac/core.h | 2 +
> .../broadcom/brcm80211/brcmfmac/fwil_types.h | 31 ++++
> .../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 19 +++
> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 19 +++
> .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 18 +++
> 8 files changed, 263 insertions(+)

Any comments or feedback about this? I'm hoping to have it in v4.15.

Regards,
Chung-Hsien

2017-11-10 06:13:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [v6] brcmfmac: add CLM download support

Wright Feng <[email protected]> wrote:

> From: Chung-Hsien Hsu <[email protected]>
>
> The firmware for brcmfmac devices includes information regarding
> regulatory constraints. For certain devices this information is kept
> separately in a binary form that needs to be downloaded to the device.
> This patch adds support to download this so-called CLM blob file. It
> uses the same naming scheme as the other firmware files with extension
> of .clm_blob.
>
> The CLM blob file is optional. If the file does not exist, the download
> process will be bypassed. It will not affect the driver loading.
>
> Signed-off-by: Chung-Hsien Hsu <[email protected]>
> Reviewed-by: Arend van Spriel <[email protected]>

Didn't apply:

error: Failed to merge in the changes.
Applying: brcmfmac: add CLM download support
Using index info to reconstruct a base tree...
M drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
M drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
M drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
M drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
M drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
M drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
M drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
CONFLICT (content): Merge conflict in drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
Patch failed at 0001 brcmfmac: add CLM download support
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

--
https://patchwork.kernel.org/patch/9986493/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2017-11-10 02:51:41

by Chung-Hsien Hsu

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

On Fri, Nov 03, 2017 at 03:40:45PM +0200, Kalle Valo wrote:
> Chung-Hsien Hsu <[email protected]> writes:
>
> > On Fri, Nov 03, 2017 at 10:20:07AM +0100, Arend van Spriel wrote:
> >> On 03-11-17 09:27, Chung-Hsien Hsu wrote:
> >> >On Thu, Oct 05, 2017 at 03:31:18PM +0800, Wright Feng wrote:
> >> >>From: Chung-Hsien Hsu <[email protected]>
> >> >>
> >> >>The firmware for brcmfmac devices includes information regarding
> >> >>regulatory constraints. For certain devices this information is kept
> >> >>separately in a binary form that needs to be downloaded to the device.
> >> >>This patch adds support to download this so-called CLM blob file. It
> >> >>uses the same naming scheme as the other firmware files with extension
> >> >>of .clm_blob.
> >> >>
> >> >>The CLM blob file is optional. If the file does not exist, the download
> >> >>process will be bypassed. It will not affect the driver loading.
> >> >>
> >> >>Signed-off-by: Chung-Hsien Hsu <[email protected]>
> >> >>---
> >> >>v2: Revise commit message to describe in more detail
> >> >>v3: Add error handling in brcmf_c_get_clm_name function
> >> >>v4: Correct the length of dload_buf in brcmf_c_download function
> >> >>v5: Remove unnecessary cast and alignment
> >> >>v6: Add debug log for the case of no CLM file present
> >> >>---
> >> >> .../net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 10 ++
> >> >> .../wireless/broadcom/brcm80211/brcmfmac/common.c | 162 +++++++++++++++++++++
> >> >> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +
> >> >> .../wireless/broadcom/brcm80211/brcmfmac/core.h | 2 +
> >> >> .../broadcom/brcm80211/brcmfmac/fwil_types.h | 31 ++++
> >> >> .../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 19 +++
> >> >> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 19 +++
> >> >> .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 18 +++
> >> >> 8 files changed, 263 insertions(+)
> >> >
> >> >Any comments or feedback about this? I'm hoping to have it in v4.15.
>
> This might not necessary make it to v4.15, it depends if Linus releases
> the final v4.14 on Sunday or not.

Since the final v4.14 was not released last Sunday, is it possible to get this to v4.15?

>
> >> Sorry for not following up. The change log for v6 made me wonder if
> >> all my comments on v5 were addressed. I just checked and it looks
> >> fine to me. Kalle has set this patch to Deferred state in patchwork
> >> maybe awaiting my response.
>
> Yup, I was waiting your comments.
>
> >> I already gave my "Reviewed-by:" on v5 so you may add that for v6,
> >> but I am sure Kalle can do that.
> >>
> >> Regards,
> >> Arend
> >
> > Hi Arend,
> >
> > Thanks for the confirmation and quick reply. How can I know the state of
> > a patch, e.g., a patch is set to Deferred state?
>
> I have documented this in the wiki:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#checking_state_of_patches_from_patchwork
>
> > Could you please help to add Arend's "Reviewed-by:" on v6? Or should I
> > add it and resend v6?
>
> Sure, I can add it. No need to resend.
>
> And actually Arend should be able to add the tag automatically by
> replying to the patch with his tag, patchwork would then pick it up and
> add it when I apply the patch. To my knowledge patchwork supports
> Acked-by, Reviewed-by and Tested-by tags.

Thanks for the info.

Regards,
Chung-Hsien

>
> --
> Kalle Valo

2017-11-10 06:16:49

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

Chung-Hsien Hsu <[email protected]> writes:

> On Fri, Nov 03, 2017 at 03:40:45PM +0200, Kalle Valo wrote:
>> Chung-Hsien Hsu <[email protected]> writes:
>>
>> > On Fri, Nov 03, 2017 at 10:20:07AM +0100, Arend van Spriel wrote:
>> >> On 03-11-17 09:27, Chung-Hsien Hsu wrote:
>> >> >On Thu, Oct 05, 2017 at 03:31:18PM +0800, Wright Feng wrote:
>> >> >>From: Chung-Hsien Hsu <[email protected]>
>> >> >>
>> >> >>The firmware for brcmfmac devices includes information regarding
>> >> >>regulatory constraints. For certain devices this information is kept
>> >> >>separately in a binary form that needs to be downloaded to the device.
>> >> >>This patch adds support to download this so-called CLM blob file. It
>> >> >>uses the same naming scheme as the other firmware files with extension
>> >> >>of .clm_blob.
>> >> >>
>> >> >>The CLM blob file is optional. If the file does not exist, the download
>> >> >>process will be bypassed. It will not affect the driver loading.
>> >> >>
>> >> >>Signed-off-by: Chung-Hsien Hsu <[email protected]>
>> >> >>---
>> >> >>v2: Revise commit message to describe in more detail
>> >> >>v3: Add error handling in brcmf_c_get_clm_name function
>> >> >>v4: Correct the length of dload_buf in brcmf_c_download function
>> >> >>v5: Remove unnecessary cast and alignment
>> >> >>v6: Add debug log for the case of no CLM file present
>> >> >>---
>> >> >> .../net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 10 ++
>> >> >> .../wireless/broadcom/brcm80211/brcmfmac/common.c | 162 +++++++++++++++++++++
>> >> >> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +
>> >> >> .../wireless/broadcom/brcm80211/brcmfmac/core.h | 2 +
>> >> >> .../broadcom/brcm80211/brcmfmac/fwil_types.h | 31 ++++
>> >> >> .../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 19 +++
>> >> >> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 19 +++
>> >> >> .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 18 +++
>> >> >> 8 files changed, 263 insertions(+)
>> >> >
>> >> >Any comments or feedback about this? I'm hoping to have it in v4.15.
>>
>> This might not necessary make it to v4.15, it depends if Linus releases
>> the final v4.14 on Sunday or not.
>
> Since the final v4.14 was not released last Sunday, is it possible to get this to v4.15?

Thanks for reminding, apparently I had forgotten to change the state of
the patch from Deferred to New. But unfortunately the patch failed to
apply so you need to rebase and submit v7.

--
Kalle Valo

2017-11-14 06:47:58

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

Chung-Hsien Hsu <[email protected]> writes:

> On Fri, Nov 10, 2017 at 08:15:19AM +0200, Kalle Valo wrote:
>> Wright Feng <[email protected]> writes:
>>
>> > From: Chung-Hsien Hsu <[email protected]>
>> >
>> > The firmware for brcmfmac devices includes information regarding
>> > regulatory constraints. For certain devices this information is kept
>> > separately in a binary form that needs to be downloaded to the device.
>> > This patch adds support to download this so-called CLM blob file. It
>> > uses the same naming scheme as the other firmware files with extension
>> > of .clm_blob.
>> >
>> > The CLM blob file is optional. If the file does not exist, the download
>> > process will be bypassed. It will not affect the driver loading.
>> >
>> > Signed-off-by: Chung-Hsien Hsu <[email protected]>
>>
>> [...]
>>
>> > + err = brcmf_fil_iovar_data_get(ifp, "clmver", buf, sizeof(buf));
>> > + if (err) {
>> > + if (err == -23) {
>>
>> No magic numbers, please. Is this supposed to be -ENFILE?
>
> It indicates "Unsupported". I will remove it since it will not affect
> the CLM downlaod and driver loading.

BTW you would not have to remove it, adding a define would have been
just fine. (I was travelling, hence the late reply.)

--
Kalle Valo

2017-11-03 13:40:50

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

Chung-Hsien Hsu <[email protected]> writes:

> On Fri, Nov 03, 2017 at 10:20:07AM +0100, Arend van Spriel wrote:
>> On 03-11-17 09:27, Chung-Hsien Hsu wrote:
>> >On Thu, Oct 05, 2017 at 03:31:18PM +0800, Wright Feng wrote:
>> >>From: Chung-Hsien Hsu <[email protected]>
>> >>
>> >>The firmware for brcmfmac devices includes information regarding
>> >>regulatory constraints. For certain devices this information is kept
>> >>separately in a binary form that needs to be downloaded to the device.
>> >>This patch adds support to download this so-called CLM blob file. It
>> >>uses the same naming scheme as the other firmware files with extension
>> >>of .clm_blob.
>> >>
>> >>The CLM blob file is optional. If the file does not exist, the download
>> >>process will be bypassed. It will not affect the driver loading.
>> >>
>> >>Signed-off-by: Chung-Hsien Hsu <[email protected]>
>> >>---
>> >>v2: Revise commit message to describe in more detail
>> >>v3: Add error handling in brcmf_c_get_clm_name function
>> >>v4: Correct the length of dload_buf in brcmf_c_download function
>> >>v5: Remove unnecessary cast and alignment
>> >>v6: Add debug log for the case of no CLM file present
>> >>---
>> >> .../net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 10 ++
>> >> .../wireless/broadcom/brcm80211/brcmfmac/common.c | 162 +++++++++++++++++++++
>> >> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +
>> >> .../wireless/broadcom/brcm80211/brcmfmac/core.h | 2 +
>> >> .../broadcom/brcm80211/brcmfmac/fwil_types.h | 31 ++++
>> >> .../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 19 +++
>> >> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 19 +++
>> >> .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 18 +++
>> >> 8 files changed, 263 insertions(+)
>> >
>> >Any comments or feedback about this? I'm hoping to have it in v4.15.

This might not necessary make it to v4.15, it depends if Linus releases
the final v4.14 on Sunday or not.

>> Sorry for not following up. The change log for v6 made me wonder if
>> all my comments on v5 were addressed. I just checked and it looks
>> fine to me. Kalle has set this patch to Deferred state in patchwork
>> maybe awaiting my response.

Yup, I was waiting your comments.

>> I already gave my "Reviewed-by:" on v5 so you may add that for v6,
>> but I am sure Kalle can do that.
>>
>> Regards,
>> Arend
>
> Hi Arend,
>
> Thanks for the confirmation and quick reply. How can I know the state of
> a patch, e.g., a patch is set to Deferred state?

I have documented this in the wiki:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#checking_state_of_patches_from_patchwork

> Could you please help to add Arend's "Reviewed-by:" on v6? Or should I
> add it and resend v6?

Sure, I can add it. No need to resend.

And actually Arend should be able to add the tag automatically by
replying to the patch with his tag, patchwork would then pick it up and
add it when I apply the patch. To my knowledge patchwork supports
Acked-by, Reviewed-by and Tested-by tags.

--
Kalle Valo

2017-11-03 09:20:12

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

On 03-11-17 09:27, Chung-Hsien Hsu wrote:
> On Thu, Oct 05, 2017 at 03:31:18PM +0800, Wright Feng wrote:
>> From: Chung-Hsien Hsu <[email protected]>
>>
>> The firmware for brcmfmac devices includes information regarding
>> regulatory constraints. For certain devices this information is kept
>> separately in a binary form that needs to be downloaded to the device.
>> This patch adds support to download this so-called CLM blob file. It
>> uses the same naming scheme as the other firmware files with extension
>> of .clm_blob.
>>
>> The CLM blob file is optional. If the file does not exist, the download
>> process will be bypassed. It will not affect the driver loading.
>>
>> Signed-off-by: Chung-Hsien Hsu <[email protected]>
>> ---
>> v2: Revise commit message to describe in more detail
>> v3: Add error handling in brcmf_c_get_clm_name function
>> v4: Correct the length of dload_buf in brcmf_c_download function
>> v5: Remove unnecessary cast and alignment
>> v6: Add debug log for the case of no CLM file present
>> ---
>> .../net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 10 ++
>> .../wireless/broadcom/brcm80211/brcmfmac/common.c | 162 +++++++++++++++++++++
>> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +
>> .../wireless/broadcom/brcm80211/brcmfmac/core.h | 2 +
>> .../broadcom/brcm80211/brcmfmac/fwil_types.h | 31 ++++
>> .../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 19 +++
>> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 19 +++
>> .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 18 +++
>> 8 files changed, 263 insertions(+)
>
> Any comments or feedback about this? I'm hoping to have it in v4.15.

Hi Chung-Hsien,

Sorry for not following up. The change log for v6 made me wonder if all
my comments on v5 were addressed. I just checked and it looks fine to
me. Kalle has set this patch to Deferred state in patchwork maybe
awaiting my response. I already gave my "Reviewed-by:" on v5 so you may
add that for v6, but I am sure Kalle can do that.

Regards,
Arend

2017-11-10 06:15:24

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

Wright Feng <[email protected]> writes:

> From: Chung-Hsien Hsu <[email protected]>
>
> The firmware for brcmfmac devices includes information regarding
> regulatory constraints. For certain devices this information is kept
> separately in a binary form that needs to be downloaded to the device.
> This patch adds support to download this so-called CLM blob file. It
> uses the same naming scheme as the other firmware files with extension
> of .clm_blob.
>
> The CLM blob file is optional. If the file does not exist, the download
> process will be bypassed. It will not affect the driver loading.
>
> Signed-off-by: Chung-Hsien Hsu <[email protected]>

[...]

> + err = brcmf_fil_iovar_data_get(ifp, "clmver", buf, sizeof(buf));
> + if (err) {
> + if (err == -23) {

No magic numbers, please. Is this supposed to be -ENFILE?

--
Kalle Valo

2017-11-06 08:47:34

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

On 10/5/2017 9:31 AM, Wright Feng wrote:
> From: Chung-Hsien Hsu <[email protected]>
>
> The firmware for brcmfmac devices includes information regarding
> regulatory constraints. For certain devices this information is kept
> separately in a binary form that needs to be downloaded to the device.
> This patch adds support to download this so-called CLM blob file. It
> uses the same naming scheme as the other firmware files with extension
> of .clm_blob.
>
> The CLM blob file is optional. If the file does not exist, the download
> process will be bypassed. It will not affect the driver loading.

Reviewed-by: Arend van Spriel <[email protected]>
> Signed-off-by: Chung-Hsien Hsu <[email protected]>
> ---
> v2: Revise commit message to describe in more detail
> v3: Add error handling in brcmf_c_get_clm_name function
> v4: Correct the length of dload_buf in brcmf_c_download function
> v5: Remove unnecessary cast and alignment
> v6: Add debug log for the case of no CLM file present
> ---

2017-11-10 09:00:24

by Chung-Hsien Hsu

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

On Fri, Nov 10, 2017 at 08:15:19AM +0200, Kalle Valo wrote:
> Wright Feng <[email protected]> writes:
>
> > From: Chung-Hsien Hsu <[email protected]>
> >
> > The firmware for brcmfmac devices includes information regarding
> > regulatory constraints. For certain devices this information is kept
> > separately in a binary form that needs to be downloaded to the device.
> > This patch adds support to download this so-called CLM blob file. It
> > uses the same naming scheme as the other firmware files with extension
> > of .clm_blob.
> >
> > The CLM blob file is optional. If the file does not exist, the download
> > process will be bypassed. It will not affect the driver loading.
> >
> > Signed-off-by: Chung-Hsien Hsu <[email protected]>
>
> [...]
>
> > + err = brcmf_fil_iovar_data_get(ifp, "clmver", buf, sizeof(buf));
> > + if (err) {
> > + if (err == -23) {
>
> No magic numbers, please. Is this supposed to be -ENFILE?

It indicates "Unsupported". I will remove it since it will not affect
the CLM downlaod and driver loading.

Regards,
Chung-Hsien

>
> --
> Kalle Valo

2017-11-03 09:40:29

by Chung-Hsien Hsu

[permalink] [raw]
Subject: Re: [PATCH v6] brcmfmac: add CLM download support

On Fri, Nov 03, 2017 at 10:20:07AM +0100, Arend van Spriel wrote:
> On 03-11-17 09:27, Chung-Hsien Hsu wrote:
> >On Thu, Oct 05, 2017 at 03:31:18PM +0800, Wright Feng wrote:
> >>From: Chung-Hsien Hsu <[email protected]>
> >>
> >>The firmware for brcmfmac devices includes information regarding
> >>regulatory constraints. For certain devices this information is kept
> >>separately in a binary form that needs to be downloaded to the device.
> >>This patch adds support to download this so-called CLM blob file. It
> >>uses the same naming scheme as the other firmware files with extension
> >>of .clm_blob.
> >>
> >>The CLM blob file is optional. If the file does not exist, the download
> >>process will be bypassed. It will not affect the driver loading.
> >>
> >>Signed-off-by: Chung-Hsien Hsu <[email protected]>
> >>---
> >>v2: Revise commit message to describe in more detail
> >>v3: Add error handling in brcmf_c_get_clm_name function
> >>v4: Correct the length of dload_buf in brcmf_c_download function
> >>v5: Remove unnecessary cast and alignment
> >>v6: Add debug log for the case of no CLM file present
> >>---
> >> .../net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 10 ++
> >> .../wireless/broadcom/brcm80211/brcmfmac/common.c | 162 +++++++++++++++++++++
> >> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +
> >> .../wireless/broadcom/brcm80211/brcmfmac/core.h | 2 +
> >> .../broadcom/brcm80211/brcmfmac/fwil_types.h | 31 ++++
> >> .../wireless/broadcom/brcm80211/brcmfmac/pcie.c | 19 +++
> >> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 19 +++
> >> .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 18 +++
> >> 8 files changed, 263 insertions(+)
> >
> >Any comments or feedback about this? I'm hoping to have it in v4.15.
>
> Hi Chung-Hsien,
>
> Sorry for not following up. The change log for v6 made me wonder if
> all my comments on v5 were addressed. I just checked and it looks
> fine to me. Kalle has set this patch to Deferred state in patchwork
> maybe awaiting my response. I already gave my "Reviewed-by:" on v5
> so you may add that for v6, but I am sure Kalle can do that.
>
> Regards,
> Arend

Hi Arend,

Thanks for the confirmation and quick reply. How can I know the state of
a patch, e.g., a patch is set to Deferred state?


Hi Kalle,

Could you please help to add Arend's "Reviewed-by:" on v6? Or should I
add it and resend v6?

Regards,
Chung-Hsien