1. Patchew
  2. linux
  3. [PATCH v3 0/2] Resolve the failure in downloading
  4. View patch

[PATCH v3 1/2] wifi: mwifiex: Part A of resolving the failure in downloading calibration data.

Jeff Chen posted 2 patches 10 months ago
[PATCH v3 1/2] wifi: mwifiex: Part A of resolving the failure in downloading calibration data.
Posted by Jeff Chen 10 months ago 
This patch corrects the command format used for downloading RF
calibration data to the firmware.

This patch is a split from the previous submission.

Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>
---
 drivers/net/wireless/marvell/mwifiex/fw.h      |  7 +++++++
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 14 +++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 4a96281792cc..0c75a574a7ee 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -2352,6 +2352,12 @@ struct host_cmd_ds_add_station {
 	u8 tlv[];
 } __packed;
 
+struct host_cmd_ds_802_11_cfg_data {
+	__le16 action;
+	__le16 type;
+	__le16 data_len;
+} __packed;
+
 struct host_cmd_ds_command {
 	__le16 command;
 	__le16 size;
@@ -2431,6 +2437,7 @@ struct host_cmd_ds_command {
 		struct host_cmd_ds_pkt_aggr_ctrl pkt_aggr_ctrl;
 		struct host_cmd_ds_sta_configure sta_cfg;
 		struct host_cmd_ds_add_station sta_info;
+		struct host_cmd_ds_802_11_cfg_data cfg_data;
 	} params;
 } __packed;
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index e2800a831c8e..6e7b2b5c7dc5 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -1500,18 +1500,19 @@ int mwifiex_dnld_dt_cfgdata(struct mwifiex_private *priv,
 
 /* This function prepares command of set_cfg_data. */
 static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
-				struct host_cmd_ds_command *cmd, void *data_buf)
+				struct host_cmd_ds_command *cmd, void *data_buf, u16 cmd_action)
 {
 	struct mwifiex_adapter *adapter = priv->adapter;
 	struct property *prop = data_buf;
 	u32 len;
 	u8 *data = (u8 *)cmd + S_DS_GEN;
 	int ret;
+	struct host_cmd_ds_802_11_cfg_data *pcfg_data = &cmd->params.cfg_data;
 
 	if (prop) {
 		len = prop->length;
 		ret = of_property_read_u8_array(adapter->dt_node, prop->name,
-						data, len);
+						data + sizeof(*pcfg_data), len);
 		if (ret)
 			return ret;
 		mwifiex_dbg(adapter, INFO,
@@ -1519,15 +1520,18 @@ static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
 			    prop->name);
 	} else if (adapter->cal_data->data && adapter->cal_data->size > 0) {
 		len = mwifiex_parse_cal_cfg((u8 *)adapter->cal_data->data,
-					    adapter->cal_data->size, data);
+					    adapter->cal_data->size, data + sizeof(*pcfg_data));
 		mwifiex_dbg(adapter, INFO,
 			    "download cfg_data from config file\n");
 	} else {
 		return -1;
 	}
 
+	pcfg_data->action = cpu_to_le16(cmd_action);
+	pcfg_data->type = cpu_to_le16(2);
+	pcfg_data->data_len = cpu_to_le16(len);
 	cmd->command = cpu_to_le16(HostCmd_CMD_CFG_DATA);
-	cmd->size = cpu_to_le16(S_DS_GEN + len);
+	cmd->size = cpu_to_le16(S_DS_GEN + sizeof(*pcfg_data) + len);
 
 	return 0;
 }
