[PATCH v2 1/2] wifi: mwifiex: Resolve the failure in downloading calibration data.

[PATCH v2 1/2] wifi: mwifiex: Resolve the failure in downloading calibration data.

[Jeff Chen]

This patch corrects the command format used for downloading RF
calibration data to the firmware. Additionally, this patch resolves
an issue where calibration data was being released before the
download process. Without this fix, the calibration data would not
be downloaded at all.

Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>
---
 drivers/net/wireless/marvell/mwifiex/fw.h     |  7 +++++++
 drivers/net/wireless/marvell/mwifiex/main.c   |  4 ----
 .../net/wireless/marvell/mwifiex/sta_cmd.c    | 19 +++++++++++++------
 3 files changed, 20 insertions(+), 10 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/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..027555211a99 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,
@@ -2293,9 +2297,12 @@ 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,

base-commit: 22f3551b60be7d126db9233998d262edfc577d0b
-- 
2.34.1

[PATCH v2 2/2] wifi: mwifiex: Fix HT40 bandwidth issue.

[Jeff Chen]

This patch addresses an issue where, despite the AP supporting 40MHz
bandwidth, the connection was limited to 20MHz. Without this fix,
even if the access point supports 40MHz, the bandwidth after
connection remains at 20MHz. This issue is not a regression.

Signed-off-by: Jeff Chen <jeff.chen_1@nxp.com>
---
 drivers/net/wireless/marvell/mwifiex/11n.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/11n.c b/drivers/net/wireless/marvell/mwifiex/11n.c
index 66f0f5377ac1..4ae0b4aaa09a 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n.c
@@ -308,7 +308,7 @@ mwifiex_cmd_append_11n_tlv(struct mwifiex_private *priv,
 	int ret_len = 0;
 	struct ieee80211_supported_band *sband;
 	struct ieee_types_header *hdr;
-	u8 radio_type;
+	u8 radio_type, secch_offset;
 
 	if (!buffer || !*buffer)
 		return ret_len;
@@ -401,13 +401,15 @@ mwifiex_cmd_append_11n_tlv(struct mwifiex_private *priv,
 		chan_list->chan_scan_param[0].radio_type =
 			mwifiex_band_to_radio_type((u8) bss_desc->bss_band);
 
-		if (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40 &&
-		    bss_desc->bcn_ht_oper->ht_param &
-		    IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)
-			SET_SECONDARYCHAN(chan_list->chan_scan_param[0].
-					  radio_type,
-					  (bss_desc->bcn_ht_oper->ht_param &
-					  IEEE80211_HT_PARAM_CHA_SEC_OFFSET));
+		if (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) {
+			if (bss_desc->bcn_ht_oper->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY) {
+				chan_list->chan_scan_param[0].radio_type |= (CHAN_BW_40MHZ << 2);
+				secch_offset = bss_desc->bcn_ht_oper->ht_param &
+				IEEE80211_HT_PARAM_CHA_SEC_OFFSET;
+				SET_SECONDARYCHAN(chan_list->chan_scan_param[0].radio_type,
+						  secch_offset);
+			}
+		}
 
 		*buffer += struct_size(chan_list, chan_scan_param, 1);
 		ret_len += struct_size(chan_list, chan_scan_param, 1);
-- 
2.34.1

Re: [PATCH v2 1/2] wifi: mwifiex: Resolve the failure in downloading calibration data.

[Johannes Berg]

On Wed, 2025-02-05 at 09:28 +0800, Jeff Chen wrote:
> This patch corrects the command format used for downloading RF
> calibration data to the firmware. Additionally, this patch resolves
> an issue where calibration data was being released before the
> download process.

Sounds like that should be two separate patches then.

johannes

[PATCH v3 0/2] Resolve the failure in downloading

[Jeff Chen]

Hi Johannes,

Based on your feedback for the previous submission, split the
original patch into two separate patches.

Best regards,

Jeff

Jeff Chen (2):
  wifi: mwifiex: Part A of resolving the failure in downloading
    calibration data.
  wifi: mwifiex: Part B of resolving the failure in downloading
    calibration data.

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


base-commit: c61da149b9c2e439abe27845a71dae5ce5f5985c
-- 
2.34.1

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

[Jeff Chen]

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 v3 2/2] wifi: mwifiex: Part B of resolving the failure in downloading calibration data.

[Jeff Chen]

This patch resolves an issue where calibration data was being
released before the download process. Without this fix, the
calibration data would not be downloaded at all.

This patch is a split from the previous submission.

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 6e7b2b5c7dc5..2ba5269b0f3a 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -2297,9 +2297,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 v3 0/2] Resolve the failure in downloading

[Johannes Berg]

