linux-kernel - Re: [PATCH v7 12/22] wifi: nxpwifi: introduce command and event handling infrastructure#

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aSXFYfWo3fade3Dg@pengutronix.de>
Date: Tue, 25 Nov 2025 16:04:01 +0100
From: Sascha Hauer <s.hauer@...gutronix.de>
To: Jeff Chen <jeff.chen_1@....com>
Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org,
	briannorris@...omium.org, johannes@...solutions.net,
	francesco@...cini.it, tsung-hsien.hsieh@....com
Subject: Re: [PATCH v7 12/22] wifi: nxpwifi: introduce command and event
 handling infrastructure#

Hi Jeff,

On Mon, Nov 17, 2025 at 07:00:36PM +0800, Jeff Chen wrote:
 +/* This function handles the command response.
> + *
> + * After processing, the function cleans the command node and puts
> + * it back to the command free queue.
> + */
> +int nxpwifi_process_cmdresp(struct nxpwifi_adapter *adapter)
> +{
> +	struct host_cmd_ds_command *resp;
> +	struct nxpwifi_private *priv =
> +		nxpwifi_get_priv(adapter, NXPWIFI_BSS_ROLE_ANY);
> +	int ret = 0;
> +	u16 orig_cmdresp_no;
> +	u16 cmdresp_no;
> +	u16 cmdresp_result;
> +
> +	if (!adapter->curr_cmd || !adapter->curr_cmd->resp_skb) {
> +		resp = (struct host_cmd_ds_command *)adapter->upld_buf;
> +		nxpwifi_dbg(adapter, ERROR,
> +			    "CMD_RESP: NULL curr_cmd, %#x\n",
> +			    le16_to_cpu(resp->command));
> +		return -EINVAL;
> +	}
> +
> +	resp = (struct host_cmd_ds_command *)adapter->curr_cmd->resp_skb->data;
> +	orig_cmdresp_no = le16_to_cpu(resp->command);
> +	cmdresp_no = (orig_cmdresp_no & HOST_CMD_ID_MASK);
> +
> +	if (adapter->curr_cmd->cmd_no != cmdresp_no) {
> +		nxpwifi_dbg(adapter, ERROR,
> +			    "cmdresp error: cmd=0x%x cmd_resp=0x%x\n",
> +			    adapter->curr_cmd->cmd_no, cmdresp_no);
> +		return -EINVAL;
> +	}
> +	/* Now we got response from FW, cancel the command timer */
> +	timer_delete_sync(&adapter->cmd_timer);
> +	clear_bit(NXPWIFI_IS_CMD_TIMEDOUT, &adapter->work_flags);
> +
> +	if (adapter->curr_cmd->cmd_flag & CMD_F_HOSTCMD) {
> +		/* Copy original response back to response buffer */
> +		struct nxpwifi_ds_misc_cmd *hostcmd;
> +		u16 size = le16_to_cpu(resp->size);
> +
> +		nxpwifi_dbg(adapter, INFO,
> +			    "info: host cmd resp size = %d\n", size);
> +		size = min_t(u16, size, NXPWIFI_SIZE_OF_CMD_BUFFER);
> +		if (adapter->curr_cmd->data_buf) {
> +			hostcmd = adapter->curr_cmd->data_buf;
> +			hostcmd->len = size;
> +			memcpy(hostcmd->cmd, resp, size);
> +		}
> +	}
> +
> +	/* Get BSS number and corresponding priv */
> +	priv = nxpwifi_get_priv_by_id
> +	       (adapter, HOST_GET_BSS_NO(le16_to_cpu(resp->seq_num)),
> +		HOST_GET_BSS_TYPE(le16_to_cpu(resp->seq_num)));
> +	if (!priv)
> +		priv = nxpwifi_get_priv(adapter, NXPWIFI_BSS_ROLE_ANY);
> +	/* Clear RET_BIT from HOST */
> +	resp->command = cpu_to_le16(orig_cmdresp_no & HOST_CMD_ID_MASK);
> +
> +	cmdresp_no = le16_to_cpu(resp->command);
> +	cmdresp_result = le16_to_cpu(resp->result);
> +
> +	/* Save the last command response to debug log */
> +	adapter->dbg.last_cmd_resp_index =
> +			(adapter->dbg.last_cmd_resp_index + 1) % DBG_CMD_NUM;
> +	adapter->dbg.last_cmd_resp_id[adapter->dbg.last_cmd_resp_index] =
> +								orig_cmdresp_no;
> +
> +	nxpwifi_dbg(adapter, CMD,
> +		    "cmd: CMD_RESP: 0x%x, result %d, len %d, seqno 0x%x\n",
> +		    orig_cmdresp_no, cmdresp_result,
> +		    le16_to_cpu(resp->size), le16_to_cpu(resp->seq_num));
> +	nxpwifi_dbg_dump(adapter, CMD_D, "CMD_RESP buffer:", resp,
> +			 le16_to_cpu(resp->size));
> +
> +	if (!(orig_cmdresp_no & HOST_RET_BIT)) {
> +		nxpwifi_dbg(adapter, ERROR, "CMD_RESP: invalid cmd resp\n");
> +		if (adapter->curr_cmd->wait_q_enabled)
> +			adapter->cmd_wait_q.status = -1;
> +
> +		nxpwifi_recycle_cmd_node(adapter, adapter->curr_cmd);
> +		spin_lock_bh(&adapter->nxpwifi_cmd_lock);
> +		adapter->curr_cmd = NULL;
> +		spin_unlock_bh(&adapter->nxpwifi_cmd_lock);
> +		return -EINVAL;
> +	}
> +
> +	if (adapter->curr_cmd->cmd_flag & CMD_F_HOSTCMD) {
> +		adapter->curr_cmd->cmd_flag &= ~CMD_F_HOSTCMD;
> +		if (cmdresp_result == HOST_RESULT_OK &&
> +		    cmdresp_no == HOST_CMD_802_11_HS_CFG_ENH)
> +			ret = nxpwifi_ret_802_11_hs_cfg(priv, resp);
> +	} else {
> +		if (resp->result != HOST_RESULT_OK) {
> +			nxpwifi_process_cmdresp_error(priv, resp);
> +			return -EFAULT;
> +		}
> +		if (adapter->curr_cmd->cmd_resp) {
> +			void *data_buf = adapter->curr_cmd->data_buf;
> +
> +			ret = adapter->curr_cmd->cmd_resp(priv, resp,
> +							  cmdresp_no,
> +							  data_buf);
> +		}
> +	}
> +
> +	/* Check init command response */
> +	if (adapter->hw_status == NXPWIFI_HW_STATUS_INITIALIZING) {
> +		if (ret) {
> +			nxpwifi_dbg(adapter, ERROR,
> +				    "%s: cmd %#x failed during\t"
> +				    "initialization\n", __func__, cmdresp_no);
> +			nxpwifi_init_fw_complete(adapter);
> +			return ret;
> +		} else if (adapter->last_init_cmd == cmdresp_no) {
> +			adapter->hw_status = NXPWIFI_HW_STATUS_INIT_DONE;
> +		}
> +	}

If this driver now goes upstream could you have a look at the cleanup
patches I sent for the mwifiex driver and see which ones apply here as
well?

At least this asynchronous initialization seems entirely unnecessary,
see:

https://lore.kernel.org/all/20241202-mwifiex-cleanup-1-v3-12-317a6ce0dd5b@pengutronix.de/

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 |