@@ -1949,7 +1953,7 @@ int mwifiex_sta_prepare_cmd(struct mwifiex_private *priv, uint16_t cmd_no,
 		ret = mwifiex_cmd_get_hw_spec(priv, cmd_ptr);
 		break;
 	case HostCmd_CMD_CFG_DATA:
-		ret = mwifiex_cmd_cfg_data(priv, cmd_ptr, data_buf);
+		ret = mwifiex_cmd_cfg_data(priv, cmd_ptr, data_buf, cmd_action);
 		break;
 	case HostCmd_CMD_MAC_CONTROL:
 		ret = mwifiex_cmd_mac_control(priv, cmd_ptr, cmd_action,
-- 
2.34.1
[PATCH v4 0/2] wifi: mwifiex: Fix RF calibration data handling issues
Posted by Jeff Chen 9 months ago 
This patch series addresses two related issues with downloading RF
calibration data in the mwifiex driver. The first patch resolves the
problem of calibration data being prematurely released before the
download process. The second patch focuses on restoring the functionality
to download RF calibration data from a file, which was broken by a previous
commit.

These fixes ensure proper handling of calibration data while avoiding any
impact on downloading configuration data from the device tree.

Typically, calibration data comes from OTP/EEPROM. This patch merely
enables the functionality to download CAL data to the firmware from a file
and does not require backporting to stable versions.
---

**Changelog**:
v4:
- Patch 1: Clarified the title and included the Fixes tag.
- Patch 2: Refactored the patch to ensure that changes for restoring
file-based RF calibration data downloads do not impact the functionality
for downloading configuration data from the device tree. Added detailed
explanation about the potential impact on device tree-based calibration
data downloads.

v3:
- Split the patch into two separate parts for better clarity.
- Improved commit messages to provide more context about the issues being
addressed.

v2:
- Expanded the commit messages with detailed descriptions of the problems
and their impact.
- Highlighted the premature release of calibration data before the download
process.

v1:
- Initial submission. Focused on correcting the command format for
downloading calibration data.

Jeff Chen (2):
  wifi: mwifiex: Fix premature release of RF calibration data.
  wifi: mwifiex: Fix RF calibration data download from file

 drivers/net/wireless/marvell/mwifiex/fw.h      | 14 ++++++++++++++
 drivers/net/wireless/marvell/mwifiex/main.c    |  4 ----
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 17 ++++++++++++++---
 3 files changed, 28 insertions(+), 7 deletions(-)

-- 
2.34.1
Re: [PATCH v3 1/2] wifi: mwifiex: Part A of resolving the failure in downloading calibration data.
Posted by Francesco Dolcini 9 months, 2 weeks ago 
Hello Jeff,

On Thu, Feb 20, 2025 at 02:11:42PM +0800, Jeff Chen wrote:
> This patch corrects the command format used for downloading RF
> calibration data to the firmware.

Do we need any fixes tag? is this supposed to be backported to stable?

Was the command format always broken? Do this format depends on the
firmware version? We would need to explain why changing the format of
this command here is safe.

> 
> This patch is a split from the previous submission.
> 
> Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/fw.h      |  7 +++++++
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 14 +++++++++-----
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> index 4a96281792cc..0c75a574a7ee 100644
> --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> @@ -2352,6 +2352,12 @@ struct host_cmd_ds_add_station {
>  	u8 tlv[];
>  } __packed;
>  
> +struct host_cmd_ds_802_11_cfg_data {
> +	__le16 action;
> +	__le16 type;
> +	__le16 data_len;
> +} __packed;
> +
>  struct host_cmd_ds_command {
>  	__le16 command;
>  	__le16 size;
> @@ -2431,6 +2437,7 @@ struct host_cmd_ds_command {
>  		struct host_cmd_ds_pkt_aggr_ctrl pkt_aggr_ctrl;
>  		struct host_cmd_ds_sta_configure sta_cfg;
>  		struct host_cmd_ds_add_station sta_info;
> +		struct host_cmd_ds_802_11_cfg_data cfg_data;
>  	} params;
>  } __packed;
>  
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index e2800a831c8e..6e7b2b5c7dc5 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -1500,18 +1500,19 @@ int mwifiex_dnld_dt_cfgdata(struct mwifiex_private *priv,
>  
>  /* This function prepares command of set_cfg_data. */
>  static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
> -				struct host_cmd_ds_command *cmd, void *data_buf)
> +				struct host_cmd_ds_command *cmd, void *data_buf, u16 cmd_action)
>  {
>  	struct mwifiex_adapter *adapter = priv->adapter;
>  	struct property *prop = data_buf;
>  	u32 len;
>  	u8 *data = (u8 *)cmd + S_DS_GEN;
>  	int ret;
> +	struct host_cmd_ds_802_11_cfg_data *pcfg_data = &cmd->params.cfg_data;
>  
>  	if (prop) {
>  		len = prop->length;
>  		ret = of_property_read_u8_array(adapter->dt_node, prop->name,
> -						data, len);
> +						data + sizeof(*pcfg_data), len);
>  		if (ret)
>  			return ret;
>  		mwifiex_dbg(adapter, INFO,
> @@ -1519,15 +1520,18 @@ static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
>  			    prop->name);
>  	} else if (adapter->cal_data->data && adapter->cal_data->size > 0) {
>  		len = mwifiex_parse_cal_cfg((u8 *)adapter->cal_data->data,
> -					    adapter->cal_data->size, data);
> +					    adapter->cal_data->size, data + sizeof(*pcfg_data));
>  		mwifiex_dbg(adapter, INFO,
>  			    "download cfg_data from config file\n");
>  	} else {
>  		return -1;
>  	}
>  
> +	pcfg_data->action = cpu_to_le16(cmd_action);
> +	pcfg_data->type = cpu_to_le16(2);
> +	pcfg_data->data_len = cpu_to_le16(len);
>  	cmd->command = cpu_to_le16(HostCmd_CMD_CFG_DATA);
> -	cmd->size = cpu_to_le16(S_DS_GEN + len);
> +	cmd->size = cpu_to_le16(S_DS_GEN + sizeof(*pcfg_data) + len);
>  
>  	return 0;
>  }
> @@ -1949,7 +1953,7 @@ int mwifiex_sta_prepare_cmd(struct mwifiex_private *priv, uint16_t cmd_no,
>  		ret = mwifiex_cmd_get_hw_spec(priv, cmd_ptr);
>  		break;
>  	case HostCmd_CMD_CFG_DATA:
> -		ret = mwifiex_cmd_cfg_data(priv, cmd_ptr, data_buf);
> +		ret = mwifiex_cmd_cfg_data(priv, cmd_ptr, data_buf, cmd_action);
>  		break;
>  	case HostCmd_CMD_MAC_CONTROL:
>  		ret = mwifiex_cmd_mac_control(priv, cmd_ptr, cmd_action,
> -- 
> 2.34.1
>
[PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.
Posted by Jeff Chen 9 months ago 
This patch resolves an issue where RF calibration data was being
released before the download process. Without this fix, the
external calibration data file would not be downloaded
at all.

Fixes: d39fbc88956e ("mwifiex: remove cfg_data construction")
Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>
---
 drivers/net/wireless/marvell/mwifiex/main.c    | 4 ----
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 6 +++++-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 45eecb5f643b..b07cb302a00c 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -691,10 +691,6 @@ static int _mwifiex_fw_dpc(const struct firmware *firmware, void *context)
 
 	init_failed = true;
 done:
-	if (adapter->cal_data) {
-		release_firmware(adapter->cal_data);
-		adapter->cal_data = NULL;
-	}
 	if (adapter->firmware) {
 		release_firmware(adapter->firmware);
 		adapter->firmware = NULL;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index e2800a831c8e..c0e6ce1a82fe 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -2293,9 +2293,13 @@ int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init)
 						"marvell,caldata");
 		}
 
-		if (adapter->cal_data)
+		if (adapter->cal_data) {
 			mwifiex_send_cmd(priv, HostCmd_CMD_CFG_DATA,
 					 HostCmd_ACT_GEN_SET, 0, NULL, true);
+			release_firmware(adapter->cal_data);
+			adapter->cal_data = NULL;
+		}
+
 
 		/* Read MAC address from HW */
 		ret = mwifiex_send_cmd(priv, HostCmd_CMD_GET_HW_SPEC,
-- 
2.34.1
Re: [PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.
Posted by Francesco Dolcini 9 months ago 
Hello Jeff,

On Tue, Mar 18, 2025 at 01:07:38PM +0800, Jeff Chen wrote:
> This patch resolves an issue where RF calibration data was being
> released before the download process. Without this fix, the
> external calibration data file would not be downloaded
> at all.
> 
> Fixes: d39fbc88956e ("mwifiex: remove cfg_data construction")
> Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>

The code looks ok to me, however I do not understand the commit you
selected as fixes tag.

From what I understand releasing the data before using it was done since
the initial commit 388ec385d5ce ("mwifiex: add calibration data download
feature"). What am I missing?

Francesco
[PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data download from file
Posted by Jeff Chen 9 months ago 
This patch resolves an issue where RF calibration data from a
file could not be downloaded to the firmware. The feature to
download calibration data from a file was broken by the commit:
d39fbc88956e.

The issue arose because the function `mwifiex_cmd_cfg_data()`
was modified in a way that prevented proper handling of
file-based calibration data. While this patch restores the ability
to download RF calibration data from a file, it may inadvertently
break the feature to download calibration data from the device
tree. This is because the function `mwifiex_dnld_dt_cfgdata()`,
which also relies on `mwifiex_cmd_cfg_data()`, is still used for
device tree-based calibration data downloads.

Fixes: d39fbc88956e ("mwifiex: remove cfg_data construction")
Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>
---
 drivers/net/wireless/marvell/mwifiex/fw.h      | 14 ++++++++++++++
 drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 4a96281792cc..91458f3bd14a 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -454,6 +454,11 @@ enum mwifiex_channel_flags {
 #define HostCmd_RET_BIT                       0x8000
 #define HostCmd_ACT_GEN_GET                   0x0000
 #define HostCmd_ACT_GEN_SET                   0x0001
+#define HOST_CMD_ACT_GEN_SET                  0x0001
+/* Add this non-CamelCase-style macro to comply with checkpatch requirements.
+ *  This macro will eventually replace all existing CamelCase-style macros in
+ *  the future for consistency.
+ */
 #define HostCmd_ACT_GEN_REMOVE                0x0004
 #define HostCmd_ACT_BITWISE_SET               0x0002
 #define HostCmd_ACT_BITWISE_CLR               0x0003
@@ -2352,6 +2357,14 @@ struct host_cmd_ds_add_station {
 	u8 tlv[];
 } __packed;
 
+#define MWIFIEX_CFG_TYPE_CAL 0x2
+
+struct host_cmd_ds_802_11_cfg_data {
+	__le16 action;
+	__le16 type;
+	__le16 data_len;
+} __packed;
+
 struct host_cmd_ds_command {
 	__le16 command;
 	__le16 size;
@@ -2431,6 +2444,7 @@ struct host_cmd_ds_command {
 		struct host_cmd_ds_pkt_aggr_ctrl pkt_aggr_ctrl;
 		struct host_cmd_ds_sta_configure sta_cfg;
 		struct host_cmd_ds_add_station sta_info;
+		struct host_cmd_ds_802_11_cfg_data cfg_data;
 	} params;
 } __packed;
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index c0e6ce1a82fe..52678e213050 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -1507,6 +1507,7 @@ static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
 	u32 len;
 	u8 *data = (u8 *)cmd + S_DS_GEN;
 	int ret;
+	struct host_cmd_ds_802_11_cfg_data *pcfg_data;
 
 	if (prop) {
 		len = prop->length;
@@ -1514,12 +1515,19 @@ static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
 						data, len);
 		if (ret)
 			return ret;
+
+		cmd->size = cpu_to_le16(S_DS_GEN + len);
 		mwifiex_dbg(adapter, INFO,
 			    "download cfg_data from device tree: %s\n",
 			    prop->name);
 	} else if (adapter->cal_data->data && adapter->cal_data->size > 0) {
 		len = mwifiex_parse_cal_cfg((u8 *)adapter->cal_data->data,
-					    adapter->cal_data->size, data);
+					    adapter->cal_data->size, data + sizeof(*pcfg_data));
+		pcfg_data = &cmd->params.cfg_data;
+		pcfg_data->action = cpu_to_le16(HOST_CMD_ACT_GEN_SET);
+		pcfg_data->type = cpu_to_le16(MWIFIEX_CFG_TYPE_CAL);
+		pcfg_data->data_len = cpu_to_le16(len);
+		cmd->size = cpu_to_le16(S_DS_GEN + sizeof(*pcfg_data) + len);
 		mwifiex_dbg(adapter, INFO,
 			    "download cfg_data from config file\n");
 	} else {
@@ -1527,7 +1535,6 @@ static int mwifiex_cmd_cfg_data(struct mwifiex_private *priv,
 	}
 
 	cmd->command = cpu_to_le16(HostCmd_CMD_CFG_DATA);
-	cmd->size = cpu_to_le16(S_DS_GEN + len);
 
 	return 0;
 }
-- 
2.34.1
Re: [PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data download from file
Posted by Sascha Hauer 8 months, 4 weeks ago 
On Tue, Mar 18, 2025 at 01:07:39PM +0800, Jeff Chen wrote:
> This patch resolves an issue where RF calibration data from a
> file could not be downloaded to the firmware. The feature to
> download calibration data from a file was broken by the commit:
> d39fbc88956e.
> 
> The issue arose because the function `mwifiex_cmd_cfg_data()`
> was modified in a way that prevented proper handling of
> file-based calibration data. While this patch restores the ability
> to download RF calibration data from a file, it may inadvertently
> break the feature to download calibration data from the device
> tree. This is because the function `mwifiex_dnld_dt_cfgdata()`,
> which also relies on `mwifiex_cmd_cfg_data()`, is still used for
> device tree-based calibration data downloads.
> 
> Fixes: d39fbc88956e ("mwifiex: remove cfg_data construction")
> Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/fw.h      | 14 ++++++++++++++
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++++++++--
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> index 4a96281792cc..91458f3bd14a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> @@ -454,6 +454,11 @@ enum mwifiex_channel_flags {
>  #define HostCmd_RET_BIT                       0x8000
>  #define HostCmd_ACT_GEN_GET                   0x0000
>  #define HostCmd_ACT_GEN_SET                   0x0001
> +#define HOST_CMD_ACT_GEN_SET                  0x0001
> +/* Add this non-CamelCase-style macro to comply with checkpatch requirements.
> + *  This macro will eventually replace all existing CamelCase-style macros in
> + *  the future for consistency.
> + */

Just ignore this checkpatch warning. We don't want to have duplicated
defines just for silencing checkpatch. If anything we could change all
the CamelCase defines throughout the driver in one go.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Re: [PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data download from file
Posted by Francesco Dolcini 9 months ago 
Hello Jeff,

On Tue, Mar 18, 2025 at 01:07:39PM +0800, Jeff Chen wrote:
> While this patch restores the ability to download RF calibration data
> from a file, it may inadvertently break the feature to download
> calibration data from the device tree.

I do not think this is acceptable. Fixing something by adding another
bug is not ok. You should fix the calibration data from file, without
breaking the device tree calibration functionality.

Francesco

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.