On Thu, 2025-02-20 at 14:11 +0800, Jeff Chen wrote:
> Hi Johannes,
> 
> Based on your feedback for the previous submission, split the
> original patch into two separate patches.
> 

Thanks. I guess it really should have better subject lines now, and it
doesn't need to (or shouldn't really) indicate in the commit messages
that it was split (you can do that after a --- separator if you like.)

johannes

Re: [PATCH v2 2/2] wifi: mwifiex: Fix HT40 bandwidth issue.

[Francesco Dolcini]

Hello Jeff,

On Wed, Feb 05, 2025 at 09:28:43AM +0800, Jeff Chen wrote:
> This patch addresses an issue where, despite the AP supporting 40MHz
> bandwidth, the connection was limited to 20MHz. Without this fix,
> even if the access point supports 40MHz, the bandwidth after
> connection remains at 20MHz. This issue is not a regression.

As you know this patch came after us (Toradex) reported some issue
connecting to 2.4GHz network using IW416.

However according to this commit message this actual fix is not related
to the issue in which it was not possible to connect at all, but it's
just an improvement. Can you confirm this?

Can you please also answer the last comment I had in the previous version of
this patch, see https://lore.kernel.org/all/Z44vj59nWIiswq7s@gaggiata.pivistrello.it/
?

Reported here again for you convenience:

  setting `radio_type |= (CHAN_BW_40MHZ << 2)` seems the only real change on this
  patch, correct? Anything else is cosmetic, correct?
  
  would doing just this change be equivalent, right?
  
  	SET_SECONDARYCHAN(chan_list->chan_scan_param[0].
  			  radio_type | (CHAN_BW_40MHZ << 2),
  			  (bss_desc->bcn_ht_oper->ht_param &
  			  IEEE80211_HT_PARAM_CHA_SEC_OFFSET));


Thanks,
Francesco

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

[Francesco Dolcini]

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
> 

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

[Jeff Chen]

Hi Francesco,

Thank you for reviewing and providing great comments.
> 
> 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.
> 
Yes, we need a Fixes tag. It appears that the feature to download CAL from file was broken by the commit below.
commit d39fbc88956ef56a67b8030d53c7e8fa645a4e00
Author: Bing Zhao <bzhao@marvell.com>
Date:   Fri Dec 13 18:33:00 2013 -0800

    mwifiex: remove cfg_data construction

    The cfg_data buffer will include the cfg_data structure header
    (action, type, data_len). This makes it work for all data types
    without extra parsing.

This patch enables the feature to download RF calibration data from a file. I don't think it needs to be backported to stable. 
It doesn't depend on the firmware version.
However, I think it will break the feature to download CAL data from the device tree. See the code segment below.
'mwifiex_dnld_dt_cfgdata()' also uses 'mwifiex_cmd_cfg_data()' to prepare a command.
It would be better to add a new function to download CAL data from a file, instead of modifying the function `mwifiex_cmd_cfg_data()`

int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init)
{
...
		/* Download calibration data to firmware.
		 * The cal-data can be read from device tree and/or
		 * a configuration file and downloaded to firmware.
		 */
		if (adapter->dt_node) {
			if (of_property_read_u32(adapter->dt_node,
						 "marvell,wakeup-pin",
						 &data) == 0) {
				pr_debug("Wakeup pin = 0x%x\n", data);
				adapter->hs_cfg.gpio = data;
			}

			mwifiex_dnld_dt_cfgdata(priv, adapter->dt_node,
						"marvell,caldata");
		}
...
}
> >
> > 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
> >

RE: [EXT] Re: [PATCH v2 2/2] wifi: mwifiex: Fix HT40 bandwidth issue.

[Jeff Chen]

Hello Francesco,

Thank you for reviewing and providing great comments.

> -----Original Message-----
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Thursday, March 6, 2025 6:38 PM
> To: Jeff Chen <jeff.chen_1@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.org; kvalo@kernel.org; francesco@dolcini.it; Pete
> Hsieh <tsung-hsien.hsieh@nxp.com>; s.hauer@pengutronix.de
> Subject: [EXT] Re: [PATCH v2 2/2] wifi: mwifiex: Fix HT40 bandwidth issue.
>
>
> As you know this patch came after us (Toradex) reported some issue
> connecting to 2.4GHz network using IW416.
>
> However according to this commit message this actual fix is not related to the
> issue in which it was not possible to connect at all, but it's just an
> improvement. Can you confirm this?
>

You are correct that this patch primarily addresses an improvement rather than
resolving an outright failure to connect. It ensures that when the AP supports
40MHz bandwidth, the connection operates at the expected 40MHz instead of
being limited to 20MHz. This fix is not directly related to the reported issue
where the connection to the 2.4GHz network with IW416 was not possible at all.

> Can you please also answer the last comment I had in the previous version of
> this patch, see
> https://lore.ker/
> nel.org%2Fall%2FZ44vj59nWIiswq7s%40gaggiata.pivistrello.it%2F&data=05%7
> C02%7Cjeff.chen_1%40nxp.com%7C608bec5740f046d9409908dd5c9af5e1%7C
> 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638768542804304282%7
> CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDA
> wMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7
> C&sdata=piD%2BBLUEqdCZgomWsxtKFGS7RAfj0nk0tTsQYu%2BEEwU%3D&res
> erved=0
> ?
>
> Reported here again for you convenience:
>
>   setting `radio_type |= (CHAN_BW_40MHZ << 2)` seems the only real change
> on this
>   patch, correct? Anything else is cosmetic, correct?
>
>   would doing just this change be equivalent, right?
>
>         SET_SECONDARYCHAN(chan_list->chan_scan_param[0].
>                           radio_type | (CHAN_BW_40MHZ << 2),
>                           (bss_desc->bcn_ht_oper->ht_param &
>                           IEEE80211_HT_PARAM_CHA_SEC_OFFSET));
>
>

Yes, the primary functional change in the patch is indeed the addition of
`radio_type |= (CHAN_BW_40MHZ << 2)`. The rest of the changes were
made to pass checkpatch and seem unnecessary. I will update to your
version.

[PATCH v4 0/2] wifi: mwifiex: Fix RF calibration data handling issues

[Jeff Chen]

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

[PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.

[Jeff Chen]

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

[PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data download from file

[Jeff Chen]

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 1/2] wifi: mwifiex: Fix premature release of RF calibration data.

[Francesco Dolcini]

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

Re: [PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data download from file

[Francesco Dolcini]

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

Re: [PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data download from file

[Sascha Hauer]

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: [EXT] Re: [PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data download from file

[Jeff Chen]

> 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

Hello Francesco,  


There seems to be a misunderstanding. The issue with breaking the device tree
calibration functionality was actually introduced in version 3 of the patch. Version 4
resolves this problem by ensuring that both file-based and device tree-based
calibration work as intended.  

The current patch restores the ability to download RF calibration data from a file 
without affecting the device tree functionality. I'll clarify this in the commit message
to avoid further confusion.  

Best regards,  
Jeff

RE: [EXT] Re: [PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data download from file

[Jeff Chen]

> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Monday, March 24, 2025 3:02 PM
> To: Jeff Chen <jeff.chen_1@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.org; johannes@sipsolutions.net; francesco@dolcini.it;
> Pete Hsieh <tsung-hsien.hsieh@nxp.com>
> Subject: [EXT] Re: [PATCH v4 2/2] wifi: mwifiex: Fix RF calibration data
> download from file
> 
> > @@ -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
> 

Hello Sascha,  

I'll proceed with reverting the duplicated defines and ignoring the checkpatch warning
for now.  

Thank you for pointing this out.  

Best regards,  
Jeff  

RE: [EXT] Re: [PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.

[Jeff Chen]

> -----Original Message-----
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Thursday, March 20, 2025 12:29 AM
> To: Jeff Chen <jeff.chen_1@nxp.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.org; johannes@sipsolutions.net; francesco@dolcini.it;
> Pete Hsieh <tsung-hsien.hsieh@nxp.com>; s.hauer@pengutronix.de
> Subject: [EXT] Re: [PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF
> calibration data.
> 
> 
> 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

Hello Francesco,  

Thank you for reviewing the patch. You are correct-the Fixes tag I included was incorrect.
After re-examining the issue, I found that the premature release of RF calibration data 
cannot be reproduced, which invalidates the problem statement for this patch.  

I have decided to withdraw the patch. I appreciate your feedback and attention to detail, 
which helped identify this oversight.  

Best regards,  
Jeff

Re: [EXT] Re: [PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.

[Johannes Berg]

On Tue, 2025-03-25 at 16:43 +0000, Jeff Chen wrote:
> 
> 
> I have decided to withdraw the patch. I appreciate your feedback and attention to detail, 
> which helped identify this oversight.  
> 

This goes for _everyone_ on this thread... I applied this patch a long
time ago. Whatever you need to fix, you need to send new patches.

And I guess next time I'm not going to apply any patches for mwifiex
however innocent they look ... thus making the situation of that driver
even worse than it is now.

So please get together and form a plan on how to maintain it.

johannes

Re: [EXT] Re: [PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.

[Brian Norris]

On Tue, Mar 25, 2025 at 05:45:10PM +0100, Johannes Berg wrote:
> On Tue, 2025-03-25 at 16:43 +0000, Jeff Chen wrote:
> > 
> > 
> > I have decided to withdraw the patch. I appreciate your feedback and attention to detail, 
> > which helped identify this oversight.  
> > 
> 
> This goes for _everyone_ on this thread... I applied this patch a long
> time ago. Whatever you need to fix, you need to send new patches.

If it needs withdrawn, I suppose Jeff should send a revert patch then.

> And I guess next time I'm not going to apply any patches for mwifiex
> however innocent they look ... thus making the situation of that driver
> even worse than it is now.
> 
> So please get together and form a plan on how to maintain it.

My 2 cents:

 * Technically, I'm listed as maintainer still. I'm not always prompt,
   but I try to eventually get around to stuff (or at least see that
   Francesco reviews). I believe the previous implicit agreement would
   be that the wireless-drivers maintainer would wait for an Ack from
   sub-maintainer(s) before applying, unless they were truly trivial. I
   don't require that, of course, if you'd like to take things on your
   own Johannes, but that was my previous understanding.
 * I'm also used to seeing email replies when patches get applied. Kalle
   used to do that (presumably from some kind of push-time automation?),
   but I see you don't. You're of course free to do this however works
   best for you, but I find such emails useful for all interested
   parties (authors, reviewers, etc.). For example, if I thought the
   patches were controversial and were on my ToDo list, I'd probably
   speak up sooner.

Brian

Re: [EXT] Re: [PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.

[Francesco Dolcini]

On Tue, Mar 25, 2025 at 04:43:33PM +0000, Jeff Chen wrote:
> From: Francesco Dolcini <francesco@dolcini.it>
> > 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?
> 
> Thank you for reviewing the patch. You are correct-the Fixes tag I included was incorrect.
> After re-examining the issue, I found that the premature release of RF calibration data 
> cannot be reproduced, which invalidates the problem statement for this patch.  
> 
> I have decided to withdraw the patch. I appreciate your feedback and attention to detail, 
> which helped identify this oversight.  

To me the code change is correct, and it is also merged in wireless-next. No
reason to drop it because of my comment on the fixes tag.

Brian: are you ok with that?

Francesco

Re: [EXT] Re: [PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.

[Brian Norris]

On Wed, Mar 26, 2025 at 01:18:10PM +0100, Francesco Dolcini wrote:
> On Tue, Mar 25, 2025 at 04:43:33PM +0000, Jeff Chen wrote:
> > From: Francesco Dolcini <francesco@dolcini.it>
> > > 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?
> > 
> > Thank you for reviewing the patch. You are correct-the Fixes tag I included was incorrect.
> > After re-examining the issue, I found that the premature release of RF calibration data 
> > cannot be reproduced, which invalidates the problem statement for this patch.  
> > 
> > I have decided to withdraw the patch. I appreciate your feedback and attention to detail, 
> > which helped identify this oversight.  
> 
> To me the code change is correct, and it is also merged in wireless-next. No
> reason to drop it because of my comment on the fixes tag.
> 
> Brian: are you ok with that?

Oh, sorry, I don't think I really analyzed the nature of the reasons for
"withdrawal". Yes, if it's just the Fixes tag, then reverting isn't even
that helpful. I'm fine with keeping it as-is.

Brian

RE: [EXT] Re: [PATCH v4 1/2] wifi: mwifiex: Fix premature release of RF calibration data.

[Jeff Chen]

> -----Original Message-----
> From: Brian Norris <briannorris@chromium.org>
> Sent: Wednesday, March 26, 2025 11:23 PM
> 
> On Wed, Mar 26, 2025 at 01:18:10PM +0100, Francesco Dolcini wrote:
> > On Tue, Mar 25, 2025 at 04:43:33PM +0000, Jeff Chen wrote:
> > > From: Francesco Dolcini <francesco@dolcini.it>
> > > > 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?
> > >
> > > Thank you for reviewing the patch. You are correct-the Fixes tag I included
> was incorrect.
> > > After re-examining the issue, I found that the premature release of
> > > RF calibration data cannot be reproduced, which invalidates the problem
> statement for this patch.
> > >
> > > I have decided to withdraw the patch. I appreciate your feedback and
> > > attention to detail, which helped identify this oversight.
> >
> > To me the code change is correct, and it is also merged in
> > wireless-next. No reason to drop it because of my comment on the fixes tag.
> >
> > Brian: are you ok with that?
> 
> Oh, sorry, I don't think I really analyzed the nature of the reasons for
> "withdrawal". Yes, if it's just the Fixes tag, then reverting isn't even that helpful.
> I'm fine with keeping it as-is.
> 
> Brian

Hi Johannes, Francesco, Brian, and all,
 
Thank you for your feedback and for bringing attention to this discussion. 
Moving forward, I will work on improving the process to avoid similar issues and
ensure better maintenance of the `mwifiex` driver.
Thank you for your understanding and support.

Best regards,  
Jeff